Skip to content

review: lock the engine's deployment manifests (deploy/**) as self-modification, before the first one exists [PROTECTED: owner merges] - #853

Merged
jayden1711 merged 2 commits into
mainfrom
protect-deploy
Oct 20, 2026
Merged

jayden1711 merged 2 commits into
mainfrom
protect-deploy

Conversation

@jayden1711

@jayden1711 jayden1711 commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Intent: Lock the engine's deployment manifests (deploy/**) as self-modification now, before any exist, so that no deploy/** PR (#703's compose, chart and module) can merge below the owner. On main, the first deploy/ PR already waits, because it adds a new top-level module: "design: new top-level module (deploy/)", L1, auto=false, even under a loosened review.yaml (measured by the reviewer: #853 (comment)). What this PR closes is every later deploy/ PR. On main, once deploy/ exists, such a PR is L1 with auto=true under a review.yaml without deploy-config (measured, below). This PR also adds the missing code-locked L3. Deployment manifests set the engine's isolation and exposure: the user it runs as, what it mounts, which ports it publishes. deploy/** joins "the machine setup" part in src/review.ts SELF_MODIFICATION_PARTS (always L3, in the merge policy's floor), and the engine's own .worklane/review.yaml lists it under engine.

Risk class: protected. It changes src/review.ts (engine, and itself locked as "the review levels"), .worklane/review.yaml (engine, locked) and test/merge-policy.test.ts (a locked test of the harness controls).

Owner task, 2026-10-19 (no issue). Related: #703 (its acceptance criterion "New deploy files are protected from the first PR", done here ahead of it).

What changed

  • src/review.ts: deploy/** in "the machine setup", with a comment saying why. deploy/ added to ENGINE_TREE, so this lock, like every lock on src/, scripts/, test/ and docs/, stands aside only where root names a repo as a managed app (Self-modification locks: cover the tool guard's whole import tree, and apply only in the engine's own repo #665). An app's own deploy/ stays its deploy-config, exactly as on main.
  • .worklane/review.yaml: deploy/** in engine, so an engine that predates the code lock (it reads review.yaml from main) holds it too.
  • test/merge-policy.test.ts: the new regression test (below). deploy/** added to NOT_YET_FILES (see "Exception added to a check").
  • test/coordinator.test.ts: the end-to-end self-modification test now gives a locked glob that has no file yet a file it matches, and asserts that the file matches. Before, it crashed on deploy/**.
  • docs/separate-users.md: the managed-apps row now names deploy/ among the engine-source locks that stand aside.
  • CHANGELOG.md: one bullet under ### Guardrail changes: what each one does in ## v0.5.3. When the branch was cut, main still read ## v0.5.3 (2026-10-19) (measured: grep -n '^## ' CHANGELOG.md at 3e193c7), so the bullet went in that section, per the release rule. Main has since become ## v0.5.3 (unreleased) (measured at 2ce7726), and this branch merges with it cleanly (measured: git merge-tree --write-tree HEAD origin/main), so the bullet lands in the unreleased section. It sits above the first generated heading, so the release keeps it once the section is undated (test/changelog.test.ts passes, measured).

How it was verified

  • Gap re-measured on current main (3e193c7) (measured: node measure.mjs, a scratch script calling the built computeLevel, mergeDecision, selfModificationPart on deploy/compose.yaml):
    • SELF_MODIFICATION_PARTS (src/review.ts:325 onward) has no deploy/ glob, and .worklane/review.yaml:7 engine has no deploy/**. selfModificationPart('deploy/compose.yaml') = null.
    • With the engine's own review.yaml: L3, categories deploy-config, app-non-money, auto-merge false. It was held, but only by the built-in deploy-config category that review.yaml lists at L3, which a review.yaml can drop.
    • With a review.yaml that drops deploy-config and engine and lists nothing at L3 or waiting: L1, auto-merge true. My script passed no new top-level module, which is the case for every deploy/ PR after the first. The first one also gets the design wait for a new top-level module and does not auto-merge on main (measured by the reviewer, comment linked above).
  • After (measured, same script on this branch): own review.yaml: L3, deploy-config, engine, self-modification, auto false. The loosened review.yaml: L3, self-modification, auto false. Part: the machine setup. Managed-app scope: null (unchanged).
  • Fail-without (measured): the test was written first and run against unchanged src/review.ts and review.yaml: node --import ./dist/test/isolate.js --test --test-name-pattern "deploy/compose.yaml|self-modification: every glob" dist/test/merge-policy.test.js gave 1 pass, 1 fail: this repo / + undefined - ['deploy/compose.yaml'] (no self-modification category). With the change: pass.
  • The new test, modelled on the .github/release-signers one, for this repo's review.yaml, the example's, and one that drops deploy-config, engine, every L3 and every wait: L3, self-modification: [deploy/compose.yaml], mergeDecision.auto === false, the reason high-risk: self-modification (deploy/compose.yaml), the why changes the machine setup: deploy/compose.yaml, and the floor with no wait categories passed at all. Then: this repo's engine category holds it; and in managed-app scope there is no engine lock and its deploy-config keeps it at L3.
  • test/merge-policy.test.ts, test/review.test.ts, test/changelog.test.ts, test/brand.test.ts: 54/54 pass (measured).
  • npm run typecheck: clean (measured). npm run check:denylist: 1319 files clean (measured).
  • Full suite (fullsuite.sh, excludes mcp-* and plugin-fetch) at the first commit (measured): 3913 tests, 3855 pass, 57 skipped, 1 fail. The failure was test/coordinator.test.ts "self-modification: a harness PR touching only one of the harness's own controls ...". That test walks every locked glob through a real PR and treated a glob with no file as an exact path, so deploy/** got no path (Cannot read properties of undefined (reading 'startsWith')). The second commit gives such a glob a file it matches (deploy/new-file) and asserts the match, so deploy/** now goes through the coordinator flow end to end. That test passes (measured: run alone by name, 1/1). I did not rerun the full suite after that one-test fix; CI runs it.
  • Overlap with Tool guard: its lanes' rules move to src/guard/, so it loads 57 modules, not 83, and only those are locked as the guard (#785) [PROTECTED: owner merges] #839 (measured): git diff origin/main...FETCH_HEAD -- src/review.ts (issue-785) has hunks at @@ -334,8 and @@ -345,10. This change is at line 333 and in ENGINE_TREE (~393), outside both. git merge-tree --write-tree HEAD FETCH_HEAD merges clean (tree ee6caff6).

Exception added to a check (decision guide 9)

Adversarial pass

Each attack run through the built computeLevel / selfModificationPart with the engine's own review.yaml (measured: attack.mjs in the scratchpad), unless labelled.

  1. review.yaml drops it (no deploy-config, no engine, empty L3 and waits): still L3 and waits (measured, the new test).
  2. Caller passes no wait categories: the floor holds (measured, the new test).
  3. Nested paths: deploy/helm/templates/deployment.yaml and deploy/terraform/main.tf: L3, self-modification, "the machine setup" (measured).
  4. Rename out of or into deploy/: counts under both paths since Review levels: a renamed file counts under its old and its new path, in changeFiles and every path reader [PROTECTED: owner merges] #688 (inferred, not run here).
  5. Editing the exemption away: test/merge-policy.test.ts is itself locked ("the tests of the harness controls") (inferred from SELF_MODIFICATION_PARTS).
  6. Managed app (rule 3, nothing main closed reopens): on main a managed app's deploy/compose.yaml had no engine lock (selfModificationPart(…, 'managed-app') = null, measured on 3e193c7), and it still has none. Nothing closed is reopened (measured). The existing "globs that hold in every repo" test is unchanged and passes (measured).
  7. Gaps found, not fixed here (outside the brief), proposed separately:

What this can never do

  • It can't lower any path's level or let anything merge that waited before. It only adds a glob to a lock and to engine, and deploy/ to ENGINE_TREE (measured: the before/after above; git diff origin/main -- src/review.ts .worklane/review.yaml changes three lines, each adding one entry to a list, and removes no glob or category).
  • It can't lock a managed app's own deploy/ as the engine's: deploy/ is in ENGINE_TREE (measured: managed-app part null).
  • It changes no runtime behaviour beyond classification: no setup script, sudoers line, unit or permission (measured: 6 files changed, listed above).

Compromised component

  • Agent user / worker run: could open a PR touching deploy/**. It is now L3 and in the floor, so it waits for the owner whatever review.yaml says (measured). It could still put a manifest outside deploy/ (Adversarial pass 7).
  • Coordinator: computes the level with this code. A compromised coordinator could ignore any lock, as before. This PR doesn't change that.
  • Dashboard / hub viewer: no route touches review levels. No change (inferred).
  • Other instances: none. Each instance reads review.yaml from its repo's main (inferred).
  • Root naming a managed app: can make the engine-source locks, now including deploy/, stand aside for that app's repo, never the engine's (src/managed-apps.ts refuses the engine's repo). Same loosening as review: lock the tool guard's whole import tree; engine-source locks stand aside only where root names a managed app [PROTECTED: owner merges] #691, now covering deploy/ too (inferred from lockedCategories).
  • Someone with push access to main: could edit src/review.ts or review.yaml directly. That's the ruleset's job, unchanged.

Package size

Measured with npm pack --dry-run --json (through the npm wrapper), main at 3e193c7 against this branch, each built in its own worktree:

  • Tarball: 1,403,800 → 1,403,950 bytes (+150).
  • Unpacked: 4,712,762 → 4,713,148 (+386).
  • Per file: dist/src/review.js 35,197 → 35,583 (+386, the new glob and its comment). Nothing else in the package changed.

New dependencies

None.

…dification, before the first one exists

Deployment manifests (a compose file, a chart, a module) set the engine's isolation and exposure: the user it runs
as, what it mounts, which ports it publishes. Until now only review.yaml's deploy-config category held deploy/**, so
a review.yaml without it left a deploy/compose.yaml change at L1, free to auto-merge (measured on main at 3e193c7).

deploy/** joins "the machine setup" part in SELF_MODIFICATION_PARTS (always L3, in the merge policy's floor), and
the engine's own review.yaml lists it under engine, for an engine that predates the lock (it reads the file from
main). deploy/ joins ENGINE_TREE, so like src/ and scripts/ the lock stands aside where root names a managed app:
an app's own deploy/ stays its deploy-config, as before. No file exists under deploy/ yet, so the dead-glob check
takes deploy/** in NOT_YET_FILES; that check now matches the entry as a glob, so the first deploy file fails it
until the entry is taken out.

Test (fails without): test/merge-policy.test.ts, "a PR touching only deploy/compose.yaml is L3 ...". Against the
unchanged src/review.ts and review.yaml it fails at "this repo" (no self-modification category); 1 of 2 selected
tests pass.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…es, so deploy/** runs through the flow

The coordinator's self-modification test walks every locked glob through a real PR and took a glob with no file
as an exact path. deploy/** has no file yet, so it got no path and the test crashed ("Cannot read properties of
undefined (reading 'startsWith')", measured with fullsuite.sh). It now writes a file the glob matches
(deploy/new-file), and asserts that it does, so deploy/** is checked end to end: the PR waits for the owner and
isn't merged. An exact path (experiments.yaml) is unchanged.

Test (fails without): test/coordinator.test.ts, "self-modification: a harness PR touching only one of the
harness's own controls ...": failed in the full suite at the previous commit (3855 of 3856 run pass), passes here.

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

Copy link
Copy Markdown
Owner Author

Evaluator verdict: PASS (with notes)

Reviewed head ba29ae05 (base 3e193c7, main now 2ce7726). Independent review of a loosening under decision-guide principle 9 (the NOT_YET_FILES entry) included.

Acceptance (owner task, no issue; #703's "new deploy files are protected from the first PR")

Criterion Result Evidence
deploy/** is L3 self-modification and never auto-merges, whatever review.yaml says met measured: built computeLevel + mergeDecision on head, for this repo's review.yaml, the example's, one that drops deploy-config/engine/L3/waits, and one that empties every category: deploy/compose.yaml and deploy/helm/templates/deployment.yaml are L3, self-modification, part "the machine setup", auto=false, and auto=false with no wait categories passed (the floor). On main the last two configs gave L1, auto=true.
A managed app's own deploy/ keeps its deploy-config behaviour (#665 not regressed) met measured: in managed-app scope, rows for the example, loosened and every-category-dropped configs are identical on main and head (deploy-config L3 under the example, L1 when the app's own review.yaml drops it, part null). test/review.test.ts "globs that hold in every repo" (exact list) passes on head.
The engine's own review.yaml lists it under engine met measured: engine category = [deploy/compose.yaml] on head.
Nothing main closed reopens met measured: 72 rows (9 paths x 4 configs x 2 scopes) compared main vs head; 10 differ, every one tighter (added self-modification/engine, L1 to L3, auto/floor true to false).

Fail-without (method b)

measured: fresh worktree at main 2ce7726, the PR's test/merge-policy.test.ts and test/coordinator.test.ts written in (byte-identical to head, git diff --no-index empty), npx tsc, then node --import ./dist/test/isolate.js --test dist/test/merge-policy.test.js: 23 tests, 22 pass, 1 fail: "regression: a PR touching only deploy/compose.yaml ..." at this repo, + undefined - ['deploy/compose.yaml'] (no self-modification category: the bug). On head: merge-policy, review, changelog, brand: 54/54 pass; the coordinator self-modification test alone: 1/1 pass (136 s).

The exception (principle 9)

  • measured: it is narrow. NOT_YET_FILES gains exactly deploy/**; it is consulted only by exact includes(glob), and the second use (line 170) compares exact paths, where deploy/** never appears.
  • measured: it expires itself. In a scratch worktree at head with one staged deploy/compose.yaml (never committed), the dead-glob test fails: deploy/** matches a file now: take it out of NOT_YET_FILES ... (2 run, 1 fail).
  • inferred: for experiments.yaml the new check equals the old one (globToRegExp escapes . and anchors ^...$).

coordinator.test.ts

measured from the diff: -2 lines (a comment, the path line), +4 (comment, path line, a new assert.ok(globToRegExp(glob).test(path))). No assertion removed or weakened; one added.

Protection

Protected paths touched: src/review.ts, .worklane/review.yaml, test/merge-policy.test.ts. Marked protected: title, **Risk class:** protected, needs-owner label, "What this can never do", "Compromised component", "Adversarial pass", "Package size" all present (measured). "Can never do" claims check out against the 72-row comparison (measured). The protection is by design (runtime classification and the merge floor), not a lint (inferred from code).

Merges

measured: git merge-tree --write-tree origin/main HEAD clean (667aea63); with #839 head 63f1cc8 clean (1b789696), and the merged src/review.ts keeps deploy/** and deploy/ in ENGINE_TREE. #839's hunks touch other parts only (inferred: no semantic overlap). The CHANGELOG bullet lands under ## v0.5.3 (unreleased) / ### Guardrail changes: what each one does in the merged tree (measured).

Builder's named gaps (all pre-existing, identical on main and head, none reopened)

  • top-level deploy path (file or symlink): L1, part null, auto=true under every config (measured). A path without / is not a new top-level module (inferred, merge-flow.ts:509).
  • root compose.yaml, docker-compose.yml: L1, auto=true (measured).
  • Dockerfile: L3 only via deploy-config (own and example), floor auto=true; L1 auto=true when review.yaml drops it (measured).
  • also Deploy/compose.yaml and nested apps/x/deploy/compose.yaml: L1 (measured).

Notes (non-blocking)

  1. inferred: the coordinator test's new fallback applies to every wildcard glob with no file, not only deploy/**; before, such a glob crashed the test by accident. The explicit dead-glob check in test/merge-policy.test.ts still fails any such glob, so no coverage is lost.
  2. measured: on main the first deploy/ PR already waited, as a new top-level module (design: new top-level module (deploy/), L1), even under a loosened review.yaml. The gap this closes is every later deploy/ PR (L1, auto=true on main) and the missing L3. The body's "the first deploy/** PR ... can never merge below the owner" is true but was not open on main for that first PR.
  3. measured: fullsuite.sh on head: 3913 tests, 3855 pass, 57 skipped, 1 fail: dashboard-speed "event cache: every view read incrementally ..." (read ECONNRESET). It fails the same way alone on main 2ce7726, so it is not this PR.
  4. Package size: measured npm pack --dry-run --json main vs head: tarball +150 B, unpacked +386 B, only dist/src/review.js 35,197 to 35,583. Matches the body.
  5. Commits: both signed off, no attribution trailers, no new dependencies, npm run typecheck clean, check:denylist 1319 files clean (measured).

CI: shard 4/4 (node 24), scope, dco, scan, desktop, installed-engine-config, new-deps passed; the other test shards, push-gate, denylist and agentshield were pending when I looked (measured, gh pr checks 853).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant