Repository navigation
fix(codex): Orca's Codex hook stays on, is recognised, and trust reaches every home (2/2) - #23931
brennanb2025 wants to merge 11 commits into
Conversation
Fixes the Orca part of #23289. A Codex worker could sit forever at Codex's hooks screen: Orca's own hook entry had the right trusted_hash but `enabled = false`, so the SessionStart event Orca waits for never fired. Orca only ever granted the hash and never re-checked `enabled` (the ledger fast path compared hashes only, the text fallback kept an existing `enabled = false`, and the SSH installer passed no value), so a stale `enabled = false` at the position Orca's entry occupies stuck. Now every launch prep that installs Orca's hook re-derives it: the Codex app-server grant writes `{ trusted_hash, enabled = true }` for Orca's exact managed keys and verify fails if one is still disabled; the ledger fast path misses when the entry is disabled; the local, WSL and SSH fallbacks write `enabled = true` for Orca's entries. Project and user hooks are never touched, so their review stays the user's decision. Orca's own settings stay the only off switch: all of these writes run only on the install path, which launch prep, session resume, startup and the SSH/WSL installs take only when status hooks are on for Codex (#23667's per-agent check). With Codex turned off, launch prep takes the refresh-user-hooks path, which never writes Orca's hook state, so a disabled entry stays off; turning Codex back on re-enables it on the next launch. Supersedes #23292, which enabled the hook once at account creation with nothing to re-derive it later.
…tity Fixes #22434. Codex 0.156.1+ warned at every start that it was clamping a 10 s SessionEnd hook to 3 s. The hook was Orca's Claude status hook: Codex's own "import from Claude Code" copies Claude hooks into ~/.codex/hooks.json, and Orca recognised its entries there only by the Codex hook's script name, so it treated the copied entry as the user's, mirrored it into Orca's managed Codex homes, carried its trust state, and Codex ran Orca's Claude hook from a Codex pane. Orca now recognises its hook entries by one identity: the command runs a script from Orca's `.orca/agent-hooks/` directory, whatever the agent. In Orca-owned Codex homes (the managed runtime home, its user-hook mirror, trust promotion, and WSL runtime homes) such an entry is Orca's: it is not mirrored, its trust is not carried, and stale copies are swept. The user's real ~/.codex/hooks.json is left as it is; removing Orca's copied entry there is an owner decision given the rule that restricts real-home removals.
…y why a worker stalls #23847: worker-start --agent codex could stall at Codex's folder-trust screen with no cause. Trust was written to only some Codex homes and failures were dropped. - Each Codex home is written on its own, including the per-account home a launch actually reads; the shared runtime home is written once. Launch prep's two trust writes share one 20 s budget. - A failed write throws the original refusal (or all of them), so Codex config refusals are still reported once per file; an OS error names config.toml, not the atomic writer's temp file. - When a worker stalls at an agent's trust screen, worker-start re-runs that agent's own trust write for the same folder and host (bounded at 5 s) and reports what it returns: written, failed with the file and reason, still waiting, or not pre-trusted. Nothing is stored, so a cause can never come from another launch, account, agent or host. - Remote trust writes report when nothing was written (no remote home, or no remote writer for the agent).
d6bca18 to
2263d3f
Compare
…eck; honest worker trust messages - Launch prep checks it is a Codex launch before resolving the per-account home, so non-Codex launch prep never touches it (base per-agent opt-out tests were red). - The trust-write test now asserts the original refusal, which refusal reporting relies on. - An OS error that already names config.toml is not prefixed twice. - Worker trust messages no longer promise a retry works when the write may land in a config the agent never reads, no longer blame a config lane for an SSH timeout, and point at 'the cause above' when no file was named.
A substring check let config.toml.bak and rename errors through without saying which write failed; naming the file twice is harmless.
Review status (head
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Why (#22434): every Orca agent hook script lives in Orca's `.orca/agent-hooks` | ||
| // directory, so that directory is Orca's one identity in any agent's hook file, | ||
| // including an entry another agent's importer copied across. | ||
| const ORCA_AGENT_HOOK_SCRIPT = /(?:^|[/'"\s=])\.orca\/agent-hooks\/[^/'"\s]+/ |
There was a problem hiding this comment.
If a user’s Codex hook command merely references a file under .orca/agent-hooks/—for example, to check whether it exists—this matcher treats the entire command as an Orca hook. The next install or refresh then removes it from the Codex hooks mirrored into Orca’s home, so that user hook no longer runs there. Identify the script being executed rather than any mention of the directory.
Knowledge Base Used: Agent integrations and hooks
| const results = await Promise.allSettled([ | ||
| markCodexProjectTrusted(workspacePath), | ||
| resolveLaunchHome().then((home) => | ||
| isSeparateCodexLaunchHome(home) | ||
| ? markCodexProjectTrustedInHome(workspacePath, home) | ||
| : undefined | ||
| ) | ||
| ]) | ||
| const failures = results.flatMap((result) => | ||
| result.status === 'rejected' ? [result.reason] : [] | ||
| ) | ||
| if (failures.length === 1) { | ||
| throw failures[0] | ||
| } | ||
| if (failures.length > 1) { | ||
| throw new AggregateError(failures, `Orca could not mark ${workspacePath} trusted`) |
There was a problem hiding this comment.
When a stalled Codex worker uses a valid per-account home or the real ~/.codex, this retry still includes failures from the shared home. If that unused home has a broken config.toml, the worker’s recovery message names it as a reason trust could not be written. That sends the user to fix a file the worker does not read; report failures for the worker’s actual launch home instead.
Knowledge Base Used: Agent integrations and hooks
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — two rough edges in the stalled-worker messages, plus one nitpick.
Reviewed changes
- Hook re-enable (#23289): every Orca-owned Codex write now stamps
enabled: true(managed install, WSL runtime, remote/SSH); the app-server grant sendsenabled: trueand fails post-grant verify if the entry is still disabled; and the ledger shortcut declines whenhooks.state.enabled === false. - Hook identity (#22434):
isOrcaAgentHookCommandtreats any command referencing.orca/agent-hooks/as Orca's, so an importer-copied Claude hook is filtered out of Orca-owned Codex homes; the user's real~/.codexis deliberately left alone. - Per-home trust + stalled-worker cause (#23847): trust writes now succeed or fail per
config.tomlinstead of all-or-nothing, launch prep also writes the exact per-account launch home, and a stalled worker re-runs that write and fails with a named cause and recovery step.
I verified that enabled participates in no trust key, hash, signature, or ledger record (so forcing it on cannot corrupt ledger matching), that the grant's matchManaged cannot enable a user hook, and that the new writes target disjoint config.toml files with no lock-order inversion.
ℹ️ Nitpicks
- In
src/main/startup/codex-launch-preparation.ts:106, the per-account launch-home write is handeddeadlineMs: trustWaitBudgetMs, which is0once the first write has consumed the full 20 s budget. A free config lane still wins the microtask race, but a busy one returns'expired'immediately and launch prep stops waiting on the exact write #23847 is about (the write itself still lands uncancelled). A small floor would guarantee that path always gets a few hundred milliseconds.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…ves their hooks Codex keys hook approvals by position in hooks.json. When the WSL runtime install removed a Claude hook Codex's importer copied (or first prepended Orca's entry), the user's later hooks moved to new keys with no approval, so Codex asked the user to review hooks they had already trusted. The install now carries each moved hook's recorded approval (trusted_hash and enabled) to its new key when the recorded hash still matches the hook, drops the removed copies' approvals, and logs once when it cannot prove an approval belongs to the hook.
…ership #23930 merged current main, whose #23744/#23937 moved trust writing into applyWorkspaceTrustOnThisHost (local) and the SSH relay. Resolved by taking that design and dropping what it already covers: the per-home trust writes, the launch-prep trust writes and their shared budget, and the worker-start trust diagnosis (moved to a follow-up; #23847 is now referenced, not fixed). Kept on top of it: - Local Codex trust also reaches the per-account home the launch reads: the launch env's CODEX_HOME joins getLocalCodexTrustConfigFiles, as the relay already does. - An OS error that stops a trust write names config.toml, not the atomic writer's temp file; config refusals pass through unchanged. - The WSL hook-trust relocation now logs through warnCodexConfigOnce and writes through withConfigPath, the base's single refusal and write rules.
There was a problem hiding this comment.
ℹ️ No critical issues — the new relocation logic is sound; one gap in the reverse (disable) direction.
Reviewed changes
- Added
planCodexHookTrustRelocation/relocateCodexHookTrust(newsrc/main/codex/codex-hook-trust-relocation.ts): maps handlers across an Orca rewrite of a WSL-managedhooks.jsonby object identity and carries a moved handler'strusted_hash/enabledto its new positional trust key when the recorded hash still matches, drops a removed copy's approval only when its hash proves ownership, and refuses (leaves the old key) otherwise. - Added
replaceHookTrustEntriestoconfig-toml-trust.tsto remove old keys and write carried entries in one checked, atomic write. - Wired the relocation into
installManagedHooksIntoWslRuntimeExclusivelyafterwriteCodexHooksJsonand before the app-server grant / fallback, passing Orca's new managed keys askeepKeys. - Added
hook-service-wsl-runtime-trust-relocation.test.ts(4 cases) and listed the new module inconfig/tsconfig.cli.json.
ℹ️ Disabling the Codex integration does not reverse the relocation
installManagedHooksIntoWslRuntimeExclusively now carries a user approval from a hook's old index to its new index, but the disable path refreshWslRuntimeUserHooks removes the managed entry and returns the user's hooks to their original indices without carrying their approvals back. While Orca's Codex integration is off, a user hook therefore sits at its original key with its approval still at the shifted key, so Codex asks to review a hook the user already trusted — the outcome this PR set out to avoid. Re-enabling shifts the hook back onto the shifted approval and self-corrects, so the impact is confined to the disabled period.
Technical details
# Disable path leaves relocated user approvals at stale positions
## Affected sites
- `src/main/codex/codex-hook-wsl-runtime.ts:103-109` — install relocates user approvals to the post-rewrite positions.
- `src/main/codex/codex-hook-wsl-runtime.ts:156-198` — `refreshWslRuntimeUserHooks` removes Orca's managed entries, shifting the user's hooks back, but only calls `removeWslRuntimeManagedHookTrustEntries` (managed keys only) and never relocates the user's approvals.
- `src/main/codex/codex-hook-service-implementation.ts:169-172` — the disable branch is reached via `prepareRuntimeHomeForLaunch(..., hooksEnabled: false)`.
## Required outcome
While Orca's Codex integration is off, the user's own hooks in the WSL runtime home stay trusted, exactly as they did before the enable-side relocation existed.
## Suggested approach
Call `relocateCodexHookTrust(plan.tomlPath, { sourcePath: plan.trustConfigPath, before: config.hooks ?? {}, after: nextHooks, keepKeys: [] })` in `refreshWslRuntimeUserHooks`, mirroring the install call. The plan's removed-copy loop will drop the managed approvals and its carry loop will move each surviving user approval back to its new index.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| wslDistro: context.wslDistro | ||
| }), | ||
| codexConfigFiles: getLocalCodexTrustConfigFiles, | ||
| codexConfigFiles: () => getLocalCodexTrustConfigFiles(context.env?.CODEX_HOME), |
There was a problem hiding this comment.
For a local Codex launch using a managed account, this trust write takes the account home from context.env?.CODEX_HOME. The local spawn path selects the account home separately and adds it to the process environment only after the trust write. If the incoming spawn environment has no CODEX_HOME, Orca writes trust to the shared and system homes but not the per-account home Codex opens. The removed launch-preparation write previously covered that home, so a worker can stop at Codex’s folder-trust prompt again.
Knowledge Base Used: Agent integrations and hooks
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Status update: merged the finished #23930 (head
|
…ust-ownership Main now resolves the ~/.codex trust file from the launch env's home (launchedAgentHome) instead of the process home. getLocalCodexTrustConfigFiles takes that agent home and still adds the launch's per-account CODEX_HOME between the managed and system homes, each home once (#23847).
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Added the per-account
CODEX_HOMEa launch reads togetLocalCodexTrustConfigFiles(agentHome, launchCodexHome?), kept in the hook installer's lock order and deduped withnormalizeRuntimePathForComparison, so a home named twice (trailing slash, Windows case, WSL-UNC alias) is written once. - Threaded
launchedAgentHome(context.env)andcontext.env?.CODEX_HOMEthroughapplyAgentWorkspaceTrustso local Codex trust reaches the exact home a per-account launch resolves. - Updated the
codex-home-pathsandagent-workspace-trusttests to pin the insertion and dedupe behavior; focused run passes (48 tests).
I verified the dedupe collapses to the prior [managed, ~/.codex] list when CODEX_HOME is unset or equals a listed home, and that all callers pass an agent home (none still position a Codex home as the first argument). Two prior inline threads were retired: remote-agent-trust-presets.ts and agent-workspace-trust-diagnosis.ts no longer exist after the base refactor and the worker-start-cause split.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
ELI5
Orca installs a small status hook into Codex so it can show what Codex is doing. Three things went wrong around it:
This PR fixes the first, fixes the second in the Codex homes Orca owns, and makes trust reach the Codex home a launch actually reads. It respects Orca's own per-agent switch for turning the Codex integration off, and moving hooks around never costs you an approval you already gave.
What Changed
This PR is step 2 of 2, stacked on #23930. The plan fixes how Orca edits Codex's config, trust and hooks:
config.toml, which fixes [Bug]: Codex config.toml gets a duplicate [projects."…"] table (Orca does not recognize Codex's ["projects"."…"] headers), breakingcodexstartup #22592's duplicate tables.1. Orca's hook stays off (#23289). Codex keeps an on/off switch for each hook. Orca approved its own status hook but never switched it on, and nothing re-checked it. For some users it stayed off, so Orca couldn't see Codex's status, and orchestration workers on remote hosts stalled at readiness.
/hooksscreen, the next Orca launch turns it back on. To turn it off, use Orca's per-agent Codex setting (fix(codex): turning Codex off per agent keeps its hook entry off #23667) or the global status-hooks setting.2. A copied Claude hook runs in Codex (#22434). Codex's "import from Claude Code" copied Orca's Claude hook into Codex's hook file. Orca only recognised its Codex hook script there, so the copy stayed, and Codex warned at startup about cutting its 10-second timeout to 3 seconds.
.orca/agent-hooks/folder counts as Orca's. In Orca-owned Codex homes, a copied Claude entry isn't mirrored in, its trust isn't carried over, and stale copies are removed.~/.codex/hooks.json. On macOS and Linux, Codex launched from Orca reads your real~/.codexby default (no managed account and no customCODEX_HOME), so on that default setup the startup warning continues until that pending decision is made. This PR therefore only refers to Codex warns about a 10s SessionEnd entry calling Orca’s Claude hook #22434 instead of closing it. It is fixed where Codex runs on an Orca-owned home: Windows, managed accounts, and the fallback home.3. Worker trust misses the account's own Codex home (#23847, partly).
worker-start --agent codexcould stall at Codex's "trust this folder?" screen. Main (with #23930) now writes trust once per launch on the host that runs the agent, but locally it wrote only~/.codexand Orca's shared Codex home, never the per-account home a launch actually reads.CODEX_HOMEnames, as the SSH relay already does. If a write fails at the OS level, the log names thatconfig.tomlinstead of a temporary file.Agent startup blocked: agent-trust-workspace.4. Moving hooks never costs you an approval. When Orca's install in a WSL-managed Codex home removes a copied Claude hook, or puts its own entry first, your later hooks move to new positions, and Codex keys approvals by position. Orca now carries each moved hook's approval (
trusted_hashand on/off) to its new position when the hook is unchanged, so Codex doesn't ask you to review hooks you already trusted. If it can't prove that, it leaves the approval alone and logs once.Why
enabled = falsecan't outlive the next launch. Orca's per-agent setting remains the real off switch.Linked Issue
Fixes #23289
Refs #22434 (fixed for Orca-owned Codex homes; the default macOS/Linux setup that reads your real
~/.codexis a pending decision, see above)Refs #23847 (per-account home fixed here; the worker-start cause message is a follow-up)
Visual Proof
No UI change. The changes are to Codex's hook and config files and to worker-start's failure message. Live QA ran on a separate Mac against an isolated copy of Orca built from this branch (its own home folder and settings; the machine's real
~/.codexwas checked by hash and never changed). A stand-incodexprogram showed Codex's trust screen.The folder-trust screen an unattended worker cannot answer. This PR makes trust reach the Codex home a per-account launch actually reads; explaining this screen when it still appears is the follow-up:
The per-agent Codex switch (Enabled / Disabled) that turns Orca's Codex hook off and keeps it off:
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Every test file under
src/main/startup,src/main/codex,src/main/agent-hooks,src/relay, the PTY spawn code and the orchestration worker code, plus the trust and runtime suites: 572 files, 7,235 tests pass. Two tests timed out under heavy machine load and pass on their own. The process-spawning integration suites were not run locally.New tests (each fails without its fix):
config.toml;Typecheck clean (node, cli); oxlint, oxfmt and the changed-code quality gate clean. No package or lockfile changes.
Live QA on a separate Mac (see Visual Proof) with a stand-in
codex, before the merge with fix(codex): edit Codex config.toml through one parser-checked writer (1/2) #23930; not run against a real Codex, a real SSH host, WSL or Windows. The worker-start screenshot shows the trust screen the follow-up will explain.AI Disclosure
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
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)