mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 16:02:24 +00:00
f8d017a2ff55be745fcb35d23efe48dff91eee0e
8768
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo-H
|
f8d017a2ff | test(renderer): cover the late-mount republication clause in the worktree gate | ||
|
Jinwoo-H
|
b64d4162bb |
perf(renderer): rebuild runtime-graph snapshots only for changed worktrees
Publishing the runtime graph re-walked every accumulated worktree and every tab on every frame, regardless of how little had changed. On an 870-worktree / 1,740-tab session that is two full tab walks per title frame and 870 per-worktree snapshot rebuilds, when typically one worktree moved. Each worktree's snapshot now carries a fingerprint of the 25 store inputs its builders read. A publication reuses the cached snapshot for any worktree whose fingerprint is unchanged by reference, so per-frame work becomes O(what the frame changed) rather than O(session size). Slice scans (ambiguous tab ids, status grouping, the mounted-tab ownership index) are memoized on store-slice identity behind the same rule. This subsumes the memoization in #21075 rather than stacking on it: `collectAmbiguousTerminalTabIds` becomes a wrapper over the ownership index, and the agent-status grouping memo is re-keyed. #21075's key was `(agentStatusByPaneKey, tabsByWorktree)`, and a status frame replaces `agentStatusByPaneKey` by construction, so that memo missed on exactly the frame class it targeted — 1,740 tab reads and a 1,740-entry Map allocation per status frame for no saving. Measured with a tab-read counter on an 870-worktree fixture, reads per publication by frame class (#21075 -> this branch): pane-title 0 -> 0, agent-status 1,740 -> 0, tab-title 5,220 -> 6,960 on the cold walk but one walk instead of two thereafter. This is not a fix for typing latency. Attribution puts runtime-graph publication at 14.3% of renderer CPU and 6.3% of long-task time; deleting it outright moves typing p90 from 190ms to 134ms. What this buys is that the cost stops growing with session size. |
||
 Jinwoo Hong
|
4a86b2dc56 |
refactor(mobile): checked reply readers for files, dictation, host-screen and agent-history (step 7) (#21269)
* test(mobile): record main's file-preview and markdown-disk-fallback replies
Four of this branch's read sites had no malformed-reply coverage, so the reader
change would have had nothing to move at them. `familyGoldens` matrixes only the
first scenario of each family, and `files.preview-load`'s base is the grant-refresh
chain while `session.tab-documents`' is the served markdown tab — which left
`files.read` and `files.readPreview` on the worktree preview path, the artifact
image read, and the markdown tab's on-disk fallback recorded on their success path
only. This commit is the before picture, taken from main's own tree with no product
edit in it.
Three new families, five scenarios, ten goldens:
- `files.preview-worktree-text` / `files.preview-worktree-image` — `files.read` and
`files.readPreview` as the preview screen asks them for a worktree file.
- `files.preview-artifact-image` — `files.readTerminalArtifactPreview`.
- `session.markdown-disk-fallback` — the `files.read` leg a headless host's
`renderer_unavailable` sends the markdown tab down. It carries a second scenario
that serves `markdown.readTab`, because a matrix site needs a fulfilled reply
recorded somewhere in its own family to replay as the `normal` partition.
No existing scenario moved to a new family and no adapter changed, so every
pre-existing golden keeps its `adapterSha256` and `scenarioSha256`. Recorded in a
detached worktree at the manifest's pin (`4b876758d3`) with this manifest copied in;
the control is that all 748 pre-existing goldens came back byte-identical to
origin/main's, which `git diff
|
||
|
Jinwoo Hong
|
9de6f2c6cd |
test(terminal): bump the pane hook-order parity pin past #9035 (#21276)
* test(terminal): bump the pane hook-order parity pin past #9035 #9035 added a useRef and a useCallback to use-terminal-pane-foundation (search input ref, focus-search-input) without moving the parity pin, and its own PR run never executed the shard that holds it. Every PR opened since fails `tests node 24 7/8` on `expected 211 to have a length of 209`. The two hooks are in order behind the existing ones and useMemo stays at 8. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(terminal): re-pin the hook-order hash for the two #9035 hooks The count alone was not the pin: the flattened order is hashed too. The new order is the old one with useRef and useCallback inserted at the foundation stage and nothing else moved (diffed before and after #9035). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
 Brennan Benson
|
b66ef2e8a8 |
fix(agent-launch): resolve a launch scope, not a git worktree record (#21193)
* fix(agent-launch): resolve a launch scope, not a git worktree record `agent.launch` asked the runtime for a managed worktree record and then read exactly one field off it, `.id`. That record does not exist for every workspace a launch can run in, so the request refused launches the method could otherwise run: the floating workspace resolves to a scope with an id and a path but no worktree row, and `showManagedTerminalWorkspace` throws `selector_not_found` rather than hand back the id it had already resolved. A folder workspace survived that only because the resolver fabricates a worktree row for it. The scope is the answer that is real for all three kinds, so the launch asks for that instead. `showManagedTerminalWorkspace` is unchanged - callers that genuinely need the git record still get it, and still get the refusal. With floating now reaching the mode decision, the host must know which kind of workspace it resolved. The kind is derived from the id it resolved itself, never accepted from a caller, and the route module's existing `floating` blocker does the rest: a workspace with nowhere to keep a session runs a terminal agent. Behaviour change, deliberate: a floating-workspace `agent.launch` used to fail with `selector_not_found` and now succeeds as a terminal agent. That is what lets the floating titlebar agent button move onto the shared launch command instead of driving tab startup itself. No wire change: `AgentLaunchTarget` is untouched. * test(agent-launch): cover floating RPC workspace resolution |
||
|
Brennan Benson
|
434365d2de |
Offer to reconnect native chats that were working when Orca restarted (#21096)
* feat(native-chat): resume structured chats that were working at restart
Teardown records a marker for every session this host was genuinely running a
turn for, derived from the LIVE runtime rather than a persisted status row, so
a stale `running` row left by an older crash can never trigger a resume. On the
next launch a modal lists exactly which chats would resume and resumes them via
native continuation (Claude resume/resumeSessionAt, Codex thread id) — never by
re-sending the prompt, which is what makes an agent redo finished work.
A session resumes only when all of these hold: a teardown marker exists and has
not expired, the record's lease is released and reconciled, a provider resume
cursor exists and still matches the marker, the journal's own turn record names
the same turn, and the marker has not already been spent. Markers are consumed
before the resume is submitted, so a crash mid-resume cannot double-fire, and an
admission gate refuses a second concurrent resume for one session. Resumes are
staggered three at a time rather than spawning every provider at once.
The modal's "Don't ask again" checkbox writes the nativeChatResumeWorkOnRestart
setting, which Settings can turn back off; automatic mode runs the identical
predicate and staggering and reports what it did. Declining consumes the markers
so the prompt cannot return every launch — nothing is lost, because opening a
chat still re-acquires it at the same cursor.
* fix(native-chat): compare handle ROOT and turn state when offering a resume
Four defects QA found in the restart-resume offer, fixed together because the
first two interact: shipping the root fix without the state fix would convert a
silent no-op into actively offering finished chats.
1. Claude was never offered (0/4). The marker recorded agentSessionProviderHandleKey,
which embeds Claude's leaf uuid — a branch cursor. The adapter's own close path
appends a `resumed` link with an advanced leaf during the SAME teardown, so the
marker went stale seconds after it was written and the drift guard refused every
Claude session forever. Record and compare agentSessionProviderHandleRoot instead:
the root is the part a resume must preserve, and changing it is a fork, which is
exactly what this guard is for. Codex is unaffected (its thread id is the whole
key) but uses the root too, so the rule is uniform.
2. The predicate compared turn IDENTITY but discarded turn STATE, so a `completed`
turn satisfied it as readily as an interrupted one. Eviction rewrites `running`
to `interrupted` and never to `completed`, so the state is what separates work
that was cut off from work that finished. Require `interrupted` or `unverifiable`.
3. A chat blocked on a pending approval or question was marked as working, because
the teardown reader accepted any `running` turn while the product's own projection
calls that state `attention`. Teardown now defers to that projection: an agent
waiting on the USER is not interrupted work.
4. "Resume all" could silently no-op. The modal fetched candidates at mount; by click
time the chat's own pane may have bound and taken the hold, moving the lease to
`live` so the predicate dropped it and the call returned no results, leaving the
dialog open behind a dead button. Re-derive at click time and settle an
already-live session as resumed — it is running, which is what the user asked for.
Test fakes now model the Claude close path that advances the leaf, which is why no
unit test could previously exhibit defect 1. Ablation covers all eleven guards.
* fix(native-chat): gate the already-live settlement on the full resume predicate
Two follow-ups from re-QA, both cases of a rule stated by intent rather than by
discriminator.
1. The already-live path bypassed the predicate. "Resume all" sends no session
ids, so the fallback's target set was every marker, and it was gated only on
the session having a live provider child. A chat the predicate had refused --
a completed turn, say -- whose pane happened to own the lease was therefore
settled as `already_live` and had its marker spent, inflating the "Resumed N"
count with chats that were never eligible. No provider spawned and no tokens
were spent, but a marker the predicate rejected must never be consumed.
The resumable set now takes an explicit `leaseState`. The already-live path
derives a second set with ONLY the released-lease clause relaxed, and settles
a session just when it is in that set. Every other clause still applies.
2. The `attention` rule was one-sided. Teardown refuses to mint a marker for a
chat blocked on the user, but the set predicate had no equivalent, so a marker
arriving by any other route was offered once eviction rewrote its turn to
`interrupted` -- the same asymmetry the completed-turn case had.
Gated on projectStructuredAgentSessionStatus === 'attention'. That projection
tests for a pending approval or question BEFORE it looks at turn state, so it
still reports `attention` after the turn is settled, which makes it the durable
signal and keeps one source of truth with teardown.
Ablation now covers thirteen guards, including one for each of the above.
* fix(native-chat): capture awaits-user on the marker instead of re-deriving it
The awaits-user clause could never fire. It asked the live projection for
`attention`, which needs a prompt whose resolution is still `pending` -- but
teardown CANCELS that prompt a few phases after it writes the marker. By the next
launch the evidence is gone, for precisely the sessions the clause was written
for. QA measured the injection still being offered and then resumed.
This is the same shape as the leaf-drift bug: state read after teardown is not the
state that justified the marker. The discriminator, now applied across the whole
predicate:
- a fact teardown itself destroys or mutates must be CAPTURED on the marker
while it is still true;
- a fact that evolves on its own must be RE-DERIVED at read time, never
snapshotted.
So `awaitsUser` is now recorded at teardown and the predicate reads the recorded
value. Teardown still declines to mint a marker for such a session, so the
recorded flag is the second line rather than the only one.
Audit of every other clause against the same test:
- turn id (captured) -- teardown rewrites turn STATE but never the id. Correct.
- provider handle root (captured) -- the close path appends a resumed link, and
appendAgentSessionProviderHandleLink refuses one that changes the root, so the
root is invariant under exactly the mutation that broke the key. Correct.
- turn state (re-derived) -- DELIBERATE exception, stated here rather than left
implicit: we are not reading the state that justified the marker, we are
reading teardown's receipt that it settled the turn. A turn still `running`
means eviction never finished, and we refuse. Correct, and intentionally so.
- lease reconciled / released / handoff stage (re-derived) -- these answer a
different, launch-time question: may this host take the lease NOW. The
teardown-time value would be meaningless, and `unreconciled` is cleared by
this launch's own reconciliation. Correct.
- adapter support, marker TTL, marker consumption (re-derived) -- all evolve
independently of teardown. Correct.
Only awaitsUser was on the wrong side.
* fix(native-chat): drop the unreachable awaits-user marker flag
The captured flag was dead code. `awaitsUser` could only be true when the
projected status was `attention`, and `attention` hits the `continue` above the
push -- so every marker teardown can ever write carries `false` (QA measured
22 of 22 across two real teardowns). The predicate clause reading it was
unreachable by any production path.
A flag that is structurally always false is worse than no flag: it reads as a
safeguard, so the next person to touch this trusts it. The asymmetry it was
added to close was only ever reachable by fault injection, because teardown is
the sole writer of markers and already refuses attention sessions.
Removing it also drops an upgrade discontinuity: as a required field it made a
marker written by the previous build fail validation and be silently discarded,
costing a resume offer on precisely the upgrade where the user was mid-turn.
Markers predating the providerHandleRoot rename still will not parse, but those
carry a leaf-sensitive key the predicate would refuse anyway, so nothing usable
is lost.
In its place the teardown gate now states that `status !== 'working'` is the
SINGLE gate for awaiting-user sessions, why a predicate-side mirror would be
unreachable, and why it could not even re-derive the fact -- so the reasoning is
inherited rather than rediscovered.
Ablation is back to twelve guards; every other clause is unchanged.
* fix(native-chat): say reconnect, not resume, and show each offer's age
Two changes, both independent of the parked continuation decision.
1. The copy claimed something QA disproved. "Resuming continues each agent where
it left off" is false: reconnection restores the session at the point it
stopped, with full context and without re-sending the prompt, but the
interrupted reply does not continue on its own. The toast's "Resumed N chats"
implied work had restarted.
Audited every user-facing string against the rule that none may claim work
continues or that a reply resumes -- which caught more than the three strings
the fix started from. The title, the row button, "Resume all", "Resuming...",
the not-now hint ("picks it up where it left off"), the checkbox and its hint
("resume on their own"), the list's aria-label and the Settings row all made
the same claim. The user-facing verb is now reconnect throughout; the body and
update variant state outright that the interrupted reply will not continue.
en.json synced, runtime boot catalog regenerated.
If we later decide to send a continuation instruction, this is one commit to
change back. Shipping text we know to be false was the worse option.
2. Rows now show each offer's age. The TTL is 24 hours and a stale offer looked
identical to a fresh one. The marker already carried `recordedAt`, so this is
a render change plus one field on the renderer's candidate type, formatted
with the existing formatUiRelativeTime helper rather than a new one.
The clock is stamped once when the list arrives rather than read during render:
ages then stay stable across re-renders, and the render stays pure, which the
react(purity) rule requires.
Guards, predicate and RPC are untouched; ablation still covers twelve.
* feat(native-chat): show the workspace name on each reconnect row
A row read `codex · folder:8f3a1c22-… · 8 hours ago`. Recognising which chats
would reconnect is the entire point of the list, and at twenty rows a UUID
identifies nothing.
No RPC or host change was needed: the renderer can already resolve this id.
Resolved the way automation dispatch resolves the same id space
(resolveAutomationDispatchWorkspace) -- a folder workspace by its full
`folder:<uuid>` key via getKnownWorktreeById, a git worktree by its bare
`repoId::path` id via allWorktrees. Both return a Worktree, whose displayName is
a required field, and DetectedWorktree extends Worktree so either shape answers.
Falls back to the id when nothing resolves, which is what the row showed before
and also covers the window before the worktree store has hydrated.
The lookup lives in a per-row subcomponent because a hook cannot run inside
`map`, and its selector returns a primitive string so repeated selector runs
cannot churn referential equality.
* feat(native-chat): group the reconnect modal by worktree and add opt-in continuation
Grouping. Rows are now grouped under a worktree heading with the repo glyph and
an agent count, using the sidebar's own collapse mechanics. Only presentational
pieces are reused -- RepoIconGlyph, CompactAgentExpansion, AgentIcon and
formatShortTimeAgo. The sidebar's agent row cannot be: worktree-card-compact-agent-row
imports DashboardAgentRow, the dashboard's own type, so both surfaces render one
live-agent model requiring a pane, tab and status entry. Every chat offered here
is by definition stopped, so supplying that would mean inventing live state.
Two things I had assumed were reusable and were not:
- DashboardHostBadge returns null unless hostKind is ssh or remote. Structured
chat is local-only, so it would always render nothing. The host line is
omitted rather than faked; the badge is the right element to add if and when
structured chat gains remote support.
- No state dot. Every AgentDotState misleads here: idle and unverifiable both
presuppose a live pane, interrupted renders red like an error, done green,
working a spinner. A missing dot beats one saying these agents are running.
One worktree renders flat with no heading -- a name, count and chevron around a
single group says nothing the dialog has not already said.
The age column now uses formatShortTimeAgo for sidebar consistency. It takes
(timestamp, now) and subtracts internally rather than taking a delta, so the call
is (recordedAt, listedAt); passing the old delta would have rendered plausible
nonsense. The clock is still stamped once into state, so ages stay stable and the
render stays pure.
Continuation. A secondary "Reconnect and continue" action sends one message, from
a single shared constant, identical for both providers. Reconnect is unchanged and
still sends nothing. An info popover quotes the literal message read from that
same constant, so what is shown cannot drift from what is sent.
Ablation now covers fourteen guards. Two are new: continuation only follows a
reconnect that actually happened, and -- inversely -- a send injected into the
reconnect path must turn the test red, since "don't ask again" rests on reconnect
never sending.
* feat(native-chat): say terminal sessions kept running, and clear the quality gate
The modal lists stopped chats with no way to tell that CLI agents are fine, and
the true state of the world is counterintuitive: the terminal sessions survived
the restart and the chats did not. One line now says so, next to the heading
where it frames the list rather than as a footnote at the bottom.
Wording follows the app's own vocabulary rather than inventing a term: the
catalog settles on "terminal sessions" (terminalSessionCount, "Terminal sessions
are grouped by workspace", "No terminal sessions yet"), and UpdateCard already
reassures with "Your terminal sessions won't be interrupted during the update" in
the same text-xs text-muted-foreground treatment. "kept running" rather than
"were restored" -- nothing reconnected them, they never stopped, and the line
says nothing about why.
Also clears check:code-quality:changed, which I had not been running -- oxlint
alone covers neither the design-system nor the casting audit, so 18 findings had
accumulated across the branch.
- design system (4): Button spacing hand-rolled as gap-1/px-2 is just size="xs";
PopoverContent and DialogTitle own their typography and spacing, so the
text-xs moved to the popover's own children and the title's icon gap moved to
a plain wrapper.
- casting (14): production code loses its assertions outright via Reflect.get,
the idiom already used in managed-hook-detection-commands and
worktree-name-retirement. The marker validator reads each field through
Reflect.get and now checks recordedAt is a number rather than asserting it;
the store-file parse uses the existing `file` shape instead of a second
assertion; the runner narrows the admission error's owner with typeof.
Test fixtures keep their assertions behind the line-specific SAFETY:
rationale the repo mandates for exactly this case.
One trap worth recording: the audit reports an assertion at the line its
EXPRESSION OPENS, not where `as` appears, so a disable-next-line above the
closing brace of a multi-line literal is inert and silently changes nothing.
Guards unchanged; ablation re-proved 14/14 at this head.
* fix(native-chat): give the reconnect row's provider icon an accessible name
Every row rendered the provider as a bare AgentIcon, whose svg carries no
aria-label, title or alt. With a Claude chat and a Codex chat in one worktree the
two rows were identical to any non-visual consumer, and the dialog offered
several identically-named "Reconnect" buttons with nothing to tell them apart.
A regression from
|
||
  Jinjingandgum798
|
e6dcb8b938 |
fix(editor): keep preview Add-note controls out of PDF export (#21268)
Exporting Markdown to PDF from Preview printed an Add-note + button above every block. Preview exports the .markdown-body subtree, and its per-block annotation control renders inside that subtree, so the clone-scrub pass never removed it. Mark the controls container with data-orca-export-hide at the source, add the explicit class to UI_ONLY_SELECTORS (attr-strip fallback; generic attr covers renames), and hide it in EXPORT_CSS as a belt-and-suspenders backstop. Review note bodies and open composer drafts are transient review state and are intentionally excluded from the document PDF. Fixes #21198 / STA-7761 Attribution: diagnosis and core scrub entry by @gum798 (PR #21199, closed in favor of this PR) Co-authored-by: gum798 <33922655+gum798@users.noreply.github.com> |
||
|
Brennan Benson
|
7a1f55c52a |
fix(native-chat): give a failed Claude background task a typed row instead of an opcode (#20519)
* fix(native-chat): give a failed Claude background task a typed row instead of an opcode
A failed backgrounded command printed red rows whose visible text was the wire
opcode, and printed one failure twice. All five task lifecycle kinds are
catalogued status-chrome, but the payload sniffer in classifyProviderFrame runs
first and promotes any frame reporting a failure to the generic unknown-frame
fallback, whose sentence lookup has no key for the field Claude puts its own
sentence in. Two frames for one task therefore produced two rows, both of them
the method name.
Suppressing those frames is not the fix: when the last background task settles
the tracker flushes it and the strip unmounts, local_bash is excluded from the
subagent roster, and the status feed publishes only live tasks, so for a lone
backgrounded command the transcript row is the only report of the failure that
exists anywhere.
So the catalogue now binds: kinds a dedicated typed translator owns are named
as covered, and the generic fallback refuses to emit for them in either
direction. A new row owner keeps one durable row per task id, opened by the
announcement, revised in place by the lifecycle frames and closed by the
notification, carrying the provider's summary, error, output path, usage and a
run state. The row is written on the same dual carrier the subagent roster
uses: a frozen text twin plus a typed block, so a client without the block type
reads the sentence rather than nothing.
hasProviderError keeps its authority everywhere else, unchanged.
* fix(native-chat): keep tool attribution across a background-task row
A background task's row is a system message landing mid-turn between the
assistant's tool calls, exactly where the spawn-group roster row lands. Without
the same exemption it ended the run the following tool messages fold into, so a
tool result arriving after one stopped folding into its own assistant turn.
Also syncs the catalog with the row's one new translate key.
* fix(native-chat): harden background task rows
* fix(native-chat): settle background rows on provider end
* fix(native-chat): scope malformed task fallback text
* test(native-chat): assert only eligibility at the disposition layer
The malformed-task-frame test asserted the generic fallback resolves Claude's
`summary` field itself, which was true only while that key sat in the shared
key list. Eligibility is what this layer decides; the sentence the row leads
with is Claude's, supplied through the display-text seam and proven in the
translation test.
* fix(native-chat): gate background-task admission and scope rows per run
Admission now matches the reference on all three gates. Type is the whole gate
and MONITORS ARE NOT ADMITTED: a monitor runs for the life of the session and
has no outcome a row could report, so it never reaches the timeline. On first
admission only, the task's tool_use_id must name a tool call this session
forwarded at the TOP level — a Task spawned inside a subagent's sidechain names
an id that never reached the transcript, and a top-level row for it would claim
an invocation the user never saw. And a task that already exists and has not
finished is not re-opened: a duplicate announcement is a redelivery, not a
second run.
Rows are now keyed per RUN. A provider may reuse a task id for a distinct later
invocation, and a row keyed by the id alone overwrote the first run's transcript
history instead of leaving it standing. Generation 1 keeps the bare key, so
every row already written is unaffected.
The spawning tool call is carried on the row as parentToolUseId. Orca's journal
has no structural parent link for an item — AgentJournalItemIdentity has four
arms and none carries one — so the relationship is data on the item rather than
nesting.
A terminal frame that names NO tool still opens a row. That is a named
deviation, recorded at its call site, and the measurement behind it is in the PR.
* fix(native-chat): read the aggregate roster by membership, not a phantom status
The background-tasks payload types every entry as exactly
{task_id, task_type, description, ambient?}. It has no per-entry status, so the
state this owner derived from one was always undefined and the reopen branch it
guarded was unreachable on every real payload — proven by deriving the state
from an SDK-shaped entry and getting null.
Membership is the only liveness the payload carries: it is the whole live set
after a change, so presence means live and absence means merely "no longer
listed", never an outcome. Presence does not revive a settled row either — the
level's ordering against the start/stop edges is unspecified and it carries no
evidence of a new run, so the task's own frames stay the only thing that opens
or settles one. Only the identity fields it really sends are read, and ambient
housekeeping entries are excluded as the payload asks.
The two helpers that served the dead branch are removed, along with the test
that exercised it through a synthetic status the CLI cannot send.
* fix(native-chat): mirror reference task admission and drop the synthesis path
The forwarded-parent gate is conditional on the field being PRESENT. An
announcement naming a tool this session never forwarded is a nested child and is
refused; one naming no tool at all is admitted, because absence of the field is
not evidence of an unforwarded parent. The previous rule required the field and
so refused every tool-less task.
Terminal frames now match on task_id alone. The forwarded-parent question is
settled once, at admission, and is never re-asked on a notification or a patch.
A frame for a task that was never admitted yields no row, and a patch is folded
into the row it names rather than opening one.
That removes the synthesized-row path entirely, and with it the named deviation
it carried: the captured tool-less failure lands on a row that already exists,
because its own tool-less announcement is admitted. The dead builders go with
it.
Left deliberately stricter than the reference, and flagged rather than changed:
a terminal frame still records its task id as terminal even for a task never
admitted, so a late announcement cannot open a row for work already reported
finished. Two existing tests pin that.
* fix(native-chat): preserve background task ownership across restarts
* chore: restore pnpm-lock.yaml to origin/main
A local pnpm run rewrote the lockfile and the merge commit swept it in. The
branch changes no dependencies, so it must carry no lockfile delta at all.
* fix(claude): harden background task lifecycle
* fix(claude): bound task generation history
* fix(claude): preserve task identity after history eviction
* fix(claude): resolve background task identity after rebind
* fix(claude): isolate queued task runs
* refactor(claude): give the background-task ledgers one bounded owner
The bounded collections behind a background-task row were read out of the
class with `Reflect.get` to prove they stay capped, which the anti-slop
gate rejects. Move them into `ClaudeBackgroundTaskLedgers`, which owns
the caps beside the eviction helpers and reports a typed readonly size
view the tests assert against.
Also replace a `Reflect.get` in the mobile recording proxy with typed
property access.
* fix(native-chat): report a background task failure the transcript never admitted
A terminal `task_notification` for a task no announcement ever admitted rendered
nothing at all. The typed row owner declined the row because its map held no
entry for the id, and reported the frame as handled — which is exactly what
tells the generic provider-frame fallback to stay quiet. Both surfaces declined
the same frame, so a real failed background task was dropped on the floor.
A terminal frame is self-sufficient: it states an outcome, and it carries the
summary, status, error, output path and usage that outcome needs. It now opens
its own row from those fields, with the summary as the label and `unknown` as
the kind when the frame names no task type. The row map enriches a terminal
frame; it never gates one. Every terminal status writes one, not failures alone,
so there is one rule here rather than a third behaviour for failures.
The deliberate hand-offs still win, because they are recorded rather than
implied: ambient, subagent and foreground tasks are claimed in the foreign-owner
ledger the notification path already checks first. A Task spawned inside a
subagent's sidechain now records its refusal there too, under `sidechain`,
instead of leaving no trace and reading as a task nothing ever decided about. A
capacity-refused task whose outcome the generic fallback already printed records
`fallback` the same way, so a redelivery neither prints twice nor mints the row
capacity refused.
The anti-resurrection guard stays and stays scoped to announcements: a late
`task_started` cannot reopen work already reported finished. The restart rule is
now stated once instead of twice — a different parent alias is the provider's
restart signal only when BOTH runs name their parent, which is what the terminal
ledger already required of an evicted row and what the live row now requires too.
* test(native-chat): pin that an orphan task row reopens the provider's turn
* fix(native-chat): stop an orphan task row drawing its sentence twice
An orphan row took its header label from the notification's summary, which
also renders as the row's sentence, so the same string appeared in both slots
of the same collapsed row. The label now stays empty and the header falls back
to the task kind, leaving the sentence to carry the provider's words.
* fix(native-chat): keep Claude task outcomes owned through capacity and redelivery
* fix(native-chat): keep settled overflow notifications from reopening a turn
* fix(native-chat): retain Claude task rows through journal pressure
|
||
|
Brennan Benson
|
abc8386e14 |
fix(mobile): name a create's launch so a lost reply cannot build two workspaces (#21137)
* fix(mobile): name a create's launch so a lost reply cannot build two workspaces `agent.launch` admits a caller-supplied `operationId` through a durable ledger, so exactly one execution happens and every replay returns the recorded answer. No client sent one, so the machinery was inert and the original defect was still live: mobile retries a lost create by design, and a retried launch built a second agent in a second workspace. Mobile now mints an operation id per create candidate and sends it whenever the host advertises `agent.launch.replay.v1`. The invariant is one operation per candidate. `computeAgentLaunchFingerprint` folds `target` whole, so the workspace name is inside the fingerprint; carrying one id across a name-collision bump would meet its own row under a differing fingerprint and refuse `agent_session_operation_conflict`, failing the create outright on the second candidate. The id is therefore minted beside `clientMutationId` at the top of each loop iteration and reused verbatim by every retry arm inside that candidate — never re-minted, since a new id is a new operation. Admission runs ahead of every effect, so `_invalid` / `_expired` / `_capacity` prove nothing launched: those re-send the same candidate unnamed rather than let bookkeeping fail a create the host would have performed. `_unknown` is the one refusal that is not safe to re-send, and it surfaces. Also corrects a false comment: the legacy path caches the whole launch under `clientMutationId`, so inside its 60s window a replay adds neither a workspace nor a surface, and outside it adds both — not "a second surface, never a second workspace". * fix(mobile): preserve launch identity on refusals * fix(mobile): use launch receipts to authorize replay * test: move mobile launch replay coverage outside node project * fix(mobile): enforce replay-safe launch delivery at the host * test: run mobile launch contracts in mobile checks * test: cover mobile launch contract workflow dependencies |
||
  Shahar MorandNeil
|
de15227a1d |
feat(terminal): search match count + Cmd+F focus parity (#9035)
* feat(terminal): show search match count and keep Cmd+F from closing search Bring the terminal search bar to parity with the editor find bars: - Show a live match indicator (0/0, current/total, "No results", or <count>+ past the highlight limit) driven by the xterm SearchAddon onDidChangeResults event. - A repeat Cmd+F while the search is open now re-focuses and selects the query instead of toggling the panel closed; Esc remains the close path. Adds unit coverage for the indicator states and the toggle decision. * Use auto-generated localization key for TerminalSearch no-results (#9035) Replace the hand-written "noResults" i18n key with the SHA1-based auto key (auto.components.TerminalSearch.10e039b591) to match the repo's auto-keying convention, and sync the key across all locale catalogs. Addresses CodeRabbit review feedback. * test(terminal): cover search dispatch after keyboard module split * fix(terminal): refocus search from its input and verify real matches * test(terminal): use portable echo commands for search proof * refactor(terminal): keep search subscription and cleanup together --------- Co-authored-by: Neil <neil@stably.ai> |
||
 Neil
|
96eb97aad6 |
fix(runtime): split the host-contact epoch out of the connection generation (#20359)
* fix(runtime): split the host-contact epoch out of the connection generation `connectionGeneration` carried two meanings and one reader was always wrong. holding the session mirror through an outage leaves its subscriptions stranded and that edge was the only thing left to revive them. But the same value is the mirror's cache key -- use-runtime-session-mirror-environment-key.ts keys the subscription effect on it, every published frame is stamped with it, and web-session-terminal-retirement-proof-ledger.ts drops retained proofs when it moves. So the bump #20085 needed as a resubscribe signal re-keyed and rebuilt the mirror after any brief flap, which is the #19647 symptom #19873/#20059 fix. Measured first: with the reconnect bump deleted, an ended stream followed by recovery issues zero resubscribes, and the mirror's subscribe call registers no `onClose`, so main's terminal close is dropped. #20085's claim is true -- the subscription really is dead after recovery -- so the trigger has to exist. It just must not be the cache key. Give each meaning its own value: - `connectionGeneration` returns to identity only: a new runtime session, a re-pair, an explicit clear. A same-runtime return no longer moves it, so no stamp, fence or retained proof is invalidated by a flap. - `hostContactEpoch` counts "the host answered again after we lost contact". It lives on the store entry and is read only as a dependency of the two subscription effects in use-web-session-tabs-sync.ts -- never passed to an installer, never part of `environmentKey`, so it cannot become a stamp. `useRuntimeSessionMirrorEnvironmentKey` becomes `useRuntimeSessionMirrorEnvironmentKeys`, returning `environmentKey` (identity) and `resubscribeSignal` (the epoch edge) from the one target scan, so the hot ownership scan is not doubled. Each direction is pinned by its own test: removing the resubscribe dependency fails only 'reinstalls both session-tabs subscriptions when the host answers again'; restoring the reconnect bump fails only the two key-stability tests. * test(runtime): pin the mirror hydration verdict across a host flap The generation tests assert the key string; this asserts what the user feels. The mirror's hydration verdict is stamped with the connection generation, so any bump discards it and every mirrored pane re-parks -- the tab-list rebuild. Held across an unverifiable probe, still discarded when the runtime id actually moved. * test(runtime): build real host statuses instead of casting partials |
||
|
Neil
|
851befa929 |
fix(runtime): hold parking and transport reads through an unverifiable probe (#20096)
* fix(runtime): hold parking and transport reads through an unverifiable probe Three remaining sites where a non-verified status probe was read as evidence the host is gone, per docs/reference/ssh-execution-boundary.md. - runtime-status-refresh published its own copy of the "null the status unless verified" rule, then handed the snapshot to applyRuntimeHostStatusSnapshot, which re-derived it. The copy was dead but free to drift; the snapshot branch now calls applyRuntimeHostStatusSnapshot directly, leaving one implementation. - The paired-parking capability reads treated a nulled status as "host cannot park", so a transient probe failure unparked live paired terminals and dropped a parked session's reattach in favour of a fresh cold restore. Both now read lastVerifiedRuntimeStatus; a capability is a fact about the host's build. - runtimeHostConnectionStateForEntry handled transport 'disconnected' and 'ready' and let 'connecting'/'unknown' fall through to the default 'disconnected' — reporting a host mid-handshake as down, a worse verdict than an actually disconnected transport gets. It now passes the snapshot's transport through. * fix(runtime): keep a revoked host out of the parking promise Holding a capability through an unverifiable probe is right; holding it through the host's own refusal is not. `blocked` (auth rejected, protocol mismatch) stops every retry for good, and parking trades the client's only copy of the scrollback for a host-side restore that can then never happen -- the destructive direction. `isRuntimeHostContactRevoked` names that one terminal verdict once, and the connection-state derivation now reads it too so there is a single definition. Also narrows the transport hint to 'connecting'. 'unknown' means no transport was ever attempted, which is the permanent state of an unreachable paired host: as 'checking' its row lost its Connect action and the status bar read "connecting" for the whole session. * test(runtime): pin the parking gate against over-firing on a flap |
||
|
Neil
|
182ff17141 |
fix(runtime): hold four more host reads through an unverifiable probe (#20095)
* fix(runtime): hold the session mirror through an unverifiable probe Two derivations read the same host state and reached opposite verdicts, and the destructive one won. When a status probe came back unverifiable over a still-ready transport, runtimeHostConnectionStateForEntry called the host 'runtime-unavailable' (connected) while getReachableRuntimeSessionMirrorTargets dropped it, tearing down and cold-rebuilding the session-tab mirror while the host's flows were still delivering. The root cause is that applyRuntimeHostStatusSnapshot nulls entry.status for any non-verified probe while the snapshot retains the runtime identity. The connection-state reader consults the snapshot; the mirror-target reader did not. Give both readers one answer: - lastVerifiedRuntimeStatus() in shared/runtime-host-status.ts is now the single definition of "the last identity the host answered with". runtime-status.ts already had this inline as previousVerifiedStatus and now calls it. - The mirror-target reader asks the shared connection verdict instead of entry.status, gated on isDisconnectedRuntimeHostState -- only the one exit verdict earns a destructive read, per docs/reference/ssh-execution-boundary.md. 'checking' and 'reconnecting' are unverifiable, not evidence of an exit. Holding the target through the outage would strand the mirror on its own: the subscription is installed by the effect in use-web-session-tabs-sync.ts keyed on useRuntimeSessionMirrorEnvironmentKey(), a stream 'end' frame is dropped without resubscribing, and the parking layer retries only a rejected subscribe call. The teardown was the recovery. So regaining contact now advances the connection epoch, giving recovery its own "the host is back" trigger rather than leaving the mirror to be restored as a side effect of having been destroyed. The connection epoch is not the runtime session: a same-runtime return fires no restart hook, no provider session bump, and no toast. * test(runtime): drop the redundant status casts the new casting gate rejects * fix(runtime): hold four more host reads through an unverifiable probe Siblings of the session-mirror defect fixed in #20085. runtime-status-snapshot nulls `entry.status` for any non-verified probe while the snapshot retains the host's identity, so a host with a ready transport that is still delivering reads as gone to anything gating on `entry.status != null`. - client-event subscription selection dropped the stream for such a host, and its disconnect edge bumped the SSH generation, rebuilding even the active host's subscription - the web client's active session-tabs stream tore down and cold-rebuilt, twice per blip - landing preflight discarded its whole result - runtime-aware SSH selectors blanked mirrored target rows Each now reads the shared verdict, isConnectedRuntimeHostState of runtimeHostConnectionStateForEntry, or lastVerifiedRuntimeStatus where the read is host identity rather than reachability. No new predicate: every "genuinely gone" case is byte-identical, so nothing gains a retry loop. * test(runtime): build real host statuses instead of casting partials |
||
|
Neil
|
67dda9affe |
fix(runtime): hold the session mirror through an unverifiable probe (#20085)
* fix(runtime): hold the session mirror through an unverifiable probe Two derivations read the same host state and reached opposite verdicts, and the destructive one won. When a status probe came back unverifiable over a still-ready transport, runtimeHostConnectionStateForEntry called the host 'runtime-unavailable' (connected) while getReachableRuntimeSessionMirrorTargets dropped it, tearing down and cold-rebuilding the session-tab mirror while the host's flows were still delivering. The root cause is that applyRuntimeHostStatusSnapshot nulls entry.status for any non-verified probe while the snapshot retains the runtime identity. The connection-state reader consults the snapshot; the mirror-target reader did not. Give both readers one answer: - lastVerifiedRuntimeStatus() in shared/runtime-host-status.ts is now the single definition of "the last identity the host answered with". runtime-status.ts already had this inline as previousVerifiedStatus and now calls it. - The mirror-target reader asks the shared connection verdict instead of entry.status, gated on isDisconnectedRuntimeHostState -- only the one exit verdict earns a destructive read, per docs/reference/ssh-execution-boundary.md. 'checking' and 'reconnecting' are unverifiable, not evidence of an exit. Holding the target through the outage would strand the mirror on its own: the subscription is installed by the effect in use-web-session-tabs-sync.ts keyed on useRuntimeSessionMirrorEnvironmentKey(), a stream 'end' frame is dropped without resubscribing, and the parking layer retries only a rejected subscribe call. The teardown was the recovery. So regaining contact now advances the connection epoch, giving recovery its own "the host is back" trigger rather than leaving the mirror to be restored as a side effect of having been destroyed. The connection epoch is not the runtime session: a same-runtime return fires no restart hook, no provider session bump, and no toast. * test(runtime): drop the redundant status casts the new casting gate rejects |
||
|
Jinjing
|
25dd70e611 |
Test: target question card title by testid instead of text (#21153)
* test: target question card title by testid instead of text Add data-testid to NativeChatQuestionCard's title element and update the e2e test to query by testid with a text filter. The transcript row also renders the question text, so the previous test could match either location, causing flaky results. Gating on the card's own title node ensures the assertion verifies the card is actually rendered. Fixes #20724 * test(browser-history): budget the fastest sample, not p95 The prepare/match budget assertions measure wall clock inside a vitest worker that shares cores with the rest of the shard, so a slow sample records a preemption rather than the matcher. CI shard 6/8 measured a p95 of 3.57 ms against the 2 ms ceiling while the same test passes in isolation; #18788 already records this file failing the same way. Assert the fastest sample instead, matching the estimator the palette matcher budget already uses for the same reason. Ceilings stay at 2 ms. Measured locally (20 samples per batch): the fastest sample moved 0.05 ms -> 0.08 ms between idle and a 3,374-file parallel run, while p95 of those same batches swung 0.09 ms -> 0.50 ms. * test: target question card title by testid instead of text Add data-testid to NativeChatQuestionCard's title element and update the e2e test to query by testid with a text filter. The transcript row also renders the question text, so the previous test could match either location, causing flaky results. Gating on the card's own title node ensures the assertion verifies the card is actually rendered. Fixes #20724 |
||
|
Neil
|
779667c1e7 | refactor(runtime): declare the host-status entry once instead of per consumer (#20262) | ||
|
Brennan Benson
|
73b33302b9 |
Show the Source Control AI CLI arguments box only where it actually works (#21149)
* Show the Source Control AI CLI arguments field only where it applies * Fix Source Control arguments on remote launches |
||
|
Brennan Benson
|
e42f7c00bd |
feat(native-chat): render a proposed plan as a plan, not a generic approval (#21090)
* feat(native-chat): render a proposed plan as a plan, not a generic approval A finished plan arrives as an ExitPlanMode tool call. With no handling for it, the generic approval path serialized the tool input, so a plan appeared as thousands of characters of escaped JSON. A plan is content to read, not a privilege to grant. Classify the plan in the permission callback and carry it as a typed subject on the approval item, keeping the existing approval kind so the prompt still reaches every consumer. Mobile filters pending approvals on that kind, so introducing a new one would have made the prompt vanish there silently. Classification runs before registration, so a future permission-mode short-circuit cannot swallow a plan proposal. The assistant tool-use stream is a second ingress and is pinned by its own test, because neither path can be assumed to fire on its own. Rather than adding a second card, the plan renders inside the approval card's existing bounded content region. It inherits the height cap, the scrolling, the keyboard focus and the pinned action row that region already provides, and a typed plan replaces the raw detail instead of rendering both. Buttons read as plan decisions. Mobile renders the same subject through its own markdown component in the same region. * fix(native-chat): preserve plan review semantics * fix(native-chat): keep plan approval one-turn |
||
|
Brennan Benson
|
68ea3b92e3 |
fix(native-chat): stop a collapsed run claiming success when a tool call failed (#21151)
* fix(native-chat): stop a collapsed run claiming success when a tool call failed A settled activity group drew its completion mark whenever no call in it was `running`. That is not a success test: a tool call is `running`, `completed` or `failed`, so a run whose call failed had nothing running, took the mark, and asserted success over a failure the reader could only find by expanding the run. Success is now stated rather than inferred. `nativeChatToolRunSucceeded` grants the mark only to a run that is settled, has nothing still running, and has no failed call — a call's own `failed` verdict or an error result, the same composite test the task-list, edit-card and ask-row readers already use. A call with no lifecycle state is neither, so legacy transcripts still settle. A collapsed run that did contain failures now says so in the header, as a quiet `N failed` in the header's own mono type with a spoken `Failed tool calls: N`. Text only: a tool error is routine work, so no destructive tint and no swapped glyph. The count is taken over every call in the run, not the latest. * fix(native-chat): count failed tool calls without result mispairing |
||
 Brennan BensonandMerge Sim
|
5287c5cdbc |
fix(mobile): stop a created tab from jumping when the host snapshot lands (#20069)
* fix(mobile): stop a created tab from jumping when the host snapshot lands
Creating a tab from the mobile session strip painted the new tab at the end
of the strip and then visibly jumped it to a different slot a beat later.
The client asked the host to insert the tab after the active tab, but then
predicted a different placement for its own optimistic paint:
afterTabId: activeSessionTabId ?? undefined // host: splice(insertAfter + 1)
...
return [...prev, { ...created, isActive: true }] // client: append
Two independent placements that disagree, so the optimistic frame is wrong by
construction and the tab snaps to its real slot on the next published snapshot.
The disagreement dates to
|
||
 
|
0e3b71f605 |
fix(session): give an SSH workspace one owning partition so its tabs stop round-tripping as deletions (#19572)
* fix(session): give an SSH workspace one owning partition so its tabs stop round-tripping as deletions `workspaceSessionPartitionHostId` answered differently depending on who asked: the renderer mapped an SSH worktree's session to the `local` blob, the main-process runtime read-modify-wrote `ssh:<targetId>`. One workspace's session lived in two stores and no reader reunited them, so whatever landed on the unread side did not read as unknown — it round-tripped as absence. The remote-workspace upload is a `replace-session` patch, which turned that absence into deletion on the host, and the next pull applied the deletion locally and re-poisoned the snapshot. Collapse the two answers into one: every non-'local' host owns its partition. Boot hydration and the export fallback now read the SSH partition, and rows a shipping build left in `local` are folded back in once, gap-filling only — an empty tab row is a gap, never proof that anything was closed. Folder workspaces deliberately keep their existing 'local' routing: boot discovers SSH partitions from the repo catalog, so an SSH target that owns only a folder workspace has no partition any reader enumerates. They are still adopted back out of an SSH partition when a repo does name the host. Fixes #12721 Supersedes #12722 Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> * test(session): pin the old-client empty-publish skew direction * fix(session): adopt every workspace the host partition names, not only tabbed ones Review caught that gating adoption on `host.tabsByWorktree[key].length > 0` traded the #12721 deletion for a narrower one. The write path routes EVERY worktree-scoped field to the owning partition, so an SSH workspace with open editor files or browser tabs and no terminals had all of it dropped on every restart — and unlike terminal state it cannot be recovered from the host snapshot, which carries terminal fields only, so an unsaved `dirtyDraftContent` was destroyed outright. The defect was not a missing field. It was a hand-maintained field list deciding what the read recovers while the write used the ownership table, so the two could disagree. Adoption now walks `WORKSPACE_SESSION_FIELD_OWNERSHIP` with an exhaustive switch, and a new ownership kind is a compile-time decision rather than a silent omission. Session keys are normalized through the shared `normalizeWorkspaceSessionKeyToWorkspaceId` so host-qualified visit recency (`ssh:target|worktreeId`) reaches its workspace, and the regression is pinned by feeding the shipping split's own output back through the real boot read rather than a hand-built fixture. Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> * fix(session): stop adoption overwriting rows it was never told about Three losses, one cause: the reader walks its own description of the partition layout while the writer walks another, so the two agree on which ownership kinds exist and not on what a kind means. - An empty host row replaced a populated base row, destroying an unsaved dirtyDraftContent the header comment says must never be destroyed. The host holding nothing is not evidence the base is wrong. - A contested bare id was adopted as if local and ssh:<target> were one workspace written twice, which is exactly the id where that premise is false. The read already reached that verdict and adoption could not ask for it, so it is passed in; contested keys are gap-filled, never replaced. mergeWorkspaceSessionsWithHostShadow now reports the real contested set, which primaryHostBySessionKey never was. - Tab-, pane- and file-keyed rows are adopted through the split's own indexes, so unified-only tabs come back and the pane key is parsed once. - A bare lastVisitedAtByWorktreeId key only fills a gap; the split has a dedicated branch for that field and the reader had none. * test(session): pin the tombstone/gap boundary the two readings meet at An explicit empty tabsByWorktree row means the user closed the last terminal; adoption reads an empty base row as a gap to fill. Same value, opposite readings, so the boundary is asserted rather than argued: the tombstone lands in the owning partition, restores as a present empty row rather than a deleted key, is declined by the real seeding predicate, is published as an empty list, and the legacy-transition resurrection happens once and cannot recur. * docs(reliability): record the adoption guards and the tombstone boundary in the gate * test(e2e): read the SSH restart assertions from the partition that owns them ssh-cold-activation-restore asserted persistence through session.get() with no host, which is the local partition an SSH worktree's rows no longer live in. The invariant it means to check is that the state is persisted where the boot read will find it, so it now unions local and ssh:<targetId> and stays correct on both layouts. Confirmed the product invariant separately rather than by the edit: the behavioural half of both tests - the full app restart, the active worktree, the eager terminal remount and the PTY-owner reclaim against a real Docker OpenSSH host - runs after this check and passes. 2 passed in 48.9s. * test(e2e): read ssh-restart-tab-accumulation from the owning partition too Same layout-coupled read as ssh-cold-activation-restore: the pre-quit flush asserted through session.get() with no host. Verified against a real Docker OpenSSH target - both repeated quit/relaunch cycles keep exactly the restored SSH tabs, no accumulation and no loss. 2 passed in 52.9s. * fix(lint): clear the casting gate on the partition adoption main tightened typescript/consistent-type-assertions to assertionStyle: never, which the rebase brings onto these added lines. Most of the round-trip fixtures did not need a cast at all -- three were hiding wrong-shaped literals (a browser workspace keyed 'name', a unified tab keyed 'type', a layout keyed 'direction'), now written as the types they stand for. The adoption reads narrow through an isRecord predicate instead of casting, which also stops a null entry throwing out of Object.keys. What is left is dynamic-field writes and unknown-typed IPC returns, each with its own SAFETY rationale. * fix(session): give an SSH folder workspace one owning partition boot can find The partition owner rule already names `ssh:<targetId>` for a repo-backed worktree, but `getFolderWorkspacePartitionHostId` still answered 'local' for a folder workspace while main's `RuntimeWorkspaceSessionController.getPreferredHostId` answered `ssh:<targetId>` for the same key. That is #12723 unfixed for folder workspaces, and once the renderer started writing `ssh:*` at all it got worse: a save's field-level patch carries only the rows routed to that partition, so a `tabsByWorktree` write without the folder row erased the row main had put there. The reason the renderer could not route there was real - boot discovered SSH partitions from the repo catalog, which cannot name a target whose only workspace is a folder. So persistence now answers that directly over `session:list-host-ids`, and boot reads the partitions that exist rather than the ones a catalog implies. Removing a folder workspace prunes its rows from the owning partition too, or the census would adopt them back on the next launch as a workspace the user already deleted. Adoption now decides from the repo catalog instead of from co-presence. Two partitions holding one bare `repoId::path` is not evidence of a collision - that is the exact shape the repair exists for - so the verdict comes from `resolveWorktreeExecutionHost`: a repo id registered on more than one host is contested and may only be gap-filled, and one the catalog positively resolves to a different host is residue this partition does not own and is not adopted at all. Without the second rule a stale partition sorting first won the read and was then written into the live one. Nothing is deleted either way; the rows stay where they are. Finally, a workspace adopted out of a partition now routes back to that partition. Routing used to re-derive an owner from the catalog, so a boot whose repos had not hydrated moved the rows it had just reunited back into 'local' and re-stranded them. Contested ids are withheld from that override, because routing the whole bare id to one host is the loss the gap-fill prevents. The publish path resolves each workspace's owner once for the whole publish, shared with the projection, so the per-target catalog attribution does not repeat it per connected host. * fix(session): drop a deleted workspace from every partition, not just the local blob Adversarial review of the previous commit found three ways the partition census - which now reads whatever persistence holds rather than what the repo catalog implies - keeps rows alive that nothing should keep alive. `deleteProjectGroup` pruned only the local blob, so every folder workspace under a deleted group left its rows in `ssh:<targetId>`; the next boot adopted them back, named that partition their owner and wrote them there again, forever. `removeFolderWorkspace` had the same hole for a workspace whose partition its host expression could not name: main never persists a folder workspace's `executionHostId`, and `RuntimeWorkspaceSessionController` can infer a connection from the group's repos that the workspace row itself does not carry. Deriving the partition at delete time is the wrong question - a deleted workspace owns nothing anywhere - so both paths now remove it from every partition. The third is on the read side. A contested id is deliberately withheld from the read-source override so the write cannot carry one host's rows into another's partition, but the routing that then re-derives an owner answers 'local' for an id the catalog cannot name. Adopting such a row moved it out of the partition that owns it and into the blob: the two-store split this change exists to remove. A contested id the assembled session holds no row for is therefore not adopted at all. Gap-filling stays available for a contested id the session already names, since that row's own partition is what the write follows. Declining to adopt leaves a row invisible for one boot; it never deletes one. Also: the folder-key guard in both catalog attributions was dead, because `getRepoIdFromWorktreeId` hands back the whole key rather than nothing when there is no `::`. The verdict was right and the resolution wasted; it now skips by shape. And the two type assertions the changed-code casting gate rejected are gone rather than suppressed. * fix(session): park the rows a partition read declines instead of letting the next write erase them A partition write replaces each field with exactly what the unified session routed there. So a row the read left out of that session is erased from its own partition the moment any sibling workspace writes the same one - and with SSH partitions now the owning store, that row is then in no partition at all. Three separate decisions produce such rows: residue the catalog attributes to another host, a contested id withheld so the write cannot carry one host's rows into another's partition, and a workspace the base already holds the live copy of. Declining to show a row was quietly deleting it. The machinery for this already exists. `attachHostSessionShadow` writes a contested runtime co-claimant's parked rows straight back into its own slice before the write, so the primary's write cannot erase them; the ssh partitions simply were not among the slices the contention split arbitrates. The read now parks everything it is not returning to an ssh partition into that same shadow, and the existing re-attach puts it back. Leak, never kill - docs/reference/ssh-execution- boundary.md - and a row no partition holds is unrecoverable. Second, the contested branch of the tab adoption read `Object.hasOwn` as "the base has tabs here". An empty list satisfies it, so whenever a legacy id happened to be contested, #12721's empty local row won over the host's real one - the exact reading the module's own header, and the gate invariant it is pinned by, say is wrong. An empty row is the gap this repair fills, so it is now treated as one. * test(session): pin the empty-base-row gap for a contested id Mutation testing found the assertion missing: reverting the gate to `Object.hasOwn` left all 39 assertions passing, which makes the fix that reads an empty base tab row as a gap unguarded. The #12721 shape does not stop being a gap because the id happens to be contested. --------- Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> |
||
|
Neil
|
a46b5b15ec |
fix(worktree): ask the execution host whose home a remote delete would take (#19865)
* fix(worktree): ask the execution host whose home a remote delete would take
`isDangerousWorktreeRemovalPath` read `os.homedir()` — the machine running
Orca — and then applied POSIX-only shape rules. SSH orphan cleanup feeds it
remote paths, so a Windows host profile (`C:\Users\bob`) was unrecognised from
a macOS/Linux desktop and the recursive delete lost its last guard, while a
coincidental client-home prefix could refuse a legitimate remote delete.
The removal route already resolves one execution host for the whole removal;
it now resolves one home authority the same way. `WorktreeRemovalHomeAuthority`
is `{ kind: 'client' }` or `{ kind: 'executionHost'; homePath }`, required at
every guard entry point, so the ambient read is unreachable from a remote
removal. The host's answer is the `$HOME` the SSH session already read on the
host during relay deploy — no new probe. Unresolved stays `null`, meaning
unknown, never "same as this client's".
Path-shape rules now cover Windows profiles (`C:\Users`, `C:\Users\<name>`,
any drive or UNC root, case-insensitively) and WSL UNC aliases, which front a
Linux filesystem and so take the POSIX shapes.
Fixes #18275
* fix(worktree): merge the duplicated removal-route import
The focused code-quality plugins deny `import/no-duplicates`.
* test(worktree): pin the IPC removal call site and the unknown-host-home refusal
Mutation testing found four survivors in the home guard:
- Swapping the IPC unregistered-removal call site to the client's home passed
every suite while the remote delete could reach the host's own home. Only
the runtime call site was pinned. Add the mirror test for the IPC path.
- Falling back to os.homedir() when the execution host reported nothing was
indistinguishable from refusing; the client home never coincided with the
probed path. Assert the fallback stays off with homedir pinned to the path.
- Comparing an execution-host home across path syntaxes survived because no
row exercised the win32 home under POSIX ops: path.resolve manufactures
<cwd>/C:/Users/bob, which every ancestor of the cwd contains.
- Dropping the bare /Users rule survived; add the row.
Also cover the forward-slash C:/Users/bob form normalizeRemoteHome reports
for a Windows host, which no existing row used.
* ci: re-run after an unrelated Electron probe startup timeout
* fix(lint): clear the casting and max-lines gates on the home guard
Rebasing onto main brings two gates this branch predates:
typescript/consistent-type-assertions at assertionStyle: never, and the
300-line ceiling that the added home lookup pushed
orca-runtime-remove-managed-worktree.ts past. The fixture casts carry
per-site SAFETY rationales; the route's git-options-and-listing step
moves into its own module, which also stops the local/SSH branch being
spelled twice in one expression.
* fix(lint): name the home predicate for what it matches
main enabled anti-slop/no-shape-in-symbol-names (#20785) after this
branch was written. The predicate answers whether a path IS a home root,
not whether it resembles one.
* fix(worktree): refuse a removal the execution host cannot vouch for
Review of the home guard found three ways it still let a delete proceed on
evidence about the wrong machine, or on no evidence at all.
`getPathOps` switches to win32 as soon as EITHER the worktree path or the repo
path looks Windows-absolute, and `//nas/share/repo` does. A POSIX worktree path
was then judged by Windows-only shape rules, which recognise `<root>\Users\<name>`
and nothing else, so `/home/alice` — and any client home outside `\Users` —
stopped matching and the last guard in front of a recursive delete went quiet.
The home question involves the worktree path and a home, never the repo path, so
the predicate now reads the path in its own syntax as well and refuses if either
reading names a home. A union of refusals can only ever refuse more.
An execution host that never reported its `$HOME` is `unverifiable`, and
`unverifiable` does not authorise a delete. `isRemovalHomeAuthorityResolved`
gates the two paths that recursively delete a directory —
`canSafelyRemoveOrphanedWorktreeDirectory` and
`canCleanupUnregisteredOrcaLeftoverDirectory` — because the orphan proof they
accept, a `.git` file at the top of a directory, is also what a bare-repo
dotfiles `$HOME` looks like, and there the guard is the only evidence there is.
`git worktree remove` is deliberately not gated: the host's own Git registry
already established that the path is a linked worktree of that repo, and a
missing second opinion does not retract a first one. An empty `$HOME` is
normalised to unanswered rather than read as a resolved home.
The IPC entry point spelled its host two ways. The metadata prune, the
archive-hook route and now the home authority came from
`getRepoExecutionHostId(repo)`, while the `git worktree list` and every delete
came from raw `repo.connectionId`. A row carrying only
`executionHostId: 'ssh:<target>'` therefore listed a remote checkout on this
client and deleted a same-named local path while the guards vouched for the
remote one; the mirror row did the reverse (#11163, previously fixed on the
runtime path only). Neither spelling is evidence about the other, so a row that
carries two host names is refused before anything is listed or deleted. Both
sides are spelled by `getRepoExecutionHostId`, so they can differ on content but
never on normalisation.
A `runtime:<env>` row refuses here for the same reason. It is not reachable
through this handler today — the renderer sends environment targets to
`worktree.rm`, and the host-qualified catalog refuses to list a runtime host —
so that arm closes a door rather than changing a flow.
Fixtures that register an SSH provider now report a host home, because a
connected relay session always has one: `remoteCliBridgeEnv` is assigned before
`registerSshGitProvider`, is never cleared, and providers are unregistered
before the session leaves `activeSessions`. The wiring lives in its own module
called from the harness rather than in `worktrees-test-module-mocks`, which
`vi.mock` factories import: reaching the production route module from there
pulls in `providers/ssh-git-dispatch` while it is being mocked, and the module
runner deadlocks.
* fix(worktree): compare removal host names after decoding, not as stored text
`getRepoExecutionHostId` returns a row's `executionHostId` as stored, while the
same row's `connectionId` is re-spelled through `toSshExecutionHostId`, which
percent-encodes. A byte compare of the two would refuse a perfectly consistent
row over a `%20`, so the two host ids are now compared after `parseExecutionHostId`
has decoded the target id out of each.
`runtime:<env>` and an unparseable id decode to no machine at all and match
nothing, including each other — a runtime-owned row has a null `connectionId`
and would otherwise read as local, which is a delete on this client.
* fix(lint): clear the static-analysis gates on the removal home authority
The type-aware audit rejects a `default` arm on a discriminated switch, so the
host-kind switch names `runtime` and `undefined` outright — which also makes a
host kind added later a compile error here rather than a silent fallthrough.
The two test casts the changed-code gate flagged are gone: the leftover-cleanup
meta is typed instead of asserted, and the unparseable-host-id case narrows to
`ExecutionHostId` with the SAFETY rationale the gate asks for.
* docs(worktree): say why an unroutable removal host is refused by a plain compare
The comparison refuses `runtime:<env>` because only the left operand can name
no machine — `repoRowHostId` comes from `connectionId` and is always `local` or
an `ssh:` id. That invariant was doing the work silently; an explicit null test
in its place was a branch no input can reach, so the reason is written down
instead.
* fix(worktree): gate the registered removal on the host home answer too
I argued `git worktree remove --force` did not need the host's home answer,
because the host's own Git registry had already established that the path is a
linked worktree of that repo. That is true and it is not enough: `git worktree
add` accepts a pre-existing empty directory, and that directory can afterwards
be somebody's `$HOME` — a build account's home, a container's `HOME=/workspace`.
Being a linked worktree proves provenance, not that the path is not a home, and
the remove deletes the checkout either way.
With the host's answer that case is already caught by containment. Without it
only the path shapes remain, and a home at a non-standard location
(`/var/home/<u>`, `/export/home/<u>`, `D:\\Profiles\\<u>`) has no shape to match.
So `findRegisteredDeletableWorktree` now requires the answer as well, and every
gate that authorises a delete is on the same rule.
The fixture that models a connected relay session moves out of `ipc/` and is
shared: four runtime specs register an SSH provider without one, and a live
provider implies a reported home in production.
|
||
|
Neil
|
e45cf438bc |
fix(runtime): park a mirrored pane's resume until its PTY handle lands (#19882)
* test(repro): #19735 resumes a published mirrored pane before its handle lands * fix(runtime): park a mirrored pane's resume until its PTY handle lands Mirror hydration means the host's tab rows arrived, not that a given pane's liveness is decidable: the PTY handle lands one relay round trip later. On that frame the pane read as not-live and the sweep resumed a session the host was still running, producing a duplicate resume tab. An empty handle map for a published row is unverifiable, never exited. Park the pane on a per-pane wait with three bounded exits, each replaying the sweep: its own handle lands, the row is retracted, or a deadline expires. The deadline decides resume rather than an indefinite hold, and is scoped to the connection generation so a reconnect re-arms it. Closes #19735 * fix(runtime): bound the handle-gap expiry map to the current connection * fix(runtime): void a handle-gap verdict the reconnect made stale The per-pane park bounds itself with one deadline per connection, but the waiter never recorded WHICH connection it was armed on. A wait armed on generation 0 that fires after a reconnect stamps its expiry against the current generation, so hasHostMirrorHandleWaitExpired agrees, the mirror lookup returns null, and the pane is resumed after 1ms on a connection that has had no chance to publish the handle. That is #19735's fork with an extra step, reached through the guard that exists to prevent it. The module's own doc comment claims the opposite -- "a reconnect bumps the connection generation and arms a fresh wait" -- and that is true only for a wait which had ALREADY expired, which is precisely the case the existing test covered. The test and the comment agreed with each other and both were wrong about the live case. The waiter now carries the generation it was armed on and records no verdict when the generation has moved; the replay re-parks through the existing machinery and the new connection gets its own full budget. Still bounded per connection generation, which is what was documented all along. Also pins the three sibling attacks on the same window: two panes in one environment where only one handle lands, a handle published by a foreign environment, and an environment tearing its rows down mid-park (which leaves no waiter and no scheduled timer). The test file now leads with how to assert on this module at all, because the obvious shape cannot fail. "Did the waiter release" is not an observable here -- a waiter released for the wrong reason is re-parked by the replayed sweep, so the store reads identically one tick later, and a mutation releasing every waiter on any tab's handle survived twelve assertions written that way. What a spurious release costs is the deadline, so the assertions advance the clock and require the pane to decide on the ORIGINAL schedule. * fix(terminal): a live pane owns its transcript in any workspace The resume dedup was scoped to the record's own workspace on both terms -- the entry's tab had to be in worktreeTabIds AND entry.worktreeId had to match -- and additionally required entry.state !== 'done'. A record whose peer pane has finished a turn and still holds a live PTY therefore matched nothing, and the sweep launched a second agent onto a transcript the peer is still writing. Cross-workspace, it matched nothing even while the peer was mid-turn. The two ids really do drift. canonicalizeTerminalSessionWorktreeId re-keys tabsByWorktree, tabGroups, tabGroupLayouts, activeTabIdByWorktree and activeGroupIdByWorktree onto the canonical worktree id, and does NOT re-key sleepingAgentSessionsByPaneKey, whose records carry worktreeId inside them. So adopting an orphaned terminal is a direct producer of a record naming one workspace while its pane and status row name another. Split into two arms rather than widening the existing condition. The new arm carries no workspace scope but demands hard evidence: a provider session id names one transcript, so a pane whose exact PTY is live right now already owns it wherever that pane sits, and no workspace boundary makes a live PTY less live. The scoped arm keeps its scope and its state !== 'done' term, because a status row with no live PTY is a claim about the past and must not reach across workspaces. Relationship to #19736: that PR fixes the SAME-workspace half of this in the same function, by relaxing only the status term. This arm covers that cell too -- measured both ways on this branch, which does not carry #19736: its thirty `checks exact live ownership before resuming` cases all pass with this change alone, and ten of them fail without it. So this supersedes #19736 rather than sitting beside it, and #19736's one-line `export` of stablePaneHasLivePty is carried here because this arm needs it. If #19736 lands first this becomes a pure widening and its tests should be kept. Both cells are pinned here either way. * fix(runtime): isolate one pane's replay from the handle-gap drain One store write releases every due pane, and the drain runs synchronously inside a zustand subscriber. `waiter.run()` was unguarded, so a single pane's replay reached two things it has no business touching: - the throw escapes out of `useAppStore.setState`, meaning the mirror apply that published the PTY handle throws at its own call site; - every pane queued behind the thrower is stranded — waiter still parked, deadline still armed — and then decides on a connection whose evidence landed long ago. The deadline path fans out the same way, so a throwing replay also escaped the timer callback. Reachable: `resumeSleepingAgentSessionsForWorktree` reaches `state.createTab` with no guard of its own. The panes in a drain are strangers to each other and to the frame that released them; none of them should be able to see another's failure. The new tests live in their own file because host-mirror-handle-gap-resume.test.ts drives the waiter through the real resume sweep and so cannot choose what a replay DOES. Note for anyone extending that file: per its header, "did the waiter release" is not an observable here — a spurious release is re-parked immediately and reads identically one tick later. These tests assert on timer count and on the deadline instead. Also records two findings next to the code, so they are not rediscovered: `expiredGenerationByPane` is never pruned for a removed environment (bounded and inert, since removal advances the generation, but it does not drain — and a DIFFERENT leak in that same map is being fixed concurrently, so reconcile rather than patch around it); and sustained reconnect churn holding a pane parked indefinitely is CORRECT, not the latch-that-never-releases defect, because under churn liveness genuinely is unverifiable and ssh-execution-boundary.md forbids resolving that to `exited`. It has the shape of the defect and will eventually be "fixed" by someone who does not know that. Mutation: dropping the guard kills exactly the three new assertions and leaves all twelve existing waiter tests passing. * fix(runtime): drain a removed environment's handle-gap verdicts on teardown `expiredGenerationByPane` is pruned only by rules that run when a verdict is RECORDED — the stale-generation sweep here, and the tab-death sweep added separately ( |
||
|
Neil
|
2531dc9d5a |
fix(runtime): bound the connect phase against an unreachable host, at the transport (#20053)
* fix(runtime): bound the remote-runtime connect against an unreachable host A host that is powered off or firewalled black-holes the TCP SYN, so the remote-runtime WebSocket neither opens nor errors. The Node-side transports set no connect bound, leaving the caller's whole-request timeout as the only one: every `orca <cmd> --environment <unreachable>` sat silent for 60s before failing with a generic `runtime_timeout`. Measured on an unreachable paired host (win-lowspec, SYNs dropped): terminal list / worktree list / repo list / status each took 60.19-60.26s; the same command against a reachable host answered in 0.24s. So this was the shared transport, not one command. Pass `handshakeTimeout` at the three shared remote-runtime WebSocket construction sites, which `ws` applies across TCP connect and the HTTP upgrade. The value matches the bound the browser transport already used. The failure keeps code `remote_runtime_unavailable` so the existing transport-loss classification in terminal-process-inspection still applies, and the message names the endpoint and stops at "unverifiable" — per docs/reference/ssh-execution-boundary.md, loss of contact is never evidence that the host's work stopped. * fix(relay): bound the control socket's connect phase at the transport The relay control socket was constructed with no `handshakeTimeout`, the same gap fixed for the remote-runtime transports. It was not a live defect: the class-level `connectDeadlineMs` (15s) also covers a stalled connect, and that deadline does fire — its `unref()` is safe because the pending TCP connect is itself a ref'd libuv handle that holds the event loop open. Measured in a bare Node process: unref'd timer with an empty loop never fires (exit at 0ms), but the same timer alongside a black-holed connect fired at 2003ms. It was a defect waiting on a refactor. The two bounds cover different phases, and the class deadline covers the connect phase only incidentally. DO NOT REMOVE EITHER BOUND AS REDUNDANT. They are not. Proven by mutation: - Remove the transport bound -> a stalled *connect* falls through to the class deadline, rejecting with `relay_control_connect_timeout` after the full deadline instead of the transport error. - Remove the class deadline -> a stall during the *proving* phase (socket open, host proof never answered) is unbounded; the incumbent test hangs 30s. `handshakeTimeout` cannot see that phase at all. Reuses `remoteRuntimeConnectOptions` rather than forking a second helper, and moves the construction into `relay-control-socket-factory.ts` so a caller that needs a relay control socket gets the bound instead of re-deriving an unbounded one. `handshakeTimeoutMs` is settable apart from `connectDeadlineMs` so a test can stall the connect alone and assert which bound produced the rejection — error identity, not elapsed time. The connect-bound ratchet now covers the relay site and asserts the site still resolves, so an allowlist that silently stopped matching cannot pass vacuously. * fix(lint): carry SAFETY rationales for the connect-bound casts main tightened typescript/consistent-type-assertions to assertionStyle: never, which the rebase brings onto these added lines. Dropping the generic default is not typeable, so each cast keeps its own rationale. * fix(runtime): keep the bounded connect failure inside both message gates The connect bound's new wording dropped out of the two gates that classify remote-transport failures by message text, and those gates are the only ones that run on the path the bound made reachable. `subscribeRemoteRuntimeTransport` reports a connect failure by *rejecting* the subscribe promise, and that rejection crosses `ipcMain.handle`, which keeps only the message. The renderer then classifies it with `RECOVERABLE_MESSAGE_FRAGMENTS`. `Could not reach the remote Orca runtime at …` matched no fragment, so it read as fatal: `recovery.cancel()` and a red banner instead of a retry. Before the bound existed this case reached the 15s subscription-start timer, whose message did match a fragment, so introducing a 12s bound turned an auto-recovering pane into a dead-ended one — the #12650 shape. The same wording also fell outside `REMOTE_RUNTIME_UNREACHABLE_RE`, so the Tailscale remedy was dropped for precisely the unreachable-host failure it exists for. Keep the canonical phrase both gates already recognise rather than teaching each gate a second synonym for one condition, and pin it: the phrase is now a named constant, the corpus in `remote-runtime-transport-error-agreement.test.ts` grows the coded, hinted and code-stripped producers derived from the real helper, and a new subscribe-path test proves the connect bound (not the start timer) is what fires and that its message still classifies as recoverable once the code is gone. Verdict wording is unchanged: `unverifiable`, never a synonym for exited. Also states the bound in seconds, corrects the module comment (`handshakeTimeout` is a socket inactivity timer, so a slow-but-answering host is not cut off), and splits the subscription contract types out to stay under `max-lines`. * fix(relay): drop the duplicate connect bound on the control socket The claim that `connectDeadlineMs` cannot see a black-holed connect is false. `RelayControlClient.connect()` constructs the socket and arms `connectTimer` in the same synchronous call — `new WebSocket()` never blocks — and `expireConnect` fires from `opening` as well as `proving`. The class deadline was already a strict superset of a transport `handshakeTimeout` on that socket. It was also inert. Production passes neither option, so the transport bound was derived from `connectDeadlineMs` and both timers were 15_000, armed in the same tick; the ws timer is an inactivity timer armed on the later `socket` event, so it could not win. Its only reachable effect was changing which string a stalled relay connect rejects with, and it narrowed an existing test's 20ms deadline into a handshake bound it could race. So this removes the factory, the test-only `handshakeTimeoutMs` option and the source-grep test whose premise was wrong, and replaces them with a test that holds the real ground: a connect whose upgrade is never answered expires on the class deadline. Moving the timer arm after `open`, or narrowing `expireConnect` to `proving`, both turn it red — which is what a future reader needs before concluding the phase is uncovered and adding a second bound again. No behaviour change for a reachable relay, and none for the verdict: a stalled connect still rejects and still reaches `unverifiable`, never `exited`. * fix(runtime): stop the endpoint in the failure message from undoing the fix Putting the endpoint into the message created three problems the message itself caused. The Tailscale hint is idempotent by testing whether "tailscale" already appears anywhere in the message. That held while the message was fixed copy. Now a host called `tailscale-box` puts the word there itself, and the hint — the only actionable remedy on an unreachable host — is suppressed for it. Key the guard on the two hints instead of the word. The endpoint comes from a pasted pairing code, which is only length-capped; `normalizePairingUrl` rejects userinfo but nothing re-validates a stored offer. Render scheme, host and port only, so a pasted `wss://user:secret@host` cannot reach a surface the user reads. And drop the elapsed time from the wording. `handshakeTimeout` is a socket inactivity timer, so a `wss://` host that completes TCP and then goes silent re-arms it once and fails at about twice the bound; measured at 2008ms against a 1000ms bound. "within 12s" would have been wrong there, and the endpoint is the actionable part regardless. Also refuse a non-positive or non-finite bound: `ws` and `net` both gate on a truthy timeout, so `0` left the connect completely unbounded while still satisfying the connect-bound ratchet. * fix(runtime): keep the endpoint from smuggling a verdict into the message `isRemoteTerminalGoneMessage` in the pty transport substring-matches `terminal_gone` / `terminal_exited` / `no_connected_pty`, and it runs before the recoverable-connection gate: a match retires the pane's terminal id and cancels recovery. WHATWG URL accepts `_` in a special-scheme host, so once the failure message carried the endpoint, `ws://terminal_gone.example:6768` turned loss of contact into a terminal-gone verdict — the one conclusion `docs/reference/ssh-execution-boundary.md` forbids. Render the host only when it matches a hostname or IP-literal grammar that cannot carry such a token, and fall back to naming no endpoint at all. A well-formed host, including a bracketed IPv6 literal, is still shown. * docs(runtime): say why this connect bound is not the relay's removed duplicate |
||
|
Neil
|
6c3b97b950 |
fix(mobile): a scope refusal is not a missing method on the Relay pairing probes (#19952)
* fix(mobile): a scope refusal is not a missing method on the Relay pairing probes
The desktop's mobile allowlist gate runs before its RPC dispatcher, so a method an
older desktop predates is absent from both and the phone is answered `forbidden`,
never `method_not_found`. Keying the "too old for Relay, stay on LAN" fallback on
`method_not_found` alone therefore never fired against the exact desktop it exists
for: first-time pairing threw instead of committing a LAN host.
`isPairingRelayRpcUnavailable` accepts both codes at the three pairing probe sites.
It is pairing-scoped on purpose - `isMethodNotFoundRefusal` has four other consumers
that must keep reading `forbidden` as a refusal, not as absence.
The main-side test pins the claim the fallback rests on: the dispatcher really does
answer `forbidden` to a mobile-scoped device and `method_not_found` to a runtime one,
and this build allowlists both probes, so `forbidden` on either can only mean an
older desktop.
* fix(mobile): leave a breadcrumb when a desktop refuses relay pairing
The LAN fallback now commits a host instead of throwing, so the refusal code
was the only record of why a phone ended up without a relay endpoint and
nothing wrote it down. Log it on the path that swallows it.
Narrow `isPairingRelayRpcUnavailable` to the two codes it matches rather than
to `RpcFailure`: a plain failure guard would collapse the *false* branch to
`RpcSuccess`, which a refusal carrying any other code still reaches.
Rename the `'method-not-found'` sentinel in the direct-upgrade reader, which
stopped describing what it covers, and correct two comments that named a
`method_not_found` mechanism the desktop cannot produce for these methods:
both probes have been allowlisted and registered by the same commit since
Relay landed, and an unwired pairing provider answers `runtime_error`.
* docs(wire): record that the mobile surface refuses by scope, not by absence
Two comments cited this page for "a scope refusal is not a missing method" and
the page did not say it — the only nearby statement says the opposite, because
it describes the runtime-scoped surface, where the dispatcher does answer
`method_not_found`. The allowlist gate makes the mobile surface the exception,
and the harness does not run that surface, so this note is the only record.
* docs(mobile): name the pairing site the scope refusal actually reached
The comments and the wire-compat note said this fixed first-time QR pairing.
It cannot: the `relay` block on the pairing offer, both RPC handlers and both
allowlist entries all landed in
|
||
|
Brennan Benson
|
28a2b628bc |
fix(native-chat): open the message rail panel on the current message (#21143)
* fix(native-chat): open the message rail panel on the current message
The rail's hover panel mounts fresh at scrollTop 0 every time it opens, so
in a long thread it showed the top of the conversation instead of where the
reader actually is. It already knew which row was current — activeId drives
the highlight — it just never scrolled to it.
Attach a ref to the current row that calls scrollIntoView({ block: 'nearest' }).
Radix unmounts popover content on close, so ref attachment is the open edge;
it also re-fires when a different row goes active under an open panel.
* fix(native-chat): keep current rail item focused
* fix(native-chat): resync rail after list changes
* fix(native-chat): own focus across retained rail opens
|
||
|
Brennan Benson
|
c2962a765a |
feat(desktop): let the renderer reach agent.launch on its own main process (#21132)
* feat(desktop): let the renderer reach agent.launch on its own main process The desktop renderer aimed at a remote host was admitted to `agent.launch`; the same renderer aimed at its own main process was refused `agent_launch_unsupported`. Main sends `ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES` on the remote path, which carries the capability, while `runtime:call` built its own hardcoded list that did not. Collapse the two hand-maintained copies in `runtime.ts` — the unary and the streaming path held separate literals — into one constant, add the capability to it, and pin its divergence from the remote Electron list so the next capability cannot drift the same way. No caller is migrated: this makes the call possible and changes no behaviour. * docs(test): mark which ledger rationales are grouped rather than audited |
||
|
Jinwoo Hong
|
631b51f508 |
perf(usage): run the Claude/Codex/OpenCode usage scans on a worker thread (#21114)
* perf(codex-usage): resume rollout scans at the last parsed byte Codex rollout files are append-only and grow all day, but any append changed both mtime and size, so `canReuse` discarded the cached entry and the scanner re-read the whole file from byte 0 on the Electron main process. On one real corpus that was 6.59 GB re-read per cycle across 26.63 GB / 21,110 files. Each parsed file now persists a resume point: the offset just past the last newline-terminated line, the parse context at that offset (session id, cwd, model, running totals), a sha256 of the 4 KiB before it, and the file's dev:ino. A grown file resumes there and merges the appended rollup into the cached one; anything unproven falls back to a full reparse — truncation, an in-place rewrite, rotation, a counted tail with no trailing newline, a legacy copied-session suffix offset, or a file that must reclaim deferred fork claims. Resume never depends on mtime equality, so a coarse-mtime filesystem cannot hide an append. Fixture: a 75,737-byte rollout with a 758-byte append re-read 76,495 bytes before and 8,950 after (the append plus two bounded 4 KiB boundary windows). Also bounds the automation-attribution force predicate for both Codex and Claude: it keyed on `lastScanError`, so a persistently failing scan forced a fresh full rescan on every single lookup. It now keys on the most recent scan attempt, which is one forced scan per run regardless of outcome. * perf(usage): run the Claude/Codex/OpenCode usage scans on a worker thread The three first-party usage scans walk whole rollout and transcript corpora and read OpenCode's SQLite synchronously, all on the Electron main process. They rarely produce a long stall — the JSONL reader streams, so it yields to the loop between chunks — but they pin the main-process event loop at ~95% utilization for the scan's whole duration, which is what every IPC message, timer and window event then queues behind. Move that work to one lazily-spawned, unref'd worker thread shared by all three providers, following the OpenCode SQLite scanner precedent (#8864). Measured on a synthetic 4,000-rollout corpus (25.8 MB cache): a cold scan drops from 2,147 ms of main-thread time to 31 ms, and a steady-state incremental scan from 165 ms to 64 ms. The worker is stateless and the cache crosses the boundary both ways. That costs ~64 ms of structured clone at this corpus size, against 2,147 ms saved on the cold path, and it keeps the persisted cache the single source of truth — a worker-owned copy would need an invalidation protocol and a second resident copy of the same multi-MB array. Failure is closed, never a silent empty result: a worker that cannot spawn, times out, or crash-loops rejects, and the store records the scan error and keeps the previous projection. Two clients already carried the same FIFO/timeout/crash-cap machinery, so extract it once as WorkerThreadRequestQueue (with the packaged entry-path resolver as worker-thread-entry-path) and move all three onto it, rather than adding a third copy. Their existing tests pass unchanged. The oracle is event-loop utilization on the calling thread, not a stopwatch: usage-scan-worker-event-loop.test.ts runs the same scan both ways and asserts the worker leg leaves the caller idle while the main-thread leg does not, so CI load moves both legs together (#18788). * test(usage): compare the two scan arms instead of two fixed thresholds The event-loop oracle claimed to be self-calibrating — its header said "the ratio is self-calibrating, so CI load moves both legs together (#18788) instead of tipping a fixed millisecond threshold." It computed no ratio. Two separate `it()` blocks each asserted an absolute threshold against its own arm, run separately, so load moved them independently. The comment described a test nobody wrote, and the flake it promised was impossible is the one that landed: `activeRatio > 0.8` on the calling-thread arm measured 0.764 on an ubuntu runner. Fixing the comment is not enough, because the fraction is the wrong quantity. CPU contention drags the calling-thread arm's active/wall fraction *down* toward the worker's, since the loop parks waiting on a contended libuv pool. A 4-vCPU Linux container measured that arm at 0.175-0.756 across twenty runs, idle and loaded — never once above 0.8. Active *milliseconds* move the other way: contention stretches the caller's JS time far more than it stretches the worker arm's fixed post-and-deserialize cost, so the gap widens under load. Merge the two arms into one case over one corpus and assert the worker arm costs the caller under a fifth of the inline arm's active milliseconds. Same twenty Linux runs: 10.9x-83.6x, passing throughout. Keep the presence preconditions on both arms — an arm that silently scanned nothing satisfies the comparison trivially — and extend them to the calling-thread arm, which previously checked only file and session counts. * fix(ports): name the dropped command when the probe queue is full The shared-queue extraction turned `Port scan command queue is full; dropped ${command}.` into a constant string, because `describeFull` was given no way to see the request. Pile-up is per-probe, so the name is the only thing in that log that identifies which of lsof/ps/netstat was shed. Pass the rejected request to `describeFull` and restore the name. The request is built before the cap check so it exists to be named; the id it burns is a correlation token, so a gap costs nothing. The existing overflow test asserted only the error class, which is why the regression escaped a 29-test suite. It now dispatches the overflow under a different command than the accepted ones and asserts the message text, so a message that names the wrong request fails too. Also add a direct WorkerThreadRequestQueue test. Three subsystems share the queue and each client test only sees the parts its own protocol exercises, with `queueCap` reachable from port-scan alone. Covers one-at-a-time FIFO dispatch, the deadline starting at dispatch rather than enqueue, the consecutive-death cap, and both points where that count clears. And record the child-process hazard at the usage worker entry. `terminate()` reaps nothing the thread spawned, and OpenCode discovery reaches a fork today: `wslGated*` forks the WSL transcript sidecar for a `\\wsl$\...` path, which a Windows `OPENCODE_DB` or `XDG_DATA_HOME` can be. One scan through that entry with a UNC `OPENCODE_DB` forked a sidecar that outlived `terminate()`. * test(ai-vault): assert the OpenCode worker messages exactly, not by fragment Checked every message string in the two clients the shared-queue extraction rewrote against origin/main. Only the port-scan queue-full one regressed (fixed in the previous commit); the OpenCode SQLite client's four messages render identically, the remaining source diffs being renames — `error.message` to `lastError`, `call.timeoutMs` and `CALL_DEADLINE_MS` to `timeoutMs`. `session-scanner-worker-client.ts` was not touched by the extraction. But its suite could not have caught it either. `/timed out/`, `/exited with code/` and a bare `rejects.toThrow()` all still match a message that has lost its interpolated value, which is the same blind spot that let the port-scan regression through. Assert the rendered text instead: the timeout names its deadline, the exit names its code, and the crash-loop drain still carries the text of the fault that killed the run. * fix(usage): correct the worker entry's child-process note The previous note said `worker.terminate()` leaves a forked sidecar orphaned. It does not, and the reproduction that appeared to show it used a stub sidecar missing the `process.on('disconnect', () => process.exit(0))` the real entry has. With a faithful one: the sidecar lives exactly as long as the thread and is gone within 2s of `terminate()`, because tearing the thread down closes the IPC channel it owned. Two worker lifecycles forked two sidecars and leaked neither, and the pre-worker main-thread path reaps its sidecar the same way, on host exit. What is true and worth recording: a fork is reachable from this bundle at all, which is easy to miss; it survives only as long as the channel does; and the sidecar is now re-forked per worker lifecycle instead of pooled for the app's life. State those, and warn that a future child which does not exit on channel close would not get the same free cleanup. * fix(usage): kill a wedged scan worker on no progress, not on wall clock `USAGE_SCAN_TIMEOUT_MS` was a 10-minute deadline on the whole scan. A cold scan of a real history is legitimately minutes — 637 s measured on a 30 GB corpus with 300 worktrees before the per-cwd memo, ~51 s after — so a larger corpus or a slower disk crosses it. Crossing it killed the worker, recorded a scan error and left the cache unadvanced, so the next refresh started cold and died at the same point, forever. The deadline is now a no-progress window. The worker posts a file counter as it walks the corpus (`UsageScanWorkerProgress`, rate-limited to one message a second), and `WorkerThreadRequestQueue` re-arms the active call's timer on each one via the new optional `isProgress`. Clients that do not pass it keep the plain wall-clock deadline. `MAX_CONSECUTIVE_DEATHS` and idle teardown are unchanged. * refactor(usage): report scan progress as a file count, not one call per file Claude's scanner walks batches, so a per-file callback made it loop just to bump a counter. |
||
|
Jinwoo Hong
|
f36a7cecf2 |
perf(codex-usage): resume rollout scans at the last parsed byte (#21102)
* perf(codex-usage): resume rollout scans at the last parsed byte
Codex rollout files are append-only and grow all day, but any append
changed both mtime and size, so `canReuse` discarded the cached entry and
the scanner re-read the whole file from byte 0 on the Electron main
process. On one real corpus that was 6.59 GB re-read per cycle across
26.63 GB / 21,110 files.
Each parsed file now persists a resume point: the offset just past the
last newline-terminated line, the parse context at that offset (session
id, cwd, model, running totals), a sha256 of the 4 KiB before it, and the
file's dev:ino. A grown file resumes there and merges the appended
rollup into the cached one; anything unproven falls back to a full
reparse — truncation, an in-place rewrite, rotation, a counted tail with
no trailing newline, a legacy copied-session suffix offset, or a file
that must reclaim deferred fork claims. Resume never depends on mtime
equality, so a coarse-mtime filesystem cannot hide an append.
Fixture: a 75,737-byte rollout with a 758-byte append re-read 76,495
bytes before and 8,950 after (the append plus two bounded 4 KiB boundary
windows).
Also bounds the automation-attribution force predicate for both Codex and
Claude: it keyed on `lastScanError`, so a persistently failing scan forced
a fresh full rescan on every single lookup. It now keys on the most recent
scan attempt, which is one forced scan per run regardless of outcome.
* fix(codex-usage): verify the head of a resumed rollout prefix
The resume guard proved only the 4 KiB before the resume offset, and leaned
on dev:ino to catch a rollout that was replaced at the same path. ext4 and
overlayfs hand a recreated file the inode the old one freed, so on Linux that
check passes and a same-length prefix swap resumes over changed history.
Measured 20/20 inode reuse on ext4 and overlayfs, 0/20 on APFS and tmpfs --
which is why the case only failed in CI.
An in-place prefix rewrite kept no inode change on any platform, so that
variant was missed on macOS too.
Digest a bounded window at the start of the parsed prefix as well. When the
two windows meet, one read covers the whole prefix and leaves no gap. The
head window is carried across a resume rather than re-read, so a resumed scan
reads the appended bytes plus three 4 KiB windows.
* test(codex-usage): cover the resume window layout switch
* test(codex-usage): cover the boundary window in isolation
* test(codex-usage): isolate the boundary window with disjoint windows
* fix(codex-usage): restart a rollout parse when its verified prefix is gone
The scanner verifies a rollout's prefix in its first pass and reads it in
the second, so a truncation in between left the merged projection holding
the whole pre-truncation history while `processedFile` was re-stat'd to the
new, smaller size. Size and mtime then matched disk with no resume state
left to reject, so the reuse path served the stale total on every later
scan. The resume-state builder returns null only on a short read, which is
exactly that signal; on it, drop the merge and reparse the file from zero.
Also covers three guards that no test was holding: the unterminated-tail
resume suppression (a tail that is valid JSON minus its newline is counted,
so resuming over it double-counts), the short-read check in
`readWindowDigest` (without it a resume point past EOF verifies against
itself), and the legacy-suffix exclusion in the scanner's resume guard
(bridge markers can appear on a file that already has a resume state).
* fix(codex-usage): re-verify a rollout resume point at the point of use
The scanner verified each resume point while walking the sessions
directory, then parsed the files afterwards, so every file discovered or
parsed in between widened the gap between the check and the read. A
rollout replaced in that gap resumed at the old offset into unrelated
bytes: the cached session id, cwd, model and running totals were stitched
onto another file's records, and because the projection was then re-stat'd
to the new size, the reuse path froze the corrupted numbers. A shrink was
the visible half of this; a replacement larger than the recorded offset
never short-reads and corrupts instead of going stale.
Re-run the full check — inode, head window and boundary window — inside
the parse, against the file about to be read. The short-read fallback
added alongside it still covers the narrower case of a truncation landing
after that check, during the read itself.
Cost, measured on the existing byte oracle: a resumed file now reads
`appended + 5 * 4096` rather than `appended + 3 * 4096`, paid only by
files that changed since the last scan; untouched rollouts still read
nothing. Two byte-total assertions that a 15 KB rollout can no longer
satisfy now assert their intent directly — that the parse read did not
reopen at byte 0 — via a stream oracle that records each read's offset.
* test(codex-usage): pin mid-scan replacement on attribution, not totals
The mid-scan replacement case was written with a heavier replacement so
the token totals diverged, which overstated how visible the defect is.
Rebuilt on the variant where the stale prefix contributes exactly as many
events as the resumed read skips: daily aggregates and token totals then
match a cold scan byte for byte, and the misattribution — 60 records of
one session recorded against another — is the only remaining signal.
Oracle is now the session shape. Removing the point-of-use re-verification
fails it with `session-grower` in place of `session-other`; every
totals-based assertion still passes under that mutation.
* perf(codex-usage): stop resuming a rollout prefix too short to pay for it
Point-of-use re-verification made a resumed scan cost five bounded windows,
which is more than re-reading a small rollout outright. Measured against a
cold reparse of the same file, resuming lost below a 12,288 B prefix and
lost badly under 8 KiB, where the coalesced-window layout rehashed the
whole prefix on each of the three verification passes.
Set the floor at that break-even — 3 * 4096, the point where two
verification passes plus the recorded boundary stop being cheaper than
reading the prefix once — and refuse to record or accept a resume point
below it. Measured: a 12,568 B prefix now reads 21,234 B resumed against
21,514 B cold, and a 76,484 B rollout reads 21,238 B against 84,676 B. No
size band reads more than a cold scan any more; under the floor the
windows are skipped entirely and a scan reads exactly the file.
With every offset past the floor the two windows can no longer overlap, so
the coalesced-layout branch and the empty-window branch are gone. The
floor is also input validation: a persisted offset below it would put the
boundary window at a negative start and throw ERR_OUT_OF_RANGE.
Tests that meant to exercise the resume path were silently reparsing whole
once the floor landed — the suite stayed green while three guards lost
their only coverage. They now size their rollouts off RESUMABLE_RECORDS
and assert the offsets their parse reads actually opened at, so a test
that stops resuming fails instead of passing quietly.
* test(codex-usage): cover the reuse gate's own legacy-bridge check
`scanner.ts` carries the same `legacySourceSkipBytes === 0` term twice and
they are different guards: line 83 gates resuming, line 71 gates reuse.
Only the first had a test, so dropping the second left the suite green.
It is load-bearing. A cached entry can predate the bridge marker while the
source file is untouched, so size and mtime still match and nothing else
stops the scan serving a full-history projection for a file that is now
parsed suffix-only. With a total-only record after the copy point the two
readings diverge — baseline worth nothing against a delta worth three —
and the reused entry reports 18 tokens where a cold scan reports 15.
* fix(codex-usage): annotate the mid-scan seam instead of asserting it
The changed-code quality gate rejects any non-const type assertion, and
`onStreamOpen: { current: null as (...) | null }` is one, so `static
analysis` failed on this PR. A typed local carries the same intent.
* fix(usage): force an automation lookup onto a scan already in flight
`shouldForceAutomationUsageScan` keyed on `max(lastScanStartedAt,
lastScanCompletedAt)`, so a scan that started after the run completed but
is still running counted as a finished attempt. The lookup then called
`refresh(false)`, which returns early inside the 5-minute staleness
window instead of joining the scan, and the run's usage read
`unavailable`. Forcing instead just awaits the shared `scanPromise`.
While a scan is in flight its start time is no longer treated as an
attempt, so the once-per-run bound still holds: a failed scan leaves
`lastScanStartedAt` past the run and stops re-forcing.
The two providers' copies of the predicate were byte-identical, so it now
lives in `src/main/usage/automation-usage-scan-forcing.ts`.
|
||
|
Jinwoo Hong
|
2c2d068b26 |
perf(usage): resolve each cwd's worktree once per scan (#21130)
* perf(usage): resolve each cwd's worktree once per scan Codex and OpenCode attribution ran the worktree containment search for every parsed event, so a cold scan cost events x worktrees. On 745 MB of real rollouts (~20k events) that is 1.2s with 0 worktrees, 5.0s with 100, 12.8s with 300 and 39.8s with 1000; a full corpus with hundreds of remembered worktrees is where the STA-7724 reparse burned minutes of main-thread CPU. A scan holds only a few hundred distinct cwds, so both scanners now build one memoized resolver per scan and thread it through parsing instead of passing the worktree list to every event. * refactor(usage): make the worktree resolver own canonicalization `createUsageWorktreeResolver` now takes raw worktree refs and canonicalizes them itself, so each scanner has one entry point and neither keeps a private `buildWorktreesWithCanonicalPaths` or `canonicalizePath`. The resolver unit test counts comparisons through the same `areWorktreePathsEqual` mock the scanner-level test uses instead of a property getter. |
||
|
Brennan Benson
|
7f5141ae2d |
Make the Agent Permissions toggle apply to Codex chat (#20977)
* fix(structured-chat): deliver the permission posture through each transport's own contract Codex posture moves off app-server argv onto typed `thread/start` and `thread/resume` params. Manual states `on-request` / `workspace-write` explicitly instead of omitting the fields, which app-server resolved through the mirrored config.toml — a Manual thread on a home carrying `approval_policy = "never"` never prompted. Claude keeps its owned `--dangerously-skip-permissions` flag through SDK `extraArgs`; the SDK's typed bypass option emits a newer allow flag that older user-installed binaries reject. Posture is re-derived from current settings on every session acquisition. * fix(structured-chat): parse permission arguments as argv * fix(structured-chat): keep permission policy authoritative |
||
|
Brennan Benson
|
0bf815a480 |
fix(agent-launch): make a lost launch safe to retry (#21106)
* feat(agent-launch): make a lost launch safe to retry `agent.launch` could not be retried safely. Only a create-worktree target carrying a clientMutationId got any idempotency at all, and that was a 60s in-memory cache with no caller partition that dies with the process; an existing-workspace launch got none. Mobile retries a lost create by design, so the retry is the ordinary case — and a retry past that cache meant a second worktree and a second agent. A caller may now name its launch with an optional `operationId` and get one execution, the recorded answer on every replay, and a truthful refusal when the outcome is unknown. Admission runs before the worktree selector is resolved, so a replay answers from the record rather than re-deciding against today's world. The core is an atomic claim. Admission alone cannot decide who runs: two replays both read `pending`, and settling `unknown` replaces the outcome blind, so two serialized writes are not a compare-and-swap and both callers execute. A conditional current-state swap now reports which caller won, and settlement is monotone so a late `unknown` cannot erase a recorded success. Also here: a host-computed fingerprint over the launch intent that excludes mutable settings, the full launch result persisted so a replay returns the receipt and warning that cannot be recomputed once settings move, and a derived child operation id for the inner attach — the ledger key carries no method, so forwarding the launch id would make the attach conflict with its own launch. Safety, not recovery. Nothing here probes for a surface a dead attempt left behind, adopts one, or finishes an interrupted publication. Callers that send no `operationId` keep today's behaviour exactly, which is why the field is optional and the host advertises `agent.launch.replay.v1`: an older host strips an unknown param and launches anyway, so a client may only treat a retry as safe once the host has said it enforces the ledger. * fix(agent-launch): keep an unreadable launch payload from costing the store Review follow-ups on the replay-safety ledger. A recorded `launch` payload must not gate row validity. `isAgentLaunchResult` is a hand-maintained mirror of a result type later work will edit, and `isAgentSessionOperationRow` is consulted by the store loader, where one rejected row makes the whole file unparseable — a primary and backup that both fail to parse raise `agent_session_store_corrupt` and the profile loses every lease. That is the same argument the row already makes for keeping `sessionId` required, applied to the field this PR added. The payload is now typed `unknown`, left out of the row guard, and narrowed where it is read, so a payload this build cannot read refuses exactly one replay. A recorded failure now replays as the code the launch raised. Narrowing it through the closed `agentSession.*` refusal list answered `worktree_not_found` with `agent_session_operation_invalid` — the ledger's "your id is malformed" signal, which invites a client to mint a fresh id when the truthful answer is that this launch definitively did not run and the same id is safe to retry. The persisted failure code is bounded on the way in. A code is an identifier, but `error.message` is free text: an errno sentence carrying an absolute path arrived here as one and was written into a file re-serialized whole on every later operation. Bounded on write only — a length check in the row validator would reject rows this same build wrote, which is the hazard above. Comments: the caller key does not give one client a single namespace across surfaces, because the structured attach this launch performs partitions under `structuredCallerFor`; the two coincide only for a bearer-identity caller with no paired device, which is exactly when the derived child id is load-bearing. Recorded as a known limit that a `lost` claim cannot tell a sibling executing now from one a restart abandoned; telling them apart needs execution-generation tagging, which is recovery. Tests: the store-level ablation was inert — it defined a local stand-in and passed identically with and without the guard. It now substitutes the non-atomic composition into the handler's own store and watches one tap create two workspaces. Each of the four new guards was watched failing against the unfixed code: `agent_session_store_corrupt` on reopen, `expected false to be true` on the row guard, `agent_session_operation_invalid` in place of `worktree_not_found`, and a 6042-character code where 128 is the bound. * fix(agent-launch): keep live retries in one execution * docs(agent-launch): clarify failed replay guidance |
||
|
Jinwoo Hong
|
de4dab93cb |
test(shared): drop the duplicated separator-only git grep test (#21116)
|
||
|
Brennan Benson
|
aad41b1a40 |
fix(native-chat): render approvals from the harness presentation, not serialized tool input (#21087)
* fix(native-chat): render approvals from the harness presentation, not serialized tool input The approval card built its title from the tool name and rendered JSON.stringify(input) into an element with no height bound. Any large payload - a file write's contents, a proposed plan - pushed the action buttons past the viewport with no way to scroll to them, leaving the prompt unanswerable without zooming the pane out. Thread the agent SDK's own presentation fields through the prompt registry into the journal item: title, displayName, description, decisionReason, blockedPath and matchedAskRule. The SDK documents its title as the prompt text to use instead of reconstructing one, and warns that the decision reason may carry terminal escapes, so those are stripped before rendering. The card now also shows why a request was raised rather than only what it was. Bound the detail in a scrollable region that is reachable by keyboard, and cap it main-side with the existing shared tool-detail limit rather than the far looser journal payload bound. Focus moves to the card when a prompt appears and Escape resolves it, which previously did nothing because the composer owning that handler is unmounted while a prompt is pending. Mobile rendered the same unbounded detail and is fixed alongside. * fix(native-chat): keep approval actions reachable |
||
|
Neil
|
55ae3b393c | fix: make git grep directory filters recursive | ||
|
Brennan Benson
|
4b87bc718e |
refactor(agent-launch): redefine the agent.launch contract (#20999)
* refactor(agent-launch): redefine the agent.launch contract
`agent.launch` has no clients yet, so the contract is redefined in place
rather than versioned.
- params require `operation.id`, pinned to the shipped operation-id mint so
the host can read the embedded timestamp back. No caller-supplied
fingerprint: the host derives its own.
- the result carries `disposition` ('created' | 'replayed', the same
vocabulary `RuntimeCreateAgentSessionResult` already uses) and a single
top-level `warning` instead of one on the terminal arm only.
- the prompt receipt becomes an outcome enum, so a receipt can under-claim
instead of reporting a bare `delivered: false`.
- the dead `customization` field is deleted, and the mode-reason union and
receipt are declared once in shared with main re-exporting.
- `clientMutationId` joins the reserved create fields, with a test pinning
the list to the create schema in both directions.
Contract only; no behaviour change and no ledger wiring.
* docs(agent-launch): stop calling the stripped set "agent fields"
`clientMutationId` joined AGENT_LAUNCH_RESERVED_CREATE_FIELDS, so three
comments describing the stripped set as agent fields now teach the wrong
model — including a SAFETY rationale, where a reader is trusting it most.
The rationale's claim is unchanged and still sound: deleting keys from a
parsed object leaves the rest the parsed shape.
* refactor(agent-launch): make the attempt id the launch's only idempotency key
Review follow-ups on the contract redefinition.
`operation: { id }` becomes a flat `clientOperationId`, spelled the way
`terminal.createAgentSession` and the structured mutation envelope already
spell the same concept, and admitted by the shipped
`parseAgentSessionOperationTimestamp` rather than a second copy of its
pattern — so `agent-session-host-authority` keeps the regex private.
The handler now dedupes on that id instead of the create payload's
`clientMutationId`. That field is optional, so keying on it left any launch
that omitted one with no idempotency at all, while the required attempt id
did nothing. Reserving `clientMutationId` is still right, but for the reason
the comments now give: `createManagedWorktree` never reads it, so a copy left
in the forwarded payload is inert while still reading as a guarantee. The
previous rationale — that it was a second live dedupe key — was not true.
`messageId` moves onto the prompt receipt's `journaled` arm so a producer
cannot report the text as committed without saying where, and `rpcCallerKey`
picks up the `terminal.create` call site it was lifted from instead of
shipping with no callers.
* docs(agent-launch): record why disposition is two-valued only for now
The ledger admits attempts whose outcome was never recorded, and neither
`created` nor `replayed` can say "I cannot tell you" — a caller handed
`created` for an unresolved attempt starts a second agent. Noted at the type
rather than in review, so whoever wires the ledger reads it where they edit.
* fix(agent-launch): keep contract within implemented guarantees
|
||
|
Brennan Benson
|
2fbdada551 |
docs(native-chat): correct why a slash command is inert in the answer row (#21111)
The previous note said running a command from the question card's free-text row could only answer with command text or abandon the prompt. That is wrong about skills, and silent on the real cause. Verified against a live structured session: the typed answer is delivered verbatim as the AskUserQuestion tool result, so it reaches the model but never the command parser. A client-side command is therefore inert; a skill name can still be acted on because the model simply reads it. |
||
|
Jinwoo Hong
|
2e3a24c30f |
fix(cloud-auth): keep Sign in clickable during a pending browser wait (#21078)
* fix(cloud-auth): keep Sign in clickable during a pending browser wait Closing the cloud sign-in tab used to leave every Sign in button disabled as "Signing in…" until the 5-minute loopback timeout. A second click now starts another wait, the first tab can still complete, and the first successful callback wins. STA-7610 * fix(cloud-auth): satisfy typecheck and localization after Sign in unlock Keep the account-pane mock able to represent a missing auth status, and drop unused Signing in catalog entries now that the wait no longer relabels the button. * fix(cloud-auth): ignore a stale sign-in after a later wait succeeds A second Sign in click still starts a new loopback wait. Completing that newer wait links the session; finishing the older tab afterwards is cancelled instead of overwriting the linked identity or toasting again. * test(cloud-auth): cover post-exchange stale connect and pending Sign in Pin the branch that discards an earlier token exchange after a later wait has already linked, keep Sign in enabled while connect is still pending, and suppress a failed toast when auth is already connected. * fix(cloud-auth): do not relink an in-flight sign-in after sign-out Signing out now invalidates outstanding PKCE attempts in main and the renderer so a later browser tab cannot restore the session. * fix(cloud-auth): do not wipe a newer connect that finishes during sign-out If sign-in completes while revoke is still in flight, skip session clear and unlink so the new session survives. Do not toast signed-out when auth is already connected again. |
||
|
Brennan Benson
|
533b0bd02e |
fix(native-chat): count a turn from the send that opened it (#21086)
* fix(native-chat): count a turn from the send that opened it The live turn indicator switched on at the submission but anchored its clock at the provider turn-open, so it jumped back by exactly the dispatch latency the moment the turn opened. Measured on a real Claude session: the counter climbed to "Working for 25s", reset to "Working for 0s", then settled "Worked for 26s" — three readings of one turn, from two different instants. The host now resolves the send that opened a turn and publishes it as an additive optional `requestedAt` on the turn lifecycle row. `startedAt` keeps its exact meaning, the provider turn-open, and is never rewritten, so clients that cannot be upgraded see no change to any value they already read. Both providers write it; it is omitted when no send can be named (provider-resumed turns, replayed history). Readers take one origin, `requestedAt ?? startedAt`, for both the live counter and the settled host interval, so the two cannot disagree. The provider's own reported duration keeps outranking the host interval, unchanged. The host-to-local clock conversion is now latched once per turn rather than re-derived per render. `receivedAt - hostNow` carries that sample's one-way delivery latency as well as skew, and the reducer replaces the sample on every frame, so re-deriving imported fresh jitter and could move the anchor later — the same class of backwards jump this change removes. With the conversion fixed, an origin that improves moves the anchor earlier by exactly that much, so displayed elapsed only grows. No monotonicity guard is added; the ordering is structural. Desktop and mobile drove byte-identical copies of the timing hook, so both are collapsed onto one React-free helper in shared. Regression tests drive the origin resolution rather than an already-resolved anchor, assert in milliseconds because second-flooring hides the sub-second case, and include a deliberate host/client skew so a raw timestamp assignment cannot pass on a machine where the two clocks agree. * fix(native-chat): correlate Codex turn origins by echo * fix(native-chat): preserve causal turn timing ownership * fix(native-chat): keep settled turn timing continuous |
||
|
Neil
|
52b53c5bda |
feat(settings): choose the default terminal shell (#21085)
* feat(settings): configure default terminal shell * test(terminal): cover default shell selection * fix(terminal): apply shell setting to daemon PTYs * test(terminal): provide PTY dimensions in shell cases * fix(settings): clarify default shell behavior * feat(settings): make shell choice explicit * fix(settings): keep shell control testable without preload * fix(settings): slim terminal shell control * chore(i18n): allow terminal shell setting labels * chore(i18n): mark dynamic shell label |
||
|
Jinwoo Hong
|
9add08bb59 |
test(mobile): recorder follow-ups — write ordinal, teardown streams, context anchor, salvage observation, provider pass-through, React draw (#21088)
* refactor(mobile-recorder): one shared write ordinal for requests, payloads and effects
`sent` stamped each payload and effect with the number of requests sent at
write time, which orders those two lists against sends but never against each
other. A family that sends no requests therefore had every stamp at `0`:
moving `host-worktree-refresh.ts`'s two initial snapshot reads from after
`client.subscribe` to before it moved none of the 705 goldens.
One monotonic counter per recording now stamps requests, payloads and effects
alike at the moment each is written, so the three append-only lists are ordered
against each other. The same reorder now fails five goldens. A request is
stamped at the logical `sendRequest` call rather than when its physical payload
is published, so a send that waited for connected carries two distinct stamps.
Full re-record from the pinned baseline: 699 bodies moved, 6 header-only,
0 added, 0 deleted; the only moved JSON paths are `sent` leaving and `ordinal`
arriving on `sender`, `payloads` and `effects`. Decoding with those two fields
stripped leaves all 705 header-only.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* feat(mobile-recorder): observe streams still registered at teardown
Closing a stream only writes to the wire when its method has an unsubscribe
builder. `notifications.subscribe` has none, so a cleanup that forgets its
local `unsubscribeStream()` leaks a live registry record and nothing on the
wire changes. Until now that class was covered by one hand-written scenario
per method, which stops the stream and cuts over so the leak reappears as a
second subscribe payload.
Teardown now asks each session's `RpcClientStreamRegistry` what it still holds,
after the product's cleanup and before the transport disposes it, and records a
non-empty answer as a `streams-registered-at-teardown` effect carrying each
stream's method, subscribe payload and cancelled flag. The set is read off the
registry's own map: a mirror kept by the recorder would reproduce the product's
bookkeeping rather than observe it. Deleting `unsubscribeStream()` from
`mobile-notifications.ts` fails 7 goldens now, against 1 before.
Re-record: 4 bodies moved, 701 header-only, 0 added, 0 deleted. All four are
the two `runtime.clientEvents.subscribe` matrices, on partitions whose subscribe
reply is not a well-formed `ready`: with no subscription id to unsubscribe with,
the registry deliberately holds the cancelled record, which is why the
observation carries `cancelled`.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile-recorder): one host-client context exposure, anchored on the product source
Five adapter modules each carried `exports.recorderHostClientContext = Ctx;`
inside a source string appended to `client-context.tsx`. `Ctx` is a
module-private local, so the reference lives in a string no type checker
follows: renaming it typechecks clean and fails a recording with a
`ReferenceError` a hundred seconds in, five times over.
`hostClientContextExposure` and `loadHostClientContext` are the one copy, and
`adapter-seam.test.ts` asserts the declaration the exposure names still exists
exactly once in `client-context.tsx` and refuses a sixth inline copy. A rename
remains invisible to `tsc` — nothing but editing the fenced product module
makes a private local checkable — so the anchor is what turns it into one
failure that says what moved.
Also splits the subscription tests out of `recording-runner.test.ts`, which
items 1 and 2 had pushed past `max-lines`.
Re-record: 705 header-only, 0 bodies moved, 0 added, 0 deleted; `recorderSha256`
on all 705 and `adapterSha256` on the 23 goldens mounted through the five
modules.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* feat(mobile-recorder): record what a checked read salvaged
`collectSalvageDrops` builds a report on every decoded reply — which array
elements a `salvagingArray` threw away, which members a `salvagedOptional`
read as absent — and `classifyRpcReply` puts it on the outcome, where nothing
reads it. Which rows a reply lost was therefore visible nowhere, including in
a golden.
The recorder wraps `classifyRpcReply` on the mounted module, the one seam every
checked read passes through and the only one that knows the operation the drop
happened under, and records a non-empty report as a `reply-salvage` effect. No
product code changes; the report was already being built and discarded.
No golden carries one. All 19,384 checked reads in the corpus decode their reply
whole, because the reply matrix varies the envelope a host sends rather than the
shape of a row inside a result. The observation pins that absence, and moves the
first time a narrowed element or member schema drops a recorded row — including
where nothing downstream reads it. `salvage-observation.test.ts` is what keeps
the observation honest, driving a malformed row and a malformed optional through
the real `git.status` reply schema.
Re-record: 705 header-only on `recorderSha256`, 0 bodies moved, 0 added,
0 deleted.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(source-control): let hostedReview.create carry a provider token this build does not list
`HostedReviewCreate.provider` was a closed `z.enum`, so a client repeating back
a provider a newer host named in its own eligibility reply had its create
rejected at params validation. Mobile worked around it with a SAFETY-annotated
assertion: narrowing to `'unsupported'` before sending would have made the host
refuse its own provider, so the token was cast through instead.
The schema member is now `z.string()`, and both create handlers narrow through
`supportsHostedReviewCreation` before calling the runtime, so an arm this build
does not know answers `unsupported_provider` with readable copy rather than a
params error the client cannot act on. `createHostedReview`'s own refusal is
the single source of that copy. The mobile assertion is deleted.
Product change on a fenced path, so the goldens are not re-recorded: the whole
recording suite replays green against the corpus committed in the previous
commit, 825 passed, zero golden movement.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(source-control): annotate the runtime stub cast in the provider refusal test
The changed-code quality gate counts a new `as unknown as OrcaRuntimeService`
as a finding. A narrower stand-in does not exist: the interface has 1047
members and `Pick` of the three this test uses is not assignable.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile-recorder): pay React's lazy Math.random draw before the seeded run
React resolves `enqueueTask` by reading `module['require' + Math.random()]` and memoizes the
result, so a process draws exactly one `Math.random()` the first time it awaits `act`. The runner
drains through `act` after every step, so that draw landed inside whichever recording ran first and
ate the seeded sequence's first value: a family recording a `Math.random()`-derived param recorded
one value when it ran alone and a different one when it ran after any other family, and an adapter
could only dodge it by drawing in its factory ahead of the first drain.
The scheduler now pays that draw once per process, before it installs the seeded generator, so the
seeded sequence starts at the same value for every recording. Priming is awaited, which makes
`start` async.
Goldens re-recorded: 705 header-only, `recorderSha256` alone. No golden carried a first-in-process
value, so nothing moved in a body.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* fix(mobile-recorder): drain before reading the streams left at teardown
The teardown observation read the registry after `dispose()` returned but before the scheduler
drained, so a cleanup that closes its stream on a due 0ms timer had not run yet and was recorded as
an uncancelled registration — the one shape this observation reserves for a cleanup that never ran.
A deferred close and a stream nobody ever closed were byte-identical.
The drain now runs before the read, with the transport still disposed after it. A second drain stays
after disposal: tearing the registries down rejects what the product still awaited, and an unhandled
rejection is an effect the cleanup checkpoint has to see.
Also: the registry size comparison in `registeredStreams()` could never fire, because `size()`
returns `this.streams.size` on the same object; `RECORDER_HOST_CLIENT_CONTEXT` is used only in its
own module and no longer exported; and `streamPayloads` now says what it holds, which is every frame
the registry publishes rather than only subscribes.
Goldens are stale in this commit and are re-recorded in the next one.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): re-record the corpus after the baseline repin and the teardown drain
Recorded from a detached worktree pinned at
|
||
 Gon SongandNeil
|
85d1ffc072 |
fix: accept enterprise managed GitHub owner logins (#20450)
Unify owner validation across project pickers and repository overrides. Preserve EMU usernames in API and auth-status branch-prefix resolution, with regression coverage. Co-authored-by: Neil <neil@stably.ai> |
||
  OrcaWinandm4air
|
6101f0169f |
Make CLI reveal labels translatable (#21079)
* fix(i18n): make reveal labels translatable in CliSection
Platform-specific reveal labels ("Show in Finder", "Show in Explorer",
"Show in File Manager") are now wrapped with translate() for i18n
support. Also backfills missing translations in non-English locales.
* add trams;atopm foxes
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
|
||
|
Brennan Benson
|
e6a3b5d019 |
docs(native-chat): record why the question answer row has no slash grammar (#21084)
The free-text row on an AskUserQuestion card is a plain input on purpose, but nothing said so, and its absence reads as a missing picker rather than a decision. Note the reason at the input. |
||
|
Brennan BensonandMerge Sim
|
97aa5ff19b |
fix(mobile): open native chat when a new worktree launches a default agent (#19850)
* refactor(agent-launch): make the launch-mode decision surface-neutral
`decideWorkerStartMode` was the only shared answer to "structured chat session
or terminal agent?", but it lived in an orchestration-named module and spoke
orchestration's vocabulary, so the other launch surfaces could not call it.
Move the decision to `main/agent-launch/agent-launch-mode` unchanged and leave
`orchestration-worker-start-mode` as the adapter that supplies the noun.
A worker is not a special kind of launch; it is the same launch with a dispatch
attached. Naming the receipt's subject is the only thing orchestration actually
contributed, so that is the only thing the adapter keeps: "worker" in both
sentences, plus the `--terminal` wording, which reads as nonsense anywhere a
`--terminal` flag does not exist. Both are pinned, because they are asserted.
No behavior change. The receipts are byte-identical for every reachable case,
proven by running the new pin against both implementations.
Also pins the wording, which nothing was holding. The existing suites assert
`toContain` fragments ('terminal agent', 'cannot create') and the CLI suite
asserts a receipt handed to it by a mock rather than one this code produced;
all six files stayed green against a deliberately corrupted vocabulary. A
dispatch receipt is the only place a structured-to-terminal downgrade explains
itself, so the whole sentence is the contract, not a fragment of it.
* feat(agent-launch): add the launch intent and the one executor that runs it
The sequencing around the launch decision was duplicated per surface, and the
duplicate is where the bug lives. A new worktree was created agent-first, so
its startup terminal WAS the agent and the structured branch below it could
never be reached — every new-worktree launch was a PTY regardless of the user's
default. Orchestration fixed that for itself in #19431; mobile and the CLI
still have it.
`executeAgentLaunch` inverts the order once, for everyone. When the preference
is structured the worktree is created with NO startup agent, the executing host
is then asked whether it can host a session for the workspace that now exists,
and only then is a surface created. The host verdict cannot be hoisted above
creation: `agentSession.createSupport` only answers for a workspace it can
resolve, which is why the decision stays in two halves.
Agent-first creation is deliberately preserved for PTY launches — it is what
sequences the agent's startup command behind the setup runner, so wait-for-setup
comes for free there.
What actually differs per surface is only how a surface is built (an
orchestration worker's session takes a dispatch hold and a mailbox a plain
launch must not take), so that is injected as a factory rather than branched on.
The intent also strips the reserved agent fields from a migrated create payload:
a caller moving off `worktree.create` passes its existing params, and a stale
`startupAgent` in there would re-create the very path this replaces.
Tests assert order and arguments, not just the resulting mode. Reintroducing
agent-first creation reddens 4 of 11.
* feat(agent-launch): expose the launch executor as the agent.launch RPC
Adds `agent.launch` — one host-side method that decides structured-vs-terminal and
creates the surface — wired to the real runtime factories: `createManagedWorktree`
for the workspace, forking on `startupAgent` exactly as the orchestration worker
path does; `createStructuredAgentSessionForWorktree` for a chat session; and
`createTerminal` for a PTY agent. Allowlisted for mobile, which is the surface the
routing gap was reported on.
`worktree.create` is untouched. Its `startupAgent` keeps meaning "spawn a PTY agent"
verbatim, because it answers with `agentTerminalHandle` only on that path: a host
that quietly routed it to a structured session would hand every older client a
response with no handle and no error. All new behaviour sits behind
`agent.launch.v1`, which the host now advertises and a remote client must negotiate,
so a client that does not gets today's behaviour unchanged.
* feat(mobile): route workspace creates through agent.launch
Picking an agent on the mobile create sheet always produced a terminal, even
when the user's default was native chat, because all three create paths put
`startupAgent` on `worktree.create`. That means "create the worktree
agent-first", so its startup terminal IS the agent and the structured branch
below it is unreachable — while the same phone's in-workspace "+" button opened
a chat.
The blank, branch and new-branch creates now send the same payload through
`agent.launch` and let the host settle the surface. `worktree.create` is
untouched, and a host that does not advertise `agent.launch.v1` (read from the
existing `status.get` probe) keeps today's path exactly.
Work-item creates stay on `worktree.create`: they pre-fill the issue/PR URL as
an unsent `startupDraft`, which a structured session cannot hold yet, so routing
them would submit the URL as a first turn.
* fix(agent-launch): drop the deleted draft-prompt blocker from the reason map
main removed the draft-prompt blocker in #19681 (a structured session now holds
an unsent draft), so the exhaustive Record no longer typechecks.
* chore(agent-launch): carry a SAFETY rationale on the agent placement cast
The type-assertion gate landed after this branch's base, so the new file's
copy of the worker-start cast is now a changed-code finding.
* chore(agent-launch): carry agent.launch through main's RPC typing and casting gates
The typed-method contract, the generated params catalog and the
`assertionStyle: never` casting scan all landed after this branch's base.
- AGENT_LAUNCH_METHODS kept an `RpcMethod[]` annotation, which widened its
method name to `string` and broke assignability; every sibling infers instead.
- `agent.launch` binds a schema under src/main, so it joins the catalog's
RPC_METHODS_WITHOUT_SHARED_PARAMS and the parity gate's hand-listed twin.
- The now-typed methods make most test casts unnecessary; the few that remain
carry the line-specific SAFETY rationale the casting gate requires.
* test(mobile): supply the agent-launch fixture the create-submit recording needs
The golden RPC recordings landed upstream while this branch was out, so they
first met agent.launch here. Three things had to happen, and only one of them is
a fixture bump.
1. workspace-settings-mounts.ts mounts useNewWorkspaceCreateSubmit against a
fixture model that throws on any member it was not given. This PR added a
required getAgentLaunchSupport, so the submit aborted with "Missing model
fixture" before it ever issued the create, and three cleanup checkpoints
vanished. That read like a product regression and was not one. Supplying the
member restores the recording byte-for-byte; it is pinned false for the same
reason the cutover probe is, so the baseline stays on worktree.create.
2. Editing that adapter moves adapterSha256 for the twelve settings goldens it
mounts. Their recordings are unchanged - header only, by design: the digest
is per-golden so editing a module fails exactly the goldens that mounted it.
3. Five goldens changed behaviourally, and both changes are this PR's:
the capability probe now reports agentLaunch, and a create whose reply
carries no worktree returns "Failed to create workspace" instead of throwing
a TypeError off an unguarded result.worktree read. The launch route needs
that guard, since a receipt can arrive without a worktreeId.
* refactor(mobile): decode the launch receipt instead of asserting its shape
The changed-code quality gate refuses type assertions, and the eight it flagged
were worth removing rather than suppressing.
The production one was the point. readAgentLaunchCreateOutcome asserted the RPC
payload into Partial<AgentLaunchResult> and then runtime-checked it anyway, so
the assertion bought nothing and claimed a contract the host had not proven. It
now narrows with `in` and validates each hop, which is the same nullability
question readCreateResult already answers on the sibling path - a launch receipt
can legitimately arrive without a worktreeId. AgentLaunchCreateOutcome ties
worktreeId to the shared contract so a change there fails this reader's
typecheck rather than passing a differently-typed field through.
The test fakes claimed a whole RpcClient via `as unknown as RpcClient` while
implementing one member. They now build a typed literal, matching the pattern in
use-mobile-structured-agent-options.test.ts. The read sites cast params and then
read one field; they now assert the payload with toMatchObject, which removes
the cast and pins more of the shape than the cast did.
Also pins the warning passthrough, which nothing covered: a terminal launch that
seats the workspace but cannot start the pty reports why, and the absent, blank,
non-string and structured-surface cases report nothing. Writing that test caught
a real drop I had introduced in the reader.
* ci(mobile): re-run Mobile Checks when a shared capability changes
Mobile Checks is path-filtered to mobile/**, but mobile imports the negotiated
capability names straight from src/shared/protocol-version.ts and records the
whole capability read verbatim in its goldens. So a capability added desktop-side
rewrites a mobile fixture while never triggering the suite that would catch it.
That is what happened here: #19849 introduced agent.launch.v1 and Mobile Checks
never ran on it. Verified at the run level rather than by check name - the
window-free check-runs API on
|
||
|
Jinwoo Hong
|
12d744f253 |
fix(skills): keep computer-use off filesystem and shell tasks (#21069)
* fix(skills): keep computer-use off filesystem and shell tasks STA-7615: "On my desktop create a folder" was matching computer-use because discovery copy said OS/window-level and neighboring skills advertised desktop UI. Scope the trigger to visible GUI with no CLI path, and exclude files/folders/git/shell. * fix(skills): prefer programmatic paths over computer-use State the last-resort rule in discovery copy instead of enumerating files/folders/git/shell. computer-use prefers shell, filesystem, git, HTTP, CLIs, and Playwright/CDP; neighboring skills route to Computer Use only when a visible window needs GUI control those cannot do. * fix(skills): stop advertising computer-use from orchestration Orchestration coordinates workers; it does not drive a GUI. Drop Computer Use and Playwright/embedded-browser routing from its discovery description so those tools are not pulled in from a coordination skill. * fix(skills): drop Playwright from orca-cli discovery orca-cli should not prescribe Playwright or CDP. Those tools may not be installed, and page automation is not this skill's job. * fix(skills): drop the page-only ban from computer-use discovery Page automation is a preference, not a prohibition. If Playwright or CDP is not available, a visible browser window is valid Computer Use. Keep the hard split for Orca's embedded browser (`orca-cli`) only. |
||
|
Brennan Benson
|
f02d09c1ba |
fix(native-chat): deliver queued messages while the chat pane is hidden (#20659)
* fix(native-chat): deliver queued messages while the chat pane is hidden With two or more messages queued, everything behind the head waited on the user's attention. The drain only inspected the head and returned unless it was `queued`, and a `pending` send deliberately leaves the head `dispatching`. An entry only leaves that state through the journal subscription, which is torn down when the pane goes hidden -- and a worktree switch hides it. Two changes, both needed: - One shared admission rule now says what the queue does next, and the drain takes its `dispatch`: the first `queued` entry, skipping entries the host has already acknowledged. It still stops at an `unconfirmed` entry or a refusal the user must act on. Order is not the outbox's to keep -- the host appends the submission inside the per-session serialize chain before dispatching, so journal order is arrival order. Holding the tail bought no ordering guarantee and cost delivery. Single-flight still keeps sends strictly sequential, and a launch prompt's in-flight send, which runs outside it, still stops the queue. - The journal subscription now stays open while a session has undelivered outbox entries, published from the `writeOutbox` choke point. The subscription's retaining hold is what also keeps the host from evicting the session 15s after the last turn, which would otherwise turn the stall into a blocked head refusing `agent_session_ownership_unknown`. An acknowledged entry stays in the outbox rather than retiring on `pending`: the text is safe either way, since the journal upserts a render item from the submission's own body, but a `pending` can still settle `rejected` or `unknown` and only the entry carries the retry state that answer needs. Follow-on corrections the head-only assumption had hidden: - Single-flight is released where the disposition is applied, not in a later `.finally`. That state write is what re-runs the drain, so the release has to land first or the queue has no trigger left. - One ref now holds the in-flight entry's id instead of a bare boolean, and the reconcile effect keys its release on that, not on the head, so a journal update about the head can no longer discard a still-unsettled send of the tail. - A refusal blocks the entry it refused, read back by index so a rotated id is preserved. - The automatic unknown probe and the Retry affordance both read the blocker at whatever index it sits, the Retry through the same shared rule as the drain. `raises no delivery notice for a stuck message behind a healthy head` asserted that a message behind an admitted head raises nothing, because a Retry could not act on it. It now can, so that guard is rewritten to assert the notice names that entry and its Retry sends that entry. * fix(native-chat): resume outbox after journal admission and scope subscriptions * test: name outbox send request by domain role |
||
|
Brennan Benson
|
36cdb34097 |
test(agent-status): pin each legacy-bypass detector to its own case (#21004)
The ratchet's planted-fixture test collapsed every detection into a deduplicated kind set, so `passed-map` — which has two independent producing sites — stayed green when either one broke on its own. Give each planted form its own case with an exact expected detection. |
||
|
Jinwoo Hong
|
a28085adbf |
refactor(mobile): checked reply readers for the source-control domain (step 7 pilot) (#20950)
* test(mobile): ratchet the 201 unchecked RPC reply readers Step 4 moved every call-site cast into an RpcOperation's `read`, but 201 of those readers still answer `compatible: true` for any payload: `rpcUncheckedPayloadReader` (163), `rpcReadUnchecked` (26 outside its own module) and `rpcUncheckedMemberReader` (12), across 42 files. The cast moved; it did not become true. Held as data with an AST boundary test, shaped on the raw-request-port ratchet: a file that is not listed fails, a listed file that no longer has one fails, and a count that rises fails. Only a call counts, so an import is not a reader and prose never is. No behaviour change: this commit adds a list and a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): validate the source-control domain's RPC replies at arrival Replaces all 17 unchecked readers in mobile/src/source-control/ with `rpcResultVariant(variant, schema)`, so a malformed reply is an `RpcIncompatibleReplyError` naming the operation instead of a TypeError three frames downstream. The inventory drops 201 -> 184 and the five source-control operations files leave it entirely. This is a behaviour change, scoped to malformed replies. Six reply-matrix goldens move; every named-scenario golden and every `normal` partition is byte-identical, which is the parity claim. Schemas live one module per reply domain, beside the operations that read them: git-status, git-compare, git-history, hosted-review and worktree-metadata. A member is required only where a consumer reads it unguarded, and each schema records the consumer line that justifies it. Nothing is `.strict()`; every reply a consumer publishes verbatim keeps `z.looseObject` so an undeclared host member still passes through. Six replies have no reader anywhere in mobile and get `z.unknown()`, which is the honest schema for them, not a holdout. Three readers stay total by construction, because their contract is that an unreadable reply is a value rather than an error: the `git.status` projection (a null status three screens route on), the `session.tabs.list` reveal (a null list means poll again) and the generated commit message (a screen's copy, never a decode error in a text field). They gain the salvage report, not a verdict. Consumers take the schema's output type, so `MobileGitStatusResult` and the branch-compare aliases now name what mobile reads rather than the desktop aggregate, and seven call-site casts are gone. Three requirements came from the goldens, not from the host types: `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the six source-control reply-matrix goldens step 7 moves Six goldens, all on malformed partitions. Every named-scenario golden and every `normal` partition is unchanged, which is the parity claim for this step. git.history-read / git.history#1 result-absent, result-null, inner-ok-missing, inner-false-string-error, inner-false-object-error: the load rejected with a TypeError reading 'items' or 'map' off undefined/null; it now rejects with `incompatible_reply: git.history-page (git.history)`. hostedReview.eligibility + create-intent / hostedReview.getCreationEligibility result-absent, result-null, inner-ok-*: the fetch fulfilled with the error envelope itself, re-typed as an eligibility and published into the compose prefill; it now rejects, and both callers already route that to the same "eligibility unavailable" state a null answer produced. hostedReview.create-chain + create-intent / hostedReview.create result-absent, result-null, inner-ok-missing, inner-false-object-error: the create form showed the raw TypeError text "Cannot read properties of undefined (reading 'ok')"; it now shows the incompatible-reply message. Every header digest is unchanged -- baseline, recorder, adapter, scenario and lockfile all match -- so the diff is the behaviour and nothing else. Recorded from this branch into a scratch directory and copied in, because there is no scoped honest alternative: scripts/rpc-recording.mts refuses to run unless the product tree equals the pinned baseline, and the README's remedy for an intended behaviour change is to repin, which rewrites the `baseline` header of all 667 goldens. So these six now carry a pin whose tree no longer produces them. That is a real gap in the oracle's design for behaviour changes, not a detail of this step, and it needs a decision before this lands. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the four reply-schema properties the goldens found Each of these cost a reply-matrix golden while writing the source-control schemas, and none of them follows from reading the consumers or the host types: a newer host's undeclared members must still decode, `git.history` sends `timestamp: null`, `hostedReview.getCreationEligibility` sends a `reviewLookupOutcome` the shared union does not list, and the `git.status` projection writes an absent member as a present `undefined`. The `.strict()` case is the one worth stating twice: at the top level it rejects the reply, and on the entry it drops the row, which shows a dirty worktree an empty Changes list. The fifth test pins the salvage report that makes such a drop visible instead of silent. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): give an unreadable reply a message a user can read `RpcIncompatibleReplyError` put `incompatible_reply: <op> (<method>)` in `message`, and `message` is what the screens hand to a toast. Step 7 is the first change that can reach this error at all, so the token would have shipped to users as its own error copy. Fixed at the boundary rather than per site: `message` is now plain copy, and the machine token moved to `code` (`incompatible_reply`) and `name` (`RpcIncompatibleReplyError`), both readable by callers. The cross-bundle fallback in `isRpcIncompatibleReplyError` matched on the old message prefix, so it now matches on `name`, which a foreign copy of the module still carries. No existing test pinned the old text. Two new ones pin the copy, the token and the foreign-copy match. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the recording baseline to this branch and re-record Commit |
||
|
Jinwoo Hong
|
740887fbbb |
feat(settings): connected computers rows for session history indexing (#20887)
* feat(session-search): add ranked history panel search and consent * test: wait for initial session indexing before refreshing results * feat(session-history): add local search settings and index controls * Use shared local host identifier for session index status * feat(session-search): merge all-computers search across hosts The `all` scope on `aiVault:searchSessions` now fans out from the desktop to every host the session list enumerates and merges the pages into one. Legs run in parallel: the local index through the search service, SSH and runtime hosts through the existing remote search client. Two fixed orders, because relevance scores from independent indexes are not comparable. `newest` asks every leg for recency and k-way merges on `updatedAt`, nulls last, ties broken on execution host id. `relevance` rotates hosts in host-id order by their own rank. The merged cursor is an opaque base64url payload holding each host's cursor, how many of its current page were already emitted, and the generation that offset counts into, plus the page size and sort the cursor belongs to. A host whose index moved is fenced to `stale` and stops contributing; the rest keep paging. Per-host outcomes ride back on one new optional `hosts` field on the results response. `aiVault:searchStatus` with `all` stays refused, and neither the runtime RPC nor the CLI gains the scope, so a fan-out is never two hops. * fix(preload): let the search bridge address the all-computers scope * feat(settings): live index status, enable confirm, advanced delete * feat(session-search): search every computer from the history panel The panel's "All computers" scope produced no request: the hook parsed the scope into a single host id and stopped when that was null, so the panel answered "Choose one computer to search its sessions." The desktop already merges every enumerated host behind `aiVault:searchSessions`, so pass the scope straight through and stamp each hit with the host it came back on. Hosts the merge could not search are named under the results header with a short reason, since a silent partial answer reads as "no such session". (cherry picked from commit |