[aviggiano]

Problem

Bugs 1 and 2 were found while fixing #1141 (#1169). A reviewer of this PR found bug 3. All three reproduce on the wave-1 base (origin/integration/wave1, a48ad9c3).

1. A run with a model fan-out node can be synchronized only once. Take a topology that fans a node out over two model profiles. After one syncRun, the next one fails with WORKFLOW_CONTROL_EVIDENCE_INVALID: "workflow control completeness binding could not be re-derived: run state node set does not exactly match the sealed graph". status (getRunHealth) reports the same message as WORKFLOW_CONTROL_EVIDENCE_DIVERGED and skips synchronization (WORKFLOW_STATE_SYNC_SKIPPED), so the run's local state stops advancing.
   • By code reading, every other reader that does not tolerate divergence hits the same error:
     • stats, why, timeline, snapshots, node, replay and fork fail;
     • inspect turns the error into warnings and omits the runner's view.
   • resume is not affected. An ad-hoc run (below) resumed such a run with --retry-failed after its first sync.
2. A fan-out aggregate stays skipped after its attempts reopen and succeed. When one discovery attempt fails, Smithers skips both analysis attempts that consume it. After resume --retry-failed (which reopens them since fix(runtime): resume --retry-failed reopens descendants skipped behind a recovered producer #1169), both run and succeed, but the signal-analysis aggregate stays skipped. It is a blocking node, so the recovered run reports failed, although every attempt succeeded and the runner reports the run succeeded.
3. A private eval fails once its run expands a dynamic group. After eval run returns, the private Modal worker reads the run to decide whether model work began (privateEvalModelWorkEvidence). That reader threw "private eval run state must exactly match its planned graph nodes" unless the keys of state.json equaled the node IDs of graph.json.
   • Synchronization records a generated dynamic node under its storage ID (dynamic-<group>-<digest>), which graph.json does not name. So the reader threw for every run whose dynamic group had expanded.
   • runWithPrivateModelWorkCorroboration rethrows that error, so the worker attempt fails although eval run succeeded. By code reading, a private launch then relaunches the worker, because model work stays flagged.

Reach.

• Bugs 1 and 2 need a custom topology in which a node, static or dynamic, fans out over two or more profiles. Each shipped topology's model_profiles list has one entry.
• Bug 3 needs only an expanded dynamic group. The private worker configures its target with the default audit profile, whose threat-goals and class-goals lanes expand from goal-plan's output. So by code reading, a private eval hits it whenever goal-plan plans a goal, and has done so since Release/v0.1.0 #744 introduced storage IDs. I did not run it on Modal.

Root cause

1. The checker and the writer disagreed on which keys state.json may hold.
   • The writer: ensureWorkflowTaskStateRecords (workflow-sync.ts) adds one state.json record per model attempt, keyed by attempt ID (project-discovery__model_1__attempt_1), beside the concrete node's aggregate record. A generated dynamic node's records are keyed by its storage ID.
   • The checker: every read of the control evidence re-derives the completeness binding. For a static run, deriveWorkflowControlBindings (workflow-integrity.ts) required the keys of state.json to equal the node IDs of graph.json exactly. Its dynamic branch admitted attempt and storage IDs; the static branch did not.
   • The check never fed the binding: expected_state_node_ids is derived from graph.json. It was a second copy of the writer's key rule, and it had already drifted from the writer once, when Archive dynamic expansion generations before producer retry #1064 had to prune archived dynamic records from state.json to get past it.
2. The aggregate had its own stickiness rule. The aggregate loop in synchronizeTasks kept any terminal aggregate status unless it went failed → succeeded. The guard was added in feat: add strict producer-visible JSON contracts with release-lane fix #558 and widened in fix(runtime): reconcile recovered terminal run status (#578) #603.
   • Each attempt's record already applies the supersession rules: newer Smithers evidence replaces it (workflowEvidenceSupersedesPrevious), and success and invalid-output dispositions are immutable (immutableTerminalFinalization).
   • The aggregate is computed from those records, so the guard could only make it disagree with its own attempts.
3. The Modal reader kept a third copy of the same key rule. private-model-work.ts got its exact comparison in feat: add strict producer-visible JSON contracts with release-lane fix #558. Fan-out attempt IDs were never graph node IDs, and since Release/v0.1.0 #744 generated dynamic nodes are recorded under storage IDs. I found no other reader that compares the two key sets; the CLI's run-statistics reader already accepts attempt keys.

Change

Source is +12/−47 lines. 4 of the added lines are a rewritten docstring and 3 are comments.

• workflow-integrity.ts
  • Deleted the run-state node-set comparison from deriveWorkflowControlBindings, both the static and the dynamic branch.
  • Deleted the runtime node-ID list that verifyWorkflowControlSnapshot computed only to feed the dynamic branch. verifyDynamicRuntimeMaterialization still runs; only its unused return value is dropped.
  • The run state's run ID, graph fingerprint and config fingerprint checks stay.
  • A one-line comment where the binding is returned says that expected_state_node_ids is the sealed graph's node set and that run-state keys are deliberately not checked. The field is not renamed, because that would change the seal schema.
• workflow-sync.ts: deleted the aggregate terminal guard. The aggregate is now always aggregateAttemptStatuses over its attempts' current records.
• dynamic-expansion-retry.ts: the archiveDynamicExpansionsForRetry docstring said the archived generation's records are pruned because the admission check refuses them. That is no longer true, so it now gives the reason that remains: the synchronizer creates a record only for an attempt that has none and never re-finalizes a successful one. Without the prune, a regenerated attempt that reuses a storage ID would start from the archived record.
• packages/modal/src/private-model-work.ts: deleted the exact key-set throw.
  • The model-work scan now reads every record except those of the graph's model-free nodes, so attempt and storage records count as model work.
  • That is sound because only agentic nodes compile to tasks, so only they get attempt and storage records.
• Tests
  • Two new tests in runtime.test.ts, one per runtime bug.
  • The dynamic admission test in dynamic-lifecycle.test.ts loses its state-injection case, which asserted the deleted rule. It is renamed to "…rejects graph and task extensions not derived from sealed controls".
  • One new test in packages/modal/test/private-model-work.test.ts covers a fan-out run whose attempt records sit beside the node's record. With every record pending, the reader returns none; with one attempt running, it returns started.

Deliberately not built

• Teaching the static branch to admit attempt IDs. That would fix the symptom by keeping two copies of the writer's key rule in sync by hand, which is what failed here and in Archive dynamic expansion generations before producer retry #1064.
• Deleting pruneArchivedRunStateNodes. Its original reason was this check, but it still has one. Without the prune, a regenerated attempt that reuses a storage ID would start from the archived record, and an archived success, which is immutable, would stand in for the new attempt's output.
• A seal or binding change. expected_state_node_ids stays derived from graph.json, so existing seals verify unchanged.
• Stamping the aggregate's finished_at. Fan-out aggregates carry none, on the base and on this branch: the record is created without one, and the aggregate patch passes no evidence timestamp. Modal's terminal-disposition reader requires one on a terminal aggregate. That matters only for a model fan-out run on Modal, and evals run one model. Left for a follow-up.
• Returning unknown for a missing model-node record in the Modal reader, as a reviewer suggested. The reader now treats a missing record as no evidence.
  • Planning creates a record for every planned node, and synchronization removes only archived dynamic records, so a record goes missing only by hand edit. The runtime no longer refuses hand-edited keys either.
  • A generated node has no record under its graph ID, so the rule would also have to know storage IDs, which would be another copy of the writer's key rule.
• A dynamic fan-out or real-engine test for the aggregate.
  • The dynamic storage-ID aggregate goes through the same loop, so the static test runs the changed lines.
  • Reopening skipped attempts in the engine is fix(runtime): resume --retry-failed reopens descendants skipped behind a recovered producer #1169's change; this PR only changes how synchronization projects them.
• Letting a newer occurrence supersede an invalid-output verdict after resume --retry-failed or --reset-node. This is a separate, pre-existing gap in immutableTerminalFinalization, which this PR leaves unchanged. It is recorded for its own follow-up.

Verification

Discriminating runs (runtime). Each source variant was compiled into its own dist-test copy with the same final test files, and the tests were run by name:

Source	syncRun records model fan-out attempts independently (existing)	syncRun synchronizes a model fan-out run again after recording its attempts (new)	syncRun lets a model fan-out aggregate follow its reopened skipped attempts to success (new)
wave-1 base	passes	fails: status returns WORKFLOW_CONTROL_EVIDENCE_DIVERGED ("…run state node set does not exactly match the sealed graph") and WORKFLOW_STATE_SYNC_SKIPPED	fails: the second syncRun returns WORKFLOW_CONTROL_EVIDENCE_INVALID with the same message
check deleted, guard kept	passes	passes	fails: signal-analysis is skipped. Instrumented, the run status is failed while project-discovery is succeeded
this branch	passes	passes	passes

• An earlier version of the first new test had no status step. It failed on the base at the second syncRun, with WORKFLOW_CONTROL_EVIDENCE_INVALID and the same message.
• Ad hoc, not committed: a failed fan-out run was synchronized once, then resumeRun({ retryFailed: true }) was called. It returned ok on all three variants and issued timetravel … --node-id node:project-discovery__model_1__attempt_1 … --no-deps. So resume was never affected by bug 1.

Discriminating runs (Modal reader, bug 3).

• The new test fails on the unfixed reader with "private eval run state must exactly match its planned graph nodes". It passes on this branch.
• A variant that deletes the throw but still scans only the graph's model nodes fails the test's second assertion: it returns none instead of started.
• Ad hoc, not committed: the reader ran on state that the runtime wrote in two dynamic-lifecycle fixtures: a single-model dynamic child that succeeded, and a model fan-out dynamic group with pending attempts. This PR does not change that writer. The unfixed reader threw in both. This branch returns started in both, from the planner's record.

Also run on this branch:

• Supporting runtime test files: dynamic-lifecycle, dynamic-expansion, observation-snapshot, workflow-control, terminal-report-completion and workflow-completion-policy, 73/73 pass. dynamic-lifecycle drives the dynamic branch of the deleted check, including the Archive dynamic expansion generations before producer retry #1064 retry-and-re-expand test.
  • These ran on the previous head. On this head, whose runtime code differs from it only by one comment line, dynamic-lifecycle and workflow-control were re-run, 34/34 pass, and so were the three fan-out tests in the table, 3/3 pass.
• Modal test files: private-model-work, terminal-disposition and worker-result, 63/63 pass.
• runtime.test.ts sweep of every test whose name starts with syncRun, resume, ordinary resume, native continuation, controller refresh, status, getRunHealth, an unobserved, observe, pause or cancel. It ran in the 4 release shards on the previous head, which had the same runtime code apart from one comment line: 137 run, 134 pass, 3 fail, none caused by this PR.
  • Fail on the base independently of this PR: status keeps reporting runner health when run-state synchronization fails and native continuation hands generated agents the run's TOML config, so CodexAgent keeps API-key auth. Both are wave-1 merge interactions, fixed separately.
  • Load flake: syncRun leaves a Kimi model unpriced when Moonshot does not list it failed during startRun with "workflow execution file … changed while reading". It passes when run alone on that head.
• Static checks: all pass.
  • npx prettier --check and npx eslint on the changed files
  • CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/integration/wave1 pnpm -w lint:strict:ci
  • pnpm -w lint, pnpm -w knip and pnpm -w docs:check
  • pnpm --filter @ultrafuzz/runtime typecheck and pnpm --filter @ultrafuzz/modal typecheck
• Complexity ceiling: unchanged. The repository maximum is still synchronizeLinkedWorkflowRun at 90, which this PR does not touch. After the change, synchronizeTasks measures 68 and verifyWorkflowControlSnapshot 49.

Not run:

• the rest of runtime.test.ts, the other runtime test files, the rest of the Modal suite, and the CLI suite;
• a real campaign or a private eval on Modal;
• the real Smithers engine;
• dynamic fan-out on the aggregate path.

Risk / compatibility

• Hand-edited state.json keys are no longer refused.
  • A record added under an unknown key is kept, and it shows up wherever run state is listed.
  • A missing task-attempt or aggregate record is recreated by the next sync.
  • The check never caught edited statuses, so it was not an integrity boundary for this document.
• Aggregate display during a retry. While a reset attempt waits or runs again, its aggregate reads pending or running instead of keeping failed, as a single-model node already does. The run status does not change: while the runner reports the run active, finalRunStatus returns running before it looks at node statuses.
• The Modal model-work flag.
  • The reader now counts every record the graph does not name as model work. The worker lowers its flag only when the reader returns none, so an unexpected record can only keep the flag raised.
  • Where state.json keys equal graph.json node IDs, which is every state the old reader accepted, the result is unchanged.
• In-flight runs. Nothing changes in documents, schemas or seals. A run that already hit bug 1 synchronizes again after the upgrade, because the failing passes stopped before writing anything.
• Overlap with other wave-2 items.
  • workflow-sync.ts: one 8-line hunk, the aggregate guard.
  • workflow-integrity.ts: hunks only in verifyWorkflowControlSnapshot and deriveWorkflowControlBindings.
  • runtime.test.ts: two tests inserted after syncRun records model fan-out attempts independently.
  • packages/modal/src/private-model-work.ts and its test: no other wave-2 branch touches them.

Changelog entry

A run whose topology fans a node out over several model profiles no longer stops synchronizing after its first pass. Before, status reported the run's controls as diverged and skipped synchronization, and commands that read the run strictly, such as stats, why and timeline, failed with "run state node set does not exactly match the sealed graph". A fan-out node whose skipped attempts reopen and succeed after resume --retry-failed now reports succeeded instead of staying skipped, so the recovered run no longer ends failed. A private eval on Modal no longer fails after eval run returns when its run expanded a dynamic group, as the default topology's goal lanes do. Before, the worker failed with "private eval run state must exactly match its planned graph nodes".

Refs #1141.

🤖 Generated with Claude Code

Confidence Score: 4/5

The PR appears safe to merge; the remaining concern is non-blocking test coverage for expanded dynamic groups.

Findings

1.  Dynamic path lacks test coverage ▶

Fix with agent prompt

### Issue 1
packages/modal/test/private-model-work.test.ts:72
This test adds model attempt records, but the reported private-eval failure involves records keyed by a generated dynamic node’s storage ID. Without a test using an expanded dynamic group and its storage-keyed records, the test could still pass if that part of the fix regresses.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR removes a run-state key-set check that rejected legitimate fan-out and dynamic records, recalculates fan-out aggregates from current attempts, and lets the private-eval reader recognize records outside the planned graph’s node IDs.

• Runtime tests cover repeated fan-out synchronization and recovery of a skipped aggregate.
• Modal tests cover attempt-keyed records, but not the dynamic storage-keyed path.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Sealed graph and task manifest] --> B[Workflow synchronization]
  B --> C[Run state: graph nodes, attempts, dynamic storage IDs]
  C --> D[Fan-out aggregate from attempt statuses]
  C --> E[Private-eval model-work evidence]

Loading

_{Reviews (1) · Last reviewed commit: "fix(modal): read private-eval model work..."}

[greptile-apps]

Dynamic path lacks test coverage This test adds model attempt records, but the reported private-eval failure involves records keyed by a generated dynamic node’s storage ID. Without a test using an expanded dynamic group and its storage-keyed records, the test could still pass if that part of the fix regresses.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/modal/test/private-model-work.test.ts
Line: 72

Comment:
**Dynamic path lacks test coverage** This test adds model attempt records, but the reported private-eval failure involves records keyed by a generated dynamic node’s storage ID. Without a test using an expanded dynamic group and its storage-keyed records, the test could still pass if that part of the fix regresses.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!