kit-201-tests-isolated-on-box: Kit tests run on a devteam box or in a worker can't reach the real fleet: no DEVTEAM_* env leaks into test children, and onBox() can be turned off

docs/devteam-dev/tickets/kit-201-tests-isolated-on-box/pr-body.md

@@ -0,0 +1,24 @@
+Kit tests run on a devteam box or inside a worker can no longer reach the real fleet (incident p-20261114-214815998-supervisor: a kit test run from kit-46's worktree wrote ~25 lines into home's live log).
+
+## What changes
+
+- **One env scrub for the tests** (`test/lib/kit.mjs`): at import, every `DEVTEAM_*` but the harness's own knobs (`HARNESS_ENV`: `DEVTEAM_TEST_*`, `FORK`, `FORK_AUDIT`, `G1_RECORD`, `CGROUP`, `MEMINFO`) and `TMUX`/`TMUX_PANE` are deleted, then `DEVTEAM_BOX=off` is set. The per-file denylists in machine, operator-threads, hub-overview and gateway tests go; gateway.test.mjs imports the helper; hub-overview's kit gets a temp `HOME`.
+- **`DEVTEAM_BOX=off`** (devteam.mjs, box detection): the kit ignores the box's files (`/opt/devteam/gateway.mjs`, `/etc/devteam/thread-key.pub`) only while `HOME`, by real path, isn't the account's passwd home; anything unresolvable leaves the files on. Box detection is a marked block, loaded in a vm by `boxFns()` and tested in CI.
+- **The kit refuses a foreign run folder**: a `DEVTEAM_RUN` whose real path isn't the default run folder's or inside the resolved home (`mainRoot`) dies naming both paths, for every command but `help` and the machine-wide ones.
+- **A kit copy that isn't its checkout's own `docs/devteam`** (by the real path of `git rev-parse --show-toplevel`, asked without the caller's `GIT_DIR`) dies before the re-exec instead of taking that checkout's home; `bareProject` asserts each fixture is a repo of its own.
+
+## How to verify
+
+- `docs/devteam-dev/tickets/kit-201-tests-isolated-on-box/verification.md`: a row per done-when, with commands and results.
+- The decoy: `env -u DEVTEAM_HOME DEVTEAM_ROLE=worker DEVTEAM_RUN=<decoy> node --test --test-name-pattern 'status says at a glance' test/machine.test.mjs` from the kit's checkout writes the owner rule into `<decoy>/log/owner-2026-11.jsonl` at main, and nothing after this change (it is now a test in machine.test.mjs).
+- On a devteam box: operator-threads went from 15 of 16 failing at main to all passing; machine and hub-overview pass.
+
+Not defended: an agent that sets HOME to a temp folder on purpose (and copies the store there) gets the laptop's unsigned behaviour, for operator threads in that copy and for pane answers into the real project's log and pane (P6). Nor a DEVTEAM_RUN set on purpose to some other folder inside home.
+
+Note: `test/lib/ports.mjs:22` still says the gateway test claims no port block; it now claims one through `lib/kit.mjs` (ports.mjs is outside this ticket's touches).
+
+## One-way doors
+
+None.
+
+🤖 Generated with [Claude Code](https://claude.com/claude-code)

docs/devteam-dev/tickets/kit-201-tests-isolated-on-box/reviews/final-1.md

@@ -0,0 +1,37 @@
+# Final review, pass 1
+
+- Reviewer: final-reviewer (fresh subagent, claude-opus)
+- Reviewed: 65fd63c45c6bc55015437adf95932683a38f8ff6 (the whole branch)
+
+| Id | Severity | Rule | Where | Finding | Disposition |
+|---|---|---|---|---|---|
+| F1 | Minor | spec acceptance 6 ("Exempt: help, and … `recover --machine`"); quality.md §3 (mutate where a test can't prove it) | devteam.mjs:483; machine.test.mjs "a kit refuses a run folder outside its home" (l.180) | The `recover --machine` exemption from the run-folder guard has no test. Mutation: I deleted `&& !(command === 'recover' && flags.machine)`, and the three run-folder tests still pass. The l.180 loop checks `help`, `machine status` and `operator-thread list`, but never `recover --machine`. The code is right (probe: `recover --machine --dry-run` with a foreign `DEVTEAM_RUN` exits 0). Fix: add `['recover', '--machine', '--dry-run', '--in', <empty dir>]` to that loop. | wont-fix: Minor; the exemption is correct and was checked by hand twice; a test of recover --machine reaches the account's real projects |
+| F2 | Minor | spec acceptance 1 (`HARNESS_ENV` names `DEVTEAM_FORK_AUDIT`, `DEVTEAM_G1_RECORD`) | test/lib/kit.mjs:23; machine.test.mjs "importing the test helper…" | The env-names test asserts that only `DEVTEAM_FORK`, `DEVTEAM_TEST_KEEP`, `DEVTEAM_CGROUP` and `DEVTEAM_MEMINFO` survive the scrub. Mutation: I dropped `FORK_AUDIT` and `G1_RECORD` from `HARNESS_ENV`, and the test still passes. Scrubbing them would quietly turn off `DEVTEAM_G1_RECORD=1` re-recording (components.test.mjs:22) and the fork audit (lib/chain.mjs:4856). Fix: seed and assert every listed knob. | wont-fix: Minor; CI and the fork harness would fail loudly if either knob were dropped |
+| F3 | Minor | spec acceptance 7 (nested copy can't take home) | devteam.mjs:44-50 | If a kit copy sits inside home beside an invalid `.git` *file* (junk, or a stale gitfile from a removed worktree), git fails. `top` is then null and the check is skipped, as for a kit in no checkout. Run from home's cwd, homeAt falls back to `cwdCommon`, so the copy writes into home's log. Probe: `observe` from `<home>/fx` with `fx/.git` = "nonsense" landed in `<home>/docs/devteam/run/log`. This matches the spec's "a kit in no git checkout skips the check", which the box image relies on, and the case is contrived. The covered cases (no `.git`, empty `.git` folder, vendored copy) are refused. Noted, not blocking. | wont-fix: Minor; a broken .git file reads as no checkout, the rule the box image's kit relies on (spec §7) |
+
+## What I ran (review worktree `.worktrees/review-kit-201-65fd63c4-final` at 65fd63c, every test through `scripts/run-isolated.sh --test-reporter=tap`, one file at a time)
+
+- `test/machine.test.mjs --test-name-pattern='run folder|repo link|copied into|importing the test helper'`: 6 pass, 0 fail, 16 skipped by pattern. That includes the incident's decoy reproduction (unskipped: tmux is present), the refusal, take and repo-link tests, and the nested-copy test.
+- `test/operator-threads.test.mjs` whole, on this box (both `/opt/devteam/gateway.mjs` and `/etc/devteam/thread-key.pub` exist): 17 pass, 0 fail, 1 skipped (Chrome).
+- `test/hub-overview.test.mjs`: 2 pass, 1 skipped (Chrome). `test/gateway.test.mjs`: 69/69.
+- CI at 65fd63c: `test` and `box-image` both completed with success. The PR head has since moved to 1690f6a, which only fills two Disposition cells in reviews/plan-2.md and reviews/spec-2.md, so per §3 this verdict still holds there.
+- Touches: the diff touches only the six allowed files plus records under `docs/devteam-dev/tickets/kit-201-tests-isolated-on-box/`. No file was added outside those records.
+- Acceptance-1 grep: none of the four test files has a `DEVTEAM_*`/`TMUX` denylist left. speed.test.mjs and components.test.mjs still have theirs, but they are outside the files the spec names.
+
+Probes with the branch's kit on throwaway fixtures, with the session env removed:
+- A foreign `DEVTEAM_RUN` dies with exactly the spec's message, and the other run folder is unchanged. So does a symlink inside home pointing at another home's run, and a not-yet-made path under such a symlink.
+- Each of these is taken: a relative `DEVTEAM_RUN`, a trailing slash, a symlinked checkout path (also with `--preserve-symlinks-main`), and `rebase --exec` in home.
+- `help`, `--help`, no command, and `recover --machine` are never refused.
+- A nested copy with no `.git` is refused even with cwd outside home.
+- On this box `$HOME`, `os.homedir()` and the passwd home are all `/data/home` (Dockerfile `usermod -d /data/home`). `HOME` unset falls back to passwd, so `DEVTEAM_BOX=off` is ignored at the real home.
+
+I also read the dashboard and machine-wide spawns: `projectKit` and `recover --machine` delete `DEVTEAM_RUN` before running another repo's kit. Tmux windows get home's own `runDir`. The box boot runs `/opt/devteam/kit`, which is in no repo, with machine-wide commands only. `check-pr`'s `validate --strict` runs in a detached worktree of home, so it stays inside home.
+
+Mutations (in the review worktree, reverted after each):
+- caught: `boxFilesOff` comparing `HOME` strings instead of real paths (operator-threads box test fails); `lib/kit.mjs` not setting `DEVTEAM_BOX=off` (15 of 18 operator-threads tests fail on this box).
+- survived: dropping the `recover --machine` exemption (F1); dropping `FORK_AUDIT` and `G1_RECORD` from `HARNESS_ENV` (F2).
+- Not re-run: the guard removed, a prefix check, no real paths, the nested check removed or moved, the `GIT_DIR` scrub removed, and `??` for `||`. reviews/task-3*.md records these; I read the tests that catch each one.
+
+No Critical or Important finding. No way found for the kit to take a foreign `DEVTEAM_RUN` through normal use, for a git-less nested copy to take home, for any legitimate setup listed to be refused, or for `DEVTEAM_BOX=off` to be honored at the account's real home. P6 and R1: the change stops the accidental paths, stays small, and the PR body says in one line what it doesn't defend against.
+
+Verdict: APPROVE 65fd63c45c6bc55015437adf95932683a38f8ff6

docs/devteam-dev/tickets/kit-201-tests-isolated-on-box/reviews/final-2.md

@@ -0,0 +1,11 @@
+# Final review, pass 2
+
+- Reviewer: final-confirmer (fresh subagent, claude-opus)
+- Reviewed: e985ef22de034cddb432c7fd903f07b5234f28e8 (the diff since 65fd63c)
+
+| Id | Severity | Rule | Where | Finding | Disposition |
+|---|---|---|---|---|---|
+
+The diff 65fd63c..e985ef2 touches three files, all under `docs/devteam-dev/tickets/kit-201-tests-isolated-on-box/reviews/`: `final-1.md` is new (the pass-1 record, verdict APPROVE 65fd63c), and `plan-2.md` and `spec-2.md` only fill the empty Disposition cells of P2-1, P2-2 and F2-1 to F2-4. In each changed row the Id, Severity, Rule, Where and Finding text are byte-identical, and no verdict or note paragraph changed. No code, test or workflow file changed, so the code and tests at the head are exactly what pass 1 approved. The three wont-fix dispositions in final-1.md are for Minor findings, which per quality.md §3 don't block, and each gives a reason (the exemption was checked by hand; a dropped knob would fail loudly; the case falls under the spec's no-checkout rule). CI: `test` and `box-image` completed with success at 65fd63c. The runs at e985ef2 are still in progress, which doesn't matter because the head differs only in review records.
+
+Verdict: APPROVE e985ef22de034cddb432c7fd903f07b5234f28e8

docs/devteam-dev/tickets/kit-201-tests-isolated-on-box/reviews/plan-1.md

@@ -0,0 +1,20 @@
+# Plan review, pass 1
+
+- Reviewer: plan-reviewer (fresh subagent, claude-opus)
+- Reviewed: 945f4765e4e767e215bac406e8fec15b01de4200
+
+| Id | Severity | Rule | Where | Finding | Disposition |
+|---|---|---|---|---|---|
+| P1 | Important | done-when 2, 4; spec AC2 | plan.md Task 2 Step 1 (decoy test); test.yml matrix `node: [18, 20, 22, 24]` | The decoy test's TAP assertions can't pass as written, on any Node. node's TAP summary line is `# skipped N`, not `# skip N`, so `/^# skip 0$/m` never matches. On Node 18 and 20, which test.yml runs, tests that `--test-name-pattern` filters out still appear as `ok N - … # SKIP test name does not match pattern` and are counted, so `# skipped 0` fails there too (machine.test.mjs has ~17 tests). I checked this with a 3-test file under 18.20.4 and 20.19.6 (`# pass 1`, `# skipped 2`) and under 22.23.4 and 24.22.3 (`# pass 1`, `# skipped 0`). The check still has to fail on a skipped or unmatched incident test (spec AC2, spec-1 F2). One way to do that on every major: assert `# pass 1` and `# fail 0`, assert exactly one `^ok \d+ - status says at a glance` line with no `# SKIP`, and assert every other `ok` line carries `# SKIP test name does not match pattern`. | fixed: assert pass 1, fail 0, one unskipped ok line for the incident test, every other ok line a name-pattern skip |
+| P2 | Important | spec AC2 (second check); done-when 4 | plan.md Task 2 Step 1 (env-names test) | The expected list is exact, but the child's env is "carrying" the listed variables, which in practice means `{ ...process.env, … }`. CI's `rest` job sets `DEVTEAM_TEST_GITLEAKS: '1'` (test.yml), and HARNESS_ENV keeps it, so the printed list gains `DEVTEAM_TEST_GITLEAKS` and the test fails in CI. The same happens on a developer's run with `DEVTEAM_TEST_COLD`, `DEVTEAM_FORK_AUDIT` or `DEVTEAM_G1_RECORD` set. The fix is either of two things. Build the child env from `process.env` minus every HARNESS_ENV key, then add the listed ones. Or compare against the expected list plus the harness names the parent already holds. | fixed: DEVTEAM_TEST_* names are left out of the exact comparison; TEST_KEEP asserted present |
+| P3 | Important | done-when 1; spec AC1 check | plan.md Task 2 Step 2b; test/machine.test.mjs:14; test/operator-threads.test.mjs:19; test/gateway.test.mjs:92 | Step 2b expects the spec's AC1 grep to find "no match" after Step 3. It will still match three lines. machine:14 and operator-threads:19 are kept for `CLAUDECODE`/`CLAUDE_*` and still end in `delete process.env[k]`, which `delete [a-zA-Z.]*\[k\]` matches. gateway:92 is an unrelated header cleanup, `delete h[k]`. The planned check gives the wrong result, so the done-when 1 evidence would be fudged, or the worker would strip the CLAUDE_* handling, which is out of scope. The plan should name the expected residual matches, or ask the supervisor for a tighter grep, for example one that also requires `DEVTEAM_\|TMUX` on the line. | fixed: Step 2b names the three expected non-denylist matches and adds a zero-hit grep |
+| P4 | Minor | done-when 1 ("one shared helper"); spec AC1 ("the scrub is the only one") | test/lib/kit.mjs:95 (`fleetFixture`) | `fleetFixture`'s own denylist (`DEVTEAM_ROLE/TICKET/FEATURE/RUN`, `TMUX`, `TMUX_PANE`) stays. After the scrub it is dead and is a second denylist. kit.mjs is in touches, so remove it in Task 2, or say why it stays. | fixed: fleetFixture's denylist at kit.mjs:95 goes too |
+| P5 | Minor | spec AC2 | plan.md Task 2 Step 1 (decoy spawn) | The child is spawned on the relative `test/machine.test.mjs` and no `cwd` is given. Use `path.join(testDir, 'machine.test.mjs')`, or pass `cwd: path.resolve(testDir, '..')`, so the proof doesn't depend on where the parent runs. | fixed: cwd REPO |
+| P6 | Minor | spec AC5 | plan.md Task 1 Interfaces (`boxFns`), Step 1 | The `files` shape contradicts itself. The signature defaults it to `[]`, but the text says it is `{ [path]: text }`, and Step 1 says "`files` = both box paths". Settle on the object form, default `{}`, and have Step 1 pass each box path with its content (the key's PEM read from `GW_PUBLIC`, which is a path, not text). | fixed: files is { [boxPath]: text } throughout |
+| P7 | Minor | spec Rulings (d-20261114-215844973-supervisor: "The PR body says in one line what it doesn't defend against") | plan.md Task 3 Step 4 | No step writes the PR body line the ruling requires. Add it to Step 4. Name the ports.mjs:22 comment ("The gateway test claims no block") there as well, which the spec's Edge cases say is "left and noted" because ports.mjs isn't in touches. | fixed: Task 3 Step 5 writes the PR line and the ports.mjs note |
+| P8 | Minor | plan Global Constraints; ruling m-20261114-215602684-supervisor | plan.md Task 3 Steps 2-3 | Two steps conflict with the Global Constraint that every test run goes through run-isolated.sh. Step 3's `devteam.mjs gate test` runs the whole suite under `gateEnv()` (devteam.mjs:4375), that is `{ ...process.env, DEVTEAM_HOME: mainRoot, … }` with the live `DEVTEAM_RUN`. Step 2 adds `run-as-worker.sh` for "the session's own env" but keeps `DEVTEAM_RUN` at a "decoy copy", which it never defines. Both are reasonable as the end-to-end check once the decoy proof is green, but the plan should state them as deliberate exceptions, run only after Task 2's proof passes. It should also say exactly what `run-as-worker.sh` sets and unsets, so it can't hand a test the live log. | fixed: run-as-worker uses a decoy run only after the proof passes; gate runs run-isolated.sh under it |
+| P9 | Suggestion | done-when 3 | test/hub-overview.test.mjs:15, 86; test/lib/kit.mjs:94 | Only the Chrome test gets a temp `HOME`. hub-overview's `fleetFixture` tests run with the real `HOME`, so on the box `DEVTEAM_BOX=off` is ignored there and the box files still win. They look box-insensitive (`hello` pulse), and Task 3 Step 1 would catch it, but the plan could say so. | fixed: noted in Task 3 Step 2 (they keep the real HOME and must still pass) |
+
+Checked and holding. Task 1's vm block evaluates. The cut code (`previewBox` plus `onBox … gatewayThreadKey`, including `threadKeyMemo` and `OPERATOR_PROOF_MS`) needs only `fs`, `os`, `crypto`, `process` and `URL`, and the plan supplies all five. `BOX_GATEWAY_FILE` is declared after `onBox`, which is safe because it is read only at call time. A fresh context per `boxFns` call keeps `threadKeyMemo` from going stale. The stand-in `existsSync` answers for the two box paths, so the real `/opt` and `/etc` files on this box don't leak. `/* box start */` is not a substring of `/* preview box start */`, and no marker exists yet. Task 2's expected names match what kit.mjs and its imports leave: ports.mjs and cli.mjs set no env, and `cli.mjs`'s `machineless` only builds per-call env. ESM evaluates kit.mjs's imports before its body, but neither one reads `DEVTEAM_*` at top level. `DEVTEAM_FORK` and `DEVTEAM_TEST_KEEP` survive for lib/fork.mjs. Dropping the per-file loops loses nothing the scrub doesn't cover. Deleting `NODE_TEST_CONTEXT` is right. TL;DR matches the tasks (quality §6).
+
+Verdict: CHANGES 945f4765e4e767e215bac406e8fec15b01de4200