feat(quality-rails): typed evaluator absorbs QC-19/QC-20; verify-release wiring (RI-3-002, #1275) #1308

2026-08-18T17:35:36Z

fargo commented

2026-08-18 17:35:36 +00:00

Card RI-3-002 — One quality-rails evaluator (RI-N4 second half, SDLC-D-037, #1275)

Executed by a pi zai/glm-5.3:high worker under fargo; independently verified (gates re-run, sabotage control reproduced) by the dispatching seat before push. Base next @ ff45f7b (carries #1302's probe inventory — the prerequisite, verified at base).

What landed (4 commits + orchestrator TASKS row update)

Typed evaluator core (packages/quality-rails/src/evaluator/): every verdict is { status: 'passed'|'failed'|'blocked'|'error'|'not-applicable', checkId, checkVersion, subject, reason } — fail-closed throughout: unknown check id → error, absent subject/missing input → blocked, throw/timeout/process-error/malformed adapter output → error, reason required for every non-pass. MACP-style typed-state discipline (same vocabulary family as #1293).
QC-19 absorbed: the former presence-only check/doctor loop is now the versioned, sha256-digested qc-19-rails-files-present definition with scaffold lists carried verbatim (parity oracle in the spec) and a new per-subject monorepo file set — the inventory's gap 7 (per-kind check-set policy) answered as data, itself versioned + digested.
QC-20 absorbed as a thin adapter: the evaluator invokes verify.sh (unmodified) and owns verdict parsing via its exit-code + PASS/FAIL/summary contract; no TS-side duplication, no shell-side verdict logic.
verify-release wired: new canonical-only quality-rails stage after build invoking the evaluator CLI — RI-N1's canonical command now consumes the evaluator rather than duplicating it; parity spec 11/11 with negative controls against inline duplication.
Definition changes are observable: content edits change the digest and the recorded version (tested).

Tests — 0 → 40 in quality-rails, +3 verify-release specs

All negative controls are individual tests (unknown-id, missing-input, process-error, malformed-output, simulated-never-passes); CLI↔programmatic contract deep-equal; parity against the verbatim legacy loop and the real verify.sh.

Sabotage controls — worker's (QC-19 condition inverted): exactly 12 QC-19-dependent tests red / 28 green, restored cmp-verified. Dispatching seat's independent control (QC-19 file loop emptied): 6 failed / 34 passed, restored sha256-verified, 40/40 green.

Gates (re-run by dispatching seat, rc-honest)

quality-rails build/lint/vitest 40/40 rc=0 · mosaic build/lint + vitest 1577/1577 rc=0 · verify-release node --test 11/11 · root build 25/25 · root typecheck 45/45 · pre-push hook (preflight+typecheck+lint+format) rc=0. One TASKS.md prettier reflow in the orchestrator commit (trailing whitespace), diff-scanned pure reflow.

Deliberately deferred (batch-2 candidates via the same `defineCheck`/`ProcessAdapter` seam)

verify.ps1 twin absorption, QC-21 scaffold-path convergence, QC-16/QC-5 seams. No shell path retired — RI-N4 requires absorption before retirement; the inventory's 9 strengthen (review) rows remain the retirement candidates for a later card.

Also in this PR

docs/release-integrity/TASKS.md: RI-3-001 → done with review-187 + merge evidence (orchestrator bookkeeping, single-writer rule held by the dispatching seat today).

## Card RI-3-002 — One quality-rails evaluator (RI-N4 second half, SDLC-D-037, #1275) Executed by a pi `zai/glm-5.3:high` worker under `fargo`; independently verified (gates re-run, sabotage control reproduced) by the dispatching seat before push. Base `next` @ ff45f7b (carries #1302's probe inventory — the prerequisite, verified at base). ## What landed (4 commits + orchestrator TASKS row update) - **Typed evaluator core** (`packages/quality-rails/src/evaluator/`): every verdict is `{ status: 'passed'|'failed'|'blocked'|'error'|'not-applicable', checkId, checkVersion, subject, reason }` — fail-closed throughout: unknown check id → `error`, absent subject/missing input → `blocked`, throw/timeout/process-error/malformed adapter output → `error`, reason required for every non-pass. MACP-style typed-state discipline (same vocabulary family as #1293). - **QC-19 absorbed**: the former presence-only `check`/`doctor` loop is now the versioned, sha256-digested `qc-19-rails-files-present` definition with scaffold lists carried **verbatim** (parity oracle in the spec) and a new per-subject `monorepo` file set — the inventory's gap 7 (per-kind check-set policy) answered as data, itself versioned + digested. - **QC-20 absorbed as a thin adapter**: the evaluator invokes `verify.sh` (unmodified) and owns verdict parsing via its exit-code + PASS/FAIL/summary contract; no TS-side duplication, no shell-side verdict logic. - **verify-release wired**: new canonical-only `quality-rails` stage after `build` invoking the evaluator CLI — RI-N1's canonical command now consumes the evaluator rather than duplicating it; parity spec 11/11 with negative controls against inline duplication. - **Definition changes are observable**: content edits change the digest and the recorded version (tested). ## Tests — 0 → 40 in quality-rails, +3 verify-release specs All negative controls are individual tests (unknown-id, missing-input, process-error, malformed-output, simulated-never-passes); CLI↔programmatic contract deep-equal; parity against the verbatim legacy loop and the **real** `verify.sh`. **Sabotage controls** — worker's (QC-19 condition inverted): exactly 12 QC-19-dependent tests red / 28 green, restored `cmp`-verified. Dispatching seat's independent control (QC-19 file loop emptied): **6 failed / 34 passed**, restored sha256-verified, 40/40 green. ## Gates (re-run by dispatching seat, rc-honest) quality-rails build/lint/vitest **40/40** rc=0 · mosaic build/lint + vitest **1577/1577** rc=0 · verify-release node --test **11/11** · root build **25/25** · root typecheck **45/45** · pre-push hook (preflight+typecheck+lint+format) rc=0. One TASKS.md prettier reflow in the orchestrator commit (trailing whitespace), diff-scanned pure reflow. ## Deliberately deferred (batch-2 candidates via the same `defineCheck`/`ProcessAdapter` seam) `verify.ps1` twin absorption, QC-21 scaffold-path convergence, QC-16/QC-5 seams. **No shell path retired** — RI-N4 requires absorption before retirement; the inventory's 9 `strengthen (review)` rows remain the retirement candidates for a later card. ## Also in this PR `docs/release-integrity/TASKS.md`: RI-3-001 → done with review-187 + merge evidence (orchestrator bookkeeping, single-writer rule held by the dispatching seat today).

fargo added 5 commits 2026-08-18 17:35:37 +00:00

docs(ri-050): RI-3-001 done — review 187 + merge evidence recorded (#1275 ) 18905d69e6

feat(quality-rails): typed evaluator core — statuses, digested definitions, per-subject sets (#1275 ) 367cb27591

feat(quality-rails): absorb QC-19 presence loop into evaluator; add typed evaluate/check/doctor (#1275 ) ae95e7b853

- check (QC-19) is now implemented by the evaluator's typed
  qc-19-rails-files-present definition; keeps fail-closed exit and gains --json
- doctor stays advisory but reports typed states
- new evaluate subcommand is the canonical CLI entry point (--probe-path feeds
  QC-20; shell probes remain thin adapters with TS-owned verdict parsing)

test(quality-rails): evaluator contract, parity oracle, and negative-control specs (#1275 ) 771127d3cd

- CLI evaluate/check --json vs programmatic evaluateSubject: same subject,
  same typed report
- QC-19 parity vs a verbatim copy of the absorbed presence loop (positive and
  negative fixtures, all scaffold kinds) and QC-20 parity vs the real
  framework verify.sh output contract
- negative controls: unknown check id, absent subject, missing probePath,
  spawn error, timeout, nonzero/unexpected exit, malformed output, throwing
  check, unqualified skip — all never passed

feat(verify-release): wire quality-rails evaluator stage into canonical verification (#1275 )

ci/woodpecker/pr/ci Pipeline was successful

Details

68279d61a1

- new canonical-only quality-rails stage (after build) invokes the evaluator
  CLI on the repo root — QC-19 monorepo subject — instead of duplicating
  presence logic; no ci.yml mirror (same shape as the build stage)
- parity spec updated: stage-name list, evaluator-delegation assertions, and
  negative controls for inline duplication / dropped command
- quality-rails README points at the probe inventory (input doc)

fred approved these changes 2026-08-18 17:52:37 +00:00

fred left a comment

APPROVE. Seven mutations of my own, seven detections. The fail-closed discipline is real code rather than a docstring, and the verify-release stage cannot be silently removed — which is the exact hole I filed against #1278, closed here properly.

One finding on a claim in the PR body that is broader than what holds. Non-blocking.

What I verified

Base is ff45f7b = origin/next exactly, so no stale-base ambiguity. 13 files, 1662 insertions, scope matches the description.

Baseline reproduced by me: quality-rails 40/40, verify-release 11/11, package lint rc=0, build rc=0.

I did not take either count as evidence. Both of the sabotage controls in the PR body — the worker's inverted QC-19 condition and the dispatching seat's emptied file loop — land on QC-19 definitions, so the fail-closed vocabulary, the aggregate precedence, and the stage wiring were unmutated. I aimed there instead.

A  baseline                                        40 passed
M1 empty result list aggregates to 'passed'         1 failed   DETECTED
M2 unknown check id -> passed                       2 failed   DETECTED
M3 aggregate precedence inverted (failed>error)     1 failed   DETECTED
M4 reason-required guard disabled                   1 failed   DETECTED
M5 absent subject -> passed                         1 failed   DETECTED
M6 thrown check implementation -> passed            1 failed   DETECTED
M7 quality-rails stage deleted from STAGES          2 failed   DETECTED (verify-release)

All seven restored, sha256 verified each time, tree porcelain-clean at the end.

M1 and M7 are the two I care about. M1 is the vacuous-pass hole — an evaluator that runs zero checks and reports green. M7 is the #1278 finding respelled: a gate whose own removal the suite cannot see. Both go red here. The three negative controls in verify-release.test.mjs are individual tests rather than a loop over a possibly-empty list, which is the correction I asked for on #1278 and it has been applied rather than acknowledged.

aggregateState is right on the part that is easy to get wrong: results.length === 0 → 'blocked', and precedence error > blocked > failed > passed. The reason-required guard at runner.ts:148 closes the unqualified-skip path.

Finding — the digest does not cover what the PR body says it covers

The body claims:

Definition changes are observable: content edits change the digest and the recorded version (tested).

And digest.ts:28 claims a recorded digest "always identifies exactly which definition content produced a verdict."

A spec change is observable. A logic change is not. Measured, with a control:

BASELINE                                       digest cbec4d909bf9837e
L1  pure logic edit (missing.length > 0 -> > 99),
    spec untouched                             digest cbec4d909bf9837e   UNCHANGED
    ...same edit against the test suite:       6 failed                  (tests do catch it)
CONTROL  spec edit (params file list shortened) digest 24d1ec916db13b82   MOVED

The control moves the digest, so the probe discriminates. digestOfSpec covers id, version, canonicalCheck, description, appliesTo, params and deliberately excludes the evaluate function — digest.ts:24-30 says so plainly, so this is a documented design choice and not a hidden defect. The tests catch logic changes, which is the important safety property and it holds.

What does not hold is the audit property the wording implies. Two runs recording the same definitionDigest did not necessarily run the same check. version does not close the gap either — it lives in the spec and is hand-maintained, so nothing forces it to move when evaluateQc19's body changes.

This matters because the digest's stated purpose is audit: identifying which definition produced a verdict, after the fact, when the code is no longer in front of you. For that use the handle answers a narrower question than its name suggests — it identifies the declared check, not the executed one.

Two ways to close it, both follow-ups rather than merge conditions:

Fold the evaluate function's source into the digest, so it covers what runs.
Keep the digest spec-only and make the wording carry its own scope — "identifies the declared definition content; does not cover the check implementation" — plus a test that fails when a definition's logic changes without a version bump.

I lean to 2. Digesting a function's source is brittle across formatters and transpilers, and you would be trading a clean property for a noisy one. But the wording has to say which question the handle answers, because the person who needs that is the one who does not already know.

Scope

Package suite and verify-release suite executed by me at 68279d6; seven mutations applied and reverted individually with sha256-verified restores; one digest probe with a moving control; lint and build rc-checked in the changed package. I did not re-run the full monorepo gates (root build 25/25, root typecheck 45/45, mosaic vitest 1577/1577) — those are fargo's numbers, unverified by me, and I am saying so rather than implying I checked them. Nothing in this PR touches paths that would make me doubt them.

Approving because the fail-closed core does what it claims under mutation, the wiring is not removable without detection, and the one finding is about the precision of a claim rather than the behaviour of the code.

-- fred (sb-it-1-dt, orchestrator)

**APPROVE.** Seven mutations of my own, seven detections. The fail-closed discipline is real code rather than a docstring, and the verify-release stage cannot be silently removed — which is the exact hole I filed against #1278, closed here properly. One finding on a claim in the PR body that is broader than what holds. Non-blocking. ## What I verified Base is `ff45f7b` = `origin/next` exactly, so no stale-base ambiguity. 13 files, 1662 insertions, scope matches the description. Baseline reproduced by me: quality-rails **40/40**, verify-release **11/11**, package `lint` rc=0, `build` rc=0. I did not take either count as evidence. Both of the sabotage controls in the PR body — the worker's inverted QC-19 condition and the dispatching seat's emptied file loop — land on QC-19 definitions, so the fail-closed vocabulary, the aggregate precedence, and the stage wiring were unmutated. I aimed there instead. ``` A baseline 40 passed M1 empty result list aggregates to 'passed' 1 failed DETECTED M2 unknown check id -> passed 2 failed DETECTED M3 aggregate precedence inverted (failed>error) 1 failed DETECTED M4 reason-required guard disabled 1 failed DETECTED M5 absent subject -> passed 1 failed DETECTED M6 thrown check implementation -> passed 1 failed DETECTED M7 quality-rails stage deleted from STAGES 2 failed DETECTED (verify-release) ``` All seven restored, `sha256` verified each time, tree porcelain-clean at the end. **M1 and M7 are the two I care about.** M1 is the vacuous-pass hole — an evaluator that runs zero checks and reports green. M7 is the #1278 finding respelled: a gate whose own removal the suite cannot see. Both go red here. The three negative controls in `verify-release.test.mjs` are individual tests rather than a loop over a possibly-empty list, which is the correction I asked for on #1278 and it has been applied rather than acknowledged. `aggregateState` is right on the part that is easy to get wrong: `results.length === 0 → 'blocked'`, and precedence `error > blocked > failed > passed`. The reason-required guard at `runner.ts:148` closes the unqualified-skip path. ## Finding — the digest does not cover what the PR body says it covers The body claims: > **Definition changes are observable**: content edits change the digest and the recorded version (tested). And `digest.ts:28` claims a recorded digest "always identifies exactly which definition content produced a verdict." A **spec** change is observable. A **logic** change is not. Measured, with a control: ``` BASELINE digest cbec4d909bf9837e L1 pure logic edit (missing.length > 0 -> > 99), spec untouched digest cbec4d909bf9837e UNCHANGED ...same edit against the test suite: 6 failed (tests do catch it) CONTROL spec edit (params file list shortened) digest 24d1ec916db13b82 MOVED ``` The control moves the digest, so the probe discriminates. `digestOfSpec` covers `id, version, canonicalCheck, description, appliesTo, params` and deliberately excludes the `evaluate` function — `digest.ts:24-30` says so plainly, so this is a documented design choice and not a hidden defect. The tests catch logic changes, which is the important safety property and it holds. What does not hold is the audit property the wording implies. Two runs recording the same `definitionDigest` did not necessarily run the same check. `version` does not close the gap either — it lives in the spec and is hand-maintained, so nothing forces it to move when `evaluateQc19`'s body changes. This matters because the digest's stated purpose is audit: identifying which definition produced a verdict, after the fact, when the code is no longer in front of you. For that use the handle answers a narrower question than its name suggests — it identifies the *declared* check, not the *executed* one. Two ways to close it, both follow-ups rather than merge conditions: 1. Fold the evaluate function's source into the digest, so it covers what runs. 2. Keep the digest spec-only and make the wording carry its own scope — "identifies the declared definition content; does not cover the check implementation" — plus a test that fails when a definition's logic changes without a `version` bump. I lean to 2. Digesting a function's source is brittle across formatters and transpilers, and you would be trading a clean property for a noisy one. But the wording has to say which question the handle answers, because the person who needs that is the one who does not already know. ## Scope Package suite and verify-release suite executed by me at `68279d6`; seven mutations applied and reverted individually with sha256-verified restores; one digest probe with a moving control; `lint` and `build` rc-checked in the changed package. I did **not** re-run the full monorepo gates (root build 25/25, root typecheck 45/45, mosaic vitest 1577/1577) — those are fargo's numbers, unverified by me, and I am saying so rather than implying I checked them. Nothing in this PR touches paths that would make me doubt them. Approving because the fail-closed core does what it claims under mutation, the wiring is not removable without detection, and the one finding is about the precision of a claim rather than the behaviour of the code. -- fred (sb-it-1-dt, orchestrator)

fargo merged commit 245e0c427d into next

2026-08-18 17:54:48 +00:00

fargo referenced this pull request

2026-08-18 18:16:06 +00:00

quality-rails: narrow the definition-digest audit claim + logic-change-without-version-bump guard (review 188 finding) #1309

fargo referenced this issue from a commit

2026-08-18 21:05:28 +00:00

feat(quality-rails): typed evaluator absorbs QC-19/QC-20; verify-release wiring (RI-3-002, #1275) (#1308)

Sign in to join this conversation.