risk_surface has no executable record that the check ran #130

New issue

Closed

opened 2026-08-15 14:33:34 +00:00 by john · 1 comment

john commented

2026-08-15 14:33:34 +00:00

Owner

Source: deep dive F13; strict re-review C-13/C-14 (independent convergence)
Verdict: AGREE - high

Claim as reviewed

The blocking review only fires when the orchestrating model checks a diff against a prose list. There is no executable seam proving that a completed diff was evaluated or recording which category matched, so an omitted detection step looks the same as a diff with no risky match.

At HEAD `f354864` (v3.3.0)

cadence-core/templates/config.json:3 ships "stakes": "shipped", and cadence-core/route-table.json resolves that row to plan: off, diff: off, risk_surface: blocking, phase_diff: off. risk_surface is therefore the only live gate on a default install.

Its firing condition is prose: cadence-core/workflows/execute.md:248-258 instructs the orchestrator to "Check git diff {pre-plan HEAD}..HEAD against the risk-surface list in references/review-triggers.md", and that list is the eight categories at cadence-core/references/review-triggers.md:364-378.

cadence-core/bin/lib/surface-scan.mjs:170-188 is explicitly a scoping aid, not a detector: it does not inspect source text and returns recommended = [...CATEGORIES] unconditionally.

No risk-check seam exists anywhere under cadence-core/bin/. A fire writes a lifecycle/dispatch trace event; a non-match writes nothing, so the run record cannot distinguish "the step was skipped" from "the step ran and matched nothing".

Note

Fix shape: an executable seam that always records {checked:true, categories, matches, inconclusive} for a diff range, required before plan completion. Exact semantic detection stays heuristic; what changes is that "did not run" stops masquerading as "ran clean".

**Source:** deep dive F13; strict re-review C-13/C-14 (independent convergence) **Verdict:** AGREE - high ### Claim as reviewed The blocking review only fires when the orchestrating model checks a diff against a prose list. There is no executable seam proving that a completed diff was evaluated or recording which category matched, so an omitted detection step looks the same as a diff with no risky match. ### At HEAD f354864 (v3.3.0) `cadence-core/templates/config.json:3` ships `"stakes": "shipped"`, and `cadence-core/route-table.json` resolves that row to `plan: off, diff: off, risk_surface: blocking, phase_diff: off`. `risk_surface` is therefore the only live gate on a default install. Its firing condition is prose: `cadence-core/workflows/execute.md:248-258` instructs the orchestrator to "Check `git diff {pre-plan HEAD}..HEAD` against the risk-surface list in references/review-triggers.md", and that list is the eight categories at `cadence-core/references/review-triggers.md:364-378`. `cadence-core/bin/lib/surface-scan.mjs:170-188` is explicitly a scoping aid, not a detector: it does not inspect source text and returns `recommended = [...CATEGORIES]` unconditionally. No `risk-check` seam exists anywhere under `cadence-core/bin/`. A fire writes a `lifecycle/dispatch` trace event; a non-match writes nothing, so the run record cannot distinguish "the step was skipped" from "the step ran and matched nothing". ### Note Fix shape: an executable seam that always records `{checked:true, categories, matches, inconclusive}` for a diff range, required before plan completion. Exact semantic detection stays heuristic; what changes is that "did not run" stops masquerading as "ran clean".

john added the

bug

security

external-review

labels

2026-08-15 14:33:34 +00:00

john added this to the v3.5.0 milestone

2026-08-15 15:03:17 +00:00

john commented

2026-08-15 15:03:18 +00:00

Author

Owner

Scope trap — read before planning

This issue is about the RECORD, not the DETECTOR. The obvious misreading of the title is "make risk-surface detection deterministic." Do not do that, and do not let a plan task drift into it.

cadence-core/bin/lib/surface-scan.mjs:170-188 already argues the opposite position, deliberately and at length: presence is provable from structure, absence is not, a detector set cannot be complete (framework built-in auth ships no separate dependency, so auth reads silent in a project that plainly has it), and narrowing to the evidenced set would be "the absence-from-silence conclusion this whole file exists to refuse." That reasoning is correct and this issue does not overturn it.

What is wrong is narrower: nothing records that the check happened. A fire writes a lifecycle/dispatch trace event; a non-match writes nothing. So "the orchestrator skipped step N" and "the orchestrator checked and nothing matched" are byte-identical in the run record, and at the shipped default this is the only gate there is.

The deliverable is a seam that always emits, whatever the verdict - along the lines of {checked:true, range, categories, matches, inconclusive} - plus the execute-workflow requirement that a recorded result exist before plan completion. After that, a missing record is a detectable state instead of an invisible one.

Why this is a minor bump, not a patch

New planning.mjs subcommand (its own CONTRACTS row and tests), a new trace event shape, and a changed requirement in workflows/execute.md. That is new public surface, so v3.5.0 rather than v3.4.2.

Generalizes a closed finding

#156 (reviewer set is not a hard dispatch invariant, closed as wontfix) is the same shape: route.mjs:630-636 emits reviewers as advice and the model dispatches, with nothing recording whether the dispatch matched what resolve sanctioned. references/review-triggers.md:111-118 already half-solves it - --reviewer on the trace append "names the backend that ACTUALLY ran, never the one the trigger asked for."

If this seam's record shape can carry the same sanctioned-vs-actual comparison, #156 closes for real instead of by argument. Worth one look during design; not worth widening scope if it does not fall out naturally.

Sequencing

Independent of v3.3.1, v3.4.0 and v3.4.1 - different files, no shared leases. Can be planned as soon as it is picked.

## Scope trap — read before planning **This issue is about the RECORD, not the DETECTOR.** The obvious misreading of the title is "make risk-surface detection deterministic." Do not do that, and do not let a plan task drift into it. `cadence-core/bin/lib/surface-scan.mjs:170-188` already argues the opposite position, deliberately and at length: presence is provable from structure, absence is not, a detector set cannot be complete (framework built-in auth ships no separate dependency, so `auth` reads silent in a project that plainly has it), and narrowing to the evidenced set would be "the absence-from-silence conclusion this whole file exists to refuse." That reasoning is correct and this issue does not overturn it. What is wrong is narrower: nothing records that the check happened. A fire writes a `lifecycle/dispatch` trace event; a non-match writes nothing. So "the orchestrator skipped step N" and "the orchestrator checked and nothing matched" are byte-identical in the run record, and at the shipped default this is the only gate there is. **The deliverable is a seam that always emits**, whatever the verdict - along the lines of `{checked:true, range, categories, matches, inconclusive}` - plus the execute-workflow requirement that a recorded result exist before plan completion. After that, a missing record is a detectable state instead of an invisible one. ## Why this is a minor bump, not a patch New `planning.mjs` subcommand (its own CONTRACTS row and tests), a new trace event shape, and a changed requirement in `workflows/execute.md`. That is new public surface, so v3.5.0 rather than v3.4.2. ## Generalizes a closed finding #156 (reviewer set is not a hard dispatch invariant, closed as wontfix) is the same shape: `route.mjs:630-636` emits `reviewers` as advice and the model dispatches, with nothing recording whether the dispatch matched what resolve sanctioned. `references/review-triggers.md:111-118` already half-solves it - `--reviewer` on the trace append "names the backend that ACTUALLY ran, never the one the trigger asked for." If this seam's record shape can carry the same sanctioned-vs-actual comparison, #156 closes for real instead of by argument. Worth one look during design; not worth widening scope if it does not fall out naturally. ## Sequencing Independent of v3.3.1, v3.4.0 and v3.4.1 - different files, no shared leases. Can be planned as soon as it is picked.

john referenced this issue from a pull request that will close it,

2026-08-15 22:34:59 +00:00

v3.5.0 - the check that proves it ran #178

john closed this issue

2026-08-15 22:35:26 +00:00

john referenced this issue

2026-08-15 22:37:16 +00:00

issue-check resolves the tracker by origin URL host, so an SSH-host remote never matches a tea login #180