mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
f15a5034b969fc882c8dc245200517b2e9ebffb8
8496
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Neil
|
f15a5034b9 |
docs(daemon): cost the deferred guard honestly, and say why it is unreachable
Two corrections to the note, both of which change what it tells the next person. The reason the guard's case cannot arise at 34s is structural, not a lucky margin: an `empty` verdict means the daemon answered, so it resolved fast by construction, and proven-dead means nothing is listening, so the probe and the ladder both short-circuit. The path that actually spends the budget is the wedge that never answers — and that one now ends in a hold, paying neither the ladder nor the fork. Long path and expensive tail are disjoint. Raising the budget is precisely what re-couples them, by extending how late an `empty` may arrive. And the guard was costed as a signature change through DaemonSpawner, which is wrong. createOutOfProcessLauncher is a factory called where `signal` is already in scope; a third parameter closed over there leaves the launcher's call signature untouched. Overstating the price invites skipping the guard rather than paying it. Also recorded: two terms this budget does not bound at all — the healthy branch, which never consults the clock and still ends in a cleanup and a fork, and the unbounded daemon-host copy on packaged Windows. |
||
|
Neil
|
f28888944d |
fix(daemon): delete a retry loop that could never run, at any budget
Round twelve proved the loop unreachable by algebra rather than by tracing: remaining = B - E - max(CONNECT, (B - E) - REQUEST) = REQUEST, whenever B - E > CONNECT + REQUEST The patient connect takes every millisecond the answer does not need, so what survives it is always exactly OCCUPANCY_REQUEST_BUDGET_MS — and the gate wanted CONNECT + REQUEST. That holds for every ceiling, which also settles the raise I had been holding open: at 44s the remainder is still exactly the request budget, so ten more seconds of worst-case startup would have funded zero retries. Funding one honestly needs ~71s against a 60s fail-open. So WEDGED_DAEMON_GRACE_RETRIES = 11 documented patience the launcher did not have, and no number could give it. Deleted, with the derivation left where the loop was so the next person does not re-derive it from scratch. Little is lost. A 4s retry cannot reach a daemon needing longer than 4s to answer, which is the entire wedge population, while the one patient ask waits ~12s. The only case retries caught and this does not is a daemon recovering within seconds of being asked — and DegradedDaemonFreshSpawnRouter.recover() already returns it to full daemon service on the next spawn, off the startup clock. The budget tests could not have caught any of this: they recompute the expression from imported constants and never execute the launcher, so collapsing the patient connect back to the cheap constant — the round-eleven defect exactly — left all 1475 green. There is now a test that watches the launcher spend it, verified by mutation, and the arithmetic ones say plainly that they are not the guard. Also corrected: the evidence-read comment claimed the read only affects a log line. It decides the verdict wherever the unknown hold declines to — it has neither the proven-dead check nor the rejected check — so it is what holds a daemon whose socket vanished, and what holds a hello-rejected daemon still hosting agents. |
||
|
Neil
|
81f0298cf9 |
docs(daemon): record what a budget raise would owe before it is safe
The classification budget serves two verdicts with opposite time-costs. Reaching "don't kill" slowly is free — the daemon survives however long it took. Reaching 'empty' slowly is not, because the kill ladder and the fork still have to fit before the fail-open. At 34s that case cannot arise; at 44s it can, and an overrun there is the worst branch on offer: daemon killed, replacement forked then discarded, no provider installed, Restart broken. So the raise is not a number change, it is a number change plus a guard: hold rather than replace when the headroom left cannot fund the ladder and the fork. Safe precisely because that path has proven the daemon empty, so holding costs no agents. Written down next to the warning against tuning the budget, because the next person to want a bigger number will read that warning and need this one. Also recorded: the launcher closure has no access to the startup abort signal, so the cheap version of that guard is not available without threading it through the spawner. |
||
|
Neil
|
cd2650c038 |
fix(daemon): stop offering a remedy that cannot work, and say why two modules stay
The degraded warning told the user to restart the daemon. When something other than an Orca daemon holds the endpoint, restarting clears nothing: killStaleDaemon only kills a process whose identity matches the pid record, and a foreign holder matches none, so the next launch is identical. The message now says the daemon is unreachable rather than asserting what it owns, and names the second remedy. The notice says "usually clears this" for the same reason. That case is also now written down as a known residual: an endpoint that accepts connections but never speaks the protocol reads as an incumbent on every launch, so it stays degraded with no auto-recovery, where before it was replaced. The rest is comments, because three separate deletion proposals landed on this code in one review cycle and each was a regression. The evidence read is not redundant with the unknown hold: the occupied branch has no proven-dead check and the unknown hold does, so it is the only thing between a kill and a daemon whose socket entry vanished while it still hosts agents. Its children scan is not redundant with pid verification either — a verified-live pid alone would also hold a childless daemon, which is the one #8689 case still safe to replace. And grace retries are worth more since 'unknown' stopped killing, not less: a counted 'occupied' reaches full adoption where the alternative is a degraded hold. Each now says which case dies if it is removed. Reviewers reaching for the delete key three times in a row is the code failing to explain itself, not excess. |
||
|
Neil
|
2d5a5eaad1 |
fix(daemon): repair what round eleven found, including a fix that did nothing
The patient ask was not patient. With twelve seconds reserved for the evidence read, `max(CONNECT, probeBudget - REQUEST)` resolved to exactly CONNECT — the tolerant ask got the cheap ask's four seconds, and the grace loop's gate needed 19s of a budget that only ever held 11s, so it never ran at all. Both shipped green because mocked probes consume no wall clock, so the budget never binds in a test. Two arithmetic tests now compute against realistic elapsed time, which is where the defect actually lived. The reservation was backwards anyway. Evidence can only raise 'unknown' to an uncounted 'occupied', and both now hold the daemon, so the read changes a log line and nothing else — while starving the one probe whose counted answer still reaches preserveDaemon() and full daemon mode. It is opportunistic now: if the probes spent the clock, it is skipped and the verdict stays 'unknown', which holds exactly as an evidence-raised 'occupied' would have. Recovery was a one-way flip on a two-way condition. Once a health check promoted fresh spawns back to the daemon, nothing ever demoted them, so a daemon that wedged again cost a hello timeout plus a full launcher re-classification for every new terminal, for the rest of the session. A failed spawn now routes back and re-arms the cooldown. The class had no tests at all; it has six. The notice was wrong twice. It claimed terminals already open keep working — in held mode discovery runs over the same IPC the daemon is failing, so its sessions are never routed and attach refuses rather than answering on its behalf. They are running, but unreachable. And it named half the cost of Restart: runRestartDaemon shuts down the local fallback sessions too, so the terminals it had just called safe die as well. Also fixed: the amber-on-amber body text failed AA at 3.94:1, the keys were in a namespace no sibling uses, and the scale did not match the notice it renders beside. The banner armed a destructive button with state that never refreshed. It now refetches on focus, like the sibling notice that solved this first. Also: the launch mode type in the test harness omitted 'held', so no test could describe the launch the hold produces; held mode's routing through the degraded provider was unpinned, and deleting it left every test green; and pty:signal kept a try/catch for a synchronous throw that async routing cannot produce. |
||
|
Neil
|
7142b60dbf |
test(daemon): pin the hold that no longer depends on a clock
The repro proved the launcher holds a daemon it can see is occupied. The protection that now matters most is the one for a daemon it cannot see at all, and nothing asserted it. Adds the unknown-hold to the static assertions: that it exists, that it excludes 'rejected' and a proven-dead endpoint, and that every killStaleDaemon call site in the file is downstream of it. Verified by mutation — dropping either exclusion fails the assertion. |
||
|
Neil
|
f09a0a72ae |
fix(daemon): stop killing a daemon we merely failed to observe
Ten review rounds each found a different band where this branch replaced a daemon that origin/main would have kept, and every fix bought a new one. The reason is arithmetic, not carelessness: matching the old tolerance for a single probe costs about 25s, the classification clock has 15s to give, and funding the difference puts startup past the 60s fail-open once the kill ladder and the fork are paid. No assignment of those numbers is safe. So the residual stops being lethal. daemon-occupancy.ts always said it — "'unknown' is the residual, and it is not permission" — while daemon-init.ts fell through from unknown to killStaleDaemon. That fall-through is what made every millisecond of budget a correctness parameter. Now an unclassifiable daemon is held in degraded mode: existing terminals keep working, fresh ones run locally, and being wrong costs a degraded session instead of somebody's agent. Two exclusions, both about never holding something unrecoverable. A proven-dead endpoint is a cold start or a corpse, and holding one would hand every first launch a provider pointed at no daemon. 'rejected' answered and refused, so it can never be adopted and its sessions can never be reattached. The cost is real and deliberate: a wedged-but-empty daemon is no longer replaced at launch, so #8689 degrades to "restart it from Manage Sessions". Which only works if the user knows — and degraded mode was computed, plumbed through preload, and read by nothing. It now renders where the Restart button already lives, and says what it actually costs: new terminals close when you quit, and restarting ends whatever the host is still holding. Rejected on the way here: a background reclassifier (a permanently wedged daemon never answers, and recovery is already handled by degraded-daemon-fresh-spawn- routing.ts), letting accumulated process-table reads license a kill (its errors are systematic, so more samples agree rather than converge), and sidelining the daemon onto a renamed socket (verified working at the syscall level, then abandoned: the daemon's own endpoint-ownership watch reads the moved entry as lost and retires itself, precisely when it recovers). |
||
|
Neil
|
0dc7a8dbe9 |
fix(daemon): stop three follow-ups rotting where review already found them
A truncated paste now says so. A routing throw partway through a paste was not a PtyWriteUnavailableError, so no pty:writeUnavailable reached the renderer and the pane never re-attached — the remaining chunks simply vanished with nothing to attribute the gap to. It stays deliberately distinct from SessionNotFoundError, which isPtyAlreadyGoneError matches and synthesizes into an exit the session never had; a test now pins both directions so neither drifts. The launch-budget spec no longer fails on Windows. The evidence reserve is zero there by design — neither guarded step runs without a session-leader signal — but the assertion demanded eleven seconds of it unconditionally, so `pnpm test` broke on any Windows dev machine. PR CI never saw it: only the WSL boundary spec runs on windows-2022. The identity ceiling it reserves against is imported now instead of being a 3_000 someone would have to remember to change. And the grace-retry comment described the design that preceded the clock: ~5s probes, ~60s of grace, a number worth raising. The clock binds first and usually permits far fewer, so raising it alone buys nothing. |
||
|
Neil
|
7256ea2876 |
fix(daemon): ask the tolerant question while the clock can still fund the answer
The launcher asked the cheap question first and the patient one last. That is backwards. This path is only reached because a 3s health check timed out, so every 4s probe re-asks on a stricter budget than the one that triaged the daemon here — it can only ever agree. The one ask that could disagree ran last, by which point the clock could fund its handshake but not its answer, and a daemon that answered in 12s was read as dead and replaced along with its agents. Three changes, one idea: never make an ask you cannot afford to hear out. - The patient ask goes first, with every millisecond the answer does not need. - Cheap retries follow it, and stop once the clock cannot fund both halves. Funded to knock but not to listen is not an ask. - The adoption connect is capped. It is not a classification step — it acquires a lease preserveDaemon() re-establishes anyway — but on a daemon that accepts the socket and never completes hello it would spend the entire classification budget, leaving nothing for the probes that protect that daemon's sessions. The test double now forwards the connect budget instead of dropping it, so what the launcher was willing to wait for is observable at all. |
||
 NeilandOrca
|
b4a4b13c5a |
fix(daemon): ask the wedged daemon a question it can actually answer
Round nine found the re-verification was stricter than the check that triaged the daemon onto this path. The launcher gets here because a three-second health check — one socket, one hello — timed out. It then re-asked with two sockets and two hellos inside four shared seconds, and repeated that identical question up to twelve times. A daemon that consistently needs five seconds fails every one of them, so the retries could only ever agree with the check that sent it here. main re-asked with five seconds per connection step and thirty for the answer, and kept the sessions this branch destroyed. Retries and patience solve different problems. Keep the cheap probes, which catch a daemon that recovers on its own, then spend what is left of the clock on one tolerant ask — the only question that can disagree with the triage. It is skipped when the endpoint is provably gone, since a cold start arrives here too and has nothing to wait for. Two more from the same review. The clock claimed to cover the launcher's own adoption connect and started after it, so the fourth term that went missing from the sum was still uncounted; it now starts above that connect and bounds it. And Windows was holding back twelve seconds for an identity check and a process-table read it never performs — the reserve is zero where the steps it reserves for do not run. Retuned the ceiling to leave the kill ladder and fork real margin rather than half a second, with the packaged-Windows host copy named as what the margin is for. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
d827113c54 |
fix(daemon): make the classification clock actually bound the work it names
The clock introduced in the previous commit gated the probes but not the two steps after them. The identity re-check and the process-table read ran on their own deadlines, outside the ceiling, so the launcher could still spend its whole budget on probes and then take another ten seconds — the same overrun the sum used to produce, arrived at from the other end. Hold that time back from every probe instead. A probe is only started when the clock can still fund a handshake after the reserve, and its budget is what remains minus the reserve, so no probe can eat it however long the daemon takes to answer. Worst case is now the ceiling by construction rather than by addition. The reserve has a test asserting it is large enough for what it covers, which failed on its first run and caught that ten seconds was not. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
8148ca959c |
refactor(daemon): spend the launch budget against a clock instead of a sum
Round eight found the fourth term missing from the hand-written budget — the launcher's own adoption connect, which runs before the health check on the non-shared five-second path. The three before it were an identity probe, an endpoint probe, and an evidence deadline applied twice. Every one of them passed the test meant to catch exactly that, because the test could only check the terms someone had remembered to add. So stop summing. The classification now runs against a deadline and stops when it expires, and the test asserts only that the deadline leaves room for the kill ladder and the fork that follow it. A budget that has to be remembered is a budget that will be wrong; this one cannot be, because nothing has to be counted. That also retires the platform-split grace window, which existed to hand Windows more of a sum nobody could total correctly. The same review found the regression it was compensating for was never the window. main gave each probe up to fifty seconds — five per connection step, thirty for the request — where this branch gave eight for both together. A daemon whose handshake needs more than four seconds therefore answered none of the probes, however many it got, and on Windows nothing else can speak for it. Splitting the two budgets fixes the case the window never could: connecting stays tight, because a daemon that cannot handshake is wedged and worth re-asking cheaply, while a daemon that did handshake is demonstrably alive and its count is worth waiting for. Also stops a Date.now spy leaking out of a failed test and freezing the clock for the rest of the file. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
3ee3422efa |
fix(daemon): stop the budget cut spending Windows' only protection
Round seven found the one thing this PR must never do: kill a session that main would have kept. Main's grace loop probed with a non-shared 5s connect budget, so a wedged daemon got roughly a minute to come back. Bounding the probes and adding a wall clock cut that to about twelve seconds — a good trade on POSIX, where a daemon that outlasts the window is still protected by process-table evidence, and a bad one on Windows, which has no such evidence and now has nothing else. A Windows daemon wedged for half a minute while hosting agents was adopted by main and is killed by this branch. The fail-open cannot rescue it either: ensureRunning() is not abortable, so the launcher runs to completion. Size the window per platform instead, against what each actually spends: Windows pays no evidence read and no identity probe to feed one, so it can afford far more grace, and grace is worth more where it is the only thing there. Both numbers come from the budget test rather than taste. Three more from the same review: - The evidence read applied its deadline twice, once to the fresh table and again to the cached fallback, so an attempt could cost double what the launch budget was told. Share one deadline across both. - Two tests described protection the code no longer delivers: one asserted ~60s of grace the wall clock had already retired, the other passed only because its mocked probes are free and would fail against real ones. Say what the code actually promises, and freeze the clock where the point is retry depth. - The self-spawned PTY inventory promised more than it inspects. It sees direct node-pty calls in one directory; the macOS login-session probe reaches a PTY through expect(1) and is caught by the stranded-wrapper filter instead. Scope the claim, since that indirection is the shape the next escape will take. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
50c319e098 |
refactor(daemon): clear the debris the Windows removal left behind
Behaviour-preserving. Windows now abstains once at the entry point rather than twice inside a retry loop that had nothing to retry, which also retires the platform check further down that could no longer be false. The self-spawn matcher kept backslash splitting and .exe stripping for a branch that no longer exists, and the launcher's grace loop repeated its own IPC call and stacked two explanations above the wrong statement. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
b950e1ab30 |
fix(daemon): stop guessing at Windows occupancy instead of guessing better
The readiness review found the Windows branch reading a wedged daemon's orphaned conpty hosts as live terminals. ClosePseudoConsole only runs on the daemon's own JS thread, so a daemon too wedged to answer is also too wedged to reap them, and they accumulate exactly when this code runs. A wedged, empty Windows daemon would then be held forever — #8689 re-opened, and a regression from main rather than a missing protection. The tempting fix is another exclusion. That would be the sixth revision to what counts as a live PTY, each one added because a reviewer found something that looks like a session and is not, and each one trading safety for availability in a fix whose entire purpose is the opposite trade. The list is the problem. POSIX has a real signal: forkpty makes a hosted terminal a session leader, which nothing the daemon forks for itself ever is. Windows has no equivalent, so its branch could only ever count descendants and subtract guesses. Delete it and answer 'unknown' — Windows keeps exactly the behaviour it has on main, and the protection is claimed only where it can be justified. Also stops a blind confirming read from upgrading an unconfirmed emptiness into a verdict. Emptiness is what authorizes a kill; a read that saw nothing corroborates nothing. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
db3a301ef7 |
fix(daemon): confirm emptiness before it authorizes a kill
Readiness review, sections 02/03/05/06 — no P0 or P1 in any of them. These are the P2s worth taking. The important one: on macOS a terminal contributes exactly one session leader, the login wrapper, because the shell it forks is in the same session and shows S+ rather than Ss. I had assumed the shell counted too. It does not — so a wrapper that looks childless in a single snapshot makes its whole terminal invisible, and that snapshot cannot tell a wrapper whose shell has gone from one whose shell has not yet appeared. Emptiness is the answer that authorizes a kill, so it now costs a second read; 'owns-live-ptys' still needs none. The fixtures said Ss where a real shell says S+, which is why the tests never noticed. Also from that review: a fabricated row was cast to ProcessTableRow to reuse a command-only predicate, which is sound only while that predicate reads nothing else — narrowed to Pick<'command'> so the compiler keeps it honest. The pty:signal listener relied on its provider staying async to convert a routing refusal into a rejection; it is an ipcMain.on listener with nothing above it, so it now catches synchronously too. And the repro's teardown signalled remembered pids a minute after phase 1 waited for them to die — re-verify by tag first, since signalling a recycled pid is the mistake the script exists to study. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
8b77d986e1 |
fix(daemon): stop the evidence going blind on the host it exists to protect
Readiness review, section 04. Every agent pane already drives the shared process-table reader on its own cadence, so the uncached read queues behind them — and the host with the most agents to lose is the one likeliest to blow the deadline on queueing alone. Both attempts return unknown and the daemon is killed anyway, which is the original bug wearing the fix as a costume. Fall back to the TTL-cached table, which on that host is always warm for exactly the reason the uncached read is always queued. A table a few hundred milliseconds old still answers whether this daemon has children, and the failure directions are not symmetric: over-holding costs one degraded launch that self-heals, under-counting ends running agents. The same review found the launch budget still overran the 60s startup fail-open — by ~7s on Windows — and that the test guarding it under-counted the path it was written to bound, for the second time. It omitted the identity probe before the evidence read and the endpoint check that ends the grace loop. Both are now summed, the headroom requirement covers the kill ladder and fork that follow a replace verdict, and the grace window and Windows probe deadline are sized to fit. Not taken from that review: reusing the verified pid inside killStaleDaemon to drop the duplicate probe. That second verification is what fences the signal to this incarnation, and a seconds-old result is exactly the pid-reuse hazard it exists to prevent. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
750d2f588b |
test(daemon): pin the daemon's PTY spawn sites so the exclusion list cannot silently rot
The ownership evidence discounts the PTYs the daemon opens for itself, and that list is only safe while it is complete — a self-spawned PTY nobody excluded reads as user work and holds a daemon that owns nothing. The list grew one reviewer at a time, which is the wrong mechanism for a correctness invariant. Pin the input rather than the list. The daemon has exactly three PTY spawn sites: the user's terminal, the spawn health probe, and the Windows conpty warmup. A fourth now fails this test until someone decides which side it belongs on. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
ea7033ef6e |
fix(daemon): do not read a stranded login wrapper as a hosted terminal
Raised by a colleague's handoff on the same-day reports. macOS wraps every terminal in /usr/bin/login for TCC attribution, and #13764 shows the wrapper can outlive the shell it wrapped — leaving a session leader that hosts nothing. One affected host had accumulated enough of them to reach swap pressure. That is exactly the evidence this change treats as proof of live work, so a daemon whose sessions had all ended would have been held indefinitely on the strength of the corpses, on precisely the hosts where the problem is worst. Same class as the daemon's own probe PTYs: a session leader is necessary evidence, not sufficient. A wrapper still doing its job has the shell it exec'd beneath it. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
c0adc29087 |
test(daemon): hold the classification budget to the whole path, not one term
The previous assertion compared the grace window to the fail-open cap, which passed while the real path ran to roughly twice the cap — a single probe cost 50s against a 5s assumption, and the terms on either side of the loop were never counted at all. Sum the declared budgets instead: health check, grace window, the one probe that always runs past a ceiling tested at loop entry, and the evidence read on both platforms. Raising any of them now has to face this, and the spare time the kill ladder and fork still need afterwards is stated rather than assumed. Lives outside the launcher's own spec because that file mocks daemon-health, which would shadow the constants being held to account. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
09fae31270 |
fix(daemon): bound the POSIX evidence read and re-verify the pid it describes
Round-six follow-ups, none destructive. The process-table read had a deadline on Windows but not on POSIX, where the shared reader's ps timeout does not cover queueing behind an in-flight scan — so the one step that runs after the grace window could still outlast it. The pid handed to that read was verified before the grace window, which is long enough for the daemon to die and its pid to be recycled onto a shell with children. Verify it where it is used instead, and only when there is still something to raise: the common case now skips the identity probe altogether, which also takes a few seconds off the worst-case launch. Two renderer call sites killed PTYs without handling rejection. That was harmless while an unreachable session was answered by a silent no-op; now that it honestly rejects, pane teardown and repo removal would log an unhandled rejection every time — exactly when the daemon is already sick. Deliberately not taken from that review: giving the endpoint-occupied catch the same held fallback as the failed-health path. That path arrives with occupancy unknown or empty, so holding there would swallow a real launch failure to protect nothing. Splits the repro script, which had grown past the line limit, into the sequence it proves and the two things it proves it with: process-table inspection, and the static assertions on the launcher's hold decision. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
246da423db |
test(daemon): pin that a held session cannot be closed by a provider that never had it
Covers the held-daemon routing fence, including the coupling that is invisible from the routing file: the thrown error must not match pty:kill's already-gone predicate, or the close is swallowed into a synthesized exit and the orphan is hidden again. A rename would otherwise reintroduce the bug silently. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
88f30a702d |
fix(daemon): stop the local fallback answering for sessions it does not own
Closing a held daemon's pane reported success while the agent kept running. Unrouted ids resolve to the in-process fallback, whose shutdown returns silently for an id it has never heard of and whose write and resize are no-ops — and while a daemon is held nothing ever enumerates its sessions, so every one of them is unrouted. The pane vanished, the orphan outlived the app, and typing into a stuck terminal disappeared without a word. Only attach was fenced against that route. Extend the same rule to the operations that change or feed a session: route to the fallback only when it genuinely owns the pty, and otherwise say the session cannot be reached. The error type is load-bearing. pty:kill treats "Session not found" as proof the pty is already gone and synthesizes an exit, so reusing that error would have reproduced the lie one layer down. TerminalSessionOwnerUnverifiedError means "still there, we cannot reach its host", which is reported as a failed close and keeps ownership for a retry. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
f5abf2aa7b |
fix(daemon): decide adoptability from what the daemon can do now, in one place
Round five found the same question — can this daemon complete a hello right now? — answered in three places with three different conclusions, because 'held' had been added as a fifth branch rather than as the classification the other branches route through. Two of those answers were wrong, and both ended in no daemon at all, which is the outcome 'held' exists to prevent. - A daemon whose adoption hello had just failed was handed back tagged 'degraded-new-pty-fallback'. Init skips the lease only for 'held', so it reopened the same connection, threw, and aborted startup — leaving the agents alive but unreachable and Manage Sessions -> Restart throwing. - The pty-spawn-unhealthy arm ran first and claimed a successful hello proved adoptability, but that reading is from before the grace window. A daemon that answered at t=0 and went silent through thirty seconds of retries took that arm and threw the same way. Ask whether it is answering now, first. The budget was a comment with a Date.now() beside it: one occupancy probe could cost 50s, because the client's default is a 5s hello per connection step plus a 30s request timeout. Bound the probe explicitly, start the clock before the first one, and size the window so the whole path — health check, pid verification, loop overshoot and process-table read — fits under the startup fail-open with room to spare. Also folds the four sibling branches onto the same occupancy resolution. getAliveDaemonSessionCount was byte-identical to countLiveSessionsOverIpc, so one concept had two implementations and only one of them had been fixed. The Windows probe exclusions could never match: the warmup spawns COMSPEC, an absolute path, against an exact-equality test on 'cmd.exe /c exit'. Compare the program by basename and keep the argv tail exact. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
cb0f3e4633 |
fix(daemon): hold a hello-rejected daemon instead of adopting one that refused us
Found while reviewing why a mutation looked equivalent. Gating the hold on health === 'unreachable' left 'rejected' — a daemon that answered and refused the handshake — falling through to preserveDaemon(), whose adoption opens the very hello it just refused. That throws, and the throw costs the app its daemon and its Restart remedy. Killing it instead is no better: it can still be hosting running agents. Neither of those daemons can complete a handshake, so neither may be adopted, and both must be held. Gate on that rather than on one of its two causes. Adds regression tests for the round-four fixes: the self-spawned probe exclusion is exact-match on both platforms, an adoptable pty-spawn-unhealthy daemon is never routed to a mode that cannot adopt, evidence can only raise a verdict, and the grace budget stays under the startup fail-open cap. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
c3d350f7c6 |
fix(daemon): stop counting the daemon's own probe PTYs as hosted work
Round-four review found the evidence proving the wrong thing. The filter excluded the daemon's plain subprocesses on the grounds that only a PTY child is a session leader — but the daemon opens PTYs for its own health probe and conpty warmup, and forkpty makes those session leaders too. The comment's own premise refuted its exclusion list. A daemon hosting zero user terminals could be held on the strength of its stuck probe child, and since the held daemon also had no sessions, dropping our authenticated pair let it retire and take the very state we were protecting. Exclude them by exact command, on both platforms. Two more from the same review: - pty-spawn-unhealthy is only reachable after a successful hello, so that daemon is adoptable. It was routed to 'held' — which never adopts — purely because the count had come from the process table. Check it first; hold now requires an unreachable daemon. - The grace loop rescanned the process table every pass, though it is waiting for IPC and the table cannot change its answer in five seconds. Ask the daemon during the wait and read the table once, after. raiseOccupancy- WithProcessEvidence makes that split explicit, and can only ever raise. Bound the wait by wall clock too: the retry count alone never bounded it, and startup fails open at 60s by abandoning the daemon provider outright, which would trade a wedged daemon for no daemon and a Restart that throws. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
cbcbefed04 |
fix(daemon): make the occupancy residual total
The module's contract is that 'unknown' is where every unanswerable question lands, but a throwing dependency escaped instead — routing a failed observation into the launch path rather than onto the safe residual. Latent today because both real implementations swallow their own failures, which is exactly the kind of thing that stops being true during a refactor. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
533e317cee |
refactor(daemon): decide occupancy before the kill, not inside it
Three review rounds each found a new failure state in the previous shape, which was the design telling us something. The safety rule — never destroy running work — was replicated across the launcher's branches instead of being decided once, and the last round added it to one more branch behind a boolean. Policy had been put inside a mechanism: killStaleDaemon grew an input flag to disable its new veto, an output back-channel to report it, and a caller-side re-derivation of the classification the flag had lost. One structural error, one symptom per layer it crossed. Name the question instead. resolveDaemonOccupancy answers occupied | empty | unknown, asking the daemon first (authoritative both ways) and falling back to the process table only to RAISE the answer to occupied. OS evidence can prove work exists; it can never prove absence, so it never licenses a kill. The launcher now decides before it destroys anything, so killStaleDaemon goes back to being only "make this pid go away" — no options, nothing for a future caller to forget to disable, and Manage Sessions -> Restart cannot be vetoed because there is no veto left to hit. Holding is a real outcome now. A daemon that owns live terminals but cannot answer a handshake gets mode 'held': no adoption attempt, no lease, no fork beside it. That deletes the lease-free-handle fallback, the try/catch around preserve, and the tolerated-lease branch in init that existed only because the correct outcome had no representation. Two defects this removes outright: - The endpoint check used a local boolean probe that returns false on timeout, so under load — and unconditionally on Windows named pipes, where a busy server answers ERROR_PIPE_BUSY — the guard disabled itself in exactly the conditions it was written for. Use the canonical three-valued probe, whose own docs say absence of proof is not proof of death. - A daemon that answered and refused the handshake was killed with its agents. It is now held like any other occupied daemon. Also folds the replacement verdict onto pendingReplacement, retiring a pair of mutable launcher locals whose only job was moving one warning past the kill. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
8b46147859 |
fix(daemon): require session-leader evidence and stop preserving daemons that can never be adopted
Round-three review found the veto too eager in three ways, each of which traded the original data loss for a whole-session degrade or a permanently daemon-less app. Evidence was "any non-zombie descendant", justified by re-sampling to weed out transients. The two samples are taken back to back — one ps fork apart — so nothing transient is ever weeded out, and a hung helper (often the very reason the daemon is wedged) reads as an agent. Use the structural signal instead: a PTY child is a session leader, because forkpty calls setsid, and no helper the daemon forks ever is. Re-sampling now only retries blindness. A 'rejected' daemon answered and refused the handshake, so it can never be adopted; preserving it repeated the same failed adoption on every launch, forever. And the veto read the process table, not the socket, so it could fire on a daemon whose endpoint was already gone — adoption then threw, init aborted, and the app was left with no spawner and no working Restart. Gate on both: only preserve what could still be reached. Also: releasing the launcher's temporary lease after the permanent lease failed reopened the adoption gap that ordering exists to close, and the tolerance added to discoverDaemonSessions was dead code — nothing on that path rejects. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
12a1366d52 |
test(daemon): kill the mutations that let the PTY veto ship as a no-op
Mutation testing found four survivors — changes that break the fix while every test stays green: - Swapping the POSIX reader to the 500ms-cached one passed. It is not just a staleness hazard: inside the TTL both sampling attempts receive the same array, collapsing the two-sample confirmation to one. Pin the fresh reader. - Replacing killStaleDaemon's default inspector with one that never reports live PTYs — the veto disabled in production — passed, because every veto test injects the hook. Exercise the real seam. - Adding the veto to cleanupDaemonForProtocol passed, which is verbatim the failure its own doc warns about: a user-initiated restart of a daemon owning live PTYs would refuse, then throw. Pin that call's arity. - The ppid-cycle fixture put the cycle outside the daemon's subtree, so the walk never entered it and deleting the visited guard passed. Also bound the Windows enumeration, which had no budget of its own: two CIM queries with a wmic fallback can stall the launch path for tens of seconds. Blind is a safe answer there; hanging is not. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
4d839ab388 |
fix(daemon): survive preserving a daemon too wedged to be adopted
Adversarial review found the veto's own success path could not complete against the daemon it exists to protect. Preserving routed through holdDaemonAdoptionLease(), which opens a hello — the exact operation a wedged daemon cannot answer — so it threw, aborted initDaemonPtyProvider, and left no spawner. restartDaemon() throws without one, so the user lost the documented Manage Sessions -> Restart remedy on top of having no daemon: strictly worse than the data loss being fixed. A still-listening endpoint means wedged, not gone, so keep a lease-free handle instead. The lease only cancels the adoption watchdog, which never fires on a daemon that owns sessions. Degraded mode likewise tolerates a lease and a session discovery it cannot complete. Three more from the same review: - The veto keyed on reason === 'failed_health_check', but a daemon that answered listSessions with 0 lands in that same branch and must stay replaceable. Key on liveSessionCount === null, which is what the option actually documents. - Zombies are not evidence of live work. A wedged daemon cannot reap, so its exited agents linger as <defunct> and would read as "still running" — a false positive correlated with the wedge itself. Enumerate through the process table's stat column and exclude them; sample twice so a resolver probe or health-check shell cannot masquerade as an agent. - Restore the "did anything answer?" log guard alongside the confirmed kill, so a daemon that self-retires before the kill is still announced. Co-authored-by: Orca <help@stably.ai> |
||
|
NeilandOrca
|
7837629845 |
fix(daemon): stop killing a wedged daemon that still owns live agent PTYs
A daemon too busy to answer listSessions was indistinguishable from a dead
one: getAliveDaemonSessionCount() returns null ("could not verify"), the
preserve gate required `!== null && > 0`, so the run fell through to
killStaleDaemon() and every running coding agent died with it. The sibling
replace branches all preserve on null; this one alone collapsed "can't tell"
into "empty", which src/main/daemon/AGENTS.md already forbids.
Give the decision an out-of-band second opinion. inspectDaemonPtyOwnership()
reads the OS process table — never the daemon socket, which is exactly what
failed — and reports whether the daemon's own process still has live PTY
descendants. Under preserveWhenOwningLivePtys, that evidence vetoes the
signal and the launcher adopts the daemon in degraded mode instead.
The veto is opt-in so it cannot make a daemon unkillable: only the
failed_health_check path enables it. Manage Sessions -> Restart still kills.
Only positive evidence preserves, so a wedged daemon with nothing to lose is
still replaced (#8689).
Also stop replacing silently: the verdict now prints on the post-kill truth,
which stays quiet on a cold start because nothing was killed.
Co-authored-by: Orca <help@stably.ai>
|
||
 Jinjing
|
96c17b44d5 |
Resolve threads as primary acknowledgement, not replies (#13894)
Collapsed threads visibly close on the host — this is the acknowledgement. An extra reply only adds noise. Snapshot the resolution target at launch to prevent silent drops if the panel navigates before agent delivery. Only reply for comments without resolution endpoints. |
||
|
Jinjing
|
675400e318 |
fix(automations): give list table more breathing room (#13898)
Increase row height and the gap above the table so the Automations list feels less dense. |
||
|
Neil
|
0824351fe0 |
fix(bitbucket): commit credentials atomically and stop blaming the token for network faults (#13887)
* fix(bitbucket): commit credentials atomically and stop blaming the token for network faults STA-3941 (P0): a credential edit could destroy the last working pair. `writeFileSync` replaces in place, so a failed or interrupted write truncated the previous secret, and the secret and metadata files were published independently — a crash between them left a new secret paired with the old email, unusable on restart. - Write credential files through a temp + fsync + rename, so a reader sees either the old bytes or the complete new ones, never a truncated file. - Carry authMode/email/baseUrl inside the encrypted envelope and treat the plaintext metadata as display-only. A torn write can now only leave a stale displayed account, never an unusable credential. Envelopes written before this change still resolve through the metadata fallback. STA-3944 (P2): timeouts, DNS failures, 5xx and unparseable bodies collapsed to the same miss as a 401, so Orca told users their credentials were invalid when the host was simply unreachable — sending them to regenerate a working token. `/user` now reports rejected vs unreachable, connect explains which happened, and an unreachable host no longer renders as "Auth failed". Adds failure-injection coverage for a partial secret write, an interrupt between publishing the secret and the metadata, legacy envelopes, and the transport-vs-auth split. * fix(bitbucket): loop short writes and keep eligibility out of the keychain Review of the atomic writer found it ignored writeSync's return value. write(2) may return a short count, so a partial buffer could be fsynced and renamed into place — the same truncation the change exists to prevent, moved one step later. Write now loops to completion, with a test that fails without it. Also stops isBitbucketReviewCreationAuthenticated force-decrypting the stored secret. Create-PR eligibility is evaluated proactively for the sidebar, so that popped an OS keychain prompt just from opening a worktree. Presence is enough; creation itself still fails closed on an unusable credential. |
||
|
JinjingandOrca
|
0c13e51097 |
feat(pr-comments): sort grouped PR comment sections newest-first (#13893)
Grouped/triage sections now read newest-first so recent discussion surfaces at the top, while Timeline keeps its oldest-first history order. - rename sortPRCommentGroupsForTimeline to sortPRCommentGroupsByRecency and add an order parameter - rank threads by their latest activity under newest-first so a fresh reply refreshes an old thread - break timestamp ties by numeric comment id in the sort direction, so a GitHub review batch sharing one timestamp orders correctly - sink groups with an unparseable createdAt to the bottom in both orders Co-authored-by: Orca <help@stably.ai> |
||
|
Jinjing
|
55beeebd25 |
Allow directly search in google (#13863)
* Allow direct search with configured search engines Users can now search directly from the tab creation menu using their configured search engine. Forced search mode (`?` prefix) skips file and tab matching for guaranteed search. Refactored tab-entry operations into focused modules for clarity: forced-search parsing, network-safe selection, keyboard focus, copy strings, empty options, and props types. Search routes through the same workspace browser tab opening mechanism used for URLs, with safe title and query presentation that doesn't retain sensitive details. * Allow direct search with configured search engines - Permit search and URL navigation while file index loads; require explicit selection only when needed, not automatic opening - Block malformed IPv6 addresses in bracket notation to prevent misclassification - Support Kagi private-session links via searchUrlOptions - Improve error handling with accessibility: show error messages in status region, disable input during submission, display loading state - Fall back to local browser tab creation when remote creation fails instead of throwing; avoids remote availability blocking local search/navigation - Add error translations for all supported locales (es, ja, ko, zh) * Allow direct search with configured search engines - Extract tab create entry lifecycle to key-driven component remounting, replacing conditional state reset with useEffect cleanup - Consolidate network tab entry classification and request building into reusable helpers, eliminating duplicate logic - Simplify owner resolution by inlining logic directly into openWorkspaceBrowserTab - Replace custom surrogate-pair handling with native String.toWellFormed() for search queries - Disable explicit URL classification to prioritize search-engine queries over raw URLs * Allow direct search from quick-open tab bar entry - Single-token queries keep file matches ranked above search (quick-open intent) - Multi-word phrases promote search to top, since they cannot be file paths - Arm network actions once file index fails or text is unambiguous search - Cache prepared file index to avoid re-processing per keystroke - Generate specific tab titles (e.g. 'example.com/docs') instead of generic 'Open URL' - Surface opening workspace when launching browser tabs remotely * Use readOnly instead of disabled for pending search input Maintain keyboard focus during submission so arrow/Escape navigation continues to work. Use aria-busy to indicate loading state accessibly. Also fixes button hover styling when disabled and cleans up error message handling in the classifier. * Treat bare searches as prompts; refine path and IP classification - Bare search queries (e.g., "?") no longer display as error rows - Path prefixes with existing matches are no longer blocked mid-keystroke - Private IPv4 addresses now use http, public addresses use https - Ambiguous inputs with non-numeric ports fall through to search instead of blocking - Improve diagnostics by logging failure reasons in openFailure |
||
 Brennan Benson
|
23cbe6dfe2 |
fix(agent-hooks): guard copilot and kimi Windows hooks before they own stdin (#13379)
Same hang class #11568 closed for .cmd: copilot-hook.ps1 ran [Console]::In.ReadToEnd() and kimi-hook.sh captured stdin before checking the Orca env, so a user-wide hook fired outside an Orca pane blocked forever when the caller abandoned the pipe — one stranded powershell.exe or bash.exe per hook event. Move the env guard (after the endpoint refresh, which can supply PORT/TOKEN) above the read on the Windows-local variants. The payload is discarded on the missing-env path anyway; the broken writer is the trade #11568 established. POSIX variants keep capture-first — those callers close stdin, and exiting mid-write there surfaces as EPIPE the agent can see (#8110). Kimi's remote install now asks for the posix variant explicitly. Verified on a real Windows host: pre-fix copilot with an abandoning caller never exits; post-fix both exit 0 immediately, and the env-present copilot path still consumes stdin and delivers the POST (paneKey + payload received by a local listener). |
||
  Jinwoo HongandE2E Test
|
9f0bf39b04 |
fix(github): fail closed when stack metadata is unavailable (#13866)
* fix(github): fail closed on unavailable stack metadata * fix(github): validate REST pull request response shape * fix(github): allow omitted stack metadata * test(github): cover enterprise stack probe failure * test(github): cover null ordinary stack metadata --------- Co-authored-by: E2E Test <e2e@test.local> |
||
|
Brennan Benson
|
686f5dca1a |
fix(browser): exclude Google cookies from imports - direct sign-in is the only Google path (STA-3811) (#13670)
* fix(browser): exclude Google cookies from imports (STA-3811) Imports never write and never remove a google.com-family cookie, on any path. Signing in directly inside Orca is the only Google session that survives, so the live jar always beats anything an import could plant. * fix(browser): roll back selective cookie clears |
||
|
Brennan Benson
|
f19ff5be68 |
fix(browser): keep one identity across hosts during Google sign-in (STA-3811) (#13667)
While the auth document is on screen the WebContents UA is Firefox, so its cross-host subresource/XHR requests (gstatic, play.google.com, the sign-in challenge endpoints) reached the header layer carrying the Firefox UA yet still bearing Chromium client hints, which the else-branch rewrote to Chrome. That paired a Firefox UA with Chrome client hints on every non-auth Google host — a sharper cross-host identity tell than either signal alone, and a plausible cause of the password-submit challenge greying out and stalling. Strip client hints on any request already carrying the Firefox auth UA so the UA and hint surfaces tell one Firefox story for the whole flow. Gated on the same googleAuthOverride flag as the auth-host switch, so imported-native profiles are unaffected and the clean-Chrome default for non-Google sites (Cloudflare) is untouched. Extends tests/tools/google-signin-ua-probe.cjs with app-current/app-fixed modes that mirror the shipped code and log per-request identity; on the real accounts.google.com load they show 18 firefox-ua-with-chrome-hints cross-host mismatches before and 0 after. |
||
|
NeilandOrca
|
ff727c3caf |
fix(settings): read voice settings from a ref, not a stale closure (#13883)
* fix(settings): read voice settings from a ref, not a stale closure updateVoiceSettings wrote from a closure captured at mount, so an async continuation could clobber a newer value. Uses the latest-ref pattern already in Settings.tsx:405-407. Found while investigating voice-microphone-selection.spec.ts:107 and confirmed NOT to be its cause — that failure has no established root cause. The spec gains a hydration barrier and toBeEnabled assertions here, which improve its failure message but do not fix it. Co-authored-by: Orca <help@stably.ai> * test(e2e): drop the voice spec changes from this PR The hydration poll was a no-op: prepareVoiceSettings awaits updateSettings inside the same page.evaluate, so the store is already committed before the poll samples it, in all three tests (the reload precedes it). Worse, its comment asserted that the Select is disabled until hydration — the CI accessibility snapshot at the moment of failure shows the switch checked and the combobox enabled, so that theory is refuted, and a wrong 'why' comment is worse than none. The toBeEnabled assertions go with it: the control was enabled when it hung, so they would not have caught the real failure either. Leaves this PR as what it actually is — a product bug fix. The spec's failure stays open and unexplained. Co-authored-by: Orca <help@stably.ai> * test(settings): type the clear-key mock as its API contract vi.fn(() => clearing) returned Promise<void> where the preload contract is Promise<{ configured: boolean }>. Vitest does not typecheck, so this passed locally while tsc -p tsconfig.tc.web.json failed — it would have gone red in CI's required typecheck job. Co-authored-by: Orca <help@stably.ai> * fix(settings): write the voice-settings ref in an effect, not during render React Doctor's 'Ref mutated during render' rule is blocking in CI's static analysis job, and it is right: React can replay or discard render work, so a render-time ref write can leak from UI that never commits. Moving it to an effect keeps the behavior this fix needs — every writer here (key-status probe, save-key, clear-key) runs post-commit, so the ref is current by the time any of them reads it. Test still discriminates: red without the fix, green with. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai> |
||
|
Jinwoo Hong
|
09c8597fb7 | perf(orchestration): route federated reads over shared control (#13814) | ||
|
Brennan Benson
|
e77e1fe850 |
fix(claude): guard cold-restore resume selectors (#13868)
* fix(claude): guard cold-restore resume selectors Persisted Claude default args or a custom command can carry their own --resume/-r/--continue/-c selectors (a bare picker default or a stale id). Cold restore appended the authoritative --resume <id> after them, typing a command with competing selectors into the restored pane (#12982). buildAgentResumeStartupPlan now routes Claude through a selector guard that tokenizes the base with the existing startup tokenizer, strips selectors in option position only (value-taking options keep dash-leading values), and appends exactly one authoritative selector, inserting before Claude's own -- terminator when present. Splicing is span-based so untouched bytes stay verbatim, wrapper commands are left alone, and any tokenization failure falls back to the previous append-only behavior. Launch paths, other agents, persistence, and the wire are unchanged. * fix(claude): harden resume selector guard against false matches Round-1 review findings: locate the claude executable by command position (index 0, after a wrapper --, or behind NAME=value assignments) so an argument merely ending in /claude can never be mistaken for it; stop matching the joined -r<id> form, which was ambiguous with dash-leading option values and forced an unmaintainable arity table (now deleted). Ambiguous shapes degrade to the pre-guard append-only behavior. * fix(claude): fail resume guard open on chained shell syntax Round-2 review findings: an unquoted operator or newline after the claude token means the base chains other commands, and splicing across that boundary handed the selector to the wrong command — detect it and fall back to plain appending. Also recognize claude behind PowerShell's & call operator, decouple the test oracle from the implementation's selector predicate, add Windows tokenizer span tests, and rename the module after its public API. * fix(claude): flag bare shell operators inside the tokenizers Round-3 review findings: the guard's operator scan compared raw source to token value, so one quote or escape anywhere in a token hid a shell-active operator outside the quotes and the splice crossed a live command boundary, losing the resume entirely. Both tokenizers now flag tokens carrying an unquoted, unescaped operator byte (or a word-leading # comment on posix/powershell) on their spans, where quote state actually lives, and the guard fails open on that flag. Also strengthens the redirect fail-open test to carry a stale selector, re-tokenizes each raw span in the shell span tests, and documents agent-resume-argv-drop as codex-only. * fix(claude): flag expansions and clamp separator backoff Round-4 review findings: unquoted multi-token expansions (backtick, $(, ${) split across whitespace, so removing only the recognized selector token left a broken construct tail — both tokenizers now raise the span flag (renamed bareShellSyntax) for those openers, on cmd also for operators between single quotes, which cmd does not treat as quoting. The separator backoff is clamped to the previous token's span end so a token ending in an escaped space can no longer donate its escape to the appended selector. * fix(claude): treat cmd single-quoted regions as unmodelable Round-5 review finding: cmd.exe has no single-quote syntax, so the Windows tokenizer's grouping of a single-quoted region diverges from what cmd parses — literal argv like 'claude ...--resume... old' was being read as a real selector and stripped, and a literal '--' as claude's terminator. Flag any cmd single-quoted token as bareShellSyntax so the guard fails open. * fix(claude): flag quoted expansions and scope assignment prefixes Round-6 review findings: the span flag was only evaluated in the unquoted branch, so an expansion opener inside double quotes went unflagged — and inside $(…)/backticks a nested quote re-opens a context this tokenizer does not model, so the splice could cut mid-construct (syntax error, or a silently mutated substitution body). Both tokenizers now flag those, and the flag is renamed divergesFromShell to say what it means. Restrict the NAME=value command-position prefix to posix, where that syntax exists. Drops two branches proven dead. * fix(claude): model shell-literal escapes and scan the whole base Round-7 review findings: (1) the divergence scan started after the claude token, so an expansion opened in a prefix — $(x; npx -- claude --resume s) — had its closer spliced away, producing a base bash cannot parse; it now covers every token including the executable, exempting only PowerShell's leading call operator. (2) posix drops a double-quoted backslash the shell keeps literal, and the Windows escape branch ran inside quoted regions where cmd/PowerShell keep the escape byte literal — both now flagged, so a literal can never be misread as a selector. (3) an unquoted line continuation hid a selector inside a token and skipped the newline gap check. Also removes a third provably dead branch and collapses the cut floor into the cut itself. * fix(claude): flag escapes the tokenizer models but the shell removes Round-8 review findings, all one family — escapes whose token value hides a selector the shell would see: a double-quoted line continuation (bash deletes both bytes), posix $'…'/$"…" quoting, a windows escaped newline, and a trailing unpaired escape. The last one was previously written off as pre-fix-identical, but once stripping happens the dangling escape swallows the separator and no exact --resume reaches claude at all — strictly worse than appending, so it must fail open. Also folds the three gap predicates into one scan. * fix(claude): stop over-flagging a literal dollar sign Round-9 review findings from both lanes: inside double quotes only $( and ${ open an expansion — $' and $" are literal there — and a trailing $ was flagged unconditionally because JS ''.includes('') is true. Both made the guard fail open on modelable bases, leaving the stale selector to compete, so #12982 went unfixed for them. Separately, cmd strips ^ before the child re-splits on the bare whitespace, so an escaped separator hides two real arguments and must fail open rather than drop one. * fix(claude): fail open on cmd caret-quotes and bare PowerShell syntax Round-10 review findings, both Windows-only (a bash oracle cannot reach them): cmd strips a caret before a quote and the child's parser then reads a bare quote delimiter, so the tokenizer's word boundaries stop matching argv — one case turned a working resume into no resume at all, another let a stale selector survive the splice. And bare (…)/{…} are live PowerShell syntax in argument position, so splicing through them emitted unbalanced output that PowerShell cannot parse. * fix(claude): fail open on the PowerShell stop-parsing token Round-11 review finding: after a bare --%, PowerShell passes the rest of the line to the child literally, so the guard stripped a real selector and then appended quoting that arrives as literal bytes — claude ends up with no exact --resume at all, worse than leaving the stale one. Quoted "--%" and cmd, where the token is ordinary, still splice. * fix(claude): model cmd backslash-escaped quotes Round-11 review finding: an odd run of backslashes before a quote makes it a literal byte to the child's CommandLineToArgvW parser, not a delimiter, so the tokenizer's word boundaries stopped matching argv. Orca manufactures that pattern itself — quoteStartupArg wraps every token in quotes without escaping a trailing backslash — so a pasted Windows path was enough to move the selector into a desynced region and leave claude with no resume flag. Also replaces a caret test case that was byte-identical before and after its own fix, and merges two stacked comment blocks. * fix(claude): fail open on PowerShell double-quoted escape sequences Round-12 finding: PowerShell expands backtick escapes only inside double quotes, so a sequence there produces a token value argv never sees — the guard could strip "-`r" plus the argument after it. Also narrows the stop-parsing comment: a quoted --% can engage stop-parsing before a parameter token, where the base is already mangled either way. * fix(claude): flag PowerShell escape sequences in bare arguments too Round-13 finding: the previous commit gated on quote === '"', but PowerShell's tokenizer calls Backtick() from ScanGenericToken, so it expands these sequences in unquoted arguments as well — bare -`r really is a control character, not -r. The guard read it as a selector and dropped it plus the argument after it. Widening to all PowerShell contexts measures 0 under-flag and 0 over-flag across the full printable matrix; the backtick-escaped-space idiom still splices. Also swaps a test case that was byte-identical with and without its own fix. * fix(claude): drop a token-leading PowerShell backtick before whitespace Round-14 observations, all pre-existing and measured: PowerShell drops a token-leading backtick together with the whitespace after it, emitting no token, so the tokenizer's extra token shifted the locator; and a backtick before a bare CR is a line continuation too. Flagging both takes the lane's 329k-base sweep from 87 bad to 0 with no new failures and the must-splice list byte-unchanged. Also corrects a comment that no longer listed every PowerShell divergence. * docs(claude): correct the bare-CR rationale in the tokenizer comment Round-15 verified against a real PowerShell 7.6.4 engine: a backtick before a bare CR is not a line continuation there — pwsh keeps the CR in the token. The flag stays because 5.1 is unverified and failing open costs nothing, but the comment now says that rather than claiming continuation. |
||
 OrcaWinandBrennan Benson
|
234caecb82 |
fix(windows): refresh PATH ordering for newly installed tools (#13545)
* fix(windows): refresh persisted PATH ordering New terminals now preserve Orca-injected entries while adopting current machine and user PATH precedence, so newly installed tools are not shadowed by stale aliases. Invalidate the cached registry snapshot on Windows setting changes and guard in-flight refreshes from restoring stale cache data. * fix(windows): retry invalidated PATH reads * fix(windows): reuse current PATH refresh * fix(windows): preserve per-hive PATH fallback state --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> |
||
|
Jinjing
|
648d17372b |
feat(sidebar): add jump-to-top button for hard upward scrolling (#13864)
* feat(sidebar): add jump-to-top button for hard upward scrolling Detect intentional hard scroll-up gestures (wheel or scrollbar drag) and offer a one-click jump-to-top affordance. Auto-hide after idle to avoid persistent visual clutter. Addresses the common case of fast navigation through long worktree lists ranked by agent activity. * test(sidebar): improve scroll-to-top detection for active gestures Only detect velocity from active scrollbar drag or touch, not programmatic scrolls. Return focus to list after jump-to-top. Add comprehensive hook tests with gesture simulation. Improve cumulative down-delta tracking for dismissal. * test(sidebar): add scroll-to-top gesture detection tests Add comprehensive test coverage for the hard-upward-scroll detection hook, verifying idle timer behavior, gesture suppression, scrollability checks, and cleanup on unmount. Extract the post-jump suppression window into a named constant for maintainability. |
||
|
Jinwoo Hong
|
077f5a11cd |
feat(github): create stacked pull requests (#13750)
Adds GitHub stacked pull request creation: a contextual "Stack this PR above #N" option that appears only when the selected base branch has an open PR, plus the main-process stack preflight and registration. Also reworks the create-review composer for cohesion: shadcn Checkbox and Label primitives, base label above a full-width searchable combobox with attached results, keyboard navigation, and a unified field skin, spacing and typography scale. Verified end to end against real GitHub: extending an existing stack and creating a new one. |
||
 Neilanddevatnull
|
63271a5933 |
feat(bitbucket): connect Bitbucket from Settings and create pull requests (#5832)
* feat(bitbucket): connect Bitbucket from Settings with encrypted credential storage Bitbucket Cloud was the only review provider with no in-app auth: GitHub and GitLab delegate to the gh/glab CLIs, but Bitbucket has no comparable first-party CLI, so the only option was ORCA_BITBUCKET_* env vars plus a restart (discussion #5364). Adds a Connect/Edit/Disconnect flow on the Bitbucket integration card, modeled on Linear and Jira: - Credentials are verified against /user before they are persisted, so a dead token is rejected inline instead of silently stored. - The secret is encrypted with safeStorage (0600 plaintext fallback when no OS keyring); non-secret metadata lives in a separate plaintext file so status reads render the connected account without decrypting. Opening Settings therefore never triggers a keychain prompt. - Env vars keep precedence over stored credentials, so existing headless and SSH setups are unaffected. Env-managed connections hide Disconnect. - connect/disconnect reset the preflight cache, so no relaunch is needed. The Bitbucket card moves to its own file to stay under the tsx max-lines cap. * feat(bitbucket): support creating pull requests from Orca Bitbucket was the only configured provider whose Create button reported "This repository provider does not support creating a pull request from Orca" — supportsReviewCreation was false and the forge provider had no createReview, so even a correctly authenticated setup was blocked. Adds createBitbucketPullRequest against POST /repositories/{ws}/{repo}/ pullrequests, using the same env-first / stored-credential resolution as PR lookups (extracted into resolve-auth.ts so both share one path). Bitbucket Cloud has no draft pull requests, so a draft request is rejected with a clear message rather than silently publishing a live PR. * fix(bitbucket): hide the draft toggle where drafts do not exist, plus review fixes Bitbucket Cloud has no draft pull requests, so the composer no longer offers the toggle for it and forces the flag off at submit — better than failing after the user has filled the form in. Review fixes: - writeFileSync's `mode` only applies when it creates the file, so rewriting a credential kept whatever permissions it already had. chmod after every write, for the secret and the metadata. - An explicit ORCA_BITBUCKET_API_BASE_URL now wins over a stored base URL. Env precedence is per-setting, not all-or-nothing. - Enter in the credentials dialog only submits from a text field, so it no longer hijacks Cancel and the docs link. - Replace the chmod-based delete-failure test with a mocked unlinkSync: file modes are not portable to Windows and elevated runners unlink anyway. * fix(bitbucket): stop a merged pull request from blocking the branch's next one Reported on #5832: with a merged PR on a branch, Create reported "Pull request already exists" and offered no way forward. The branch lookup queries every PR state and returns the most recently updated one, so a merged PR came back as the branch's current review and eligibility blocked on it. Bitbucket only discarded such a match on the repo default branch (#9171), while GitHub already drops any merged PR it matched by branch alone — "a merged PR without an explicit link is just a historical branch match, not implicit review context". Applies that rule to Bitbucket. An explicitly linked review still resolves through the linked-number fallback, so merging a PR Orca knows about keeps showing it. * fix(bitbucket): add bitbucket to the shared review-creation provider list Reported on #5832: on a Bitbucket repo with no existing PR, Create still said "This repository provider does not support creating a pull request from Orca", even after the forge provider gained createReview. There are two capability lists. Enabling supportsReviewCreation on the forge provider was necessary but not sufficient — the blocker and the whole renderer read the separate shared list, which never included bitbucket. Adds it, gives Bitbucket its own provider name so review copy stops saying "GitHub", and asserts the two lists agree so they cannot drift apart again. * fix(bitbucket): persist pull request links after creation * fix(bitbucket): fetch linked pull requests by number first * fix(i18n): use generated Bitbucket integration keys * test(bitbucket): cover forge creation delegation * fix(bitbucket): fall back when linked pull request is stale * docs(bitbucket): explain notFoundIsNull and fix a garbled permissions comment notFoundIsNull arrived without the rationale its sibling flag carries, and reads as a bare `true` at the only call site that opts in. * fix(bitbucket): address review findings before merge Two of these made the feature unusable in real setups: - Create PR checked GitHub authentication for Bitbucket. isProviderAuthenticated fell through to isGitHubAuthenticated, which was unreachable while Bitbucket could not create reviews at all. Anyone with Bitbucket connected but no `gh auth login` got auth_required with no way forward. - The draft flag was only gated in ChecksPanel, not the two SourceControl call sites. With "create as draft" saved as a default, the composer hides the toggle for Bitbucket, so the flag could not be cleared and creation failed every time. Bitbucket now ignores draft instead of rejecting it. Also: - Blocked-create copy said "GitHub is not authenticated. Run gh auth login" on Bitbucket repos, in both the main-process and renderer paths. - A decryption failure resolved to an anonymous config and queried anyway; a private repo answers 404, which reads as "no pull request" and offers Create for a branch that already has one. Requests now fail closed. - Hiding non-open implicit branch matches was too broad: a declined PR became permanently invisible off the default branch. Scoped to merged, restoring the default-branch rule (#9171) for the rest. - A failed disconnect rejected unhandled and the card silently re-rendered as connected; a partial delete left the secret live in memory for the session. - The credentials dialog refused to open on a remote runtime, so a local repo could never store a credential. Now only the storage note changes, matching the Jira dialog. --------- Co-authored-by: devatnull <59279509+devatnull@users.noreply.github.com> |
||
|
OrcaWin
|
e6eec11b9f | fix(wsl): preserve single-letter POSIX terminal cwd (#13859) | ||
|
Brennan Benson
|
46756b386a |
fix(terminal): retain WebGL for recently hidden worktrees (#13372)
* fix(terminal): retain WebGL for recently hidden worktrees Suspending a hidden worktree disposes every pane's WebGL addon, so every switch-back presents DOM-renderer frames (unfloored ~5% wider advance, and the channel through which any poisoned cell metrics reach the screen) until reattach completes — 0.5-3s on loaded sessions. Keep the live addons for the most recently hidden worktrees instead: LRU capped at 6 contexts (Chromium allows ~16/page, visible worktrees use 1-4), least-recent evicted to dispose exactly as before. Reveal then has no renderer swap and nothing to flash. Window-occlusion callers pass no retention context and are unchanged. * fix(terminal): pause retained hidden cursor work * fix(terminal): preserve retained WebGL through deferred rebuilds |