v3.5.0 - the check that proves it ran #178
No reviewers
Labels
No labels
already-shipped
bug
documentation
duplicate
enhancement
external-review
good first issue
help wanted
in progress
invalid
needs-decision
proposal
question
security
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: crenshawdev/cadence-archived#178
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "cadence/v3.5.0"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #130.
risk_surfaceis 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.mdtold the orchestrator to check a diff range against eight categories, andworkflows/task.mdsaid 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-checkcloses it.runanswers a resolved commit range with{checked, categories, matches, inconclusive}and appends that record totrace.jsonlon every invocation, clean range included.statusis 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.
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=1execute.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 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.