Skip to content

review: the repo's merge process in docs/process.md, locked, outside the decisions index [PROTECTED: owner merges] - #920

Merged
jayden1711 merged 20 commits into
mainfrom
docs-merge-rule
Oct 21, 2026
Merged

review: the repo's merge process in docs/process.md, locked, outside the decisions index [PROTECTED: owner merges]#920
jayden1711 merged 20 commits into
mainfrom
docs-merge-rule

Conversation

@jayden1711

@jayden1711 jayden1711 commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Intent: Record the owner's merge rule for this repo's own PRs (owner decisions, 2026-10-21) in a separate process page, docs/process.md, instead of docs/design.md. The engine indexes design.md as recorded decisions for its planner agents, so the rule must not live there. The new page is locked like the design rules and kept out of that index. Since round 5, every path the index reads, and the index code itself, is locked too, so a PR that brings a copy of the rule into a committed record needs both reviews. That is not the only way to the planners. docs/process.md lists the known open routes, for example project instructions loaded into planner runs (#973). The list is not proven complete, and the by-design closure is #975 (open: planner input will come only from an allowlist enforced in code). The page states the rule fail-closed, as the adversarial reviews asked.

Risk class: protected. It changes src/review.ts (engine, and the self-modification locks), src/plan-decisions.ts (engine, now locked), .worklane/review.yaml (engine) and locked tests (engine-guards). By its changed files it is in the critical set: review levels and locks, plus engine-guards paths (inferred from docs/process.md's own list). It needs the second adversarial review.

Round 16 (main moved to b3176d3e: #947 and #899; one body count fix)

Head b36a9525: a merge of origin/main b3176d3e into 14cdf561, signed off with no other trailers (measured: git log -1; npm run check:dco -- 14cdf561 HEAD printed "DCO: all commits signed off"). This push resets both reviews, and the combined check must be built on b3176d3e or later.

Main merge (#899)

git merge-tree --write-tree --name-only 14cdf561 b3176d3e showed one conflicted file, src/review.ts, with one hunk (measured). Everything else, including all of #947, merged cleanly.

Hunk This PR's side (14cdf561) main's side (#899) How it was kept
src/review.ts, after hidesSubmodules (the only hunk) The decision-record JSDoc, DECISION_FILES, DECISION_DIRS, DECISION_RECORD_GLOBS and onePartPerGlob The harness-config JSDoc, export const HARNESS_CONFIG = 'harness-config', // check-table: never-shrink and const HARNESS_CONFIG_GLOBS = [...] Both kept whole and in order: this PR's block, then #899's block (with its own /** opener restored), then SELF_MODIFICATION as before. The resolution used Edit only. It removed the three conflict markers and added one blank line and one /** line

The rest of #899's change to src/review.ts merged without conflict: [HARNESS_CONFIG]: HARNESS_CONFIG_GLOBS in LOCKED_CATEGORIES, ALWAYS_L3 = [SELF_MODIFICATION, HARNESS_CONFIG], DEFAULT_CATEGORIES' [HARNESS_CONFIG] key and computeLevel's never-fewer harness-config globs.

Nothing lost (measured, by a scratch script that reads main, the head and the merged tree, and edits no repo file):

Body fix: round 15's registry counts left out the 5 template-literal GUARD_FILES entries. 73/149/150 are now 78/154/155 in the round 15 section below (measured: the same scratch count at 36eafd03, 7b0a96a0 and 14cdf561: 78, 154 and 155).

Verified on the merged tree (measured, with a temp HOME, TMPDIR and state dir; build through npmw.sh):

Adversarial pass (round 16)

Attack on my own change Result Label
Did keeping both blocks drop a line from either side? No. 0 lost lines across 1458 files, and 0 lost entries in SELF_MODIFICATION_PARTS, NOT_YET_FILES, GUARD_FILES, LOCKED_CATEGORIES and HARNESS_CONFIG_GLOBS measured
Did a #886 mark or registration go missing? No. Marks: union 78, merged 78. Registrations: 156 = main's 155 plus this PR's one measured
Does #899's lock still hold with this PR merged, against a review.yaml that empties harness-config's globs and drops it from L3? Yes. In both the engine scope and a managed app's, .worklane/tests.yaml, .claude/settings.json and CLAUDE.md stay L3 with harness-config (scratch computeLevel run) measured
Do the two locks fight over .worklane/decisions.md and .worklane/decisions/**? No. In the engine's scope they are L3 with both self-modification (part "the planners' decision records") and harness-config. In a managed app's scope, self-modification stands aside as before (engineSourceGlob), and #899's harness-config keeps them L3. That is #899's tightening, not a loosening by this PR measured
Do docs/process.md, docs/design.md, the decision records and src/plan-decisions.ts keep their parts after the merge? Yes, the same parts as at 14cdf561 ("the engine repo's merge process", "the release procedure and the design rules", "the planners' decision records", "the planners' decisions index") measured

Round 15 (round-14 delta check issuecomment-6311712976: PASS on the repo; merge main, two body fixes)

Head 14cdf561: a merge of origin/main 7b0a96a0 into 36eafd03, signed off with no other trailers (measured: git log -1). main brought #904 and #886. #886 moved check registration from central lists to in-file marks. This push resets both reviews, and the combined check must be built on 7b0a96a0 or later.

The merge (measured):

This PR's registrations, re-expressed (measured):

Registry comparison (measured): a scratch script runs registrationsOf over every markable tracked file and reads GUARD_FILES at each rev.

  • The merged tree has 155 registrations (corrected in round 16 from 150: the count left out the 5 template-literal GUARD_FILES entries).
  • origin/main 7b0a96a0 has 154 (corrected from 149), and 0 are lost in the merged tree. The one extra is guard src/plan-decisions.ts, this PR's.
  • The pre-merge head 36eafd03 has 78 (corrected from 73; its GUARD_FILES entries, 73 quoted and 5 template literals; its other registrations were central lists the script doesn't read), and 0 are lost.
  • main's own REGISTERED_BEFORE_MARKS comparison test passes on the merged tree, so the 75 pre-mark registrations all still register.

How NOT_YET_FILES meets #886's never-grow mark:

  • The mark sits on the whole multi-line list, so this PR's 13 entries are part of the table it holds.
  • Against main, the list grows, and the hold flags it. A scratch run of weakeningReasons over the diff from 7b0a96a0 to the merged tree reports it as the declaring line changed, which is fail-closed (measured).
    • The same run flags SELF_MODIFICATION_PARTS's declaring line, engineSourceGlob's use of ENGINE_TREE and ENGINE_ROOT_FILES, the added entries in a negative-path list in engine-guard-locks, and the replaced lines of src/plan-decisions.ts. All are disclosed in earlier rounds (round 6, round 7 and A12).
  • These are expected waits: the PR is critical, and the NOT_YET_FILES growth is disclosed under principle 9 below.
  • Once it merges, the 13 entries are the table's base, and only later growth waits (inferred, from Merge policy: a check table or lint test registers itself where it's declared; only removing or loosening one waits (#869) [PROTECTED: owner merges] #886's base-to-head comparison).

Body fixes:

Verified on the merged tree (measured, with a temp HOME, TMPDIR and state dir):

  • 9 named files gave 152 tests, 152 pass, 0 fail: plan-decisions, engine-guard-locks, merge-policy, review, docs-guardrails, changelog, changelog-fragments, changelog-fragments-selfcheck and not-yet-files-locks.
  • main's new registry tests (merge-policy-chars, -decoy, -giveup, -paren, -raw, -reader and -tip) gave 31 tests, 31 pass, 0 fail.
  • build and typecheck exit 0; the denylist reports "1436 files clean"; the push gate reported "1 commit(s) and 1 tip(s) clean".
  • The push gate also noted that the installed hook differs from main's scripts/push-gate-hook.mjs. Reinstalling it is a host setup step, so I left it for the orchestrator.

Adversarial pass (round 15)

Attack on my own change Result Label
Did keeping one side of either conflict drop a registration? No. Both marks are kept, both PR-side declarations are kept, and the registry comparison shows 0 lost from main or from the pre-merge head measured
Does tableSpan still read SELF_MODIFICATION_PARTS behind onePartPerGlob([ (an opening ( and [, closed by ]);)? Yes. The registry backstop and the merge-policy tests pass, and a table the hold can't find fails that backstop measured (tests)
Does the merge bring anything beyond main into the PR's diff? No. git diff --stat 7b0a96a0 HEAD lists the same 12 files measured
Did the hold go quiet on any of this PR's disclosed loosenings after the merge? No. The scratch weakeningReasons run still flags the NOT_YET_FILES and SELF_MODIFICATION_PARTS declarations and the engineSourceGlob change measured

Round 14 (round-13 second review issuecomment-6311276826: wording only, no code route)

Head 36eafd03, one commit on 89425a73, signed off with no other trailers (measured: git log -1). It changes docs/process.md, docs/design.md and the changelog fragment. Both docs are protected, so this push resets both reviews. origin/main is still 9f9533aa (measured: the push gate).

  • process.md:7, the task-text sentence (A5). It said the task text and the checkout are "data … not decisions or instructions". The review measured that as false: the issue text comes first in the planner's brief, unfenced. It now reads: "The task text (the issue's title, body and done-when list) is the planner's assignment: it tells the planner what to plan, and the code doesn't mark it as data. The checkout's code is data that planners read by design. Neither is a decision record." It then points to the agent-written task text entry.

  • planner runs: take input only from an allowlist enforced in code #975 is marked open (A6). The changelog fragment now reads "planner runs: take input only from an allowlist enforced in code #975 (open): planner runs will take input only from an allowlist enforced in code". process.md:7, this body's Intent and its "What remains" line say the same.

  • Three more known routes go under planner runs: take input only from an allowlist enforced in code #975 in process.md. They are stated generically, with no file names or steps (Public adversarial suite in CI: the merge gate refuses every hostile PR case (prompt and delimiter injection, rename and copy tricks, poisoned .claude settings and hooks, credential smuggling), with the existing guard tests folded in #707):

    • task text an agent wrote;
    • text pulled in by reference, which keeps its own, possibly lower, level;
    • instruction files outside the locked set that planner runs may load (a guess).

    "What remains" lists them too, each with its label.

  • Optional, done: docs/design.md's plan-citation rule (A12). It said the index takes the records "in the checkout". It now says "committed at the plan's base", as the code does. That is main's text, so this is a one-phrase change to a protected file this PR already touches.

  • Verified (measured):

    • plan-decisions, docs-guardrails, changelog, changelog-fragments, changelog-fragments-selfcheck, engine-guard-locks and merge-policy: 120 tests, 120 pass, 0 fail, with a temp HOME, TMPDIR and state dir. No src/ change this round, so dist was current;
    • the denylist reports "1416 files clean", and the push gate reported "1 commit(s) and 1 tip(s) clean".
    • No test changed. There is no fail-without test: this round changes wording only.

Adversarial pass (round 14)

Attack on my own change Result Label
Does the new task-text sentence still say "not instructions" anywhere? No. It says "assignment", and "the code doesn't mark it as data" inferred, by reading
Present-tense #975 anywhere? process.md:7, the changelog fragment and this body's Intent now say "(open)" and "will". The src/ comments name #975 only as a pointer. Corrected in round 15: this row said the result was clean, but Round 13's section in this body still said "take input only from …" in the present tense. My grep covered the repo files and the Intent, not the body's earlier round sections. That line is fixed in round 15 measured (grep), incomplete
Does the design.md change alter an indexed section's meaning? It narrows a description to match the code, and the plan-decisions tests (which index design.md) pass measured (tests), inferred (meaning)
Exploit steps (#707) none. No file names, settings or commands in the new entries inferred, by reading

Round 13 (round-12 second review issuecomment-6310912283: text only, no code route)

Head 89425a73, one commit on 062beb8f, signed off with no other trailers (measured: git log -1). docs/process.md is protected, so this push resets both reviews. origin/main is still 9f9533aa (measured: the push gate).

For the fourth round running, a review found routes the list left out. This round fixes the class: the list no longer claims to be complete, and nothing else claims it either.

  • The list says what it is. process.md's lead now reads:
    • "These are the known open routes … Reviews found them one by one, and the list is not proven complete: assume there are others."
    • It names the by-design closure: planner runs: take input only from an allowlist enforced in code #975 (open): planner and plan-reviewer runs will take input only from an allowlist enforced in code. (Corrected in round 15. This line said "take", in the present tense, until then.)
    • Other text "points here and never says a route is the only one". Round 10's wording "the one list" is gone.
  • Two new routes, listed under planner runs: take input only from an allowlist enforced in code #975. Both are stated generically, with no file names or steps (Public adversarial suite in CI: the merge gate refuses every hostile PR case (prompt and delimiter injection, rename and copy tricks, poisoned .claude settings and hooks, credential smuggling), with the existing guard tests folded in #707), and both are on main too (inferred):
    • messages into a live planner or plan-reviewer run, where the coordinator takes the request's own word for who sent it;
    • the owner's revise note, read from the event log without checking who wrote it.
  • Optional item, done. One sentence says the plan reviewer's send-back reason reaches the planner as feedback, and can carry what the reviewer's known inputs hold.
  • Completeness claims removed. Each now says what a PR through a committed record needs, and that it isn't the only way in:
  • The src/ changes are comments only, and all on lines this PR added. Against main, these three files remove the same 21 lines as before this round (measured: git diff origin/main | grep -c '^-[^-]'), so the weakening hold sees nothing new (inferred).
  • Verified (measured):
    • plan-decisions, docs-guardrails, changelog, changelog-fragments, changelog-fragments-selfcheck, engine-guard-locks and merge-policy gave 120 tests, 120 pass, 0 fail, with a temp HOME, TMPDIR and state dir;
    • build and typecheck exit 0; the denylist reports "1416 files clean"; the push gate reported "1 commit(s) and 1 tip(s) clean".
    • No test changed. There is no fail-without test: this round changes wording only.

Adversarial pass (round 13)

Attack on my own change Result Label
grep this body for "only route/way", "the one list", "no other route", "listed once", "any route" and "can't reach the planners" the only live hit is the Intent's "That is not the only way to the planners". The other hits are earlier rounds' sections quoting old wording measured
Does my new send-back sentence claim completeness? A first draft said the reviewer reads "only" those inputs. It now says "known inputs" inferred, by reading
Does the Planner line enumerate the list (so it could drift from it)? A draft did. It now points to the list without naming its entries inferred
Is "As a record, a copy can enter only as a regular file committed at a record path" still a completeness claim? It is scoped to index records, which the round-7/8 tests measure. Other ways in are explicitly the list measured (tests), inferred (scope)
Exploit steps (#707) none in the new entries inferred, by reading

Round 12 (round-11 second review issuecomment-6310598355: text only, no code route)

Head 062beb8f, one commit on f672543c, signed off with no other trailers (measured: git log -1). It changes docs/process.md only. The file is protected, so this push resets both reviews. origin/main is still 9f9533aa, already merged in round 11 (measured: the push gate).

Adversarial pass (round 12)

Attack on my own change Result Label
Does the #973 entry claim a classification I can't back? It says "no subject of the critical set below names it" and "may get one review", not that it is outside the set. The page's own "when unsure, the PR is in" rule still applies inferred, by reading
Does the scoped lead now leave out a route the earlier rounds listed? No. #962, #956 and #969 are decision text; #970 and #973 are text planners are told to rely on, as records or instructions inferred
Exploit steps (#707) none. The #973 entry names no file, setting or command inferred, by reading
Does any other place still say "indexes nothing" for an unreadable page? The changelog already said "no decision record is indexed. Plans can still cite the decisions already made". In this body, round 5's table row and round 7's subfolder line are now scoped the same way measured (grep)

Round 11 (round-10 second review issuecomment-6310271940; round-10 delta check issuecomment-6310344340)

Head f672543c (measured: git push). On d13857d9 there are three commits, each signed off with no other trailers (measured: git log):

  • 6a187284, the list;
  • 4b50017c, the delta check's items;
  • f672543c, a merge of origin/main 9f9533aa.

The merge re-derives exactly: git merge-tree --write-tree 4b50017c 9f9533aa gives tree cc4ff684, the merge commit's tree (measured). The combined check must be built on 9f9533aa or later. docs/process.md is protected, so this push resets both reviews.

Neither check found a code route. The second review found three routes that the open-routes list left out, all on main too, so none is a regression (inferred). This round only adds them to the list, and the text that points to the list now points to the new routes as well.

Adversarial pass (round 11)

Attack on my own change Result Label
Does any sentence outside process.md still enumerate the routes as if complete? grep for "(#962, #956)" in the body: all dropped. The fragment and docstring say "such as" / "for example" measured (grep), inferred (reading)
Exploit steps for #969 or #970 in public text (#707) none. No event-log file name, path, key or command is given. Each route says only what it allows inferred, by reading
Does the merge-policy-weakening.ts edit remove a registry line that main has? no. Against main, the file's diff is additions only measured
Does the merge bring anything beyond main's content? no. The tree matches git merge-tree --write-tree exactly measured
process.md:4, "an uncommitted edit … add no record" against #970 true for the index. It is now scoped "(to the index; what planners read in the checkout is an open route, below)" inferred

Round 10 (round-9 second review issuecomment-6310047202: claims only, no new working route)

Head d13857d9 (one commit on 2647eeaf, signed off, no other trailers; measured: git log -1). docs/process.md is protected, so this push resets both reviews.

For the third round running, a sentence said there was no other route into the decisions index, and a review found one it left out. The review measured #956's route (C1) with an in-memory fixture. Following #899's pattern, this round fixes the class, not the sentence:

Adversarial pass (round 10)

Attack on my own change Result Label
grep process.md, the changelog fragment and both src/ files for "only through a PR", "any route", "apart from that route", "reads nothing else", #962 and #956 each remaining "only through a PR" now sits next to the open-routes qualifier; #962 and #956 are spelled out only in process.md's list, the fragment's pointer and the PROCESS_DOC docstring measured
"What this can never do" > "Loosen a merge rule": "no text becomes citable unless … committed" false under #956 (a made decision's question is citable text). Now "no record text", plus the open-routes pointer inferred, by reading
does any src/ change remove a line that main has? no. decisions.ts: 2 lines added. plan-decisions.ts: only round 9's own 2 lines reworded measured (git diff -U0)
exploit steps in public text (#707) none: the list names each route by what it allows, with no keys or commands inferred, by reading
process.md :7, "No plan, agent or PR can cite this page" left as is. The round-9 delta and second review both read it as true for the page itself, which is excluded at any base. Copies are covered by the list inferred

Round 9 (round-7+8 delta check issuecomment-6309508803: wording only)

Head 2647eeaf (one commit on c0df669a, signed off, no other trailers; measured: git log -1). docs/process.md is protected, so this push resets both reviews.

The delta check measured, with a fixture, a route the text didn't cover: a process running as the engine's user in a task worktree can reach the source the coordinator fetches the base from. The next base is then one that process chose, and its committed records are indexed with every id check passing. The code is otherwise unchanged by this round. The by-design fix (pin the base to the forge's branch) is #962, out of this PR's scope. Per the #707 rule, no steps are given here.

Round 9, body only (round-9 delta check issuecomment-6309874629)

No new commit: the head stays 2647eeaf.

Adversarial pass (round 9)

Attack on my own change Result Label
grep the PR's docs, changelog, src and tests for the old absolute phrases ("only through a PR", "doesn't read what", "reaches the decisions index", "base sha trusted") found the places listed above as beyond the brief; each now names #962, and a re-grep finds the phrases only next to that qualifier measured
does the src edit remove or change a line in a guard file? no: 2 lines added, the original line kept as it was measured
does any public text give exploit steps? no: the route is named only as "the fetch source is reachable by a process running as the engine's user", with #962 inferred, by reading
denylist and brand rule on the new text "1410 files clean"; no product name added to src/ measured / inferred

Round 8 (round-7 adversarial review at 6edea7b, stopped before measuring; its routes forwarded as inferred)

Head c0df669a. origin/main is still d2a549f4 (measured, after a fetch).

  • A1 and A3 measured, then closed by design.
    • A1: git doesn't check a packed object's hash on read. My test builds a pack whose index maps a record's real blob id to other text, and git cat-file returns the forged text (measured).
    • A3: a worktree's .git file rewritten to point at another repository holding that forged pack.
    • At 6edea7b both put the forged record into the index (measured: decisions.md § D-1: Agents merge).
    • The fix in src/plan-decisions.ts: the index walks the base commit itself. It reads the commit, each tree down to a record folder, and each record's blob with git --no-replace-objects cat-file --batch. It recomputes each object's id from its content (<type> <size>\0<content>, SHA-1 or SHA-256 by the id's length) and compares.
    • A missing object, an object of another type, or any mismatch indexes no record. The tree entries are parsed from the verified tree bytes, so ls-tree's unverified view is no longer used.
    • The plan step (src/coordinator/decisions.ts) passes the coordinator's base sha and the instance's own clone (this.d.repo). The objects are read there, never through the worktree's .git file.
    • With every object checked against its id, no object store, however written, can change what a given base commit's records say (inferred: it would need a hash preimage). Corrected in round 9: this assumed the base sha is trusted, and it isn't fully. The coordinator takes the base from a fetch whose source a process running as the engine's user can still reach, so such a process can choose the base, and that base's records are indexed (measured by the round-7+8 delta check; decisions index: pin the base it reads to the forge's branch #962).
    • New tests: the forged pack, and the worktree .git redirect. Both index nothing through the forged store and the right record from the clone.
  • process.md:
    • :11's carve-out now reads "except those that go to the owner (see "What goes to the owner", below)";
    • :4 now says "the plan's base commit, which the coordinator gives", with the clone and id checks, instead of "the checkout's HEAD".
  • plan-decisions.ts docstrings for recordEntries and decisionIndex: "the plan's base commit (HEAD only when no base is given)" (A7).
  • A5, question text in decisions already made: filed as Decisions index cites agent- or reporter-written question text as an owner decision #956 and listed under the open routes in docs/process.md (since round 10). Not a regression: main does the same (inferred).
  • Fail-without (measured): a NEW file test/fw920r8-plan.test.ts in a fresh worktree at 6edea7b holds the two tests and their helpers, verbatim except a cast for the 4-argument decisionIndex. Result: 2 tests, 2 fail: the forged record was indexed both times.
  • After (measured):
    • the 15 named files: 284 tests, 272 pass, 0 fail, 12 todo;
    • the changelog tests again after the fragment edit: 39 pass;
    • typecheck clean; the denylist reports "1410 files clean"; the push gate reported "1 commit(s) and 1 tip(s) clean".
  • Size (measured): 1,486,452 / 4,977,040 bytes, against main d2a549f4 1,481,905 / 4,963,126. That is +4,547 tarball and +13,914 unpacked, the same 493 entries.

Round 7 (second adversarial review issuecomment-6308710896; round-6 delta check issuecomment-6308732220, PASS)

Head 6edea7b8. origin/main d2a549f4 is merged in (merge commit 29b85112, signed off; measured: rev-list --count HEAD..origin/main = 0 after a fetch). The combined check must be built on d2a549f4 or later. Every "41f6dd42" below is a past round's main, not today's.

  • A1 closed by design: the index reads only committed records (required change 1).

    • The route, measured by the review: the lock holds git changes to the record paths, but recordEntries read the working tree. So a file at a record path that no PR added was indexed and citable. Untracked files are left by a setup step (npm ci runs prepare and dependency install scripts), and pooled worktrees keep them.
    • Now recordEntries takes the records git tracks at the plan's base commit:
      • git ls-tree -r -z --full-tree <base> -- <record paths>, run through the OS layer's child-process module;
      • regular files only (mode 100644 or 100755), at an exact record path;
      • each text read from its blob with git cat-file --batch, never from the working tree.
    • Beyond the brief: the plan step passes its base commit (src/coordinator/decisions.ts) instead of the worktree's HEAD. So a setup step that commits a record in the worktree adds nothing either. --no-replace-objects means a replace ref can't stand another tree in for the base.
    • No record is indexed when any of these holds: the rev isn't HEAD or a full commit id; the checkout isn't its repository's top (only when no base is given, that is with HEAD; corrected in round 10); git fails; or git's output can't be read.
    • The round-5 working-tree checks stay as a second check: the real path is a record path inside the checkout, the record isn't the page's inode, and the page is readable.
    • This also closes, by design (inferred, from the code):
    • Managed apps (a tightening): uncommitted or symlinked records drop out of the index, and plans citing them are refused. With no base given, a checkout that isn't a git checkout's top indexes no records. The coordinator always gives a base, so the changelog no longer says this (corrected in round 10). The rest the changelog says under Upgrading.
    • Test fixtures changed: the existing plan-decisions fixtures, main's included (records() and the config-folder test), now commit their files, since the index reads only committed records. No assertion of main's changed.
      • Two assertions this PR added in rounds 4 and 5 now expect a committed symlink not to be indexed: docs/decisions.md -> ../decisions.md, decisions -> docs/decisions and docs/DECISIONS.md -> design.md. Main never had them.

  • Fail-without tests (required change 2). These are new in test/plan-decisions.test.ts:

    • untracked, staged, edited and committed-as-a-link records are not indexed, while a tracked regular record is, with its committed text;
    • a non-git directory or a subfolder indexes no record (a subfolder only with no base given; see round 10);
    • read at the base commit: a later commit, a git replace of the base and a rev that isn't a full id (main, a short id, --output=x, <sha>^) add nothing;
    • an untracked record alone isn't indexed.

    New in engine-guard-locks: a removed line in src/plan-decisions.ts raises the weakening hold.

  • False claims corrected (required change 3, principle 5):

    • "A copy can only enter through a locked record path" (Compromised component): it was false at 1d11ea3 (A1). It is now true for committed records: the index reads nothing else, except through the open routes listed in docs/process.md (since round 10).
    • process.md:5, "a reworded copy can get past it, the lock can't": it was false at 1d11ea3 (A1). It now reads: the index reads only records committed at the plan's base, every committed record path is locked, and a reworded copy gets past the backstop test but not past the lock.
    • "No text becomes citable that isn't a real record at a record path" (What this can never do): it was false at 1d11ea3 for untracked files. It is corrected below to "a record committed at the plan's base".

  • Recommendations, done:

    • src/plan-decisions.ts is added to GUARD_FILES in src/merge-policy-weakening.ts. That's an addition, under the registry permission, and a PR touching it is a critical digest.
    • process.md:11: the orchestrator merges every PR except those that go to the owner (release signing).
    • process.md:16: the reviewer who clears a conditional PASS is as independent as the review the conditions came from.
    • process.md:23: "setup and machine scripts" names the worktree setup inputs: .worklane/tests.yaml setup steps, package.json install-time scripts and what they run. It notes that the index no longer reads what they write.

  • The weakening hold (measured by the round-6 delta check): main's hold flags SELF_MODIFICATION_PARTS's changed declaration and closing lines, as expected. It will also flag this round's removed lines in src/plan-decisions.ts (inferred), since that is now a guard file. The PR waits anyway as critical.

  • Merge-order note, updated (measured by the round-6 delta check): against Merge policy: a check table or lint test registers itself where it's declared; only removing or loosening one waits (#869) [PROTECTED: owner merges] #886 c1f13341 there are now conflicts in two files:

    Whichever PR merges second keeps each mark directly above its new declaration line (NOT_YET_FILES's multi-line list, and SELF_MODIFICATION_PARTS ... = onePartPerGlob([).

  • Fail-without (measured): NEW files test/fw920r7-plan.test.ts and test/fw920r7-guard.test.ts were written into a fresh worktree at 1d11ea3. They hold the 4 new plan-decisions tests and the guard-file test. The round-4 symlink check is there as its own test, and the base-commit test calls decisionIndex through a cast, since 1d11ea3's has no rev parameter. Result: 5 tests, 5 fail. For example:

    • untracked, staged and linked records were indexed: decisions.md § D-1: Agents merge, docs/decisions/0002-link.md, docs/decisions/0003-merges.md;
    • the link docs/decisions.md § D-1: Kept was indexed;
    • the weakening hold gave nothing for the removed line.

  • After (measured):

    • 15 named files gave 282 tests, 270 pass, 0 fail, 12 todo: plan-decisions, engine-guard-locks, not-yet-files-locks, merge-policy, review, adversarial, brand, changelog, changelog-fragments, changelog-fragments-selfcheck, docs-guardrails, plan-mode, child-process, no-wall-clock-durations and no-clock-helpers;
    • typecheck clean; the denylist reports "1410 files clean"; the push gate reported "2 commit(s) and 1 tip(s) clean".
    • test/coordinator.test.ts isn't run locally (it inherits PATH). Its plan-mode cases commit their fixture files and plan on a real base, so they should be unaffected (inferred). CI runs it.

Round 6 (round-5 delta check, issuecomment-6308270567)

Head 1d11ea38. origin/main was then 41f6dd42 (measured, after a fetch). It has since moved, and round 7 merges d2a549f4.

  • CI red because of this PR: fixed (change 1).
    • The cause, measured by the delta check: round 5 put docs/design.md (a DECISION_FILES entry) in two self-modification parts. selfModificationPart names the first part. test/coordinator.test.ts:2142 walks every part's globs and expected the second, so shard 1/4 failed on Node 22 and 24.
    • The fix: SELF_MODIFICATION_PARTS is now built with onePartPerGlob, which keeps each glob only in the first part that lists it. docs/design.md stays "the release procedure and the design rules".
    • The records part is still built from DECISION_FILES and DECISION_DIRS, and every path they name is still locked. The lock itself is unchanged; only the duplicate listing is gone.
    • test/coordinator.test.ts is not edited.
    • Measured with a scratch script that mirrors coordinator.test.ts's walk (each part's glob, the first tracked file it matches, and the part selfModificationPart names): 133 globs checked, 0 wrong at the new head; at 41343a7, the docs/design.md mismatch.
    • Shard 1/4 at the new head (measured, run 39546549630, headSha 1d11ea3): success on Node 22 (job 119045901958) and Node 24 (job 119045901964).
    • New test in engine-guard-locks: no glob is in two parts; each exact path names its own part; docs/design.md is the design rules'; every DECISION_FILES and DECISION_DIRS path is still a lock glob.
    • A side effect to know: the SELF_MODIFICATION_PARTS declaration line and its closing line change (= onePartPerGlob([ ... ]);). It is a never-shrink table, so the weakening hold flags those two changed lines (inferred). No glob is dropped from the lock: LOCKED_CATEGORIES and selfModificationPart give the same answers for every path as at 41343a7, except that docs/design.md now has one name (measured: the partnames walk, and the 12 named files below).
  • process.md:23 is additive only (change 2).
    • The owner's subjects define the critical set. A PR is in when its changed files touch any of them, or when computeLevel puts any changed file under self-modification, engine-guards or release-signing.
    • The code can only add a PR, never take one out. The page names the delta check's examples that the code doesn't classify but the subjects cover: scripts/lib/signing.mjs, src/update-drain.ts and src/updater-setup.ts.
    • When it's unsure whether a file is under a subject, or the code's result can't be run or read, the PR is in.
    • Round 5's line about :23 below is corrected to match.
  • Claim corrected (change 3, principle 5). Round 5's "make recordEntries index fewer records, never more" is not true in one case (A8d, measured by the delta check). On main, a decisions folder with a dangling entry throws ENOENT, so the plan step crashes. The head skips that entry and indexes the real records (27 in that fixture). No text becomes citable that isn't a real record at a record path (inferred). Corrected in "What this can never do".
  • Merge-order note (change 4): this PR now conflicts with Merge policy: a check table or lint test registers itself where it's declared; only removing or loosening one waits (#869) [PROTECTED: owner merges] #886 (3b23fa41) in test/merge-policy.test.ts (measured by the delta check with git merge-tree). Merge policy: a check table or lint test registers itself where it's declared; only removing or loosening one waits (#869) [PROTECTED: owner merges] #886 adds // check-table: never-grow above NOT_YET_FILES, and this PR rewrites that declaration as a multi-line list. Whichever PR merges second keeps the marker above the multi-line list, and the combined check must cover that merge.
  • Fail-without (measured): the new test, written as a NEW file (test/fw920r6-locks.test.ts) into a fresh worktree at 41343a7, gave 1 test, 1 fail: a glob in two parts, actual ['docs/design.md'].
  • After (measured):
    • 12 named files gave 260 tests, 248 pass, 0 fail, 12 todo: plan-decisions, engine-guard-locks, not-yet-files-locks, merge-policy, review, adversarial, brand, changelog, changelog-fragments, changelog-fragments-selfcheck, docs-guardrails and plan-mode;
    • typecheck clean; the denylist reports "1397 files clean"; the push gate was clean.
    • test/coordinator.test.ts was not run locally: it inherits PATH, and its flow may reach host tools. CI runs it.

What remains

The known open routes are listed in docs/process.md ("Open routes into the decisions index"). Reviews found them one by one, so the list is not proven complete. The by-design closure is #975. The #962, #956, #969, #970, #973 and #975 lines below are highlights of that list, and the list itself is the reference.

  • Case-variant locked/record paths on case-insensitive checkouts (A10 from #920) #953: A10, case-variant record paths on case-insensitive checkouts.

    • The risk: on macOS or Windows, a file such as Docs/adr/x.md or docs/ADR/x.md could be read through a record path. Its git path isn't locked: the delta check measured these variants as L0 [docs].
    • Since round 7, the index takes record paths from git's tree at the base, compared byte for byte, and reads the text from git. So such a file is no record on any filesystem (inferred; it can't be measured on Linux).
    • Case-variant locked/record paths on case-insensitive checkouts (A10 from #920) #953 can confirm that on a case-insensitive filesystem and close, or keep the lock-side question open.

  • Agents act as the owner's GitHub account and main needs 0 approvals: carry out the approved wl-dev migration (own App, owner-only main ruleset) #917: every agent posts as the same GitHub account (the round-4 named residual).

  • decisions index: pin the base it reads to the forge's branch #962: which commit is the base. The index checks everything it reads from the base commit, but not which commit that is. The coordinator takes it from a fetch whose source a process running as the engine's user can still reach (measured by the round-7+8 delta check). Not a regression: main reads the working tree, which is weaker (inferred). The by-design fix is pinning the base to the forge's branch, out of this PR's scope.

  • Decisions index cites agent- or reporter-written question text as an owner decision #956: question and answer text in decisions already made. A made decision is indexed as question -> answer. The question can be an agent's or a reporter's words, and the answer is one of the asker's options, so for an agent's question an agent wrote it too, uncapped and unredacted (A5 in round 8, measured by reading the code; the answer measured by the round-10 second review). Not a regression: main does the same (inferred).

  • decisions index: trust only answers recorded through the answer path #969: decisions already made that no one answered. The index doesn't check who answered or how, so whatever can write the instance's event log can add a citable decision (measured by the round-10 second review). Not a regression (inferred).

  • plan brief: give planners the committed record text, not the checkout copy #970: the checkout copy planners read. Planners and the plan reviewer are told to read the record files in their checkout, which can differ from the committed text the index cites (measured by the round-10 second review with a fixture). Not a regression: main's index read the checkout itself (inferred).

  • planner runs: load no project-level agent instructions from the checkout #973: project instructions in planner runs. Planner and plan-reviewer runs load the agent's project instructions and config from their checkout. That text reaches every planner as instructions. In this repo it is L3, but no critical-set subject names it (the run's command line, working directory and categories were measured by the round-11 second review with a probe; the loading itself is inferred from vendor docs). Not a regression: main loads them too (inferred).

  • planner runs: take input only from an allowlist enforced in code #975 (open): planner input from an allowlist enforced in code, the by-design closure. These known routes are listed under it:

    • Messages into a live planner or plan-reviewer run. The sender's role is taken on trust. The trust step was measured by the round-12 second review with a probe; delivery to planner runs is inferred.
    • The owner's revise note, read from the event log with no check of who wrote it (inferred, by reading).
    • Task text an agent wrote, for example a goal's child issues, which start without an owner step (inferred by the round-13 second review, from reading).
    • Text that project instructions or config pull in by reference. It keeps its own, possibly lower, level (inferred: the level of a docs file was measured, and the reference behaviour comes from vendor docs).
    • The run user's own user-level memory, and a local memory file in the checkout, if planner runs load them (a guess, not measured).

    None is a regression: main has them all (inferred).

Round 5 (second adversarial review issuecomment-6307750538, round-4 delta check issuecomment-6307787151)

Head 41343a76. origin/main had not moved (measured: rev-list --count HEAD..origin/main = 0 after a fetch), so there was nothing to merge.

  • The decision records are locked, by design (review change 1, delta change 2).
    • Two new SELF_MODIFICATION_PARTS in src/review.ts:
      • "the planners' decision records": every DECISION_FILES path, and every DECISION_DIRS folder itself (a symlink to anywhere) and <dir>/**;
      • "the planners' decisions index": src/plan-decisions.ts.
    • The globs are built from the index's own lists, not a copy. The lists now live in src/review.ts, and src/plan-decisions.ts imports and re-exports them, so every existing import still works.
      • Why they moved instead of review.ts importing plan-decisions.ts: src/review.ts is in the agents' tool guard's pinned import tree. Importing plan-decisions.ts there would pull it and src/events/redact.ts into that tree. test/review.test.ts would then fail (GUARD_TREE; redact.ts is in LEFT_THE_TREE), and adding them to GUARD_TREE would be an allowlist entry in a locked test (inferred from the import lines; not run).
    • Mirrored under engine-guards in .worklane/review.yaml (13 record paths plus src/plan-decisions.ts). Also in locked, present and before in test/engine-guard-locks.test.ts, and in NOT_YET_FILES (below).
    • A new test reads the index's own lists. For each path the index reads (each file, and an entry in each folder), it builds a fixture checkout and checks that recordEntries indexes the record there. It then checks the path is L3 self-modification under all three review.yaml files and under engine-guards (measured: passes).
    • Engine repo only. In a managed app, the new globs stand aside: engineSourceGlob now also matches the decision-record globs, exactly. A managed app's records keep the categories its review.yaml gives them (measured: the before table for all 14 new paths, as measured at cfc69f0; and test/review.test.ts's exact list of the globs that hold in every repo still passes unchanged).
    • Two new locked-paths.json cases wait with high-risk: self-modification:
      • decision-record-docs-only: a new docs/decisions/0001-x.md with a reworded copy of the rule;
      • decisions-index-code: an edit of src/plan-decisions.ts.
    • With the lock, the review's attacks 1, 2 and 4 are closed as a class. A whole, partial or reworded copy in any decision record, a new record folder and an edit of the index code are each L3 and in the critical set, whatever the text says (measured: the cases and the new test).
  • The index fails closed (delta change 1, review recommendation). recordEntries now indexes a record only when all of these hold:
    • its real path (realpathSync) is inside the checkout's real path;
    • that real path is itself a record path: a DECISION_FILES entry, or a file directly in a DECISION_DIRS folder;
    • it isn't docs/process.md (same device and inode);
    • each of those could be told. A realpath or stat error on a record skips that record. When docs/process.md exists but can't be stat'd (anything but ENOENT: EACCES, ELOOP, ...), or the checkout's own realpath fails, no record is indexed.
    • Closed (measured, new tests):
      • review attack 3: a planning worktree at <clone>/.claude/worktrees/plan-1 with docs/decisions.md -> ../../../../docs/process.md, plus links out to the clone's other files and a decisions folder linked to the clone's docs/;
      • an ELOOP docs/process.md;
      • an unsearchable docs/ (mode 0600) with a top-level decisions.md hard-linked to the page.
    • Beyond the brief (my adversarial pass, attack A6 below): a record that links to another file inside the checkout is no longer indexed, unless that file is itself a record path. The lock holds the link, not its target. So once a link docs/adr -> ../notes merged, a later L0 edit to notes/0001.md changed an indexed record (measured on the first round-5 build: indexed, and notes/0001-merges.md unlocked). A link between record paths (decisions -> docs/decisions, docs/DECISIONS.md -> design.md) is still indexed (measured).
    • Managed apps (a tightening, principle 1): a record that links out of its checkout, or to a file that isn't a record path (decisions.md -> notes.md), drops out of the index. A plan that cites it is then refused, with the message that lists the record paths. When docs/process.md exists but can't be read, no record is indexed. Plans can still cite decisions already made. The changelog fragment says this under Upgrading. Round 4's test asserted that docs/decisions.md -> notes.md is indexed; that assertion is now a link between record paths. The PR added that assertion, so main never had it.
  • process.md (review change 2, recommendations, delta nits):
    • :23, the critical set: both paths of a rename or copy count, including the old path that GitHub's PR file list leaves out. The engine's own computeLevel or selfModificationPart, run on the PR's diff, decides. Corrected in round 6: that wording let the code take a PR out of the owner's subjects. The code now only adds, and the subjects decide. If it can't be run or read, the PR is in.
    • :13, the combined check: built on main's tip, and main's tip must still be its base commit when the batch's first merge runs (compare the SHAs).
    • :15, green: the newest run of each required check is success. An older success doesn't count when a newer run failed or hasn't finished.
    • :41: "before a batch starts (a single PR included), and after every batch". This removes the clash with "strict is turned off for the batch".
  • False claims corrected (principle 5):
    • This body's round-4 "The symlink route is closed by design": false. A symlink out of the checkout to the clone's copy was indexed (review attack 3, delta check). Now closed as above.
    • process.md:3 "never includes it, even through a symlink or hard link": false, for the same reason, and a partial copy was indexed. The page now says what keeps it out: the index code (what it leaves out, and that it can't recognise a copy) and the lock on every record path.
    • plan-decisions.ts "A copy of its text is held by test/plan-decisions.test.ts": overstated. Only a copy carrying one of the test's four phrases, inside the checkout, was held. The comment now says the code can't recognise a copy, and that the lock holds the record paths instead.
    • The changelog fragment's "never includes it": false. It is rewritten to say what is closed and how.
    • "What this can never do: the only code change adds a self-modification glob": false since round 4, which changed recordEntries. Corrected below.
  • Loosening disclosure (principle 9). This PR adds 13 entries to NOT_YET_FILES in test/merge-policy.test.ts (owner decision Review levels: the security pages, their citations test, the adversarial suite and the Dockerfile are locked as engine guards (#858) [PROTECTED: owner merges] #885 option 2: added entries, each paired with a new lock). That list exempts a glob from the check "self-modification: every glob names real engine files (none dead)". The check's finding for these 13 globs would be "matches no file". That finding is true but not a fault: they are locks placed before the first record exists (measured: no tracked file matches them). test/not-yet-files-locks.test.ts passes, so each entry is paired with its lock. NOT_YET_FILES is a never-grow table, so the weakening hold flags these lines (inferred).
  • Delta nit, size: measured below.

Round 4 (second adversarial review, issuecomment-6307564006)

  • The symlink route is closed by design. Corrected in round 5: false for a symlink out of the checkout. See Round 5.
    • recordEntries (src/plan-decisions.ts) skips any decisions file or folder entry that is the checkout's docs/process.md. It compares identity (device and inode, through symlinks), so a hard link is covered too.
    • A checkout without that file indexes exactly as before, and so does a record that links to any other file (measured: the new test's second fixture). Round 5 changed this: a record that links to a file that isn't a record path is no longer indexed.
    • A copy of the text under another name is caught by the test only when it carries one of the test's four phrases (corrected in round 5; the lock on every record path now holds copies).
  • The backstop is locked. test/plan-decisions.test.ts is now under "the tests of the harness controls" in SELF_MODIFICATION_PARTS and under engine-guards in review.yaml. It is also in locked, present and before in test/engine-guard-locks.test.ts.
  • The stale-base route in process.md is closed:
    • every merge goes through the procedure, and a single PR is a batch of one;
    • the orchestrator reads the ruleset and confirms strict up-to-date is on before a batch starts and after every batch, and nothing merges until it is;
    • a combined check counts only if it was built on main's tip, still main's tip at the first merge (round 5);
    • after a stopped batch, the remaining PRs need a new combined check.
  • Recommended changes, taken:
    • the signer paths are examples of every place a release's signature is checked, not the whole set;
    • the owner's explicit OK, and adding an exception, allowlist entry or skip is a weakening;
    • green is on the exact head commit or the exact combined commit, and an unreadable or empty required list means nothing is green.
  • Fail-without (measured): with the tests written and the fix not yet in, engine-guard-locks and plan-decisions gave 3 fails: the two lock tests for the test file, and the new symlink and hard-link regression. After the fix: 0 fail.
  • Named residual: every agent posts as the same GitHub account until Agents act as the owner's GitHub account and main needs 0 approvals: carry out the approved wl-dev migration (own App, owner-only main ruleset) #917, so GitHub can't show that a PASS came from someone other than the builder or the orchestrator. Until then, independence rests on the orchestrator's records (inferred; no change asked).

What changed (round 3, with rounds 4 and 5 above on top)

  • docs/process.md (new): the rule.
    • Both reviews: each is on the current head and posted on the PR naming that head. Any push to a critical PR needs both again. "Independent" means not the builder and not the orchestrator, and for the second review, not the first reviewer. A conditional PASS is not a pass.
    • The critical set (widened by the owner): the 7 areas, plus every self-modification lock in src/review.ts, plus review.yaml's release-signing and engine-guards (this page included). The changed files decide, never the title marker; when unsure, a PR is in.
    • Green: the newest run of every required check reported success on that exact head or tree. Pending or missing is not green, and the combined tree is held to the same rule.
    • Release signers: a change to who may sign a release goes to the owner.
    • The batch:
      • It stops on any mismatch, failed step or check that isn't green.
      • The strict toggle is the orchestrator's only ruleset change.
      • After each merge, main's tree is compared with that step's tested tree, and at the end with the whole combined tree.
      • Strict comes back on through an exit trap, on a normal or an error exit (not on SIGKILL).
    • The owner list: it is scoped "for merges" and keeps "physically". It adds that the owner's OK for removing or weakening a check (rule 10) stays.
    • The marker: [PROTECTED: owner merges] no longer means the owner merges. It marks a protected path, which is far wider than the critical set.
    • Scope: the harness's merge policy, review levels and holds (design.md §9 and §10, guardrails.md) are unchanged, and no plan can cite the page to let the engine merge more.
  • docs/design.md: §20 is removed. Its untitled intro gets one pointer sentence. The index drops that intro, because sectionsOf drops the level-1 title section, so the pointer is not indexed (measured by the test below).
  • src/review.ts: a new self-modification part, "the engine repo's merge process", docs/process.md (round 3). Round 5 adds the decision-record and index parts, and the record lists (above).
  • .worklane/review.yaml: docs/process.md added under engine-guards, beside design.md, and the round-5 entries.
  • Tests:
    • test/plan-decisions.test.ts: in this repo, no decisions-index entry carries the merge process, and a fixture's docs/process.md is never indexed. Round 5 adds the fail-closed cases.
    • test/engine-guard-locks.test.ts and test/merge-policy.test.ts: docs/process.md, the decision records and src/plan-decisions.ts are self-modification and L3, never auto-merge, and sit under engine-guards. They are not locked in a managed app.
    • Adversarial cases merge-process-docs-only, decision-record-docs-only and decisions-index-code.
  • changelog.d/920-lock-merge-process-doc.md: "Upgrading" (the index fails closed) and "Guardrail changes" notes.

Corrections to earlier claims in this body (principle 5)

  • Round 5's corrections are listed in its section above.
  • Earlier: "not read by the engine at run time". False while the rule was in design.md. The coordinator's decisions index read it, and the second reviewer measured decisionIndex returning §20. Now the rule is in docs/process.md, which no DECISION_FILES entry or DECISION_DIRS folder names, and the test holds that (measured).
  • Earlier: "Coordinator ... never read this file; no effect". False for design.md, for the same reason. It is true of docs/process.md (measured by the same test; inferred for other readers: a grep of src/ finds no reader of docs/process.md other than the lock glob and PROCESS_DOC).
  • Earlier: "under the rule recorded here it needs two independent reviewers" (round 1: "a locked path puts it in the critical set"). The round-1 text didn't say that. The page now does, because the owner widened the critical set to every self-modification lock and engine-guards path.

How it was verified

  • Round 5 fail-without (measured). I wrote the new tests as NEW files into a fresh worktree at the round-4 head cfc69f06:

    • test/fw920r5-plan.test.ts: the 4 new plan-decisions tests, verbatim;
    • test/fw920r5-locks.test.ts: the head's engine-guard-locks.test.ts, verbatim;
    • test/fw920r5-mp.test.ts: the new merge-policy assertions (part names, and L3 under a review.yaml that lists nothing);
    • test/adversarial/cases/fw920r5.json: the 2 new cases.

    Result: 12 tests, 11 fail, 1 pass. The one that passes is the managed-app "unchanged" test, which should pass before and after. Each failure is for the right reason:

    • out of the checkout: decisions.md § D-1: Agents merge, docs/decisions.md § Who merges, § The critical set, decisions/process.md and docs/adr/0001-merges.md were all indexed;
    • ELOOP: decisions.md § D-1: Kept and docs/design.md § 8. Roles were indexed;
    • unsearchable docs/: decisions.md § Who merges and § The critical set were indexed;
    • a link to a non-record file: indexed;
    • locks: selfModificationPart('decisions.md') was null, and decisions.md was L0 under this repo's review.yaml;
    • adversarial: decision-record-docs-only merged under the always-PASS stub. decisions-index-code had only high-risk: engine (src/plan-decisions.ts), with no self-modification.

  • Round 5 after (measured): all run with a scratch HOME, TMPDIR, state and held-out dirs, and claude and gh stand-ins first on PATH. gh was never called. None of these tests sets its own PATH.

    • node --import ./dist/test/isolate.js --test on plan-decisions, engine-guard-locks, not-yet-files-locks, merge-policy, review, adversarial, brand, changelog, changelog-fragments, changelog-fragments-selfcheck, docs-guardrails and plan-mode: 259 tests, 247 pass, 0 fail, 12 todo;
    • run typecheck: clean;
    • run check:denylist: "1397 files clean";
    • push gate: "1 commit(s) and 1 tip(s) clean"; the commit is signed off, with no other trailer.

  • Not run locally: test/managed-apps.test.ts, because it sets its own PATH with /usr/bin. CI runs the full suite, which I did not rerun.

  • Earlier rounds' results are in the git history of this body. Round 4: 240 tests, 228 pass, 0 fail, 12 todo.

Adversarial pass (round 5)

All on scratch fixtures, against the round-5 build.

# Attack Result Label
A1 Reworded copy of the rule in a new docs/decisions/0001-x.md L3, high-risk: self-modification, under all three review.yaml files measured (adversarial case, locks test)
A2 Add a new decisions folder, or change what the index reads, in src/plan-decisions.ts L3 self-modification (decisions-index-code) measured
A3 Change the record lists themselves They live in src/review.ts ("the review levels", already L3) measured (existing lock)
A4 Symlink out of a nested planning worktree to the clone's copy not indexed measured
A5 Make docs/process.md unreadable (ELOOP, or an unsearchable docs/ with a hard link) no record indexed (decisions already made stay; round 12) measured
A6 Merge a link docs/adr -> ../notes (locked, so it needs both reviews), then edit notes/0001.md at L0 Worked on the first round-5 build: indexed, target unlocked. Fixed: a record whose real path isn't a record path isn't indexed measured, before and after
A7 Bare folder symlink docs/adr (a file in git, not under docs/adr/**) locked: each folder itself is a glob measured (locks test, docs/adr)
A8 Over-reach into managed apps none: all 14 paths keep their measured categories, and the every-repo glob list is unchanged measured
A9 Rename a record to an unlocked path the engine counts both paths, and process.md now says so inferred (round 5 review measured --no-renames)
A10 Case variant on a case-insensitive checkout (Docs/adr/x.md, sorted before docs/) Open, inferred: present() checks only the last path part's case, so on macOS or Windows such a file could be read as docs/adr/x.md while its git path isn't locked. Linux CI can't build this. Proposal: check every path part's exact case in recordEntries, failing closed. Not done here because it can't be tested in CI inferred
A11 Hard link committed in a PR not possible: git doesn't record hard links inferred
A12 Weakening anything No lock, test or guard is removed. Lines changed in place: engineSourceGlob (one condition added), the NOT_YET_FILES first line (it lost its ];), and the plan-decisions record lists (moved to src/review.ts, same values) measured (diff)

Package size

Measured with npm pack --dry-run --json --ignore-scripts, without the web UI built. This is the method the round-4 delta check used. My measurement of cfc69f0 matches theirs exactly (1,470,597 / 4,928,020), so their main figure is comparable.

Build Tarball (bytes) Unpacked (bytes) Entries
origin/main 41f6dd42 (delta check's measurement) 1,470,176 4,926,629 491
round 4 cfc69f06 (measured, mine) 1,470,597 4,928,020 491
round 5 41343a76 (measured, mine) 1,471,883 4,932,086 491
round 6 1d11ea38 (measured, mine) 1,472,145 4,932,851 491
origin/main d2a549f4 (measured, mine, a worktree at main) 1,481,905 4,963,126 493
round 7 6edea7b8 (measured, mine) 1,485,190 4,972,867 493
  • Against today's main (d2a549f), at round 7: +3,285 tarball, +9,741 unpacked, the same entry count, well within PACKAGE_BUDGET.
    • dist/src/plan-decisions.js 11,109 to 16,973 (+5,864);
    • dist/src/review.js 39,950 to 43,550 (+3,600);
    • the remaining +277: merge-policy-weakening.js (one GUARD_FILES entry) and coordinator/decisions.js (the base argument). Inferred, not split per file.

The earlier rows compare with the earlier main 41f6dd42:

  • Against main at round 6: +1,969 tarball, +6,222 unpacked, the same entry count. Round 6's own share is +262 / +765, all in dist/src/review.js (onePartPerGlob and its comment).
  • Against main at round 5: +1,707 tarball, +5,457 unpacked. That is well within PACKAGE_BUDGET (2,050,000 / 6,400,000).
  • Round 5's own delta, per file: dist/src/review.js 40,439 to 42,785 (+2,346), and dist/src/plan-decisions.js 12,011 to 13,731 (+1,720), together +4,066. docs/, test/, changelog.d/ and the config folder are not packed.

New dependencies

None.

What this can never do

  • Loosen a merge rule. The code changes add self-modification parts (locks only). onePartPerGlob drops only a glob's repeat listing, never its only listing. recordEntries indexes fewer records wherever main didn't throw. The one exception is a decisions folder with a dangling entry: main throws ENOENT there and the plan step crashes, while the head skips that entry and indexes the real records (A8d, measured by the delta check). Since round 7, a record's text is indexed only when it is a regular file committed at a record path in the plan's base commit (measured: the round-7 tests). Text can still reach planners in other ways. The known ones are listed in docs/process.md, and that list is not proven complete (planner runs: take input only from an allowlist enforced in code #975). At 1d11ea3 that was false for untracked files (A1). engineSourceGlob now also matches the new decision-record globs, exactly. So those new locks stand aside in a managed app, but no existing lock does (measured: the every-repo glob list in test/review.test.ts is unchanged).
  • Make the harness merge more. docs/process.md isn't in the decisions index. Every path the index reads, and the index code, is locked in the engine's repo. That covers what the base commit holds. It doesn't close every route to the planners: the known open routes are in docs/process.md, and that list is not proven complete (planner runs: take input only from an allowlist enforced in code #975). The page says the harness's policy is unchanged.
  • Touch real state. It is docs, globs, an index that skips more, and tests that write only under tmpdir() (inferred, from reading the tests).

Compromised component

  • Agent user / builder:
    • Could rewrite docs/process.md, or put a copy of it, whole, in part or reworded, in any decision record, or change the index code. Each is now L3 self-modification and never auto-merged (measured), and the page puts such a PR in the critical set, so it needs two independent reviews.
    • Could commit a record that links elsewhere. A link out of the checkout, or to a file that isn't a record path, isn't indexed (measured).
    • When no agent user is configured, a builder runs as the engine's user, so it can take the open routes listed in docs/process.md (inferred, from src/worktrees.ts and src/runner.ts, per the round-9 second review).
  • Planner or plan-review agent: what holds is about records in the index.
    • The page isn't indexed, and neither is a link.
    • An untracked or uncommitted file isn't indexed, and neither is a commit made after the base.
    • As a record, a copy can enter only as a regular file committed at a record path in the base. Every record path is locked, so a PR that adds one is critical (measured on fixtures).
    • None of this closes the known open routes listed in docs/process.md. That list is not proven complete. The by-design closure is planner runs: take input only from an allowlist enforced in code #975.
    • Round 6's wording, "a copy can only enter through a locked record path", was false at 1d11ea3 (A1: an untracked file). It was corrected in round 7.
    • Case variants on a case-insensitive checkout: Case-variant locked/record paths on case-insensitive checkouts (A10 from #920) #953 (inferred closed on the index side since round 7).
  • A setup step or install script (new in round 7, corrected in round 9): it runs code in every new worktree.
  • Coordinator, dashboard, other instances, hub viewer:
    • The coordinator reads the new globs through computeLevel, and the only effect is that more paths wait.
    • Its decisions index skips more records: those in a managed app that link outside it or to a non-record file, and every record when docs/process.md exists but can't be read. That fails closed: plans citing them are refused.
    • Nothing else reads the page (inferred: a grep of src/ finds no other reader).
    • Other instances, and their agents when no agent user is configured, run as the engine's user, so they can take the open routes listed in docs/process.md (inferred).
  • Someone with push access: could edit the page, but branch rules and required checks enforce merges, not this text (inferred).

@jayden1711
design: how the engine repo itself is merged (owner decision, 2026-10…
c04ccfa 
…-21)

The owner's final merge rule for this repo's own PRs was not written down in the
design. A new short section 20 states it: the orchestrator merges every PR after an
independent final check, the combined check and green required CI; the critical set
also needs a second adversarial reviewer; the orchestrator runs the batch-merge
procedure itself; reviewers and builders never merge; what still goes to the owner;
the weekly summary of critical merges; and what the [PROTECTED: owner merges] title
marker now means.

Test (fails without): none, docs only. No test pins docs/design.md's sections.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 1/2)

Final check of head c04ccfa8. One file, docs/design.md, +30 lines, new §20.

1. Does §20 say what the owner said, and nothing else?

Owner's words (2026-10-21) §20 Result
"you merge every PR yourself once it passes its independent final check, the combined check and required CI" Orchestrator merges every PR after a final check (a separate reviewer's PASS on its current head), the combined check (the batch merged on a do-not-merge draft PR, CI green on that tree) and green required CI Matches. The extra detail matches current practice (inferred)
"critical-set PRs also need a second, independent adversarial reviewer to pass" + the 7-item set Same wording; the 7 items are listed verbatim Matches (measured: compared line by line)
"You run the batch-merge scripts" "The orchestrator runs the batch-merge procedure itself", with 4 sub-bullets Matches, but one sub-bullet is inexact. See required change 3
"Only bring me: release signing, turning auto_merge on per project, direction changes, and steps that physically need my hands or my GitHub admin rights" "...steps that need the owner's hands or GitHub admin rights" "physically" is gone. See required change 2
"Weekly summary of critical merges" Same Matches
(not in the owner's words) "Reviewers and builders never merge." Added, but it follows from "you merge ... yourself" and from the reviewer and builder briefs that already say so. Acceptable (inferred)
(not in the owner's words) The [PROTECTED: owner merges] marker "means the PR is in the critical set" Not accurate. See required change 1

The batch-merge procedure compared with the batch-merge script (measured: I read the whole script):

  • Strict turned off, then back on when the script exits: matches. trap '... setstrict true' EXIT is set before setstrict false (lines 88-89). DRY runs and preflight failures exit before strict is touched (lines 82-85). One caveat (inferred): the EXIT trap doesn't run on SIGKILL or if the machine is lost, and a failed re-enable call isn't retried. So "on any exit" is slightly stronger than the script can promise. This is optional to change.
  • Each merge pinned to its tested head: matches. The preflight requires headRefOid == pinned (line 67), and every merge uses gh pr merge --match-head-commit "$h" (line 113).
  • No admin bypass, no force: matches. The script never uses --admin (comment on line 14; no use anywhere), never force-pushes, and its ruleset PUT keeps bypass_actors unchanged (line 32).
  • "After every step, main's tree is checked against the tested combined tree": inexact. After each merge, the script compares main's tree with that step's tested tree, taken from the combined branch's first-parent chain ([ "$got" = "$t" ], lines 118-119; QUEUE field 4). Only at the end does it compare with the whole combined tree $TREE (lines 124-128). Read literally, the doc describes a check that would fail on the first step.

2. The marker

Required change 1. §20 says the marker "means the PR is in the critical set." But the marker isn't applied by critical-set membership. It goes on every PR that touches a protected path, and protected paths include .worklane/review.yaml's whole engine category (src/**, scripts/**, templates/**, deploy/**, .github/**, package.json, package-lock.json, .worklane/**, review.yaml line 8) plus engine-guards (measured).

Measured with gh pr list --search "PROTECTED in:title": the marker is on #916 (a vite dependency bump), #918 (merge receipts), #759 and #863 (bench), #645 (a coordinator incident link), #630 (web buttons) and #718 (stale docs). None of these is in the owner's 7-item set. As written, §20 would require a second adversarial review for each of them. That makes the critical set much wider than the owner's list, which is a tightening beyond the owner's words. The PR body takes the same reading: "a locked path puts it in the critical set".

3. Names, brand, paths

  • npm run check:denylist (through npmw.sh): denylist: 1379 files clean (measured).
  • node --import ./dist/test/isolate.js --test dist/test/brand.test.js after npm run build: 2 pass, 0 fail (measured).
  • I grepped §20 (lines 712-740) for the product name, the owner's name, the company, /home, /tmp, .sh, the scratchpad, wl-dev and issue numbers: no hits (measured). Roles are generic.

4. Contradictions elsewhere in docs/design.md

I grepped for "owner merges", "PROTECTED", "self-modif", "L3", "owner approv", "this repo", "engine repo", "critical", "adversarial", "strict" and "admin" (measured). Every hit describes how the harness merges a project's PRs: §8 review levels, the merge policy, batching's "Owner approval" bullet at line 417, and line 351's "not strict by default" recommendation for projects. §20's first paragraph scopes itself away from all of them. None contradicts §20 (inferred).

Outside design.md, so not part of this check: .worklane/review.yaml still says the engine category is "always the owner's call" and lists it in merge.wait_categories. That is the harness's own config for this repo. It holds harness-driven merges and is stricter than §20, not a contradiction of the orchestrator's process (inferred). It's worth tidying later, as its own critical-set PR.

5. CI at c04ccfa8 (not rerun)

denylist, dco, push-gate, scan (x2), new-deps, scope and agentshield pass. All 8 test shards (Node 22 and 24, 1-4/4) and desktop were pending when I checked. The real-systemd jobs and full-ci were skipping (measured: gh pr checks 920). Required CI is not yet green.

Other checks: the commit is signed off, with no attribution trailers (measured: git log). git merge-tree --write-tree origin/main c04ccfa8 merges cleanly (measured). Rule A: nothing in this PR can touch real state; it is one Markdown file (inferred).

Required changes

  1. Marker sentence (line 740). Don't tie the marker to the critical set. Suggested wording: "The [PROTECTED: owner merges] title marker no longer means the owner merges the PR. It marks a PR that touches a protected path. Such a PR merges under this section like any other, and it needs the second adversarial review only if it is in the critical set above." Fix the PR body's "a locked path puts it in the critical set" to match. If the owner wants every protected PR to get the second review, that is a new decision for the owner. The current text shouldn't assume it.
  2. Restore "physically" (line 738). Suggested: "steps that physically need the owner's hands, or the owner's GitHub admin rights."
  3. Tree check wording (line 731). Suggested: "after every merge, main's tree must equal the tested tree for that step (the combined branch after the same merges), and at the end the whole tested combined tree; any mismatch stops the batch."

Optional: change "on any exit" to "on any exit the script controls (an exit trap)".

Note for the owner (inferred, no decision needed now): the script turns strict off by PUT on the ruleset, which needs repo-admin credentials. Under the approved move to an unprivileged agent user with an owner-only ruleset, the orchestrator could no longer run that step. The procedure would then become a "needs the owner's GitHub admin rights" item.

@jayden1711
Merge remote-tracking branch 'origin/main' into docs-merge-rule
d349be6 
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review: CHANGES NEEDED (head c04ccfa)

The question I tested: could a future agent that reads design.md as policy use §20 to justify a merge it shouldn't make? Yes. There are three routes, and two claims in the PR body are false. I compared every line with the owner's own words for the rule. Each claim below is labelled.

Attacks

# Attack Result Label
1 Merge a critical PR on a stale adversarial PASS. Works. The final check is tied to "its current head", but the second review only has to "pass it", with no head (design.md:720). So: the adversarial review passes head A, the builder pushes head B, the final check passes B, and the orchestrator merges B on the PASS that A got. inferred
2 Count a conditional verdict as a pass. Ambiguous. "PASS" is never defined, so "PASS, once X is fixed" reads as a pass. inferred
3 The orchestrator reviews its own merge. Ambiguous. "A separate reviewer" doesn't say separate from whom. Nothing excludes the orchestrator or the builder, or says the second reviewer must differ from the first. inferred
4 Get a locked path past the second review. Works. The seven-item critical set is much narrower than the paths locked in code (SELF_MODIFICATION_PARTS, src/review.ts:325-395) and in review.yaml's release-signing and engine-guards. Not covered: sign-in, credentials and secrets; who may approve; the evaluator and runner; the sandbox; the push gate, denylist and secret scan; the main watch; the push guard; the tests of the controls; the coordinator; what an agent run is given; managed-apps; and docs/design.md itself. So a PR that rewrites §20 to drop the second review needs only one reviewer. The PR body says "a locked path puts it in the critical set", but the text doesn't say that. measured (read from src/review.ts and review.yaml)
5 Use the title marker as the membership test. Ambiguous. §20 defines the marker as "is in the critical set", and nothing else says who decides membership. A builder could leave the marker off, and the PR would read as not critical. inferred
6 Treat changing who may sign a release as an ordinary critical merge. Works. "Release signing and the updater" is in the critical set, so the orchestrator may merge a change to .github/release-signers or scripts/verify-release-tag* after two reviews. review.yaml:9-10 calls this "the owner's alone", and the owner said release signing comes to them. measured (read)
7 Merge with required CI pending. Trap. On this PR, gh pr checks 920 --required printed 7 passes. It left out test (ubuntu-latest, 22) and test (ubuntu-latest, 24), which are in main's required list but hadn't reported yet. GitHub blocks a real merge while a required check is missing (inferred). But the combined check on a draft PR is judged by reading CI, and "green required CI" doesn't say that missing or pending checks are not green. measured
8 Keep going after a batch step fails. Ambiguous. "Main's tree is checked against the tested combined tree" never says what happens on a mismatch. inferred
9 Read the batch procedure as permission for admin actions. Ambiguous. Turning strict up-to-date off takes admin rights (inferred), yet "steps that need … GitHub admin rights" go to the owner. Nothing limits the orchestrator to that one toggle. inferred
10 §20 used as precedent for the engine itself. Reachable. The engine indexes docs/design.md by heading and hands each section's text to its planner and plan-reviewer agents as a recorded decision (DECISION_FILES, src/plan-decisions.ts:17; src/coordinator/decisions.ts:41). I ran decisionIndex on this head: it returns the entry docs/design.md § 20. How the engine repo itself is merged … with the full text. Only the first sentence scopes §20, and it never says §9, §10 and guardrails.md are unchanged. A planner could cite §20 for "an agent merges L3 after a second adversarial review", against "A model never decides a merge" (guardrails.md:17). measured
11 Narrow what goes to the owner. Ambiguous. "The owner is brought only:" sits in a locked doc with nothing saying "for merges". So it can be read as removing the owner's OK for removing or weakening a check (rule 10). inferred
12 The dropped word "physically". No finding. Dropping it can only send more to the owner. inferred
13 Scope of the diff. No finding. One file, +30 -0, nothing else touched. measured

False claims in the PR body (principle 5)

  • "What this can never do": "not read by the engine at run time". False. See attack 10. (measured)
  • "Compromised component": "Coordinator … never read this file; no effect". False. The coordinator's decisions index reads it. (measured)
  • "Under the rule recorded here it needs two independent reviewers". Not true of the text. docs/design.md isn't in the seven-item list. See attack 4. (measured, by reading)

Required changes (each one fails closed, and none loosens anything)

  1. Second review.
    • Pin it to the current head.
    • A new push needs both reviews again.
    • Define "independent": not the builder, not the orchestrator, and for the second review, not the first reviewer.
    • A PASS with conditions is not a pass until a reviewer checks the head that meets them.
    • Each PASS is posted on the PR and names the head.
  2. Critical set.
    • Keep the seven areas, and add every path locked in code as self-modification (src/review.ts) or listed in review.yaml's release-signing or engine-guards, this section included.
    • The PR's changed files decide membership, not its title marker.
    • When unsure, the PR is in.
  3. Release signing. Release signing, and any PR that changes who may sign a release (.github/release-signers, scripts/verify-release-tag*), goes to the owner. The orchestrator doesn't merge those.
  4. CI. "Green" means every check on main's ruleset's required list reported success on that exact head or tree. Pending, missing, failed or cancelled is not green. The same holds for the combined check's tree.
  5. Batch.
    • A mismatch, a failed step or a check that isn't green stops the batch, and the rest wait for a new combined check.
    • The strict toggle is the only ruleset or settings change the orchestrator makes. Any other change is the owner's.
  6. Scope.
    • Say "For merges, the owner is brought only: …".
    • Add: "This removes no other owner approval. A change that removes or weakens a check, lint, test or guard still needs the owner's OK first."
    • Add: "This section changes nothing in the harness. Its merge policy, review levels and holds (§9, §10, docs/guardrails.md) still apply to every project, and a plan can't cite this section to let the engine merge more."
  7. PR body. Correct the three claims above.

Suggested wording for items 1 and 2:

  • The orchestrator merges every PR once it has, on its current head: an independent final check (a PASS posted on the PR, naming that head, from a reviewer who is neither its builder nor the orchestrator; a PASS with conditions is not a pass until a reviewer checks the head that meets them); the combined check; and green required CI (every check on main's required list reported success on that head; pending or missing is not green).
  • The critical set needs more: a second adversarial reviewer, independent of the builder, the orchestrator and the first reviewer, passes the same head. A new push needs both reviews again. A PR is in the critical set when its changed files touch any of: [the seven areas]; any path locked as self-modification in src/review.ts; or review.yaml's release-signing or engine-guards, this section included. The files decide, not the title marker. When unsure, it's in.

For the owner (a genuinely new trade-off): do you mean "Only bring me" to replace rule 10, the owner's OK for removing or weakening a check? Until you say so, item 6 keeps rule 10, which is the status quo.

Refusals: none.

@jayden1711
design: the PROTECTED marker means a protected path, not the critical…
b3f1be3 
… set

Review round 1 of #920. The title marker goes on every PR that touches a
protected path (a dependency bump included), so it can't mean the critical
set: it now says the marker no longer means the owner merges, and the second
adversarial review applies only to the critical set, which the orchestrator
decides from the changed files. Also restores "physically" in the owner's
words, and states the tree check as the batch-merge procedure does it: after
each merge against that step's tested tree, at the end against the whole
combined tree. Strict comes back on through an exit trap, on a normal or an
error exit, not when the process is killed outright.

Test (fails without): none, docs only. No test pins docs/design.md's sections.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Round 2, head b3f1be35. origin/main (41f6dd4) is merged in; the PR's diff against main is still docs/design.md only, +30 lines (measured: git diff --stat origin/main HEAD).

  1. Marker: the bullet now says the marker no longer means the owner merges. It marks a PR that touches a protected path, which covers far more than the critical set (a dependency bump carries it too). Such a PR is merged by the orchestrator under §20 like any other, and it needs the second adversarial review only if it is in the critical set, which the orchestrator decides from the changed files. The PR body's "a locked path puts it in the critical set" is gone and now says the same.
  2. "physically" restored: "steps that physically need the owner's hands or GitHub admin rights."
  3. Tree check: "after each merge, main's tree is compared with that step's tested tree (the combined branch after the same merges), and at the end with the whole tested combined tree."
    Optional point taken: strict comes back on "by an exit trap, on a normal or an error exit (not when the process is killed outright, as by SIGKILL)".

Verified at b3f1be3, after a rebuild (all measured):

  • brand + docs-guardrails tests: 11 pass, 0 fail;
  • denylist: 1395 files clean;
  • DCO: all commits signed off;
  • push gate: clean.

@jayden1711
review: the repo's merge process moves to docs/process.md, locked, ou…
8cb562b 
…tside the decisions index

Owner decisions on #920's adversarial review (2026-10-21). docs/design.md is
indexed by heading as recorded decisions for the planner and plan-review agents
(src/plan-decisions.ts DECISION_FILES), so a section there on how this repo's
own PRs are merged could be cited to let the engine merge more. The rule now
lives in docs/process.md, which the index never reads; design.md keeps only a
pointer in its untitled intro, which the index drops.

docs/process.md is locked like the design rules: its own self-modification
part in src/review.ts ("the engine repo's merge process") and an engine-guards
entry in review.yaml. This only adds a lock.

The page states the rule fail-closed, as the review asked: both reviews on the
current head, posted naming it, and redone on any push; who counts as
independent; a conditional PASS is not a pass; green means every required
check reported success on that exact head or tree; the critical set widened to
the owner's seven areas plus every self-modification lock and the
release-signing and engine-guards categories, decided by changed files, never
the title marker, and when unsure it's in; changes to who may sign a release
go to the owner; a batch stops on any mismatch or failure; the strict toggle
is the orchestrator's only ruleset change; the owner list is for merges, and
the owner's OK for removing or weakening a check stays; and the harness's
merge policy, review levels and holds are unchanged.

Test (fails without): test/plan-decisions.test.ts (process.md exists and no
index entry carries the merge process), test/engine-guard-locks.test.ts and
test/merge-policy.test.ts (docs/process.md is self-modification, L3, never
auto-merged, under engine-guards), and the adversarial case
merge-process-docs-only.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711 jayden1711 changed the title design: how the engine repo itself is merged (owner decision, 2026-10-21) [PROTECTED: owner merges] review: the repo's merge process in docs/process.md, locked, outside the decisions index [PROTECTED: owner merges] Oct 21, 2026
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Round 3, head 8cb562bb, following the owner's decisions on the second review. It supersedes round 2. The full list of changes is in the PR body.

  • The rule moved to docs/process.md. design.md drops §20 and keeps one pointer sentence in its untitled intro, which the decisions index drops.
    • A new test, test/plan-decisions.test.ts, shows no index entry carries the process, and that a fixture's docs/process.md is never indexed (measured).
  • docs/process.md is locked: a new self-modification part in src/review.ts ("the engine repo's merge process") and an engine-guards entry in review.yaml.
    • Tests: engine-guard-locks, merge-policy and a new adversarial case, merge-process-docs-only (measured).
  • All seven of the second reviewer's fail-closed wording fixes are in, as are the widened critical set, the rule-10 sentence and the harness-unchanged sentence. Round 2's three fixes are kept.
  • The PR body's three false claims are corrected under "Corrections to earlier claims".

Verified (measured):

  • Fail-without on the unfixed tree: 6 fail.
  • After the fix:
    • 235 tests: 223 pass, 0 fail, 12 todo;
    • plus 15 pass, 0 fail;
    • typecheck clean;
    • denylist clean;
    • DCO and the push gate clean.

@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Merged
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: PASS (head 8cb562bb)

Final check of round 3, head 8cb562bbb081ccaf10c211bf470eda1db6c0648a. Base and origin/main are both 41f6dd42 (measured).

1. docs/process.md compared with the owner's decisions and the earlier required changes

Item In docs/process.md Result
Owner: merge every PR after the independent final check, the combined check and required CI "Who merges", all three, each on the current head Met (measured, by reading)
Owner: critical set needs a second, independent adversarial reviewer "The critical set", first bullet Met (measured)
Owner: you run the batch-merge scripts "The batch merge" Met (measured)
Owner: only bring me release signing, auto_merge per project, direction changes, steps that physically need my hands or GitHub admin rights "For merges, the owner is brought only", with "physically" kept Met (measured)
Owner: weekly summary of critical merges Last bullet Met (measured)
Owner: widen the critical set to every self-modification lock plus review.yaml release-signing and engine-guards The 7 areas (word for word as round 1) plus both additions, "this page included" Met (measured)
Owner: rule 10 stays "This removes no other owner approval…" Met (measured)
Owner: a separate process doc, not design.md design.md §20 removed; a single pointer sentence in the untitled intro Met (measured)
Adversarial 1: second review pinned to the head; a push needs both reviews again; who counts as independent; a conditional PASS is not a pass; each PASS is posted naming the head All five present Met (measured)
Adversarial 2: critical set widened; changed files decide, never the marker; when unsure it's in Present Met (measured)
Adversarial 3: changes to who may sign a release go to the owner (.github/release-signers, scripts/verify-release-tag*) Present, and in the owner list Met (measured)
Adversarial 4: green = every required check reported success on that exact head or tree; the same for the combined tree Present. It also counts "skipped" as not green, which is stricter than the review asked. It matches the script (reqmissing needs conclusion == success) and blocks nothing today: no required check is skipped at this head Met, fail-closed (measured)
Adversarial 5: the batch stops on a mismatch, failed step or non-green check, and the rest wait for a new combined check; the strict toggle is the only ruleset change Present Met (measured)
Adversarial 6: "for merges"; rule-10 sentence; the harness is unchanged and can't be cited Present (intro and owner section) Met (measured)
Adversarial 7: PR body's three false claims corrected "Corrections to earlier claims" Met (measured)
Round 2, fix 1: the marker isn't tied to the critical set Present. It needs the second review only if its changed files put it in the critical set Met (measured)
Round 2, fix 2: "physically" Restored Met (measured)
Round 2, fix 3: the per-step tree vs the final combined tree Present Met (measured)
Round 2 optional: exit trap, not SIGKILL Present Met (measured)

Loosened or added beyond these? No (measured, by reading the whole page). The only additions are stricter: "skipped" in the green rule, and "Reviewers and builders never merge", which round 1 accepted.

The batch procedure compared with the batch-merge script (measured: read the script in full):

  • The trap is set before strict is turned off (lines 88-89), and die exits through it.
  • Every merge uses --match-head-commit (line 113).
  • The per-step tree is checked at lines 118-119 and the final tree at lines 124-128.
  • Every failure path calls die.
  • There is no --admin and no force push. The ruleset PUT keeps bypass_actors.

All of this matches the page.

One note, not a change to this PR (inferred): the script's preflight can resume a stopped batch from the merged prefix using the same combined check. The page says the remaining PRs wait for a new combined check, which is stricter. Whoever runs the script should rebuild the combined check after a stop, not rerun it.

2. Decisions index (src/plan-decisions.ts)

  • DECISION_FILES and DECISION_DIRS don't name docs/process.md or any docs/ folder it could fall under. The pointer is at design.md line 15, under the level-1 title. sectionsOf drops that section when deeper headings exist (measured, by reading).
  • At the head, the new test passes (ok 165), along with the rest of plan-decisions, engine-guard-locks, merge-policy and adversarial: 165 tests, 153 pass, 0 fail, 12 todo (measured).
  • Fail-without (measured): I wrote the new test verbatim as a new file in a fresh base worktree at 41f6dd42 (test/fw920-plan-decisions.test.ts). It fails with 'docs/process.md is a real file' (expected true, actual false). On main the existence check fails first, so I also ran the test's index assertions against round 1's design.md (c04ccfa8, with §20) in a temp checkout. They fail on docs/design.md § 20. How the engine repo itself is merged (owner decision, 2026-10-21) and pass on the head's design.md (measured). So the assertions do catch the leak the second review found.

3. The lock

  • Only additions: src/review.ts +3 -0 (one new SELF_MODIFICATION_PARTS entry, "the engine repo's merge process", docs/process.md), and .worklane/review.yaml +1 -0 (an engine-guards entry). No existing lock, glob or assertion is changed or removed (measured: git diff origin/main...HEAD). The only other edited line in a test is the present list, which gains docs/process.md.
  • engine-guard-locks and merge-policy: pass at the head (above). review, not-yet-files-locks, docs-guardrails and brand also pass: 35 tests, 0 fail (measured).
  • Managed app: docs/process.md stays L0 ['docs'], with selfModificationPart(…, 'managed-app') === null. The PR's managed-app test checks this, and so does my fail-without file, which passes on main as well: no regression (measured).
  • Fail-without of the lock assertions (measured, new file test/fw920-locks.test.ts at base):
    • the self-modification test fails: expected "the engine repo's merge process", actual null;
    • the engine-guards test fails: expected 'L3', actual 'L0'.

4. Adversarial case merge-process-docs-only

  • On main (measured): the case, written as a new case file test/adversarial/cases/fw920.json in the base worktree, gives not ok 25 … merge-process-docs-only … waits with expected: false, actual: true. That means it merged with the evaluator fooled.
  • At the head: ok 59 (measured).

5. Other checks

  • Changelog: changelog.d/920-lock-merge-process-doc.md sits under "Guardrail changes: what each one does", like the 16 other fragments. Its claims match what the tests measure (measured).
  • Typecheck: npm run typecheck is clean (measured, through npmw.sh).
  • Denylist: "denylist: 1397 files clean" (measured).
  • Brand: the brand test passes. Nothing in src/ hardcodes the product name; the diff adds only the lock entry (measured).
  • Size: measured with npm pack --dry-run --json on base and head builds.
    • tarball: 1,470,176 → 1,470,274 (+98 B);
    • unpacked: 4,926,629 → 4,926,952 (+323 B);
    • 491 entries in both.
    • Both are far under the budget (2,050,000 / 6,400,000). The body's "+200 B" was inferred; the measured unpacked delta is +323 B. That is a small correction, and no change is required.
  • git merge-tree --write-tree (measured), all exit 0 with no conflicts:
  • CI at 8cb562bb (measured, not rerun; check-runs on the exact SHA): all 8 required checks are completed/success. So are desktop, scope, installed-engine-config and the 8 test shards. The real-systemd, bootstrap and sign-in-probe jobs are skipped; none of them is on the required list.
  • Commits: all 4 are signed off, with no attribution trailers (measured: git log).
  • Rule A: the tests use only mkdtemp temp dirs and in-memory config; nothing can touch real state (inferred, by reading).
  • Protection: marked protected (title marker, **Risk class:** protected, needs-owner), with real "What this can never do" and "Compromised component" sections. I checked the claims there against the code and tests: they hold (measured for the lock and the index, inferred for "no other reader": a grep of src/ finds only the lock glob).
  • Adversarial pass section: present in the body.
  • Closes: there is no "Closes #N"; this records an owner decision with no issue (inferred). Not required.

For the merge (not a builder change)

By the rule this PR records, and already by the existing critical-set areas (it changes src/review.ts locks), this PR is in the critical set. The only adversarial review so far was on c04ccfa8. A second adversarial PASS naming 8cb562bb is needed before it merges (inferred from docs/process.md and the review history).

Refusals: none.

@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review: CHANGES NEEDED (head 8cb562b)

Head checked: 8cb562bbb081ccaf10c211bf470eda1db6c0648a. It is up to date with origin/main 41f6dd42 (measured: rev-list --count HEAD..origin/main = 0). I'm a different reviewer from issuecomment-6307252128, and I wrote my attack list before reading it. I attacked the guardrail; I didn't re-verify the diff.

What holds (measured)

  • No lock is loosened. I ran selfModificationPart and lockedCategories from the base build (41f6dd4) and the head build over all 1411 tracked files plus 11 extra paths, in absent, engine and managed-app scope. The only difference is the addition of docs/process.md, and nothing goes from locked to unlocked.
  • The tests pass. engine-guard-locks, merge-policy, plan-decisions, adversarial and review: 187 tests, 175 pass, 0 fail, 12 todo. I ran them with a scratch HOME, scratch state dirs and stand-ins for the host tools.
  • The page waits. Under this repo's review.yaml, computeLevel on docs/process.md gives L3 [docs, engine-guards, self-modification]. In a managed-app scope it isn't locked, by design, and that scope comes only from root.
  • The current index is clean. decisionIndex on this head has no entry that mentions process.md, the critical set or the orchestrator.
  • No edit route to the index through locked files. A level-2 heading added above design.md's intro pointer, or a new design.md section that points at the page, does leak into the index (scratch fixtures A4 and A5). Both need a design.md edit, which is L3 and in the critical set.
  • Case variants don't reach the index. docs/Process.md and docs/Design.md aren't indexed, because the indexed file names match exactly.

Attacks

# Attack Result Label
1 Get the process into the planner's index with a symlink. I added docs/decisions/0001-merge.md -> ../process.md to a scratch fixture. Works. It is indexed with the page's full text. As docs/decisions.md -> process.md, it gives three citable entries (docs/decisions.md § Who merges, § The critical set, § The batch merge). recordEntries follows symlinks (src/plan-decisions.ts:48 and :94-100). measured (scratch fixture)
2 Is that PR held? No. Under this repo's review.yaml, computeLevel on docs/decisions/0001-merge.md gives L0 [docs]. It isn't in the critical set either, so it needs one review. A plain copy of the text works the same way. measured
3 Remove the backstop in the same PR. Works. The only thing that catches attacks 1 and 2 is the new test in test/plan-decisions.test.ts. It scans the repo's index for "critical set" and "orchestrator", and that same pattern flags my fixtures. But the file isn't locked: L0 [tests]. Not in SELF_MODIFICATION_PARTS, not in engine-guards. So one L0, non-critical PR can index the process and delete the test that would catch it. Only rule 10, a written rule, stands in the way. measured (levels), inferred (CI result)
4 Point the index code at the page. Add docs/process.md to DECISION_FILES. Held only partly. src/plan-decisions.ts is L3 [engine] but isn't self-modification, so it needs one review. CI fails while the test stays, which brings us back to attack 3. measured (levels)
5 Merge on a stale base. A batch is killed (SIGKILL) or its exit trap fails, so strict up-to-date stays off. The orchestrator then merges a PR on its own, using a combined check built on an older main. Not forbidden by the text. The tree comparison is only in "The batch merge" (process.md:41). Nothing says every merge goes through that procedure, that strict must be confirmed back on before the next merge, or that the combined check must be built on current main (process.md:11, :39). The page names the SIGKILL gap but says nothing about what to do when it happens. inferred
6 Read the release-signers path list as complete. A change to the signer check in .github/workflows/release.yml gets two reviews and is merged by the orchestrator. Ambiguous. process.md:33 states the rule in general terms, then lists two paths as "a PR that touches …". A fail-closed reader brings it to the owner; a narrow reader doesn't. inferred
7 Narrow rule 10. Mostly holds. process.md:53 keeps the owner's OK. It drops "explicit", and it doesn't say that adding an exception, allowlist entry or skip is a weakening (principle 9). inferred
8 Stale CI through "or tree" (process.md:13). Low risk. CI from a different head with the same tree could be cited. Reviews still pin the head, and the batch compares trees. Wording only. inferred
9 An empty or unreadable required list. Low risk. "Every check on main's required list" is vacuously true if the list is empty or can't be read. Today the list has 8 checks (measured, via the rules API). inferred
10 Use the title marker as membership. Holds. process.md:31-32 says the changed files decide, and when unsure the PR is in. inferred
11 Skip the second review. Holds. It's pinned to the current head, re-run on any push, and must be independent of the builder, the orchestrator and the first reviewer (process.md:19-20). inferred
12 Merge process.md with no review, through the harness. Holds. It's L3, self-modification and engine-guards, so it waits (measured: computeLevel above, and merge-policy's "waits" test). Its own text also puts it in the critical set. measured
13 A case variant docs/PROCESS.md that shadows the page on a case-insensitive checkout. Gives L0 [docs]. design.md has the same issue on main, so this is not a regression. CI runs Linux only, so I don't count it as a finding. measured (level), inferred (effect)
14 Pending CI on this head. test (ubuntu-latest, 22), a required check, was pending when I checked. Under process.md:13 the PR isn't green yet. measured

Claims in the PR body

  • "Planner or plan-review agent: can't cite the process as a decision. It isn't indexed (test)." True of the file today, but overstated. A builder can get the page's text indexed with an L0, non-critical PR, and the test that holds the claim is unlocked (attacks 1 to 3).
  • "Weakening anything: no." Confirmed (measured, the lock comparison above).

Required changes

  1. Lock the backstop. Put test/plan-decisions.test.ts under "the tests of the harness controls" in SELF_MODIFICATION_PARTS and under engine-guards in .worklane/review.yaml, and add it to locked and present in test/engine-guard-locks.test.ts. Removing the test then becomes L3 and puts the PR in the critical set.
  2. Close the symlink route in code. recordEntries should never index a record whose real path is the checkout's docs/process.md, whether it's a symlinked decisions file or a file in a decisions folder. Add a test that fails without the fix: the fixture from attack 1. If you scope it to that one real path, nothing changes for a managed app that has no such link.
  3. process.md, the stale route (attack 5). Add something like this:
    • "Every merge goes through this procedure. A single PR is a batch of one."
    • "Before any merge, and after every batch, the orchestrator reads the ruleset and confirms strict up-to-date is on outside a batch. If the exit trap didn't restore it, nothing merges until it is back on."
    • "A combined check counts only if it was built on main as it is now."

Recommended (not blocking)

  • process.md:33: say "including any change to who may sign a release, for example a PR that touches …", so the path list isn't read as complete. A change to the signer check in the release workflow goes to the owner as well.
  • process.md:53: say "the owner's explicit OK", and "adding an exception, allowlist entry or skip is a weakening".
  • process.md:13:
    • Say "on that exact head commit (a PR) or that exact combined commit (the combined check)".
    • Add: "if the required list can't be read or is empty, nothing is green".
  • Residual, no change asked here: every agent posts as the same GitHub account, so GitHub can't show that a PASS came from someone other than the builder or the orchestrator. Independence rests on the orchestrator's records until the agents get their own identities (the planned migration).

Refusals: none.

@jayden1711
plan-decisions: never index docs/process.md under another name, and l…
cfc69f0 
…ock its test

Second adversarial review of #920. A decisions file or folder entry that is a
symlink to docs/process.md (docs/decisions.md -> process.md, or
docs/decisions/0001-merge.md -> ../process.md) was indexed with the page's full
text, as an L0 docs change, and the only backstop, test/plan-decisions.test.ts,
was itself L0. Now recordEntries skips any record that is the checkout's
docs/process.md by identity (device and inode, through symlinks), which also
covers a hard link; a checkout without that file indexes as before. The test is
locked under "the tests of the harness controls" and in review.yaml
engine-guards, so removing it is L3.

docs/process.md closes the stale-base route: every merge goes through the batch
procedure (a single PR is a batch of one); strict up-to-date is confirmed on
before any merge and after every batch, and nothing merges until it is; a
combined check counts only if built on current main, and a stopped batch's
remaining PRs need a new one. Also: the signer paths are examples of every
place a release's signature is checked; the owner's explicit OK, with an
exception or skip counted as a weakening; green is on the exact head or
combined commit, and an unreadable or empty required list means nothing is
green.

Test (fails without): test/plan-decisions.test.ts "regression: a decisions file
or folder entry that is docs/process.md under another name ...", and the two
lock tests in test/engine-guard-locks.test.ts for test/plan-decisions.test.ts.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Round 4, head cfc69f06, following the second adversarial review (issuecomment-6307564006). The full list of changes is in the PR body under "Round 4".

  1. Backstop locked: test/plan-decisions.test.ts is now in SELF_MODIFICATION_PARTS ("the tests of the harness controls") and in review.yaml engine-guards. It is also in locked, present and before in engine-guard-locks.
  2. Symlink route closed by design: recordEntries skips any record that is the checkout's docs/process.md by device and inode, through symlinks, which also covers hard links.
    • There is a new fail-without test with your fixture (docs/decisions/0001-merge.md -> ../process.md, docs/decisions.md -> process.md), plus a hard link.
    • It also checks that a record linking to any other file is still indexed.
  3. Stale-base route closed in process.md:
    • every merge goes through the procedure, and a single PR is a batch of one;
    • strict up-to-date is confirmed on before any merge and after every batch, and nothing merges until it is;
    • a combined check counts only if it was built on current main;
    • a stopped batch's remaining PRs need a new combined check.
  4. Recommended changes, taken:
    • the signer paths are examples of every place a release's signature is checked;
    • the owner's explicit OK, and an exception or skip counts as a weakening;
    • green is on the exact head or combined commit, and an empty or unreadable required list means nothing is green.
  5. The Agents act as the owner's GitHub account and main needs 0 approvals: carry out the approved wl-dev migration (own App, owner-only main ruleset) #917 residual is noted in the body.

Verified (measured):

  • Fail-without: 3 fail.
  • After the fix:
    • 240 tests, 228 pass, 0 fail, 12 todo (engine-guard-locks, merge-policy, plan-decisions, adversarial, review, brand, docs-guardrails, changelog*, not-yet-files-locks);
    • typecheck clean;
    • denylist clean;
    • DCO and the push gate clean.

@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Merged
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review (round 5): CHANGES NEEDED (head cfc69f0)

Head checked: cfc69f06b7a608a18d81bb7a0bcead792664c091, up to date with origin/main 41f6dd42 (measured: the merge base is main's tip). I'm a third reviewer, not the author of issuecomment-6307252128 or issuecomment-6307564006. I wrote my attack list before reading either of them. All payloads ran against scratch fixtures, with a scratch HOME and state dirs.

What holds (measured)

  • No lock is loosened. I compared base 41f6dd42 and the head over all 1402 tracked files of both trees. For each file I compared computeLevel and the wait categories under this repo's review.yaml and the example's, in engine and managed-app scope, plus selfModificationPart. There are 8 differences, all tightenings: docs/process.md and test/plan-decisions.test.ts go from L0 to L3, under self-modification and engine-guards. Nothing goes from locked to unlocked.
  • The PR's own tests pass: plan-decisions, engine-guard-locks, merge-policy and adversarial gave 166 tests, 154 pass, 0 fail, 12 todo.
  • These routes are closed:
    • an in-repo symlink to docs/process.md;
    • a folder symlink (docs/adr -> .): the folder's process.md entry is skipped;
    • a full copy, or a case-variant copy (docs/Process.md) reached through a symlink: the locked test's pattern catches both;
    • a rename of docs/process.md: the engine counts both paths (--no-renames), and the locked present list fails CI.
  • CI: at the time of this review, test shard 1/4 (22 and 24) and test shard 4/4 (22) were still pending, so the PR isn't green yet under process.md:13.

Attacks

# Attack (claim aimed at) Result Label
1 Partial copy. process.md:3 says the index "never includes it", and plan-decisions.ts:87 says a copy is held by the test. I wrote docs/decisions/0001-merges.md with process.md:49-53 (the "For merges, the owner is brought only" list) plus the first sentence of :32 (the [PROTECTED: owner merges] marker "no longer means the owner merges"). Works. It is indexed and citable as docs/decisions/0001-merges. The locked test's pattern (process.md, "engine repo itself is merged", "critical set", "orchestrator") doesn't match it. measured (scratch fixture)
2 Is the attack-1 PR held? No. decisions.md, DECISIONS.md, docs/decisions.md, docs/DECISIONS.md, docs/decisions/*, docs/adr/* and decisions/* all give L0 [docs], no wait and no self-modification. That makes it a single, non-critical, one-review PR. The repo has no file at any of these paths today. measured
3 Symlink out of the checkout ("the symlink route is closed by design"). With the default worktree.root: .claude/worktrees, a planning worktree sits at <clone>/.claude/worktrees/<name>. A committed docs/decisions.md -> ../../../../docs/process.md resolves to the clone's own docs/process.md. That is a different inode from the worktree's copy, so identity doesn't match. Works at run time. Indexed: docs/decisions.md § Who merges, § The critical set, § The batch merge and § What goes to the owner. In a CI-shaped checkout the target is absent, so present() is false and the locked test sees nothing. CI stays green. measured (scratch fixtures in both layouts); inferred that the real clone has a working tree with the page in it
4 Change the index code. Add a DECISION_DIRS entry, or edit PROCESS_DOC or identity, in src/plan-decisions.ts. Held only by CI, plus one review. selfModificationPart('src/plan-decisions.ts') is null, so the file is L3 [engine] but isn't in the critical set's literal list. A new folder plus a partial copy (attack 1) passes the locked test. measured (lock), inferred (route)
5 Include or transclusion. Not a route. sectionsOf and the folder reader take raw text and resolve nothing (plan-decisions.ts:60-71, :120). inferred (code read)
6 Stat failure. Make identity(docs/process.md) null. Opens nothing new. The page has to be missing (a locked path) or sparse-excluded (.worklane, L3). Either way, attacks 1 and 3 don't depend on it. inferred
7 Case variant. Add docs/PROCESS.md or docs/Process.md. L0 [docs]. It isn't indexed by name. Through a symlink it's caught as a full copy (fixture A3). On a case-insensitive checkout it shadows the page, the same as design.md on main. Not a regression. measured (level, A3), inferred (shadowing)
8 Skip the second review by a rename (process.md:21, "its changed files touch"). Rename a locked file, such as one under a self-modification glob, to an unlocked path. Wording gap. The engine counts both paths, but GitHub's PR file list (gh pr view --json files) gives only the new path. An orchestrator that reads "changed files" from it sees no locked path, so the PR looks non-critical. The page doesn't say that both sides of a rename count. inferred
9 Stale base or strict off (:11, :39-43). Mostly holds. "Built on main as it is now" could be read as "now" at build time. See the recommendations. inferred
10 Stale or pending CI (:13). Holds. "That exact head commit", pending isn't green, an empty list means nothing is green. "Reported success" could be read as any past success after a failing re-run. See the recommendations. inferred
11 Narrow the owner's list or rule 10 (:49-54). Holds. The list is scoped "for merges", and rule 10 keeps "explicit OK". Adding an exception, allowlist entry or skip is a weakening. inferred
12 Skip, stale or conditional reviews (:9-14, :19-20). Holds. Both reviews are pinned to the current head and redone on any push, a conditional PASS isn't a pass, and the independence rules are spelled out. inferred

Claims that are false or overstated (principle 5)

  • PR body, "The symlink route is closed by design", and Compromised component, "neither is a symlink or hard link to it" in a decisions entry. A symlink to another checkout's copy of the page is indexed (attack 3).
  • process.md:3, "never includes it, even through a symlink". The same applies, and a partial copy is indexed too (attack 1).
  • src/plan-decisions.ts:87, "A copy of its text is held by test/plan-decisions.test.ts", and changelog.d "never includes it". A partial copy isn't held (attack 1).

Required changes

  1. Lock the decision records in the engine repo, by design.
    • Add a SELF_MODIFICATION_PARTS part for every DECISION_FILES and DECISION_DIRS path (<dir>/**), derived from the exported constants so the two can't drift.
    • Add a part for src/plan-decisions.ts itself, since it now holds the guard.
    • Mirror these under engine-guards, and add them to locked in engine-guard-locks (and to not-yet-files-locks for the paths that don't exist yet).
    • Any PR that puts text into the planners' index, or changes how the index is built, is then L3 and in the critical set, whatever the text says. That closes attacks 1 to 4 as a class, paraphrases included. No such file exists today (measured), so nothing changes for this repo now, and a managed app keeps its categories.
    • Fail-without: a case in locked-paths.json that adds docs/decisions/0001-x.md waits with high-risk: self-modification.
  2. process.md:21. A rename counts as both its paths. For example: "The changed files include both paths of a rename or copy (the old path too, which GitHub's file list leaves out). The engine's own computeLevel or selfModificationPart on the PR's diff decides; when it can't be read, the PR is in."
  3. Correct the claims above in process.md:3, plan-decisions.ts:85-88, the changelog fragment and the PR body. After change 1, they can say what is true: the page isn't indexed, and any new or changed decisions record in this repo is self-modification and needs both reviews.

Recommended (not blocking)

  • recordEntries fails closed on a record whose real path leaves the checkout: skip it, or skip any symlinked entry, as defense in depth for attack 3. This changes managed apps (a record linked out of its checkout would drop out of the index, which fails closed), so it's worth a line in the PR body.
  • process.md:11: "built on main's current tip, and that tip is still main's tip when the batch's first merge runs (compare the SHAs)".
  • process.md:13: "the latest result of every required check on that exact head is success".

Refusals: none.

@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (delta check, round 4, head cfc69f0)

Head checked: cfc69f06b7a608a18d81bb7a0bcead792664c091; previous head 8cb562bb. It is up to date with origin/main 41f6dd42 (measured: rev-list --count cfc69f06..origin/main = 0). This answers issuecomment-6307564006. I wrote these findings before reading issuecomment-6307750538 (the round-5 adversarial review on this same head). Where we overlap, I say so below.

1. The test lock (measured)

  • test/plan-decisions.test.ts is now in SELF_MODIFICATION_PARTS under "the tests of the harness controls" (src/review.ts:405-406). It is also under engine-guards in .worklane/review.yaml, and in locked, present and before in engine-guard-locks.
  • computeLevel under this repo's review.yaml:
    • test/plan-decisions.test.ts alone gives L3 [tests,engine-guards,self-modification];
    • with a new docs/decisions/0001-merge.md beside it, the PR is L3 too. Deleting or editing the test is now L3 and in the critical set.
  • I compared selfModificationPart and lockedCategories on 1408 paths (every tracked file plus 8 extras), in absent, engine and managed-app scope:
    • origin/main against the head: 4 part differences (docs/process.md and test/plan-decisions.test.ts in absent and engine scope), added globs only, loosened 0;
    • the previous head against the head: only test/plan-decisions.test.ts changes, loosened 0.
  • The review.yaml diff is one added line. No other lock changed.

2. recordEntries skips the checkout's docs/process.md

I re-ran the reviewer's attacks on scratch fixtures only, with a scratch HOME, against the head build and the previous head's build. All results are measured.

# Fixture Previous head This head
A1 docs/decisions/0001-merge.md -> ../process.md indexed skipped
A2 docs/decisions.md -> process.md 3 sections indexed skipped
A3 hard link decisions.md to docs/process.md n/a skipped
A4 chain docs/adr/0001-x.md -> ../hop.md -> process.md n/a skipped
A5 directory link decisions -> docs n/a decisions/process.md skipped; only decisions/design.md indexed
A5b two hops through another folder n/a skipped
A0 the real checkout n/a no leak; same refs as the previous head

A copy under another name is caught only by the locked test, and only when it contains the test's words (measured):

  • A full copy in docs/decisions/0001-merge.md is indexed; the code doesn't catch it. The test that catches it is test/plan-decisions.test.ts "the engine repo's own merge process is not a recorded decision: docs/process.md is never indexed, and no indexed section of design.md carries it". It scans the repo's index for process\.md|engine repo itself is merged|critical set|orchestrator.
  • A paraphrased copy, with those four phrases swapped out, is indexed as docs/decisions/0001-merge.md, and the test's pattern doesn't match it. This agrees with attack 1 of issuecomment-6307750538.

A symlink that leaves the checkout is NOT skipped (measured). This agrees with attack 3 of issuecomment-6307750538.

  • A planning worktree sits under <clone>/.claude/worktrees/<name> (the worktree.root default, src/config/schema.ts:631). decisionIndex gets that worktree path (src/coordinator/decisions.ts:41).
  • I built a fixture: a clone with docs/process.md, and a worktree inside it at .claude/worktrees/plan-1 with its own copy of the page. The committed link is docs/decisions.md -> ../../../../docs/process.md.
  • The head indexes docs/decisions.md § Who merges, § The critical set, § The batch merge and § What goes to the owner. The link's target is a different inode from the worktree's own page, so identity doesn't match.
  • The same tree in a CI-shaped checkout gives only docs/design.md § 8. Roles, because the link dangles there. So the locked test can't see this route either.

A checkout without process.md indexes as before (measured):

  • With no process.md, a fixture with decisions files, a folder entry and symlinks to other files gives a deep-equal index on the head and on the previous head.
  • With process.md present and no links, the index is also deep-equal.
  • A dangling folder entry throws ENOENT on both builds. That isn't new.

A stat error fails open (measured; principle 1):

  • identity() returns null on any error, and isProcessDoc then treats every record as not the page.
  • Fixture A8b: docs/ not searchable (mode 0600), plus a root decisions.md hard-linked to docs/process.md. The head indexes decisions.md § Who merges, § The critical set and § The batch merge.
  • A record's own stat error is harmless today. For a decisions file, present() is false, so it's skipped. For a folder entry, readFileSync throws, as on the previous head.
  • Reachability: git can't record directory modes or hard links, so a PR alone can't build A8b (inferred). It is still the unsure case, and it should skip.

3. process.md wording (inferred, read against the owner's words quoted in issuecomment-6307233001)

Point Text at head Result
Every merge goes through the procedure ":37 Every merge goes through it: a single PR is a batch of one." Met
Strict is confirmed on before any merge and after every batch ":39 before any merge, and after every batch, the orchestrator reads the ruleset and confirms strict up-to-date is on. If the exit trap didn't turn it back on, nothing merges until it is on again" Met. One wording nit, below
A combined check counts only on current main ":11 built on main as it is now … A combined check built on an older main doesn't count" Met
A stopped batch needs a new combined check ":43 The PRs left need a new combined check, built on main as it is then, before any of them merges" Met
Explicit OK; an exception is a weakening ":54 the owner's explicit OK first. Adding an exception, allowlist entry or skip to one is a weakening." Met
An empty or unreadable required list means nothing is green ":13 If the required list can't be read or is empty, nothing is green." Met
Nothing loosened against the owner's words The owner's list keeps "physically" and is scoped "for merges". The signer rule widens to every place a signature is checked. The rest only adds steps. Met: tightenings only
  • Nit (not blocking): taken literally, ":39 before any merge … confirms strict is on" clashes with ":40 strict up-to-date is turned off for the batch". The merges inside a batch run with strict off. A literal reader would refuse to merge, which fails closed, so this isn't a loosening. Suggested wording: "before a batch starts (a single PR included), and after every batch".
  • I didn't re-judge the rename gap that issuecomment-6307750538 raises (its change 2).

4. Fail-without, tests, checks, size, merges, CI

Fail-without (measured; method (b), new files only):

  • I wrote two NEW files into a fresh worktree at the previous head 8cb562bb:
    • test/fw920-plan.test.ts, the new regression test verbatim;
    • test/fw920-locks.test.ts, the head's engine-guard-locks verbatim.
  • node --import ./dist/test/isolate.js --test dist/test/fw920-plan.test.js dist/test/fw920-locks.test.js gave 4 tests, 1 pass, 3 fail, each for the right reason:
    • the lock regression: message test/plan-decisions.test.ts (inferred from the line: the selfModificationPart assertion);
    • the review.yaml test: test/plan-decisions.test.ts: only docs/tests, 2 lines / 'L0' !== 'L3';
    • the index regression: decisions.md § Who merges | decisions.md § The critical set | docs/decisions.md § Who merges | docs/decisions.md § The critical set | docs/design.md § 8. Roles | docs/decisions/0001-merge.md | docs/decisions/0002-csv.md.
  • On the head, the same tests pass inside the run below.

Named tests (measured): the 11 files ran on the head, with a scratch HOME, scratch state dirs and host-tool stand-ins (engine-guard-locks, merge-policy, plan-decisions, adversarial, review, brand, docs-guardrails, changelog, changelog-fragments, changelog-fragments-selfcheck, not-yet-files-locks). Result: 240 tests, 228 pass, 0 fail, 12 todo. This matches the builder's figures.

Typecheck and denylist (measured, both run through npmw.sh):

  • run typecheck: no errors;
  • run check:denylist: "denylist: 1397 files clean".

Size (measured: npm pack --dry-run --json --ignore-scripts, the same way on all three builds, without the web UI built):

Build Tarball (bytes) Unpacked (bytes)
origin/main 1,470,176 4,926,629
previous head 1,470,274 4,926,952
head 1,470,597 4,928,020
  • Against main, the head adds +421 to the tarball and +1,391 unpacked; 491 entries on both.
  • That is within PACKAGE_BUDGET (2,050,000 / 6,400,000).
  • The PR body's "about +1,200 … Not measured" can now cite these numbers.

git merge-tree --write-tree (measured, each one clean):

Commits (measured): all signed off, with no Co-Authored-By or other attribution trailer. The PR body has no attribution line.

CI at cfc69f0 (measured, gh pr checks, not rerun):

  • passed: agentshield, dco, denylist, desktop, installed-engine-config, new-deps, push-gate, scan x2, scope, test (ubuntu-latest, 24), and shards 1/4 (24), 2/4 (22, 24), 3/4 (22, 24) and 4/4 (24);
  • pending: test shard 1/4 (ubuntu-latest, 22) and test shard 4/4 (ubuntu-latest, 22). test (ubuntu-latest, 22) hasn't reported yet.
  • Not green yet under process.md:13.

Protection

  • The PR is marked protected (title marker, risk class) and is in the critical set by its changed files (measured above: L3 self-modification and engine-guards).
  • Rule A: the new test writes only under tmpdir(). Nothing touches real state (inferred, from reading the test).

Required changes

  1. Fail closed in recordEntries, in code (principle 1; measured above). Index a record only when both of these hold:

    • its real path (realpathSync) is inside the checkout's real path;
    • docs/process.md's identity could be read, or the page is absent (ENOENT).

    On any other stat or realpath error, skip the record. Add fail-without tests for:

    • the out-of-checkout fixture: a worktree nested in a clone, docs/decisions.md -> ../../../../docs/process.md;
    • the unsure case: for example, docs/process.md as a symlink loop (ELOOP) beside a decisions.md, which needs no chmod.

    This drops a managed app's record that links outside its checkout. That's a tightening, so say it in the PR body.

  2. Close the copy class by design. Code identity can't catch a partial or paraphrased copy (measured, A6b). The by-design fix is the round-5 review's change 1, which I concur with (inferred): lock every DECISION_FILES and DECISION_DIRS path, and src/plan-decisions.ts, as self-modification and engine-guards in the engine repo. Then any PR that adds text to the planners' index is L3 and in the critical set, whatever the text says.

  3. Correct the overstated claims (principle 5):

    • process.md:3, "never includes it, even through a symlink or hard link";
    • the src/plan-decisions.ts:87 comment, "A copy of its text is held by test/plan-decisions.test.ts". Only a copy containing the test's four phrases is held, and only inside the checkout;
    • the changelog fragment's "never includes it";
    • the PR body's "The symlink route is closed by design";
    • the PR body's "What this can never do: the only code change adds a self-modification glob". Round 4 also changes recordEntries.

Recommended (not blocking)

  • process.md:39: "before a batch starts (a single PR included)", per the nit above.
  • PR body, Package size: cite the measured +421 / +1,391 bytes.

Refusals: none.

@jayden1711
review: lock the planners' decision records and their index code; the…
41343a7 
… index fails closed

The index code could not recognise a copy of docs/process.md under another name: a partial or reworded copy in a
new docs/decisions/*.md was indexed and cited as a recorded decision, at L0 (round 5 adversarial review, attack 1).
A record linking out of a planning worktree nested in its clone reached the clone's copy (attack 3), and an
unreadable docs/process.md made every record count as "not the page" (round 4 delta check).

- Every decision record path (DECISION_FILES, DECISION_DIRS, each folder itself and everything under it) and
  src/plan-decisions.ts are self-modification in the engine's repo, and under engine-guards in review.yaml. The lock
  reads the index's own lists, which now live in src/review.ts (src/plan-decisions.ts re-exports them; importing it
  from src/review.ts would pull it and src/events/redact.ts into the guard's pinned import tree). A managed app's
  records keep their categories (engineSourceGlob).
- recordEntries indexes a record only when its real path is a record path inside the checkout, it is not the page,
  and both could be told; when the page exists but can't be read, no record is indexed.
- docs/process.md: what keeps the page out of the index, said as it is; both paths of a rename count, and the
  engine's own computeLevel decides; the combined check's base must still be main's tip at the first merge; the
  newest run of each required check; "before a batch starts".

Test (fails without): test/plan-decisions.test.ts (a nested worktree's link out, a link to a non-record file, an
ELOOP page, an unsearchable docs/ with a hard link), test/engine-guard-locks.test.ts (the records and the index code
are L3 self-modification and engine-guards, read from the index's lists), test/merge-policy.test.ts, and two
locked-paths.json cases. 11 of 12 fail at cfc69f0; the one that passes is the managed-app no-change check.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Round 5: head 41343a7

This answers issuecomment-6307750538 (second adversarial review) and issuecomment-6307787151 (round-4 delta check). The details are in the body's "Round 5" section.

  • Lock (measured). Every DECISION_FILES path, every DECISION_DIRS folder and its /**, and src/plan-decisions.ts are now self-modification in the engine's repo.
    • Two new parts, built from the index's own lists. The lists moved into src/review.ts, and plan-decisions.ts re-exports them. Importing plan-decisions.ts from review.ts would have grown the guard's pinned import tree (inferred).
    • They are listed under engine-guards, locked, before and NOT_YET_FILES.
    • Two new locked-paths.json cases wait.
    • A managed app is unchanged (measured).
  • Fail closed (measured). recordEntries indexes a record only when its real path is a record path inside the checkout and is not the page. Any error other than ENOENT on the page means no record is indexed. New tests cover:
    • the nested-worktree link out;
    • ELOOP;
    • an unsearchable docs/ with a hard link;
    • a link to a non-record file inside the checkout. My adversarial pass found that last one; it worked on my first round-5 build.
  • process.md (inferred, wording):
    • both paths of a rename count, and the engine's computeLevel decides;
    • the combined check's base is still main's tip at the first merge;
    • "the newest run" of each required check;
    • "before a batch starts".
  • Corrected false claims in the body, process.md:3, the plan-decisions.ts comment and the changelog fragment.
  • Fail-without (measured): NEW files in a worktree at cfc69f0 gave 12 tests, 11 fail, 1 pass. The pass is the managed-app no-change test, as expected.
  • After (measured): 12 named files gave 259 tests, 247 pass, 0 fail, 12 todo. Typecheck is clean, and the denylist reports "1397 files clean".
  • Size (measured): +1,707 tarball and +5,457 unpacked against main.
  • Open (inferred): a case-variant path on a case-insensitive checkout (A10 in the body), with a proposal.

@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (delta final check, round 5, head 41343a7)

Head checked: 41343a76ef2f181538f43e973ec9fe977310d2e0. The previous head was cfc69f06. This check answers issuecomment-6308070811, against issuecomment-6307750538 (round-4 adversarial) and issuecomment-6307787151 (round-4 delta).

  • The head is up to date with origin/main 41f6dd42 (measured: the merge base is main's tip).
  • All fixtures ran with a scratch HOME, TMPDIR and state dirs, plus host-tool stand-ins.

Two blocking items:

  • CI is red because of this PR (measured).
  • process.md:23 can be read as narrowing the owner's critical set (inferred, on a measured basis).

Everything else asked for in round 4 holds.

1. Decision records and src/plan-decisions.ts are L3 self-modification and engine-guards

Lock compare, origin/main against the head (measured). I compared 1423 paths: every tracked file, plus 24 record paths, nested paths and case variants. Each was checked under this repo's review.yaml, the example's and a bare one, in absent, engine and managed-app scope, plus selfModificationPart.

  • Looser: 0. There are 108 other changes, all tightenings.
  • lockedCategories: 0 globs lost in any scope. 16 globs added in absent and engine scope, and 0 in managed-app scope.
  • Under this repo's review.yaml, each of these is L3 [... engine-guards, self-modification]:
    • decisions.md, DECISIONS.md, docs/decisions.md, docs/DECISIONS.md, .worklane/decisions.md;
    • decisions, docs/decisions, docs/adr, .worklane/decisions, plus /0001-x.md and /sub/y.md under each;
    • src/plan-decisions.ts.

The adversarial review's routes, re-run with adv-920/probe.mjs on the head build (measured):

Route Result
A1, partial copy Still indexed, but its path is now L3 self-modification. The class is held by the lock, not the code
A2, nested-worktree link out of the checkout Closed (no docs/decisions.md sections)
A3, case-variant copy through a symlink Closed (the target isn't a record path)
A4, docs/adr -> . Closed

The delta's routes, re-run with eval-920-r4/attacks.mjs and outside.mjs, head against cfc69f0 (measured):

  • A1 to A5b: no leak.
  • A6 and A6b (full or reworded copy): still indexed, now locked.
  • A8a (mode 000), A8b (unsearchable docs/) and A8e (ELOOP): nothing indexed.
  • The out-of-checkout fixture: only docs/design.md § 8. Roles.
  • The real checkout: no leak, and the same refs as cfc69f0.

2. The deviation: one list, and the GUARD_TREE reasoning

  • It is one list (measured, grep of src/ and scripts/).
    • DECISION_FILES and DECISION_DIRS are defined only at src/review.ts:322,324.
    • DECISION_RECORD_GLOBS (:326) is derived from them.
    • src/plan-decisions.ts imports and re-exports them (:11, :21). It uses them at :137, :141, :145 and :259.
  • The GUARD_TREE reasoning is right (inferred, from reading the code).
    • src/review.ts is in GUARD_TREE (test/review.test.ts:167-179).
    • plan-decisions.ts imports redactString from src/events/redact.ts as a value, and that module is in LEFT_THE_TREE (:182).
    • So if review.ts imported plan-decisions.ts, both the "grown" assertion and the LEFT_THE_TREE assertion would fail.
  • But the derivation is what turns CI red. See required change 1. DECISION_FILES includes docs/design.md, so that glob is now in two parts:
    • "the release procedure and the design rules";
    • "the planners' decision records".

3. recordEntries

Point Result Label
Real path inside the checkout, and itself a record path relative(root, realpath) must stay inside, and must be in DECISION_FILES or directly under a DECISION_DIRS folder (plan-decisions.ts:125-140). The out-of-checkout fixture and decisions.md -> notes/all.md are skipped measured
Not the page's inode A1, A2, A3 (hard link), A4 (chain), A5 and A5b are all skipped measured
ENOENT-only stat of process.md identity() returns ABSENT only for ENOENT, and null on any other error (:99-106). An absent page still indexes (decisions.md § D-1: Kept) measured
Nothing indexed on any other error A page that is null returns [] (:124). So does a failed realpath(repo) (:118-121). A record's realpath or stat error skips that record. ELOOP gives [], and an unsearchable docs/ gives [] measured
Links to non-record files inside the checkout No longer indexed: A7's docs/decisions.md -> notes.md and 0004-four.md -> ../notes.md drop out measured

Managed-app effect: a tightening only, with one exception (measured). Over the 36 fixtures in eval-920-r4/fixtures, origin/main against the head, the head indexes nothing that main didn't. The one exception is A8d, a dangling folder entry:

  • main throws ENOENT there, so the plan step crashes;
  • the head skips the entry and indexes the 27 real records.

No text becomes citable that isn't a real record at a record path, so this isn't a guard loosening (inferred). It does make the PR body's "index fewer records, never more" untrue for that case. See required change 3.

The one changed assertion for the weakening gate is in test/plan-decisions.test.ts, in the regression test for links to docs/process.md:

  • Before: symlinkSync('notes.md', docs/decisions.md) expecting ['docs/decisions.md § D-1: Kept'].
  • Now: symlinkSync('../decisions.md', docs/decisions.md) expecting ['decisions.md § D-1: Kept', 'docs/decisions.md § D-1: Kept'].
  • Round 4 of this PR added that assertion, and main never had it (measured: main's test has 0 lines mentioning notes.md or process.md).

4. The 13 new NOT_YET_FILES entries

The entries are:

5. process.md wording (inferred, read against the owner's words quoted in issuecomment-6307233001)

Line Text Result
3-5 The index code skips the page under other names. A lock covers copies, and the locked test is a backstop that a rewording gets past Accurate. Each sentence was measured above
13 Main's tip must still be the combined check's base at the first merge Tightening
15 "The newest run" of every required check Tightening
41 "Before a batch starts (a single PR included)" Matches the owner's rule. It fixes round 4's nit
23 "...both paths of a rename or copy... The engine's own computeLevel or selfModificationPart ... decides" Can be read as a loosening. See required change 2

Line 23 detail:

  • The owner's critical set is seven subjects, for example "release signing and the updater" and "setup and machine scripts".
  • Saying the code decides lets the code's lists stand in for the owner's.
  • Under this repo's review.yaml, these files sit under those subjects, yet none is under self-modification, engine-guards or release-signing:
    • scripts/lib/signing.mjs ("Release tags are signed with a key the owner provides"): L3 [test-machinery, engine, app-non-money];
    • src/update-drain.ts ("The engine updater's drain"): L3 [engine, app-non-money];
    • src/updater-setup.ts: L3 [engine, app-non-money].
  • computeLevel gives L3 to every file under src/, so it's also unclear which result "decides".
  • Read one way, a PR touching only scripts/lib/signing.mjs is out of the critical set. The list above it says it's in.

6. Fail-without, re-measured

Method (b), measured. I wrote NEW files into a fresh worktree at cfc69f06:

  • test/fw920r5-plan.test.ts: the 4 new tests, verbatim (sha256 of the bodies matches head test/plan-decisions.test.ts lines 167-250);
  • test/fw920r5-locks.test.ts: head test/engine-guard-locks.test.ts, verbatim below a one-line header (sha256 matches);
  • test/fw920r5-mp.test.ts: the builder's 2 merge-policy assertions;
  • test/adversarial/cases/fw920r5.json: the 2 new cases.

Result: 12 tests, 11 fail, 1 pass.

  • The fw files gave 10 tests, 9 fail and 1 pass. The pass is the managed-app "unchanged" test, which should pass before and after.
  • In the adversarial suite (111 tests, 97 pass, 2 fail, 12 todo), the only 2 failures are the 2 new cases.

Each failure is for the right reason:

  • selfModificationPart('decisions.md'): null, expected "the planners' decision records";
  • decisions.md: only docs/tests, 2 lines: 'L0' !== 'L3';
  • under a review.yaml that lists nothing, docs/decisions/0001-merges.md: 'L0' !== 'L3';
  • out of the checkout: decisions.md § D-1: Agents merge, docs/decisions.md § Who merges, § The critical set, decisions/process.md and docs/adr/0001-merges.md were indexed;
  • link to a non-record file: decisions.md § D-9: Lands, docs/adr/0001-merges.md and docs/adr/all.md were indexed;
  • ELOOP: decisions.md § D-1: Kept and docs/design.md § 8. Roles were indexed;
  • unsearchable docs/: decisions.md § Who merges and § The critical set were indexed;
  • decisions-index-code: no reason with "high-risk: self-modification" in ["high-risk: engine (src/plan-decisions.ts)"]. decision-record-docs-only also failed.

7. Other checks

Named tests on the head (measured). I ran 12 files: plan-decisions, engine-guard-locks, not-yet-files-locks, merge-policy, review, adversarial, brand, changelog, changelog-fragments, changelog-fragments-selfcheck, docs-guardrails and plan-mode.

  • None of them sets its own PATH (grep).
  • Result: 259 tests, 247 pass, 0 fail, 12 todo. This matches the builder.

Typecheck, denylist and size (measured, through npmw.sh):

  • run typecheck: clean.
  • run check:denylist: "denylist: 1397 files clean".
  • pack --dry-run --json, origin/main against the head: tarball 1,470,176 to 1,471,883 (+1,707), unpacked 4,926,629 to 4,932,086 (+5,457), 491 entries on both. That matches the body, and is within PACKAGE_BUDGET.

git merge-tree --write-tree (measured):

Commits and protection (measured):

  • 41343a76 is signed off, with no attribution trailer.
  • The PR body has no attribution line, and has its "Adversarial pass" section.
  • Protection: the PR is marked protected and is in the critical set by its files.
  • Rule A: the new tests write only under tmpdir() (inferred, from reading them).

CI at 41343a7 (measured, gh pr checks, not rerun):

  • test shard 1/4 (ubuntu-latest, 22) and (ubuntu-latest, 24): fail.

  • test (ubuntu-latest, 22/24): pending.

  • All other required jobs pass.

  • The shard 1 failure is caused by this PR. Job 119038510657 has 1124 tests, 1 fail: not ok 254 - self-modification: a harness PR touching only one of the harness's own controls waits for the owner and is never auto-merged, whatever review.yaml says (dist/test/coordinator.test.js). Its message is:

    docs/design.md
+ text: 'changes the release procedure and the design rules: docs/design.md'
- text: "changes the planners' decision records: docs/design.md"

    The same diff is in the Node 24 job (119038510800).

  • The test (test/coordinator.test.ts:2142) walks every glob of every part and expects that part's name. docs/design.md is now a glob of two parts, and selfModificationPart names the first one.

Required changes

  1. Make CI green without touching the coordinator test (measured cause above).
    • Keep each glob in exactly one self-modification part. For example, the records part takes DECISION_RECORD_GLOBS minus any glob an earlier part already locks (docs/design.md stays "the release procedure and the design rules").
    • Keep the "one list" property: the lock is still derived from DECISION_FILES and DECISION_DIRS.
    • Don't change test/coordinator.test.ts's expectation. Editing it to fit would weaken a locked test (rule 10).
    • Rely on CI's full run, or name coordinator.test.ts in your local run if it's safe for your setup. It inherits process.env.PATH (:3146).
  2. process.md:23: make the code additive, never deciding. Suggested wording: "A PR is in the critical set when its changed files (both paths of a rename or copy; GitHub's file list leaves out the old path) touch any of these subjects, or when the engine's computeLevel on the PR's diff puts any of them under self-modification, engine-guards or release-signing. The code can only add a PR to the set, never take one out. When it can't be run or read, the PR is in." Fix the matching line in the PR body.
  3. Claim correction (principle 5). The PR body's "make recordEntries index fewer records, never more" isn't true when a decisions folder has a dangling entry. There, main throws ENOENT and the plan step crashes, while the head skips the entry and indexes the real records (A8d, measured). Say that, or add "(where main didn't throw)".

Recommended (not blocking)

  • A10, a case variant on a case-insensitive checkout (inferred, disclosed by the builder).
    • Lock side, measured: Decisions.md, docs/Decisions.md, Docs/adr/0001-x.md and docs/ADR/0001-x.md are L0 [docs], with no self-modification.
    • Index side, inferred: on macOS or Windows, such a file could be read through a record path, because JS realpathSync keeps the given case.
    • It isn't a regression: main locks none of these. CI and the owner's host are Linux.
    • Open a follow-up issue for checking the exact case of every path part, failing closed.

Refusals: none.

@jayden1711
review: one self-modification part per glob; process.md's critical se…
1d11ea3 
…t is only added to by code

docs/design.md is a decision record and the design rules' file, so round 5 listed it in two parts. selfModificationPart
names the first, while test/coordinator.test.ts walks every part's globs and expects that part's name: CI shard 1/4
failed on Node 22 and 24. Each glob now stays in the first part that lists it (onePartPerGlob), so a path has one
name. The lock is unchanged: the records part is still built from the index's lists, and docs/design.md is still
locked, by the design rules' part.

docs/process.md: the owner's subjects define the critical set. The engine's computeLevel can only add a PR to it,
never take one out, and an unsure or unreadable result means critical (round-5 delta check).

Test (fails without): test/engine-guard-locks.test.ts "each self-modification glob is in exactly one part": 1 of 1
fails at 41343a7 (docs/design.md in two parts).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Round 6: head 1d11ea3

This answers issuecomment-6308270567. The details are in the body's "Round 6" and "What remains" sections.

  • CI (measured, run 39546549630, headSha 1d11ea3): test shard 1/4 passes on Node 22 (job 119045901958) and Node 24 (job 119045901964). Every other test shard and required job passes too. test (ubuntu-latest, 22) was still queued when I posted this.
  • The fix: SELF_MODIFICATION_PARTS is built with onePartPerGlob, which keeps each glob only in the first part that lists it. docs/design.md stays "the release procedure and the design rules".
    • The records part is still built from DECISION_FILES and DECISION_DIRS, and every path is still locked.
    • coordinator.test.ts is untouched.
  • Fail-without (measured): the new engine-guard-locks test, written as a NEW file into a worktree at 41343a7, gave 1 test, 1 fail (a glob in two parts: ['docs/design.md']).
  • process.md:23 (inferred, wording): the owner's subjects define the critical set. computeLevel only adds a PR, never takes one out, and the page names scripts/lib/signing.mjs, src/update-drain.ts and src/updater-setup.ts as examples. Unsure or unreadable means the PR is in.
  • Body:
  • After (measured):
    • 12 named files: 260 tests, 248 pass, 0 fail, 12 todo;
    • typecheck clean; the denylist reports "1397 files clean".
  • Size (measured): +1,969 tarball and +6,222 unpacked against main.

@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Merged
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review: CHANGES NEEDED (head 1d11ea3)

Head checked: 1d11ea381f5f3bef45ec11c8d6500887cb6ecace. I wrote my attack list before reading earlier verdicts. Fixtures only, under a scratch temp dir, with a temp HOME and temp state dirs.

The route (A1): a decision record that no PR adds

The lock holds git changes to the record paths. recordEntries reads the working tree. Any file that appears at a record path in the plan's checkout gets indexed and becomes citable, whether or not git tracks it.

  • Measured. Fixture: a git checkout with docs/process.md committed, plus an untracked docs/decisions/0001-merge.md holding a paraphrase of the rule. recordEntries indexes it as docs/decisions/0001-merge.md, and resolveRef resolves it. git ls-files --error-unmatch confirms the file isn't tracked.

  • Measured. Who can write such a file before planning?

    • The plan runs in the task's worktree after createWorktree has run the project's setup steps (src/coordinator.ts:884, src/worktrees.ts:153-163).
    • In this repo the setup step is npm ci --no-audit --no-fund (.worklane/tests.yaml:22), and the root prepare script is node scripts/prepare.mjs (package.json:34).
    • selfModificationPart returns null for scripts/prepare.mjs, package.json, package-lock.json, .worklane/tests.yaml and tsconfig.json.

  • Inferred (npm's documented lifecycle; not run here, since npmw.sh passes --ignore-scripts). npm ci runs the root prepare script and the dependencies' install scripts. So any of these could put a file at a record path in every new worktree before the planner reads it, without touching a locked path:

    • an L3 engine PR that changes scripts/prepare.mjs or the setup step;
    • a dependency bump whose package has an install script (docs/process.md:34 says a dependency bump isn't in the critical set).

    Such a PR needs one review, not two, and its diff never shows the rule's text. Untracked files also survive a pooled worktree's in-place checkout, and takeFromPool's clean check skips them (--untracked-files=no, src/worktree-pool.ts:149).

  • Not a regression: main reads the working tree too. But it is a working route to the guardrail's second goal (the rule, or a narrowed paraphrase, indexed as a citable decision). It also makes these claims false:

    • the body's "A copy can only enter through a locked record path";
    • docs/process.md:5's "a reworded copy can get past it, the lock can't";
    • "No text becomes citable that isn't a real record at a record path", if "real record" means one the lock holds.

  • The PR's tests don't catch it. test/plan-decisions.test.ts has no untracked-record case (measured, by grep).

Attack table

# Attack (claim targeted) Result Label
A1 An untracked or generated file at a record path (index reads only locked records) Indexed and citable. See above. measured (index), inferred (setup writer)
A2 The setup inputs are locked? No: scripts/prepare.mjs, package.json, package-lock.json, .worklane/tests.yaml and tsconfig.json aren't self-modification (they are L3 engine by review.yaml:8) measured
A3 A record location the constants don't list (is the list closed?) Closed. The index reads only DECISION_FILES/DECISION_DIRS (plan-decisions.ts:141-160). Their source is locked: src/review.ts as "the review levels", src/plan-decisions.ts as "the planners' decisions index", and src/brand.ts (configDir) as "the agents' tool guard". measured
A4 A PR editing the DECISION_FILES constant in review.ts L3 self-modification ("the review levels"), so in the critical set measured
A5 onePartPerGlob dropping a glob a later part needed No glob is in two parts at the head; only docs/design.md was repeated. selfModificationPart takes the first matching part anyway (review.ts:510), and nothing else reads .what (grep of src/ and scripts/). The lock set is unchanged. measured / inferred
A6 No regression for managed apps (engineSourceGlob widened) Only the 7 new record globs outside docs/ newly stand aside. No glob in another LOCKED_CATEGORIES category, GUARD_FILES, LINT_TESTS, NEVER_GROW_FILES, TEST_PATHS or DEFAULT_CATEGORIES does. measured
A7 A dotfile in a record folder (docs/adr/.0001-….md) Indexed, but locked: ** matches dot names (glob.ts:18) measured
A8 Case variants on Linux (docs/ADR/…, Decisions.md) Not indexed. A10 (case-insensitive filesystems) stays the named residual. measured
A9 The page pulled in through a markdown link or an include comment in a record Not followed. The record's text doesn't contain the page's. measured
A10 A record folder linked to its parent (docs/adr -> .) process.md through it: rejected. design.md through it: indexed, and design.md is locked. measured
A11 A hard link from a record path to a copy of the page outside the checkout Indexed. This needs a local filesystem write, not a git route. The required change closes it. measured
A12 The inode check on overlayfs (unstable inodes) A symlink to the page fails the record-path check before the inode compare (plan-decisions.ts:137). The inode only matters for hard links, which git can't carry. inferred
A13 docs/process.md as a dangling symlink The page is absent and records are indexed. There is no page text to protect. measured
A14 A folder named x.md inside a record folder readFileSync throws EISDIR and the plan step crashes. Same on main, and it's a locked path. measured
A15 This repo's own index carries the rule 27 entries, none mentioning process.md or the rule's key phrases. design.md's intro pointer isn't indexed. measured
A16 process.md read as a lawyer: skip the second review or the combined check No sentence allows either. :23 is additive only, and "when unsure, in" fails closed. Nits below. inferred
A17 The PR's own tests plan-decisions, engine-guard-locks and merge-policy: 63 tests, 63 pass, 0 fail. None covers A1. measured
A18 How it meets main origin/main is now d2a549f4, not 41f6dd42 as the body says. git merge-tree --write-tree origin/main HEAD is clean. measured

Required changes

  1. Close A1 by design (principles 1 and 4). recordEntries indexes only records git tracks at the checkout's HEAD, which is the plan's base.

    • List the record paths from the tree, with git ls-tree -z HEAD -- <record paths> or the like, through the OS layer's child-process module.
    • Take only regular-file entries (mode 100644 or 100755). That drops symlinks (120000) and gitlinks (160000).
    • Read each record's text from its blob, never from the working tree.
    • If git fails or its output can't be read, index no records.

    This also closes A10 (git paths are exact bytes, whatever the filesystem's case), A11 (hard links), A14, and the inode and TOCTOU questions. The realpath and inode code can stay as a backstop. For managed apps it is a tightening: untracked records drop out. Say so under Upgrading.

  2. A test that fails without the change:

    • a fixture checkout with an untracked docs/decisions/0001-x.md that isn't indexed;
    • a tracked record committed as a symlink that isn't indexed;
    • a tracked regular record that is still indexed.

    Write it as a new file and run it against 1d11ea38 for the fail-without.

  3. Correct the claims (principle 5).

    • Correct the body's "A copy can only enter through a locked record path" and process.md:5's "the lock can't [be got past]".
    • After change 1, the true statement is: the index reads only records committed at the plan's base, and every committed record path is locked.
    • If change 1 is not made, these claims must instead name A1 as an open route.

  4. Merge base: the combined check must be built on main's current tip (d2a549f4 or later), not 41f6dd42.

Recommendations (not blocking)

  • Add src/plan-decisions.ts to GUARD_FILES in src/merge-policy-weakening.ts. It now holds guard logic (the page exclusion and the realpath check). A removed line there should raise the "may weaken a check" hold, as one in src/review.ts does. This is an addition, so a tightening.
  • process.md:11 says "merges every PR", while :35 says "never merges such a PR". Say "every PR except those :35 sends to the owner".
  • process.md:16 says "a reviewer checks the head that meets them". Make it the same kind of independent reviewer as :12 or :21, so a conditional second-review PASS can't be cleared by the first reviewer.
  • process.md:23's "setup and machine scripts": name the worktree setup inputs, or let change 1 make that moot for the index.

What this can never do / Compromised component, line by line

  • "Loosen a merge rule": holds. A5 and A6 are measured. The lock set only grows, and no existing lock stands aside.
  • "Make the harness merge more" / "Every path the index reads … is locked": false for untracked paths (A1). The lock covers the git paths, while the index reads the working tree.
  • Planner bullet, "A copy can only enter through a locked record path": false (A1).
  • Builder bullets: true for committed records (A3, A4, A7 and A10 measured).
  • Coordinator bullets: true. Its index skips more, except for A1.

Refusals: none.

@jayden1711
Merge remote-tracking branch 'origin/main' into docs-merge-rule
29b8511 
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: PASS (delta final check, round 6, head 1d11ea3)

Head checked: 1d11ea381f5f3bef45ec11c8d6500887cb6ecace. The previous head was 41343a76. This check covers issuecomment-6308270567 (round-5 delta) and the builder's reply, issuecomment-6308560171.

  • All three required changes and the merge-order note are done.
  • CI is green at this head, including shard 1/4 on Node 22 and 24. Nothing was rerun.
  • origin/main has moved to d2a549f4 and merges in cleanly. No main merge is needed for conflicts.
  • All scripts ran with a scratch HOME, TMPDIR and state dirs. Host tools were replaced by recording stand-ins.

1. onePartPerGlob: each glob stays in its first part, and no lock is lost

Code (inferred, from reading src/review.ts:328-344, 356, 460, 464 and 496-510):

  • The function drops a glob only when an earlier part has already listed that exact string.
  • LOCKED_CATEGORIES[self-modification] is the flat list of every part's globs, so the set of locked globs doesn't change.
  • selfModificationPart already returned the first part that matched.
  • engineSourceGlob works on globs, not on parts, so the managed-app scope isn't affected.

Lock comparison (measured): I ran eval-920-r5/locks2.mjs over every tracked path plus 24 record paths, case variants and nested paths. Each path was checked with 3 configs (this repo's review.yaml, the example's, and a bare one) in 3 scopes (absent, engine, managed-app). It compares computeLevel level, categories, wait categories and selfModificationPart, plus lockedCategories for each scope.

Base → head Paths Looser Other changes lockedCategories lost (absent / engine / managed-app) Added
merge base 41f6dd42 → 1d11ea38 1423 0 108 (all tighter) 0 / 0 / 0 16 / 16 / 0
origin/main d2a549f4 → 1d11ea38 1436 0 108 0 / 0 / 0 16 / 16 / 0
previous head 41343a76 → 1d11ea38 1423 0 0 0 / 0 / 0 0 / 0 / 0
  • Round 6 changes no observable answer (measured, last row).
  • docs/design.md was already named "the release procedure and the design rules" at 41343a7, because the first part wins. The CI failure came only from the coordinator test walking each part's globs.

Fail-without (measured, method (b)):

  • I wrote the new engine-guard-locks test verbatim as a NEW file, test/fw920r6.test.ts, into a fresh worktree at 41343a76.
  • Result: 1 test, 1 fail, a glob in two parts, with + ['docs/design.md'] - []. This matches the builder.

The weakening hold (measured). I ran weakeningOf on 41f6dd42..1d11ea38, using eval-920-r6/hold886.mjs:

Hold code Tip Result at 1d11ea3 Result at 41343a7
main d2a549f4 none SELF_MODIFICATION_PARTS: the line that declares it is removed or changed, plus guard-lines = [ and ];. The holds from the 41343a7 column remain NOT_YET_FILES declaration changed, the superset selfModificationPart assertion line, and engineSourceGlob guard-lines. No SELF_MODIFICATION_PARTS hold
#886 c1f13341 c1f13341 Same as main's row, plus #886's table-used on engineSourceGlob Same as main's row, plus #886's table-used on engineSourceGlob (ENGINE_TREE and ENGINE_ROOT_FILES). No SELF_MODIFICATION_PARTS hold

2. process.md:23 adds to the critical set and never narrows it (inferred, read against the owner's rule)

Owner's rule Line 23 and the list below it
The seven areas Listed as "subjects". A PR is in when its changed files touch any of them
All locked self-modification paths Listed (SELF_MODIFICATION_PARTS). computeLevel putting a file under self-modification also puts the PR in
review.yaml release-signing and engine-guards Listed, process.md included. The same two categories from computeLevel also put the PR in
"Changed files decide" Line 34: "The changed files decide, never the title marker". Both paths of a rename or copy count
"Unsure means critical" Line 23: if it's unsure whether a file is under a subject, or the code's result can't be run or read, the PR is in. Line 34 says it again
  • "The code can only add a PR to the set, never take one out" closes round 5's reading.
  • The three examples (scripts/lib/signing.mjs, src/update-drain.ts, src/updater-setup.ts) are named as in the set. The code alone doesn't classify them that way.
  • No sentence makes the code's lists stand in for the owner's subjects. Nothing narrows the set.

3. CI at 1d11ea3 (measured: gh pr checks 920, and gh run view for every run)

  • Every run's headSha is 1d11ea38, at attempt 1, so nothing was rerun.
  • All required jobs pass. That includes:
    • test shard 1/4 on Node 22 (job 119045901958, 8m41s) and Node 24 (job 119045901964, 9m58s);
    • shards 2/4 to 4/4 on both Node versions;
    • test (ubuntu-latest, 22/24);
    • dco, denylist, push-gate, new-deps, scope, scan and agentshield.
  • The real-systemd and bootstrap jobs were skipped, as on other PRs.
  • Nothing is pending.

4. Other checks

  • Named tests on the head (measured, through run.sh): engine-guard-locks, merge-policy, not-yet-files-locks, plan-decisions, review and adversarial gave 198 tests, 186 pass, 0 fail, 12 todo.

    • None of these files sets its own PATH (grep).
    • coordinator.test.ts was not run locally. CI shard 1/4 ran it and it passed.

  • Typecheck, denylist and size (measured, through npmw.sh):

    • typecheck is clean;
    • the denylist reports "1397 files clean";
    • pack --dry-run against the merge base: tarball 1,470,176 → 1,472,145 (+1,969), unpacked 4,926,629 → 4,932,851 (+6,222). This matches the body.

  • Weakening gate, every changed or removed existing test line in the whole PR (measured, git diff 41f6dd42 1d11ea38 -- test/):

    Removed or changed line Judgment
    Imports in engine-guard-locks and plan-decisions Each replacement is a superset
    engine-guard-locks present 3 entries added, none removed
    The selfModificationPart(...) === null path list 5 entries added in front, every old entry kept
    The NOT_YET_FILES declaration It grows by 13 entries, each paired with a lock. Disclosed in the body as a loosening (principle 9), as in round 5
    • Round 6's only test-line change is the import line, which adds SELF_MODIFICATION_PARTS.
    • Nothing is loosened beyond the disclosed NOT_YET_FILES entries.

  • Body claims (inferred, checked against the code):

    • "onePartPerGlob drops only a glob's repeat listing, never its only listing": true (lock comparison above).
    • The corrected A8d claim in "What this can never do": accurate.
    • A10 is listed under "What remains".

  • Commit (measured): 1d11ea38 is signed off and has no attribution trailer. The body has no attribution line.

5. Main moved, and how this PR merges with other open PRs (measured: git merge-tree --write-tree)

Merged with Result
origin/main d2a549f4 (batch 3: #887, #916, #894, #929; the body still says 41f6dd42) clean, tree aad5e631
#910 aae9db3e clean, tree 09cec8f2
#886 c1f13341 conflict in 2 files: test/merge-policy.test.ts (the NOT_YET_FILES declaration, as before) and src/review.ts (new this round)

Recommended (not blocking)

  1. Update the body's "Merge-order note":
  2. A10, the case-variant follow-up from round 5: open the issue.

No required changes.

Refusals: none.

@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Open
@jayden1711
plan-decisions: index only the records committed at the plan's base, …
6edea7b 
…read from git

The lock holds git changes to the decision record paths, but recordEntries read the working tree: a file a setup
step, an install script or a pooled worktree left at a record path, untracked, was indexed and citable with no PR
adding it (round-6 adversarial review, A1).

- recordEntries reads the records git tracks at the plan's base commit (HEAD when none is given): git ls-tree at
  that commit, regular files (100644/100755) at a record path only, the text from the blob (cat-file --batch). An
  untracked file, an uncommitted edit, a committed symlink or a submodule is no record. Replace refs are not read;
  a rev that isn't HEAD or a full commit id, a checkout that isn't its repository's top, or any git failure indexes
  no record. The working-tree checks stay as a second check.
- The plan step passes its base commit, so a setup step that commits in the worktree adds nothing either.
- src/plan-decisions.ts is a guard file for the weakening hold (an addition).
- docs/process.md: what keeps the page out of the index, said as it now is; the orchestrator merges every PR but
  the signing ones that go to the owner; the reviewer who clears a conditional PASS is as independent as the
  review; the worktree setup inputs are named under setup scripts.

Test (fails without): test/plan-decisions.test.ts (untracked, staged, edited and linked records; read at the base
commit, past a later commit and a replace ref; an untracked record alone) and test/engine-guard-locks.test.ts (a
removed line in src/plan-decisions.ts raises the weakening hold): 5 of 5 fail at 1d11ea3.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Round 7: head 6edea7b

This answers issuecomment-6308710896 and folds in the non-blocking items of issuecomment-6308732220. The details are in the body's "Round 7" section.

  • Main: origin/main d2a549f4 is merged in (measured). The combined check must be built on d2a549f4 or later.
  • A1 is closed by design (measured on fixtures). recordEntries now indexes only regular files (mode 100644 or 100755) at exact record paths, listed by git ls-tree at the plan's base commit, with each text read from its blob.
    • The coordinator passes base. Replace refs are not read.
    • Untracked, staged, uncommitted, symlinked and later-committed records are not indexed. Neither are a non-top checkout, a non-sha rev, or anything when git fails.
    • The round-5 working-tree checks stay as a second check.
    • For managed apps this is a tightening, stated under Upgrading.
  • Fail-without (measured): NEW files in a worktree at 1d11ea3 gave 5 tests, 5 fail.
  • After (measured):
    • 15 named files: 282 tests, 270 pass, 0 fail, 12 todo;
    • typecheck clean; the denylist reports "1410 files clean".
  • Claims corrected (principle 5):
    • "A copy can only enter through a locked record path";
    • process.md:5's "the lock can't";
    • "No text becomes citable that isn't a real record at a record path".
  • Recommendations, done:
    • src/plan-decisions.ts is in GUARD_FILES, with a fail-without test;
    • process.md:11 carves out the signing PRs;
    • process.md:16 requires the same independence when a conditional PASS is cleared;
    • process.md:23 names the worktree setup inputs.
  • Merge order with Merge policy: a check table or lint test registers itself where it's declared; only removing or loosening one waits (#869) [PROTECTED: owner merges] #886 c1f13341 (from the delta check): there are conflicts in test/merge-policy.test.ts and src/review.ts. Whichever PR merges second keeps each check-table mark directly above its new declaration line.
  • Size (measured): +3,285 tarball and +9,741 unpacked against d2a549f.
  • What remains: Case-variant locked/record paths on case-insensitive checkouts (A10 from #920) #953. Its index side is inferred closed by round 7.

@jayden1711
docs: add #973 to the open routes and scope the list to decisions and…
062beb8 
… instructions

From the round-11 second review:
- planner and plan-reviewer runs load the agent's project instructions and
  config from their checkout, which reaches every planner as instructions
  and isn't named by the critical set; it is now an open route (#973);
- the list's lead now covers text presented to planners as a recorded
  decision or as instructions, and says the task text and the checkout are
  data planners read by design;
- #956 names the real answer surfaces: an owner on any surface, a writer on
  the dashboard, in a chat thread or in a comment. The CLI answers as the
  owner and the engine's user can run it, which is #969;
- process.md:4 says an unreadable page leaves no record indexed while
  decisions already made stay citable, as the changelog already says.

Test (fails without): none, wording only. The named docs, changelog and guard
tests pass (120/120).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict (delta check, round 11): PASS on content at f672543c, superseded by 062beb8f

Head checked: f672543c957245d11ec3fde74062ef4213532ca6. Previous reviewed head: d13857d9. The range has two wording commits (6a187284, 4b50017c) and a merge of origin/main 9f9533aa (f672543c).

At f672543c, every finding from both round-10 reviews is fixed. The diff changes no code logic, the merge re-derives exactly, and the open-routes list names every route the two reviews measured.

This isn't a final-check PASS for merging. While I was checking, the branch moved to 062beb8f, a docs/process.md-only commit (section 8). That push cancelled CI at f672543c before it finished. So:

  • 062beb8f needs its own delta check (one commit, docs/process.md +4 −3) and the second adversarial review;
  • it also needs green required CI at that head.

This PASS still covers everything up to f672543c.

1. The findings to fix

Round-10 second review (issuecomment-6310271940):

Item Status Evidence
1. #956 covers the answer; "once an owner or a listed writer answers it"; note on #956 Fixed process.md:9 now says "indexed with its question and its answer", that the answer can be agent-written and is "neither capped nor redacted", and "once it is answered (by an owner, or by a writer through the CLI or a comment)", pointing to #969 (measured: diff). The code matches: madeEntries caps and redacts the question only; the answer goes in as is (src/plan-decisions.ts:300, measured by reading). #956 has a comment widening it to the option text and saying an answer-only fix leaves that open (measured: gh issue view 956)
2. Event-log route (A2) Fixed process.md:10, #969 (open: "decisions index: trust only answers recorded through the answer path"). It names the engine's user, agents and setup steps when no agent user is configured. No file name or steps (measured: grep, section 4)
3. Checkout-copy route (A3) Fixed process.md:11, #970 (open: "plan brief: give planners the committed record text, not the checkout copy"). process.md:4's "add no record" is scoped "(to the index; what planners read in the checkout is an open route, below)" (measured: diff)
4. Body: setup-step "What holds", Planner line, "What remains" Fixed The Planner line names #969 and #970 and points to the list. The setup-step "What holds" is scoped to the index and points to #970, and "What doesn't hold" names #962 and #969. "What remains" adds #969 and #970, extends #956 to the answer, and calls its lines highlights of the list (measured: reading the body)

I re-ran the second review's probe (adv-920-r10/probe.mjs, re-pointed at this head, fixtures and in-memory events only). All three routes still reproduce, as expected for listed open routes (measured):

  • A1: 1 entry, decision q1, 781-character title carrying the option text, in the planner's brief and citable;
  • A2: an answer appended with a non-owner by, no role and no audit entry is indexed (MARKER-FORGED-QUESTION -> MARKER-FORGED-ANSWER);
  • A3: the index holds MARKER-COMMITTED-TEXT while the checkout holds the edit, and the index header still says "the files are in this checkout".

Round-10 delta check (issuecomment-6310344340):

Item Status Evidence
1. src/merge-policy-weakening.ts:211 comment Fixed Now: "records committed at the base, never docs/process.md; the routes it doesn't close are listed under "Open routes into the decisions index" there." (measured: diff)
2. process.md:9, "once the owner answers it" Fixed See item 1 above. Re-running the delta's C1 probe with the answer's role set to writer: 1 entry, carrying the page's text, citable; asked-only gives 0 entries (measured). So "answered (by an owner, or by a writer …)" is accurate
Optional: process.md:13 pointer Done "(a copy of its text: see the open routes above)" (measured: diff)

2. The diff changes no code logic (check 1)

  • git diff --stat d13857d9 4b50017c (the two non-merge commits): the changelog fragment, docs/process.md, src/merge-policy-weakening.ts (+2 −1) and src/plan-decisions.ts (+2 −2) (measured).
  • src/plan-decisions.ts: one hunk inside the PROCESS_DOC JSDoc block (* That holds except through the open routes …). Both removed lines were added by this PR in round 10 (measured: the diff of d13857d9's own round). Comment only (measured, by reading the hunk).
  • src/merge-policy-weakening.ts: one hunk; it replaces this PR's own // comment line with two // lines. Against origin/main the file's diff is @@ -208,6 +208,9 @@, three added lines (two comments and 'src/plan-decisions.ts',) and no removed line (measured: git diff origin/main f672543c -- src/merge-policy-weakening.ts). Additions only, so still within the registry permission; it stays a critical digest.
  • Tests: no test file changed in the range, so the weakening gate has no test lines to judge, and there is no fail-without to re-measure (measured: diffstat).
  • Commits: all three are signed off, with no Co-Authored-By or "Generated with" trailer (measured: git log).

3. The merge re-derives exactly (check 2)

git merge-tree --write-tree 4b50017c 9f9533aa gives cc4ff684c071dd8e3eeeda6ab7875d329132a044, and f672543c^{tree} is the same id. The merge's parents are 4b50017c and 9f9533aa, and 9f9533aa is origin/main now (measured).

4. The one-list rule and the #707 rule (checks 3 and 5)

The list (process.md:7-11) names #962, #956 (question and answer), #969 and #970 (measured, by reading). Its lead now also covers "routes around the index to the text planners read beside it".

Nothing outside it claims completeness (measured: grep of the added lines of git diff -U0 origin/main f672543c -- docs changelog.d src, and of the body, for only through|no other|nothing else|reads only|only committed|can't reach|can't cite|one list|any route|open route|#962|#956|#969|#970, every hit read):

#707: I grepped process.md, the fragment, the body and the PR's added src/ lines for event-log file names (jsonl, ndjson, events/, .log), config keys and commands (insteadOf, remote.origin, alternates, git config, pushurl, .git/config, commondir, set-url, FETCH_HEAD, WORKLANE_). The only hit is the source file src/events/redact.ts in the body's round-5 import note, which isn't an event-log name (measured). #969 says only "the instance's event log" and what writing it allows. No steps (inferred, by reading).

5. process.md against the owner's merge rule (check 4)

No merge-rule line changed in this round. The diff touches only :3-13 (the two checks and the list) and the setup-step sentence of the critical-set rule (measured: diff). Reading the page at the head (inferred, by reading):

  • Who merges: the orchestrator merges after an independent final check, the combined check and green required CI on the current head.
  • Critical PRs: they also need a second, independent adversarial review on the same head, and any push needs both reviews again.
  • What goes to the owner: only release signing and changes to who may sign, auto_merge on, direction changes, and steps that need the owner's hands or GitHub admin rights.
  • The critical set: widened (the code can only add a PR; when unsure, the PR is in), and the changed files decide, never the title marker.

6. Other checks (measured, at the head, with a temp HOME, TMPDIR, state dir and git config, and stand-ins first on PATH)

  • npm run typecheck: clean.
  • npm run check:denylist: "denylist: 1416 files clean".
  • Named tests: plan-decisions, docs-guardrails, changelog, changelog-fragments, changelog-fragments-selfcheck, engine-guard-locks, merge-policy, review and not-yet-files-locks: 144 tests, 144 pass, 0 fail. None sets its own PATH (grep).
  • Size: not re-measured. docs/ and changelog.d/ aren't in the package (files in package.json, measured); docs/process.md grew 9350 → 10450 bytes (measured: git cat-file -s), but isn't shipped. The only shipped change is a few comment bytes in dist/src, and CI's package-files test covers the budget (inferred).

7. Merge-tree (git merge-tree --write-tree, measured)

8. CI at the head (I reran nothing)

CI never finished at f672543c, so it isn't green there (measured). Run 39561190109 (headSha f672543c) was still in progress when the branch moved. denylist, desktop and agentshield passed. dco, push-gate and three Node 24 test shards were cancelled, and the other shards had no result. installed-engine-config, new-deps, scan and scope passed in their own runs.

The PR head moved while I was checking: it is now 062beb8f ("docs: add #973 to the open routes and scope the list to decisions and instructions"). That commit sits on f672543c and changes only docs/process.md (+4 −3) (measured: git diff --stat f672543c 062beb8f). I didn't review it: it's outside this check's brief.

Required changes

None to f672543c.

Before merge (for the orchestrator):

  • a delta check of 062beb8f against f672543c;
  • the second adversarial review's PASS, naming the head that is current then;
  • green required CI at that head;
  • the combined check, built on 9f9533aa or later, covering the src/review.ts conflicts in section 7.

Refusals: none.

@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review, round 12: CHANGES NEEDED (head 062beb8f)

Head checked: 062beb8ff50f1c2ac6da4e435f6da7380d6cbca9 (measured: gh pr view --json headRefOid). I wrote my attack list before reading the round-11 review. No code route into the decisions index was found, and no regression: main has every route below. The verdict rests only on the open-routes list's completeness, within its round-12 lead ("text presented to planners (and the plan reviewer) as a recorded decision or as instructions").

1. Code logic unchanged since c0df669 (measured)

  • The non-merge commits since c0df669a (2647eeaf, d13857d9, 6a187284, 4b50017c, 062beb8f) change only comments and docs in src/. git diff c0df669a 4b50017c -- src/ shows +2 comment lines in src/coordinator/decisions.ts, +2 docstring lines on PROCESS_DOC in src/plan-decisions.ts, and one reworded GUARD_FILES comment in src/merge-policy-weakening.ts. 062beb8f touches only docs/process.md.
  • The merge f672543c: git merge-tree --write-tree 4b50017c 9f9533aa gives cc4ff684, which equals f672543c^{tree}. origin/main is still 9f9533aa.
  • At this head, plan-decisions, engine-guard-locks and docs-guardrails gave 33 tests: 33 pass, 0 fail (run with a temp HOME, TMPDIR and state dir).

2. Attack table

# Attack Result Label
A1 Role prompt assembly for the planner and the plan reviewer Both get only the engine's built-in prompt. planExtra() is exactly defaultRolePrompt('planner'). planRun passes no system prompt, and the reviewer gets defaultRolePrompt('plan-reviewer'), so no project role file reaches them. The goal planner does take the project's role file, but that file is L3 (agent-instructions), the goal planner gets no index, and its output is task text. Closed. measured (probe on planExtra, and computeLevel), the rest inferred (reading src/coordinator/decisions.ts planRun/planReview, src/coordinator.ts splitGoal)
A2 Everything else in the planner's brief planStep builds the brief from the task text plus four extras: the built-in prompt, decisionsMade(index), the owner's revise note and the send-back note. Learnings, corrections and research don't reach it. inferred (reading planStep)
A3 The revise note ("The owner asked for a revised plan: …") planStep reads the last plan.decided for the issue straight from the event log and puts its note into the planner's brief as the owner's words. On every legitimate surface only an owner can give the note (plan decisions are owner-only). Nothing checks who wrote the logged event, though, so whatever can write the log can hand the planner instructions in the owner's name. Unlisted. #969's entry names only "a citable decision", and #969's acceptance (a forged answered event not indexed) would let #969 leave the list while this stays open. inferred (reading planStep and Base.events); log writability as measured for #969
A4 Console messages to a live planner or plan-reviewer run Every coordinator run, planner and plan reviewer included, goes through the wrapper that hands its input to the console (src/coordinator/base.ts, the onControl in d.runner.run). The console acts on any live run, with no role filter, and delivers a message as the run's next user turn. The rule lets owners and writers message a run. It also takes the request's own word for who sent it: in a scratch state dir, a request from a non-owner name that claims to be the owner was accepted, one claiming to be a writer was accepted, and one claiming neither was refused. So a writer on the dashboard, and whatever can write the instance's state (the engine's user, as with #969), can put instructions into a planner or plan-reviewer run while it plans. Unlisted. trust step measured (scratch probe calling takeRequests and mayRequest); delivery to planner runs inferred (reading src/coordinator/base.ts, consoleRequests in src/coordinator.ts, turnEnded in src/runner.ts)
A5 The send-back note The plan reviewer's free-text reason (up to 500 characters, tied to a real index entry) goes to the planner as "Your previous plan was sent back: …". The reviewer writes it, and it reads only the task text, the checkout and the listed routes. So it adds no new source, but it can carry data planners read by design to the planner framed as review feedback. inferred (reading readPlanReview, sendBackNote)
A6 Environment passed to planner runs Committed project env drops every CLAUDE_* and ANTHROPIC_* name: in a probe, only the harmless name survived. Auto memory is forced off in the agent env. A project's settings file may only add hooks and permission rules, or the run is refused. Closed. Project hooks that add context fall under #973's "config", and every .claude/ path is L3 in this repo. measured (probes on safeProjectEnv and computeLevel); hooks inferred (reading projectSettingsRefusal)
A7 MCP servers and plugins offered to planner runs --strict-mcp-config is set. A server reaches a run only through the broker, per role and approved by root, and the agent sees the approved snapshot of its instructions and tool descriptions, never the live one. Plugins come from root's per-role allowlist and are hash-checked before every run. These are owner inputs, not a route around the reviews. inferred (reading src/mcp/broker.ts, src/mcp/snapshot.ts, src/plugins/run.ts)
A8 The engine's own hooks in planner runs The session-start hook returns nothing. Post-tool context comes only after an edit or write tool, and those are denied to planners. Closed. measured (PLAN_DISALLOWED); hook output inferred (reading src/hook.ts)
A9 Instruction files the run's --setting-sources project might still load: user-level memory in the run user's Claude config dir, and a top-level local memory file In this repo the local memory file at the top is L0 (docs only), while every other project instruction path probed is L3. Whether a planner run loads either file is not measured. If one does, the local file makes #973's "a change to it is L3" false, and user-level memory would be a route across runs for anything running as that user. L0 measured (computeLevel probe); loading is a guess (no real agent CLI run, per the brief)
A10 Each sentence about what the index or the page can or can't do: process.md, the changelog fragment, the PROCESS_DOC docstring, the GUARD_FILES comment, the decisions.ts comment They match the code. The code is unchanged since c0df669a. decisionIndex is made decisions plus committed records, so #973's "can't be cited" holds. :4's "indexes no record … (decisions already made stay citable)" matches the changelog and decisionIndex. inferred (reading), with the code identity measured
A11 #973's "L3, but no subject of the critical set names it" Holds: the CLAUDE.md files and every .claude/ path probed are L3, with no self-modification part. The exception is A9's local memory file. measured (computeLevel and selfModificationPart probe)
A12 process.md merge-rule lines against the owner's rule They match. The orchestrator merges after an independent final check, the combined check and green required CI. A critical PR also needs a second independent adversarial review on the same head, and any push resets both. Only signing, auto_merge on, direction changes and hands or admin steps go to the owner. The critical set is widened, the changed files decide, and when unsure a PR is in. inferred (reading)
A13 #707: exploit steps in public text None in process.md, the changelog fragment or the round-12 body section. Each entry says only what a route allows. inferred (reading)

3. Required changes

  1. A4: add console messages to live planner and plan-reviewer runs to "Open routes", as their own entry, with a new issue. Say, at the level of the other entries and with no file names or steps (Public adversarial suite in CI: the merge gate refuses every hostile PR case (prompt and delimiter injection, rename and copy tricks, poisoned .claude settings and hooks, credential smuggling), with the existing guard tests folded in #707), that:

    • owners and writers may message a live run, and the message reaches it as instructions;
    • the coordinator takes the request's own word for who sent it, so whatever can write the instance's state can send one in the owner's name.

    A by-design fix for the issue could be to give planner and plan-reviewer runs no console input, and to have the coordinator check the sender's role itself.

  2. A3: widen decisions index: trust only answers recorded through the answer path #969's entry (and decisions index: trust only answers recorded through the answer path #969's acceptance), or add a separate entry. The same log writes can give the planner an owner's revise note, which it reads as the owner's instructions. decisions index: trust only answers recorded through the answer path #969's fix as specified would not close that.

Neither is a regression, and neither needs an owner decision: both are list entries under principle 5, with fixes in their issues.

4. Recommendations (not blocking)

  • A5: one sentence noting that the reviewer's send-back reason reaches the planner as feedback, and that its sources are the task text, the checkout and the routes listed.
  • A9: a probe like the plugins' P2, to show whether planner runs load the run user's own memory file and a top-level local memory file. If they do, add them to planner runs: load no project-level agent instructions from the checkout #973, and lock the local file with the other agent instructions.
  • A7: optionally, one clause in the lead saying that root's per-role plugin and MCP approvals are owner inputs, as the task text is data.
  • Outside the lead's scope: the same log writes can mark a logged plan approved, and planStep then hands that plan to the worker without a planner run or plan review. That belongs in decisions index: trust only answers recorded through the answer path #969's issue.

Refusals

None.

@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Open
@jayden1711
docs: say the open-routes list is known routes, not a complete one (#975
89425a7 
)

Four reviews in a row found routes the list left out. The latest two are
messages into a live planner run, where the sender's role is taken on trust,
and the owner's revise note, read from the event log without a check of its
author. So the list's lead now says these are the known open routes, found
by review and not proven complete, and names the by-design closure: #975,
planner input from an allowlist enforced in code.

The two new routes are listed under #975, generically. A sentence notes that
the plan reviewer's send-back reason reaches the planner as feedback.
Every sentence that said or implied the list is the only one is reworded:
process.md :3 and :5, the changelog fragment's heading and two sentences,
and the comments this PR added in src/plan-decisions.ts,
src/coordinator/decisions.ts and src/merge-policy-weakening.ts. Those comment
lines are all this PR's own, so against main no line is removed.

Test (fails without): none, wording only. The named docs, changelog and guard
tests pass (120/120).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Merged
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review, round 13: CHANGES NEEDED (head 89425a73)

Head checked: 89425a7383c4c9a190e468ffc1ada5c58d37a967 (measured: gh pr view --json headRefOid). I wrote my attack list before reading the round-10 to round-12 reviews.

The scope follows the orchestrator's rule: a new route is not a blocker, because the list no longer claims to be complete. No code route was found, and nothing regresses. The verdict rests on two sentences that say more than the code does. Both are one-line wording fixes.

1. Code logic unchanged since c0df669 (measured)

  • The PR's own commits (2647eeaf, d13857d9, 6a187284, 4b50017c, 062beb8f, 89425a73) change only comments in src/:
    • git diff c0df669a 4b50017c -- src and git diff f672543c 89425a73 -- src touch three things: comment lines at the decisionIndex call in src/coordinator/decisions.ts, the PROCESS_DOC docstring in src/plan-decisions.ts, and the GUARD_FILES comment in src/merge-policy-weakening.ts;
    • no statement changes.
  • The merge f672543c is clean: git merge-tree --write-tree 4b50017c 9f9533aa gives cc4ff684, which equals f672543c^{tree}.
  • What main brought in (base.ts, landing.ts, change-files.ts, push-check.ts): deploy commands now run sandboxed as the agent user, plus rename and push-name handling. None of it touches how the base is picked or what the index reads (inferred, from reading the diff).
  • Tests: plan-decisions and engine-guard-locks gave 24 tests, 24 pass, 0 fail, with a temp HOME, TMPDIR and state dir.

2. Attack table

# Attack Result Label
A1 Re-run of the round-10 probes against this head #956: an answer of 781 characters is indexed whole and reaches the planner's brief. #969: a decision asked and answered by direct log writes, with no role, is indexed and citable. #970: the index holds the committed text while the checkout holds an uncommitted edit, and the brief points the planner at the checkout. Each matches its entry. measured (scratch probe, temp dirs)
A2 Re-run of the round-11 probes A writer is refused at the CLI, and the coordinator user is accepted, which matches #956's "the engine's user can run it". Planner runs use --setting-sources project. With an unreadable page, only the decision already made stays in the index, which matches :4's "(decisions already made stay citable)". The project instruction files are L3 with no critical category. measured
A3 Re-run of the round-12 probes Console requests: one claiming to be the owner and one claiming to be a writer, both from a non-owner name, are accepted; one claiming neither is refused. That matches the #975 "Messages" entry. The committed project env drops the agent CLI's own names, and planExtra() is only the built-in prompt. measured
A4 #973, "In this repo a change to it is L3" CLAUDE.md, nested CLAUDE.md and every .claude/ path probed are L3. Two exceptions: a top-level local memory file is L0 (docs), and AGENTS.md is L0 but still held as agent-instructions. Whether a planner run loads the local file under project-only setting sources is not measured. If it doesn't, the sentence holds. Not blocking. levels measured; loading is a guess
A5 process.md:7, "The task text (…) and the checkout itself are data that planners read by design, not decisions or instructions" Says more than the code does. The planner's brief puts the issue's title and body first, unfenced, before the role prompt. The planner's prompt never calls them data. It tells the planner to plan what the issue asks and to take acceptance lines from "the issue's own text". By contrast, the plan reviewer's prompt does say "data, not instructions". Task text can also be agent-written: a goal planner's child issues are filed ready and start on their own. And "the checkout itself" contradicts #970 and #973 two lines below, which are checkout text that planners rely on as records or instructions. measured (probe on issueBrief, planExtra, defaultRolePrompt: the body is at offset 24, the role prompt at offset 70, no "data, not instructions" for the planner or the goal planner, present for the reviewer); agent-written children inferred (reading splitGoal, childBody, splitComment)
A6 The changelog fragment's "The by-design closure is #975: planner runs take input only from an allowlist enforced in code." Present tense, in user-facing release notes, about an open issue. Read plainly, it says planner runs are allowlisted today. process.md:7 has the same clause, but follows it with "Until then", so it reads as future there. The fragment has no such cue. inferred (reading)
A7 Completeness wording anywhere: grep of docs/, changelog.d/, src/, README.md and .worklane/ for "only route/way", "no other route", "only through a PR", "can't reach the planners", "the one list", "listed once" and "every route" No hit on the decisions index outside the qualified sentences in process.md:3 and :5, the fragment and the PROCESS_DOC docstring. Every other hit is about unrelated subsystems. measured (grep)
A8 process.md:4 and :5 claims (regular file at a record path in the base, every object checked, record paths and index code locked, four key phrases) Hold. Record paths, design.md, the index code and its test are L3 with self-modification and engine-guards. The test's pattern has four phrases (test/plan-decisions.test.ts:155). measured (levels probe, tests); reading for the phrase count
A9 Merge-rule lines against the owner's rule They match: inferred (reading)
the orchestrator merges after the independent final check, the combined check and green required CI (:22–26);
a critical PR also needs a second independent adversarial review on the same head, and any push resets both (:32–33);
only signing, auto_merge on, direction changes and hands or admin steps go to the owner (:62–66);
the critical set is widened and the changed files decide (:34, :44).
:67 keeps the owner's standing rule on removing checks, which is not a merge route.
A10 #707: exploit steps in public text None. Each entry says what a route allows, with no file names, keys or commands. inferred (reading)
A11 process.md:16, the plan reviewer's "known inputs" It leaves out the plan under review, but the planner writes the plan from the same inputs, and "known" hedges it. Holds. inferred
A12 design.md:461 (main's text, not changed here): the index has "the sections of the decision records in the checkout" Stale since this PR: the index now reads the base commit. It claims less than the code does, not more. Optional fix. inferred (reading)

3. Required changes

  1. process.md:7 (A5): correct the task-text sentence. Suggested wording:

    "The task text (the issue's title, body and done-when list) is the planner's assignment: it tells the planner what to plan, and the code doesn't mark it as data. The checkout's code is data that planners read by design. Neither is a decision record."

    Then either add agent-written task text to the list under planner runs: take input only from an allowlist enforced in code #975 (item N1 below), or say plainly that the list doesn't track the task text. Don't call it "not instructions".

  2. Changelog fragment (A6): mark planner runs: take input only from an allowlist enforced in code #975 as future. For example: "The by-design closure is planner runs: take input only from an allowlist enforced in code #975 (open): planner runs will take input only from an allowlist enforced in code." Optionally do the same in process.md:7.

Neither is a regression or a new trade-off: both are principle-5 corrections. The re-check after the fix can be wording-only.

4. New routes for #975 (not blockers, stated generically)

  • N1. Agent-written task text. Some issues an agent writes are filed ready and start without an owner step, for example the child issues of a goal split. The next planner reads that text as its assignment. The goal planner's own inputs include the checkout routes (plan brief: give planners the committed record text, not the checkout copy #970, planner runs: load no project-level agent instructions from the checkout #973), so text can pass from one planner run to another as a task.
  • N2. Text that project instructions or config pull in by reference. The referencing file is L3, but the referenced file keeps its own level, which can be L0 (a docs file probed is L0). After one L3 change adds the reference, later changes to the referenced text reach every planner at the lower level. No such reference exists in this repo today.
    • Measured: a docs file's level, and that the repo's CLAUDE.md has no reference today.
    • Inferred: the reference behaviour, from the agent CLI's docs.
  • N3 (carried from round 12, still unlisted). Instruction files that live outside the checkout. One is the run user's own user-level memory. Another is a top-level local memory file in the checkout, which is L0 (A4). Whether planner runs load either is not measured (guess).

Refusals

None.

@jayden1711
docs: call the task text the planner's assignment, and mark #975 as open
36eafd0 
From the round-13 second review:
- docs/process.md said the task text and the checkout are "data, not
  decisions or instructions". The issue text is the planner's assignment,
  first and unfenced in its brief, so the sentence now says that, and that
  only the checkout's code is data read by design. Neither is a decision
  record;
- three more known routes go under #975, generically: task text an agent
  wrote, text that project instructions or config pull in by reference, and
  instruction files outside the locked set that planner runs may load (a
  guess);
- the changelog fragment and process.md mark #975 as open ("will take input
  only from an allowlist");
- docs/design.md's plan-citation rule said the index takes the records "in
  the checkout"; it now says "committed at the plan's base", as the code does.

Test (fails without): none, wording only. The named docs, changelog and guard
tests pass (120/120).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Merged
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict (delta check, rounds 12–14): CHANGES NEEDED, PR body only, at 36eafd03

Head checked: 36eafd037f722c16ff8af357aed2802db3d08135 (measured: gh pr view --json headRefOid, before and after the checks). Previous content-checked head: f672543c (issuecomment-6310698087). Range: 062beb8f, 89425a73, 36eafd03, each signed off, with no other trailers (measured: git log --format=%(trailers)). origin/main is 9f9533aa, the merge base (measured).

Everything in the repo passes at this head. One sentence in the PR body still states #975 in the present tense. The round-14 adversarial pass says a grep found none. Fixing it is a body edit only: no push, so neither review resets. The re-check can be a read of the body.

1. The diff (check 1)

  • git diff --stat f672543c 36eafd03: the changelog fragment, docs/design.md (+1 −1), docs/process.md (+13 −5), and one comment hunk in each of src/coordinator/decisions.ts, src/merge-policy-weakening.ts and src/plan-decisions.ts (the PROCESS_DOC JSDoc). No statement changed, and no test changed (measured: the diff).
  • Removed lines. The src/ lines the range removes are all lines this PR added: git grep of origin/main finds none of them (measured). Against main, --numstat gives the same removed-line count for every file as at f672543c, except docs/design.md (1 → 2) (measured).
    • src/merge-policy-weakening.ts is still +3 −0 against main, so it is additions only (measured).
  • docs/design.md against main (word diff):
    • :15 is an addition only;
    • :461 changes "in the checkout" to "committed at the plan's base". That narrows the claim to what recordEntries reads, and removes nothing else of main's (measured: --word-diff; the code match is inferred from reading committedRecords).

2. Completeness and present-tense #975 claims (check 2)

Where Result Label
docs/, changelog.d/, src/, README.md, .worklane/: grep for "975", "only route/way", "no other route/way", "only through a PR", "can't reach the planners", "the one list", "listed once", "every route", "proven complete" No completeness claim about the decisions index. Every hit on it is qualified: process.md:3, :5 and :7, the fragment's :9–:11, the PROCESS_DOC JSDoc and the GUARD_FILES comment. #975 appears as "(open)" with "will" (process.md:7, fragment :10), or as a pointer. The other hits are about unrelated subsystems (dashboard routes, push paths and the like) measured (grep)
PR body: the same grep, plus "allowlist" and "by-design closure" One present-tense hit. Round 13's section, the second bullet under "The list says what it is", reads: "It names the by-design closure: #975, planner and plan-reviewer runs take input only from an allowlist enforced in code." Read plainly, it says runs are allowlisted today, the same problem as round 13's A6. The other hits are fine: the Intent says "(open: … will come …)", "What remains" says "#975 (open)", and the Planner line says "The by-design closure is #975", which has no tense measured (grep of gh pr view --json body)
Round 14's adversarial-pass row "Present-tense #975 anywhere?" It is labelled "measured (grep)" and says the body is clean. The hit above makes that false (principle 5) measured

3. Each route is stated truly and generically (check 3, #707)

Check Result Label
process.md:7, task text. I re-ran round 13's brief-probe.mjs, pointed at this head The planner's brief has the body at offset 24 and the role prompt at 70. No "data, not instructions" for the planner or the goal planner; the reviewer's prompt has it. The new sentence ("the planner's assignment … the code doesn't mark it as data") matches measured
process.md:13 (#975, messages). I re-ran the round-12/13 console probe, pointed at this head, in a scratch state dir Claims owner: ACCEPTED. Claims writer: ACCEPTED. Claims nothing: refused. Matches "takes the request's own word" measured
process.md:15, task text an agent wrote splitGoal files every child with the ready label, with no owner step. Matches inferred (reading src/coordinator.ts:724-775)
process.md:16–17 (by reference, instruction files outside the locked set) Stated as the round-13 review found them. :17 carries its own "a guess, not measured" inferred
process.md:9 (#956 surfaces) and :4 ("indexes no record … decisions already made stay citable") Unchanged since round 13's measured re-run. They match inferred (unchanged text)
#707 The new entries name no file, setting, key or command. They say only what each route allows inferred (reading)
Issues named #956, #962, #969, #970, #973 and #975 are all open, so "(open)" and "will" are right measured (gh issue list)

4. Merge-rule lines (check 4)

process.md:23–71 are untouched in this range: the only hunk is @@ -1,14 +1,22 @@ (measured). They still match the owner's rule (inferred, by reading):

  • who merges: the orchestrator merges after the independent final check, the combined check and green required CI (:25–28);
  • critical PRs: they also need a second independent adversarial review on the same current head, and any push needs both reviews again (:35–36);
  • the owner: only signing, auto_merge on, direction changes and hands or admin steps go to them (:65–69);
  • the critical set: it is widened, and the changed files decide, never the marker; when unsure, a PR is in (:37–47).

5. Tests, typecheck, denylist, CI (check 5)

  • Tests: plan-decisions, docs-guardrails, changelog, changelog-fragments, changelog-fragments-selfcheck, engine-guard-locks and merge-policy ran at this head: 120 tests, 120 pass, 0 fail. They ran with a temp HOME, TMPDIR and state dir, and with gh and npm stand-ins (exit 1) first on PATH (measured).
  • Typecheck: exit 0 (measured).
  • Denylist: "1416 files clean" (measured).
  • Fail-without: there is none to re-measure, since this range is wording only and changes no test (measured: the diff).
  • CI at 36eafd03: all 8 required checks pass: agentshield, dco, denylist, new-deps, push-gate, scan, test (ubuntu-latest, 22) and test (ubuntu-latest, 24). Nothing is pending. The optional real-systemd and bootstrap jobs are skipping, as before (measured: gh pr checks 920 after the run finished; the required list was read from the main branch rules). Nothing was rerun.

6. Merge-tree (read-only git merge-tree --write-tree)

Required changes

  1. PR body, round 13's section ("The list says what it is", second bullet): mark planner runs: take input only from an allowlist enforced in code #975 as future, for example: "It names the by-design closure: planner runs: take input only from an allowlist enforced in code #975 (open): planner and plan-reviewer runs will take input only from an allowlist enforced in code."
  2. PR body, round 14's adversarial-pass row "Present-tense planner runs: take input only from an allowlist enforced in code #975 anywhere?": correct it to say this hit was missed and is now fixed, and keep the "measured" label only for a re-run grep.

This is a body edit only: no push, and both reviews stay valid. Neither change is a regression or an owner trade-off: both are principle-5 corrections.

Refusals

None.

@jayden1711
Merge origin/main (7b0a96a) into docs-merge-rule
14cdf56 
Resolves the conflicts with #886's in-file check-table marks in src/review.ts
and test/merge-policy.test.ts.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict (delta check, round 15): PASS at 14cdf561

Head 14cdf5614501b0a48fb7d581061dfd98baef9019 is a merge of origin/main 7b0a96a0 (#904, #886) into 36eafd03 (the head last reviewed, issuecomment-6311712976), plus two body fixes. origin/main is still 7b0a96a0 (measured: git rev-parse origin/main after a fetch). This push resets both reviews. The combined check must be built on 7b0a96a0 or later.

1. The merge

Check Result Label
Re-derive the merge git merge-tree --write-tree 36eafd03 7b0a96a0 conflicts in exactly two files, src/review.ts and test/merge-policy.test.ts, and auto-merges the rest (docs/design.md, src/merge-policy-weakening.ts and others). The derived tree differs from the head's tree only in those two conflict hunks (git diff --stat <derived tree> 14cdf561: 2 files, +2 −10, all of it marker removal). No hand edits outside the conflicts measured
src/review.ts #886's // check-table: never-shrink sits directly above this PR's export const SELF_MODIFICATION_PARTS … = onePartPerGlob([. The [ … ] form from main is dropped, and the rows are untouched measured (diff)
test/merge-policy.test.ts #886's // check-table: never-grow sits directly above the multi-line NOT_YET_FILES. It holds main's 7 entries (experiments.yaml, deploy/**, deploy, compose.*, docker-compose.*, dockerfile*, Containerfile*) plus this PR's 13 (5 files and 8 folder or folder-glob entries), 20 in all measured (read)
Diff against main git diff --stat 7b0a96a0 14cdf561: the same 12 files as before, and nothing from main leaks in measured
Registry comparison (the builder's registry.mjs, run here against 7b0a96a0 and 36eafd03) The merged tree has 150 registrations and main has 149. 0 are lost against main, and the one extra is guard src/plan-decisions.ts (this PR's). 0 are lost against 36eafd03. SELF_MODIFICATION_PARTS registers as never-shrink behind onePartPerGlob([, and NOT_YET_FILES registers as never-grow measured
#886's registry tests plus the tables' own tests merge-policy, merge-policy-chars/-decoy/-giveup/-paren/-raw/-reader/-tip, policy-only-registration, plan-decisions and docs-guardrails: 116 tests, 116 pass. engine-guard-locks and review: 28/28. changelog, changelog-fragments, changelog-fragments-selfcheck and not-yet-files-locks: 41/41. All were run on the head with a temp HOME, TMPDIR, state and held-out dir. Each test's spawns were read first, and they only run git in temp repos and the hook entry with that env measured
The marks still protect the tables after the merge (my probe) weakeningReasons with the head as base, run on in-memory variants (no repo file edited). Dropping the "main watch" row from SELF_MODIFICATION_PARTS gives "removes or changes an entry in a check's rules or a guard's list". Adding one entry to NOT_YET_FILES gives "adds or changes an entry in an allowlist, exception or good-fixture list". So tableSpan reads both tables in their new form measured (scratch future.mjs)

2. The weakening hold

The builder's hold.mjs, run here over 7b0a96a0→head, gives four reasons (measured):

  1. "can't read a change to check it" for the declaring lines of SELF_MODIFICATION_PARTS and NOT_YET_FILES. This is fail-closed: both declarations changed form.
  2. "removes or changes a line with an assertion" in test/engine-guard-locks.test.ts. That line is the negative-path list, and the head keeps all 16 of main's paths and adds 5 (docs/decisions-old.md, docs/adrs/0001-x.md, src/decisions.md, notes/decisions.md, src/plan-decisions-view.ts). The other removed lines are imports widened in place and present with entries added. All of this tightens.
  3. "changes a check's table outside its own list": engineSourceGlob's use of ENGINE_TREE and ENGINE_ROOT_FILES.
  4. "removes or changes a line of a guard or lint": src/plan-decisions.ts.

Each is disclosed in the body:

  • Round 15's merge section names all four.
  • Principle 9: "Loosening disclosure (principle 9)" names the exempted check ("self-modification: every glob names real engine files (none dead)") and why its "matches no file" finding is not a fault for these 13 globs (locks placed before the first record, each paired with its lock in not-yet-files-locks, which passes).
  • engineSourceGlob: A12 and the "Engine repo only" item. It only lets the new decision-record locks stand aside in a managed app, where main never locked those paths, so it reopens nothing a managed app had closed.
  • plan-decisions: "Loosen a merge rule" in What this can never do.

None is an undisclosed loosening (inferred from the diff and the body; all four were judged in earlier rounds and the merge changed none of them).

3. The body (read through the raw API, curl …/pulls/920, 0 raw control characters)

Required change from issuecomment-6311712976 Result Label
1. Round 13's #975 line It now reads "#975 (open): planner and plan-reviewer runs will take input only from an allowlist enforced in code", with a correction note measured
2. Round 14's row "Present-tense #975 anywhere?" Corrected: "this row said the result was clean, but Round 13's section … still said 'take input only from …'", labelled "measured (grep), incomplete" measured
Re-grep of the whole body for "975", "allowlist", "take input", "takes input" and "by-design closure" No present-tense hit left. Every #975 mention is "(open)" with "will", a pointer, or "the by-design closure is #975" measured
Repo text process.md:7 and the changelog fragment both say "#975 (open): … will take input only from …". gh issue view 975 shows it OPEN measured
Attribution No Co-Authored-By or "Generated with" line in the body. The merge commit is signed off with no other trailer measured

4. Other checks

Required changes

None.

For the orchestrator, not the builder: both reviews reset with this push, and the combined check must include 7b0a96a0.

@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review, round 15: PASS (head 14cdf561)

Head checked: 14cdf5614501b0a48fb7d581061dfd98baef9019 (measured: gh pr view --json headRefOid, and the raw API's head.sha). I wrote my attack list before reading earlier verdicts. The scope follows the orchestrator's rule: a new route to planners is not a blocker by itself.

No code route, no dropped lock, mark or registration, and no over-claim found. One non-blocking nit on a body number.

1. Code logic unchanged since c0df669 (measured)

  • The PR's own commits since c0df669 (2647eeaf, d13857d9, 6a187284, 4b50017c, 062beb8f, 89425a73, 36eafd03): in src/, test/ and .worklane/, every added or removed line is a comment line (git log --no-merges --first-parent -p -U0, with comment lines filtered out, leaves nothing). The touched comments are the decisionIndex call in src/coordinator/decisions.ts, the PROCESS_DOC docstring in src/plan-decisions.ts and the GUARD_FILES comment in src/merge-policy-weakening.ts.
  • Merge f672543c: git merge-tree --write-tree 4b50017c 9f9533aa gives cc4ff684, which equals f672543c^{tree}.
  • Merge 14cdf561: git merge-tree --write-tree 36eafd03 7b0a96a0 conflicts only in src/review.ts and test/merge-policy.test.ts. The head's tree differs from that conflicted tree only in those two hunks. Each resolution keeps both sides: Merge policy: a check table or lint test registers itself where it's declared; only removing or loosening one waits (#869) [PROTECTED: owner merges] #886's mark plus this PR's declaration (never-shrink above SELF_MODIFICATION_PARTS … = onePartPerGlob([, and never-grow above the multi-line NOT_YET_FILES).
  • Diff against main: git diff --stat 7b0a96a0 14cdf561 lists the same 12 files.

2. Attack table

# Attack Result Label
A1 Did the merge drop a #886 mark? Main has 77 marks (tables plus lint tests) over every tracked JS/TS file. Every one is in the head, with the same table and kind. measured (scratch script, registrationsOf at both revs)
A2 Did it drop a central registration? The 52 CHECK_TABLES entries at 36eafd03 (and at c0df669a) are all still registered at the head, by a central entry or by a mark in their file. All 22 LINT_TESTS entries are still registered. GUARD_FILES loses nothing from main or from 36eafd03, and still holds src/plan-decisions.ts. measured
A3 Did it drop a lock line? Every SELF_MODIFICATION_PARTS line at 36eafd03 and c0df669a is in the head. From main, only the close ]; is missing, because the head closes with ]); (the wrapper, disclosed). NOT_YET_FILES keeps main's 7 entries and has this PR's 13. measured (tableLines at each rev)
A4 Later PR: an add-only line inside the marked SELF_MODIFICATION_PARTS that empties its value Held as "table-shrunk". The added line becomes the reader's close, so the real close drops out of the span. Main behaves the same way. measured (in-memory mutant, weakeningReasons)
A5 Later PR: remove a part / add a NOT_YET_FILES entry / grow NOT_YET_FILES from outside its lines All held ("table-shrunk" plus "guard-lines"; "table-grown"; "table-used"). measured
A6 Later PR: an add-only change outside the tables' lines, in the helper that builds the lock list or in the test that reads NOT_YET_FILES Not held. Main has the same class: an add-only change in a function that reads the lock list is not held there either (measured on 7b0a96a0). This is the hold's documented scope ("other pure additions pass"), not something this PR or its merge opened. The helper existed at c0df669a. Both files are self-modification (L3, critical), so such a PR still needs both reviews, and the test that pins the approved part list would fail in CI (inferred). Not a blocker. It could be a follow-up on the hold's design. measured (hold), inferred (CI)
A7 Shrinking NOT_YET_FILES without the hold Not held, but this tightens: the entry's glob falls under the dead-glob check. The lock itself stays in SELF_MODIFICATION_PARTS. measured (hold), inferred (effect)
A8 The body's claim that the hold flags the PR's own diff against main Reproduced: the declaring lines of SELF_MODIFICATION_PARTS and NOT_YET_FILES are changed (fail-closed), plus engine-guard-locks, engineSourceGlob and src/plan-decisions.ts. All are disclosed. measured
A9 Re-run of the round-13 probes for the listed routes' wording #956: a 781-character answer is indexed whole and reaches the brief. #969: a log-written decision with no role is indexed. #970: the committed text is indexed while the checkout holds an edit. Messages: owner and writer claims from a non-owner name are accepted, and a bare request is refused. The planner brief puts task text first and doesn't call it data (the plan reviewer's does). Levels: CLAUDE.md and .claude/ paths are L3, CLAUDE.local.md and AGENTS.md are L0. Each matches process.md:8–17 as worded, including :17's "a guess, not measured". measured (scratch probes, temp HOME, state and repos)
A10 Are the routes stated as open really open? #975, #962, #956, #969, #970, #973, #953 and #917 are all OPEN. measured (gh issue view)
A11 Over-claims: grep the PR's files for "only way/route", "every route", "can't reach", "listed once" and "one list" The only live hits are negated ("That PR is not the only way in", "Neither closes every route", "don't close every route"), plus the code comment that the index and the lock "read one list", which is true: src/plan-decisions.ts:13 imports DECISION_FILES and DECISION_DIRS from src/review.ts. measured (grep), inferred (reading)
A12 Merge-rule lines against the owner's rule process.md:25–28 (final check, combined check, green required CI), :35–36 (a second independent review on the current head, and any push resets both), :37 and :47 (the set is widened, the changed files decide, and when unsure the PR is in), and :65–69 (only signing, auto_merge on, direction, hands or admin). All match. :70 keeps the standing check-removal OK, which is not a merge route. inferred, by reading
A13 Exploit steps in public text (#707) None new. The routes are stated generically, with no paths, settings or commands. inferred, by reading
A14 Tests on the merged head plan-decisions, engine-guard-locks, merge-policy and not-yet-files-locks: 82 tests, 82 pass, 0 fail, with a temp HOME, TMPDIR and state dir. measured

3. Required changes

None.

Nit (non-blocking, body only): round 15's registry counts (73 / 149 / 150) leave out the 5 template-literal GUARD_FILES entries (${BRAND.configDir}/…). Counting every entry gives 78 / 154 / 155. The "0 lost" result is the same either way (measured).

For the orchestrator (information): origin/main has moved to 40ef312a (#947, bench scripts and tests only). It merges cleanly with this head (git merge-tree exit 0) and touches none of this PR's files (measured). The merge rule's "main's tip must still be the combined check's base" applies.

Refusals: none.

This was referenced Oct 21, 2026
Merged
Open
@jayden1711
Merge origin/main (b3176d3) into docs-merge-rule
b36a952 
main brought #947 and #899. One conflict, in src/review.ts: the
HARNESS_CONFIG block from #899 (its JSDoc, HARNESS_CONFIG and the
never-shrink HARNESS_CONFIG_GLOBS) and this PR's decision-record block
(DECISION_FILES, DECISION_DIRS, DECISION_RECORD_GLOBS and onePartPerGlob)
were both added after hidesSubmodules. Both are kept whole: this PR's
block first, then the one from #899, then SELF_MODIFICATION as before.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Merged
@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Evaluator verdict: PASS (round-16 delta final check, head b36a9525)

Head checked: b36a95255183c88cb291cb1d81705aea0795a867, a merge of main b3176d3e (#947, #899) into 14cdf561, the previous head, whose two reviews both passed. This round is a main merge plus one body count fix. Probes are in scratch delta-920-r16/ (keep.mjs, count.mjs, levels.mjs, same.sh, mt*.sh). No repo file was edited, and every run used a temp HOME, state dir and TMPDIR.

1. The merge re-derives (measured)

2. Nothing lost (measured; keep.mjs reads the committed head, main and the previous head; registrationsOf is main's, since the PR changes merge-policy-weakening.ts only inside GUARD_FILES)

What main b3176d3e prev head 14cdf561 merged b36a9525 Lost
Lines over all tracked files (merge base 7b0a96a0; a line may go only where the other side removed it) 1458 files 0
SELF_MODIFICATION_PARTS entries 128 134 134 0 / 0
NOT_YET_FILES entries 7 20 20 0 / 0
GUARD_FILES (quoted + template) 77 78 78 0 / 0
LOCKED_CATEGORIES string entries 4 4 4 0 ([HARNESS_CONFIG]: HARNESS_CONFIG_GLOBS present, read)
HARNESS_CONFIG_GLOBS 3 n/a 3 0
#886 marks (registrationsOf, every JS/TS file) 78 77 78 (union 78, 0 extra) 0
#899's added lines in src/review.ts (git diff 40ef312a b3176d3e) 18 18 present 0

Marks still hold (measured, r15's future.mjs rerun on this head). With in-memory variants fed to weakeningReasons:

  • dropping the "main watch" row of SELF_MODIFICATION_PARTS gives "removes or changes an entry in a check's rules or a guard's list";
  • adding one entry to NOT_YET_FILES gives "adds or changes an entry in an allowlist, exception or good-fixture list".

3. #899's harness-config lock holds (measured)

  • Probe. levels.mjs runs computeLevel at the head for 3 configs: this repo's review.yaml; a hostile one with harness-config and engine-guards emptied and dropped from every level; and one with every category emptied and every level's when cleared. It runs in both scopes, engine and managed-app.
    • .worklane/tests.yaml, .worklane/review.yaml, .worklane/decisions.md, .worklane/decisions/x.md, CLAUDE.md and .claude/settings.json are L3 with harness-config in all 6 combinations.
    • docs/process.md, docs/design.md and src/plan-decisions.ts are L3 with self-modification in the engine scope under all 3 configs.
    • Expected-L3 misses: 0.
  • Tests. 14 named test files gave 147 tests, 147 pass, 0 fail: harness-config-lock, engine-guard-locks, plan-decisions, review, merge-policy plus the 7 merge-policy-* files, not-yet-files-locks and docs-guardrails. The run used the npmw.sh build and a temp HOME and state dir. I read each file's imports and spawns first: none sets its own PATH or calls a host tool.
  • Not run here. separate-users.test runs a real sudo -n, so I left it to CI. All 8 test shards passed at this head (below).

4. Body count fix (measured: own count.mjs, GUARD_FILES quoted + template entries plus marks)

Rev GUARD_FILES Marks Sum Body says
36eafd03 78 (73 + 5) 0 78 78
7b0a96a0 77 (72 + 5) 77 154 154
14cdf561 78 77 155 155
b3176d3e 77 78 155 155
b36a9525 78 78 156 156

The 78/154/155 correction in the round-15 section and the round-16 counts (155/155/156) are both right. The body was read through the raw API and has 0 raw control characters and no attribution line (measured).

5. Usual checks

Required changes

None.

Notes for the merge

@jayden1711

jayden1711 commented Oct 21, 2026

Copy link
Copy Markdown
Owner Author

Second adversarial review, round 16: PASS (head b36a9525)

Head checked: b36a95255183c88cb291cb1d81705aea0795a867 (measured: gh pr view --json headRefOid). I focused on what the merge with #899 changed. I wrote my attack list before reading the round-15 review. Probes ran on a detached worktree of the head, with a temp HOME, state dir, TMPDIR and git config. They used scratch repos and in-memory mutants, and edited no repo file.

I found no route to a level below L3, no shrink of either list without a hold (outside the known add-only class, #992), no lost lock line, mark or registration, and no new path into the decisions index. I have two wording nits and one point for the owner. None of them blocks.

1. What the merge changed (measured)

  • Files. git diff --stat 14cdf561 b36a9525 lists only files that main changed between the merge base 7b0a96a0 and b3176d3e. The only one that differs from main is src/review.ts. The merge changes no src/ file on the planner side: src/plan-decisions.ts, src/coordinator*, src/roles.ts and src/runner.ts are untouched.
  • Lines in src/review.ts. Every line of either parent is in the head, unless the other side deliberately removed it from the merge base. That leaves 0 lost from this PR's side and 0 from main's. No line in the head comes from neither parent.

2. Attack table

# Attack Result Label
B1 Level of every decision record path (DECISION_FILES, each DECISION_DIRS folder, a file in it and one in a subfolder), docs/process.md, docs/design.md, src/plan-decisions.ts, test/plan-decisions.test.ts, and #899's paths (.worklane/tests.yaml, review.yaml, any .worklane/*, .claude/settings.json, .claude/**, CLAUDE.md). Scopes: engine, managed-app and undefined. Each under this repo's review.yaml and under hostile ones: empty L1–L3 lists with L0 max_lines 100000; harness-config, self-modification, engine and engine-guards redefined to [] with docs: ['**']; a __proto__ category; and the same empty categories passed as extraCategories 0 rows below L3 where L3 is expected. In the engine scope and the undefined scope, every path is L3 and waits (whyHighRisk is non-empty). Records wait on self-modification, and the two .worklane/ records on harness-config too. In the managed-app scope, #899's paths and the two .worklane/ records stay L3 through harness-config. The engine-only paths stand aside there, as designed and disclosed measured (levels.mjs, computeLevel + whyHighRisk)
B2 A later PR shrinks HARNESS_CONFIG_GLOBS: it drops a glob (H1), drops the mark and shrinks in one change (H2), slices it in LOCKED_CATEGORIES (H3), or takes it out of ALWAYS_L3 (H4) All held: "can't read", "registration gone", "table entry removed" and "guard lines" measured (weakeningReasons on in-memory mutants of the head's src/review.ts)
B3 A later PR shrinks the records' lock: it drops a DECISION_FILES or DECISION_DIRS entry (H5, H6), cuts DECISION_RECORD_GLOBS to the files (H7), widens onePartPerGlob's skip line (H8), or slices the part's globs (H9) All held. src/review.ts is a GUARD_FILES entry, so any removed or changed line in it waits measured
B4 Combination of onePartPerGlob and HARNESS_CONFIG_GLOBS: the harness list is fed through onePartPerGlob after the self-modification parts (H11). Also: engineSourceGlob widened to the config folder, which would make harness-config stand aside in a managed app (H12). Also: computeLevel's never-fewer union loses its locked side (H10) All held measured
B5 Unbind a mark: an add-only line between // check-table: never-shrink and HARNESS_CONFIG_GLOBS (H15), or SELF_MODIFICATION_PARTS (H16) Both held ("removes or changes the mark") measured
B6 Add-only narrowing (#992 class, listed and not reported): a skip line added in onePartPerGlob (H13); HARNESS_CONFIG_GLOBS.length = 0 added outside the table (H14) H13 is not held, which is #992 as known. H14 is held ("table used outside its list") measured
B7 Did the merge lose a lock line, mark or registration? Each marked table in src/review.ts at either parent has all its lines in the head. Exceptions: ALWAYS_L3's old line (main changed it), and the SELF_MODIFICATION_PARTS open and close (the wrapper, disclosed). #886 marks over every tracked JS/TS file: 14cdf561 77, b3176d3e 78, head 78, 0 lost. GUARD_FILES quoted and template entries: main 77, 14cdf561 78, head 78, 0 lost, src/plan-decisions.ts and src/review.ts present. So there are 156 registrations, as the body says measured (keep.mjs, counts.mjs)
B8 Merge-resolution slip: did the restored /** comment out code, or detach a declaration from its mark? No. At runtime LOCKED_CATEGORIES has 3 keys and harness-config holds its 3 globs (B1). tableSpan finds HARNESS_CONFIG_GLOBS (one line, 355), SELF_MODIFICATION_PARTS (368–479), LOCKED_CATEGORIES (482–486) and ALWAYS_L3, each under its mark measured
B9 Can the index read docs/process.md or #899's harness config (.worklane/tests.yaml, .claude/**, CLAUDE.md) through a record path? I tried committed links and working-tree swaps. Committed links: .worklane/decisions.md to tests.yaml, the .worklane/decisions folder to ../.claude, docs/adr/0002-x.md to ../../CLAUDE.md, and docs/decisions.md to process.md. Swaps after the commit: the same file and folder, and docs/adr/0001-ok.md to ../process.md No text leaked in any case. Committed links aren't regular files, so they aren't indexed. A swap leaves out the record whose real path isn't a record path measured (index.mjs, scratch repos)
B10 Re-run of the round-13 and round-15 probes that still apply Same results as round 15. #956: a 781-character answer is indexed whole and reaches the brief. #969: a log-written answer with no role is indexed. #970: the committed text is indexed while the checkout differs. Console: owner and writer claims are accepted, and a bare request is refused. The planner brief doesn't call the task text data. Levels: CLAUDE.md and .claude/** are L3 (now with harness-config); CLAUDE.local.md and AGENTS.md are L0. All of these match the open routes as process.md words them measured
B11 Tests on the merged head plan-decisions, engine-guard-locks, harness-config-lock, review, merge-policy, not-yet-files-locks and docs-guardrails: 116 tests, 116 pass, 0 fail. The adversarial suite: 111 tests, 99 pass, 0 fail, 0 skipped (the other 12 are its todo entries). For lock loss masked by harness-config: engine-guard-locks asserts categories['self-modification'] equals the path for every record path under every config. So a .worklane/ record that lost its self-modification glob would fail there, even though it stays L3 measured (tests); inferred (masking, by reading test/engine-guard-locks.test.ts)
B12 Main has moved since b3176d3e origin/main is cec056a5 (#902). git merge-tree --write-tree against the head is clean (exit 0, no conflicts). git diff --stat b3176d3e origin/main is empty for src/review.ts, src/plan-decisions.ts, src/merge-policy-weakening.ts, docs/process.md, test/merge-policy.test.ts and test/engine-guard-locks.test.ts measured

3. Required changes

None.

Nits (optional, wording only):

  1. The body's "LOCKED_CATEGORIES and HARNESS_CONFIG_GLOBS … 4 and 3 entries". LOCKED_CATEGORIES has 3 entries at main and in the head (5 table lines), and 2 at 14cdf561. "0 lost" stands (measured).
  2. changelog.d/920-lock-merge-process-doc.md's "A managed app's decision records keep the categories its own review.yaml gives them". Since Sandbox: the agent's home is read-only to a project command, so it can't plant config for a later agent-user process; harness-config review locked (#798) [PROTECTED: owner merges] #899, a managed app's .worklane/decisions.md and .worklane/decisions/** are always L3 harness-config, whatever its review.yaml says (B1, measured). This understates the protection. It doesn't overstate it, and the body's round-16 row already says so. A half-sentence would make it exact.

For the owner (information, a scope choice, not a defect of this PR): process.md's critical set names self-modification, engine-guards and release-signing from computeLevel. It doesn't name #899's new code lock harness-config. So in this repo, a PR that touches only .worklane/tests.yaml, .worklane/deploy.yaml, .claude/** or CLAUDE.md is L3 by code but gets one review, not two (measured for CLAUDE.md and .claude/**: "critical by code: none"). process.md:12 already discloses the project-instructions part of this (#973). The merge didn't open it, and main had no written critical set. Adding harness-config to the set would cost more reviews. That makes it a scope decision for the owner.

Refusals: none.

This was referenced Oct 21, 2026
Closed
Merged
@jayden1711
jayden1711 merged commit 1808043 into main Oct 21, 2026
27 of 28 checks passed
@jayden1711
jayden1711 deleted the docs-merge-rule branch October 21, 2026 17:44
@jayden1711 jayden1711 mentioned this pull request Oct 21, 2026
Closed
jayden1711 added a commit that referenced this pull request Oct 21, 2026
@jayden1711
Merge origin/main (1808043: #920) into issue-807-git-runner
5d8c98a 
src/review.ts: the engine-guards lock list takes the union, test/git-runner.test.ts and #920's
test/plan-decisions.test.ts, each with its comment.

src/plan-decisions.ts (#920, locked as the planners' decisions index): its two raw spawnSync('git', ...) calls, the
object read (cat-file --batch) and committedRecords' rev-parse helper, run through gitSpawnSync, which the runner scan
requires. Their own --no-replace-objects, the object-id checks and the hash re-check stay as they were; the runner
adds grafts and the commit-graph off. No OUTSIDE_THE_RUNNER entry, and no plan-decisions test changed. Measured on
this merge before the routing: git-runner's "every git call ... goes through the runner" fails with
"src/plan-decisions.ts: starts git 2 time(s) (not on the list)".

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
jayden1711 added a commit that referenced this pull request Oct 21, 2026
@jayden1711
Merge origin/main (1808043) into issue-890-sandbox-hidden
dbe83bb 
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>

#920 merged on main; no conflicts.
jayden1711 added a commit that referenced this pull request Oct 21, 2026
@jayden1711
Merge origin/main 1808043 into issue-784
8109b64 
One conflict, src/review.ts SELF_MODIFICATION_PARTS. It keeps #920's `onePartPerGlob([` and this branch's merge-policy
row, which includes src/vacuity.ts and src/vacuity-read.ts. On the merged tree, measured: dependencies reinstalled,
typecheck and build clean, 99/99 on review, merge-policy, engine-guard-locks, not-yet-files-locks and
merge-policy-vacuity.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
jayden1711 added a commit that referenced this pull request Oct 21, 2026
@jayden1711
Merge origin/main (1cee9bd: #920 and #921) into issue-744-remove-desktop
89b007c 
No conflicts (git merge-tree --write-tree origin/main HEAD exit 0).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
jayden1711 added a commit that referenced this pull request Oct 21, 2026
@jayden1711
test: hold every module the desktop backstop runs, read its imports w…
7b42005 
…ith TypeScript, and pin yaml

Both reviews of 87cf216 found that round 18's claim, that everything the
backstop runs is held, was false.

- Four src/ modules run in its process through test/isolate.ts and the
  weakening module: src/brand.ts, src/os/system-paths.ts,
  src/os/own-env.ts and src/os/child-process.ts. A changed line in any of
  them got no hold. One such line, patching what fs returns, turned the
  backstop green over a red desktop twin. They are now in GUARD_FILES,
  which is an addition only. A new test walks the built import closure
  of test/isolate.ts and the backstop with TypeScript's import reader. It
  fails on any local module whose changed line isn't held, and on a
  closure module that loads another by name at run time.
- yaml could be swapped through the lockfile, or shadowed by a
  dist/node_modules/yaml that Node finds first. The backstop now pins
  each package its closure loads (yaml, and typescript, its new import
  reader) to the lockfile's integrity, held as a line of this lint test.
  It also checks that the package resolves under the repo's own
  node_modules (a plain directory, not a link), that no node_modules sits
  between a built module and the repo top, and that the package isn't
  the repo itself.
- The import-list test read only column-0 imports with a space after
  `import`. It now reads imports with ts.preProcessFile. typescript is
  added to MAY_IMPORT with it. The test also fails on the text that
  loads a built-in by name. IMPORT_FORM_FIXTURES holds the forms the
  regexes missed: indented, a tab, a comment first, no space (side effect
  and named), export* with no space, and a comment inside import().

Also merges origin/main 1cee9bd (#920, #921), with no conflicts.

Test (fails without): test/desktop-removed.test.ts. A new file in a
worktree at 87cf216 turns each fix red there: the 8 import forms pass
that head's scan; the 4 modules get [] from its hold; its closure walk
finds them unheld; and with a dist/node_modules/yaml shadow and a red
twin added as new files, its backstop passes all its tests. At this
head the same shadow turns the pin test red.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711 jayden1711 mentioned this pull request Oct 24, 2026
Merged
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

needs-owner

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jayden1711