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.
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).
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.
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.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
* 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.
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>
* 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
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).
* 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
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.
* 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>
* 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.
* 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>
* 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.
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.
* 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>
* 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
* fix(terminal): explain a dead terminal host instead of leaking a socket error
When the daemon that owned a session has exited, resuming that terminal surfaced
the raw connect error for its endpoint — on Windows a named pipe path, since a pipe
disappears with its server process — in a red toast ending in "file an issue". The
session is unrecoverable and the text is unactionable.
Translate it at the daemon boundary into terminal_host_gone, following the existing
terminal_pane_owner_unverified path, and humanize it in the toast. Suppress the
issue link for a condition Orca can fully explain.
Covers the remote-host case: the conversion sits in attachStablePaneOwner, the first
catch a paired client's resume reaches before the error is serialized onto the wire.
* fix(terminal): preserve host-gone errors across versions
* fix(terminal): scope host-gone error matching
* refactor(terminal): derive the host-gone test and replace forms from one source
Two regex literals matched the same token with subtly different shapes
(capturing vs non-capturing, consuming vs lookahead), so an edit to one could
silently drift from the other. Build both from a single source string; the test
form stays non-global so it carries no lastIndex state between calls.
* test(terminal): cover host-gone regex boundaries
* test(terminal): isolate leading host-gone boundaries
Neil
Jinjing
Brennan Benson
Jinwoo HongandE2E Test
OrcaWinandBrennan Benson
OrcaWin
Pr1p