Workflow branches on a timeout the dispatch seam cannot produce, and the default reviewer is the only unbounded arm #168

New issue

Closed

opened 2026-08-15 15:10:01 +00:00 by john · 1 comment

john commented

2026-08-15 15:10:01 +00:00

Owner

Source: narrowed from strict re-review C-02 (#153, closed as not-a-Cadence-defect)
Verdict: the actionable residual of that finding

C-02 asked for a wall-clock timeout and cancel on subagent dispatch. That is a host capability Cadence does not have - cadence-core/references/seams.md:56-59 says so in those words, and subagent_timeout was already deleted in v2.7.0 rather than kept as an unenforceable knob. #153 is closed on that basis and stays closed.

But the finding's second remedy - "explicitly constrain the workflow to what the host can actually stop" - is a wiring complaint, and two things are in fact miswired.

1. `execute.md:242` branches on a state the seam cannot produce

cadence-core/workflows/execute.md:242 opens a recovery arm labelled "timeout or no report". Nothing in the agent dispatch path can time out - there is no wall-clock kill (seams.md:56-59). So either that word is dead, or it silently means "the user interrupted", which is a different condition with different recovery.

Fix: delete the word, or define what produces the state and say so. A workflow branch keyed on a condition the host cannot generate is exactly what the finding's second arm is about.

2. The default reviewer is the only unbounded one

The cross-model arm runs cadence-core/bin/review-provider.mjs as a Bash subprocess, and cadence-core/references/review-triggers.md:238-248 explicitly instructs running it "with an explicit command timeout of at least review.request_timeout_ms" - a real wall-clock bound supplied by the host.

The claude-subagent arm is an agent dispatch bounded only by maxTurns: 200.

cadence-core/config.schema.json:54 defaults review.reviewers to ["claude-subagent"]. So the bounded arms are the opt-in ones and the unbounded arm is what ships. That ordering is a Cadence choice, not a host limit.

Fix, at minimum: say so where an operator will read it, so nobody infers that review.request_timeout_ms bounds the default reviewer. It does not - that key is cross-model only, as its own schema purpose already states.

Scope decision required before planning

These two halves carry different version consequences:

Part 1 is prose. Patch.
Part 2 is a patch if the fix is documentation. If the fix changes which reviewer arm ships by default, that is changed default behaviour and a minor bump.

Decide which before this is planned.

Not in scope

A wall-clock kill for agent dispatch. While a dispatch is in flight the orchestrator is blocked, so no Cadence code is running that could notice or act. Only the host or the human can. trace.mjs already timestamps every event, computes per-bracket ms, and tracks unpaired dispatches, so a hang is visible after the fact - which is the most that is available here.

**Source:** narrowed from strict re-review C-02 (#153, closed as not-a-Cadence-defect) **Verdict:** the actionable residual of that finding C-02 asked for a wall-clock timeout and cancel on subagent dispatch. That is a host capability Cadence does not have - `cadence-core/references/seams.md:56-59` says so in those words, and `subagent_timeout` was already deleted in v2.7.0 rather than kept as an unenforceable knob. #153 is closed on that basis and stays closed. But the finding's second remedy - "explicitly constrain the workflow to what the host can actually stop" - is a wiring complaint, and two things are in fact miswired. ## 1. `execute.md:242` branches on a state the seam cannot produce `cadence-core/workflows/execute.md:242` opens a recovery arm labelled **"timeout or no report"**. Nothing in the agent dispatch path can time out - there is no wall-clock kill (`seams.md:56-59`). So either that word is dead, or it silently means "the user interrupted", which is a different condition with different recovery. Fix: delete the word, or define what produces the state and say so. A workflow branch keyed on a condition the host cannot generate is exactly what the finding's second arm is about. ## 2. The default reviewer is the only unbounded one The cross-model arm runs `cadence-core/bin/review-provider.mjs` as a Bash subprocess, and `cadence-core/references/review-triggers.md:238-248` explicitly instructs running it "with an explicit command timeout of at least `review.request_timeout_ms`" - a real wall-clock bound supplied by the host. The `claude-subagent` arm is an agent dispatch bounded only by `maxTurns: 200`. `cadence-core/config.schema.json:54` defaults `review.reviewers` to `["claude-subagent"]`. So the bounded arms are the opt-in ones and the unbounded arm is what ships. That ordering is a Cadence choice, not a host limit. Fix, at minimum: say so where an operator will read it, so nobody infers that `review.request_timeout_ms` bounds the default reviewer. It does not - that key is cross-model only, as its own schema purpose already states. ## Scope decision required before planning These two halves carry different version consequences: - Part 1 is prose. Patch. - Part 2 is a patch if the fix is documentation. If the fix changes which reviewer arm ships by default, that is changed default behaviour and a minor bump. Decide which before this is planned. ## Not in scope A wall-clock kill for agent dispatch. While a dispatch is in flight the orchestrator is blocked, so no Cadence code is running that could notice or act. Only the host or the human can. `trace.mjs` already timestamps every event, computes per-bracket `ms`, and tracks `unpaired` dispatches, so a hang is visible after the fact - which is the most that is available here.

john added this to the v3.5.3 milestone

2026-08-15 15:10:01 +00:00

john added the

bug

external-review

labels

2026-08-15 15:10:01 +00:00

john commented

2026-08-15 15:10:08 +00:00

Author

Owner

Scope decision, restated at the top of the queue: part 1 (the dead 'timeout' wording at execute.md:242) is prose and unambiguously patch. Part 2 is a patch if the fix is documentation, and a minor bump if it changes which reviewer arm ships by default - which would move this milestone to v3.6.0. Decide before planning.

john closed this issue

2026-08-18 09:37:45 +00:00