Repository navigation
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)📝 WalkthroughWalkthroughCodex trust creation now records newly created project-table paths in a ledger. When a local worktree is removed, the revocation flow removes eligible Orca-written entries from config files and stops tracking candidate paths. It preserves tables that were not written by Orca, were edited, or still belong to a remaining workspace. Account-home discovery supports locating owned Codex homes for config cleanup. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to Removing a hostless remote worktree could revoke a matching local Codex trust entry. Require proof that the worktree is local before starting cleanup. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change reduces lingering trust after local workspace removal and preserves several ownership safeguards. However, overlapping removal and recreation can confuse which workspace owns a trust entry. The supported concern is bounded to local trust settings; no new remote-access or privilege-escalation path was established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 338d7ab6-5116-480d-aab2-35907198aec5
📥 Commits
Reviewing files that changed from the base of the PR and between 1fc24d3 and 7ccf3af28c19800d3ca32a5246913b7d14011947.
📒 Files selected for processing (17)
src/main/agent-trust-presets.tssrc/main/agent-workspace-trust.test.tssrc/main/agent-workspace-trust.tssrc/main/codex/codex-account-home-discovery.tssrc/main/codex/codex-project-trust-ledger.tssrc/main/codex/codex-project-trust-revocation.test.tssrc/main/codex/codex-project-trust-revocation.tssrc/main/codex/config-toml-project-duplicate-repair.tssrc/main/codex/config-toml-project-trust-removal.test.tssrc/main/codex/config-toml-project-trust-removal.tssrc/main/codex/config-toml-project-trust.tssrc/main/codex/config-toml-trust.tssrc/main/execution-host-workspace-trust.tssrc/main/ipc/worktrees/removal/worktree-removal-ownership.tssrc/main/runtime/orca-runtime-resolve-worktree-removal-target.tssrc/main/workspace-removal-codex-trust.test.tssrc/main/workspace-removal-codex-trust.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
Important
The happy path is correct and well-tested, but a few error paths can strand the ledger or delete trust prematurely. All are anchored inline.
Reviewed changes
- Trust ledger (
codex-project-trust-ledger.ts): records each config file and project path only whenupsertProjectTrustLevelcreates the table, incodex-runtime-home/project-trust-ledger.json. - Revocation (
codex-project-trust-revocation.ts): on local workspace removal, deletes ledgered[projects."<path>"]tables at or below the removed root from every recorded config, then the mirror copies from each owned account home — only when the table is still exactly Orca's shape and no remaining workspace at or below the root claims the path. - TOML removal (
config-toml-project-trust-removal.ts): line-splice removal reusingreadTomlTables/isOrcaWrittenTable/getTrustTableKind(newly exported), preserving CRLF and the trailing separator. - Removal wiring (
workspace-removal-codex-trust.ts): fire-and-forget after metadata is gone, on both the runtime purge and the UI metadata purge; SSH and other hosts are skipped, and folder-session ids are reduced to their real folder. - Discovery refactor (
codex-account-home-discovery.ts): exposesgetOwnedCodexAccountHomePaths()for the account-home sweep.
Technical details
# Review scope
Full 17-file diff at `7ccf3af`. Ran the focused new suites (21 tests, all pass), the
changed-code-quality gate (0 findings), and traced the real callers
(`removeWorktreeMetadataAndTransientState`, `removeWorktreeMetadataAndHistory`,
`getLocalCodexTrustConfigFiles`, the config mirror's `untrusted` handling). No critical or
security issue found; the findings below are durability/error-path concerns only.deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
de1b3f2 ("keep trust cleanup recoverable when a step fails") hardens the Codex trust cleanup against partial failures and addresses every finding from the prior review:
- Ledger durability (
codex-project-trust-ledger.ts): writes now go throughwriteFileAtomically; the ledger gainspendingMirrorCleanup, and the newreleaseCodexProjectTruststops tracking released paths and queues their mirror cleanup in a single atomic write. - Unreadable-source safety (
codex-project-trust-revocation.ts):readConfigOrEmptynow treats only definitive absence as empty and rethrows otherwise, and the newprojectTrustTableTrustscounts an explicituntrustedsource as not trusting the path. - Account-home sweep (
codex-account-home-discovery.ts):getOwnedCodexAccountHomePaths({ strict: true })surfaces temporary home-unavailability, so a failed sweep preserves the pending record for the next revocation instead of dropping it. - Launch resilience (
agent-trust-presets.ts): a throwingonTableCreatedcan no longer abort the remaining config writes. - Tests (
codex-project-trust-revocation.test.ts): new cases cover an unreadable source keeping the mirror copy, a retry after an unreachable account root, and a throwing ledger callback still writing trust everywhere.
Verification: the three focused suites pass (16 tests) and check:code-quality:changed reports 0 findings across the 17 changed files.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
…oved Orca pre-trusts each folder it starts Codex in by writing a [projects."<path>"] table to config.toml, and the config mirror copies those tables into every managed account home. Nothing ever removed them, so trust outlived the worktree and covered whatever later appeared at a reused path. Record in a ledger each table Orca's pre-trust created (never one that already existed). When a local workspace is removed, delete the ledgered tables at or below its root that still hold exactly what Orca wrote, then their mirrored copies in account homes once no source trusts the path. Tables a user edited, and folders another remaining workspace still uses, are kept. Fixes stablyai#24697 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-ups for stablyai#24697: - A ledger write that throws no longer stops trust reaching the remaining config files. - The ledger is written atomically, so a torn write cannot erase it. - Released paths are queued in the ledger before account homes are touched, and the queue is retried on the next revocation, so an unreadable accounts root no longer strands mirror copies. - Only a definitively absent config reads as empty; an unreadable one aborts instead of letting mirror copies be deleted. - A source that says `untrusted` no longer shields its mirror copies, matching how the config mirror reads it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
de1b3f2 to
1bfa22a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
74dcd532-48ad-45a7-b693-f23245e6cb2c
📥 Commits
Reviewing files that changed from the base of the PR and between de1b3f21b92242b004c79ae9897796fe2aae35c9 and 1bfa22a.
📒 Files selected for processing (2)
src/main/execution-host-workspace-trust.tssrc/main/runtime/orca-runtime-resolve-worktree-removal-target.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

ELI5
Before Orca starts Codex in a worktree, it tells Codex in advance that it can trust that folder. Until now Orca never took that back, so the trust stayed after the worktree was deleted. Now, when a worktree is removed, Orca removes the trust it added for it. Trust you set up yourself stays.
What Changed
Before: every Codex launch into a new folder added this to
config.toml:Removing the worktree (
orca worktree rmor Delete in the sidebar) left the entry in place. The config mirror had also copied it into every managed account home (codex-accounts/<id>/home/config.toml), and the mirror only adds tables, never removes them. Over time these files filled up with trust for paths that no longer exist. Worktree names get reused (-r1,-r2, …), so a stale entry would also trust whatever later appeared at that path.After: removing a local workspace removes the trust Orca wrote for it, both from the files Orca wrote to and from the account-home copies.
How it works:
codex-runtime-home/project-trust-ledger.json. If the table already existed, it belongs to the user and is never recorded.purgeRemovedWorktree, used by the CLI and by interrupted removals that resume) and the UI path (removeWorktreeMetadataAndTransientState, used by worktree, folder-workspace and forget removals). For each recorded path at or below the removed root, Orca deletes the table only if it still contains exactly what Orca wrote. Every write goes through the existing per-file Codex config lock.~/.codexnor the runtime home still trusts the path (no table, or an explicituntrusted), the path goes into a pending list in the ledger. This happens in the same atomic write that stops tracking it. The mirrored copies are then deleted from each owned account home, again only if they still contain exactly what Orca wrote. If an account home or a source config cannot be read, nothing is deleted and the pending list is kept; the next revocation retries it.What is kept:
/repo/.worktrees/xstill cleans upx.Why
The issue asks that only entries Orca itself added be removed. A config file cannot tell you who wrote a table, so a record made at write time is the only reliable way to know. The alternatives each had a problem:
The recording hook is optional on the shared trust writer, so the SSH relay, which runs the same code, does not create a ledger on remote hosts.
Linked Issue
Fixes #24697
Visual Proof
N/A: nothing visible changes in the UI. The only change is what is left in Codex's
config.tomlafter a removal, and the tests below check those files directly.Testing
New tests:
config-toml-project-trust-removal.test.ts:untrusted, have an extra key, use a literal-string header, or use Codex's quoted"trust_level"key.codex-project-trust-revocation.test.ts: real files in a temporary home, written through the actualmarkCodexProjectTrustedand the ledger.~/.codexand an account home.workspace-removal-codex-trust.test.ts: cleanup runs for local workspaces and for workspaces with no recorded host, folder-session ids are reduced to their real folder, and SSH hosts are skipped.agent-workspace-trust.test.ts: updated for the new optional callback argument.Commands run (macOS):
pnpm test src/main/codex src/main/agent-workspace-trust src/main/execution-host-workspace-trust src/main/ipc/worktrees/removal src/main/workspace-removal-codex-trust.test.ts src/relay/agent-workspace-trust: 254 files, 2780 tests passed.pnpm typecheck:nodeandpnpm run check:code-quality:changedpassed, with 0 new findings.Not tested: I did not run a real Orca launch-then-remove cycle, and I did not run anything on Linux, Windows or over SSH.
AI Disclosure
Written with Claude Code (Claude Opus 5.5). I reviewed the design and verification.
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
remote-wire-compatibility.mdshould come in its own PR.isPathInsideOrEqualand realpath forms. A removed checkout is resolved through its parent folder, which still exists.config.tomlwriter. This PR touchesupsertProjectTrustLevelin only one place, to report whether it created the table, so whichever lands second should be easy to rebase.config.tomlfiles when a recorded path matches.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)🤖 Generated with Claude Code