mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 08:03:58 +00:00
531b620a44bcda42cffadbae59808933aa8a1e11
124
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Neil
|
48e63c015f |
refactor agent config and auth services (#16195)
* refactor: split agent config and auth services * chore: repoint wsl and global-fetch guards at split module paths * fix: restore merge-base Claude CLI error propagation Drop the secret-redaction rewriting added to Claude CLI error paths in the refactor: spawn errors again reject with the original Error (preserving .code/.errno/.syscall/.stack) and command output/auth-status logs are no longer rewritten. |
||
|
Neil
|
2b1b094aa8 |
fix(cli): pair every resolved CLI with its runtime, and ratchet it (#16383)
Follow-up to #16365, which paired 8 spawn sites by hand. Hand-pairing is how the class got introduced, so close it structurally instead. cliPath is now required on CodexAppServerInvocation, `null` only for the guest-side wsl.exe launcher where a host path pairs nothing. Optional let a native builder omit it and silently fall back to pairing against a cmd.exe wrapper with no type error. Every production site already passed it; only test fixtures needed updating, which is the type doing its job. Four more sites now pair. codex-state-db-backfill-recovery spawns the same `codex app-server` subcommand #16365 fixed elsewhere. cli/handlers/account was the worst case: addAgentNodePaths prepends the *newest* version-manager bin, which is not necessarily where the CLI being launched lives, so it actively created the mismatch — pairing now runs last so the CLI's own node wins. commit-message-text-generation and skills/skill-update-run spawn resolved binaries with inherited env. cli/handlers/skills had grown its own buildNpxPath: a weaker local copy that prepended unconditionally, ignored the Windows `Path` key, and special-cased a '.' dirname. Deleted in favor of the shared helper, which checks the sibling node actually exists — the behavior change one test had pinned. The ratchet is the point: any file that resolves a CLI and spawns must reference withCliRuntimeOnPath, with a shrink-only allowlist. It caught skill-update-run, which I had missed. Its first draft required a call paren and so let dependency-injected resolvers (`resolveCommand: resolveCodexCommand`) through — verified by removing a pairing and watching it stay green, then widened until it failed. A second assertion fails on a stale allowlist entry so an exemption cannot outlive its reason. external-editor-launch stays allowlisted: it launches a GUI editor, not a Node CLI whose ABI matters. |
||
|
Neil
|
a7505fd911 |
fix(cli): spawn a version-manager CLI with its own node runtime (#16365)
* fix(cli): spawn a version-manager CLI with its own node runtime resolveCliCommand falls back to scanning every version-manager install when PATH misses, so it can hand back ~/.nvm/versions/node/v20.x/bin/codex while PATH still leads with v22. Nothing paired the binary with the runtime it was installed against, so its `#!/usr/bin/env node` shebang loaded a v20-built native module under a v22 ABI and the agent died on first require (#10932). Reproduced with a real addon rather than asserted: a CLI requiring a cpu-features build for NODE_MODULE_VERSION 115, spawned with v24 leading PATH, fails with ERR_DLOPEN_FAILED and exit 1. With the CLI's own bin directory prepended it runs clean. withCliRuntimeOnPath prepends the resolved command's directory when that directory ships a sibling node, and is a no-op otherwise — so a Homebrew or /usr/local CLI is untouched, and the WSL paths pass a bare `codex`/`claude` that is not absolute and so never matches. Host CLI resolution in the Claude login path is now lazy, keeping the WSL branch from resolving a host binary it never spawns. * fix(cli): split PATH on the delimiter we join with, pair app-server too Readiness review findings, all four addressed. withCliRuntimeOnPath chose its join delimiter from the platform option but split with the host's. Passing platform:'win32' from a posix host turned `C:\Windows;C:\Windows\System32` into `C;\Windows;C;\Windows\System32` — every drive letter torn off at its colon. Latent, since no shipped caller passes platform, but the sole win32 test was written against the corrupted value and asserted one split segment, so it green-lit the shredding. That test's other assertion was vacuous: it seeded only `Path`, so the `PATH` key it asserted absent could never exist. Deleting the whole case-dedupe block left the suite green. It now seeds both keys and asserts the full joined string; removing the block fails it. Nothing covered the wiring, and the argument choice is the easy thing to get silently wrong. Note it only diverges on win32 — on posix getSpawnArgsForWindows returns the CLI itself, so pairing the spawn command is indistinguishable there. The new test drives the win32 branch with a .cmd fixture; pairing spawnCmd or dropping the wrapper both fail it now. codex-trust-grant-host and codex-session-index-heal spawn the same `codex app-server` subcommand through runCodexAppServerSession and were left unpaired. Pair centrally there via a new optional cliPath, since invocation.command may be a cmd.exe wrapper. Pairing tests live in their own file: adding them inline pushed codex-fetcher.test.ts past the 800-line ratchet. * fix(cli): read the Windows path key the child will actually use Round-2 review finding. The read was narrower than the delete: the key was picked from exactly two spellings (`Path`, else `PATH`), while the twin dedupe removed every key whose lowercase form is `path`. A block spelling it `path` or `pATh` therefore had its value deleted without ever being read, handing the child a PATH containing only the CLI's own directory — a strictly worse outcome than not pairing at all. Win32 resolves env names case-insensitively and object order preserves block order, so the entry the child reads is the first case-insensitive match. The repo already encodes that rule in resolvePathEnvKey (src/main/pty/windows-path-segment-merge.ts); src/shared cannot import from src/main, so mirror it locally. Verified by execution across six env shapes: lowercase, mixed-case, Path-only, PATH-only, both twins, and a PATHEXT control that must not be touched. All preserve the original PATH; before the fix the first two lost it entirely. Reverting the selector fails the new test and nothing else. |
||
 Brennan Benson
|
8d08d0078d |
fix(codex): stop an unreadable legacy hooks.json from clearing managed trust (#16147)
`cleanupLegacySystemManagedHooks` reads `~/.codex/hooks.json` and, when it finds
no hooks, removes Orca's managed trust entries from the system config.toml and
deletes that home's grant-ledger record.
Since the STA-4823 read classification landed, `readHooksJsonWithRaw` reports a
genuine absence as `{ raw: null, config: {} }` and a failed read as
`{ raw: null, config: null }`. Both still fell into the same branch, so a read
that merely failed discarded hook approvals the user had already given — and the
ledger record that would have let a later pass notice.
Only the definitive-absence answer may reach the removal now.
Found while reviewing #15417 and not covered by it: that PR fixed the classifier
and this is a consumer of it that still collapsed the two answers.
The sibling `assertHooksJsonGeneration` guard in codex-real-home-hook-install.ts
has the same `existsSync ? read : null` shape and is deliberately NOT changed
here. Measured, a permission denial leaves existsSync true and throws from the
read, so that path already fails closed; a guard there could not be made to fail
in a test and would be unprovable code.
|
||
|
Brennan Benson
|
677718c4a5 |
fix(codex): stop rebuilding shared Codex state from a read that failed (STA-4823) (#15417)
* fix(codex): stop rebuilding shared Codex state from a read that failed (STA-4823) Six shared files were rebuilt, erased or reported healthy after a read that had only failed. Batch A of the STA-4606 split: every one of these is reachable and testable on the host lane, so none of them wait on the WSL work. - `config-toml-trust.ts` upsertHookTrustEntries: `existsSync` reported a locked config.toml as absent, so the base content became '' and the upsert rewrote the file from the trust entries alone — a trust-only stub, with the user's model, provider, MCP servers, approvals and comments gone. It refuses now; every hook-service caller already turns that into "trust entries could not be written. Run /hooks in Codex to approve." - `codex-trust-grant-ledger.ts`: an unreadable ledger degraded to empty and the next write persisted a file holding only the home being written, dropping every other home's grants. The write paths refuse; the read path still degrades, and a corrupt ledger is still rebuilt. - `codex-pane-account-registry.ts`: an unreadable registry erased every pane's attribution AND cached that erasure, so it survived the file recovering. The failure is no longer cached, and both write sites refuse rather than persist a registry derived from an empty stand-in. - `hooks-json-read.ts`: the read arm already separated "no hooks" from "could not read", but the `existsSync` arm in front of it returned a valid empty config for a file that could not be opened. One read now classifies both. - `config-settings-baseline.ts`: absent, unparseable and unreadable all collapsed into `null`, so the snapshot rebuilt a baseline it could not read — recording an in-Codex edit as Orca's own write, after which promotion skips it forever. - `config-sync-stall.ts`: an unreadable runtime config read as absent and the status reported `synced` while the mirror was refusing. It reports `managed-home-unavailable`, the existing reason for exactly this, rather than borrowing a source-side one and blaming the wrong path. Absent and malformed still rebuild throughout — resetting corrupt state is the intent, and conflating it with unreadable would wedge a user on a broken file. * fix(codex): close shared state read-denial gaps * fix(codex): recover oversized settings baselines * fix(codex): name the stalled managed config * fix(codex): preserve hooks after failed source reads * test(codex): correct what the denyExistence rig actually models MEASURED on both platforms: a file-permission denial leaves existsSync TRUE and fails only the content read — chmod 000 gives EACCES on macOS, icacls /deny (R) gives EPERM errno -4048 on Windows, with stat/lstat succeeding in both. The rig's docblock claimed this mode modelled that denial. It does not. What it models is the UNC / \\wsl$ transport, where an unreachable distro reports errno UNKNOWN at every level and existsSync folds it to false. The distinction decides what the D29 guard is worth: under a permission denial the pre-fix code already failed safe, because existsSync was true so it took the read branch and threw. Only a transport that lies about existence reaches the rebuild-from-empty path. No behaviour change; the comment was wrong, not the code. * test(codex): exercise live baseline read denial * fix(codex): retry pane attribution writes * fix(codex): retry reconciliation registry writes * fix(codex): report unreadable sync baselines |
||
|
Neil
|
e9e238c883 |
refactor(wsl): delete the environment-policy layer the reviews kept failing on (#16007)
* refactor(wsl): delete the environment-policy layer the reviews kept failing on
A design council (Opus, Grok, GPT-5.6-Sol) reviewed the merged runner after it
took eleven review rounds to land. All three reached the same conclusion: the
invocation half is sound, the environment/probe half is not, and every round had
been debugging the second one.
The finding that settled it, from Opus: `environmentResolved` had **54
references, all in tests and the runner itself. Not one production reader.** The
safety mechanism the strict default existed for was never wired to anything, so
all 19 degrading sites reported absence with full confidence anyway -- #9725
live at every one, under comments claiming it was handled. Two of those comments
say so out loud; I wrote them.
Root cause, in one line: every knob existed only because a failed probe was
fatal. So it no longer is.
- `allowDegradedEnvironment` and `WslGuestEnvironmentUnavailableError` are gone.
A missing login PATH is a fact in the result, not an exception. That deletes
23 opt-outs, six catch-and-remap blocks, the transient/rejected cooldown
split, `probedWithBudget`, and the 1.5x re-probe heuristic -- none of which
had a reason to exist once the case stopped throwing.
- `lane` + `allowDegradedEnvironment` collapse into `loginPath: 'none' |
'preferred'`. 19 of 23 sites passed the opt-out, and two said in comments that
they did not want the login PATH at all: the flag had become the `'none'` the
union was missing.
- The `interactive` lane is deleted. It had zero production callers and kept ~30
lines of fence plumbing alive for tests only.
Net -98 production lines; the runner itself sheds 86 for 38.
Also carries three fixes from the W3 orphan-PR sweep I had not done:
- `WSL_UTF8=1` in the runner. My relay migration deleted the only place setting
it, so wsl.exe's own error text arrived UTF-16LE and read as NUL-riddled.
A regression I introduced. Credit: #9010 (Chang-Jin-Lee).
- `GITLAB_HOST` is now named in WSLENV, so a ported self-hosted host actually
crosses into a distro-routed glab (#12557). Credit: #12558 (makoto-developer).
- The WSL skill-setup command pipes into `sh` instead of `eval "$(...)"`, whose
nested quoting produced `word unexpected (expecting "in")` (#14292). Credit:
#14785 (innocarpe).
* fix(wsl): restore the login PATH for the Codex availability lookup
loginPath:'none' on a PATH lookup reports an nvm-installed codex as absent,
which is #9725. A miss without a resolved environment is now 'could not
check', not 'not installed'.
Also hardens the guards that should have caught it:
- bashism ratchet is per-call, not per-file, and fails closed on lexer desync
- blankStringContents handles regex literals (an apostrophe in /'/g desynced
the lexer, so the scan silently found zero calls)
- windowsHide allowlist 85 -> 80, stale once the lexer parsed those files
Credit: Grok (P0), GPT-Sol (ratchet gaps).
* test(wsl): close the two ratchet gaps that let planted spawns pass
- variable-indirected wsl.exe (`const b = 'wsl.exe'; spawnProcess(b)`) is now
tracked, so the 5 files recorded only in a comment become real allowlist
entries. Three actually spawn that way; the other two never spawned wsl.exe
at all, so the prose record was wrong by three in the hiding direction.
- promisify(renamedAlias) is now resolved, so `const run = promisify(execFile)`
behind an `execFile as x` import can no longer skip windowsHide.
Each verified by planting the violation, watching it fail, restoring, watching
it pass. Credit: GPT-Sol.
* fix(source-scan): stop the regex-literal reader from eating block comments
At index 0 there is no preceding token, so a file opening with a banner
comment had its `/*` read as a pattern and swallowed to the next slash --
110k characters of preload/index.ts, in the direction that hides offenders.
Measured across the tree, old lexer vs new: worst-case over-blanking drops
from -110564 to -1116 characters, and files that desync drop from 51 to 22.
The remaining extra blanking is regex interiors, which is the intent.
Regression tests for both lexer bugs, each verified to fail with its fix
reverted. The first draft of the comment test did not bind -- it asserted on
text after the swallowed span.
* fix(wsl): restore the unverifiable signal on the two remaining probe sites
Round 2. Three call sites used to throw when the login-PATH probe failed;
the redesign rewired one (Codex) and left two reporting confident absence.
- skill-wsl-provider-detection: the script ends in `|| true`, so a lookup
without the login PATH exits 0 with empty stdout -- identical to 'nothing
installed'. Callers skip the ~/.codex and ~/.claude skill roots on an empty
list, losing an nvm-installed provider's skills.
- wsl-cli-installer: the dead catch is replaced by an explicit check. Its
`case ":$PATH:"` probe otherwise answers from the distro default PATH and
Settings states as fact that the CLI is not on PATH. Timeout is checked
first, since a timed-out run also leaves the environment unresolved.
Also narrows the regex-literal prev-token set. '!', '+', '-', '>' and '}' are
value terminators as often as operators, so postfix `n-- / 2` and JSX
`<A size={14} /> : <B` were read as patterns and their spans blanked -- 13
live JSX spans, and one swallowed execFile call that left no desync behind.
False negatives only risk a desync, and desync fails closed.
Plus: WSL_UTF8 on the probe spawn (#9010 reached the runner, not the probe),
and the allowlist header I shuffled by sorting comments along with entries.
Credit: Grok (both P1s), Opus (lexer false positives).
* docs(wsl): drop the lane comments the redesign made false
The interactive lane is gone, so 'both lanes' and the fenced-stdout note
described code that no longer exists. Also states plainly that
environmentResolved is always true under loginPath:'none' -- the field cannot
rescue a PATH lookup that was mislabelled, which is how #9725 came back.
Credit: Grok.
* fix(wsl): stop piping user scripts into the shell's stdin
The W3 migration moved hooks from `wsl.exe --exec bash -c <script>` to a
script piped into `bash -s`. Anything the script runs that reads stdin then
drains the rest of the script, bash hits EOF and exits 0, and the caller logs
success -- an orca.yaml hook of `ssh -T git@github.com || true` followed by
`pnpm install` silently never installs.
Scripts now travel in argv by default, which is what the pre-migration code
did and what --exec makes safe. `scriptDelivery: 'stdin'` stays for the one
caller that needs it: the hook-relay installer embeds a base64 JS bundle far
past any command-line limit, and reads no stdin.
A runner test already described this exact EOF hazard -- for the login shell,
not for the guest command it was itself creating.
Credit: code review.
* fix(skills): make the unverifiable check unconditional, and stop double-probing
Round 3.
- provider detection threw only on an EMPTY result, so a degraded partial hit
slipped through: `claude` visible on the default PATH via Windows interop
plus an nvm-only `codex` returns a plausible ['claude'], and the caller then
skips the ~/.codex skill roots for a provider that is installed. The
installer already got this right with an unconditional throw.
- three sites asked for 'preferred' without needing it. The GROK_HOME probe
runs its own `"$login_shell" -lc`, so the runner's probe was a second login
shell eating up to half an 8s budget; the two skill scans are
find/base64/head/printf/stat over $HOME.
- the indirection binder missed `private readonly x = 'wsl.exe'` (the
modifier was captured as the name), backtick literals, and
`spawnProcess(this.x)`. Commit
|
||
|
Neil
|
5651662494 |
fix(wsl): migrate 21 call sites onto the WSL runner (#15923)
* fix(wsl): migrate 21 call sites onto the runner, after five review rounds Rebased onto main now that the runner (#15903) has landed. 21 sites across 15 files move off ad-hoc `execFile('wsl.exe', ...)`. Allowlist 23 -> 16 on the WSL guard; 163 -> 152 on the W1 child_process guard, which moved as a consequence. Five review rounds, each finding real defects -- several introduced by the previous round's fixes: 1. Hooks ran user orca.yaml scripts under dash; probe failure fell back to the login shell, reintroducing the ~/.profile stall the runner exists to remove. 2. An unparseable probe was cached permanently, disabling every WSL feature on the distro; hooks regressed from "runs degraded" to "fails". 3. Exit 127 had no expiry; a starved 5s probe hard-failed the 10s scan behind it; a joiner burned its budget on someone else's probe. 4. The comment stripper blanked live code, so the windowsHide guard walked past a real unguarded spawn and reported the file clean; an ownership-probe timeout silently deselected the user's Claude account. 5. Verification of the guards themselves. The recurring finding -- a call answering "is this installed?" on a degraded PATH -- was eventually fixed structurally rather than per-caller: the runner refuses an unresolved guest PATH unless the caller opts in. Per-site vigilance was demonstrably not holding; 3 of 8 sites had already forgotten the analogous exit-code check. Remaining 16 files need a runner mode that does not exist: a long-lived streaming child (OAuth logins, hook relay), a synchronous caller, or a host-level flag like --status that the guest-command API cannot express. * fix(wsl): close round 5's P1s -- degrade where PATH was never needed Round 5 measured the guards by re-executing their algorithms standalone rather than reading them, and found four things. P1 -- four skill/plugin paths gained a hard dependency on the login-shell probe that they never had. They ran under a plain non-login `sh -c` on main, so a probe failure now breaks WSL skill discovery and install on exactly the distro the runner was built for: one with a slow `~/.profile`. Worse, the throw escapes before each site's own error mapping, so the UI gets a raw internal string. They degrade now, per the rule this branch already wrote down in `wsl-fish-history-cleanup.ts`. P1 -- Codex and Claude were asymmetric. Claude's five credential sites degrade; Codex's were strict, so adding a WSL Codex account failed where adding a Claude one succeeded. Three of the four are byte-equivalent to Claude sites, and their scripts read `$HOME`/`$WSL_DISTRO_NAME`, which wsl.exe supplies without a login shell. `assertWslCodexCliAvailable` stays strict on purpose -- that one really does answer "is this installed?" (#9725). P1 -- the ownership-probe timeout fix did not survive the rebase onto main. A timeout still returned "not owned", which the caller *persists*, clearing the user's account selection. P1 -- `blankStringContents` desynced on a nested template literal (`` `${`x`}` ``), leaving 116 lines of a child_process importer outside the ratchet, with 27 importers structurally at risk. Now tracks template depth. Regenerating against the fixed blanker: 70 -> 68 offenders. Also: the windowsHide vacuity check could not fail while the allowlist alone exceeded its bound -- the exact defect the sibling guard documents avoiding. It now names a file that definitely offends. * fix(wsl): close round 6 -- my blanker fix had traded a false positive for a miss Round 6 re-derived the guard's answer from a TypeScript AST instead of trusting the regex, and caught two things. P1 -- the nested-template fix I shipped in round 5 introduced a worse bug than the one it closed. Switching to "code mode" inside `${...}` without also resetting the quote at a newline meant an apostrophe in a regex literal -- `` `'${value.replace(/'/g, "'\\''")}'` `` , which is exactly the shellQuote shape all over this codebase -- inverted the lexer for the rest of the file. `claude-accounts/service.ts` went blind from line 96, hiding a REAL unguarded `spawn` at :1097: the WSL Claude managed-login path, which opens a console and steals foreground on Windows. Round 5 traded one false positive for one false negative and I did not notice, because the offender count went down. The blanker now resets non-backtick quotes at a newline (the rule stripComments already had) and tracks brace depth per interpolation. The spawn is fixed rather than allowlisted, and the count is 69 -- the number the AST predicted. P1 -- the ownership-timeout guard was dead code: it threw into its own `catch` three lines below, which returned null, which the caller persists as "not owned" and clears the user's account selection. Now a typed sentinel the catch rethrows. P2 -- `WslGuestEnvironmentUnavailableError` reached the UI verbatim from the CLI installer and the Codex availability check. Both mapped. Method note: I had been regenerating the allowlist with a Python transcription of the scanner, and the two drifted -- the same two-implementations problem this workstream keeps finding. The allowlist is now generated by running the shipped test with an empty list and taking what it reports. * fix(guards): stop patching the lexer -- make the scanner fail closed instead Round 7 proved my round-6 fix also did not work, by planting a plainly-named unguarded `spawn` in `claude-accounts/service.ts` and watching the guard pass 3/3. That is three consecutive attempts at an exact lexer, each shipping a desync that hid real calls, and each time the offender count went DOWN, which I read as progress. Round 6's diagnosis was wrong too: the culprit is the `templates` brace-depth stack, which nothing resets, not quote state. So stop trying to be exact. `blankStringContentsDesynced` reports when the lexer lost its bearings, and the guard treats that as an offender. Over-reporting is a nuisance; under-reporting is a false clean, and a false clean is what let a real console-flash spawn out of the ratchet twice. The allowlist goes 69 -> 82: the 13 extra are files whose scan cannot be trusted, now named rather than assumed fine. The planted violation is now caught. Also from round 7: - `SPAWN_CALL` missed promisified and renamed bindings, so `exec('where gemini')` (a real Windows cmd.exe spawn) and a detached `shell: true` in `cli/runtime/launch.ts` were invisible. Added execAsync/execFileAsync/ execFileCb/spawnDetached. - `BASHISM` matched `set -o pipefail` but not `set -euo pipefail`, which is the only spelling this tree uses -- so the check could not have caught the #14292 signature it exists for. Fixed, and it immediately flagged a file; that one turned out to be a comment, so the bashism scan now strips comments too. - The CLI installer error mapping my round-6 commit claimed was "both mapped" was never applied -- only the Codex side had been. Now actually mapped. * fix(guards): close the four holes round 8 found by planting violations Round 8 stopped reasoning about the guard and planted spawns into it. Four holes, none of which reading had found: - `windowsHide: false` **passed**. The check was `args.includes('windowsHide')`, a substring test. Now matches `windowsHide: true`. - A ternary first argument was silently skipped: the method-declaration filter `/^\(\s*\w+\s*[:?]/` also matches `exec(useAlt ? 'a' : 'b', …)`. Now requires a type after the colon. - Renamed bindings were not covered, despite the comment I wrote saying they were -- I had hardcoded three names. Aliases are now resolved from the import. Each is verified closed by planting it and watching the guard fail. `fork` is deliberately still unscanned. Round 8 is right that Node forwards the option, but `ForkOptions` does not declare it, so the two live sites cannot be fixed without a cast. Recorded in the verification doc rather than left as a silent gap, along with two others worth knowing: the allowlist is file-granular, so its ~18 false-positive entries carry a standing pre-approval for real regressions in those files and cannot be retired by fixing code; and `stripComments` has no desync report, so the fail-closed check is only half applied. The doc now also says how to verify a guard change: plant a violation. Every guard fix here that was verified by reading was wrong. * fix(wsl): stop preflight reporting installed CLIs as absent on a slow distro Round 9's merge blocker, and the sharpest finding of the whole workstream: the branch built to close #9725 had reopened it from the other side. `preflight-wsl-command.ts` was one of five sites without `allowDegradedEnvironment`, so a guest-PATH probe failure threw. Every consumer collapses a throw into a verdict: `isCommandAvailable` and `isCommandOnPath` catch to `false` ("not installed"), `isGhAuthenticated` and `isGlabAuthenticated` read an empty payload as "not authenticated". So a slow distro made WSL git, gh and glab read as missing. Two things made it likely rather than theoretical. The probe took two thirds of a 5s budget, leaving the command ~1667ms where main gave it the full 5s inside its own login shell -- a cold WSL VM start routinely lands in that band. And a probe timeout is cached for 30s with a re-probe threshold of 1.5x the failed budget, which a 5s caller can never clear, so every preflight command short-circuited without spawning wsl.exe at all -- and Re-check does not invalidate the cache. Fixes: preflight degrades instead of refusing, and the probe is capped at half the caller's budget and at 4s, so no caller ends up with less time than it had before the runner existed. Also fixes a real console flash found on the way: `preflight-command-exec.ts` spawns git/gh/node through `promisify(execFile)` with no `windowsHide`. Round 9 also confirmed the credential paths are now *safer* than main: all 11 account sites degrade, every destructive guest operation is still marker-gated, and main's `getOwnedManagedAuthPath` could disown an account on a 5s timeout -- which this branch turns into a failed launch instead of a destroyed selection. * fix(wsl): make "Try again" able to succeed, and test the round-9 fix Round 10 returned MERGE with one residual worth closing first. A transient probe failure left the null-resolving promise in `inFlight`, so the only way back was `retryAfter` -- and the 4s probe cap made the 1.5x budget escape unreachable, because no caller can pass more than 4s. For the full 30s window the four non-degrading sites returned their error *without spawning wsl.exe at all*, and each of those errors says "Try again". The advice was guaranteed to fail. The entry is now dropped on a transient outcome and an explicit cooldown gate replaces it, so the window alone decides. The window drops 30s -> 5s: long enough to stop a stampede, short enough that the user's next click reaches a distro that has since warmed up. Round 10 also noted the round-9 fix shipped untested, which was fair. Added: the probe-budget floor for 5s/8s/10s callers, and preflight's degrade opt-in plus its stdout/stderr-carrying rejection, which isGhAuthenticated reads off the caught error as an auth-success fallback. * test(wsl): make the probe-budget guard actually guard Round 11 caught that the regression test I added for the probe cap did not bind: it seeded the guest environment, so the probe resolved in ~0ms and the assertion read the command leg's timeout instead. Reverting the cap to the old 2/3 split left all three cases green. Dropping the seed and asserting on the probe leg fixes it -- verified by reverting the cap and watching all three fail. A regression guard that cannot fail is the shape that has cost the most in this workstream: the windowsHide guard silently passed a real unguarded spawn twice for the same reason. |
||
 Jinwoo Hong
|
8462fa72da | fix(rate-limits): support Codex 0.149 approval policy (#15823) | ||
|
Neil
|
a61b39a9a6 |
fix(runtime): stamp a runtime's own project setups as local, and report remote status about the remote (STA-4792) (#15376)
* fix(runtime): stamp a runtime's own project setups as local, and report remote status about the remote (STA-4792) Two independent frame-of-reference bugs, both from code describing one machine while labelled as another. #15366 — projectHostSetup.* persisted the caller's host id verbatim. Those `runtime:<environment-id>` ids are minted by the calling client's own pairing store, so they name a machine only relative to that client. A client sending one is addressing this runtime, and runtimes do not proxy these calls onward, so the host it names is us. Storing the client's spelling made one machine look like a different host to every other client, hid its rows from them, and defeated the (projectId, hostId) duplicate check — two laptops paired to one server each created their own setup for the same checkout. Re-spell it as `local` at the RPC boundary. Rows written earlier keep their old stamp; readers already project `local` back to `runtime:<their-id>`, so the client-visible model is unchanged and no ids are rewritten. STA-4792 defect 4 — `status --environment <name>` hardcoded app.running:false to mean "no desktop on THIS machine" while every other field in the same object described the target, including a desktopWindowStatus echoed straight from it. The result contradicted itself and read as "that run was headless" when the remote GUI was up. `app` now describes the target, keyed off the one window status that requires a live renderer, and the result names its own subject so the frame can't be misread again. The remote pid is not knowable, so it stays null. STA-4792 defect 2 gets a regression test rather than a fix: routing already made the client remote, which is what stops a Windows destination being joined to the local cwd. The test pins the exact reported invocation. * fix(status): share the remote app projection with the SSH host passthrough, and name the version gap on project host setup Two review follow-ups. The SSH host passthrough answered `app.running: true` unconditionally for the Orca host a caller reached over SSH, claiming a desktop app even for a headless `serve`. That is the same defect as the paired-server path, one transport over, so the projection moved to shared and both now answer the question the same way. `--host runtime:<id>` routes project commands to a paired server, which means a client can reach a server that predates project host setup without meaning to. That answered a raw `method_not_found`, which reads as an Orca bug rather than a version gap; the CLI now names it the way the desktop already does. Reverted a third change: making the persistence duplicate check treat `local` and `runtime:*` as one machine. That assumption holds at the RPC boundary, where a `runtime:` host means the runtime being addressed, but not in the store, which also records independent provisioning metadata for machines that are not itself. An existing test covers exactly that, and it was right. The duplicate convergence therefore stays bounded to rows written after the normalization. |
||
|
Brennan Benson
|
ddaaa4628c |
fix(codex): keep two host-lane records that a failed read used to destroy (STA-4735) (#15289)
`snapshotCodexRuntimeHookTrustProvenance` rebuilt `.orca-hook-trust-provenance.json` from the current `config.toml` on every install and refresh, including when the existing record could not be read. That record is the only thing separating a trust entry Orca wrote from one the user approved inside Codex, so rewriting it after a failed read stamps the approval as Orca-written — and `promoteCodexRuntimeHookApprovalsToSystem`, which runs earlier in the same pass and had already bailed on the same unreadable file, then skips it on every later pass too. One denied read, permanent loss. Only the unreadable case is preserved. A malformed or absent record is still rebuilt, because resetting those IS the intent; conflating the two would wedge a user on a corrupt file forever. `fileContentsEqual` returned `false` from a bare `catch`, so "I could not read this" reached `writeRuntimeAuth` as "these differ" and sent it to the unconditional write below — replacing a refresh token Codex may have rotated a moment earlier with Orca's stale copy. It now reports the difference only when the bytes were actually compared, and the caller refuses instead. `fileContentsMatchExpected`'s `!existsSync` has the same collapse but is left alone deliberately: the write it guards is `writeFileAtomicallyIfUnchanged`, whose rename-and-compare re-checks the real file and refuses on its own, so classifying there would add a guard no test can drive. Noted in a comment. The three `writeRuntimeAuthAtPath` call sites have the same overwrite shape but are all on the WSL lane, which STA-4606 restructures; refusing there without its lane bookkeeping would set a baseline for a write that never happened. |
||
|
Brennan Benson
|
0b80a773a4 |
fix(codex): stop overwriting and deleting Codex files that were merely unreadable (STA-4737) (#15287)
* fix(codex): stop overwriting and deleting Codex files that were merely unreadable (STA-4737)
Three modules shared by the host and WSL Codex lanes decided a file was absent
from a read that had only failed, and then wrote over it or removed it.
- `codex-config-mirror`: `existsSync` on the RUNTIME config.toml returned false
for a locked file exactly as for an absent one, so the mirror took the
"seed a fresh runtime config" branch and replaced the user's config wholesale.
- `config-settings-promotion`: an unreadable ~/.codex/config.toml counted as
having no promoted settings, and the write path then rebuilt the user's
canonical Codex config from Orca's runtime copy.
- `codex-home-paths`: both delete branches in `linkSystemCodexResource` remove
Orca's mirrored copy because the system resource "is not there". `existsSync`
and `systemResourceIsRegularFile`'s `catch { return false }` both reported
that for a source nobody could read, so one denied read on ~/.codex/AGENTS.md
removed the managed copy on the next launch.
`src/shared/definitive-filesystem-absence.ts` now owns the one errno allowlist —
ENOENT and ENOTDIR, with every other code including unrecognised ones treated as
indeterminate — and `host-codex-managed-home-ownership.ts` drops its private
copy rather than letting the two drift. `codex-path-observation.ts` builds the
three-valued observation on top of it.
The resource sync's two `existsSync`/`statSync` probes collapse into one
resolved stat, which answers reachability and regular-file-ness together and
closes the window between them.
`config-settings-promotion.ts` crossed its max-lines budget, so the write-target
resolution moves to its own module rather than taking a lint exemption.
Deliberately not here: the hook-service trust writes that run after a refused
mirror, and the promotion write target's own classification, which is
unreachable because it always resolves to the same file the read above already
refused. Both are noted in comments rather than half-built.
* fix(codex): preserve resource copies on indeterminate reads
|
||
  OrcaWinandOrcaWin
|
3a9f40ed70 |
fix(wsl): read machine output from a fenced login shell (#15290)
* fix(wsl): read machine output from a fenced login shell Orca runs WSL reads through the distro's *interactive* login shell so PATH matches the user's own terminal (nvm, mise and asdf only install into rc files interactive shells read). An interactive shell also runs the distro's rc/motd, and stock Ubuntu 24.04 writes its "run a command as administrator" hint to stdout -- no user customization required. Every caller parsing that stream was reading the banner as data: statPath -> "To run a command as administrator...\n\ndirectory" readPath -> banner prepended to the contents of every file read preflight -> banner prepended to `gh --version` / auth output `.trim()` cannot recover any of these, so a WSL worktree's file explorer sees no valid entry types and file reads return junk. Three call sites had independently grown their own marker to survive this (`__ORCA_AGENT_PATH__`, `ORCA_WSL_GIT_READ_ENV_V1`, and a `>/dev/null` fd dance), which is the tell that it belongs in one place. Fence the payload once, in the shared builder, and hand callers a reader that returns just their bytes. The fence carries a per-call nonce so `cat`-ing a file that happens to quote a marker is not truncated. Exit status is preserved, so the ENOENT mapping still works. wsl-git-read-environment drops its bespoke marker and parsing. * test(wsl): fence the login-shell path-lookup boundary test It asserted a raw interactive login-shell read matched an absolute path, so the distro rc banner made it fail on any stock Ubuntu. It is part of the shell-contracts CI gate, where it skips on Linux and hid the break. * docs(wsl): record the guest command-execution contract Both failure modes are silent - the command runs, exits 0, and returns the wrong bytes - so the rules need to live somewhere a reader will find them before writing the next wsl.exe call site. * fix(codex): fence the WSL Codex identity probe buildWslCodexBinaryStamp reads the login shell's stdout positionally -- path before the first newline, version after -- through an interactive login shell. On a stock Ubuntu the rc banner lands ahead of the payload, so the first newline falls inside the banner and the stamp becomes path="To run a command as administrator..." with the rest as version. Both halves are non-empty, so nothing throws: the stamp is silently wrong, and an unstable stamp reads as "the Codex binary changed" and reissues the trust grant. The identity script ends in `exec`, so it never writes a closing fence; the reader returns everything after the opening one, which is exactly this case. buildWslCodexIdentityArgs becomes buildWslCodexIdentityProbe and returns the reader with the argv so the two cannot drift apart. The other three WSL Codex commands are deliberately left unfenced: availability is exit-code only, and app-server/login hand stdout to a long-running program. * fix(wsl): harden the capture fence after review - readStdout now takes the LAST opening fence, matching the lastIndexOf the wsl-git-read-environment marker used deliberately: a login shell can echo the command text before running it, repeating the fence. - local-worktree-filesystem throws instead of falling back to raw stdout when the fence is missing. The fallback silently reinstated the bug being fixed -- statPath would return the banner as a file type and readPath would return banner+contents, with no signal. Preflight keeps its fallback; its matchers scan the whole blob and tolerate a prefix. - The exit-status test asserted only that the script CONTAINS `exit $?`, which is true for any input and never executed those lines. It now runs a real distro and asserts status 2 reaches the caller, which is what statPath's ENOENT mapping depends on. - Corrected the doc: a sed backreference has no `$`, so `--` never rewrote it. Replaced with the positional and shell-local cases that were measured to differ. * fix(wsl): stop running a login shell for filesystem reads statPath/readPath/rm run coreutils at standard paths and shell builtins. They need nothing from the user's PATH, so there was never a reason to start a login shell -- and starting one is what put the distro's rc/motd on the stdout these callers parse. Fencing that output treated the symptom. Using a plain `sh -c` removes the cause: no profile, no rc, no banner, by construction. The fence and its missing-fence error go away with it. The fence stays where it is actually needed: the three places that must run the user's shell to resolve their PATH (the preflight CLI probe, the WSL git environment probe, and the Codex identity probe). Net -12 lines. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |
||
|
OrcaWinandOrcaWin
|
6e8da1df8d |
fix(wsl): pass guest argv verbatim through --exec (#15039)
* fix(wsl): pass guest argv verbatim through --exec
`wsl.exe <...> -- <argv>` expands `$name` in every argument against the
guest environment before the guest ever runs. It does this even when no
shell is involved, so `-- /usr/bin/printf %s '$HOME'` prints /home/you.
Every WSL invocation went through that preprocessor, so scripts arrived
already rewritten: `awk '{print $2}'` lost its field reference, and a
POSIX script asking for the literal `$HOME` got the expanded path.
`escapeWslShCommandForWindows` tried to compensate by escaping `$`, but
it skipped any `$` preceded by a backslash, so a script containing `\$`
was still corrupted -- and half the call sites never applied it at all.
Route every invocation through `--exec`, which passes argv through
untouched, and delete the escaper. The direct-git path already used
`--exec`, so this is not a new compatibility dependency.
A guard test fails if the `--` form reappears anywhere in the tree.
Net -50 lines of production code.
* test(wsl): drop remaining escaped-dollar assertions
* fix(wsl): cover the --exec migration's blind spots
An audit of every wsl.exe invocation found sites the first pass missed,
including two it actively broke:
- config/scripts/wsl-git-shell-benchmark.mjs imported
escapeWslShCommandForWindows, which no longer exists, so the script
threw on startup. Its wslShellArgs helper also still used `--`; the
file already had an --exec helper, so route both call sites there.
- classifySubprocessCommand unwrapped `wsl.exe <...> -- <binary>` by
breaking on `--` alone. With every Orca spawn now on --exec it never
found the guest binary and bucketed all WSL subprocesses as plain
"wsl", losing the git/gh/glab breakdown. Break on either separator,
since foreign wsl.exe processes still use `--`.
CliSkillRuntimeSetup builds its setup command as a template literal
rather than an argv array, so no array-shaped search could see it. Its
decoder accepts both separators so commands persisted before this
change still decode.
The guard now scans config/ and tests/ as well as src/, and checks the
command-string spelling alongside the argv one — the two shapes that
have each shipped a regression. It skips comment lines so prose about
the old form stays allowed, and asserts it scanned a plausible file
count so a bad root cannot make it vacuous.
* fix(wsl): restore the guard's multi-line sensitivity
The guard matched line by line, so `'--',\s*'bash'` could not span a
newline -- and every argv array in this repo is formatted one element
per line, which is exactly the shape it exists to catch. Measured
against the pre-migration tree it caught 17 files before and 9 fewer
after. It now strips comment lines and matches the rejoined text, with
a case that pins the multi-line shape so this cannot silently return.
The program list is wider than shells now, which surfaced a false
positive: tmux takes a `--` separator followed by a program too
(`split-window ... -- cat`). Matching is scoped to files that mention
WSL rather than narrowing the list back.
Also:
- Replaced the `sed` regression case, which was vacuous. A backreference
contains no `$`, so it returned `bac` under both separators and would
have passed without the fix. The block claimed every case proved the
bug. Swapped in a positional argument and a shell local, both measured
to differ -- the positional is the shape `wslUncDirectoryExists` uses,
where `--` blanked `$1` so every existing directory probed as missing.
- windows-shell-args.test.ts derived its expected argv from
buildWslExecArgs, the helper under test, so six assertions would still
pass if it regressed to `--`. Spelled the expectation out.
- Dropped two comments citing the removed `--` behavior as rationale.
---------
Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
|
||
|
Brennan Benson
|
5652fb7469 |
fix(codex): stop resuming a session under the wrong account when a sessions tree is locked (#15093)
* fix(codex): stop resuming a session under the wrong account when a sessions tree is locked Two probes reported "this rollout is not bridged here" for any filesystem error, not just a genuine absence: - codex-session-resume-home.ts used existsSync on each ranked home's sessions directory. existsSync returns false on EBUSY/EPERM, so a briefly locked tree made the scan continue to the next ranked home — and the winning home becomes the resumed pane's CODEX_HOME, so it picks the account. - codex-legacy-session-resume.ts caught every lstat failure for the selected account's candidate rollout and returned null, after which the caller kept the source per-account home. Either way the session resumed under a different account's credentials while the UI still showed the selected one. Only a definitive ENOENT/ENOTDIR now means "not bridged here". Any other error raises the typed temporary-unavailability refusal the ownership gate already uses, which both PTY paths convert into a clean abort before spawn. The refusal is scoped to the SELECTED account's home. An unreadable home that is not the selected account cannot cause a wrong-account resume, so it is still skipped rather than stranding the user. These are pre-existing and independent of the STA-4422 ownership-marker failure: they route to another account today with the gate uninvolved. Fixes STA-4607 * fix(codex): refuse a resume when the selected sessions tree is locked mid-listing Review found the first pass incomplete in two places, both the same category error one layer further down. The preliminary statSync on the selected sessions root was guarded, but the real directory read happens later and listCodexSessionRolloutFilesIncrementally swallows every opendir error. A lock held during enumeration — where nearly all the I/O is, and so the far more likely case — still yielded nothing for the selected account and fell through to another one. The listing now reports directory errors through its existing onDirectoryError hook, and a non-definitive error anywhere under the selected sessions root raises the typed refusal. Non-selected homes and definitive absence still skip. Separately, index.ts wrapped prepareLegacySharedCodexSessionResume in a blanket catch and fell back to the source home, so the typed refusal from the candidate lstat was swallowed and the resume still ran under the peer account's credentials. That catch now rethrows ManagedCodexHomeTemporarilyUnavailableError while ordinary migration failures keep warning and falling back, since a genuine migration failure legitimately should not block a resume. A typed refusal is only as strong as the narrowest catch between the throw and the spawn. The frames between both throw sites and the PTY spawn were audited: findTrustedCodexSessionResume, resolveCodexSessionResumeProvenance and prepareCodexSessionResume have no catches, and the PTY layer already maps the typed error before spawn. Both fixes are mutation-checked. Disabling the listing hook makes the resume resolve to the other account again; the index.ts rethrow is covered only by typecheck, because src/main/index.ts has no unit-test entry point in this repo. * test(codex): pin nested-directory lock coverage; document the resume repin contract Review flagged the listing guard as matching only the exact sessions root, so a nested dated directory would leak. It does not — the guard keys on the root being listed, not the failing directory — but nothing pinned that. Added a test that faults only sessions/2026/07/20 while the root stats fine; it fails under mutation alongside the root case. Also documented why the index.ts rethrow cannot fire today. That launch path pins CODEX_HOME to the account that owns the rollout and deliberately refuses to repin onto whichever account is selected now (#10793), so it does not wire the selected-home resolver. The branch stays as a contract guard so the blanket catch below can never silently swallow a typed refusal if that changes. |
||
|
Brennan Benson
|
7ae6aedc02 |
fix(codex): stop a transient filesystem error from logging out the active account (#15046)
* fix(codex): stop a transient filesystem error from logging out the active account A single unreadable read of a managed Codex home's ownership marker cleared the user's active account selection, permanently. On Windows any exclusive lock — Defender real-time scanning, a backup agent, a sync client — makes every read of that marker fail with EBUSY, and the background rate-limit poll runs every 15 minutes plus once at every app start. Root cause: the ownership gate answered two very different questions through one channel. "This home is not ours" (a successful observation that failed a trust check) and "we could not read it" both surfaced as a throw, which the caller flattened to null, which three call sites took as proof the home was untrustworthy and wrote activeCodexManagedAccountId: null. Refusing to USE an unverified home is correct. Erasing the user's account selection because a file was briefly locked is not. The gate now returns a tri-state verdict. `untrusted` comes only from a proven trust failure or a definitive ENOENT/ENOTDIR where absence is itself the verdict; every other filesystem exception is `indeterminate`. Only `untrusted` may touch persisted state. Because `null` already meant "fall through to the system default" on both the launch and poll paths, not-clearing on its own would have run a DIFFERENT account behind a UI still showing the selected one. So the refusal needed real channels rather than a sentinel: - the poll returns an explicit skip; returning null would not have skipped at all, since the fetcher maps null to ~/.codex and would have spawned a token-refreshing app-server inside the user's real credential home - pane launch throws a typed temporary-unavailability error that both PTY implementations convert into a clean refusal with a retry message, including the re-resolution after the async auth-readiness wait - automatic session resume resolves the selected home eagerly, so an unreadable account can no longer be silently replaced by another one in the ranking - config-sync status reports a distinct managed-home-unavailable stall instead of "synced", with a bounded renderer retry so it clears on its own Also fixes the ticket's second symptom. The status bar's Sign in button called a re-auth that captured the selection before login and restored it after, so re-authenticating a deselected account restored `null` — a successful login that left the account inactive, with no success toast to distinguish it from failure. It now activates the account it just signed in, but only when the pre-login selection was empty, so it cannot silently switch accounts for multi-account users, and it runs the same restart prompt an explicit switch does. No retry or grace window inside the synchronous gate: it runs on the Electron main process in a loop over accounts, so a sleep there would freeze the UI. Recovery is simply the next readable evaluation. The WSL lane has the same class of defect, including one path that deletes a credential mirror. It is pre-existing, unreachable from these host code paths, and deliberately left for its own change; the host clearing sites cannot reach a WSL account because getSelfContainedManagedHostAccount excludes them. Fixes STA-4422 * test(codex): cover pending reset home ownership |
||
|
OrcaWinandOrcaWin
|
02ba70a847 |
fix(agent-hooks): make the Windows managed hook survive Claude-hooks-compat consumers (#14825)
* fix(agent-hooks): make the Windows managed hook survive Claude-hooks-compat consumers `~/.claude/settings.json` is not read only by Claude Code. Third-party Claude-hooks-compat layers (cursor-agent, Devin) import the same file and reimplement hook execution, so Orca's entry has to survive consumers that support strictly less than the documented schema. Three separate defects came from assuming otherwise. 1. The entry depended on `args`, which a compat consumer ignores. `args` is valid Claude Code syntax, but cursor-agent spawns `command` alone -- so `conhost.exe` ran bare, which opens an interactive console that never closes. Hook payloads were typed into those stranded shells (#14815). The entry is now one self-contained `command` string that depends on nothing optional. 2. `conhost.exe --headless` never relayed anything. It implements the ConPTY server protocol, not a generic no-window wrapper: it does not wait for the hosted process and relays neither exit code nor stdout. Measured directly -- `conhost --headless cmd /c "echo X& exit /b 42"` yields empty stdout and no exit code, while the replacement returns both and waits. So every hook was fire-and-forget, and whatever it printed was discarded. Replaced with `-WindowStyle Hidden`, which suppresses the window and keeps wait/exit-code/stdout intact. 3. The hook never wrote anything to stdout. Guards exited silently and curl's output went to nul. Claude Code documents empty stdout as "no decision", but cursor-agent treats PreToolUse as a permission gate, fails to parse empty stdout as JSON, and blocks the tool call -- so every shell command in every cursor-agent session on Windows failed (#14818). The script now writes `{}` first, on both the Windows and POSIX branches, which is documented to be identical to writing nothing for real Claude Code. Gemini and Antigravity already did this. Defects 2 and 3 are causally linked: `{}` cannot reach any consumer while conhost is swallowing stdout, so neither fix works without the other. Also fixed while establishing the contract: - The launcher's own missing-script fallback returned empty stdout, reproducing #14818 whenever `~/.orca` was cleaned or an install was half-finished. It now emits `{}` too. - PowerShell serializes progress records to stderr as CLIXML when stderr is redirected; a consumer merging stderr into stdout would see those bytes before the JSON. Every encoded payload now silences progress. - `runtime-home-hook-command.ts` built its own launcher without window suppression -- exactly the drift #14815 asks to prevent. All launcher construction now goes through `windows-powershell-hook-launcher.ts`, so the switch list cannot be present in one installer and missing in another. - Renamed `usesWindowsHeadlessHook` to `usesWindowsPowerShellLauncher`; nothing is headless anymore, and the flag selects a launcher. Testing: the new regression test asserts the effect a consumer observes -- it runs the exact `command` string from settings.json through both cmd.exe and Git Bash, across the guard-exit, reached-curl, and missing-script paths, and parses stdout. Verified it fails when `conhost --headless` is reintroduced. The previous tests all asserted installer intent, which is why they passed through all three defects. * fix(agent-hooks): close hook launcher review gaps --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |
||
|
Neil
|
9367169888 |
refactor(tests): split every oversized test file off the max-lines suppression list (#14728)
* refactor(tests): split oversized test files off the max-lines suppression list Every `*.test.ts`/`*.spec.ts` that carried an `eslint/oxlint-disable max-lines` directive is now split into focused, behavior-scoped suites that fit the 800-line test budget, with shared setup extracted into co-located `*-test-harness.ts` / `*-test-fixtures.ts` modules (300-line budget). 83 files became ~930; the largest output is 797 effective lines. `orca-runtime.test.ts` is intentionally untouched. Test bodies were moved by scripted line-range slicing rather than retyped, so assertions are byte-identical. The only permitted body edits were mechanical rebinding where a shared value moved into a harness (e.g. `tmpHome` -> `homes.tmpHome`). Registries that enumerate test files were updated in lockstep: - config/max-lines-baseline.txt: pruned 341 -> 258 entries (all 83 removed). - config/reliability-gates.jsonc: 33 gates repointed at the split files, with assertionRefs split per file where a gate's coverage now spans several. - .github/workflows/pr.yml: the real-zsh lane now lists the 4 split files that actually exercise zsh, so they keep running in the dedicated shell lane. Also renamed agent-hooks `server-test-fixtures.ts` to `server.test-fixtures.ts` so the global-fetch call-site audit keeps skipping it, and added `.js` extensions to the CLI suites' dynamic harness imports (node16 resolution) to unbreak `build:cli`. Verification: full suite 52,449 passing vs 52,448 at baseline with zero assertions lost; `pnpm lint`, `pnpm typecheck`, and `pnpm build:cli` all exit 0; the terminal-pane e2e spec runs 31/31 headless. * refactor(tests): split hook-idle arbitration suite that oxfmt pushed over budget The pre-commit oxfmt pass reflowed pty-connection-hook-idle-arbitration.test.ts to 811 effective lines, 11 over the test budget. Split the hook-completion side effect and replacement-agent veto cases into their own suite; both files now sit well under the cap and the 15 tests are unchanged. * test: port upstream test changes into the split files after rebase Rebasing onto main surfaced 27 tests that main had added to files this branch deleted, plus edits to tests that had already moved. Taking the deletion side of those modify/delete conflicts would have dropped that coverage silently, so each upstream change is ported into the split file that now owns the behavior — for example main's six orchestration mailbox tests land across orchestration-runs, -send, and -check. Also repoints `orchestration.notification-mailbox-consistency`, a gate main added after this branch's gate remap, at those same three split files, and re-prunes the max-lines baseline against main's (257 entries). Verified: all 27 upstream test titles present; full suite 52,761 passing with the only diff vs baseline being 12 tests main itself removed and 3 that moved from skipped to passing; lint and typecheck exit 0. * fix(test): flush pending continuations before tearing down terminal test globals CI shard 5/16 failed on both Node 24 and 26 with `ReferenceError: window is not defined` from pty-connection.ts, surfacing through pty-connection-daemon-snapshot-replay.test.ts. The reattach/settle chains `await` a real promise and then touch `window.api`. Under fake timers those continuations cannot run, so they only become schedulable once restoreTerminalTestGlobals() switches back to real timers — which previously happened immediately before `delete globalThis.window`, so a late continuation threw and failed the whole file. Flush async ticks in that window instead. This is latent in the source rather than new: the pre-split 25k-line file kept running other tests after these, which gave the chains time to settle before teardown. Splitting the file moved teardown directly behind them. * fix(test): keep an inert window after terminal test teardown instead of deleting it The async-tick flush was not enough: the reattach/settle chain can resolve after teardown regardless of how long we drain, so CI shard 5/16 still failed with `ReferenceError: window is not defined` from pty-connection.ts. A real renderer never loses `window`, so deleting it was the artificial part. Swap in an inert proxy whose properties resolve to callables and whose calls resolve to undefined, making a late `window.api.pty.*` call a harmless no-op. The next test replaces it wholesale via installTerminalTestGlobals(), and no test asserts that `window` is absent. |
||
|
Neil
|
77f23b013f |
refactor(shared): drop the shared/types barrel and import from the real modules (#14447)
#14397 split `shared/types.ts` into 46 per-domain modules but kept the path as a re-export barrel so the import sites did not have to change. This removes the barrel: every consumer now imports from the module that actually declares the type, and `src/shared/types.ts` is deleted. Barrels hide where a type lives, make every consumer look like it depends on the whole domain, and let an unrelated edit invalidate a module that ~2,000 files transitively import. 2,323 import declarations across 2,321 files. Rewritten mechanically: each specifier was resolved to an absolute path via the TypeScript AST and recomputed, rather than string-substituted, so alias forms (`@/../../shared/ types`) and per-specifier `type` modifiers survive. Four cases the mechanical pass had to handle, each found by a gate rather than by reading the diff: - Modules inside `src/shared` import the barrel as `./types`, not `shared/types`. A pre-filter on the latter string skipped 176 of them and left imports dangling at a deleted file, which surfaced as confusing `Property 'x' is optional in type 'Repo' but required in Pick<Repo, ...>` errors rather than "module not found". - The barrel RENAMED one type on the way through (`WorkspaceSource as WorkspaceCreateTelemetrySource`), so the original name in the owning module has to be re-aliased at each consumer. - Three test files put `;(globalThis as ...)` on the line after the import. TypeScript parses that `;` as the import statement's terminator, so replacing through `statement.getEnd()` deletes it and breaks ASI. The rewrite now stops at the module specifier. - A file that already imported directly from a module got a SECOND import from it, because the barrel re-exported those same names — which trips `import/no-duplicates` under `--deny-warnings`. A post-pass merges declarations sharing a specifier and type-only-ness; the `import type` plus `import` pair from one module is left alone, since that form is allowed. Splitting one barrel import into several genuinely adds lines, which pushed `terminal-layout-pty-ownership.ts` to 301 counted lines: its 107-character import must wrap, and neither local type collapses onto one line (101 and 116 characters). Rather than contort a type declaration to fit a line budget, `collectLeafIds` and `pruneLeaves` move to `terminal-pane-layout-tree.ts` — they are pure structural operations on the layout tree and independent of PTY ownership. `visible-worktrees.ts` similarly loses its own mini-barrel re-export of `isDefaultBranchWorkspace`, with the four real consumers repointed at the declaring module. No `max-lines` bypass added. Verified: cold `tsc --noEmit` green on node, cli, and web (buildinfo deleted first — these projects are `composite: true` and reuse stale caches); the full `pnpm lint` green, not just bare oxlint — the narrower local check is what let the duplicate imports reach CI; max-lines ratchet OK at 344. |
||
|
Brennan Benson
|
537864a248 |
Fix Codex hook trust before manual shell launches (#14326)
* fix codex hook trust before shell launch * fix packaged cli preflight dependency * fix codex shell preflight safety * fix Codex shell preflight settings and startup safety |
||
 NeilandOrca
|
5ea7df1a5b |
fix(terminal): make DECSET 2031 subscriptions silent (#13904)
fish arms `CSI ?2031h` before painting each prompt and withdraws it when it hands the tty to a child — a ~1ms window. Orca answered that subscribe with `CSI ?997;Nn` across a 1-3ms renderer hop, so the reply landed after the withdrawal and was read as stdin by the next child, corrupting `brew`/`npx` `[y/N]` prompts. The reply is not stale by Orca's own view when written (measured staleReplies: 0), so no suppress-the-stale-reply scheme can close this — the information needed to suppress does not exist yet. Nothing asked for the reply either. The Contour spec says a terminal "should only send out the DSR when the palette has been updated"; Ghostty (Termio.zig:729 — force=true reachable only from the ?996n DSR), iTerm2 (VT100Terminal.m:995 — flag only) and xterm.js (InputHandler.ts:2035 — flag only) all emit nothing on the DECSET. So stop entering the race: record the subscription, answer nothing. Of 17 real programs measured under a pty, only fish, tmux, claude and opencode subscribe; none block on a reply, and answering produces one redundant palette re-query and zero rendering difference. tmux is the only one that sends `?996n`, which Orca still answers. - Subscribes are record-only at all four emitters (live scan, hidden-gate fact, parked byte watcher, parked responder — the last is deleted, it only replied). - `?996n` answers, the subscription registry, and the theme-flip push are unchanged. `paneLastThemeMode` is still seeded at subscribe so the next appearance re-apply is not read as a flip. - Replay grammar carries `?2031l` alongside `?2031h`, so a late-attaching remote client no longer registers a subscription the TUI already retired. Also closes fish-integration gaps found alongside: `unset` (which fish lacks) becomes `set -e` on paths parsed by the client's login shell, `config.fish` is parsed for agent-home detection, and bracketed-paste startup delivery is made consistent across local/daemon/relay. Regression test drives real fish 4.7.1 under node-pty and asserts on what the child process reads; it fails against pre-fix code with the exact payload from the issue. CI installs fish 4 and fails loudly rather than skipping. Closes #9993 Co-authored-by: Orca <help@stably.ai> |
||
|
Neil
|
991a3fe963 |
chore(lint): update oxlint to 1.77 and enable no-op cleanup rules (#13901)
Enable eleven oxlint rules that simplify code without changing behavior, and fix
every existing violation. Each candidate was gated on measured cost rather than
assumption, so rules that regressed runtime performance or type checking were
dropped instead of suppressed.
typescript/no-redundant-type-constituents is the largest addition: 113 sites, no
autofix. Dead constituents are deleted. Where the redundant literal existed to
document intent (`string | 'all'`), it is preserved as `(string & {})`, which
keeps the autocomplete hint the original code was reaching for instead of
flattening it away. The rule also caught a broken import —
remote-shared-control-retirement-probe.ts pulled RuntimeStatus from
src/shared/types, which does not export it, so the type silently degraded to
`any`; no tsconfig covers that file, so tsc never saw it.
oxlint stays at 1.77.0 rather than 1.78.0 because .npmrc sets
minimum-release-age=4320 and 1.78.0 is younger than that window.
Rules evaluated and rejected, with what disqualified each:
- prefer-string-raw: String.raw is a runtime call, not a literal (184x slower)
- prefer-string-replace-all: 26% slower
- text-encoding-identifier-case: ~5% slower, reproducible
- prefer-spread: [...str] is 110% slower than split('') and differs on surrogates
- no-implicit-coercion: `!!x` narrows types and `Boolean(x)` does not (22 tsc errors)
- prefer-arrow-callback: arrows are not constructible, breaking `new` on mocks
- object-shorthand: rewrites source text asserted by a tracked reliability gate
- switch-case-braces: pushes ten files past max-lines, which cannot be suppressed
- no-useless-switch-case: drops `case undefined:` that switch-exhaustiveness-check needs
- arrow-body-style: 115 violations have no fix, and it breaks max-lines
- newline-after-import: false-positives on the leading-semicolon ASI idiom
electron-vite-output-contract asserted on the literal
Object.prototype.hasOwnProperty.call text; retarget it to Object.hasOwn, which
rejects inherited keys identically.
|
||
|
NeilandOrca
|
a639bd355e |
[perf-remote] perf(codex): skip migration scans for nonshared reattaches (#13654)
* perf(codex): skip migration scans for nonshared reattaches * fix(codex): require provenance to skip migration scans and bound ignored launches Drops the exit-match escape hatch that skipped a scan without route provenance, and ages out ignored reattach launches on the existing 60s/256 lease policy so a lost exit can no longer pin an active launch (and its completion marker) for the process lifetime. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai> |
||
|
Brennan Benson
|
7250fde148 |
fix(codex): repair hook trust before restored shells reconnect (#13745)
* fix(codex): reconcile hooks for restored shells * docs: move restored shell validation report * perf(codex): gate retained pane inventory |
||
|
Brennan Benson
|
2ee43bfc0d |
fix(agent-hooks): refresh existing Orca launchers when agent CLIs are unavailable (#13378)
* fix(agent-hooks): refresh existing shared hook scripts when the CLI is no longer detected A CLI that falls off PATH (moved npm prefix, relocated shim) keeps its user-wide config invoking Orca's launcher script under ~/.orca/agent-hooks, but the presence gate skips install() with no removal — freezing the script at whatever Orca generated last. Anyone in that state kept the pre-#11568 more.com-leaking .cmd forever, because no launcher script is ever deleted and Windows startup deliberately skips shell PATH hydration. Reconcile before gating: every existing shared launcher/statusline script is rewritten to the current template on each install pass. Creating scripts stays behind the presence gate — an existing file is proof of a prior install; a missing one means the gate did its job. Amp and Hermes are deliberately absent: they write provider-native plugin code with its own install lifecycle, not shared launchers. - refreshManagedScriptIfPresent() in installer-utils (no-op unless the file exists) - refreshManagedScripts() on the 11 launcher-writing services (openclaude via the shared Claude class) - reconcile pass in installManagedAgentHooks before presence detection, filtered by the agents option, best-effort per agent - coverage gate: a launcher written to ~/.orca/agent-hooks without a matching refresher entry fails the suite, in both directions * perf(agent-hooks): refresh launchers off the main thread * test(agent-hooks): keep refresh mode assertion POSIX-only |
||
 Jinwoo HongandJinwoo-H
|
4b5157b147 |
fix(codex): bound state DB recovery retries (#13109)
Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> |
||
|
Brennan Benson
|
38275c2aa2 |
fix(codex): publish Windows system-default sessions (#12611)
* fix(codex): publish Windows system-default sessions * fix(codex): close launch-scheduling races in session migration scheduler * fix(codex): bound repeated session migration audits * fix(codex): preserve delayed session publication passes * fix(codex): bound failed session audit events * fix(codex): fence stale session migration markers * chore: preserve main formatting after merge * perf(codex): bound launch session migration scans * perf(codex): preserve coalesced migration scope * fix(codex): preserve scheduled migration recovery * fix(codex): preserve session migration recovery * fix(codex): close session migration launch races * fix(codex): harden session migration completion |
||
|
Brennan Benson
|
f0443c326a |
fix(codex): recover interrupted state DB backfills (#12617)
* fix(codex): recover interrupted state DB backfills * fix(codex): detect mixed-case backfill timeout * fix(codex): harden backfill recovery review findings * fix(codex): keep process identity retries safe |
||
|
Brennan Benson
|
4c49989c2e |
refactor(codex): delete the unreachable managed shared-mirror lane (#12614)
PR 9501 shipped real-home routing for the host system default, and the env override that could turn it back off was never a shipped control. The managed-account half of the shared runtime mirror has been unreachable since: every host account routes to its own self-contained CODEX_HOME before that code runs. Delete the flag module and its env plumbing plus the managed branch of syncForCurrentSelection and the six helpers only it called. The three lanes that still use the shared mirror -- Windows, a custom CODEX_HOME, and a hook-lane gate that reports unusable -- are untouched, as are every legacy migration and the WSL read-back helpers. |
||
|
Brennan Benson
|
dc5c5a89ba |
fix(codex): block launches with unproven runtime auth (#12490)
* fix(codex): block launches with unproven runtime auth * fix(codex): reconcile shared auth before resume |
||
|
Brennan Benson
|
402e49203d | fix(codex): keep persistent panes logged in after home routing (#11720) | ||
|
129d8b32bb |
fix(codex): trust extended-length resume paths (#10337)
* fix(codex): trust extended-length resume paths * test(codex): cover both sides of extended-length resume folding The provenance fix folds the extended drive spelling on the rollout path and the trusted sessions root, and runs per entry in the legacy id scan. Only the rollout-path side had coverage, so pin the rest: - normal-form rollout under an extended-length trusted home (root side) - compressed .zst sibling derived from an extended-length persisted path, where a folded comparison copy could leak into the returned path - legacy id-scan entries: extended-length accepted, device namespace refused All three fail with the production change reverted. * docs(codex): state accurately what the resume path fold rejects The helper's comment called the rejected spellings "arbitrary device namespaces", but \?\UNC\ is the extended-length form of \server\share\ — a network share, not a device. Reading it as a security decision hides that UNC is simply not folded yet, so say what is actually excluded. Also trim the two new test comments to one line to match the file. * fix(codex): reject rollout alternate data streams * fix(codex): preserve resume provenance guard --------- Co-authored-by: OrcaWin <alpha-eng@stably.ai> Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |
||
|
Brennan Benson
|
50f46889d9 |
fix(ai-vault): resume a bridged Codex session under the selected account's home (#11224)
* fix(ai-vault): resume a bridged Codex session under the selected account's home The account session bridge hardlinks every rollout into each per-account CODEX_HOME, and vault dedup keeps the lexicographically-smallest alias, so Resume could pin an inline CODEX_HOME naming a peer account — running the session under that account's auth.json and quota. At resume time the owning host now substitutes the selected account's home when it holds the same rollout at the same sessions-relative path, declining on any uncertainty so resume degrades to today's behavior instead of failing. * fix(ai-vault): repin dropped sessions without a cwd instead of resuming under the wrong account The drag payload only carried sessionCwd when session.cwd was truthy, so a null-cwd codex session dropped onto a pane silently fell back to the prebuilt command - which pins the wrong account's CODEX_HOME, the exact defect this PR eliminates on the other resume surfaces. - Serializer always sends sessionCwd (null when the session has no cwd), so absence now only means an older-serializer payload. - The repin rebuild accepts a null cwd (the builders already omit the cd prefix), matching the sidebar Resume/Copy paths which repin regardless of cwd. - An unrepinnable payload (absent sessionCwd) now fails loudly with guidance instead of silently resuming under the wrong account's home. |
||
|
Neil
|
badf91101b |
fix(quality): enforce performance-safe lint baseline (#11074)
* fix(quality): clear safe existing lint findings * fix(quality): keep lint cleanup allocation-free * fix(quality): enforce performance-safe baseline * test(terminal): drain deferred confirmation cleanup |
||
|
Brennan Benson
|
ee7ec43149 |
fix(codex): keep a host account switch inside the host lane (#10992)
* fix(codex): keep a host account switch inside the host lane markLiveCodexSessionsForRestart walked every tab's PTYs and carded any pane whose foreground looked like Codex. There was no lane check anywhere in that path, so a host account switch raised a restart notice on live SSH/relay panes — and a notice mutes the pane, so the user's remote terminal went deaf. The notice was provably spurious: a remote spawn carries a connectionId, so isDaemonHostSpawn is false and no CODEX_HOME is ever injected. The remote Codex uses the remote machine's own credentials; a local selection cannot reach it. Scope marking by lane instead. A pane's lane is (machine, runtime): `host`, `wsl:<distro>`, `env:<id>` for a relay environment, or an SSH connection that no managed selection can name. A switch made while a runtime environment is active still cards that environment's panes, which is the case that made the old "mark everything" behaviour look right. WSL was the same defect, not a separate one. A Windows run saw a WSL pane correctly escape a host switch, but only because its foreground read `wsl.exe`, which fails the Codex-foreground test — the Win32 process table cannot see into a WSL2 VM. That is incidental: `codex`, `node` and `python3` foregrounds are all eligible today, so a WSL pane that surfaces one (WSL1 pico-processes are in Win32_Process) would be carded by a host switch. The lane is now what decides. Also stop queueing remote and SSH panes into the bind-driven stale sweep at all. recordCodexPaneAccountForSpawn bails on anything that is not a daemon host spawn, so listStalePanes can never report one stale, yet each pane still spent every rung on a 15s-timeout remote RPC — ~75s per pane since the ladder widened to five rungs. The lane vocabulary moves to shared/ so the renderer keys panes exactly as a launch does rather than growing a third copy of the rules. Refs #10757 * fix(codex): key a WSL pane by the distro its launch actually used The lane guard derived a pane's WSL distro from the workspace UNC path alone. A launch does not: pty.ts hands getCodexSelectionTargetForPty a third argument, the resolved runtime's distro, so a wsl.exe pane on an ordinary Windows-path worktree launches under `wsl:Ubuntu`. The renderer keyed that same pane `wsl:__default__`, so the Ubuntu switch never reached it — the pane kept the old account with no notice, which is #10757 returning by a new route on the exact platform the issue was reported from. Resolve the distro the way the spawn does: the project execution runtime first, then terminalWindowsWslDistro. Both are already in renderer state. Also match a distro-less WSL switch against the whole `wsl:` family. Two mutations reach the renderer as `{runtime:'wsl', wslDistro:null}` while writing concrete distro slots: selecting the system default clears EVERY wsl slot (setSelectedCodexAccountIdForTarget), and `add` stores the distro it discovered from the machine. Keying those to `__default__` missed the very panes they re-pointed. The residual cost is over-marking a sibling distro after an add, bounded to this machine's WSL panes and far cheaper than a stranded pane. An owner-less remote pane colliding with the host lane was untested — that collision is what would mute a working remote terminal, so pin the disjointness rather than the literal key. Refs #10757 * fix(codex): resolve a pane's lane the way its launch resolved it Three more places where the renderer's lane and the launch's lane disagreed. Each disagreement is silent: too narrow and a stranded pane never gets its notice (#10757 returns), too wide and a healthy pane is muted, because a notice makes onData drop every keystroke. Shell: main runs the request through resolveLocalWindowsTerminalRuntimeOptions, so an unset shellOverride still lands on WSL when that is the Windows default. Reading tab.shellOverride alone called such a pane `host` — a host switch would have muted a working WSL terminal. Gate on the renderer platform, as pty.ts gates on process.platform. Cwd: a terminal's startup cwd is deliberately not constrained to the worktree (resolveTerminalStartupCwd, #7685), and main keys the lane off that cwd. Follow it through the same shared call instead of reading the workspace root, so a pane split after `cd \\wsl.localhost\...` is keyed where it actually runs. The comment claiming a pane can never start outside its workspace was simply wrong. Family match: narrow the previous commit. setSelectedCodexAccountIdForTarget only nulls every WSL slot when the account is null AND no distro is named; any other write lands in one slot. So claim the family only when the change actually cleared them all, and let `add` pass the created account's concrete target rather than the row's "WSL default". Both call sites already knew which case they were in. Refs #10757 * fix(codex): derive the pane's project runtime the way main does The previous commit reached for getLocalProjectExecutionRuntimeContext as a stand-in for main's resolveLocalProjectRuntimeForWorktreeId. They are not the same function, and the differences both produce wrong lanes: - It falls back to `state.activeRepoId` when the worktree is not a git worktree, so a folder-workspace pane inherited whichever repo happened to be selected. That is not a property of the pane at all — the lane moved when the sidebar selection moved. On a WSL project it both muted a healthy host pane and hid the notice a host switch owed it. - It synthesizes a runtime from `inherit-global` where main returns undefined, and its host branch rewrites an explicit `wsl.exe` to powershell.exe, keying a live WSL pane `host`. Walk repo -> project directly instead, which is what resolveLocalProjectRuntimeForRepo does, and use it only to supply a distro — never to downgrade a shell. That also drops the throwing call out of this path entirely; the lane runs outside scanCodexPanes' inspection guard, so a throw there would have lost the notice for every pane in the batch, not just one. Also find the added account by diffing the roster. Reading it back through the row's active id returns null once two distro slots are filled, which sent the notice to `wsl:__default__` while `add` had written a concrete distro. Refs #10757 * fix(codex): key the lane off the runtime the renderer actually shipped Reverses the project-runtime half of the previous commit. That commit assumed main resolved the project runtime itself, so it re-derived one by hand. It does not: for a local pane the RENDERER computes it with getLocalProjectExecutionRuntimeContext and ships it with the spawn (pty-connection.ts), and pty.ts feeds that straight to getCodexSelectionTargetForPty. So the helper is not an approximation to be improved on — it is the launch. The hand walk dropped the global Windows runtime default, which is what turns an `inherit-global` project preference into WSL. A user who set their runtime default to WSL but left terminalWindowsShell alone would have had every live WSL pane keyed `host`: muted by a host switch, and missed by their own. It also disagreed on folder workspaces, where the launch really does resolve through the active repo. Keep the repair-required early return: that call throws, and it sits outside the scan's per-pane failure guard, so a throw would lose the notice for every pane in the batch rather than one. Separately, floating terminals have no workspace root, so their startup cwd is used verbatim (resolveTerminalStartupCwdForWorkspace). Resolving one against a root that does not exist yielded no cwd at all, keying a floating Codex pane on a WSL filesystem as `host`. Read its cwd directly. Require exactly one new account before trusting the roster diff — an unloaded prior roster makes every account look new, and Add Account is not gated on it. Refs #10757 * fix(codex): stop claiming a floating-terminal cwd the tab never has The floating-terminal branch read tab.startupCwd, which no floating creation path ever sets (FloatingTerminalPanel, FloatingTerminalWindowControls, floating-workspace-tab-creation all pass none). Its cwd is resolved over IPC from settings.floatingTerminalCwd and handed to the transport as a prop, so it never reaches the store at all. The branch was inert and its comment described main's handling of args.cwd rather than what the code read. Say what is actually true: a floating pane is keyed by its shell, and the configured-WSL-cwd-under-a-host-shell case is a known gap. Guessing from the unresolved setting would risk the mute direction, which is the expensive one. Also pin the repair-required early return. resolveLocalWindowsTerminalRuntimeOptions throws there, and the lane runs outside scanCodexPanes' per-pane failure guard, so without it Promise.all rejects and every pane in the batch loses its notice. That guard had no coverage; removing it now fails with the spawn error. Refs #10757 * fix(codex): trust the lane main recorded at spawn over a re-derived one The switch path re-derived each pane's Codex lane from current state while main had already written the resolved shell, cwd and distro at spawn. Four review rounds each found another divergence between the two, and the derivation still answers for a launch that never happened once the user edits a runtime preference. Prefer the recorded lane where one exists; keep the derivation for the panes main never records — pre-feature panes, LocalPtyProvider spawns and remote ids — and log when the two disagree. * refactor(codex): drop a redundant guard around the recorded-lane lookup |
||
|
Brennan Benson
|
4340781c9f |
fix(codex): drop the resume argv when session provenance is unverifiable (#10805)
Closes #10793. When Orca could not verify the originating Codex session file it either threw — a red per-pane toast and a failed spawn, reported as constant spam on #10757 — or returned null. Returning null did NOT start a fresh session: the renderer had already baked ['codex','resume',<id>] into the command and pty.ts never rewrote it, so CODEX_HOME simply fell through to whichever account was selected. The resume argv is now dropped so a plain `codex` launches, with a banner telling the user. The invariant — never run `codex resume <id>` under an account that does not own that rollout — is now satisfied by construction rather than by refusing to spawn. A verified resume is unchanged and still pins CODEX_HOME to the originating home. Reviewed over two adversarial rounds; seven defects found and fixed, including a HIGH where local-provider (non-daemon) spawns still carried ORCA_SEQUENCED_STARTUP_COMMAND with `resume <id>` — the wrong account behind a banner claiming it started fresh. `env` is now declared after the strip so no point in the handler can reach the pre-strip value. Live-validated in a real Orca dev build: all five cases proven on the SPAWNED PROCESS, including a real rollout under an untrusted home (the only shape that discriminates) and the local-provider path forced by stopping the daemon. An earlier CI failure on multi-client-navigation-isolation.integration.test.ts was investigated and is a PRE-EXISTING flake — a ~4ms race in the session-tabs notify coalescer that fails 5-8/24 on clean main, more often than on this branch. Fixed separately in #11022. Not verified: no Windows execution — its POSIX-only tests skip there and the #10757 reporter is on Windows. SSH is partial: no spurious banner or drop observed against a real target, but headless spawn does not deliver startup commands so the remote argv could not be read. The relay/mobile notice channel deliberately has no banner; the argv drop does happen there, so the invariant holds. |
||
|
Brennan Benson
|
9c5d827d6a |
fix(codex): keep history, restarts, and account identity across an account switch (#10770)
Fixes #10757. Switching Codex accounts broke three ways, all rooted in the self-contained per-account CODEX_HOME from #9501. HISTORY DISAPPEARED. Codex's own /resume picker only lists rollouts under the launch CODEX_HOME, and nothing bridged history into a per-account home — only the AI Vault's discovery scan knew about the other homes. Every other Orca-visible home's rollouts are now hardlinked in, on selection and again at launch, so one physical log is listed everywhere. THE RESTART PANEL STUCK. A queued restart was only drained by a mounted TerminalPane, but the prompt covered every stale pane in the worktree including parked and cold-deferred tabs. Requesting a restart now answers the prompt immediately while the pane keeps its pending restart, and a pane drains it when its reconnected PTY binds. PANES STAYED ON THE OLD ACCOUNT. CODEX_HOME is fixed in a shell's environment at spawn and the daemon keeps those shells alive across app restarts, while the restart notices are renderer state and are discarded. Each PTY's launch account is now recorded on disk and compared against the current selection at startup. Also merged in: #10802 (a dismissed notice no longer kills the pane's keyboard), #10803 (the sweep arms on real PTY binds, and launcher Codex panes are no longer filtered out by Windows deepest-process reporting), #10804 (a resume-pinned pane now says which account it is on), #10870 (the restart card no longer parks focus on its destructive Restart button), #10853 (the retry ladder is widened past the Windows worst case). Six independent reviews found real defects in every original PR, several of them dead-keyboard bugs and three introduced by the fix for another defect in the same loop. Live QA on macOS covered every PR; Windows was validated three times. WINDOWS: pass 1 found two defects that made the stale-account fix a no-op there (the sweep fired before any PTY was bound and never retried; launcher panes were filtered out). Pass 3 at the merged head: the prompt appears on its own after a restart — warm ~3.7-4.2s, cold ~21s needing rung 4, so #10853's widening was load-bearing rather than precautionary; an ordinary sentence typed into a healthy pane while another pane's card is up reaches that pane and kills nothing; a pane running vim after exiting Codex gets no card, still none 45s later. auth.json byte-identical across every pass. KNOWN GAPS, stated rather than implied: #10804 is unverified on Windows (auto-resume could not be manufactured there); cross-volume Windows is untested and expected to yield no bridged history (EXDEV, and Codex ignores symlinked rollouts); a cold-parked pane never binds so the sweep never covers it; the subagent-deepest launcher shape could not be reproduced on Windows, so that branch is fixture-verified only; WSL passed isolation but the resume mechanism is host-lane only. A host-account switch also marks and mutes live SSH remote panes — confirmed pre-existing on main by two independent QA runs — tracked separately in #10992. Related pre-existing defect filed as #10863. |
||
 nolainjin
|
17fc40eae6 |
fix(hooks): drain POSIX hook stdin without PATH (#10885)
Reviewed with an independent reproduction on macOS and Ubuntu 20.04. Fixed five test files that asserted the old literal strings, and switched the reader to `command -p cat` so it also survives hosts without /bin/cat. |
||
|
Brennan Benson
|
dad3e3c9ef |
fix(codex): rank the Codex resume rescan by selected account, not settings order (#10801)
The legacy Codex session-id rescan (used when a persisted record has no transcriptPath) returned the FIRST trusted home holding a rollout with that id. That home becomes the pane's CODEX_HOME — i.e. it picks the ACCOUNT — and the list ended in per-account homes ordered by settings INSERTION order, so the account was decided by whichever one the user happened to add first. Ranks instead: selected account -> real system home -> shared runtime mirror -> everything else by normalized path. Both ranking inputs are required (an optional one would silently degrade to pure path order), and the selection arrives as a thunk so the common provenance-present resume never stats the ownership marker for a ranking it never runs. The mirror needs its own tier: prepareLegacySharedCodexSessionResume's guard only fires when the resolved home IS the mirror, and that guard is what migrates the rollout into ~/.codex. Without it a system-default selection silently resumed under an arbitrary account and stayed pinned there permanently once the hook stamped a transcript path. Reviewed over two independent rounds; every tier individually mutation-proved. Live-validated in a real Orca dev build by reading the spawned PTY's actual CODEX_HOME across three builds (head, base, and head-minus-the-mirror-tier). Note: fixes none of #10757's user-visible symptoms on its own — it is a correctness precondition. Verified on macOS only; Windows coverage is fixture-only. |
||
  Wooseong KimandWooseong Kim
|
0b0e42ab60 |
fix(codex): treat non-Codex transcript as stale resume metadata (#10538)
When a pane is mislabeled agent:codex but still holds a Claude transcriptPath, the Codex resume guard threw and blocked relaunch. Only hard-fail when the path claims Codex's dated rollout layout (sessions/YYYY/MM/DD/rollout-*.jsonl), under any home and without requiring the file to exist. Paths that never claimed Codex provenance return null so the pane can relaunch. Keying on rollout shape rather than trusted-home membership matters: returning null only declines to override CODEX_HOME, and the renderer has already baked 'codex resume <id>' into the command. A real rollout under an untrusted home would otherwise resume under whichever account is selected — and once the session bridge hardlinks rollouts across homes, codex would find that id and resume silently under the wrong account. Trust is decided upstream by findTrustedCodexSessionResume. Co-authored-by: Wooseong Kim <innocarpe@users.noreply.github.com> Closes #10517 |
||
|
Brennan Benson
|
c3526cc19d |
feat(codex): surface a stalled config sync instead of failing silently (#10449)
* feat(codex): surface a stalled config sync instead of failing silently Why: the mirror keeps serving the last synced settings when ~/.codex/config.toml is missing, blank, or unreadable. That is the right call for data safety, but it is invisible — a downed WSL distro or an unhydrated cloud-synced home leaves "Orca ignores my config edits" with no log line and no UI to diagnose. Status is derived on demand from the same predicates the mirror uses, so the two cannot disagree. The stall is logged once per episode rather than on every launch and quota poll, and the Codex account section names the file and what to do. * fix(codex): latch an unreadable source and stop over-claiming recovery An unreadable source throws out of the mirror, so reporting only on the success path left that stall latch-less: it logged the raw failure on every launch and quota poll while its reason never reached the surfaced status. Report from the catch path too. The clear message also claimed the source was "readable again", which is false when the stall ended because the runtime config was removed rather than because the source came back. Restoring console.warn now happens in afterEach — an inline mockRestore is skipped by a failing assertion, and the leaked spy made every later case in the block fail spuriously. * fix(codex): latch the stall promotion hits first, and scope it to the host Review round 1 findings: - The unreadable-source latch still never fired in the steady state. Once a baseline exists, promotion reads the source before the mirror does, so it throws first and `!promotionPlan` returned before any reporting — logging a reasonless failure every launch and quota poll, which is exactly what the previous commit claimed to fix. Report from that branch too. The test only passed because its fixture had no baseline; it now seeds one first and fails without the fix. - The banner named the host's ~/.codex while a WSL or per-account runtime was selected, whose real source is a different file entirely. Gate it to the host scope, matching how the sign-in warning is already gated. - Three new translate keys were missing from the locale catalogs, failing the localization gate in `pnpm lint`. - The registrar mock was never asserted, so deleting the registration left the suite green. - `codexConfigSyncStatus` hung off the `agentHooks` namespace despite having nothing to do with agent hooks; moved to its own `codexConfigSync.status` while it is still a four-file change. * fix(codex): report sync health for the home the selection actually mirrors Review round 2: - The status resolved the shared runtime home, but the system default now runs Codex directly against ~/.codex and managed accounts get their own home. So a stalled per-account mirror showed no banner at all, while a stale shared home could warn about a config the active lane never reads. Resolve the mirrored home from the current selection, and report synced when the lane has no mirror to fall behind. - The round-1 report on the promotion failure path could clear the latch on a pass where no mirror ran, claiming a recovery that never happened and silencing every later pass. Only ever latch a stall there; leave clearing to the path that actually mirrored. * fix(codex): refetch sync status when the active Codex account changes Review round 3: - Resolving the status per selection made the fetch account-dependent, but the effect was not keyed on the active account. Switching accounts left the banner describing the previous one — and switching INTO a stalled account showed nothing at all, which is the silence this change exists to remove. - Pin the home resolution itself: it had no direct test, and its shared-home path was a hand-copied literal that could drift from the real helper and silence the banner with every other test still green. - Narrow the handler's dependency to the one method it calls, which also drops an `as unknown as` cast from its test. - Skip the chmod-based test on Windows, where a read-only directory does not block writes so the scenario cannot be constructed; matches the convention already used in config-settings-promotion.test.ts. * chore(codex): restore the handler docstring and isolate the resolver suite Round 4 returned clean; these are its two non-blocking nits. Narrowing the handler param left its JSDoc stranded above the new type, so the function had no hover doc. The resolver suite also read the developer's real CODEX_HOME and shell rc, so anyone exporting one would see it fail locally. |
||
 kin001andBrennan Benson
|
6a72c8f120 |
fix(codex): preserve runtime config when system source is missing (#9127)
* fix(codex): preserve runtime config without system source * fix(codex): retain baseline when mirror is skipped * refactor(codex): extract deprecated hook-flag normalization Why: codex-config-mirror.ts sat at the 300-line cap, so the missing-source guard could not land without a max-lines disable. * fix(codex): bootstrap a baseline when the mirror is skipped Why: a runtime home seeded outside the mirror (WSL, per-account) never got a baseline while the source was missing, so promotion stayed inert and silently reverted the in-Codex change once the source returned. * fix(codex): stop a synthesized source config from wiping runtime settings Two routes still reached the #9073 data loss after the missing-source guard: - Promotion runs before the guard and, with no ~/.codex/config.toml, created one holding only the promoted keys. The next mirror treated that skeleton as authoritative and deleted every other runtime setting. It needs no missing file: `codex mcp add` inside an Orca-launched Codex plus /model was enough to drop the MCP server for good. Promotion now seeds a brand-new system config from the runtime's ordinary settings, so the mirror round-trips them. - A 0-byte source (half-written, or an unhydrated cloud-synced home) still read as an authoritative empty config and advanced the baseline, making the loss unrecoverable. A blank source is now treated like a missing one. Moves the TOML section model out of codex-config-mirror.ts so promotion can share it without a cycle. --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> |
||
|
NeilandOrca
|
aab112933e |
Revert "fix(memory): bound OOM-prone accumulators (#10179)" (#10255)
Co-authored-by: Orca <help@stably.ai> |
||
|
Brennan Benson
|
ef985ed800 |
fix(codex): re-land TUI settings promotion with anchored baseline (#10213)
* Reapply "Preserve Codex [tui] settings across managed CODEX_HOME remirrors (#9475)" (#10085)
This reverts commit
|
||
|
Neil
|
8f40ddf328 | fix(memory): bound OOM-prone accumulators (#10179) | ||
 Jinjing
|
09756dfaff |
Revert "Preserve Codex [tui] settings across managed CODEX_HOME remirrors (#9475)" (#10085)
This reverts commit
|
||
|
Brennan Benson
|
c8381f3ea7 |
Preserve Codex [tui] settings across managed CODEX_HOME remirrors (#9475)
* fix(codex): promote [tui] settings so they survive the managed-home remirror
Codex TUI preferences (/statusline, theme, terminal title) are written into
the [tui] table of the managed runtime config.toml, but the write-back
promotion allowlist only covered four top-level scalars — so the next mirror
pass rewrote the runtime config from ~/.codex and silently discarded them.
Extend promotion to the [tui] keys the Codex TUI persists (status_line,
status_line_use_colors, terminal_title, theme), keyed as structured tui.*
paths so the same three-way merge (runtime vs baseline vs ~/.codex) applies:
in-Codex changes promote into ~/.codex before the mirror, and outside edits
to ~/.codex still win over stale runtime values.
The byte-preserving upsert moves to codex-config-settings-upsert.ts (max-lines)
and learns [tui] placement: replace an existing bare or dotted key in place,
insert into the first [tui] body, insert dotted beside existing dotted tui.*
keys, or create one [tui] table at EOF — never defining tui twice, including
when the system config holds an inline tui = {...} table.
* Add codex-config-settings-upsert to the CLI tsconfig file list
* fix(codex): keep tui upserts out of array tables
* fix(codex): handle quoted tui config paths during promotion
* fix(codex): harden tui promotion writes
|
||
|
Brennan Benson
|
407d7afc11 |
feat(telemetry): classify codex trust-grant fallbacks and attribute grant lane (#10001)
* feat(telemetry): classify codex trust-grant fallbacks and attribute grant lane * fix(telemetry): tighten codex trust-grant classification |
||
|
Brennan Benson
|
e986a7ba1a |
feat(codex): surface nested subagents (#9637)
* feat(codex): surface nested subagents * fix(codex): retire child rows on root stop * fix(codex): preserve nested agent state on restart * fix(codex): preserve nested agent identity * fix(codex): preserve subagents across relay restarts * chore(skills): refresh generated manifests * fix(codex): keep inferred interrupts terminal * chore(skills): record latest release snapshots |
||
|
Brennan Benson
|
7ca3e670c5 |
fix(codex): count per-account homes in usage and state removal blast radius (#9763)
* fix(codex): count per-account homes in usage and state removal blast radius - usage scanner now includes codex-accounts/*/home/sessions so multi-account usage is no longer silently undercounted (audit F2) - account-removal dialog copy now states that session history and MCP logins are permanently deleted with the managed home (audit F1 mitigation) * fix(codex): harden account usage discovery |
||
|
Brennan Benson
|
22d9c87872 |
Fix Codex automatic resume across account switches (#9696)
* fix(codex): preserve session home on automatic resume * fix(codex): keep automatic resume homes launch-authoritative * fix(codex): resume compressed rollout representations * fix(codex): keep resume migration best effort * fix(codex): reject stale automatic resume provenance * fix(codex): enforce real home on automatic resume * fix(codex): keep automatic resume home authoritative |