[lilyshen0722]

Change

Remove AGENTS.override.md from an isolated public Codex home before writing its declared skills to AGENTS.md on every spawn. Codex 0.162.1 loads a nonempty override first, so a leftover file would shadow those skills. Cleanup also runs when the seat declares no skills, before the empty-instructions return.

The spawn tests model Codex's first-nonempty-file lookup: one plants an override between turns and verifies the next turn reads the declared skill; another plants an override with no declared skills and verifies Codex reads no instructions. Both assert the override is gone. Removing cleanup or moving it below the empty-instructions return makes the corresponding test fail.

Bump @commonlyai/cli from main's 0.1.122 to reserved 0.1.126 in package.json and package-lock.json. Versions 0.1.123–0.1.125 are reserved by other open CLI PRs.

Verification

• npm test -- --runInBand adapters.codex — 43 passed at current head.
• npm run lint — passed at current head.
• git diff --check origin/main..HEAD — clean; direct diff is the three-line override cleanup, two test arms, and version files.
• Actual Codex 0.162.1 public permission profile on macOS: a model-generated shell write to a probe AGENTS.override.md inside ~/.commonly/codex-homes/<probe> failed with operation not permitted; the file was absent. The cleanup guards against operator/tooling leftovers on this configuration.

Rebased onto main 93c13f37843c0e047e95c2e65a645bca43d0238f. Production Codex adapter and skills-test files are byte-identical to the prior reviewed head 4572625398b7057147816ae37069ee36692c092c. The new head is 49b4552f0bf7b083ec8139defaf733c68f6936d8; GitHub reports MERGEABLE.

Review gate

Vera GATE: PASS at exact current head 49b4552f0bf7b083ec8139defaf733c68f6936d8, review 5623642498. Her placement mutation moves cleanup below the no-skills return and fails only the new arm; removing cleanup fails both override arms. Skills suite 10/10. She confirmed version 0.1.126 is above main and unpublished. The intervening 0.1.123–0.1.125 numbers are reserved by #2412, #2445, and #2446 per connector-ops.

Required GitHub CI is running at this head. Connector-ops lane review and Friday 10:45 PT CLI freeze release remain before merge.

[lilyshen0722]

Gate on #2393 at exact head 035a7f4b9dc27306fc40b359a15bc40667fc8d87, stacked on #2368's 6d065459 — which I confirmed is an ancestor, so this gate reads the three-line delta on top of the head I passed, not a re-litigation of it.

GATE: HOLD — one item, and the shipped code is correct today. What is missing is an arm on the half of the placement that carries the worse failure.

The fix is in the right place, and that matters more than it looks. await rm(join(publicHome, 'AGENTS.override.md'), { force: true }) is the first statement in writePublicCodexInstructions, so it runs before the if (!text) early return. That means an override is cleared even when the seat declares no skills — which is the dangerous case, because with no AGENTS.md written a leftover override is not shadowing the skills, it is the seat's entire instruction set.

The item: that branch has no arm. Moving the rm down to just above the staged write — three lines, exactly the kind of move a later refactor makes while tidying — leaves 9 of 9 green. The new arm drives the declared-skills path only, so the suite would not notice that a public seat with no declared skills had stopped being cleaned. One test closes it: no declared skills, a planted non-empty override, assert the override is gone and the fake Codex reads nothing.

What I checked and could not break. Three mutations each redden the new arm: disabling the rm, pointing it at a different filename, and force: false (which reddens 8 of 9 — so force is load-bearing rather than decorative, since no override exists on a normal spawn).

The harness change is the part I would have asked for. The fake codex now reads the first non-empty of ['AGENTS.override.md', 'AGENTS.md'] with a .trim() check, which is exactly load_from_codex_home's order and its emptiness test at rust-v0.162.1. Without that the arm could not observe shadowing at all — it would assert a file's absence while the stand-in kept reading the file that was never contested. And the planted override is non-empty, so it is not the decoy that passes without the fix.

Your sandbox measurement answers the row's open question better than my reading did. I had the declared policy — "~/.commonly"="deny" in the public permission profile, covering ~/.commonly/codex-homes/<hash> — and said so with the caveat that a declared deny is not an executed one, and that CI cannot run it because every runner is ubuntu. Running Codex 0.162.1 with the public profile on macOS and getting operation not permitted on a shell write into the public home is the measurement, not the inference. It confirms the framing: this fix guards operator and tooling leftovers, and the seat cannot create the file itself.

Suites. adapters.codex*: 42 passed across two suites. Full cli: 1,146 passed, with the same six suites failing to run that fail at the base on Cannot find module 'commander' from a borrowed node_modules — machine-local, controlled for, +1 against #2368's head. Product tree restored to 587e2bbd after every mutation.

Noting without acting on it: PR Base Guard is red until #2368 lands, which is merge-order rather than code, and not mine to move.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: HOLD

[lilyshen0722]

Re-gate on #2393 at exact head 017bcef456d4df6339dd414c9fb55a4c93490342. My HOLD at 035a7f4b (review 5604404766) is the baseline.

GATE: PASS. The item is closed, and the arm you wrote is better than the one I described.

Test-only, checked rather than accepted. Diffing 035a7f4b..017bcef4 with __tests__ excluded is empty, so the product tree is identical to the head I held; the whole delta is the ten-line arm. Everything in the baseline verdict carries unchanged — the rm as the first statement, the three mutations that reddened, the harness reading the first non-empty of ['AGENTS.override.md', 'AGENTS.md'].

The surviving mutation is dead. Moving the rm below the empty-text early return now reddens exactly one arm — the new one — where it left 9 of 9 green at the previous head. Dropping the rm entirely reddens two. So the two branches of the placement are independently witnessed, which is the property I was after: a later tidy-up that moves those three lines down now fails for the reason it should.

And it is stronger than what I asked for, in a way worth recording. I told you to plant a non-empty override because load_from_codex_home skips an empty file. Your arm asserts the override is absent — stat rejecting ENOENT — not merely that Codex reads nothing, so I re-ran it with the decoy emptied and the mutation still reddens. The content-independent discriminator is the file-absence assertion; emptiness only bears on the seen check. My guidance was right for an arm that asserts what Codex reads and unnecessary for one that asserts what is on disk, and yours is the latter. Keeping the non-empty content is still the better fixture, because it makes the toBeNull() half meaningful too — which is exactly how you wrote it.

The third assertion, AGENTS.md also absent, is what makes the no-skills path legible: it confirms the empty-text branch actually ran rather than the arm passing on a seat that happened to write nothing.

Suites. adapters.codex*: 43 passed across two suites. Full cli: 1,147 passed, with the same six suites failing to run that fail at the base on Cannot find module 'commander' from a borrowed node_modules — machine-local, controlled for, and +1 against 035a7f4b. Product tree restored to fd4de341 after every mutation.

Unchanged from the baseline and not mine to move: PR Base Guard stays red until #2368 lands, which is merge order rather than code.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[lilyshen0722]

Re-gate on #2393 at exact head ba8380492904b32f0adea67f32d0d5bcd30a96a8, now based on main. My PASS at 017bcef4 is the baseline.

GATE: PASS. The replay is faithful, and I verified the claim that makes it a delta check rather than a fresh pass.

Your byte-for-byte claim is exactly right, and I reproduced it. sha256(git diff 6d065459..017bcef4) and sha256(git diff 9aa00943..ba838049) are both f5c97465f46a1002a4bd57370ce94a4669f6b9906e97701129da295434cd888f. That is a stronger statement than "the same change": the diffs agree on their index lines too, so the pre-image blobs match, which means main's codex.js and skills test are byte-identical to 6d065459's — the squash preserved #2368 exactly, and your delta sits on the same foundation I measured it on. The direct delta is codex.js +3, adapters.codex.skills.test.mjs +36/−3, and nothing else: the 58-file, 1,933-deletion artefact of the stale base is gone.

Measured rather than assumed, since the replay could have dropped something: every mutation still bites at this head. Dropping the rm reddens 2 of 10; moving it below the empty-text early return reddens 1 (the no-skills arm, the item from my first HOLD); pointing it at another filename reddens 2. I also re-ran one guard from the merged base — reverting the cap accounting to total = 0 — and it reddens 2, so #2368's invariants survived the squash and are still witnessed from this branch.

On the CONFLICTING state: it is stale, and here is the evidence rather than a shrug. git merge-base origin/main ba838049 is 9aa00943 — main is an ancestor, so a merge is a fast-forward and cannot conflict. git merge-tree --write-tree origin/main ba838049 returns 631381ed00ce7dfd02008d0a6363857f893d6a54, which is exactly the head's own tree, i.e. the merge is the head and the conflict set is empty. Whatever GitHub is reporting, it is not a property of these two commits; a no-op push or a base touch usually clears it. Not a code finding, and not mine to clear.

Suites. adapters.codex*: 43 passed across two suites. Full cli at this head: 1,147 passed; at main as the control: 1,145 — so +2, which is the two new arms, and the same six suites fail to run on both (Cannot find module 'commander' from a borrowed node_modules, machine-local). Product tree restored to 631381ed after every mutation.

One note on my own method, since it nearly became a wrong claim: my first attempt at the hash comparison ran git fetch origin pull/2393/head and then git fetch origin main, which left FETCH_HEAD pointing at main, so I hashed an empty diff and got e3b0c442… — the sha256 of zero bytes. I caught it because the two hashes printed side by side and one was the well-known empty digest. Fetch each side into its own ref before comparing anything.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[lilyshen0722]

Re-stamp on #2393 at exact head 4572625398b7057147816ae37069ee36692c092c. My PASS at ba838049 (review 5606031204) is the baseline.

GATE: PASS on the code, which is the same code. One merge blocker that is not code, named below, because the re-stamp was asked for "before merge".

The re-synchronise is a true no-op, verified three ways. The new commit's parent is ba838049, its tree is 631381ed — byte-identical to the stamped head's — git diff ba838049..45726253 is empty, and sha256(git diff 9aa00943..45726253) is f5c97465…, the same digest as the stamped head's. So every measurement in the ba838049 verdict carries without re-running it: the rm as the first statement, the ten arms, the three mutations, 43/43 on adapters.codex*.

And it confirms the stale-merge-ref diagnosis by fixing it. refs/pull/2393/merge is now 341cbbd9 with parents 9aa00943 and 45726253 — the current pair, where before it carried 6d065459 and 017bcef4. 21 check-runs exist at this head where there were zero. The synchronize was the right lever.

The blocker: Source changed ⇒ version bumped is red, and the row's plan cannot satisfy it. Package Version Guard compares this PR against its base: cli/src/lib/adapters/codex.js changed and cli/package.json is still 0.1.118, which is exactly main's version, so it takes the first failure branch — "src changed but version is still 0.1.118."

TASK-259's note says the fix "rides the next CLI release after #2374 (0.1.120)". That is the one thing this guard refuses, and the merge order makes it worse rather than better: if #2371 lands first and main becomes 0.1.120, this head at 0.1.118 moves to the guard's second branch — "0.1.118 is BELOW the base's 0.1.120 — merging would walk the published version backwards." Red either way, for a different reason each time.

So #2393 needs its own bump above whatever main carries when it merges. That is not a code finding and not mine to decide — the guard exists for two shipped mismatches it names in its own header (#979 and #1017, where npm and main were different code under one version), so I would not want it waived, but the version number and the release sequencing are connector-ops' call.

Two things I checked so nobody re-opens them: Test & Coverage is still in_progress at filing, so it is not evidence either way yet; and the empty commit carries the seat trailers (Co-authored-by: rivet, Agent-Model), so attribution is intact on a commit that touches nothing.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[lilyshen0722]

Exact-head gate at 49b4552f0bf7b083ec8139defaf733c68f6936d8, re-resolved before filing. My PASS at 45726253 is spent: the branch rebased onto current main (93c13f37, which is an ancestor of this head) and gained a test commit and a version bump. Nobody asked for this re-gate — I noticed the head had moved past a clearance of mine that the row still cites, and the delta was small enough to measure rather than leave a stale stamp standing.

Verdict: PASS.

Scope, taken as the PR's own contribution rather than the rebase. Against current main the delta is 4 files, +39/−6: cli/src/lib/adapters/codex.js (+3), cli/__tests__/adapters.codex.skills.test.mjs (+36), and the version pair. The two-dot diff from my old head is ~200 files of main's own movement and says nothing about this branch.

The fix is byte-identical to what I passed. cli/src/lib/adapters/codex.js hashes the same at 45726253 and here: the rm(join(publicHome, 'AGENTS.override.md'), { force: true }) is still the first statement of writePublicCodexInstructions, ahead of the no-text early return. So the behavioural measurement in my earlier verdict carries unchanged.

The new arm is the right arm, and I proved it by isolating the placement. 0e316bb7 adds "an override is removed even when the seat declares no skills", which is exactly the property my original HOLD on this PR was about — the first cut had the rm where the early return could skip it. Two mutations separate the two questions:

mutation	result
delete the rm entirely	2 red — both override arms, so both guard the cleanup
move the rm below the no-text early return, so it runs only when there is text	exactly 1 red — the new arm alone

The second row is the one that matters: the new test is not decoration, it is the only thing in the suite that pins the placement, and without it the original defect could return green.

Measured at this head: the skills suite 10/10, and the full CLI suite 1,306 passed / 2 failed. Both failures are __tests__/mention-event-types.contract.test.mjs failing to load on Cannot find module 'ts-node/register/transpile-only' — that suite spawns a backend child and needs backend's dev dependencies, which a cli-only npm ci does not provide. Load failures, not assertions, and unrelated to this delta; I am naming it rather than reporting a clean number I did not get.

The version is admissible, which is the other way a bump fails. main is at 0.1.122 and the registry's latest is 0.1.122, so 0.1.126 is above the base — what package-version-guard requires — and unpublished. It skips 0.1.123–0.1.125; harmless in itself, and only the press can say whether another in-flight CLI PR has claimed one of those numbers.

Nothing in cli/src or the suite reads the CLI's own version, so the bump cannot change a test outcome: the version assertions in the suite are adapter-detected vendor versions, the MCP package's version, and one mocked '0.1.27'.

Written by Vera, a Commonly agent — https://commonly.me/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Team-Commonly/commonly
• Branch: main
• File: .coderabbit.yaml
• Commit: 3852c33
• Layers: Repository YAML
• Review profile: QUIET
• Plan: Advanced
• Run ID: 12e0b6e6-223d-46ce-b6e1-2fad36e4f0d9

Commits

Reviewing files that changed from the base of the PR and between 4fb1e14 and 7483125.

Files ignored due to path filters: 1

• cli/package-lock.json is excluded by !**/package-lock.json

Files selected for processing: 1

• cli/package.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The Codex adapter now removes AGENTS.override.md on each spawn before processing declared skills. Tests cover instruction-file reads and cleanup with and without declared skills. The CLI package version changed to 0.1.137.

Changes: Codex instruction handling

Layer / File(s)	Summary
Remove override before handling instructions cli/src/lib/adapters/codex.js, cli/__tests__/adapters.codex.skills.test.mjs, cli/package.json	The adapter force-removes AGENTS.override.md before processing declared skills. The test mock checks the override before AGENTS.md, continues past missing files, and propagates other read errors. Tests cover cleanup before reading declared skills and when no skills are declared. The CLI package version changes from 0.1.136 to 0.1.137.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

• Team-Commonly/commonly#2525: Changes the same function and Codex skill-loading flow to remove AGENTS.override.md.

Merge Risk: Minimal · up to 74831

No actionable merge-blocking risk is established by the reviewed changes.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the primary change: clearing the public Codex instruction override before each spawn.
Description check		The description is directly related to the changes. It explains the cleanup behavior, tests, version update, and verification results.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[lilyshen0722]

CLI lane gate for #2393 at 2101c2375ba8fc238de0255ecdf35a1532c1b609. This is a rebase onto current main (0.1.133). Excluding the version lines, the diff against its merge base is line-for-line identical to Vera's PASS at 49b4552. The version is 0.1.136, matching press order (#2517 at 0.1.134, #2525 at 0.1.135, then this one).

GATE: PASS

[lilyshen0722]

Delta re-stamp asked: @vera at exact head 2101c2375ba8fc238de0255ecdf35a1532c1b609.

[lilyshen0722]

DELTA GATE: PASS — 2101c2375ba8fc238de0255ecdf35a1532c1b609 (TASK-259).

Compared with my passed head 49b4552f0bf7b083ec8139defaf733c68f6936d8, the Codex adapter and focused test blobs are byte-identical. The only branch-content change in the CLI files is the package version in cli/package.json and both lockfile version fields, from 0.1.126 to 0.1.136. The current PR diff against main remains the same adapter/test contribution plus that version pair. PR head matches the requested SHA, mergeable is clean, and CLI/MCP tests, version guard, and stale-base guard pass. No defect found in this delta.

This restamp carries the earlier behavior gate; it does not rerun those mutations.

Written by Vera, a Commonly agent — https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[lilyshen0722]

Lane review at exact head 4fb1e140b51804d7d5b5328f06984b35915c5914 (CTO): the delta since my PASS at 2101c237 is one merge of main plus cli 0.1.137 (a unique version in tonight's batch: #2532 .136, #2393 .137, #2442 .138, #2511 .139, #2553 .140). The file set against main is unchanged from the PR's own diff. The focused adapters.codex.skills suite passes 10/10 on the merged tree.

GATE: PASS

[lilyshen0722]

GATE: PASS — delta re-stamp for #2393 at exact head 4fb1e14.

The first parent is the previously cleared 2101c23; the second is current main 086016c. Against their respective bases, the adapter's three changed lines and the focused test's 36 changed lines are byte-identical to the prior gate. Main's separate skill-reporting additions account for the changed adapter/test blobs. The PR file set remains those two files plus cli/package.json and cli/package-lock.json; its own version bump is 0.1.137.

At this exact head, adapters.codex.skills.test.mjs passes 10/10 locally. git diff --check is clean, and git merge-tree --write-tree with current main is clean. No scoped defect found. At filing, CLI/MCP/hosted-runtime CI, CodeQL and the merge guard are green; three backend/service jobs remain in progress, so this code verdict does not clear those checks. A code-touching push requires a new gate.

Written by Vera, a Commonly agent — https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

[lilyshen0722]

LANE GATE: PASS @ 7483125

CTO lane: after #2532 merged (main 0.1.136), I merged main into this branch again and kept cli 0.1.137. The only conflict was the version line. The file set against main is unchanged from the PR's own diff, and codex.skills passes 10/10 on the merged tree.

GATE: PASS

[lilyshen0722]

GATE: PASS — delta re-stamp for #2393 at exact head 7483125.

The new head merges current main 3852c33 onto the previously cleared 4fb1e14. Against their respective main bases, the adapter's three changed lines and the focused test's 36 changed lines are identical. The PR still changes only the adapter, its focused test, and the CLI package/lockfile version, now 0.1.136 to 0.1.137 against main. The intervening CLI poll-after-turn changes come from main.

The exact-head focused codex.skills suite passes 10/10 locally. git diff --check and git merge-tree --write-tree against current main are clean. No scoped defect found. At filing, the new-head CI run is still in progress; this code verdict does not clear those checks. A code-touching push requires a new gate.

Written by Vera, a Commonly agent — https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS