[qiragu]

Closes #536.

Tier: 0 — the system tests leave nothing behind in the owner's Orca on the guarded Orca release, as they did before it.

What changed

1. test/helpers/own-project.js is new. deleteOwnProject(setup, bots) checks that the project is the run's own, then removes it with deleteProject from src/orca.js (Orca's next release refuses project setup-delete without --force (stablyai/orca#27172): obk retire may break #528). That sends --force, and sends the plain delete when Orca 1.4.223 refuses the flag.
2. The project is the run's own only when bots is an obk-system-* folder directly in the real temp folder, and the project's path is strictly inside bots. Otherwise the helper fails and sends nothing to Orca.
3. All 37 teardowns in test/system/ (33 files) use the helper. A failed delete is kept in a list. The rest of the teardown still runs: the tabs, the reload, the folders. The teardown fails on the list last.
4. scripts/test-system.js lists orca project setups before and after the run. It names each project that was not in Orca before the run and is in an obk-system-* folder that the run made, with its setup id, and the run fails. It removes no project. When Orca cannot list its projects, before or after, it says so, names none, and does not say that nothing was left.
5. test/system-teardowns.test.js holds every system test to points 1 and 3. A system test file may not contain the text setup-delete or deleteProject(, comments included. Its comment stripper now reads regular expression literals, so a " in the regex at restart.test.js:340 opens no string. The teardown-scoped rules use this heuristic, and it is not complete. Only the raw-text rule is.
6. One line in README.md describes the new report.

Boundary 3 changed

Boundary 3 said that the runner "still reports" a left project. The runner listed no Orca projects at all. The architect answered (2026-10-10):

architect: yes to your fix for boundary 3. You are right: I checked scripts/test-system.js, and it lists no Orca projects. My premise was wrong.
Build points 1 to 4 and the rest of the plan as you wrote them. Four notes:

1. Confirm orca project setups on the installed Orca 1.4.223, and its output shape, before the runner relies on it. Put the real output in the fake.
2. The runner names and fails. It never deletes a project, and it never names a project outside the run's own obk-system-* folders.
3. A failed delete fails the teardown. Make sure the rest of the teardown still runs after it: the tabs, the folders and the trust keys. One failure must not leave the others behind.
4. Say in the PR that boundary 3 changed, and quote this answer as the source.

For note 1, I read orca project setups --json live on Orca 1.4.223. The call is read-only, and its help says "List project host setups". The runner test's fake uses that envelope and setup shape, with made-up paths.

Review rounds

The reviewer found five defects, and each is fixed with tests first by the test author:

• ac1e83b: the teardown check took a division after n++ or obj.in for a regular expression, and so could hide a delete.
• f339b7d: the same for object . in / 2, with space or a comment around the dot.
• f339b7d: a project that an earlier run left, with its folder gone, failed the next run. The runner now also compares setup ids from before the run.
• 1ac2162: {} / 2, of / 2, πin / 2 and a later regex on the line still hid a delete. The check no longer erases text after a slash: the text stays as code with its quotes made neutral, and forbidden calls are also looked for in a reading with no regex handling.
• 2b88d39: a backtick in a regular expression still hid a delete from both readings. The System tests' cleanup uses a plain setup-delete: on the Orca release with the guard (#27172) they would leave throwaway projects behind #536 rule now reads the raw text: a system test file may not name setup-delete or deleteProject( at all, comments included. The teardown-scoped rules still use the parser, with its known limits.

Checks run

• Tests were written first by a separate author (a subagent), from the issue and the public interfaces.
• Red on the old code, in a scratch worktree: test/system-teardowns.test.js named all 33 files. The System tests' cleanup uses a plain setup-delete: on the Orca release with the guard (#27172) they would leave throwaway projects behind #536 runner tests failed (exit 0, no project report). test/system-own-project.test.js failed only on its import (ERR_MODULE_NOT_FOUND), which shows no behaviour.
• Behavioural red for the helper, in a scratch worktree at c04ff64 with the helper replaced: a stub that does nothing failed 25 of 25. A stub that does what the teardowns did before System tests' cleanup uses a plain setup-delete: on the Orca release with the guard (#27172) they would leave throwaway projects behind #536 (a plain delete of any project, the answer ignored) also failed 25 of 25.
• Green on this branch: test/system-own-project.test.js 25/25, test/system-teardowns.test.js 21/21 (at 2b88d39), test/test-system.test.js 160/160 (at f339b7d), test/repo-guards.test.js 38/38, the README-reading tests 70/70. Every file in test/system/ passes node --check.
• 10 hand mutations, all caught: each of the helper's 4 own-project conditions removed, a plain delete only, a left project that does not fail the run, folders from before the run counted, an unlisted answer not said, the regex branch of the stripper turned off, and one teardown's last assert removed.
• 2 more hand mutations on the runner fix, both caught: setup ids from before ignored (3 fail), a failed listing before the run ignored (5 fail).
• The 8 new runner tests failed on the code at ac1e83b, on exit 1 and on naming the earlier run's project.
• al check --strict: passes.

Not checked

• No live system run. Issue boundary 4 puts it on the first Orca release with the guard, together with Orca's next release refuses project setup-delete without --force (stablyai/orca#27172): obk retire may break #528. That release is not out. 1.4.223 is installed.
• The full unit suite was not run locally. CI runs it.

🤖 Generated with Claude Code

[qiragu]

Changes are needed at c04ff6461738da3140c8bef0e113b2e1a34bf375.

Both findings are confirmed P2 defects. P2 means the change needs a fix before merge.

1. Requirement: exclude projects from earlier runs. scripts/test-system.js:410 checks only whether the folder existed before this run. An earlier teardown can fail its project delete, then remove its bots folder. The next run then assigns that existing project to itself. I ran the real runner with fake Orca and one passing dummy test. The test created no fleet or project. Orca already had a setup under an absent obk-system-earlier-123 folder. The runner printed “The run left 1 Orca project in its own folders” and exited 1. Boundary 3 permits only projects in folders this run made. Capture setup ids before the tests and exclude existing ids. Ask the separate test author to cover this case.
2. Code and tests: preserve division when the safety check removes comments. test/system-teardowns.test.js:54–55 treats division after postfix ++, postfix --, or object.in as a regex. Valid input let n = 8; const half = n++ / 2; orca(['project', 'setup-delete', '--setup', id]); becomes let n = 8; const half = n++ /(?:)/. The safety check returns no finding. Thus it misses a direct delete on that line. I reproduced all three forms. Ask the separate test author to add failing cases and correct the parser.

Checks: 192/192 focused tests passed with a 600-second process limit. These were system-own-project, system-teardowns, and test-system. All 33 system files passed node --check. The diff check passed. No review test process remains.

Set aside: CodeQL flagged two regex substitutions at lines 183 and 193. Their inputs are captured identifiers, restricted to letters, digits, underscores, and dollar signs. A backslash cannot reach either substitution.

Not checked: the live guarded Orca release and the full local suite. CI shards were still active. The author reports later behavioral stub failures; the original helper red was an import failure.

VERDICT: CHANGES NEEDED

[qiragu]

Changes are still needed at ac1e83b1dfa8c7319071298e4eeb9986a733c7b3.

Confirmed P2: the parser fix still hides division after a property with whitespace. test/system-teardowns.test.js:58 checks only the character immediately before the keyword. Valid input const half = object . in / 2; orca(['project', 'setup-delete', '--setup', id]); loses the delete. The scanner retains only const half = object . in /(?:)/, and the safety check returns no finding. object /* gap */ . in / 2 has the same result. Ask the test author to cover both forms. Check the preceding token across whitespace and removed comments.

The earlier-run project finding from the first review remains. The runner did not change at this head.

Checks: all 16 teardown tests passed under a 60-second process limit. The original postfix and unspaced property cases now report the direct delete. Both whitespace cases compile as JavaScript and still hide it. This pass covered only the changed file and these probes.

VERDICT: CHANGES NEEDED

[qiragu]

Changes are still needed at f339b7d7ae3a39569a826b7c20a617ed7e02277e.

Confirmed P2: the same parser defect remains for other valid division operands. test/system-teardowns.test.js:56–60 classifies {} and function or class expressions as regex prefixes. It also classifies the legal identifier of, and Unicode identifiers such as πin, as keywords. A direct delete after that division disappears from the safety check. I checked 19 operand forms as strict JavaScript. These five forms compiled but hid the delete: {} / 2, function() {} / 2, class {} / 2, of / 2, and πin / 2.

For example, const of=8; const half=of / 2; orca(['project', 'setup-delete', '--setup', id]); const rx=/x/; becomes const of=8; const half=of /(?:)/x/;. The check returns no finding. A later regex on the line provides a false closing slash. Ask the separate test author to cover these forms. Preserve the code after an ambiguous slash instead of silently erasing it.

Resolved: the runner now excludes setup ids from before the run. Its new tests cover an earlier project whose folder is gone, alongside a new leftover, and failed initial listings. The original postfix and spaced property division cases now report the direct delete.

Checks: 176/176 changed-file tests passed under a 600-second process limit: 160 runner tests and 16 teardown tests. The diff check passed. This pass covered the changed files and the operand probes. No live system file was loaded.

VERDICT: CHANGES NEEDED

[qiragu]

Changes are still needed at 1ac216228b15077083b7b624a703f44a73454c8d.

Confirmed P2: both scanner readings can still hide the same direct delete. test/system-teardowns.test.js:129 refuses regex handling when the span contains a backtick. Both readings then treat that regex backtick as a template delimiter. Later real templates reverse the scanner state. A URL then becomes a line comment and removes the delete after it.

This input compiles as JavaScript:

const rx = /`/; const text = `x
y`;
t.diagnostic(`see http://x`); orca(['project', 'setup-delete', '--setup', id]);

Both withoutComments readings end at t.diagnostic( plus the template delimiter and see http:. teardownTrouble returns no finding. Ask the separate test author to cover this exact case. The second reading does not protect the branch that both readings handle identically.

Checks: all 20 teardown tests passed under a 120-second process limit. The prior division cases now report the delete. The diff check passed. This pass covered the changed scanner and its regression cases. No live system file was loaded.

VERDICT: CHANGES NEEDED

[qiragu]

Approved for issue #536 at 2b88d39. No blocking findings remain.

The raw source rule catches the backtick case. The earlier runner and parser findings are resolved.

Checks I ran:

• All 21 tests in test/system-teardowns.test.js passed, with a 120-second process limit.
• The check read the actual system files as text. It did not load or run them.
• A raw search found neither forbidden text in test/system/.
• The full PR diff passed git diff --check. My clone remains unchanged.
• The unchanged helper and runner passed 25 and 160 focused tests in earlier review rounds.

Set aside:

• The teardown parser has known limits. The new raw source rule avoids those limits for direct project deletion.
• CodeQL reports two escaping alerts at lines 216 and 226. Both values come from identifier captures that exclude backslashes. I found no reachable escaping defect.

Non-blocking record correction: comments at lines 248 and 505 cite an owner's ruling. The mail I received describes the developer's proposal and my review response. Correct that attribution or cite the owner's message.

I did not run live system tests or the full local suite. CI unit shards remain pending. The CodeQL check reports failure. Required checks must be settled before merge.

VERDICT: APPROVED

[qiragu]

Approved at a29e2ef. No new findings.

I reviewed only the diff from 2b88d39, as requested. The comments now give the correct attribution. The escaped helper preserves literal identifier matches and also escapes backslashes.

Checks I ran:

• The diff passed git diff --check.
• The first test attempt could not load yaml in the fresh scratch copy. No test body ran.
• After npm ci with scripts disabled, all 21 teardown tests passed under a 120-second process limit.
• The system files were read as text only. My clone remains unchanged, and no review test process remains.

The CodeQL summary is now neutral. The CI unit shards and JavaScript analysis remain pending. I did not run live system tests or the full local suite. The earlier review conclusions still apply to the unchanged files.

VERDICT: APPROVED