Skip to content

feat(review): INV-a to INV-c controls and their red vectors (WP-3) - #27

Open
constantin-jais wants to merge 20 commits into
migrate/recover-codefrom
work/wp3-inv-vectors
Open

feat(review): INV-a to INV-c controls and their red vectors (WP-3)#27
constantin-jais wants to merge 20 commits into
migrate/recover-codefrom
work/wp3-inv-vectors

Conversation

@constantin-jais

@constantin-jais constantin-jais commented Oct 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

WP-3 of the PRD (docs/prd.md, WP table): the controls of INV-a, INV-b and INV-c of ADR-0045 in tools/review, their red vectors under tools/review/vectors, and the activation table updated to match. CA-26, CA-27, CA-28. Rounds 2 and 3 add the corrections from the reviews (Kimi K3, GLM 5.3), plus the sixth activation condition decided by the owner on 2026-10-24 (CA-32, merged by #28).

All changes are under tools/review/*.ts and tools/review/vectors/**, the WP-3 globs. No file outside them; docs/ is untouched.

Round 1 (e7912e1 to 28dfe9c)

  • INV-a, CA-26. Every pass runs with --tools read,grep,find,ls --no-mcp --no-extensions --no-skills --no-approve, built in one place (buildPassArgv, fanout-core.ts). grep -n -- '--tools' tools/review/fanout-core.ts returns the closed list.
  • INV-b, CA-27. A role must be one safe path segment.
  • INV-c, CA-28. The aggregate reads closed fields only.

pi 1.4.0 measurements:

  • An extension may replace a built-in tool by name, read included (dist/core/agent-session.js:2877-2898, :2923-2924; examples/extensions/ssh.ts:128-129). Closed by --no-extensions.
  • MCP tools survive --tools (docs/cli.md:120). Closed by --no-mcp.
  • Project-local resources of the reviewed tree are untrusted. Closed by --no-approve.
  • Unknown long options are ignored (dist/cli/args.js:240-255), so the vector pins each spelling.

Round 2 (73903fa to 6c7d674)

  • F1: option values refuse a leading -, blanks and controls.
  • F2: more than one verdict block is refused.
  • F3: text of pi and of the model in errors decides nothing.
  • F4: the argv oracle is written from the clause.
  • F5: the pass-id equality is pinned by the vectors.
  • F6: outputDir follows the evidence-path policy; no write goes through a link.
  • F7: a proof is a regular vector test file.
  • F8: the activation table is frozen.
  • F9: errors printed to the terminal are neutralised.
  • F10: sixth condition pass-read-confined; both drivers refuse real passes while it is unmet.

Round 3

Finding Change (path:line at the head) Cases No-op mutant, red
G1 extractVerdict (fanout-core.ts:390-420) refuses more than one ```json opening (so a shared closing fence cannot hide a second block), any run of four or more backticks, an unclosed block, and any fence left once the verdict block is removed inv-c: 2 (shared fence, per driver); core unit tests: 7 refusals, 1 acceptance previous lazy extraction restored: 2 vector cases and 6 unit cases red
G2 the journal is opened once, after a last refuseLinks, with O_NOFOLLOW (fanout-run.ts:316); every event is written whole on that descriptor (:323), closed in dispose (:332) on both drivers inv-b: 2 (the review's race: parent renamed, link put in its place mid-campaign) reopen by path per event: 2 red, trap holds events.jsonl
G3 option values refuse \p{Cf} (fanout-core.ts:48); the neutralisation covers \p{Cc} (line feed excepted) and \p{Cf} (:434); parsePlan refusals show values through it (:129); every terminal sink below is classified inv-b: 3; inv-c: 7 \p{Cf} dropped from the option pattern: 4; from the neutralisation: 5; announcement wrapper off: 1; parsePlan refusal raw: 4
G4 campaignExitCode (fanout-run.ts:496): 2 for a refusal or a collision, 1 for a failed pass, 0 otherwise; main exits through it (fanout.ts:122) CLI: 7 policy cases, 1 wiring case return 1 turned into return 0: 2; main's exit removed: 2
G5 the base was caught up after #29; adaptive-attacks names WP-7 (fanout-activation.ts:108), since the owner split WP-4 on 2026-10-26 (WP-4 keeps CA-22 and CA-32, CA-31 moves to WP-7); the header names each unmet condition's package. grep -n 'WP-4' tools/review/*.ts tools/review/vectors/*.ts gave 11 found = 11 classified: 5 corrected (header, adaptive-attacks, 3 test lines pinning it), 6 kept (the two WP-4 conditions, their test lines, the CA-22 refusal line) activation: 2 cases follow the table adaptive-attacks set back to WP-4: 2 red
G1b extractVerdict also refuses any run of three or more tildes, anywhere (fanout-core.ts:403), and, once the verdict block is removed, an indented code line (four spaces or a tab) that opens a JSON object or array (:414); prose stays accepted (list, single tilde, indented text) inv-c: 4 (tilde-fenced and indented rejection after an approval, per driver); core unit tests: 5 refusals, 1 acceptance tilde check off: 5 red; indented check off: 4 red
G2b writeWhole (fanout-run.ts:111) loops on short writes and throws when a write makes no progress; the journal sink uses it fanout-run.test.ts: 6 cases with a stand-in writeSync (chunks of 1, 3, 7, 1000 bytes; a stall after 5 bytes; the descriptor) return value ignored: 4 of 6 red

Intermittent CI failure on 4949625, followed up. The required check failed once in vector 3, "graph driver: a consigne in detail changes nothing the campaign decides": the pass beta of a valid campaign failed. The next head, 512e6c0, passed. Locally the vectors ran 25 times (15 + 10) without a failure. The CI log does not show the pass's error, so the cause is not established. The one concurrency G2 introduced was two passes writing through one FileHandle, which the Node contract of FileHandle.write forbids. Each journal line is now written synchronously on the descriptor (993692c), whole (8c5709f). The case also asserts first that a valid campaign carries no error, so a recurrence names the error.

G3: terminal sinks

grep -n -E 'io\.(log|error)|console\.(log|error)' tools/review/fanout*.ts, outside tests: 25 on 6c7d674, 27 at the head. The neutralising wrapper of announceCampaign adds two. All 27 are classified:

  • Test support, 4. fanout-fixtures.ts:131, :138, :143, :157. This is the stand-in pi's own output, which the orchestrator treats as untrusted input.
  • Relay, 2. fanout-run.ts:57-58. consoleIo forwards what its callers pass, and the callers neutralise.
  • Neutralised, 14.
    • fanout-run.ts:359 and :369: the journal path.
    • :364: the parse error of a journal line read back from disk.
    • :373: a journal disagreement (role, pass id, reason read from disk).
    • :392 and :397: role and output path.
    • :398: errors carrying text of the model and pi.
    • :462-463: the announcement wrapper. :469, :473, :479 and :483 print through it.
    • fanout.ts:62: the unavailable-runtime cause.
  • Closed text, 7.
    • fanout-run.ts:401: counts.
    • :466 and :476: fixed strings, which also go through the wrapper.
    • fanout.ts:77: the usage string.
    • fanout.ts:83: the refusal text of the frozen table.
    • fanout.ts:89: fixed strings and a constant path.
    • fanout.ts:115: a refusal from the table, or the canonical-table message listing ids from code.

Not matched by the grep: the parsePlan refusal reaches the terminal as an uncaught error. It now shows the refused value neutralised.

Residual, declared

  • Verdict write. The write still runs refuseLinks and then opens by path (fanout-run.ts:235-236). That leaves a race window on a parent directory. Only a local process writing into the repository during the campaign can use it. The confinement of WP-4 (CA-32) is to close it.
  • Evidence of a proof. isProofInRepository checks that the evidence is a regular test file under tools/review/vectors/, not what it proves. A textual check (a test( present) would give false assurance, and git tracks no hard links. The barrier is human: the table is frozen in code, flipping a condition to satisfied: true is a reviewed change, and its reviewers replay the mutant. Replaying redness mechanically is left for later, outside WP-3.
  • read is not confined to the worktree. This is now the sixth activation condition (CA-32), delivered by WP-4. Until then both drivers refuse real passes.

Verification

  • bun test tools/review/vectors: vector 1, 41 cases; vector 2, 6; vector 3, 39. All held.
  • bun run check:review-tools locally: 462 pass, 2 fail. Both failures are environmental and identical on base 7644bfd (OSV database download blocked here; licence gate reading the local governance checkout). Both pass in CI.

Work-package: #19 WP-3

🤖 Generated with Claude Code

@constantin-jais
feat(review): read-only tool profile for every review pass, role policy
e7912e1 
INV-a (ADR-0045 decision 1, CA-26): a review pass reads untrusted content
(the commit under review, the evidence), so the model must hold no tool
whose access is write, execute or network. Until now each pass ran
`pi --print --provider --model --thinking <prompt>` with pi's default
tools (read, bash, edit, write).

The command line of a pass is now built in one place, `buildPassArgv` in
fanout-core.ts, with the profile

  --tools read,grep,find,ls --no-mcp --no-extensions --no-skills --no-approve

measured against pi 1.4.0 (dist/cli/args.js, dist/core/agent-session.js):
--tools keeps MCP tools unless an entry starts with mcp__ (--no-mcp); an
extension may register a tool under a built-in name, `read` included, and
replace the built-in, while the allowlist matches names (--no-extensions);
the worktree is the reviewed commit, so its project-local .pi/ and
.agents/skills are untrusted (--no-approve); skills add instructions and
no tool, closed so the verdict does not depend on the operator's personal
skills (--no-skills).

INV-b (CA-27): a role names the scratch worktree and the verdict file, so
parsePlan now refuses a role that is not one safe path segment.

Production code only; the vectors that prove both follow in this pull
request.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
test(review): vector 2 of ADR-0045, every pass holds read-only tools …
65216fb 
…(INV-a)

tools/review/vectors/inv-a-read-only-passes.test.ts captures the command
line of each review pass through the stand-in `pi` named by path
(OpenOptions.piBinary), under both drivers, and checks that it carries
exactly one --tools allowlist equal to read,grep,find,ls (written from the
CA-26 clause, not from the code under test), no entry that edits the
defaults or widens the set (+/-, *, mcp__), the four flags that close the
other sources of a tool (--no-mcp, --no-extensions, --no-skills,
--no-approve), and none of the flags that would load code or trust the
reviewed tree. It also checks one model call per pass and that no prompt
carries the output of another pass: no output of pi reaches a second model.
The cases print their count.

Test support (fanout-fixtures.ts): the stand-in logs its prompt and gains a
"reply" mode that prints a scripted reply, and runRealCampaign drives a
campaign with every real effect through either driver.

Gate-integrity questions for the added tests:
1. What makes it red: the no-op substitution
   `const TOOL_ALLOWLIST = [] as const;` (no --tools on the command line)
   reddens 3 of its 6 cases (the built argv and both captures: "Expected
   length: 1, Received length: 0"), run with env -i PATH HOME.
2. Would an equivalent refactor redden it: no. It reads the tool policy the
   way pi 1.4.0's parser reads a command line (flag present, its value as a
   set), not the order or the position of the arguments; it reddens only if
   a flag is dropped, misspelt or widened. The one structural case checks
   that a single non-test source spells "--tools" and "--print".

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
test(review): vector 1 of ADR-0045, a trapped verdict obtains no effe…
081eb75 
…ct (INV-b)

tools/review/vectors/inv-b-effect-arguments.test.ts plays a hostile
reviewer through the stand-in `pi`, under both drivers, against an empty
trap directory:
- a verdict with extra fields naming an output path, a command and an
  address is refused by schema validation; the run writes nothing but its
  journal, and each pass's journal holds verdict_rejected and no
  verdict_recorded;
- a schema-valid verdict whose contractHashes path, commands, residualRisks
  and finding detail all name the trap is recorded at
  <outputDir>/<role>.verdict.json and nowhere else;
- a verdict claiming another role (a traversal) or another commit is refused
  by exact equality.
In every case the trap stays empty, the journal carries none of the trapped
strings, and the command line of each pass equals the one built from the
plan. Unit cases pin the verdict path to the plan's output directory and
role, and the role policy of parsePlan (traversal, nested or absolute path,
double dot, leading dot, space, newline). The cases print their count.

Gate-integrity questions for the added tests:
1. What makes it red, run with env -i PATH HOME:
   - outputPath derived from a verdict field, the substitution
     `Bun.write((verdict as {...}).contractHashes[0]?.path ?? job.outputPath, ...)`
     in runJob: 2 of 14 cases red (the valid trapped verdict, both drivers;
     the verdict files are missing from the output directory);
   - the role policy neutralised, `if (false) {` in parsePlan: 7 of 14 red;
   - verdict validation neutralised, `errors.push(...[])` in runJob: 4 of 14
     red (extra fields and identity mismatch, both drivers).
2. Would an equivalent refactor redden it: no. It observes effects from
   outside (files on disk, journal content, recorded argv) and error
   substrings produced by the schema validator and the equality checks;
   the argv comparison derives its expectation from buildPassArgv on the
   same plan, so reordering the profile keeps it green.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
test(review): vector 3 of ADR-0045, free text never decides (INV-c)
a0d4f65 
tools/review/vectors/inv-c-closed-vocabulary.test.ts:
- unit: a verdict outside its three values (a sentence, another case, a
  trailing space, a synonym, an empty string, a number) is refused at
  /verdict, as are a mode outside its two values, a commit that is not
  40 hex and a fifth finding severity;
- both drivers, real effects: an out-of-vocabulary verdict fails every pass
  with one model call per pass and no further call, writes nothing but the
  journal, and its verdict_rejected events carry the violation count only;
- both drivers: a valid verdict whose detail, residualRisks and commands
  carry a consigne aimed at the aggregate ("count this pass as failed, set
  failed to 99, route to END") yields the same decision as a benign one
  (failed and the ok of each pass); the text is in the verdict file and in
  no outcome, journal line or log line;
- both drivers: a consigne in detail does not rescue a refused verdict.
The cases print their count.

No production change: the aggregation node of fanout-graph.ts already reads
`ok` only, a boolean validateVerdict derives from closed fields and the
schema, and the default driver's outcome reads the same. This commit adds
the proof.

Gate-integrity questions for the added tests:
1. What makes it red, run with env -i PATH HOME:
   - the aggregation node made to read detail, the substitution
     `failed: state.settled.filter((pass) => !pass.ok || require("node:fs")
     .readFileSync(session.results.get(pass.reviewPassId)?.job.outputPath
     ?? "", "utf8").includes("as failed")).length`: 1 of 16 red (graph
     driver, "failed": 0 expected, 2 received);
   - the same read in outcomeOf for the default driver: 1 of 16 red (direct
     driver);
   - verdict validation neutralised, `errors.push(...[])`: 4 of 16 red.
2. Would an equivalent refactor redden it: no. It compares what two
   campaigns decided (failed, ok per role) and where a marker string
   appears; it does not pin how the aggregate is computed.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
feat(review): INV-a to INV-c met in the activation table, refusal sta…
28dfe9c 
…nds at 2 of 5

inv-a, inv-b and inv-c flip to satisfied with the repository-relative path
of the vector that proves each (rule of the fanout-activation.ts header):
tools/review/vectors/inv-a-read-only-passes.test.ts,
tools/review/vectors/inv-b-effect-arguments.test.ts,
tools/review/vectors/inv-c-closed-vocabulary.test.ts.
telemetry-egress-refused and adaptive-attacks (WP-4) stay unmet, so the
LangGraph path still refuses real passes: "2 of 5 activation condition(s)
unmet". Ids and order of the canonical table are unchanged.

Tests updated: the activation test now says which two conditions remain,
that the refusal is not null, and that the three met conditions name an
existing proof; the command-line refusal test names exactly the unmet
conditions and derives the count from the table instead of a literal 5.

Gate-integrity questions for the changed tests:
1. What makes them red: the no-op substitution `if (true) return null;` in
   activationRefusal reddens 6 cases across fanout-activation.test.ts and
   fanout-runtime.test.ts, including "the two conditions of WP-4 are still
   unmet, so the refusal still stands" (env -i PATH HOME).
2. Would an equivalent refactor redden them: no for the refusal and the
   command line, which read unmetConditions and the printed message; the
   met-conditions case pins the three evidence paths on purpose, since a
   condition's proof is a fact the table must state.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
constantin-jais marked this pull request as ready for review October 24, 2026 12:21
@constantin-jais constantin-jais mentioned this pull request Oct 24, 2026
Merged
@constantin-jais
fix(review): plan option values and outputDir under policy, no write …
73903fa 
…through a link

Review round 1 on 28dfe9c (Kimi K3, GLM 5.3), findings F1, F4 and F6.

F1. parsePlan accepted any non-empty provider, model and thinking, and that
the command line could not move under a hostile value rested on pi 1.4.0's
parser. Each value is now refused when it starts with "-" or carries a
blank or a control character (\s and \p{Cc}, so C0, DEL and C1).

F6. outputDir had no policy and the write followed a pre-existing link.
outputDir now takes the evidence-path policy (repo-relative, safe alphabet,
no ".."), and the effects resolve it against the repository the campaign
opens (OpenOptions.repoDir, "." on the command line, so the command line is
unchanged). refuseLinks refuses a link on any component of the journal path
before and after mkdir and on the verdict path before the write; the
journal is appended with O_NOFOLLOW and the verdict created with
O_CREAT|O_EXCL|O_NOFOLLOW (measured on Bun 1.4: ELOOP for an append through
a link, EEXIST for an existing target, dangling link included). Both
tracked plans still pass: docs/reviews/example-subject-v1/0000000 and
docs/reviews/orchestrator/ca44eb3. Test plans now name a relative
outputDir; the dry-run listing prints it as the plan names it.

F4. The argv oracle of vector 1 was buildPassArgv itself. It is now written
from CA-26 and CA-27: --print first, --provider, --model and --thinking
once each followed by the plan's value, no other positional argument, the
orchestrator's prompt last.

Vector 1 gains 20 cases (34 in all): 7 hostile option values, 5 unsafe
output directories, both tracked plans valid, and per driver an output
directory that is a link, a journal that is a link, and a verdict file
planted as a dangling link.

Gate-integrity questions for the added and changed tests:
1. What makes them red (env -i PATH HOME), each a no-op substitution:
   - F1, `if (false) {` for the option-value check: 7 of 34 red;
   - F6 policy, `} else if (false) {` for the outputDir check: 5 of 34 red;
   - F6 links, `if (false) {` in refuseLinks: 4 of 34 red (directory and
     journal links, both drivers); with CREATE_NO_LINK also replaced by
     O_WRONLY|O_CREAT|O_TRUNC, the 2 dangling-verdict cases go red too
     (two layers: either one alone holds that case);
   - F4, `"--provider",` replaced by `...[],` in buildPassArgv: 4 of 34 red;
     the previous oracle stayed green under it.
2. Would an equivalent refactor redden them: no. The policy cases read
   parsePlan's refusal by field; the link cases read the trap and the
   recorded invocations; the argv oracle reads flag/value pairs and the
   absence of stray positionals, not the order of the profile.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
constantin-jais added a commit that referenced this pull request Oct 24, 2026
@constantin-jais
docs: CA-32 names what confines the read, and closes the existence or…
0a8a870 
…acle

Second-family review of #28 (GLM 5.3, ACCEPTABLE on 0f306b2). Each finding
was checked against its source.

- Major, applied. R-16 named a `pass-read-confined` condition at
  `satisfied: false` that no branch carries yet: the second round of #27 is
  not pushed. The detection column now reads "no true condition", which
  holds both before and after #27 merges.
- Minor, applied. Network namespaces and nftables do not confine a file
  read. CA-32 now says that the container mounts confine the read, that the
  network namespace and its rules confine egress, and that the second does
  not replace the first.
- Minor, applied. The vector now requires that the error returned for the
  sentinel is byte-identical to the error for a missing path inside the
  worktree, so a refusal cannot tell whether a host file exists.
- Minor, rejected. The suggestion was to note in issue #20 that acceptance
  also runs check:review-tools. A work-package issue body must be exactly
  the form output (WORK-PACKAGES.md, "Nothing else is allowed in the
  body"). The full validation chain stays in the requirements
  specification.

Exempt: docs-only
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): refuse a reply with more than one verdict block
23e9e98 
Review round 1 on 28dfe9c, finding F2. extractVerdict took the last fenced
json block, and the "first block" mutant stayed green. The prompt asks for
exactly one block; with two, the block a human reads first and the block
recorded can differ. Any count other than one is now refused (closed when
in doubt). The core unit test that pinned "the last block wins" is replaced
by one block parsed and two blocks refused.

Vector 3 gains 2 cases (18 in all): per driver, a reply with two blocks
that are each valid and disagree (approve, then reject with a blocking
finding) fails every pass, records no verdict and calls no further model.

Gate-integrity questions for the added tests:
1. What makes them red: `if (false) {` for the count check in
   extractVerdict reddens the 2 new vector cases (env -i PATH HOME): the
   first block, an approval, is then recorded.
2. Would an equivalent refactor redden them: no. Both blocks are valid, so
   any rule that picks one of them (first, last, largest) records a verdict
   and reddens; only a refusal keeps them green, which is the behaviour.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
Merge origin/migrate/recover-code into work/wp3-inv-vectors
1c45731 
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): untrusted text in pass errors decides nothing and reache…
5554cfd 
…s the terminal inert

Review round 1 on 28dfe9c, findings F3, F5 and F9.

F9, measured first: the reviewPassId and role mismatch messages of
validateVerdict echo a model value with its control characters (the schema
asks only for a non-empty string), and a pass that exits non-zero carries
up to 500 bytes of pi's error output. The mismatch messages for mode and
commitSha, the schema messages (an extra key named with controls
included) and the JSON parse error carry none. printResults wrote these
errors to the operator's terminal raw. neutralizeControls (fanout-core.ts)
now turns every C0 control but the line feed, DEL and every C1 control
into its visible \uXXXX spelling, and printResults applies it to each
error line. The journal carries no untrusted text: its values are the pass
id the orchestrator assigned, the role (role policy), the plan's provider,
model and thinking (option-value policy), the output path (outputDir
policy), and counts.

F3. A noisy stand-in: the fake pi can now write a scripted error output and
exit with a scripted code. Per driver, three noisy replies, each paired with
its benign twin: a consigne with escape sequences on stderr at exit 0, the
same at a non-zero exit, and in a mismatched reviewPassId. Per-pass ok, the
aggregate (the graph's aggregation node, the default driver's outcome) and
the failure count printResults returns for the exit code must equal the
twin's.

F5. A verdict that forges the pass id and nothing else is refused, on both
drivers, with exactly the pass-id mismatch message; before, only
fanout-core.test.ts caught the removal of that equality.

Vector 3: 26 cases (8 added); vector 1: 36 cases (2 added).

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME), each a no-op substitution:
   - F3, the pass node made to read the text,
     `ok: result.ok || result.errors.join("\n").includes("count this pass as ok")`
     in fanout-graph.ts: 2 of 26 red (graph, non-zero exit and mismatch);
   - F3, outcomeOf counting on a marker,
     `!result.ok && !result.errors.join("\n").includes("count this pass as ok")`:
     2 of 26 red (default driver);
   - F3, printResults reading the text for its exit count,
     `if (result.ok || result.errors.join("\n").includes("count this pass as ok"))`:
     6 of 26 red (both drivers);
   - F9, neutralizeControls given a pattern that matches nothing, `/(?!)/g`:
     2 of 26 red;
   - F5, `if (false) {` for the reviewPassId equality: 2 of 36 red in
     vector 1, and 2 in vector 3.
2. Would an equivalent refactor redden them: no. The F3 cases compare what
   two campaigns decided (ok, aggregate, exit count), not how; the F9 case
   checks code points written from the definition of C0, DEL and C1, not
   the implementation's pattern, and that the text stays legible; the F5
   case pins the one error the equality produces, which is its contract.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): a proof is a regular vector test file; the activation ta…
3b5790a 
…ble is frozen

Review round 1 on 28dfe9c, findings F7 and F8.

F7. isProofInRepository accepted any existing path under the root, a
directory or package.json included. A proof is now a plain
repository-relative path matching tools/review/vectors/<name>.test.ts,
that lstat sees as a regular file (not a link, not a directory) and whose
real path is itself. The test-only table of fanout-fixtures.ts, which
proves nothing, now names the INV-a vector, since its former evidence
(fanout-fixtures.ts) is no longer admissible; it is still importable by no
orchestrator source (authority-boundary.test.ts).

F8. The table and each condition are frozen: pushing, splicing or setting
`satisfied` at run time throws, and the refusal still stands after the
attempt.

fanout-activation.test.ts: 7 new refusal cases (the vectors directory, a
directory named like a test, package.json, a test outside the vectors
directory, a non-test file in it, a link in it to a file of the root, a
traversal back into it), each beside a control that the same root accepts a
real vector; the link-outside case moved under tools/review/vectors; one
freeze case.

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME):
   - `if (false) return false;` for the path pattern: 4 of the 7 refusal
     cases red (package.json, a test elsewhere, a non-test file, the
     traversal);
   - `if (entry === undefined) return false;` (regular-file check dropped):
     the directory-named-like-a-test case red; the link case stays held by
     the real-path equality, a second layer;
   - the freeze replaced by `void condition;` and `return table;`: the
     freeze case red.
2. Would an equivalent refactor redden them: no. They state which paths
   are proofs and that the table resists mutation, not how either is
   checked.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
feat(review): sixth activation condition, pass-read-confined, refused…
6c7d674 
… by both drivers

Review round 1 on 28dfe9c, finding F10: the owner's decision of 2026-10-24
on K1 (read is not confined to the worktree), verbatim "6e condition, deux
pilotes (Recommandé)", written as CA-32 by #28.

The activation table gains its sixth condition: id pass-read-confined,
requirement "a review pass cannot read outside its detached worktree,
enforced outside pi", source citing the owner decision of 2026-10-24 and
CA-32, work package WP-4, satisfied false, evidence null. The LangGraph
path applies all six: "3 of 6 activation condition(s) unmet".

The default driver now refuses real passes itself, in runCampaignDirect and
not only on the command line, while the conditions that bind it are unmet:
pass-read-confined alone, since it loads no package (defaultDriverRefusal).
The refusal comes before anything is printed or opened, names the
condition, and leaves --dry-run available; a dry run, an empty run and a
refused overwrite run no pass and need no activation. Both drivers throw
ActivationRefusedError, which fanout.ts prints with exit 2. The canonical
table check moves from fanout-graph.ts to fanout-activation.ts
(assertCanonicalTable) and serves both drivers. The LangGraph refusal no
longer suggests omitting --runtime, since the default path refuses too.

Tests that run real passes with the stand-in pi hand both drivers the
table reserved for tests (ALL_CONDITIONS_MET_FOR_TESTS), as the graph
driver's already did. fanout-cli.test.ts: the command line now pins the
refusal, that no flag or variable lifts it, and every path that runs no
pass (dry run, recorded verdict skipped, forced overwrite refused, missing
commit); what a real campaign records, a failing pass and a dirty tree are
pinned in process through the default driver and printed as the command
line prints. The CA-29 test with the package absent now shows a real run
reaching the driver's own refusal, identical to the run with the package
present and with no module resolution error. authority-boundary.test.ts
covers the default driver: it takes its refusal from fanout-activation.ts
and fanout.ts hands neither driver a table.

Gate-integrity questions for the added and changed tests:
1. What makes them red (env -i PATH HOME, full review suite):
   - `if (false) throw new ActivationRefusedError(refusal ?? "");` in
     runCampaignDirect: 4 red (driver refusal, CLI refusal, no flag lifts
     it, CA-29 with the package absent);
   - DEFAULT_DRIVER_CONDITION_IDS emptied to `[]`: 5 red (the same 4 and
     "the default path is refused by the sixth condition alone").
2. Would an equivalent refactor redden them: no for the behaviour cases,
   which read refusals, messages and what ran. The authority-boundary case
   pins two call shapes on purpose (the driver reads its table from the
   activation module; the command line passes none), as its neighbours do.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): a reply that could show a second verdict is refused, sha…
cb112ff 
…red fence included

Review round 2 on 6c7d674, finding G1 (Kimi C1, reproduced by the
orchestrator). The lazy match of extractVerdict saw one block in
"```json {A} ```json {B} ```" and returned A: an approval was recorded
while a rejection with a blocking finding stayed visible, on both drivers.

extractVerdict now refuses, closed when in doubt:
- more than one ```json opening, counted on their own (case-insensitive,
  blanks allowed before "json"), so a closing fence shared by two blocks
  cannot hide the second;
- any run of four or more backticks, which re-nests markdown fences;
- an unclosed verdict block;
- any fence left once the verdict block is removed: another code block, or
  a closing fence inside a JSON string.
Inline single backticks in the prose stay accepted.

Vector 3 gains 2 cases (28 in all): per driver, the shared-fence reply of
the review, approval then rejection with a blocking finding, fails every
pass and records nothing. fanout-core.test.ts gains 7 refusals (shared
fence, an opening in capitals, four backticks, a block nested in a
four-backtick fence, a fence inside a JSON string, another code block, an
unclosed block) and one acceptance (inline backticks).

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME): the previous extraction restored
   in place of the new checks (lazy match of /```json\s*\n([\s\S]*?)```/g,
   refusal only when it finds two matches): both new vector cases red (the
   approval is recorded), and 6 unit cases red.
2. Would an equivalent refactor redden them: no. They state which replies
   are refused and which one is read; any rule that refuses every reply in
   which a reader could see a second verdict keeps them green.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): open the journal once per campaign and append through th…
6e8ed94 
…at descriptor

Review round 2 on 6c7d674, finding G2 (Kimi C2, major; GLM finding 3).
journalWriter reopened the journal by its path for every event; refuseLinks
ran only when the campaign opened, and O_NOFOLLOW guards the last component
only. A parent of outputDir swapped for a link during the campaign
redirected every following event (57 measured in a trap by the review).

openCampaign now opens the journal once, after a last refuseLinks, with
O_WRONLY|O_APPEND|O_CREAT|O_NOFOLLOW, and every event is appended through
that descriptor (JournalSink): an open descriptor holds the file, not its
path, so a renamed or replaced parent redirects nothing. The descriptor is
closed in dispose, which both drivers call in finally; an open failure
removes the scratch worktree base.

Residual, not treated here (orchestrator's disposition): the verdict write
still checks with refuseLinks and then opens by path
(fanout-run.ts:214-215), a race window on a parent that only a local
process writing into the repository during the campaign can use; the
confinement of WP-4 is to close it. Declared in the pull request.

Vector 1 gains 2 cases (38 in all): per driver, the race of the review,
the stand-in renaming the output directory and putting a link to the trap
in its place once the first events are written. Not a byte reaches the
trap, every pass's events reach the journal opened by the campaign (each
pass shows agent_call and ends with pass_end), and the verdict write meets
the link and is refused.

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME): the sink made to reopen by path
   for each event, `await require("node:fs/promises").appendFile(journalPath,
   line, { flag: APPEND_NO_LINK })` in place of `await journal.write(line)`:
   both new cases red, the trap holds "events.jsonl".
2. Would an equivalent refactor redden them: no. They observe the trap and
   the journal file on disk; any write path that cannot be redirected by a
   parent swapped mid-campaign keeps them green.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais constantin-jais mentioned this pull request Oct 26, 2026
Merged
@constantin-jais
fix(review): format characters refused in option values, neutralised …
90e57ff 
…at every terminal sink

Review round 2 on 6c7d674, finding G3 (GLM finding 1, Kimi C5; reproduced
by the orchestrator), swept as a class.

(a) OPTION_VALUE_PATTERN accepted \p{Cf}: "m‮del", "m​del" and
"m؜del" passed. It now refuses \p{Cf} as well as blanks and \p{Cc}.
(b) The neutralisation covered \p{Cc} only. UNSAFE_CONTROLS is now
\p{Cc} (line feed excepted) or \p{Cf}; astral code points print as
\u{XXXXX}.
(c) Two sinks printed without it. announceCampaign now neutralises every
line it prints (plan values, whatever policy screened them upstream), and
the four refusals of parsePlan show the refused value through
neutralizeControls(JSON.stringify(value)), since JSON.stringify leaves DEL,
C1 and \p{Cf} raw (a raw CSI, bytes c2 9b, was measured in a refusal).
reportJournal, the ok and FAIL lines of printResults, and the unavailable-
runtime message of fanout.ts are neutralised too.

Sink count, `grep -n -E 'io\.(log|error)|console\.(log|error)'
tools/review/fanout*.ts` outside tests: 25 on 6c7d674, 27 after this commit
(the neutralising wrapper of announceCampaign adds two). All 27 classified:
4 test support (the stand-in pi's own output), 2 relay (consoleIo forwards
what its callers pass), 14 neutralised, 7 closed text. 27 found = 27
classified; the list is in the pull request.

Vector 1 gains 3 cases (41): a right-to-left override, a zero-width space
and an Arabic letter mark in an option value are refused. Vector 3 gains 7
(35): per driver, a bidi override and a zero-width character in a mismatched
pass id print visibly; four parsePlan refusals (C1 CSI and right-to-left
override in model, C1 CSI in a role, zero-width space in outputDir) are
reported inert; the announcement prints unscreened plan values inert. The
terminal oracle lists the format characters by code point, not from the
pattern under test.

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME), each a no-op substitution:
   - \p{Cf} removed from OPTION_VALUE_PATTERN: 4 red (3 in vector 1, 1 in
     vector 3);
   - \p{Cf} removed from UNSAFE_CONTROLS: 5 red in vector 3;
   - the announcement wrapper passing lines through unchanged: 1 red;
   - shown() returning JSON.stringify(value) alone: 4 red.
2. Would an equivalent refactor redden them: no. They check refusals and
   the code points that reach a terminal, not the patterns or the sites
   that produce them.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): one exit-code policy for the command line, pinned withou…
4949625 
…t a real pass

Review round 2 on 6c7d674, finding G4 (Kimi C4, coverage lost). Since
round 2 the command line runs no real pass, and nothing pinned
`if (printResults(...) > 0) process.exit(1)` any more: removing the exit
left the suite green.

campaignExitCode (fanout-run.ts) is now the one place that decides the exit
status, from what the command line ends on: 2 when a driver refused real
passes or a forced re-run collides with a recorded verdict, 1 when at least
one pass failed, 0 otherwise. main applies it after a campaign and on both
activation refusals; usage errors still exit 2 before any campaign.

fanout-cli.test.ts: 7 policy cases (all passes ok, nothing to run, one
failed, every pass failed, a collision, a collision with a failure, a
refusal) and one case that main exits through the policy and only through
it (no literal exit 1 left, the outcome and both refusals routed through
campaignExitCode, the non-zero exit applied).

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME):
   - `if (end.failed > 0) return 0;` in campaignExitCode: 2 red (one failed,
     every pass failed);
   - main's `if (code !== 0) process.exit(code);` replaced by `void code;`:
     2 red (the wiring case, and the CLI case "--force refuses to overwrite
     a recorded verdict", which expects exit 2).
2. Would an equivalent refactor redden them: the policy cases, no; they map
   outcomes to codes. The wiring case pins how main calls the policy, on
   purpose: that wiring is what the lost test used to cover, and no real
   pass can reach it from the command line until CA-32 holds.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
Merge origin/migrate/recover-code into work/wp3-inv-vectors
a75da2d 
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): adaptive-attacks belongs to WP-7 after the WP-4 split
512e6c0 
Round 3, finding G5. The owner split WP-4 on 2026-10-26 (#29): WP-4 keeps
CA-22 and CA-32, CA-31 moves to the new work package WP-7 (#30). The
base was caught up first by a merge commit.

The condition adaptive-attacks now names WP-7; its source cites ADR-0045
and CA-31 and never mentioned WP-4, so it is unchanged. The header of
fanout-activation.ts names, for each condition still unmet, its exact
package: WP-4 for telemetry-egress-refused and pass-read-confined, WP-7
for adaptive-attacks.

Count, `grep -n 'WP-4' tools/review/*.ts tools/review/vectors/*.ts` before
the change: 11 found, 11 classified. Corrected (5): the header line, the
workPackage of adaptive-attacks, and three test lines that pinned it (the
unmet list, its title, the refusal line "CA-31; delivered by WP-4").
Kept (6): the workPackage of telemetry-egress-refused and of
pass-read-confined, the two unmet-list entries for those conditions, the
expected sixth condition, and the refusal line "CA-22; delivered by WP-4".
The test text follows the table, not the current output.

Gate-integrity questions for the changed tests:
1. What makes them red (env -i PATH HOME): the workPackage of
   adaptive-attacks set back to "WP-4": 2 red (the unmet list, and the
   refusal message naming its package).
2. Would an equivalent refactor redden them: no. They read each unmet
   condition's package from the table and the message, which is the fact
   the split changes.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): one synchronous write per journal line, never two writes…
993692c 
… in flight on a handle

Follow-up to G2. The required check failed once on 4949625: in vector 3,
"graph driver: a consigne in detail changes nothing the campaign decides",
the pass beta of a valid campaign failed (failed 1 against 0). The next
head, 512e6c0, passed the same check, and the case did not fail locally in
15 runs of vector 3 nor in 10 runs of the three vectors. The CI log does
not show the error of that pass, so the cause is not established.

The one concurrency introduced by G2 is two passes writing through one
FileHandle, and the Node contract of FileHandle.write forbids issuing a
write before the previous one settled. Measured on macOS, 3 runs of 4000
concurrent writes through one handle: 0 rejected, 0 lost, 0 torn; Linux
not measured. The sink now writes each line with writeSync on the
descriptor: one write(2) in O_APPEND mode, no write in flight to overlap.
The descriptor is still opened once after the last link check, and still
closed in dispose.

The case that failed now asserts first that no pass of either campaign
carries an error, so a recurrence names the error instead of only the
decision it changed.

Gate-integrity questions:
1. What makes it red: the G2 mutant still does (env -i PATH HOME): the sink
   made to reopen by path for each event reddens both race cases of vector 1
   against the new sink. The added assertion reddens on any error in a
   valid campaign, the failure seen on 4949625.
2. Would an equivalent refactor redden it: no. A valid campaign carrying
   no error is its contract.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): tilde fences and indented JSON blocks also refuse the ve…
374143e 
…rdict

Round 3 complement, finding G1b (orchestrator's cross-check of G1 on
993692c). Two replies were accepted while a reader sees a second verdict:
a second block fenced with `~~~json`, and a JSON object on an indented code
line, since CommonMark opens a code block with either.

extractVerdict now also refuses any run of three or more tildes, anywhere
in the reply (a tilde fence or tildes inside the verdict itself), and, once
the verdict block is removed, any line indented by four spaces or a tab
that starts a JSON object or array. Ordinary prose stays accepted: a list,
a single tilde, an indented line of text.

Vector 3 gains 4 cases (39): per driver, an approval followed by a
tilde-fenced rejection with a blocking finding, and the same with the
rejection on an indented line; nothing is recorded. fanout-core.test.ts
gains 5 refusals (a tilde-fenced second block, a bare tilde fence, tildes
inside the verdict, an indented object after the verdict, a tab-indented
array before it) and one acceptance (a list, a single tilde, indented text
lines without a brace).

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME):
   - the tilde check made `if (false) {`: 5 red (the 2 tilde vector cases,
     3 unit refusals);
   - the indented-block check made `if (false) {`: 4 red (the 2 indented
     vector cases, 2 unit refusals).
2. Would an equivalent refactor redden them: no. They state which replies
   are refused and which are read; any rule that refuses every reply showing
   a reader a second code block keeps them green, and the acceptance case
   keeps an over-broad rule from passing.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
@constantin-jais
fix(review): a journal line is written whole, a stalled write is an e…
8c5709f 
…rror

Round 3 complement, finding G2b. The sink called writeSync(journal.fd,
line) and ignored its return value: a short write would have truncated a
journal line without a word.

writeWhole (fanout-run.ts) writes the line as bytes and loops on short
writes until every byte is written; a write that returns zero or no
integer is an error ("journal write made no progress at byte N of M"). The
writes are synchronous, so no other line lands between two parts of one
line. The journal sink calls it on the descriptor opened once per campaign.

fanout-run.test.ts (new): with a stand-in writeSync that writes at most 1,
3, 7 or 1000 bytes per call, the whole line (UTF-8 included) is written; a
stand-in that stalls after 5 bytes makes writeWhole throw; the descriptor
given is the one written to.

Gate-integrity questions for the added tests:
1. What makes them red (env -i PATH HOME): the return value ignored,
   `offset = bytes.length;` in place of `offset += written;`: 4 of 6 red
   (the three short-write cases, which find the line truncated, and the
   stall case, which no longer throws). The G2 race cases of vector 1 still
   go red against this sink when it reopens the journal by path per event.
2. Would an equivalent refactor redden them: no. They state that the bytes
   reaching the descriptor are the whole line, whatever the chunking, and
   that a stalled write fails loudly.

Work-package: #19 WP-3
Signed-off-by: Constantin Jais <74049135+constantin-jais@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@constantin-jais