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

Merged
john merged 19 commits from cadence/v3.5.0 into main 2026-08-15 22:35:26 +00:00
Owner

Closes #130.

risk_surface is blocking at every stakes level and, at the shipped default, it is the only review trigger that fires at all. Its entire firing condition was prose: workflows/execute.md told the orchestrator to check a diff range against eight categories, and workflows/task.md said the same thing a second time. A match wrote a lifecycle event; a non-match wrote nothing. Those two states left identical bytes, so the run record could not tell a skipped check from a clean one.

planning.mjs risk-check closes it. run answers a resolved commit range with {checked, categories, matches, inconclusive} and appends that record to trace.jsonl on every invocation, clean range included. status is the enforcement half: both completion paths call the seam instead of reading a prose list, neither reports done without a record, and range identity is the resolved commit pair rather than the ref spelling. The enforcement was watched to fail against the unpatched tree first.

Detection stays heuristic on purpose. What changed is that whether the finding step ran is now a fact in the record rather than an inference from silence.

Two requirements (RSK-01, RSK-02), one phase, 18 commits. Suite 1913/1913, self-verify clean, tsc clean. UAT 13 passed, 1 skipped with reason.

Known issue carried forward: the detector self-matches its own test fixtures on six of eight categories, since a fixture proving a signal fires has to contain something that fires it.

Closes #130. `risk_surface` is blocking at every stakes level and, at the shipped default, it is the only review trigger that fires at all. Its entire firing condition was prose: `workflows/execute.md` told the orchestrator to check a diff range against eight categories, and `workflows/task.md` said the same thing a second time. A match wrote a lifecycle event; a non-match wrote nothing. Those two states left identical bytes, so the run record could not tell a skipped check from a clean one. `planning.mjs risk-check` closes it. `run` answers a resolved commit range with `{checked, categories, matches, inconclusive}` and appends that record to `trace.jsonl` on every invocation, clean range included. `status` is the enforcement half: both completion paths call the seam instead of reading a prose list, neither reports done without a record, and range identity is the resolved commit pair rather than the ref spelling. The enforcement was watched to fail against the unpatched tree first. Detection stays heuristic on purpose. What changed is that whether the finding step ran is now a fact in the record rather than an inference from silence. Two requirements (RSK-01, RSK-02), one phase, 18 commits. Suite 1913/1913, self-verify clean, tsc clean. UAT 13 passed, 1 skipped with reason. Known issue carried forward: the detector self-matches its own test fixtures on six of eight categories, since a fixture proving a signal fires has to contain something that fires it.
john added 19 commits 2026-08-15 22:35:00 +00:00
lib/risk-diff.mjs maps a unified-diff body plus a caller-supplied category
vocabulary to {checked, categories, matches, inconclusive}. Pure: no fs, no
emit, no process, no Date - the caller reads the range, this side owns the
map from what was read to what it means.

Signals are the changed PATHS (whole segments, base names and extensions,
never substrings) and the ADDED and REMOVED lines only, every content pattern
anchored to a construct. No category-name keyword grep: that pass was measured
on this repo on 2026-08-13 and false-positived auth on sixteen `session`
matches and billing on prose about token cost.

inconclusive is independent of matches, so a partly-binary range that also
matched reports both; checked is false only when there was no body at all,
and implies inconclusive. lib/surface-scan.mjs gains the sentence naming the
split and no behaviour change: scanTree still recommends all eight.
`planning.mjs risk-check run --phase N --base <ref> --head <ref> [--plan k]
[--surfaces <a,b,c>]` reads the range with `git -C <top> diff`, hands the body
to lib/risk-diff.mjs with CATEGORIES from lib/surface-scan.mjs, and appends one
{"family":"outcome","event":"risk_check"} line carrying base, head, checked,
categories, the matched tokens and inconclusive.

The append happens on EVERY invocation past argument validation - the no-match
path and the git-failure path included - and BEFORE the envelope, so even a
refusal leaves the record that says the check was attempted. appendEvent's
{written, reason} rides the envelope and may not change the verdict; a range
that could not be read is ok:false so a caller cannot take "git refused" for
"clean".

Rails: --phase through requirePhaseArg and --plan through requireInt (VAL-01,
a valueless flag would read as plan 1); --base and --head both required and
refused when they open with `-`, which git would read as an option; a
--surfaces token outside the eight is a malformed call - fail('bad-args') with
NOTHING appended; git failure details go through redactUrl (EXP-01).

self-verify gains the 'risk-check run' CONTRACTS row and `risk-check` in
TWO_WORD, without which check 2 reads `run` as the whole subcommand.
`planning.mjs risk-check status --phase N` reads the run record through
renderTrace and nothing else, and refuses by plan when a COMPLETED executor
range - a cad-executor bracket whose terminal is a `return` - carries no
outcome/risk_check event. Grouped by plan, so a checkpoint-then-return
continuation counts once. A phase with no completed range is ok:true with an
empty list: nothing to require is not a failure, and a gate refusing there
would block the first plan of every phase.

A plan number is not a range identity and the bracket cannot supply one, so
the refs come from the caller: the optional `--plan k --base ref --head ref`
triple is satisfied only by a record carrying those exact refs. A
plan-matching record with different refs is reported STALE with both ref pairs
named - execute.md's "re-dispatch the remainder" arm is exactly the case that
would otherwise pass on the record its earlier, narrower range left. The
refusal is emit({ok:false, reason:'risk-record-missing', ...}) carrying the
offending list, the way cmdLeaseCheck's undeclared-files arm does.

The evidence arm is frozen bytes: risk-diff.test.mjs's FROZEN_PHASE_1 is this
repo's real phase-1 trace lines (one dispatch closed by a checkpoint, a second
by a return, under one phase_start anchor) copied verbatim, so the row proves
the check reports the omission that was actually there and cannot evaporate
once task 4's wiring starts writing records.

Watched against this repo's live .planning/ at HEAD~1, verbatim:

  $ node cadence-core/bin/planning.mjs risk-check status --phase 1
  {"ok":false,"reason":"risk-record-missing","phase":1,"plans":[{"plan":"1","completed":7,"state":"missing","records":[]},{"plan":"2","completed":2,"state":"missing","records":[]}],"missing":["1","2"],"hint":"run risk-check run --phase 1 --plan <k> --base <ref> --head <ref> for each plan listed, then re-run this check"}
  exit=1
execute.md's post-plan step and task.md's risk_check step now run
`planning.mjs risk-check run` for the committed range instead of instructing a
model to check the diff against the prose list in review-triggers.md. A
non-empty `matches` OR `inconclusive: true` fires the trigger - an unjudged
range is not a cleared one, and widening is the only safe direction on the one
gate that is blocking at every stakes level.

Everything downstream of the decision is unchanged: shape (c), the
plan-<k>-risk.diff and risk-task-{slug}.diff paths, the never-stage rail, the
delete-once-the-trigger-returns rail, the blocking arm, both task.md directory
arms with the ${TMPDIR:-/tmp} inline path, and the RE-READ of triage-gate.md
for the ONE-round re-arm cap (3 citations in execute.md, 1 in task.md, same
counts as before).

ENFORCEMENT, which detection alone is not: execute.md runs `risk-check status`
before reporting done and is not reported done while that call refuses, which
also catches a run that answered ok:true while its append came back
written:false. The task path has no status call of its own, so the `written`
flag is its whole guard and the step states it. Both enforcement rows in
prose-agreement.test.mjs were watched to FAIL against these two files as they
stood before this edit (0 occurrences of `risk-check` and of `written: false`
at HEAD~1).

review-triggers.md's detection section now names which file answers which
question - lib/surface-scan.mjs for which categories a project scopes,
lib/risk-diff.mjs for whether a range touched one - with the eight `- token -`
bullets and the wiring table row byte-identical. route-table.json and
config.schema.json are untouched, so no gate, key or cell moved. The three
prose surfaces were at their budgets exactly and are re-pinned in this commit.
`risk-check run`'s git-failure detail is the seventh use of the
`e && e.message ? e.message : String(e)` idiom in planning.mjs and the
second wrapped in `redactUrl`, so the census pins move 6 -> 7 and 1 -> 2.
Both wrapped sites are git failures on the same EXP-01 rail: a failing
`git rev-parse` or `git diff` quotes back a remote URL whose userinfo can
carry a credential, which is exactly the string that rail covers. The
five unwrapped sites are unchanged and still fs/internal details over
paths the caller already holds.

The expression is respelled at neither call site: the census is doing its
job, and dodging it would delete the signal that made this commit
necessary.

Suite: 1896/1896 pass. self-verify: "problems":[].
`risk-check status` kept only {base, head} off each record, so a run whose
git read FAILED - ok:false, checked:false, the line still on disk because
`run` appends on every path past argument validation - satisfied the
completion gate the moment its ref pair matched. Completion passed on a
check that never saw a diff, which is the state RSK-02 exists to refuse.

The record rows now carry `checked` and `inconclusive` beside the refs, and
only a `checked:true` record can satisfy a range. A plan whose records are
all unchecked reports the new `unchecked` state and rides the refusal.

`inconclusive:true` is the deliberate opposite call, stated in a comment at
the map: a checked-but-inconclusive record IS a completed check, so it
satisfies this gate with the flag visible on the row. "An unjudged range is
not a cleared one" is enforced where a response to it exists - the fire site
in workflows/execute.md fires `risk_surface` on `inconclusive: true` exactly
as it does on a match. Refusing here would leave a range holding a binary
file or a submodule bump permanently unclearable, and an unclearable gate
gets bypassed.

Three rows watched to FAIL against the unpatched seam first: the failed-read
record on the phase-wide arm, the same on a named range, and the
inconclusive record that must still pass with its flag on the row.
`risk-check status` compared the literal `--base`/`--head` SPELLINGS against
the ones on the record, so the record left under `--head HEAD` satisfied a
later, wider `--head HEAD`. workflows/execute.md documents exactly that
spelling for both the run call and the status call, so this was the live
path: a gate fix, a continuation commit or a concurrent write landing
between the two calls was never scanned, and the gate still said `recorded`.

`resolveRange` resolves both refs to commit ids through
`git rev-parse --verify <ref>^{commit}`, and its redacted error is the one
detail both arms emit. `run` records `base_id`/`head_id` beside the
caller's spelling and reads the diff by those ids, so the body scanned is
exactly the range recorded; `status` resolves the asked refs and compares
ids. A ref that cannot be resolved is a refusal on both arms - `no-diff`
with `checked:false` on run, the new `unresolved-range` on status - never a
match. A record carrying no ids (one written before this fix, or by a run
whose refs never resolved) cannot satisfy a named range and reports `stale`,
the only safe direction: its spelling cannot say which commits it meant.

One shape rather than an `ok`-discriminated union, stated at the helper: the
CI typecheck runs `strict: false`, where a JSDoc union does not narrow by
its boolean literal.

Four rows watched to FAIL against the unpatched seam first, including the
live one - a record made at HEAD, one more commit, and the same call that
used to answer `recorded`. The three status rows naming a range moved onto
real repository fixtures, because a range identity test built on refs that
resolve to nothing was itself the defect's cover. The redactUrl census moves
7 -> 8 sites and 2 -> 3 wrapped, naming `resolveRange` as the third.
A submodule bump produces `@@ -1 +1 @@` with `-Subproject commit <old>` /
`+Subproject commit <new>`, so the hunk set `sectionRead` and the section
counted as READ - while every line of what the submodule actually changed
sits in another repository the scanner never opened. A range whose only
change was `vendor/sdk` moving therefore read `matches: []` with
`inconclusive: false`: a judged-clean verdict over code nothing looked at,
the one collapse criterion 3 of this phase exists to refuse.

A gitlink section now lands in the `unreadable` arm, so the range reports
`inconclusive: true` and both fire sites widen onto it exactly as they do
for a binary file. Two markers, because git spells the change either way
depending on the range: the `160000` mode on the index / new file / deleted
file line, and the `Subproject commit <id>` pointer lines. The pointer line
is kept OUT of `changed` as well - it is a commit id, not code, and handing
a bare hex string to the content patterns could only produce a reason that
is not true. The path still reaches the path signals, so a bumped
`vendor/auth-sdk` still names its category, with `inconclusive` beside it
because the match is all this could see. `diff.submodule=log`'s third
spelling emits no `@@` at all and already landed in `unreadable` through the
no-hunk arm; the comment says so.

Three rows watched to FAIL against the unpatched lib first, built from the
real bytes `git diff` emits for a gitlink (captured from a repository whose
entry was written with `update-index --cacheinfo 160000`): the bump, the
submodule add, and a gitlink beside a readable file that must report BOTH.
`risk-check status` walked every `cad-executor` bracket `renderTrace` returned
with no correlation-id filter. `.planning/trace.jsonl` is append-only for the
life of a project and phase numbers restart every milestone, so `--phase 1`
reached seven prior cycles' phase 1 on this repository - including two returns
for a plan 2 belonging to a v3.4.x cycle that predates this seam and can never
be given a record. Enforcement was unsatisfiable on any project with more than
one milestone of history: the check built to stop "not run" passing as "ran
clean" never passed at all.

Both scans now take `renderTrace`'s own `corr` - the id derived from the
phase's newest anchor, the same identity the ONE-round re-arm cap in
references/triage-gate.md keys on. The RECORD scan takes it for the mirror
reason: a risk record left under a previous cycle's id must not satisfy this
cycle's range, or an unsatisfiable gate would be traded for a forgeable one.

Three rows in risk-diff.test.mjs, each watched to fail first, built on frozen
literals from the real bytes of the v3.4.x cycle's record: a prior cycle's
completed range does not hold this run open; the same two lines re-stamped
under the CURRENT id still refuse, so the scope is not a blanket pass; and a
plan-1 record under a previous cycle does not satisfy this run.

Against this repository at 86e351c:

  $ node cadence-core/bin/planning.mjs risk-check status --phase 1 \
      --plan 1 --base 279466b --head 86e351c
  {"ok":true,"phase":1,"plans":[{"plan":"1","completed":2,"state":"recorded",...}]}
  exit 0
risk-check status scoped its bracket and record scans to renderTrace's
corr - the id derived from the phase's newest phase_start. execute.md
anchors at git rev-parse --short HEAD on every invocation, so a phase run
across more than one /cad-execute takes a second id the moment its first
commits land, and every range the earlier invocation completed fell out
of the scan. The gate that exists to stop "not run" passing as "ran
clean" was handing out exemptions instead.

The cycle is now bounded by the phase's own uat_verdict detail=complete,
and each executor bracket pairs with a risk record under its own corr, so
a multi-invocation phase answers per invocation. The named-range arm still
matches by resolved commit pair across the cycle - the commit pair is a
stronger identity than the run that wrote it.

Timestamps are compared as epoch milliseconds and every ambiguity resolves
toward requiring the record: an unparseable sign-off is not a bound, and a
bracket the bound cannot place stays required. A lexicographic compare let
a sign-off stamped 'zzzz' sort above every real timestamp and report the
phase clean with no rows at all.

Five failing-capable rows, all watched red first.
docs: README states the risk check as a seam, not an instruction
All checks were successful
test / node-test (git, 22) (pull_request) Successful in 23s
test / node-test (git, 24) (pull_request) Successful in 22s
test / node-test (other, 22) (pull_request) Successful in 20s
test / node-test (other, 24) (pull_request) Successful in 18s
test / node-test (planning, 22) (pull_request) Successful in 1m2s
test / node-test (planning, 24) (pull_request) Successful in 1m4s
test / node-test (prose, 22) (pull_request) Successful in 27s
test / node-test (prose, 24) (pull_request) Successful in 25s
test / node-test (review, 22) (pull_request) Successful in 15s
test / node-test (review, 24) (pull_request) Successful in 14s
test / node-test (routing, 22) (pull_request) Successful in 29s
test / node-test (routing, 24) (pull_request) Successful in 28s
test / self-verify (pull_request) Successful in 14s
test / typecheck (pull_request) Successful in 15s
9d54b05025
john merged commit 93c9b0a2c6 into main 2026-08-15 22:35:26 +00:00
Sign in to join this conversation.
No description provided.