feat(topology)!: a failed property lens no longer skips the rest of the campaign - #1198
Conversation
75b5318 to
b0beac4
Compare
b0beac4 to
b3808c2
Compare
|
|
||
| const dependencyStatus = state.nodes[attemptId]?.status; | ||
| const finalizedSuccess = dependencyStatus !== undefined && NODE_RECOVERED_STATUSES.has(dependencyStatus); | ||
| if (finalizedSuccess && !admitted.has(attemptId) && verifiedAfterAdmission(attemptId)) continue; |
There was a problem hiding this comment.
Incomplete catalog can appear complete When
resume --retry-failed successfully reruns a property lens after the fan-in ran without it, this exception keeps the fan-in’s earlier result. The lens then counts as succeeded, so the report can say COMPLETE and a --require-complete run can succeed even though the catalog and downstream analysis never used that lens’s properties. The retry needs to preserve the partial outcome or rerun the work that used the incomplete catalog.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/workflow-sync.ts
Line: 3737
Comment:
**Incomplete catalog can appear complete** When `resume --retry-failed` successfully reruns a property lens after the fan-in ran without it, this exception keeps the fan-in’s earlier result. The lens then counts as succeeded, so the report can say COMPLETE and a `--require-complete` run can succeed even though the catalog and downstream analysis never used that lens’s properties. The retry needs to preserve the partial outcome or rerun the work that used the incomplete catalog.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| optionalDependencyArtifactDirs: template.optionalDependencyArtifactDirs.map((directory) => | ||
| remapProjectPath(directory, input.group.promptContext.projectRoot, input.projectRoot) | ||
| ) |
There was a problem hiding this comment.
Relocation remapping lacks coverage The new test materializes generated children under the same project root used to compile their templates. It therefore cannot catch an optional dependency path that still points to the old root after relocation, even when the required dependency path points to the new one. Test with distinct roots and assert that both paths identify the same producer.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/dynamic-runtime.ts
Line: 276-278
Comment:
**Relocation remapping lacks coverage** The new test materializes generated children under the same project root used to compile their templates. It therefore cannot catch an optional dependency path that still points to the old root after relocation, even when the required dependency path points to the new one. Test with distinct roots and assert that both paths identify the same producer.
---
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!
eb783f1 to
3e73bf1
Compare
| const previous = | ||
| cached ?? (attempt > 1 ? readFinalReportSelections(task).filter((selection) => selection.attempt < attempt) : []); |
There was a problem hiding this comment.
Old attempts enter report history After a reset reuses an attempt number, a report attempt that fails before reaching
generate leaves its old selection in this file. When a later attempt runs, this filter includes that selection as if it belonged to the current round. The report and a restarted verifier then agree on failed_attempts that include a model attempt that did not execute in this round, allowing incorrect history in a verified report.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/templates/smithers/workflows/workflow.tsx
Line: 2235-2236
Comment:
**Old attempts enter report history** After a reset reuses an attempt number, a report attempt that fails before reaching `generate` leaves its old selection in this file. When a later attempt runs, this filter includes that selection as if it belonged to the current round. The report and a restarted verifier then agree on `failed_attempts` that include a model attempt that did not execute in this round, allowing incorrect history in a verified report.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (cloudCleanup !== undefined) { | ||
| return runtimeFailure([cloudCleanup]); | ||
| } | ||
| try { |
There was a problem hiding this comment.
Cloud cleanup leaves resources behind When cleaning a run created with the former per-node Modal execution, this path now deletes the local run directory without terminating its sandboxes or deleting its remote volume. If that run left cloud resources behind,
clean can report success while they remain billable and remove the run evidence needed to identify them for manual cleanup.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/clean.ts
Line: 85
Comment:
**Cloud cleanup leaves resources behind** When cleaning a run created with the former per-node Modal execution, this path now deletes the local run directory without terminating its sandboxes or deleting its remote volume. If that run left cloud resources behind, `clean` can report success while they remain billable and remove the run evidence needed to identify them for manual cleanup.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…not only review #1120 made a failure_policy: continue producer's output optional only to consumers in the group literally named `review`, in both the compiler and dynamic lowering. Any other consumer that reconciles partial results, such as a property fan-in or the exhaustive dynamic strategy generator, was skipped as soon as one continuing input failed. reconcilesPartialResults now takes the producer's group: a continuing producer's output is optional to a consumer in any other group and stays required inside its own group. Chains inside one group, such as the stateful invariant stages, therefore still never run without their predecessor, which is the case #1120 protected. The compiler and dynamic lowering both call the helper. The launch task-manifest gate checks only that every optional input names a continuing producer, so it is unchanged. With the shipped topologies as they are, one thing changes: in the exhaustive profile dynamic-strategy-generator (specialists) runs with the strategy attempts that succeeded instead of being skipped when any of its 58 strategy inputs fails. It reads them through the verifier-admitted ancestor authorities, which omit a failed producer. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… not admit
When the host re-verifies a finalized property fan-in (semanticPropertyLenses
and verifyLensReferenceExpectationPreservation, both through
sealedDirectArtifactDependencies), it loaded every declared lens dependency,
including an optional one that the in-engine verifier left out because it
failed. A fan-in that correctly consolidated only the verified lenses was then
rejected on the host with PROPERTY_LENS_AUTHORITY_INVALID ("has no current
finalized output authority").
sealedDirectArtifactDependencies now drops an optional producer that is absent
from the verifier-persisted admission, using the same
optionalDeclaredProducerWasNotAdmitted check that
finalizedDeclaredContractProducers and the in-engine gate already apply.
Required lenses, and optional lenses the verifier admitted, are still loaded
and authenticated. A catalog that cites a property from the omitted lens is
still rejected by the property-source-join gate as an unknown source.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…he campaign In the shipped topologies behind the default, low-cost, exhaustive and invariant-only audit profiles, the eight property lenses shared the halting `properties` group with the property fan-in. One lens that ended in a terminal failure failed the workflow: the fan-in was skipped and no strategy, stateful stage or review node ran, so the campaign ended without a report. The `properties` group now uses failure_policy: continue, and the fan-in moves to its own halting `property-catalog` group. With the previous two commits a failed lens is then an optional input to every later node: the fan-in consolidates the lenses that passed verification, and the strategies, specialists and review run as before. The fan-in itself still halts, so no strategy runs without a property catalog. Leaving the fan-in inside a continuing `properties` group would have been worse: its own output would then be optional to the strategies. The fan-in prompt now says its lens authority lists only verified lenses and that a missing lens's properties must not be recreated. The existing property-source-join gate rejects a source the fan-in was not given. The docs describe the general continue rule instead of implying review is the only consumer of partial results. BREAKING CHANGE: in the shipped topologies a failed property lens no longer fails the campaign. Its properties are absent from the catalog, the lens is reported as a failed node, and under best-effort completion the run can still succeed with a PARTIAL report. `--require-complete` runs still end unsuccessful. The fan-in node is in the new `property-catalog` group. Projects initialized earlier keep their copied .ultrafuzz/topology.yml, and the old behaviour, until they apply the same two group edits. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ithout is retried successfully With the property lenses continuing, a lens failure no longer ends the run. When an interrupted run is resumed with --retry-failed, which Modal's durable resume always passes, the failed lens reruns while the strategies already admitted without it keep running. Once the retried lens finalized as succeeded, host finalization failed each of those strategies with "finalized optional dependency is missing from verifier admission". dedupe-findings had admitted those strategies in the engine, so it then failed the inverse check, which halts review and fails the run over bookkeeping. Base has the same race for a retried strategy and the review tasks admitted without it. The check now accepts that omission when the producer's verifier finished after the consumer's preparation started, ordered by the Smithers event timestamps the sync pass already reads. It still rejects the omission of a producer that verified before the admission began, which is the deleted-marker case it exists for. The artifact gates already treat the consumer's recorded admission as the authority. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ched through another group A same-group ancestor reached only through another group does not skip its dependent: the intermediate node runs, and the dependent fails its input admission instead. The topology reference now says so, and no longer calls continue a policy for optional branches. The reconcilesPartialResults comment is scoped to chains within one group, and the dependency-policy test names its reconciling node `join` so the old `review` name does not look significant. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…he campaign The packaged exhaustive and invariant-only topologies and new project scaffolds change on upgrade, while an existing project's .ultrafuzz/topology.yml keeps the old halting properties group until it is edited. The breaking-changes entry names the three edits and the prompt refresh, and states the general rule for custom topologies: a continuing group's results are optional to every other group, not only to review. The other-changes entry records the resume --retry-failed fix for a task admitted without an optional input that is later rerun successfully. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…etrying keeps its result `resume --retry-failed` reruns a failed continuing producer, such as a property lens, while the tasks admitted without it keep running. A sync pass during that rerun (Modal's `waitForTerminalRun` runs `ultrafuzz inspect` every 60 s) failed each of those tasks with "optional dependency is not terminal for verifier admission". That failure recovered on a later pass, but a dependent whose host gate reads the task's output, such as triage after dedupe, failed its gate in the same pass, recorded `terminal_disposition: task-output-validation-failure`, which is immutable, and left the run `failed` for good. `assertOptionalDependencyAuthoritiesCurrent` now accepts the omission of an optional producer that is not terminal. Tasks synchronize in dependency order and start only after their producers settle, so such a producer is being rerun and had no verified output when the task was admitted: if the rerun fails, the omission matches, and if it succeeds, it verified after the admission, which the retry exception added earlier in this PR accepts. A producer with no recorded state still fails the check, and so does a finalized success missing from an admission that began after it verified. A stricter rule that accepts only a rerun started after the consumer's admission would not fix the common order, where the resume starts the rerun first and the consumers are admitted while it runs. The new sync test reproduces the review finding: lens (continue) -> catalog -> dedupe (findings@2) -> triage (triaged-findings@1), with one sync while the lens is rerunning. Before this change triage ends failed and the run fails. Co-Authored-By: Claude Opus 5.5 <[email protected]>
`compileSmithersWorkflow` computed `optionalDependencyArtifactDirs` only for static tasks, so a dynamic group's task templates, and the children cloned from them, kept every inherited ancestor required. In a custom topology where a continuing producer in another group is an ancestor of a dynamic group, a failed producer let a static sibling run but failed every generated child's input admission, an `artifact-contract` failure that leaves the report unverified. That contradicted the rule the topology reference and changelog state, and main had the same gap for a dynamic group in `review`. The compiler now applies the same filter to the templates. A template without optional inputs keeps its bytes, so the packaged topologies, whose dynamic goal groups have only setup ancestors, compile exactly as before. Generated children remap their inherited optional directories the way they already remap `dependencyArtifactDirs`, so the two stay consistent in a relocated project. A run launched before this change keeps its sealed templates, so its dynamic lowering re-derives unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The fan-in moved from group `properties` to `property-catalog`, and `flowNodeType` keyed the property card on `group === "properties"`, so `/api/flow` reported the fan-in as `agentAttempt` and the frontend showed it as a one-attempt strategy card. `property-catalog` now maps to `propertySpecification` too, and the flow API test pins the fan-in's type on a freshly initialized project. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ustom topologies change Review follow-ups for the Unreleased entries and the reference docs: - The breaking entry is split in two, so the optional-input rule that affects every custom topology is its own bullet. That bullet now names nodes with no `group` and the nodes a dynamic group generates, and says a node reaching a failed same-group ancestor only through another group fails its admission as an `artifact-contract` failure, which leaves the report unverified. - The packaged-topology entry names the packaged `default` topology, qualifies "PARTIAL" with a successful `resume --retry-failed`, says what that resume reruns (the lens, and after a verifier failure every node that started after it), records that verifier failures of the fan-in, strategies and specialists lose `output_contracts.missing`, their terminal disposition and the public eval `failure_code`, offers `ultrafuzz topology copy default` for an uncustomized project, and applies the fan-in prompt refresh to every existing project, since a project prompt overrides the built-in one under every profile. - The retry entry drops "before the task's own result is synchronized", covers the in-progress case fixed two commits back, and says the retried node then counts as succeeded, so completion can read COMPLETE. restart-continue.md says the same. - topology-yaml.md states the report consequence of the cross-group chain. - A test comment no longer attributes the emitted optional-input shape to "only the review task". Co-Authored-By: Claude Opus 5.5 <[email protected]>
3e73bf1 to
caf49a3
Compare
…t helpers dynamic-runtime.ts is already over the strict lint's 500-line budget on main (668 counted lines), and ESLint reports max-lines only on the 501st counted line. lint:strict:ci keeps that report only when its line is one the branch changed. Before the rebase it fell on resolvedConfigForRuntimeRoot, 11 lines past this PR's hunk. #1198 adds lines above it (optional-input lowering), so on current main it falls inside renderRuntimePrompt, which this PR extracts, and the gate fails over a file size this PR did not create. Move renderRuntimePrompt, byte for byte, below resolvedConfigForRuntimeRoot and promptGraphContext, the helpers it calls or whose return type it takes. The report now falls inside promptGraphContext, which this PR does not change. This is the fix #1062 used for lifecycle-inspection.ts; no eslint-disable is added, since #1184 chose not to keep a suppression baseline. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Refs: #1120
The owner approved partly reversing #1120: a continuing producer's output is now optional to consumers in other groups, the
propertiesgroup continues on failure, and the property fan-in moves to a haltingproperty-cataloggroup.Problem
The shipped topologies behind the
default,low-cost,exhaustiveandinvariant-onlyaudit profiles run eight property lenses feedingproperty-specification-fanin. The lenses and the fan-in share thepropertiesgroup, which has nofailure_policy, so it halts.When one lens ends in a terminal failure (attempts exhausted, or a post-agent contract failure), the Smithers engine fails the workflow. The fan-in is skipped, and no strategy, stateful stage or review node runs, so the campaign ends without a report. This happens under the default
best-effortcompletion policy too.Observed on
origin/main(a46a496) with the real engine, using the harness described under Verification: the run endedfailed.verify:property-specification-faninwasskipped.verify:boundary-tests-0,verify:stateful-invariant-setupandverify:final-reportstayedpending. Of the 65 tasks, 1 failed, 14 finished, 1 was skipped and 49 stayed pending.Root cause
Four things combine:
continueOnFail. Smithers treats a failed task withoutcontinueOnFailas a failed workflow.reviewcould use partial results. Even withfailure_policy: continueon the lenses, the fan-in would still have required all eight.reconcilesPartialResults(packages/runtime/src/dynamic-runtime.ts) returnedtask.metadata.node.group === "review". Since feat(runtime): default to best-effort runs with agent-written PARTIAL reports #1120, the group literally namedreviewis the only consumer that may treat a continuing producer's output as optional. That is true in both the compiler (compileSmithersWorkflow) and dynamic lowering.dependencyArtifactDirsis its whole transitive ancestor closure. So a failed lens would also fail input admission for every strategy and specialist behind the fan-in, becauseassertVerifiedDependencyrequires a verification marker for each required agentic ancestor. This is from code reading ofadmitTaskDependencyInputs.semanticPropertyLenses,verifyLensReferenceExpectationPreservation, both throughsealedDirectArtifactDependencies) loaded every direct lens dependency. That included one the in-engine verifier had left out. A fan-in that correctly used only the verified lenses was then rejected withPROPERTY_LENS_AUTHORITY_INVALID.Change
Ten commits, +803/−73 over 27 files, stacked on #1183 (
claude/w27-final-report-selection-record,c9b278a7), which sits on #1201, #1197 andmain538b6188. Of that, +86/−30 is inpackages/*/src, much of it comments; the rest is tests, topology YAML, the prompt, docs and the changelog. Commits 7 to 10 address the reviews of the promoted PR.feat(runtime): continuing results are optional to every other group.reconcilesPartialResults(consumer, producerGroup)now returnsconsumer.group !== producerGroup. It is still the single helper, used by the compiler and by dynamic lowering.fix(runtime): host fan-in gates skip a lens the verifier did not admit.sealedDirectArtifactDependenciesdrops an optional producer that is absent from the verifier-persisted admission. It uses the sameoptionalDeclaredProducerWasNotAdmittedcheck thatfinalizedDeclaredContractProducersand the in-engine gate already apply.feat(topology)!: the topology switch. In.ultrafuzz/topology.ymlandpackages/config/topologies/{default,exhaustive,invariant-only}.yml:propertiesgetsfailure_policy: continue.property-cataloggroup. The fan-in itself still halts, so no strategy runs without a property catalog.topology-yaml.md,artifacts-reports.md,campaigns.md, prompt catalog) state the general rule instead of implyingreviewis the only consumer of partial results.fix(runtime): a task keeps its result when an optional input it ran without is retried successfully.--retry-failed(Modal's durable resume always passes it), the failed lens reruns alongside the strategies, and every strategy admitted before it verifies runs without it.dedupe-findingshad admitted those strategies in the engine, so it then failed the inverse check ("unfinalized optional dependency is present in verifier admission"). That haltsreviewand fails the run over bookkeeping. Thededupe-findingsstep is from code reading; the test below exercises the first check.assertOptionalDependencyAuthoritiesCurrent(workflow-sync.ts) now accepts that omission when the producer's verifier finished after the consumer's preparation started. It orders them by the Smithers event timestamps the sync pass already reads. The consumer's admission begins when itsprepare:task starts.optionalDeclaredProducerWasNotAdmittedsays: "A marker that appears or disappears later cannot enlarge or erase that immutable ancestor set".mainhas the same race for a retried strategy and the review tasks admitted without it, and this fixes it too.docs/how-to/restart-continue.mdsays what a retry of a continuing node does to work that already ran without it.docs(topology): review follow-ups.topology-yaml.mdno longer says a same-group dependent is always skipped. A same-group ancestor reached only through another group makes the dependent fail its input admission instead; see Risk, "Custom topologies".continuea policy "for optional branches".reconcilesPartialResultscomment is scoped to chains within one group.join, so the oldreviewname does not look significant.docs(changelog): theCHANGELOG.mdentries. Under## Unreleased, a "Breaking changes" entry says which packaged topologies change, that an existing project keeps the old behaviour until it makes three named edits to.ultrafuzz/topology.ymland refreshes the fan-in prompt, and the new rule for custom topologies. An "Other changes" entry covers commit 4. Commit 10 revises both.fix(runtime): a task synchronized while its optional input is still retrying keeps its result.waitForTerminalRunrunsultrafuzz inspect, which syncs, every 60 s, so such passes are the norm on that path.task-output-validation-failuredisposition, so the run endedfailed, and--retry-failednever reset the dependent because the engine shows it finished.assertOptionalDependencyAuthoritiesCurrentnow accepts the omission of an optional producer that is not terminal, and still throws when the producer has no recorded state. Tasks synchronize in dependency order and start only after their producers settle, so a producer that is not terminal at that point is being rerun and was unverified when the consumer was admitted. If the rerun fails, the omission matches; if it succeeds, it verified after the admission, which commit 4 accepts.fix(runtime): dynamic group templates follow the optional-input rule.compileSmithersWorkflowapplied the rule only to static tasks, so a dynamic group's task templates, and the children cloned from them, kept every inherited ancestor required. With a continuing producer in another group as an ancestor of a dynamic group, a static sibling ran without the failed producer while every generated child failed its input admission, anartifact-contractfailure that leaves the report unverified.mainhad the same gap for a dynamic group inreview.dependencyArtifactDirs.fix(dashboard): the property fan-in still renders as a property node.flowNodeTypekeyed the property card ongroup === "properties", so/api/flowreported the moved fan-in asagentAttempt. It now mapsproperty-catalogtopropertySpecificationtoo. None of refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197, feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201 or fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 touches this file.docs(changelog): review follow-ups to the entries and reference docs.defaulttopology; qualifies "PARTIAL" with "unless a laterresume --retry-failedreruns the lens successfully"; says what that resume reruns; records the lost failure provenance (output_contracts.missing, the terminal disposition, the public evalfailure_code); offersultrafuzz topology copy default .ultrafuzz/topology.yml --forcefor an uncustomized project; and applies the fan-in prompt refresh to every existing project, because a project prompt overrides the built-in one under every profile.groupand the nodes a dynamic group generates, and says a node that reaches a failed ancestor of its own group only through another group fails its admission as anartifact-contractfailure, which leaves the report unverified.restart-continue.mdsays the same, andtopology-yaml.mdstates the report consequence of the cross-group chain.smithers-task-manifest.test.tsno longer attributes the emitted shape to "only the review task".Why the fan-in moves group: if it stayed inside a continuing
propertiesgroup, the new rule would keep the lenses required for it (same group). Its own output would also become optional to the strategies, so they could run without a catalog. A separate halting group avoids both.Measured effect on the compiled task plan, re-measured on the stack: I planned and compiled every packaged audit profile on this PR's base (
c9b278a7) and on this branch (3e73bf1a), and compared each task'soptionalDependencyArtifactDirs. The numbers are the same as they were againstmaina46a4960.dynamic-strategy-generator's 58 strategy attemptsIn every profile, no optional input was removed, and no task has an optional input from its own group.
On
3e73bf1a, too, the number of tasks with optional inputs is the same in every profile (default 50, low-cost 31, invariant-only 13, exhaustive 76; smoke has 2, none of them new), and no packaged dynamic template has optional inputs, so commit 8 changes no packaged plan.Lens directories are optional to every later task, not only to the fan-in. They sit in each later task's ancestor closure (root cause 3), so without this a failed lens would fail every strategy's input admission. Outside
review, lens directories are the only inputs that became optional indefault; the real-engine test asserts exactly that.Why this reverses part of #1120
#1120 (82879ed) changed optional inputs from "every consumer of a continuing producer" to "only group
review". Its reason: "Continuation lets independent tasks settle. It does not make a strategy's required inputs optional". In other words, a step in a chain must not run without its predecessor, for examplestateful-invariant-handlersafterstateful-invariant-setup.Kept: inputs from the same group stay required. Every stateful and differential chain in the shipped topologies is inside one group, and the compiled plans above contain zero same-group optional inputs.
Reversed: tying the "reconciler" role to the group name
review. The property fan-in reconciles by design: theproperty-source-joingate already checks the catalog against exactly the lenses it was given. The name rule turned one lens failure into the loss of the whole campaign. It also meant that a custom topology whose reconciling group has any other name silently got fail-closed skipping, because onmainreconcilesPartialResultsreturnstask.metadata.node.group === "review".Also changed, as a consequence: in
exhaustive,dynamic-strategy-generator(groupspecialists) now runs with the strategy attempts that succeeded. Since #1120 made the strategies continue on failure, the generator was skipped whenever any of its 58 strategy inputs failed. Before #1120 the strategies halted, so this is new behaviour, not a restoration. The generator reads those inputs only through verifier-admitted ancestor authorities, which omit failed producers.For custom topologies the rule is now: a consumer in another group runs without a failed continuing input. To keep a step strict, put the chain in one group. This is the rule the owner approved.
Deliberately not built
failure_policy. That would be a schema change; a separate group is enough.workflow-sync.tsskips it for any task with optional inputs; see Risk.--retry-failedresets. A retry still reruns a failed lens that the fan-in already consolidated without, and a successful rerun then counts as succeeded, so the report can read COMPLETE although the fan-in never read the lens. The changelog andrestart-continue.mdnow say so. Risk explains the gap, and resume --retry-failed can report COMPLETE after rerunning a continuing node its consumers already ran without #1231 tracks the fix.assertOptionalDependencyAuthoritiesCurrent, both of its exceptions (commits 4 and 7) and the three marker-tamper sync tests can be removed together.dependencyGateForNode(artifact-gates.ts). It is exported but only its tests call it, and it carries a third copy of the optional-input rule that matches neithermain's rule nor this PR's. Deleting it and its tests is a follow-up, kept out of this PR's conflict-prone hunks..ultrafuzz/topology.ymland fan-in prompt; the changelog entry and Risk list the edits.Verification
Discriminating tests
Each of the first six tests fails on
origin/main(a46a496) and passes on this branch, except the sixth's control case, which is meant to pass on both. I checked this after this rebase by copying the branch's six changed test files into a cleanorigin/mainworktree and running them against its source. The last three cover commits 7 to 9; each fails without its commit's source change (checked onb3808c24, or by reverse-applying that commit's source hunks) and passes oneb783f17. These checks ran before the stacked rebase and were not repeated on it. Every test below passes on3e73bf1a.packages/runtime/test/smithers-dependency-skip.integration.test.ts, "a failed property lens leaves the packaged default fan-in, strategies and review to finish" (about 10 s).defaultprofile. It asserts that no attempt requires a lens output, and that outsidereviewonly lens directories became optional.smithers up) on a workflow with one task per compiled attempt. That workflow uses the compiled dependencies, verifier IDs,continueOnFailand optional flags, plus the template's owndependencyVerificationProducersFromCompiledTaskand skip helpers.property-specification-a16zis made to fail.type WorkflowTaskStateContext =toconst agentPromptTemplate =, and fromfunction dependencyVerificationProducersFromCompiledTasktofunction dynamicExecutionMetadata. If a marker is gone, the test fails with "… is missing from the workflow template".mainit fails because the fan-in requires all eight lens dirs. With that assertion removed, themainengine run endsfailedas described under Problem.packages/runtime/test/artifact-gates.test.ts, "property fan-in consumes the admitted lenses when an optional lens failed". This is the host gate.mainit fails withPROPERTY_LENS_AUTHORITY_INVALIDfor the failed lens.property-source-joinat$.properties[0].sources[1]). It also still rejects the failed lens if the admission claims it was admitted.packages/runtime/test/workflow-dependency-policy.test.ts. The reconciling consumer's group is renamed fromreviewtocatalog. Onmainit gets no optional inputs.packages/runtime/test/dynamic-expansion.test.ts. Adds acatalogjoin over a continuingstrategiesfan-out. Onmainthe join gets no optional inputs.packages/topology/test/packaged-topologies.test.ts. Pinsproperties: continueand the fan-in in the haltingproperty-cataloggroup. This is a configuration pin, not a behavioural test. Onmainit fails for default, exhaustive and invariant-only.packages/runtime/test/runtime.test.ts, "syncRun keeps a consumer admitted without an optional prerequisite whose retry verified after the admission", with the control "syncRun rejects … whose retry verified before the admission" (about 17 s each on this shared machine).prepare:task started.mainthe first case fails: the report is failed withARTIFACT_VERIFICATION_AUTHORITY_INVALID, "finalized optional dependency is missing from verifier admission optional-specialist".packages/runtime/test/runtime.test.ts, "syncRun finalizes consumers admitted without an optional prerequisite whose retry is still running" (commit 7, about 23 s). This is the review's reproduction.lens(continuingproperties) →catalog(haltingproperty-catalog) →dedupe(findings@2and a lifecycle ledger) →triage(triaged-findings@1and a ledger), all empty.in-progress) and dedupe and triage have finished without it. The third runs after the lens verified.b3808c24the second sync fails dedupe ("optional dependency is not terminal for verifier admission lens") and triage's gate ("finalized ultrafuzz/findings@2 authority is invalid for dedupe"). With the intermediate assertion removed, the end state has triagefailed, with no diagnostics in the final pass.packages/runtime/test/dynamic-workflow.test.ts, "dynamic templates and their children treat a continuing producer in another group as optional" (commit 8). It plans and compilesproducer(continuingstrategies) →planner(halting) → a dynamicfanoutand a statichunter(bothgoals), then materializes two children. Without the compiler hunk the template's optional inputs areundefined; with it, the template and both children carry the producer's directory. The remap hunk ininstantiateDynamicTasksruns here only as an identity remap; relocation itself is not tested.packages/dashboard/test/dashboard.test.ts, "serves logical topology flow with expanded attempt details" (commit 9) now asserts the fan-in's flow type ispropertySpecification. Without the fix it isagentAttempt.Existing suites that pass on this branch
Run on
3e73bf1a:dynamic-expansion,dynamic-workflow,dynamic-lifecycle,workflow-dependency-policy,task-workflow-identity,generated-workflow-footprint,source-revisionandpinned-submodules, full files: 57/57. refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 deletedcloud-worker-handoff, which the earlier count included.smithers-dependency-skip.integration: 3/3.artifact-gates,prompt-artifact-authority,verified-output,invariant-suite-ancestor-order,topology-transform,generated-workflow-verifier,invariant-suite-handoff-durabilityandterminal-report-completion, full files: 399/399. refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 and fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 deleted tests from these files.dist-test/test/*.test.js): 39/39.smithers-task-manifest.test.ts, full file, 14/14. It includes "optional task inputs must come from producers that continue on failure".runtime.test.tstests, 27/27:prompt-structure.test.ts, whose required-variable table still matches the fan-in prompt; the prompt keeps{{ancestor_contract_artifact_authority:ultrafuzz/property-lens@2}}.workspace-config8/8, evalsbenchmark-manifestandhistory42/42, and CLIaudit-profile-commands3/3.Gates (all pass)
Run on
3e73bf1a, with exit codes checked directly:pnpm install --frozen-lockfile(this PR changes no lockfile, manifest or patch) andpnpm -w buildpnpm -w format:checkpnpm -w lint, with the complexity ceiling of 83CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/claude/w27-final-report-selection-record pnpm -w lint:strict:cipnpm -w knip, three times: after the build, with everydistanddist-testremoved, and after the rebuildpnpm --filtertypecheck for runtime, dashboard, artifacts, config and topology, andtsc -p tsconfig.test.jsonfor the runtime, dashboard, artifacts and CLI testsnode scripts/docs-check.mjsandpnpm -w docs:check(audit-profile docs, prompt catalog,docs-check.mjs)Complexity, measured with ESLint's
complexityrule on this PR's base (c9b278a7) and on3e73bf1a: the most complex functions this PR changes aresynchronizeTasks(66) andfinalizeTerminalTask(65) inworkflow-sync.ts. Both have the same complexity on the base. No other changed function goes up by more than 2:assertOptionalDependencyAuthoritiesCurrent14 → 16 (17 before commit 7),sealedDirectArtifactDependencies12 → 13,flowNodeType12 → 13,compileSmithersWorkflowunchanged at 15, andlowerTaskDynamicDependenciesgoes down, 12 → 11. The only function at 80 or above in the changed runtime source files isverifyCoverageProductionInventory(artifact-gates.ts, 83, at the ceiling). This PR does not change it.Not run
pnpm -w size, which passed oneb783f17before the stacked rebase.runtime.test.ts, CLI, evals and modal suites.workflow.tsxwith a failed lens. The real-engine test drives the compiled scheduling inputs, not the generated workflow. The generated workflow's admission of optional ancestors is the existing path review tasks already use.resume --retry-failedafter a lens failure. Commits 4 and 7 are tested at the sync layer with fixture events.Risk / compatibility
Breaking semantic change. In the shipped topologies a failed lens no longer fails the campaign:
summarizeOutcomescounts the lens as incomplete.artifact-contract, whichassertTerminalStaterefuses to downgrade. The best-effort report is then published unverified, andreport --require-verifiedfails. Strategy contract failures already behave this way.resume --retry-failedreruns the lens successfully; see the next item, the known gap.--require-completeruns still end unsuccessful, with the same exception.Known gap, deferred to resume --retry-failed can report COMPLETE after rerunning a continuing node its consumers already ran without #1231: a successful lens retry can make an incomplete catalog read COMPLETE. Greptile raised this as P1.
resume --retry-failedreruns a failed lens and the rerun succeeds, the lens counts as succeeded.--require-completerun can endsucceeded. Yet the fan-in and every task admitted before the rerun verified never read that lens's properties.3e73bf1a. Arequire-completevariant of commit 4's retry test endssucceeded, while the report's recorded admission lists onlydirect-strategy. I did not commit that probe.mainhas the same gap for a failed strategy and the review tasks that finalized without it, becauseimmutableTerminalFinalizationnever re-finalizes a succeeded task (code reading).restart-continue.mdstate the gap.--retry-failedand in Modal's durable resume, so it is not in this PR. resume --retry-failed can report COMPLETE after rerunning a continuing node its consumers already ran without #1231 proposes that--retry-failedskip a failed continuing attempt that a started task already treated as optional. That keeps the report PARTIAL and avoids the wasted rerun. Modal's durable resume would then have to stop resuming a succeeded run for such failures, becausewaitForTerminalRunwaits until the resume changes the run.Resume with
--retry-failed. Modal's durable resume always passes this flag (packages/modal/src/resume.ts), andmodalDurableRunNeedsResumealso resumes asucceededrun that has a failed node. Commits 4 and 7 stop a retried lens from failing the tasks that ran without it, whether a sync pass sees them during the rerun or after it. These costs remain, all by code reading, and the changelog now states them:timetravelthen resets every node with an attempt that started at or after the lens attempt (@smthrs/time-travel0.35.0,resolveResetNodes), so the rest of the campaign reruns. Since fix(runtime): resume --retry-failed can recover a run whose verifier rejected output #1205 that rerun is judged on its own output.All eight lenses fail. By code reading, the fan-in still runs with only the discovery ledger as a known source. It may produce a ledger-only catalog or fail, and a fan-in failure halts as today. Not tested.
Less failure detail for strategies, specialists and the fan-in. These tasks now have optional inputs, so when their verifier fails,
workflow-sync.tsno longer re-runs the output gate on the host. Review tasks already worked this way.artifact-contract.terminal_disposition: task-output-validation-failureoroutput_contracts.missing.failure_codefor it.isGenuineTaskFailureadditionally requires the verifier's recorded workflow state to befinished, and a failed verifier's isfailed. So these failures were already classified as operational onmain.Exhaustive behaviour change.
dynamic-strategy-generatorruns with partial strategy results.Existing projects. The
defaultandlow-costprofiles use the project's.ultrafuzz/topology.yml. Already-initialized projects keep the old behaviour until they edit it in three places, which the changelog entry spells out:defaults: {failure_policy: continue}toproperties;property-cataloggroup with nofailure_policy, so it halts (the scaffold gives itlabel: Property catalogandcolor: "#854d0e");property-specification-fanin'sgrouptoproperty-catalog.An uncustomized project can instead run
ultrafuzz topology copy default .ultrafuzz/topology.yml --force, the pathdocs/how-to/edit-prompts-topology.mdalready documents; the changelog names both.Every existing project, under every profile including
exhaustiveandinvariant-only, should also refresh.ultrafuzz/prompts/properties/property-specification-fanin.md: delete it and rerunultrafuzz init, which is the pathultrafuzz validatealready gives for a project prompt that differs from the built-in one (PROMPT_DIFFERS_FROM_BUILT_IN, a warning). A project prompt overrides the built-in one under every profile, and the scaffolded copy still asks the fan-in to cover every topology-required lens. The gate checks the catalog against the admitted lenses only, so a fan-in that covers what it can read still passes, but the agent is told to cover a lens it cannot read; a catalog that cites the failed lens fails the gate (tested). The packagedexhaustiveandinvariant-onlytopologies change on upgrade.In-flight runs. The static task plan is sealed at launch. A run started before the upgrade keeps its optional inputs, and commits 2 and 4 only affect tasks that list optional inputs. Dynamic lowering is different:
verifyDynamicRuntimeMaterialization).reviewnode consumes a continuing dynamic group, an in-flight run stops after the upgrade with "published dynamic runtime controls no longer re-derive from their sealed base".dedupe-findings, inreview, which gets the same optional inputs under both rules.Custom topologies. A consumer in another group now runs without a failed continuing producer; see "Why this reverses part of feat(runtime): default to best-effort runs with agent-written PARTIAL reports #1120". That includes a node with no
group(reconcilesPartialResultscomparesundefinedwith the producer's group), which onmainrequired the input and was skipped, and, since commit 8, the nodes a dynamic group generates. A same-group ancestor reached only through another group behaves differently:s1→ specialistx→ strategys2.s1fails,xruns ands2is not skipped.s2fails its input admission instead, as anartifact-contractpreparation failure.assertTerminalStatethen refuses to derive the report's completion, so the best-effort report is published unverified. Onmainboth were dependency-cascade skips and the report could verify as PARTIAL. The changelog andtopology-yaml.mdnow say this and advise keeping a strict chain in one group.Stacked on refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197, feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201 and fix(runtime): record final-report producer selections in the run instead of querying smithers #1183. Until they merge, this branch carries their commits, and the diff against
mainshows them. Merge in the approved order: refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197, feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201, fix(runtime): record final-report producer selections in the run instead of querying smithers #1183, then this PR. After fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 merges, the diff is this PR's own ten commits.Rebase notes
Stacked rebase onto #1183 (
c9b278a7). This PR's ten commits moved frommaina46a4960ontoorigin/claude/w27-final-report-selection-recordc9b278a7. That is #1183's six commits on #1201's five, on #1197's fourteen, onmain538b6188, which adds #1228 and #1229. The pre-rebase head waseb783f17, and the new head is3e73bf1a. This PR changes no lockfile, manifest or patch, sopnpm install --frozen-lockfilewas enough. Commits 1, 2, 5, 7, 8 and 9 applied unchanged (git range-diffshows=).The conflicts, and how each was resolved:
CHANGELOG.md(commits 6 and 10). fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 and refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 added their breaking entries at the top of### Breaking changes, and fix(security): merge per-range registry advisories and patch brace-expansion and undici #1229 added its entry at the top of### Other changes.Changes made during the rebase that no conflict forced. Both are the manual edits this description's Risk section had flagged:
compiledSchedulingWorkflowSourcesliced on:type DependencyVerificationProducer =andfunction dynamicExecutionPath.const agentPromptTemplate =, like the file's two other slices.function dynamicExecutionMetadata, the function that followsdependencyVerificationProducersFromCompiledTaskon refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197's template.verifiedAfterAdmissionOf(workflow-sync.ts). The consumer's admission time used to fall back to the agent task's start, commented "a cloud attempt has none".prepare:task that its agent task depends on. The fallback could not be reached, and I deleted it and its comment.prepare:events and pass.Also re-checked on the stack: the task-plan counts under Change, and the complexity figures under Verification. Both are unchanged.
Greptile (review of
eb783f17):packages/runtime/src/workflow-sync.ts:3737(now line 3736), "Incomplete catalog can appear complete": deferred to resume --retry-failed can report COMPLETE after rerunning a continuing node its consumers already ran without #1231.3e73bf1aconfirms it (Risk, "Known gap").mainhas the same gap for strategies, the changelog andrestart-continue.mdalready state it, and the fix belongs in--retry-failedand Modal's durable resume, not in this PR's sync exception.packages/runtime/src/dynamic-runtime.ts:278, "Relocation remapping lacks coverage": declined.remapProjectPath(directory, group.promptContext.projectRoot, projectRoot)call as the requireddependencyArtifactDirsbeside them. The required remap has no relocation test either.Earlier rebase. Rebased from
2cacf4caontoorigin/maina46a4960(19 new commits onmain). There was one textual conflict, resolved by keeping both sides. The other overlaps merged cleanly, and I checked them by hand:docs/reference/artifacts-reports.md(conflict in commit 3). fix: a reworded finding no longer discards the final report #1204 added the "Each report row carries a source finding …" paragraph directly above the best-effort paragraph that this PR rewrites. I kept fix: a reworded finding no longer discards the final report #1204's paragraph unchanged and put this PR's rewritten paragraph after it.packages/runtime/src/workflow-sync.ts(merged without conflict, checked by hand). fix(runtime): resume --retry-failed can recover a run whose verifier rejected output #1205 changedimmutableTerminalFinalizationandworkflowEvidenceSupersedesPrevious, so that a rerun of a verifier-rejected node is judged on its own output. Commit 4 changessynchronizeTasks' setup,finalizeTerminalTask's input andassertOptionalDependencyAuthoritiesCurrent. These are separate functions, and the two changes combine: after fix(runtime): resume --retry-failed can recover a run whose verifier rejected output #1205 a retried verifier-rejected producer can finalize as succeeded, and in that case commit 4's exception covers the tasks that ran without it. fix(runtime): resume --retry-failed can recover a run whose verifier rejected output #1205's three sync tests pass on this branch.packages/runtime/src/dynamic-runtime.tsandpackages/runtime/test/dynamic-expansion.test.ts(merged without conflict). fix(runtime): a source retry withdraws the prompts rendered from the withdrawn expansion #1220 changeddynamic-expansion-retry.ts, notdynamic-runtime.ts, and added a test in another part ofdynamic-expansion.test.ts. Its prompt withdrawal does not usereconcilesPartialResults. The wholedynamic-expansionfile passes.timeout_seconds: 7200pin on the review group was already in this branch's base. This PR's hunks add only thepropertiesdefaults, theproperty-cataloggroup and the fan-in'sgroup.prompt-structure.test.ts. The prompts suite passes against the edited fan-in prompt.docs/reference/topology-yaml.md(merged without conflict). fix(runtime): a source retry withdraws the prompts rendered from the withdrawn expansion #1220's sentence is in another section;docs:checkpasses.CHANGELOG.mdentries. They replace the "Changelog entry" section that this description used to carry.refinalizeControllerFailures, sosynchronizeTasksis the only caller offinalizeTerminalTask. fix: model fan-out runs can be synchronized repeatedly, recovered aggregates leave skipped, and private evals survive dynamic expansion #1192's model fan-out aggregate change is independent of commit 4, and its tests pass. test: delete vacuous and prompt-prose tests outside the runtime package #1196's topology cache inpackaged-topologies.test.tsis used by commit 3's pin.🤖 Generated with Claude Code
The PR does not yet appear safe to merge because three previously reported correctness and cleanup issues remain outstanding.
Fix with agent prompt
Summary
The PR lets property-lens failures leave the campaign running, moves property consolidation into a halting catalog group, and makes continuing producers optional to consumers in other groups. It also adjusts artifact admission, retry synchronization, dynamic task templates, dashboard rendering, tests, and migration documentation. There are no changes since the previous review.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR L["Property lenses<br/>continue on failure"] -->|"verified outputs only"| C["Property catalog<br/>halts on failure"] C --> S["Strategies and specialists"] S --> R["Review and report"]Reviews (3) · Last reviewed commit: "docs(changelog): state what a failed or ..."