Skip to content

fix(dynamic-workflow): deterministic wait polling and checkpoint lineage in repair reuse - #44

Open
modacker wants to merge 1 commit into
MiniMax-AI:mainfrom
modacker:community/repair-reuse-fixes
Open

modacker wants to merge 1 commit into
MiniMax-AI:mainfrom
modacker:community/repair-reuse-fixes

Conversation

@modacker

@modacker modacker commented Sep 17, 2026

Copy link
Copy Markdown

What changes

Three small correctness fixes for plugins/hetaoBackend/mcode-dynamic-workflows (merged via #42), split out so each is reviewable on its own.

  1. Deterministic review-loop polling (checks/workspace-router.check.mjs): the wait loop called workflow_wait without afterSequence. waitEvents returns immediately whenever any event exists after the cursor (src/tools.mjs), and with after=0 events always exist — so all 30 iterations burned in ~65 ms on a warm machine while the run needed ~66 ms. The outcome is decided by IPC latency: we reproduced both a failure and a pass on the same machine at the same commit. The loop now advances status.nextSequence, so each call blocks up to 500 ms for a genuinely new event; the worst case stays bounded (30 × 500 ms).

  2. Checkpoint lineage in repair reuse (src/engine.mjs): the candidate lineage check required every dependency to carry reusedFrom.runId === sourceRunId. Checkpoints recompute every run by design and never carry reusedFrom, so any agent step with a checkpoint in dependsOn silently fell back to a fresh (billable) call even when the candidate was valid. Checkpoint dependencies now accept the recomputed step when its requestHash matches the source run's checkpoint; a changed checkpoint value still invalidates downstream reuse. A regression test covers both directions.

  3. Dead code (src/engine.mjs): removed the second byRequest(requestId) lookup in start() — unreachable, since the first lookup returns for any existing request and nothing writes the store in between (no await point). For the record: we initially also flagged the trailing re-check in resume() as redundant — wrong; it is the TOCTOU guard after the fingerprints await, and the "simultaneous resumes launch only one replay" test caught our mistake before this PR was filed.

dist/main.mjs is rebuilt from the changed engine; the build remains byte-reproducible.

Verification

  • node --test checks/*.check.mjs: 73/73 (72 existing + 1 new checkpoint-lineage test)
  • node --test test/package.test.mjs: pass
  • Repository npm run check (clean checkout, plugin node_modules removed): green
  • workspace-router check after the fix: 3/3 runs pass (previously intermittent)
  • macOS arm64, Node v26.7.0, based on the plugin as merged in Add Dynamic Workflow: visual review, execution, and script repair #42

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

- workspace-router check advances the workflow_wait event cursor so the
  polling loop blocks on new events instead of racing run completion
- repair reuse accepts checkpoint dependencies while the recomputed value
  hash matches the source run, and still invalidates on value changes
- drop the unreachable second requestId lookup in start()
- dist/main.mjs rebuilt from the changed engine (byte-reproducible build)
@modacker

Copy link
Copy Markdown
Author

Related design discussion for the repair-reuse machinery in this PR: #46 (tamper-evident reuse results). No action needed on this PR — the issue is a forward-looking proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant