mirror of
https://github.com/stablyai/orca.git
synced 2026-09-24 16:02:41 +00:00
stack-foundation
2191
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Brennan Benson
|
9ed561c1d2 |
fix(claude): judge Stop against the turn the journal published (#20921)
* fix(claude): judge Stop against the turn the journal published A Stop could be refused for the turn the user was actually looking at. The client derives the id it sends from the published journal rows, but the host compared it against the adapter's own in-memory turn. The journal sink drains asynchronously, so that in-memory value can already name a turn whose row has not landed — an id no client has been shown, and one the client's Stop can therefore never match. The user pressed Stop and nothing stopped. Fix the guard's source rather than the guard. ownsRequestedTurn stays: it is what stops a delayed request from interrupting a later turn, and without it a stale Stop would reach a session-scoped interrupt that settles every queued send as durably rejected. The host now resolves the live turn from the journal projection and hands it to the adapter, which prefers it and falls back to its in-memory read for direct callers that have no journal. It is passed as a read rather than a value because the guard re-checks after the delivery fence may have waited seconds; a value captured at request time would interrupt whatever turn ran next. Both callers supply it. The handoff's own Stop bypasses performCancel, so its body moves into stopNativeHandoffTurn beside the file's other extracted flows, which is also what lets it be tested on its own. * fix(claude): fall back to the in-memory turn while the journal drains The journal drains through a serialized async queue, so a live turn routinely has no published row yet. Judging a Stop only against the journal refused in that window, which gates a user action on bookkeeping. The journal stays authoritative while it HAS an answer; a null read falls through to the in-memory turn, and the nothing-dispatched clause is unchanged. Both call sites now read the live turn through `journal.activeTurnId()`, which folds reduced items instead of rendering and sorting a whole snapshot. |
||
 Jinjing
|
47bb473ec6 |
Remove agent map from dashboard popout (#20929)
The agent map view was not functional and its components have been removed entirely. The dashboard popout now only supports the kanban board view, with all map-related code, utilities, types, and translations cleaned up accordingly. |
||
 Jinwoo Hong
|
96d77b37c5 |
perf: always show project names and remove notification scans (#20931)
* perf: avoid repeated agent scans when labeling notifications * perf: always label notifications and remove project counting * fix: qualify the notification project group by its folder's host Folder notifications resolved the folder host-aware, then looked its project group up by bare ID. The owner index fails a bare ID closed when two hosts publish the same group ID, so a remote folder lost the project name the catalog already had. Also drops the identity rescans that recovered display fields: the catalog finders now return the caller's row type, matching findIndexedRepoOwnerForHost. Updates the idle-arbitration expectation that still asserted the removed hasMultipleActiveRepos flag. |
||
|
Brennan Benson
|
7ec2986fd1 |
fix(lint): merge the duplicate agent-status contract type imports (#20907)
main's tip fails audit:code-quality:native on import(no-duplicates), which reds the static analysis and verify jobs of every open PR via the merge ref. |
||
 Brennan BensonandMerge Sim
|
6da72383df |
feat(agent-launch): one executor for agent launches, exposed as agent.launch (#19849)
* 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.
* 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.
* docs(agent-launch): stop the receipt-wording comment claiming a migration
The decision was never moved out of orchestration-worker-start-mode; this PR
adds a second copy beside it. Say so, and name the unenforced agreement.
* docs(agent-launch): stop the executor comment claiming a migration that has not happened
The header asserted two things the tree does not support: that every launch
surface routes through the executor, and that the mode decision "already lived"
in `agent-launch-mode`. `agent.launch` is the executor's only consumer, and
`orchestration-worker-start-mode.ts` is byte-identical (blob
|
||
 Neil
|
13ba649c22 |
fix(terminal): let a runtime-created Windows terminal BE the requested shell (#20825)
* fix(terminal): let a runtime-created Windows terminal BE the requested shell
`orca terminal create --environment <windows-host> --command 'cmd.exe'` never
created a cmd terminal. `--command` is text the provider TYPES into whatever
shell it spawned, so the PTY stayed the host's default shell with cmd running
inside it. Captured on `awin`, whose default is Git Bash:
$ orca terminal create --environment awin --command 'cmd.exe' --json
$ orca terminal send --environment awin --terminal term_10656cf7... \
--text exit --enter
$ orca terminal read --environment awin --terminal term_10656cf7... --screen
neil@awin MINGW64 ~/orca/orca ((30f820708f...))
$ cmd.exe
Microsoft Windows [Version 10.0.26200.9445]
C:\Users\neil\orca\orca>exit
neil@awin MINGW64 ~/orca/orca ((30f820708f...))
$
The handle is alive the whole time and `terminal list` shows one healthy
terminal, because the PTY never changed — so the only symptom is that the
caller's terminal is now a shell it never asked for, and every later `send` is
quoted for the wrong one. On `win-lowspec` (default pwsh) the same create lands
cmd inside PowerShell.
Root cause
----------
There are two spawn preflights and they are twins:
- `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns, i.e. a
terminal tab opened in the app.
- `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: the CLI's
`terminal.create`, headless `orca serve`, and every paired remote
environment.
Only the IPC twin read the caller's requested shell. The runtime twin passed a
literal `requestedShellOverride: undefined`, so a runtime-created terminal on
Windows could only ever be the host default. Everything downstream of that
point — `spawn-options`, the daemon, `resolvePtyShellOverride` in the relay,
`local-pty-launch-plan` — already honoured `shellOverride`; nothing upstream
could supply one.
Change
------
- Thread `shellOverride` through the runtime lane: `RuntimePtySpawnArgs` ->
runtime `spawn-preflight` -> `RuntimePtyController.spawn` ->
`TerminalCreateOptions` -> the `terminal.create` RPC's new `shell` param ->
`orca terminal create --shell`.
- Thread it through the renderer-backed lane too (`createDesktopTerminal` ->
`terminal:requestTabCreate` -> `store.createTab`), so `--shell --focus` is not
silently dropped on a local Windows app.
- An agent launch quotes its startup command for the shell it will actually run
in, so a requested shell now owns the startup-shell family instead of the
global `terminalWindowsShell` setting.
- Lift the relay's `ALLOWED_WINDOWS_SHELL_OVERRIDES` into
`isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts`
(membership unchanged) so the CLI, the zod param schema, and the relay refuse
the same names. `--shell` therefore cannot carry a path or a command line into
`pty.spawn`; only allowlisted bare shell names pass.
- Gate on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY`. An older host
strips the unknown `shell` param and answers with a healthy terminal running
its default shell — a reply indistinguishable from success — so the CLI
refuses before creating anything rather than creating the wrong shell quietly.
`--shell` stays Windows-only; macOS and Linux hosts spawn the login shell and
the relay drops the value off win32 rather than honouring it half-way. A WSL
project runtime still outranks it, unchanged.
Tests
-----
- `pty-spawn-shell-override-parity.test.ts` pins both preflights against the
exact drift that caused this (verified failing with the fix reverted).
- `createTerminal` passes `shellOverride` to `ptyController.spawn` with no
startup command.
- CLI: sends `shell`, refuses a shell the host cannot spawn, and refuses a host
without the capability — in both refusals without making the round trip.
- Allowlist and `terminal.create` schema accept/refuse cases, including paths
and appended arguments.
* fix(terminal): refuse a requested shell the execution host cannot apply
The first commit made `--shell` reach the spawn, but only a LOCAL win32
execution host applies it: `spawn-options` gates the override on
`process.platform === 'win32' && !args.connectionId`. So `--shell cmd.exe`
against an SSH-routed worktree, or against a macOS/Linux host, still returned a
healthy terminal running that host's default shell — the same
indistinguishable-from-success reply the capability gate exists to prevent, one
layer down.
Refuse instead, before anything spawns. The check sits at the top of
`resolveAgentTerminalCreateOptions`, which every create lane funnels through, so
neither lane has to remember it; the desktop lane additionally refuses a
worktree-less create, which has no execution host to resolve a shell on.
An SSH host's platform and installed shells are not visible to this runtime, and
a POSIX host has no Windows shell to pick. Neither can honour the request, and
saying so is the whole point of the flag.
Docs and the CLI spec now say "refused", not "ignored".
* fix(terminal): refuse a shell that contradicts the project execution runtime
`resolveLocalWindowsTerminalRuntimeOptions` does not merely rank the project's
execution runtime above a per-terminal pick -- it REWRITES the pick, in both
directions, and says nothing:
- a WSL project forces `wsl.exe`, discarding `--shell cmd.exe`;
- a Windows-host project discards a WSL name and falls back to `COMSPEC`
(`getHostShellForProjectRuntime`), so `--shell wsl.exe` spawns cmd. That is
the common case, not an edge: `resolveProjectExecutionRuntime` resolves
`windows-host` for every project that is not WSL, while a repo belonging to no
project honours `wsl.exe` -- so the same flag behaved differently depending on
whether the repo was in a project.
Either rewrite returns a healthy terminal running a shell the caller did not ask
for, which is the failure `--shell` exists to remove.
It also split an agent launch's quoting from the shell that receives it. The
previous commit made the startup-shell family follow the REQUESTED shell, so
`--shell wsl.exe --command codex` on a Windows-host project typed POSIX-quoted
launch args into cmd. Refusing the contradiction removes that case rather than
papering over it.
Refuse instead, alongside the SSH and non-Windows refusals, from the same
`resolveAgentTerminalCreateOptions` seam every create lane funnels through.
Also from review:
- the allowlist test looped the list against itself; spell the members out.
- the runtime spec case claimed to prove the pty's shell when it asserts the
controller received the field; name it for what it checks.
Reported by an adversarial review of the branch.
* fix(terminal): canonicalize --shell and refuse a WSL-path rewrite
Review of the --shell create path turned up two ways the terminal could
still end up being a shell the caller never asked for -- the exact failure
--shell exists to remove.
Bare and mixed-case spellings passed the allowlist but reached consumers
that exact-match the canonical name: resolveWindowsShellStartupFamily
classified `cmd` as the PowerShell family, resolveWindowsShellLaunchArgs
fell through to empty shellArgs (no `chcp 65001`, no OSC 133 bootstrap that
Windows foreground status depends on), and resolveWindowsGitBashShellPath
compares case-sensitively so `Git-Bash` spawned a literal `Git-Bash`.
The allowlist is now one canonical-name map and terminal.create canonicalizes
on parse, so the spawn path only ever sees `.exe` spellings. `pwsh` and
`powershell` stay distinct binaries.
A `\\wsl$\<distro>\...` cwd made the providers force wsl.exe regardless of
the request, and terminalShellOverrideRefusal only inspected the project
runtime -- undefined for a folder workspace with no project. Refuse on the
resolved cwd and the workspace path, judging what the PTY actually gets.
Also: the capability gate reported an unreachable host as too old rather
than unavailable; the SSH CLI shim dropped capabilities from status, so
--shell there blamed the host version instead of naming SSH; and --shell
had no help entry, rendering bare in `orca terminal create --help`. Adding
that entry crossed help.ts's max-lines cap, so the flag table moved to
flag-help-text.ts rather than suppressing the rule.
Adds a behavioural test for the runtime preflight (the one-line fix was
pinned only by a source-text scan), plus coverage for the startup-command
quoting family, the no-workspace refusal, and the WSL-path refusal.
* fix(build): keep tests out of the RPC params catalog bundle
The catalog walk under methods/ already skips *.test.ts, but the contract
directory glob took every .ts. terminal-create-shell-param.test.ts is the
first test to live there, so the bundle pulled vitest into a CJS build and
the generator threw on require(). Same exclusion, same reason.
|
||
|
Brennan Benson
|
0325f1a22e |
feat(agent-status): add the canonical store and child-work contract (#20717)
* feat(agent-status): add canonical shared store contract * fix(agent-status): harden canonical store invariants * fix(agent-status): close canonical store race windows |
||
|
Neil
|
72a8c096a3 |
fix(ssh): corroborate an empty lsof answer before calling an endpoint free (#20585)
#18304 decided enumerability after `lsof` runs, keying on stderr, non-numeric output, and an abnormal exit. One failure carries none of those signals: probing as a uid that does not own the socket's holder, `lsof -t -a -U <path>` exits 1 with no stdout and no stderr. Measured on Debian 12 against #18304's own probe, a live relay owned by root probed as `nobody`: probe uid path marker pids merged nobody held.sock lsof [] <- live relay holds it merged nobody stale.sock lsof [] <- genuinely nobody merged root held.sock lsof [10] merged root stale.sock lsof [] The first two rows are byte-identical, so nothing about lsof's answer can separate them. The first reaches `verdict: exited / evidence: no-holder`, which `classifySupersededRelay` maps to `stale-endpoint-removed` and `rm -f` on an inode a live relay is still holding. `hidepid=2` produces the same shape. A positive control does not solve this. Controlling on something the probe itself holds passes precisely when we are blind: as `nobody`, `lsof -t -p $$` returns a pid while the socket query returns nothing. Blindness is to *other* uids, and another uid's process is not ours to manufacture. /proc/net/unix is. It is world-readable and lists every bound unix socket regardless of owner, so an entry for the path alongside no reported pid proves lsof was blind rather than that the path is free. Only an otherwise-clean empty answer is corroborated; a reported pid still stands on its own, and the check is skipped when the answer was already unavailable. Same run, with this change: fixed nobody held.sock unavailable [] <- no longer reapable fixed nobody stale.sock lsof [] <- still reapable fixed root held.sock lsof [10] <- unchanged fixed root stale.sock lsof [] <- unchanged The marker can only ever move from `lsof` toward `unavailable`, so this never authorises an unlink that #18304 refuses. Off Linux there is no /proc/net/unix, the check returns false, and behaviour is exactly as before -- deliberately, because defaulting to `unavailable` there would stop every macOS host from reaping a stale endpoint and trade a rare destructive bug for a universal accumulation one. The tests are Linux-gated for the same reason, with an assertion that the evidence they depend on is actually present so the block cannot pass vacuously. |
||
|
Brennan Benson
|
1457d3966c |
fix(native-chat): release sessions after provider root exit (#20502)
* fix(native-chat): bound structured chat launch * Fix post-merge test hygiene * Make structured fallback settlement exhaustive * fix(native-chat): release sessions after root exit * chore(i18n): remove legacy fallback copy * test(native-chat): remove terminal fallback census * docs(native-chat): clarify root-exit lease proof * chore(native-chat): drop unrelated formatting * fix native chat launch visibility * test(native-chat): split message rail windowing coverage * fix(native-chat): keep transport gating render-pure * fix(native-chat): coordinate launch prompt settlement * test(native-chat): align unified close ownership * fix(native-chat): correct lifecycle imports and test typing * fix(native-chat): fence restored launch cancellations * fix(native-chat): fence authoritative cancellation snapshots |
||
|
Brennan Benson
|
60d793956a |
fix(native-chat): replace the raw question tool row with an awaiting-input row (#20724)
* fix(native-chat): replace the raw question tool row with an awaiting-input row
A question tool call rendered as ordinary tool activity — "Running
AskUserQuestion" with a clipped JSON payload while live, then a "1x
AskUserQuestion {...}" run header once settled — so the one row the reader
actually has to act on read as machine output.
It now draws as "Awaiting user input: <question>", led by a comment-bubble
glyph, with the label pulsing while the answer is outstanding and reading
"Asked: <question>" once it lands. A grouped prompt names how many questions
it asks rather than quoting only the first, since one row stands for the whole
prompt. Question calls also leave the run header, so the count beside them
reports only the work that actually ran.
Codex journals only the question and never a call for it, and a pending
question was dropped from the transcript entirely — its chat log said nothing
while the agent sat blocked on the reader. Pending questions now project the
same row. Claude journals both the call and the question it raised, so the
call itself is suppressed and the one row is fed from one source.
* refactor(native-chat): derive the awaiting-input row from the question item
The first pass fabricated a synthetic `request_user_input` tool call inside the
shared journal projection so that one renderer could serve every lane. That made
a presentation choice on behalf of every consumer of that projection, including
archives and older RPC clients that never asked for it.
Question presentation is now client-local. The shared projection is restored
untouched, and the desktop transcript derives its own rows: a pending question
keeps a stable identity row through tool folding while its receipt draws the
awaiting line, and the duplicate AskUserQuestion call Claude journals beside the
question it raised is suppressed only when a matching question is open in the
same turn — so an unmatched call, or one from a lane that journals no question,
still reports itself.
Question calls now leave the run together with their paired result, which stops a
summarized ask from stranding its answer as an orphan Result row. A failed ask
keeps its error instead of being folded into the awaiting row, and an ask no
longer contends with a concurrently running tool for the active slot: both are
reported.
Adjacent pending questions — the shape Codex journals, one item per question —
group into a single awaiting row that narrows as each one is answered.
Also ships the three awaiting-row strings in the runtime-required English
catalog. Their call-site fallbacks are a shared constant rather than string
literals, so i18next cannot rebuild them from the call site and they have to be
present for the static-analysis gate to pass.
* fix(native-chat): preserve unmatched duplicate question calls
* fix(native-chat): avoid repeated grouped question text
* fix(native-chat): keep pending question text specific
* fix(native-chat): avoid repeating single question answers
* fix(native-chat): narrow question receipt subject
* fix(native-chat): preserve settled ask calls
* fix(native-chat): cover bridge ask rows
* fix(native-chat): fold settled ask receipts
* test(native-chat): cover settled ask receipt folding
|
||
|
Jinwoo Hong
|
62c5037cc3 |
fix(lint): avoid reflective status entry reads (#20872)
* fix(relay): resolve packaged node-pty from resources * fix(lint): avoid reflective status entry reads |
||
|
Brennan Benson
|
9ab0a18e82 |
refactor(agent-status): isolate legacy status ingress behind one admission point (#20716)
* refactor(agent-status): isolate legacy status ingress * fix(agent-hooks): move advertised-capability source onto the ingest envelope ingestRemote() gained a third positional argument in this PR (advertisedAgentStatusCapabilities) to satisfy a new ratchet requiring every legacy-ingress call site to name its capability source. Both production callers pass the same constant every time, so the argument carries zero runtime information — but Vitest's toHaveBeenCalledWith matches argument count exactly, so the pre-existing SSH relay integration test (which asserts a 2-argument call) started failing even though nothing about the actual admission decision changed. Capabilities are a property of the producing peer/connection, not an orthogonal call parameter, so move the field onto the envelope object instead of adding a third positional argument: ingestRemote reads envelope.advertisedAgentStatusCapabilities (defaulting to the unadvertised-legacy-peer set), and both call sites stamp the constant onto their envelope literal. Call arity stays at two arguments, so the pre-existing evidence test needs no change. The envelope never crosses the wire in either caller: SSH rebuilds it field-by-field from the RPC params, and the WSL path copies (never mutates) the wire-deserialized notification before stamping the field on, so this is purely an internal main-process shape change. Also strengthens the ingress ratchet test that required this: it previously only checked that the capability constant's name appeared somewhere in each caller's source, which a stray unused import could satisfy. It now asserts the actual `advertisedAgentStatusCapabilities: AGENT_STATUS_LEGACY_UNADVERTISED_PEER_CAPABILITIES` key:value binding is present. |
||
|
Neil
|
231e805b1e |
fix(lint): enable anti-slop/no-shape-in-symbol-names (#20785)
Flip `anti-slop/no-shape-in-symbol-names` from "off" to "error" and clear
every violation under src, config, tests and mobile.
What the rule bans
------------------
The case-insensitive substring "shape" in any JS/TS identifier: variables,
functions, parameters, types, type parameters, class members, private names,
object-literal keys and JSX identifiers. The one exemption is a statically
accessed member read owned by another value (`zodObject.shape` is fine), so
third-party APIs stay readable without a suppression.
"Shape" names a value's structure rather than its domain role. `UserShape`,
`validateArgShape` and `errorShape` all tell you the symbol is "an object
with some fields" -- which is already what a type says -- while saying
nothing about what the value is for or who owns it. The rule forces the
name to carry the domain instead.
Violations fixed
----------------
689 violations across 109 files at baseline (verified by re-running the
audit against the pre-change tree with the rule set to "error").
Fix pattern
-----------
Rename for the domain role, not the structure:
-type FieldShape = 'list' | 'map' | 'whole'
-const FIELD_SHAPES = { ... } satisfies Record<keyof Observation, FieldShape>
+type FieldEncoding = 'list' | 'map' | 'whole'
+const FIELD_ENCODINGS = { ... } satisfies Record<keyof Observation, FieldEncoding>
-function assertGitPushTargetShape(target: unknown): void
+function assertValidGitPushTarget(target: unknown): void
-function describeReadDirPathShape(p: string): ReadDirPathKind
+function classifyReadDirPath(p: string): ReadDirPathKind
Predicates became statements about the value (`isDeltaShapedProviderFrameKind`
-> `isDeltaProviderFrameKind`, `isDeleteShapedDiscardEntry` ->
`discardDeletesEntryFile`, `isSkillsCliAgentKeyShaped` ->
`isUsableSkillsCliAgentKey`). Type aliases dropped the suffix where the
remaining name was already unambiguous (`GhGraphqlErrorShape` ->
`GhGraphqlError`).
No wire-visible name was renamed: no IPC or RPC channel, stream opcode,
request/response param, persisted field, or i18n key. The `--shape=symlink|copy`
CLI flag read by .github/workflows/skill-update-roundtrip.yml is unchanged --
only the local variable holding it was renamed.
Exemptions
----------
They are file-scoped entries in config/oxlint-anti-slop.json, not inline
`oxlint-disable` comments. An inline directive naming an anti-slop rule reads
back as an UNUSED directive under the root lint scan, which does not load this
plugin -- the changed-code quality gate counts that warning, so the comment form
cannot be used for a rule that lives only in this config.
* src/renderer/src/components/browser-pane/annotate/**:
in the screenshot annotator a "shape" is the drawn geometry -- pen, arrow,
rect, ellipse, highlight. That is a genuine domain noun, and it pervades
every symbol in the module.
* repo-icon.tsx, repo-header-project-actions.tsx, mobile MobileRepoIcon.tsx:
lucide exports the icon component as `Shapes`. The name is theirs, and the
matching REPO_LUCIDE_ICONS key is the persisted icon name shared with the
desktop picker -- renaming it would orphan saved repo icons.
* src/shared/onboarding-state-types.ts, src/shared/constants.ts:
`shapedSidebar` is a persisted onboarding-checklist field and a telemetry
enum member; renaming it would orphan saved state.
* src/shared/rpc-contract/rpc-send-params.ts: matching zod's own literal `shape`
property is what selects the ZodObject branch of the conditional type.
No exemption was added merely to avoid a rename. Eight symbols initially
suppressed as "a cross-module refactor outside this change" were proven to have
zero non-TypeScript references repo-wide and renamed instead.
Zod's `ZodRawShape` needed no exemption at all: `Readonly<Record<string,
z.ZodType>>` is its definition, so repo-update-params.ts and
ui-update-value-tolerance-params.ts spell it out instead. Likewise
telemetry-event-classification.ts now reads `.shape` through an `in` narrowing,
which also retires two pre-existing type assertions; three more assertions the
rename had dragged onto changed lines (two `JSON.parse` sites, one node:sqlite
row read) became annotations and an explicit row mapping.
Verified
--------
* Audit reports zero violations; confirmed the rule genuinely fires by
planting a probe violation.
* node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0.
* Vitest over src/shared, src/main/github/project-view, the annotate module,
the repo-icon components and the Chromium SameSite electron spec: all green.
* All 66 removed "shape" identifiers grepped repo-wide across every file type;
none survive.
* node config/scripts/generate-rpc-params-catalog.mjs --check exits 0.
* node --check on every changed .mjs; oxfmt clean on all changed files.
* `pnpm run check:code-quality:changed` reports 0 findings.
Not machine-verified: the 3 mobile/ files (its Vitest run cannot resolve
`expo/tsconfig.base.json` in this worktree), and the WSL- and Playwright-gated
specs. All are rename- or comment-only hunks, read in full.
|
||
|
Neil
|
bfdec26352 |
fix(lint): enable anti-slop/no-object-parameters (#20781)
The rule rejects the broad `object` type on any function input (declarations, expressions, arrows, methods, call/construct signatures, function types), plus local aliases and unions that resolve to `object`. `object` accepts every non-primitive while exposing no properties, so it documents nothing and pushes callers into assertions at the boundary. Fixes all 185 violations across src, config, tests and mobile, and flips the rule from "off" to "error" in config/oxlint-anti-slop.json. Approach: replace each `object` input with the type its owner already has. Most sites took an existing domain type or a type-only import (36 added); 40 new aliases name shapes that had none. Where a value is genuinely only compared by reference, it gets a named identity token instead of a shape -- `Record<string, never>`, the built-in `WeakKey`, or a `unique symbol` brand, matching the branding already used in src/shared. Same treatment for WeakMap and Map key parameters. Two `as unknown as` casts became unnecessary once the parameter carried a real type and were removed; no new casts were added. Suppressions added: none. No `oxlint-disable` for this rule anywhere, and no max-lines disable or per-file bump. Three files sat exactly at their max-lines cap, so the added type imports were made line-neutral rather than suppressed: - src/main/ipc/browser.ts exports the existing guest-registration args type (renamed BrowserGuestArgs) so browser.test.ts reuses it on one line. - pane-scroll.ts takes TerminalScrollIntentTarget through the existing pane-manager-types import via a type-only re-export. - direct-rpc-client.ts drops the identity parameter entirely: the session check moved into the sendProbe callback that owns the token. Verified: anti-slop config reports zero violations over src config tests mobile; run-typecheck-projects-in-parallel exits 0; 144 affected test files pass (1749 tests); oxlint and oxfmt clean on all changed files. Mobile has no runnable test/typecheck target in this worktree (expo is not installed), so its 6 files were typechecked against a standalone config and diffed against the base branch -- error sets are byte-identical, including test files. |
||
|
Neil
|
e4a9d24e0c |
fix(automations): repair cron step expansion and day restriction (#20202)
The semantic half of the cron repair. Both defects change what an already-saved schedule does, so they ship together and behind a decision. #15723: parseCronField set end = start for a bare numeric field even with a slash step, so 5/15 expanded to [5] and fired hourly instead of every fifteen minutes. N/step is the open-ended N-max/step sequence now. #15896: day restriction came from expanded set cardinality, so 1-31 read as unrestricted and */2 as restricted. Restriction is lexical now: a day field restricts iff no term of it ranges over a star, matching vixie cron and robfig/cron rather than crontab(5)'s prose. Verified differentially against robfig/cron v1.2.0 across 22 expressions, 424 days, zero divergences. The two cannot ship apart: 0 9 1/1 * 1 matches 124 days under the old parser, 104 under #15723 alone, and 730 under both, because the old cardinality flags react to the corrected expansion. describeAutomationScheduleDrift reads a saved expression under both semantics and reports the ones that moved, so neither direction is silent; the service names them once at startup. No expression Orca's own presets generate drifts. Fixes #15723 Fixes #15896 |
||
|
Neil
|
f107499e44 |
fix(lint): enable anti-slop/no-reflect-get (#20786)
`anti-slop/no-reflect-get` rejects every call to `Reflect.get`. The
reflective read bypasses ordinary property access and throws away the
type evidence the compiler would otherwise give you: the result is
`any`/`unknown` with no narrowing, so a typo in the key or a shape drift
in the source object is invisible until runtime. The rule's remedy is to
parse dynamic input into a named domain type (or narrow it with `in`)
and then read the field normally.
Baseline: 86 violations across 67 files. Now zero unsuppressed
violations under
`npx oxlint --config config/oxlint-anti-slop.json --ignore-pattern 'config/oxlint-plugins/anti-slop/**' src config tests mobile`.
Fix pattern
-----------
44 of the 86 were rewritten. The dominant shape was an `unknown` value
read through `Reflect.get` right after a `typeof === 'object'` guard;
those became `in`-narrowed property access, which TypeScript checks:
- Reflect.get(value, 'agents')
+ 'agents' in value ? value.agents : null
Two further shapes:
- `Reflect.get(Object(x), 'k')` on a possibly-primitive envelope became a
small named reader that boxes once and indexes a
`Record<string, unknown>` (`settingsField` in
mobile/src/transport/settings-read-operations.ts).
- Tests reaching into private state moved to TypeScript's checked
bracket-index escape hatch (`runtime['layoutQueues']`), or to a
documented read-only accessor on the owning class
(`SearchSubprocessLineAccumulator.retainedCapacityBytes()`,
`CodexSubagentExecutions.retentionSizes()`).
No type assertion was added anywhere: the diff contains zero net-new
`as` casts, `as any`, `as unknown as`, `@ts-ignore`, or
`@ts-expect-error`, so nothing was laundered into the sibling
assertion rules.
Suppressions
------------
42x `// oxlint-disable-next-line anti-slop/no-reflect-get` across 38
files. Every one is the default-forward branch of a `Proxy` `get` trap:
get(target, property, receiver) {
...
return Reflect.get(target, property, receiver)
}
`Reflect.get(target, property, receiver)` is the only construct that
forwards with correct `receiver` semantics; `target[property]` invokes
an accessor with the wrong `this` and silently breaks getters that read
sibling state. There is no typed alternative, so these are suppressed
rather than rewritten.
3x `// oxlint-disable-next-line typescript-eslint/consistent-type-definitions
-- declaration merging requires interface` in
tests/e2e/github-url-smart-input-transition.spec.ts,
tests/e2e/linear-url-workspace-entry.spec.ts, and
tests/e2e/worktree-active-delete-scroll-position.spec.ts. Replacing
`Reflect.get(window, 'x')` with typed `window.x` requires a
`declare global { interface Window }` block, and `interface` is
mandatory for declaration merging. Matches the existing convention at
tests/e2e/helpers/runtime-types.ts:63.
1x `// eslint-disable-next-line no-var -- main-process gate handle for
this spec` in tests/e2e/project-group-creation-visibility.spec.ts, for
the same reason a `var` global is needed to type the handle. Matches
tests/e2e/agent-session-log-tail-stability.spec.ts:24.
Also updates two source-text anchors in mobile's rpc-recording mutation
harness (mobile/src/test-support/rpc-recording/operation-mutations.ts
and recording-runner.test.ts), which pin the exact text of the rewritten
line in settings-read-operations.ts and would otherwise fail with
"Mutant anchor matched 0 sites, expected 1".
|
||
|
Neil
|
f7b2736d6d |
fix(worktree): block removal when the archive hook fails (#20153)
* fix(worktree): block removal when the archive hook fails A repo's orca.yaml archive hook is the user's last chance to save work off a checkout Orca is about to delete. A failed hook was logged as advisory and stepped over, so the removal went ahead with nothing archived — and the caller could still be told it succeeded. The hook is now a blocking precondition, evaluated while the checkout, its Git registration, its agents and Orca's ownership evidence are all still intact: it sits ahead of the registration re-read, the lock/dirty preflights, stopPtys() and removeWorktree in every orchestrator that runs it. Failure is typed (worktree_archive_hook_failed) and carries the worktree path, outcome, exit code where one was observed, and the hook's output. unverifiable stays distinct from exited, so loss of contact is never read as a pass. The waiver rides its own field at every layer and is never implied by --force, which already carries the PTY-stop waiver; when used, the waived failure comes back on result.archiveHookOverride rather than being swallowed. worktree.archive-failure-blocking.v1 is advertised so an integration can tell "accepts --run-hooks" from "safely propagates a failing hook" without risking the data loss to find out. The runtime's SSH path cannot run a hook at all, so rather than delete with the archive step silently skipped it refuses — waivable like every other refusal here. #18563 retires that gate by making the path run the hook for real. Stacked on #20559, which makes a timed-out hook report honestly; without it a hook that traps SIGTERM and exits 0 would defeat this gate. Fixes #19334 * fix(worktree): close the skip-confirm dead end and the client/hook timeout gap Four review findings on the gate. A retry from the failure toast could fail for a DIFFERENT reason than the one the user had just answered, and that second failure got a bare toast with no buttons. With skipDeleteWorktreeConfirm set, the delete helpers pass no force, so waiving a failed archive hook on a dirty checkout landed on the dirty preflight and stopped there. Retry failures now re-enter the same failure toast, so every retry stays as actionable as the first attempt. Third instance of this class. The renderer gave worktree.rm a 60s budget while an archive hook may run for 120s. A hook that took 90s and succeeded timed the client out and reported failure while the host went on to delete — telling the user their delete failed and their checkout was gone. The budget is now derived from the hook's, and only when a hook can run. The SSH fail-open is logged rather than silent, and the capability's doc comment scopes what it claims: a hook that RUNS and fails cannot delete the checkout; it is not a promise the hook was found. The SSH owner-resolution test now reads a real remote orca.yaml through a stubbed provider and asserts the returned script is the remote one. It previously stopped at the lookup key, which is the coverage that let this path break twice. It fails against the row-only resolution. * fix(worktree): name a signalled hook exit, and state why prunable cleanup skips the gate Two things the rebase onto #20617 and #20576 surfaced, both found by rerunning the real-repo harness rather than by reading the diff. - #20617 added a registration-cleanup branch that returns before the archive gate. That ordering is correct — both of its arms describe a row with no checkout behind it, so there is nothing to archive and running the hook would fail on the missing cwd — but the gate's ordering invariant is documented, so the exception should be too. - A signalled hook reported `Command failed with exit code null.`, which reads as a reporting glitch rather than the `unverifiable` verdict it is about to produce. It now says the command was terminated without reporting an exit code. Introduced by #20576; the withheld `exitCode` itself was always right. Fixes #19334 |
||
|
Neil
|
49e5fa597a |
refactor(lint): enable anti-slop/no-reflect-apply (#20782)
`anti-slop/no-reflect-apply` rejects `Reflect.apply(fn, thisArg, argsArray)`.
It defeats the call-signature checks TypeScript applies to an ordinary call:
the args array is checked as an array, not positionally against the callee's
parameters, so arity and type errors pass silently. Dynamic dispatch belongs
behind a named interface, not behind a reflective call.
Flipped the rule from "off" to "error" and cleared all 17 baseline violations
across `src config tests mobile` (16 sites; one file had two).
Fix pattern: `Reflect.apply(fn, recv, args)` becomes `fn.call(recv, ...args)`,
or a direct method call when the implicit receiver is already the right object.
The receiver is preserved at every site.
Where the callee is a captured built-in whose overloads split on an argument's
shape (`String.prototype.split`, `JSON.stringify`), a call-signature capture no
longer compiles once the args are passed positionally. Those three sites capture
the function through a method-shaped type
(`{ split(separator: unknown, limit?: number): string[] }['split']`), which keeps
the forwarding call checked rather than asserted.
Behaviour notes:
- `diff-section-layout.test.ts` drops a `limit === undefined ? [sep] : [sep, limit]`
conditional. Equivalent: `String.prototype.split` maps an undefined limit to
2^32-1, and the `Symbol.split` path forwards undefined either way.
- `workspace-space-compaction.test.ts` forwards `reduce`'s two arguments unchanged,
so the `arguments.length >= 2` initial-value branch is unaffected.
- `agent-session-history-byte-accounting.test.ts` is the one site where the receiver
is not literally preserved (`JSON` -> undefined). `JSON.stringify` never reads
`this` per spec, and restoring `.call(JSON, ...)` would reintroduce the overload
failure under strictBindCallApply.
No suppression comments added — the rule has zero `oxlint-disable` sites.
`Reflect.apply` still appears at electron.vite.config.ts:159, inside a template
literal of generated bootstrap source. That is string content, not lintable code.
|
||
|
Jinjing
|
b8554f1c59 |
fix(composer): clarify failed attachment drops (#20704)
* refactor(renderer): give the IPC error reader a clamped and an unclamped shape * fix(composer): name the attachments a drop could not add, in one toast * fix(composer, source-control): use one stable failure toast slot - Replace per-worktree toast IDs with single slot that replaces on each failure - Remove destructive retry actions; discard must confirm in dialog - Consolidate filesystem import types to shared location - Add compactIpcErrorMessage for string error handling * refactor: centralize filesystem import types and clarify failure naming Move import result types from main/ipc to shared layer so they're available across preload and renderer. Rename uniformFailure → commonFailure and skippedOrFailed → failureCount for clarity. Simplify preload/API type definitions by reusing shared types directly instead of duplicating inlined union shapes. * Reuse single toast slot for composer drop failures Multiple drop failures now replace the previous toast instead of stacking, preventing notification clutter. Uses a dedicated toast ID separate from Source Control's stage/discard notifications. |
||
|
Brennan BensonandMerge Sim
|
f55b7ba680 |
fix(native-chat): cancel pending prompts precisely (#20601)
* fix(native-chat): hide activity while awaiting input * fix(native-chat): keep approval turns cancellable * test(native-chat): satisfy split PR quality gate * fix(native-chat): catalog approval cancellation label * fix(native-chat): include approval cancellation runtime label * fix(codex): settle prompts when cancelled turns complete * fix(codex): settle prompt registry fallbacks * test(native-chat): cover pending interaction fallbacks * test(native-chat): split prompt state coverage * test(native-chat): keep prompt state isolated * fix(native-chat): bound prompt turn backfill * refactor(codex): centralize prompt registry bounds * fix(native-chat): cancel pending prompts precisely * fix(native-chat): consolidate capability imports * fix(native-chat): harden precise prompt cancellation * fix claude cancellation teardown races * retry claude prompt lifecycle admission * bound claude prompt cancellation retry work * fix(codex): bound prompt turn identity on registration * fix(native-chat): route rejected late dispatch settlements * fix(codex): retain exact cancellable prompt turn ids --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
dd85e5fc81 |
fix: keep OMP terminals when folder workspaces become Git repos (#20653)
Preserve the original folder locator through Git upgrade and subsequent listing, persistence, and removal decisions after proving it still names the same checkout. Independently reviewed with 60 focused persistence/listing/removal tests and six native Windows real-Git/NTFS cases covering case/slashes, junction retention and retargeting, remote-host isolation and unrelated checkout preservation. Prior source-connected native OMP proof confirms process survival. Full PR CI passed; no rebuilt full-app after-proof claimed. |
||
|
Neil
|
bac96b212e |
fix(hooks): actually terminate a timed-out hook's process tree (#20576)
Repairs #20559, whose termination was a no-op: `detached` is a spawn-only option and `exec` ignored it, so the shell never became a group leader. Verified against real processes. Refs #19334 |
||
|
Brennan Benson
|
4a027626e9 |
fix(agent-session): honour the backup-recovery fence floor on surface release (#20708)
* Fix surface release fence recovery floor * fix(agents): advance backup recovery floor past lost mint |
||
  mmarabelandNeil
|
68f0b2e835 |
feat(runtime): stream file uploads instead of buffering whole files (#16106)
* feat(runtime): stream file uploads instead of buffering whole files Staging read each dropped file whole with readFile(), base64-encoded it (a 4/3 expansion), and passed the string through IPC to the renderer, which re-chunked it. Peak memory was ~2.3x the file size before a byte moved, so a 25 MB per-file cap existed to protect the heap. Staging now records identity only. The byte pump moves into main, where the file handle and the runtime socket both live: 384 KiB slices (512 KiB once base64-encoded, matching the chunk size the renderer used) appended through the existing files.writeBase64Chunk RPC. Peak memory is one slice regardless of file size, so the ceilings become user-safety limits on an unattended transfer — 2 GB per file, 8 GB per drop — and over-limit errors name both the size and the limit. Because staging and streaming are separate calls, the staged entry carries size, inode, device and mtime, and the streamer re-checks all four against the pre-open lstat and against the handle it actually reads. A source replaced or rewritten at the same size between the two calls is refused rather than uploaded under the original name. The post-read check compares mtime as well as size, so an in-place rewrite mid-transfer aborts before commitUpload renames anything into place. O_NOFOLLOW, realpath containment and stat identity are preserved, and the pairing revision plus the runtime id ride every chunk, so a re-pair or a replacement runtime aborts instead of appending the rest of the file to a different host. No wire change: files.writeBase64Chunk and its params are untouched, so old and new hosts behave identically. The SSH import path is separate and unchanged. The web client has no local filesystem to stream from and says so instead of failing obscurely. * fix(runtime): close the empty-upload and per-drop budget holes Two gaps the first pass left open. A zero-byte source returned before the post-transfer identity check, so a file that gained content during the empty write's round trip committed as an empty file at the user's chosen name. The empty chunk now falls through to the same final check the slice loop uses. Each staged source also started its own byte counter, so the 8 GB ceiling capped one source rather than the drop: five 2 GB files staged cleanly at 10 GB total. The IPC handler now carries one budget across sourcePaths and adds only what each source actually staged. The per-file ceiling is still re-enforced where the bytes move; the drop total holds at staging because identity enforcement means each file streams exactly the bytes measured. * docs(runtime): name the invariants the upload helpers carry * fix(runtime): name the source in errors and stop uploads with their window Three problems an independent review turned up. A dropped file's relative path is '', so the over-limit error read "'' is 3 GB, over the 2 GB per-file remote import limit" — the message this change exists to fix, naming nothing. Errors now fall back to the file's own name; the staged entry keeps '' so the destination path is unaffected. The streamer had the same shape, falling back to the hidden .orca-upload-<nonce> temp destination, a path the user never chose. The byte loop used to live in the renderer and died with it. Moving it into main meant closing or reloading the window left the rest of a multi-GB transfer running, with the renderer's temp cleanup never reaching its finally. An AbortSignal now rides the caller's lifetime and every chunk, is re-checked per slice, and main sweeps the abandoned temp path itself when the renderer is no longer there to do it. Upload failures also reached the import result wrapped in Electron's "Error invoking remote method '...'" prefix, because the throw crossed IPC instead of happening in-renderer; extractIpcErrorMessage unwraps it. An existing staging test asserted the empty-name message, so it encoded the bug rather than catching it; it now asserts the file name. * test(runtime): cover the containment check and the per-chunk host guards The "escapes the dropped root" test only reached the lstat symlink guard, so assertEntryInsideRoot had no coverage at all. The shape that actually needs it is a regular file under a symlinked intermediate directory: lstat sees a plain file, and realpath containment is the only thing that refuses it. Disabling the guard now fails this test and nothing else. Nothing asserted that the SSH target, connection generation and execution host reach the writeBase64Chunk params either — the renderer tests stop at the IPC boundary, so the streamer's half of that contract was untested. * fix(runtime): survive a straggling append when sweeping an aborted upload Aborting rejects the in-flight chunk locally, but the host may still apply that append, and appends open with flag 'a' — which recreates the file the sweep just deleted. The delete and the straggler also race: they are separate calls on a queue that is not ordered between them. Slices are strictly sequential, so at most one append can be outstanding. A second pass after it has had time to land is therefore sufficient, not merely a heuristic. The sweep moves out of filesystem-mutations.ts into its own module so the behaviour is testable directly. Found by an independent review pass, which also pointed out that the "escapes the dropped root" test only reached the lstat symlink guard. * fix(runtime): abort uploads only when the document commits, and honour manual disconnect per chunk did-start-navigation fires before will-navigate blocks an external link or a stray file drop, and the renderer survives those (verified against Electron 43 with a hidden window). Aborting there killed a healthy upload with a misleading 'window went away' error. did-navigate fires only once a new document has replaced the caller. The renderer's per-chunk calls used to go through the IPC handler that refuses a manually disconnected environment; the loop in main made no such check, so a disconnect mid-upload kept pushing the rest of the file. The handler now resolves the selector to an environment id and the streamer checks it per slice. Adds slice-boundary coverage against the real chunk schema and host write flags, staging-to-stream on a real filesystem, and handler-level lifetime tests. --------- Co-authored-by: Neil <neil@stably.ai> |
||
 Neilandshahidbeig-a11y
|
3632311d0b |
fix(omp): preserve status after terminal title owner rewrite (#20610)
Validated and independently reviewed OMP integration fix. Co-authored-by: shahidbeig-a11y <258701601+shahidbeig-a11y@users.noreply.github.com> |
||
|
Brennan BensonandMerge Sim
|
955051ded0 |
fix(codex): settle a structured send on admission, and stop minting a colliding identity (#20138)
* fix(codex): settle a structured send on admission, and stop minting a colliding identity Two sends could be written into the journal under one durable identity. Codex coalesces a mid-turn `turn/start` into the running turn rather than refusing it -- measured against real `codex app-server` builds 0.147.0, 0.150.1 and 0.153.4, none of which refuse and none of which fire a second `turn/started`. The dispatch path read the turn id from the turn/start response and stamped every accepted send `ordinal: 0`. Since a coalesced send gets the running turn's id back, two submissions persisted the same `providerItemId`. That string is durable, and it is the key a restore uses to match a submission against provider history, so the second message's real history row matched nothing and rendered as an extra bubble on replay. On 0.147.0 it is worse than a collision: the coalesced response returns a turn id that never starts and never completes, so the persisted key named a turn absent from history and NEITHER message could match. Identity is now minted from the echoed user message at `identityFor` -- the single point that mints the journal row's own identity -- so the settled key is by construction the one replay computes, rather than a parallel calculation that can drift. Dispatch returns `admitted` when the transport write completes; identity settles on the echo through a channel that did not previously exist for Codex. Waiters are keyed by client message id instead of being shifted off the front of an array by arrival order, and they are cleared on session close and child exit -- previously a timeout was the only thing that ever ended one. `TURN_ID_WAIT_MS` is deleted. It was never reachable on any build measured: `readCodexTurnId` returns non-null on all three, so the 10s wait never fired. The comment justifying it claimed older builds acknowledge before the id exists, which no tested build does. Three comments asserting Codex answers a mid-turn send with `turn already running` are corrected. Their only backing was a test fixture inventing that error string. The correction is factual only -- every changed line in `src/main/runtime/orchestration/` is a comment, and mid-turn delivery is still refused for both providers. Whether that policy is right is a separate question; it was resting on a false premise. Known gap, stated rather than implied: this prevents new collisions and does not repair journals already written with a colliding or phantom key. Those conversations keep duplicating on restore. Repairing them means re-matching persisted submissions against provider history and rewriting `providerItemId` -- which is what `journal-submission-reconciler.ts` is written for, and it still has no production caller. * test(codex): drop the synchronous-accept contract and the colliding `:0` from the integration fakes Three tests in the structured-session integration suites encoded the dispatch contract this branch replaces, and two of them pinned the defect it fixes. They asserted `agentSession.send` answers `dispatchState: 'accepted'` carrying `providerItemId: codex:<thread>:<turn>:0` at send time. That ordinal was never observed; it was stamped on every accepted send, which is exactly the collision this branch removes -- a send coalesced into a running turn is answered with the running turn's id, so two submissions persisted one durable key. The visible failure was a 30s timeout rather than a failed assertion. The fake client advertised no `agent-session.pending-send-result.v1`, and without it the host holds the reply until the send settles: a shim for clients too old to render a pending bubble. The fake provider then echoed the user message with no `clientId`, so nothing could correlate that echo back to the submission, and the wait ran to its own 30s ceiling. Real Codex sends `clientId` on that echo, and the fake now does too, which is what makes it a model of the provider rather than a sketch of one. The identity assertion is kept rather than dropped. Each send now asserts `pending` with no identity at admission, then asserts the submission settles `accepted` at `codex:<thread>:<turn>:0` once the echo lands. Same ordinal, but earned from `identityFor` on the echo -- the key a replay recomputes -- instead of guessed from the turn/start response. Ablated: removing `clientId` from the two echoes leaves both submissions `pending` and fails both assertions, so the assertion is load-bearing and not satisfied by something incidental. Both suites' client fixtures now advertise the capability set the desktop renderer sends in `src/main/ipc/runtime.ts`, which is what these suites mean by a client. The older-client settlement wait keeps its own coverage in `src/main/runtime/rpc/methods/structured-agent-session.test.ts`. `structured-agent-session-runtime-exit.test.ts` asserts `pending` for the same reason; it drives the host directly, so it never took the compatibility path, and what proves delivery there is still the turn the reacquired provider starts. The replay suite's "without dispatching it twice" property is untouched: one `turn/start` call, one replayed ledger row. * fix(codex): preserve unsettled dispatch correlations * test(codex): type the dispatch fixtures instead of asserting over them main's new casting gate (#20367 base) flags type assertions on changed lines. Replace them with checked types: the recording sink already satisfies its interface, both CodexSession fixtures are now annotated and carry real collaborators, the settlement assertion compares whole identities, and the integration helper reads submissions through the host's public journalSnapshot instead of its private session map. * fix(test): merge the duplicate doubt-reasons import the merge left behind Both sides added an import from journal-dispatch-doubt-reasons and the merge kept both statements, which the whole-repo native plugin gate refuses under --deny-warnings. * test(codex): a Fast mode turn is admitted, not accepted #20506 landed its Fast mode tests against the dispatch contract this branch replaces: a Codex send now returns admitted and settles its identity on the provider echo. The tier assertions the test exists for are untouched. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
eba56f2f69 |
feat(ai-vault-search): construct the session search indexer in the scanner service behind a setting (#20516)
* feat(ai-vault-search): persist agent-session search consent and retention Two booleans and nothing else: `enabled` and `historyDays`, off by default because building the index reads every transcript on the machine. No `paused` -- the PR 3 indexer is immutable, so every change is close-and-construct. The settings IPC normalizes a write like every other field and hands the change to the index; there is no UI for it until PR 8. * feat(ai-vault-search): hold one indexer and engine pair per host The object that owns a host's live index and the three recipes that change it. The indexer is immutable, so a settings change is close-and-construct, disabling is close with no replacement, and clearing is close, remove the database, construct. The new instance's first sweep purges a narrowed window and admits a widened one, so neither needs a code path. The database sits beside the scanner's parse cache, one file per host. A runtime with no node:sqlite can hold no index at all, which the Node 18 floor on orcad and the relay makes a real case rather than a hypothetical one. * feat(ai-vault): let the scanner child own the session search index The transcript reader runs in that child, so the index consumer has to as well: one read serves both the session list and the index. Three request operations (search, status, reconcile) and one fire-and-forget settings message carry everything a parent needs; main never opens the database file. The init frame becomes a factory because it is read at every spawn, so a respawned child sees current consent rather than the first frame's. A child holding a running index is never idle from the parent's side, so idle retirement is suppressed while the index is on -- retiring it would stop the reconcile loop until some later scan happened to respawn one. Both files this lands in were already at the max-lines ceiling, so three collaborators move to where they belong rather than being disabled around: the invalidation deadline into the class that owns invalidations, call cancellation and the start requeue into the call-state module, and orcad's flag parsing into its own file. * feat(ai-vault-search): register a search service on every host that answers Without a registered service a host answers no-service, which means "this host does not have the feature" rather than "the index is off". All three hosts now answer the second thing. The desktop forwards to the scanner child. orcad and the SSH relay daemon have no such child -- orcad ships only the watcher and daemon entries, and the relay's AI Vault sidecar runs the remote scanner, which publishes nothing to the transcript channel -- so on those two the index lives in the process that would drive its reads, gated on a runtime that has node:sqlite at all. The relay registers with consent off and no way to turn it on: nothing carries a setting to a remote host yet. That is the honest state, and it is still worth registering, because it is what tells a client the difference between off and too old. * test(ai-vault-search): price a warm pass over five thousand transcripts The number the reconcile interval will be revisited against, measured rather than argued: a warm sweep stats every file under every root, a warm cycle stats the newest N per agent, and neither reads what the index already holds. It does not tune the interval. * fix(ai-vault-search): answer the casting gate without assertions main's new type-assertion rule reaches every file this branch touches. All nine sites drop the cast rather than carry a SAFETY: rationale: the operation guard narrows with `in`, the sqlite probe narrows the builtin it loads, the child test keeps the discriminated reply instead of widening it, and the settings resolver takes `unknown` -- which is what it really reads, since a persisted profile can hold a value no version of this code wrote. * fix(ai-vault-search): let a refreshed scan root reach the live index The parent re-resolves scan roots before every policy push, precisely so a WSL distro or extra Codex home that appeared since the child spawned enters the window. The child forwarded only the settings to a live instance and used the roots solely in its `??=` initializer, so those roots were dropped for the child's lifetime. The indexer stays immutable: a structurally different root set closes the pair and constructs a new one, the same way a changed databasePath already does. Compare via `sameSessionSearchRoots` rather than a plain JSON compare, because nothing fixes the key order two producers write; lists are sorted too, since the indexer walks every root and a re-enumeration that reorders is not a change. An unchanged set still never restarts a running index. The orcad and relay in-process hosts resolve roots once at install and never re-apply, so they have no such seam. * fix(ai-vault): restart the scanner child the index is holding Three review items. The hold keeps a child alive for the index, but only a queued call ever started one: `pump()` skipped a hold with an empty queue, so an idle indexing child that crashed, or an `ensureChild()` that failed at start, left indexing stopped until an unrelated request happened to arrive. `pump()` now starts the child the hold requires, which is also the restart callback the fault policy already schedules, so the existing delay and circuit bound the retry exactly as they bound a queued call's start. `updateSessionSearch` goes through the same seam instead of its own `ensureChild` call. A search registers no AbortController, so a cancel sent for a search id was added to the `cancelled` set and never consumed. Nothing can reach that today -- no caller passes a signal and the child answers in milliseconds -- so this is only a leak of ids: consume it when the search settles. The orcad argument doc claimed a `--`-prefixed value stays a flag. The parser takes the next token regardless, and orcad-launch-contract.test.ts pins that, so the doc is what was wrong. Behaviour is unchanged. * fix(ai-vault): recover search indexing and refresh scan roots * fix(ai-vault): defer search refresh policy reads * fix(session-search): stabilize paging and host enablement * fix(session-search): refresh host roots within full sweeps * docs(session-search): clarify initial root fallback |
||
|
Brennan Benson
|
a4c11f1889 |
fix(native-chat): stop a bounded tail read from moving the chat cursor past unapplied rows (#20581)
* fix(native-chat): stop a bounded tail read from moving the chat cursor past unapplied rows
A structured chat pane could latch "Working for N" forever after the agent had
finished, showing the send arrow rather than Stop, while the sidebar and
`worktree ps` correctly read idle.
The client replica has one position (`state.cursor`) and one body. Two
operations keep those consistent: replace (both from one host snapshot) and
append (rows contiguous with the cursor). The `tail-page` branch was a third
thing: it took the cursor from the journal head, the items from a bounded page
(200 items, byte-capped), then merged retained client submissions over the
page's. Under continuous journal writes the client is always slightly behind,
so the branch ran on every window focus and on every pane re-activation. When
more than a page of rows had landed since a send, that send's user item fell
off the page, its submission was not carried, the retained `pending` survived,
and the cursor jumped past the dispatch-acceptance row. Nothing re-sends it: a
batch carries only touched items and that submission is never touched again.
Delete the third operation rather than guard it. A live subscription is now the
only thing that moves the cursor, and `subscribe({ cursor })` already replays
exactly the missed rows.
- remove the window `focus` listener and the owner/transport `refresh` contract
- skip warm hydration: a retained owner subscribes at its applied cursor
- cold hydration keeps its history read, applied as the existing `snapshot`
(replace) event rather than `tail-page`
- delete the `tail-page` action and its reducer branch
- delete `resumeCursor` and `shouldAdvanceStructuredResumeCursor`; two cursors
with two advancement rules were how position and body drifted apart
`older-page`/`loadOlder`, the unattached-refusal grace, generation guards and
the coalescer are unchanged. No host, wire or schema change.
Also fixes a second cost of the same branch: focus during a busy turn discarded
paged-in older items, shrinking the transcript to one bounded page mid-turn.
* fix(native-chat): preserve unavailable mixed-version session fences
|
||
  manuaudioandClaude Opus 5
|
170dbdb874 |
fix(ai-vault): ignore non-absolute env overrides for agent scan roots (#13118)
Six scan roots took a directory from an environment variable and used it verbatim. A relative value is resolved by whichever Orca process reads it — main sits at `/` when Finder-launched, the terminal daemon chdirs itself to the user data dir, the AI Vault service inherits main's cwd — so one value names a different directory in each, and walkSessionFiles walks it with no depth cap, no entry cap and no time budget, about once a minute per the session-list cache TTL. The agent CLIs do accept a relative home (verified against real Grok 1.0.30: `GROK_HOME=myhome grok du` creates `<grok-cwd>/myhome`), but they resolve it against their own per-terminal cwd, which no Orca reader shares. Falling back to the default home is therefore not a lost configuration — it replaces an unbounded walk of an arbitrary tree with a bounded read of a known one, and matches what readGrokHomeEnvelope, skill-provider normalizedRoot and absoluteConfiguredDir already do with the same values. Add resolveAbsoluteDirOverride and apply it to CODEX_HOME, COPILOT_HOME, OPENCLAW_STATE_DIR, DEVIN_HOME, KIMI_CODE_HOME and GROK_HOME. It takes an explicit platform so the Windows shapes are provable from a POSIX CI box: `C:\...`, `C:/...` and UNC roots are kept, while the drive-relative `C:foo` and bare `C:` fall back. Tilde expansion stays out of it — Grok creates a literal `~` directory rather than expanding one — so absoluteConfiguredDir keeps its own Pi/Prime-specific expansion and delegates the absolute check. isAbsolute is syntactic only, so `/..` still collapses to `/`. That is fine for read-only discovery; these roots never gate renderer-supplied paths. Tests assert at the call sites, not just on the helper: the four session-scanner-agent-sources roots are module-level consts evaluated at import time, so they are exercised through AI_VAULT_AGENT_SOURCES with vi.stubEnv plus vi.resetModules. Reverting any one of the six call sites fails them (11-33 cases each). Closes #13082 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
 
|
2ed89b8781 |
fix(github): name an unfiltered empty project view instead of blaming a filter (#20588)
* fix(github): skip Projects search index for unfiltered views Empty query still used items(query:\$q), which routes through GitHub's Projects search index and can return totalCount 0 while the board is full during index lag. Omit the query argument when the view filter is empty. Fixes #12648. * docs(github): drop the false stable-shape claim for empty project filters Unfiltered item fetches omit items(query:) so boards skip search-index lag. The View.filter field is still '' when GitHub returns null. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(github): name an unfiltered empty project view instead of blaming a filter The search-index workaround in this branch was a no-op. Live introspection of ProjectV2.items shows `query` is declared `String = ""`, so omitting the argument and sending `$q = ""` coerce to the identical resolver input; GitHub applies declared defaults for omitted args (verified against its own endpoint). There is no non-search item field on ProjectV2 and ProjectV2View has no `items` at all, so no request shape can dodge the index. Revert the branching query construction and the module it added. What the user actually reported in #12648 is the copy: a view with no filter rendered "No items match this view's filter", which reads as data loss when a freshly populated board momentarily comes back empty. Word the empty state from the view's own filter — the filter message only when there is a filter, and an honest "no items yet" plus a transience hint when there is not — and share the one implementation between the table and roadmap surfaces. Refs #12648. --------- Co-authored-by: bbingz <zzb@gxsmjx.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Brennan Benson
|
4634d2c03b |
fix(native-chat): let the provider reopen a Claude turn it resumed itself (#20518)
* fix(native-chat): let the provider reopen a Claude turn it resumed itself A Claude turn could only be opened by Orca's own send echo, while any `result` frame closed it. The provider resumes work on its own — a background task reports in and wakes the agent after `result` settled the turn — and nothing Orca sent ever arrives to reopen one, so the session projected `idle` for the rest of the work. The model's own output is the evidence a turn is running, the way Codex's `turn/start` is, so it opens one; whichever opened it, the next `result` settles it. Subagent frames still open nothing: children outlive the turn that spawned them, and their work is their parent turn's, never a turn of its own. * fix(native-chat): bracket a resumed turn around the output that opened it A resumed turn was published after the frame's own rows, so its first tool call sat above the turn record. Every reader that scans back to the turn record and stops — the active-tool reader behind the sidebar's tool line, and the turn-window activity selector — looked straight past it, and the row showed working with no tool until a second call landed. The turn now opens before its frame is journaled. A send's turn keeps its existing order: the user echo is that turn's anchor and is written first. Prompt journaling moves to its own module, verbatim, to keep the translator clear of the line cap. * refactor(native-chat): declare the turn open at each content site The resumed-turn rule was a predicate that re-derived whether a frame had produced anything, duplicating work the frame handler had already done. The content sites know: each one now calls an idempotent ensureTurnOpen before it journals, and the guard against reopening a live turn lives in that one place. Behaviour is unchanged; claude-turn-opening.ts is left owning only the send echo, which is the one opener that anchors its turn to a user row. * fix(native-chat): gate both turn edges on root-ness The reopen path already refused nested output; the guard now reads before the already-running check so both edges state root-ness first. The close path had no nesting check at all, so a child's result would have ended the turn that spawned it. The two edges read parent_tool_use_id differently on purpose, and both fail towards not over-claiming: opening needs proof of root-ness, so an absent field opens nothing; closing needs proof of nesting, so an absent field still closes. No real Claude stream has been observed carrying a nested result — the session that prompted this work has none in any subagent stream — so the close-side guard is symmetry, not a demonstrated fix. * fix(native-chat): stop provider output reopening a turn nothing can close Self-audit found two paths the reopen rule opened where no event could ever settle the turn it created, leaving the row working for the life of the session. Both now suppress reopening until an accepted send lifts it: - a frame arriving after the session ended, when no event will settle anything - a turn the provider failed, or the user stopped, where the next thing the provider says is not a resumption Each has a one-lever ablation: removing the suppression read alone fails exactly those two tests, and both fail as working-instead-of-idle. * fix(native-chat): open a resumed turn from its first streamed delta Streamed deltas short-circuit before the frame handler, so a resumed turn whose first output is streamed text — the common case, since partial messages are a pinned launch contract — kept reading idle while its partial text was already journaled and visible. The streamed path now opens the turn too. Also from the same audit: - a nested result no longer swallows its own failure diagnostic; the turn gate now guards only settlement, and the provider-fallback row is written either way - root-ness treats an absent parent_tool_use_id as root, so a build that omits the field cannot silently stop opening turns - the suppression latch only ever sets on a failed result; a later clean result cannot lift it, and only an accepted send does The opener moves into claude-turn-opening.ts so both entry points share one root-then-suppression-then-idempotency order. * fix(native-chat): preserve resumed-turn lifecycle semantics |
||
|
Neil
|
55b3392018 |
fix(terminal): drop the agent gutter from copied selections (#19770) (#20545)
* fix(terminal): drop the agent gutter from copied selections (#19770) xterm selections are screen cells, not logical text. Agent CLIs paint their messages behind a fixed left gutter, so every copied line carried that gutter into the clipboard and pasted replies came out indented. Terminal clipboard writes now drop the run of spaces that *every* selected line shares, so relative indentation (nested bullets, fenced code, YAML) survives and only the gutter is lost. A selection that starts mid-line, or that includes any column-0 line, has a shared run of zero and is copied verbatim. Applied at every terminal clipboard seam: the Cmd/Ctrl+C shortcut, the pane context menu's Copy, right-click-to-copy, the app menu's Copy, copy-on-select, the X11 primary selection, the dashboard popout's preview terminal, and mobile's selection Copy button. New "Trim Gutter on Copy" terminal setting (default on) restores the old verbatim-cell behaviour. * fix(terminal): honour the gutter-trim setting on mobile copy Mobile stripped the gutter unconditionally, so turning "Trim Gutter on Copy" off left one surface still rewriting the clipboard. Mobile now mirrors the desktop preference through the existing settings.get RPC — a host predating the setting sends no key, which reads as on, matching the desktop default. Also folds the single-use gutter helpers into their callers so the shared module exposes one function. * refactor(terminal): parse each selection line once in the gutter rule Also locks the Windows subtlety with a test: a blank CRLF row is '\r', which reads as a zero-indent content row and would cancel the gutter unless the CR is split off first. * fix(terminal): publish the gutter-trim setting to paired clients settings.get is an explicit allowlist projection, not the whole settings object, so terminalCopyTrimsGutter never reached mobile: the client read the key as absent, which means "older host", which means on. Mobile therefore always trimmed and the desktop opt-out was inert. Adds the field to the projection and a test that fails if it is ever dropped again — absence is indistinguishable on the client from an old host, so a silent regression here has no other signal. * chore: drop unrelated formatter drift from this branch A repo-wide `pnpm format` swept a quote-style change in pnpm-workspace.yaml and a blank line in source-tree-walk.test.ts into this branch; neither is related to the gutter fix. * fix(terminal): trim the gutter on native copy events too xterm binds its own DOM `copy` listener that writes raw screen cells (CoreBrowserTerminal `_initGlobal`). Orca's own chords never reach it — they preventDefault in keydown — but Ctrl+Insert is a Chromium copy accelerator on Windows/Linux and is not in `terminal.copySelection`'s bindings, so it still copied the gutter. Orca binds Shift+Insert for paste on those platforms, which makes the asymmetry worse. A capture-phase listener on the xterm element now writes the trimmed text, closing the class rather than the one chord: any native copy event — assistive tech, execCommand — lands on the same path. Installed for both terminal panes and the dashboard popout's preview terminal. |
||
|
Brennan Benson
|
3cd60e76e9 |
feat(agent-status): run-identity types for keying rows by agent instead of pane (#20531)
* feat(agent-status): add run identity types * fix(agent-status): harden run identity codecs |
||
|
Brennan Benson
|
33149fcde5 | fix(claude): install SessionEnd for capable versions (#20530) | ||
|
Brennan Benson
|
c287a5d9b7 |
feat(native-chat): add provider-aware Fast mode (#20506)
* feat(native-chat): add provider-aware fast mode * chore: drop unrelated formatter churn from the merge pnpm format reflowed pnpm-workspace.yaml quoting and a source-scan test that this PR does not otherwise touch. * fix(native-chat): review fixes for provider-aware fast mode Review pass over the Fast mode work. Claude reads its model catalog once per option write. The admit check, the effort guard and the Fast guard each took their own `list_models`, so a model write with Fast on paid two round trips for one list and let two guards answer from two different catalogs. The guards are now pure over a single read. Claude no longer refuses a Fast enable when the catalog identified nothing at all. An empty list is not evidence against a model -- the same rule the model admit-check already applies -- so a CLI that cannot answer would otherwise have Fast refused on every model. A catalog that did list the model and stayed silent about Fast is still not positive evidence and keeps refusing. Codex refuses a direct `serviceTier` write instead of accepting one the next turn discards. The turn derives the tier from `fastMode`; the key still restores so a session persisted before Fast existed migrates. Both option surfaces return a cached snapshot again. `SessionOptionsSurface` is read through `useSyncExternalStore`, whose contract is a stable snapshot, and rebuilding it per call breaks that for any consumer wired that way. Also records two decisions that were emergent rather than stated: routing Standard when Fast is on but no tier is named yet, and what a readback disagreement does and does not prove. Quality gate: merges the duplicate imports static analysis flagged, adds SAFETY rationales for two pre-existing casts the changed-code gate now sees, and drops a new assertion in favour of a checked narrowing. * fix(native-chat): read Claude Fast state from the session frame A fresh Claude session reports `fastModeState` while the settings readback still has no `fastMode` boolean, so the two are not redundant -- the frame answers at a moment the boolean has none. The picker fell back to "value unknown" and asked the user to disambiguate what the provider had already reported, and the state it reported had no reader at all. Falls back to the frame only when neither a pick nor the settings readback answers. `cooldown` throttles routing rather than clearing the pick, so it reads as on; reading it as off would flip a control nobody touched. Display only. The launch seed is untouched: an unset Fast preference still seeds nothing, which its own guard continues to pin. * perf(native-chat): skip the model catalog read when turning Fast off Turning Fast off needs no support evidence, so the read only cost a round trip — and restore replays a stored `false` on every acquire. Also narrows the alias-matcher comment: the effort and admit guards match on alias and resolved id only, so calling it the sole matcher overstated it. * fix(native-chat): clear a Claude Fast block once the child stops reporting it The child omits fast_mode_disabled_reason entirely when nothing blocks Fast and never sends a null, so requiring the key back latched the first reason for the session's life: switching to a model that disallows Fast and back retired the control for good, leaving a session running Fast with no way to turn it off. A frame that reports state without a reason is the all-clear. * test(native-chat): cover the mobile structured option hook useMobileStructuredAgentOptions gained generation fencing, a pending-write guard and a post-write options refresh with no test file. Pins the concurrency contract and the fast mode round trip: - a superseded options read is dropped instead of overwriting newer state - an overlapping write is refused and the pending guard is released after - an accepted same-fence write reads options back and applies the result, and a different-fence write does not - a boolean fastMode pick reaches the wire encoded and is remembered decoded - no Fast row when session support, catalog support or the model capability is missing Each behaviour was ablated against the production logic to confirm it fails without it. No production code changed. * feat(native-chat): render a boolean session option as one toggle On and Off were two radio rows under a header repeating the option name, so a binary choice cost three lines and two clicks to read. It is now a single switch row that owns its label, on desktop and mobile. An unknown value keeps its caption: a switch cannot say "unset". * fix(native-chat): resolve a boolean option's display value at the producer A boolean session option reached the UI in three states while its control had only two, so the renderer apologised for the gap with a "Current value unknown" caption beside a switch that had already collapsed to off. For `thinking`, whose catalog default is on, that caption sat next to a switch asserting the opposite of what every composed dispatch assumes. One expression fed both the displayed value and the option's provenance. Split them: the boolean descriptor now always carries a value, resolved to the same `values[id] ?? defaultValue` that buildNativeChatSessionOptionCommand already composes, while `valueSource` is untouched and still records whether anything confirmed it. `kind.currentValue` is required on the boolean arm so the third state cannot come back. The launch path is unaffected: resolveAgentSessionOptionLaunch and buildNativeChatSessionOptionCommand build the composed `--model` argument from the caller's picks and the catalog, never from a descriptor. Both surfaces mark an unconfirmed value instead of captioning it, and the two reasons stay distinct — `default` says the catalog value is what a launch will send, `unreported` says nothing has told us anything. Only `unreported` is reachable in the structured lane, where the agent may be routing a tier we have never been told about, so the two never share a label. * fix(native-chat): let assistive tech read the option value marker The marker was aria-hidden next to an explicit aria-label, so the label already won the accessible name and hiding it only cost screen reader users the default-vs-unreported distinction that sighted users get. It is now referenced by aria-describedby, which keeps the name Fast mode. Mobile's summary row said "Not set" for a boolean while the sheet behind it showed the switch on, so the two screens disagreed. A boolean always has a value; the summary states it and the sheet's marker qualifies it. * chore(i18n): drop the On/Off option strings the switch row retired Replacing the On/Off radio pair removed the only call sites for these two keys. i18next cannot rebuild a key with no call-site default, so leaving them in the catalogs forced them into the boot bundle as dead weight. Removing them shrinks it by two entries instead. |
||
|
Brennan Benson
|
2ce252f471 |
fix(grok): announce a completion once, when Grok is actually finished (#20523)
* fix(grok): announce a completion once, when Grok is actually finished Orca pinged on every Grok turn-end. Grok runs turns the user never asked for: when a background task finishes it wakes itself, does a little work, and ends another turn. One request produced several pings. Grok already reports, on every turn-end, whether it still has work outstanding. Read that instead of trying to classify which turns are "real": backgroundTasks absent -> silent, this is the session-end tail StopFailure / StopCancelled -> announce, a failure is never hidden stopHookActive -> silent, a Stop hook is keeping it working a shell task or subagent running -> silent, the work is not done otherwise -> announce Nothing here knows what an auto-wake turn is. A turn that ends with work outstanding stays quiet; the later turn where that work is finally done is the one that announces. That is also why this survives the case where Grok completes a user's goal inside one of those turns — prefix-based suppression would have silenced it. Monitors and scheduled entries are deliberately not counted as outstanding work. They can run indefinitely, so counting them would suppress a user's completion permanently, and a lost ping is worse than an extra one. Also registers StopCancelled, which Grok fires instead of Stop on a user interrupt, a declined permission, --max-turns, or a no-progress bail-out. Orca never subscribed to it, so those turns were reported as successes. Also removes a stale notification matcher that searched for prose the shipping binary never sends; the typed notification kind is matched instead, and neither idle_prompt nor task_complete is treated as a completion. Needs-input behaviour (permission prompts and ask_user_question waits) is unchanged and stays ungated by background work. * fix(grok): never hide a failed or cancelled turn behind the background-work gate The announce predicate checked field-absence before terminal outcome. Grok's StopFailure and StopCancelled payloads carry no background inventory at all, so the absent-field branch — added so the session-end tail stays silent — fired first and silenced every failure and every cancellation. That inverted the rule it was meant to serve. Before this series a cancelled turn at least surfaced as a (wrong) success; gated this way it surfaced as nothing. Terminal outcome is now checked first, so a failure or cancellation announces regardless of what other fields the payload happens to carry. The existing tests passed straight through the bug because they built failure payloads with a backgroundTasks field Grok never sends for those events. They now model the real payload shapes, verified against the provider's payload definitions and the captured envelopes. * fix(grok): settle completion from provider lifecycle state * fix(grok): fence stale turn ends without prompt ids |
||
 
|
96b450fae8 |
fix(ssh): bound relay incumbent lsof probe (#18304)
* fix ssh relay incumbent probe timeout
* test ssh relay unconfirmed probe termination
* fix(ssh): preserve connect evidence while bounding lsof
* fix: preserve uncertainty when relay holder enumeration fails
* fix(ssh): supervise lsof helpers and preserve partial holder evidence
* test(ssh): prevent GC racing unconfirmed probe cleanup
* refactor(process): keep POSIX lsof supervision in process owner
* fix(ssh): confirm census cleanup and handle probe startup signals
* fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts
lsof warns to stderr about mounts it cannot stat, and any stderr byte forced
the holder enumeration to 'unavailable' — making the 'exited' verdict, and so
husk reaping, unreachable on those hosts. Pass -w to suppress the warnings.
* Revert "fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts"
This reverts commit
|
||
|
Jinwoo Hong
|
3ab2a1b91c |
refactor(orchestration): derive delivery eligibility from messages (#19837)
* fix(orchestration): retire read deliveries and clarify mailbox recovery * fix(orchestration): simplify delivery recovery and update nudge contracts * test: align orchestration check help expectation * refactor(orchestration): derive delivery eligibility from messages * fix(orchestration): validate live consumers and simplify batch revocation * refactor(orchestration): keep deliveries.status and derive eligibility without a column drop The outstanding_deliveries view now reads status = 'outstanding' plus unread membership, so v41 only drops uniqueness from idx_deliveries_one_outstanding and adds the view and trigger. Older binaries can still open the database. Removes the column-drop migration, the v40 test fixture and hasColumn guards, the fenced skew probe, and the unrelated nudge-text change. * docs(orchestration): drop delivery storage reference The compatibility caveat it existed to explain no longer applies; the view and index comments carry the remaining rationale. * docs: revert unrelated formatter churn * test(orchestration): verify historical database downgrade round trip |
||
 PPP-JHandm4air
|
16d1ab81d3 |
perf(skills): bound WSL installed skill discovery (#12314)
* perf(skills): bound WSL installed skill discovery * fix(skills): preserve bounded discovery correctness * fix(skills): bound WSL metadata prefilter reads * fix(skills): isolate absent discovery cwd cache keys * test(skills): adapt WSL discovery mocks to runner * fix(skills): preserve filtered discovery fallbacks * fix(skills): share filtered scans and preserve WSL inventory * fix(skills): share WSL scans without losing skill aliases * fix: preserve skill metadata and retire filtered peer caches --------- Co-authored-by: m4air <m4air@Mac.localdomain> |
||
|
Jinwoo Hong
|
d138c3278d | fix: show unexpected signout notice only once across versions (#20526) | ||
|
Brennan BensonandMerge Sim
|
5e70014da8 |
feat(native-chat): support file drag and drop (#20494)
* feat(native-chat): support workspace file drops * fix(native-chat): report OS file drops that attach nothing #15782 is a silent failure on the Finder route, and that route still swallowed every way it could fail: - the preload handler returned with no feedback when the OS handed us file items `webUtils.getPathForFile` could read no path from (promised or virtual files). It now sends the existing `rejected` payload with a new `unresolved-paths` reason, which the global drop toast names. - the composer's external-attach path dropped the batch with no notice when every path failed authorization, when an upload came back empty, and (new in this branch) when the owner changed mid-flight. Each exit now sets a notice; only a disabled composer stays quiet, because it has no notice surface. Also stops `resolveNativeChatAttachmentOwnerForWorktree` throwing out of a drop/IME handler when an SSH connection's generation is gone mid-attach — that is an unknown owner, which the resolver already models as `not-ready`. * refactor(native-chat): one owner-identity check for composer attachments The branch had two near-identical "is this still the same owner" helpers, one per attach route, and they disagreed: the workspace-drop copy ignored the SSH connection generation, so a reconnect between the drop and the IME flush read as the same owner and the path landed on a new connection. Collapses both onto one predicate in the pure ownership module (the store/toast-free seam both routes already depend on), which compares the full SSH expectation and never treats `not-ready` as a match. * perf(file-explorer): resolve drag ownership at dragstart, not per render The virtualized row list resolved the selection's source execution host on every render — the virtualizer re-renders on every scroll frame, so a large multi-selection paid a full projection scan plus a route allocation per selected path per frame, and per visible row on top of that. Only `onDragStart` ever read the result. Rows now receive a resolver they call with the paths they are about to drag. The three copies of the "stamp only if both halves resolve" guard (explorer row, both combined-diff row shapes) collapse into one helper next to the writer. * fix(native-chat): refuse a guarded composer drop visibly The drop handlers claimed the drag (preventDefault + stopPropagation) before checking `disabled`, so a guarded composer told the browser it accepted the drop, left the copy cursor up, and then did nothing — the same silent swallow this branch exists to remove. Dragover now answers `none` when the composer is guarded, so the cursor refuses and no drop event follows. It still claims the event either way: the composer sits inside the terminal surface, which accepts the same drag and would paste the paths into the shell instead. Drops `stopImmediatePropagation`. The capture-phase `stopPropagation` already keeps the event off the editor below, so the stronger form only risked suppressing unrelated listeners on the React root. The fake DataTransfer in the test now starts at a dropEffect we never write, so asserting `none` or `copy` proves the handler set it. * fix(native-chat): decide attachment ownership per path, not per batch A queued batch can mix sources — a workspace drop the target host owns and a client-local paste it cannot read — because IME composition holds both until it settles. Collapsing the batch to one verdict refused the whole thing on a remote target, including the drop the user was entitled to make. The verdict now follows the path it belongs to: owned paths attach, client-local ones are refused, and the refusal is reported rather than dropped. A stale owner still refuses everything, since that means the target moved under all of them. Also guards the empty-batch case, which previously read as "every path owned". * refactor(combined-diff): resolve drag ownership from the live workspace The combined diff captured an execution host into the open-file record at tab open and drilled it through three components to reach the row. That host was never persisted, so after a restart every drag from a restored diff was refused until the tab was reopened, and the capture failure was swallowed into an undefined source with no trace. Rows now resolve the owner the same way the source-control rows already do, from the workspace the diff belongs to at the moment of the drag. That deletes the prop drilling, the store capture and its bare catch, and leaves one way to answer "who owns these paths" for every live listing. The file explorer keeps its per-node owner: its tree is a cache that can still be showing a previous host's listing, which is exactly what that field records. * revert(file-explorer): drop the workspace-id tree reset Resetting and reloading the tree when the workspace id changes at an unchanged path is not needed for the drag source to be correct. The tree already records the workspace whose root listing it committed, so a cache left over from a previous workspace stamps that workspace and the composer refuses the drop — the intended answer, reached without touching the reset rule. That rule clears selection, the name filter and undo history, which is more file-explorer behaviour change than this feature asked for. * test(native-chat): stop the external-attach mock hiding new notices The hook's test replaced the whole attachment-owner module with a hand-written stub, so the two notices added alongside the owner-change guards resolved to undefined. Calling them threw inside the async attach loop — an unhandled rejection, which leaves every test in the file reported as passing while the run as a whole fails. CI caught it; a local run reporting only pass/fail counts does not. The mock now spreads the real module, so a notice added later cannot go missing from it, and both owner-change tests assert the string a user would read instead of only asserting that nothing attached. * test(native-chat): guard the last-path owner change on a one-file drop The owner flipping while the final path is authorizing has no next loop iteration to catch it, so the post-loop check is all that stands between a single-file drop and a path attached to a host that no longer owns it — and a one-file drop is the ordinary shape. No test covered that exit. Removing the post-loop check now turns this red; before it, only the multi-path exit was guarded. * fix(native-chat): keep a mixed attachment batch in attach order applyResolvedPaths partitioned a queued batch into a target-owned half and a client-local half and concatenated them. An IME-delayed batch that mixed a workspace drop with a paste made earlier in the same composition was therefore inserted owned-first, so the dropped reference jumped ahead of the pasted one in the draft. Filter against the two verdicts in place instead. Membership is unchanged, the order the user attached in survives, and the two intermediate arrays go away. * fix(file-explorer): name the owner of a dragged path whose row is hidden A multi-selection outlives the rows that showed it. Nothing prunes selectedPaths when a directory collapses, when the name filter narrows, or when dotfiles are hidden, and the drag still carries every selected path. Drag-source resolution read those owners from the row projection, which is built from visible rows only, so one hidden path collapsed the whole drag to an unstamped one and the composer refused it as coming from another workspace. The owner was never unknowable — the dir cache the projection is built from still records which host listed that path. Fall back to it when the path has no visible row. A path in neither (a name-filter synthetic node for a directory that was never listed) still fails closed. * fix(native-chat): ask which workspace the composer serves now The IME-flush ownership check compared the workspace id captured when the drop happened against the same captured value, so for a structured pane the comparison could only ever hold. The live protection came from the host and owner checks beside it; this one asked nothing. Read the id through a ref so the check means what it reads as. A pane whose structured target moves between the drop and the composition settling now refuses the queued path instead of attaching it. * fix(native-chat): ask which workspace an external attach lands on The post-await ownership gate resolved the owner through the render closure, so it re-asked the workspace the attach started in and compared the answer with itself. A tab moved to another workspace mid-authorization passed the gate, and the paths landed in a composer that no longer served that workspace. Read the pane through a ref and compare the workspace identity as well as the owner: two workspaces can both report a local owner, so the owner alone cannot tell them apart. * test(native-chat): read the real notice on a workspace drop The drop tests hand-built their attachment-upload mock and hand-copied the not-ready wording into it, so the assertion tracked the copy rather than the string a user reads: rewording the real notice left all 15 tests green. Spread the real module and override only the owner resolver, matching the two sibling test files in this directory. Rewording the notice now fails the test. * docs(native-chat): restore the hook's doc comment to the hook The workspace comparison landed between the doc block and the function it describes, leaving the comment attached to a type alias. * test(native-chat): cover the upload window for a moved pane The workspace-currency gate guards two windows and only the authorize loop was covered. The upload window is the longer one: the paths go to the worktree the attach captured, so a pane that moved workspaces meanwhile must not receive remote paths living under the workspace it left. * test(native-chat): pin the two untested attachment refusals Refusing an already-blocked target at the drop rather than queueing it had no test: queued paths that can never attach still spend the pending budget, and the next legitimate drop is then turned away for being one too many. Also pins the immediate already-false ownership verdict. Today's only caller settles ownership synchronously so it cannot arrive false, but the hook exports this entry point and the fallback is not a refusal — a false verdict is not "owned", so a remote target blames client-local attachments for an ownership failure. Verified: removing the branch reports the wrong notice. * docs(native-chat): say which rule the ownership refusal follows The per-path comment sat directly above the batch-wide ownership refusal while describing the blocked-target logic below it, so the refusal read as a contradiction of the line under it rather than as the file's stated rule. Name the rule at the refusal: a failed ownership verdict refuses the whole completion, the same way the pending-limit rejection does. --------- Co-authored-by: Merge Sim <sim@local> |
||
 
|
241fb9ed9d |
perf(terminal): batch file-link checks on their owning host (#20463)
* perf(terminal): batch file-link existence checks on their owning host * test(relay): allow additive filesystem capabilities * fix(web): keep terminal file links working under batched existence checks createShellApi omitted pathsExist, so withFallback answered the new batch call with a truthy proxy resolving to undefined and the whole hover batch rejected — dropping every link on lines with an out-of-worktree path. * test(web): assert the shim without type assertions --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local> Co-authored-by: Neil <neil@stably.ai> |
||
|
Jinwoo Hong
|
c853e10e0c |
fix(rpc): validate provider-specific fields in TaskProviderIdentity (#20284)
* fix(rpc): validate task provider identity fields Validate provider-specific field types while preserving nullable scopes and unknown identity fields. Record the producer census and pin validation with regression tests. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(rpc): reject a blank GitHub owner or repo normalizeTaskProviderIdentity treats a blank owner or repo as no identity at all, but the schema accepted '' and whitespace-only, so the two disagreed about the same payload. Refined rather than trimmed: trimming would rewrite the parsed value and change what the handler receives. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(rpc): re-measure the identity evidence counts The blank-field commit added seven tests, so the recorded 74/16/58 described the commit before it. Re-ran both: 81 tests, and the discriminant-only mutation now gives 17 failures / 64 passes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(rpc): correct the remaining stale gate count The blank-field commit moved the full-RPC total too; 2,463 was the count before it. Re-ran: 278 files, 2,470 passed, one skipped. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
22f56f7c2a |
fix(runtime): reject malformed file Base64 padding (#20283)
Require padded file-write payloads to end on a Base64 quartet boundary. Cover both RPC methods and padded final upload chunks, and document client compatibility evidence. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
1f7655f3e3 |
feat(ai-vault-search): public session search contract and transports (#20277)
* feat(ai-vault-search): define public contract and service seam * feat(ai-vault-search): add IPC runtime relay and web transports * fix(ai-vault-search): register search IPC at the core handler site ai-vault.ts was two lines over the 300-line max-lines limit; the search handlers belong with the other register*Handlers calls anyway. * fix(ai-vault-search): withhold degraded-root paths from relay status Status carried local filesystem paths over the relay while hits redact theirs. redactStatusForTransport applies the same policy at the same boundary: relay callers keep each root's reason and the array length as the count, so the type only makes root optional. * fix(ai-vault-search): close diagnostic path leak and remove test casts * feat(ai-vault-search): carry an execution host id and per-host outcomes on hits * feat(ai-vault-search): route desktop search by execution host scope, including runtimes * feat(preload): accept an execution host scope on session search * feat(web): answer only for the paired runtime on session search * docs(ai-vault-search): describe execution-host routing and the all-hosts merge * test(ai-vault-search): cover every host scope, the all-hosts merge and wire compat * fix(ai-vault-search): resume every host mid-page so a merged page never drops a hit * fix(ai-vault-search): decode the merged cursor with a schema instead of casts CI's type-aware audit refuses type assertions; a zod record validates the per-host entries and yields the typed map without one. * refactor(ai-vault-search): defer cross-host merged search |
||
|
Brennan BensonandMerge Sim
|
8999a00281 |
refactor(native-chat): give each structured dispatch state exactly one meaning (#20133)
* refactor(native-chat): give each structured dispatch state exactly one meaning
`unknown` meant five different things. Only one of them was genuine
ambiguity.
A transport write that the provider's input pump never took is provably
undelivered -- which is what `rejected` already means. It was recorded as
`unknown` anyway, and a one-entry allowlist then existed solely to teach
Retry that this particular `unknown` was safe to re-deliver.
Collapsing that case into `rejected` deletes the allowlist and turns a
predicate into an invariant: Retry never re-delivers an `unknown`, with no
exception to reason about. The four states now each assert one thing --
`pending` written and awaiting, `accepted` the provider has it, `rejected`
provably did not happen, `unknown` genuinely cannot tell.
A fail-closed guard is the right default here because the asymmetry is
severe: refusing a legitimate retry costs the user a retype, while allowing
an illegitimate one sends the model a second copy of their message.
Also fixed, found while auditing every reader of `rejected`:
- The renderer printed `submission.reason` verbatim, so a broken pipe put
the internal token `provider_write_failed: broken pipe` on screen in
destructive red. The journal reason is unchanged -- it is the durable
evidence and the transport-versus-content discriminator -- but the screen
now gets copy that names the cause and says the message is safe to
resend. Content rejections still show the provider's own words.
- The fallback copy "Message was not accepted" read as a content refusal.
A null reason now yields "Message was not sent.", which asserts only what
every rejection shares.
- A refused worker-start preamble threw a plain Error out of the dispatch
path. It now throws `OrchestrationError('dispatch_preamble_undelivered')`
so a coordinator can tell "we could not send it" from "we sent it and
something else broke" without parsing prose. Retain/discard behaviour is
unchanged; only the verdict's legibility improves.
Two behaviours improve as a consequence rather than by design: a provably
undelivered message no longer blocks conversation commands, and no longer
leaves the session reading as "working" in chat and in every session list.
Not addressed here, and named rather than implied: a message left `unknown`
by a dead child or a host restart still has no recourse but retyping. The
restart reconciler that would decide those on evidence is written and has
never had a production caller. Parking the refused entry instead would
reintroduce the head-of-queue wedge removed in #19863, so it is not an
option.
Note for whoever edits `journal-reducer.ts` next: it sits at 297 of its 300
counted lines. The next statement added there needs a split, not a shave.
* fix(native-chat): close two gaps review found in the rejection taxonomy
Both are narrow and both were real.
A journal written before a refused write became `rejected` still holds that
submission as `unknown` with the transport marker. The predicate this change
replaced excluded exactly that shape from provider-echo matching; the
state-only check that replaced it does not, so on replay such a row could
claim the echo of a later, genuinely delivered send of the same text and
attach the delivery to the wrong message. Fail-closed still prevented any
re-delivery, so nothing duplicated — but the wrong submission was credited.
Replay now excludes the legacy shape too.
And the content-versus-transport split had a third case neither side covers:
a local capacity refusal is neither the provider explaining itself nor a
frame that failed to leave. It fell through to the verbatim branch, so
`claude structured dispatch queue is full` reached the screen — the same
class of leak this change set out to fix, one reason short of being caught.
Internal reasons now get copy; only a provider's own words are shown as
written.
Each is pinned by a test that fails with its guard reverted and passes with
it restored.
* fix(native-chat): preserve dispatch refusal across clients
* fix(native-chat): rotate immediately rejected retries
* docs(native-chat): correct rejection taxonomy reference
* docs(native-chat): align mobile retry comment
* docs(native-chat): clarify unknown replay semantics
* fix(native-chat): keep a mobile send's operation id when delivery is unknown
Mobile released the retained operation id whenever a send came back
`unknown`, so the user's next send of the same text went out under a fresh
id. A fresh id has no ledger row, so the host treats it as a first delivery
and dispatches it -- even though `unknown` is the one answer that says the
provider may already have the message. That is the duplicate this branch
exists to remove, reintroduced on the client that has no outbox.
Which case that was matters. Mobile only ever sees `unknown` from ack-loss
(`isRpcDeliveryUnknown`: "the host may have processed it and only the ack
was lost"), because the mapper reported every `ok` result as `accepted`
without reading `dispatchState`. So the rotation fired exclusively where
delivery was ambiguous and never where it was provably refused, which is
the inverse of the rule this branch establishes.
Retaining the id is what makes a retry safe, and it costs no liveness:
`performSend` answers a second request under a recorded id from the journal
and never puts it back on the wire, so a reused id delivers when nothing
landed and replays when something did. Rotating can only ever add a second
copy. The retention stays bounded by the host's admission window, which
`retainStructuredSessionOperationId` already enforces.
`retryUnknown` goes with it: the host ignores it for delivery, and all it
does is skip the cached answer to re-read the same row.
Keeping the id exposes what the rotation was hiding, so fix that too: a
replayed `unknown` comes back `ok`, and mobile called it `accepted` and
cleared the composer as if the message had landed. `dispatchState` now
decides, in one pure function:
accepted/pending sent, and the id is spent
rejected provably did not happen and terminal in the reducer, so
reusing the id could only replay that rejection: spent,
and the next attempt is a first delivery under a new id
unknown keeps its id
Reading `dispatchState` at all is a pre-existing defect, fixed here because
the false "sent" cannot be removed without it, and scoped to the send path.
`mutate`'s rotation for prompt/option/cancel plans is untouched. The
rejection copy is the desktop's notice, so an internal reason
(`provider_write_failed: ...`) still never reaches a person.
Tests: the hook test that was flipped to assert a rotated id now pins the
opposite -- one id across an ack-loss and two `unknown` replays, each
reported `unknown` rather than `accepted`. The send fixture grew the durable
submission row a real host returns; without it every send test asserted
against a shape that cannot express the bug.
* fix(native-chat): enforce fail-closed structured send replay
* fix(native-chat): align retry and mobile RPC contracts
* fix(native-chat): keep transient admissions retryable
* test(tab-bar): expand nested create menu in harness
---------
Co-authored-by: Merge Sim <sim@local>
|
||
|
fe4237cd41 |
fix(agent-hooks): let the provider, not a keystroke, end these turns (#20149)
Escape is ambiguous at the source for Claude, OMP, Pi and Prime Agent: the same key closes an overlay and cancels a turn, and which one it meant is focus state only the TUI holds. Nothing downstream can recover it, so for these agents a plain Escape is never evidence a turn ended — the provider's own hook decides. Ctrl+C is untouched, and no other agent type changes. The renderer skips the round-trip and main re-checks the same rule, so a stale or direct inference request cannot route around it. A navigation Escape does not clear a Ctrl+C already waiting to settle: Escape is not a retraction. Fixes #13547 Fixes #9208 Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Co-authored-by: Rod Boev <rod.boev@gmail.com> |
||
|
e9065ddd16 |
fix(runtime): rank tui-idle evidence instead of inferring idle from silence (#20155)
`terminal wait --for tui-idle` returned satisfied in ~0s while an agent was mid-turn. The shared title detector defaults a name-only agent title to `idle` so the sidebar can clear a stale spinner, and the wait accepted that stored value as completion. Rank the evidence instead. An explicit idle marker in the agent's own title or a known ready prompt settles the wait; a fresh first-party OSC 9999 status saying working/blocked/waiting vetoes it; a name-only title is a last resort that settles only once the stream has also gone quiet. The rank is derived at read time from `lastOscTitle` rather than stamped onto the record, because `syncWindowGraph` rebuilds leaves from an explicit field list and would drop a bespoke provenance field on any renderer publish. Two things the ranking alone gets wrong are handled here too. A quiet non-shell foreground process no longer proves idle on a pane where Orca launched a known agent — that is an agent still booting, and resolving on it is what let `dispatch --inject` lose the prompt (#9976). And the idle poll re-reads the live leaf each tick, because a record captured at registration stops advancing and its frozen `lastOutputAt` makes the quiescence gate pass while the pane streams. The demotion is scoped to agents that go on to announce rest explicitly. Grok, Copilot, Aider, Mimo, agy and OpenCode emit their name and nothing more at rest: a real idle Grok pane repaints its banner about four times a second forever, so demanding quiescence from it left no settle signal at all and the wait ran to timeout. The design is Brennan Benson's, from #14642, which won a cross-review against #6012, #6555 and this branch's earlier approach; it is ported here only because that branch shares no git history with main and cannot be merged. Neil's #6555 first drew the explicit-vs-ambiguous line the ranking rests on, and Revofusion's #6012 first identified that a single title sample cannot prove completion. Fixes #6011 Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Revofusion <syed@moonai.org> |
||
|
Neil
|
8759b25e07 |
fix(automations): isolate the scheduler tick and refuse oversized cron steps (#20152)
Two defects that change nothing about when an existing schedule fires. #16303: evaluateDueRuns awaited each row with no catch, so one unreadable schedule skipped every later due automation in that tick. Each row is isolated now; a poison record writes one folded skipped_unavailable run explaining itself and the tick continues. A renderer send that throws is closed out as dispatch_failed rather than mislabelled as an unreadable schedule. #15895: step validation only checked integer >= 1, so a step wider than its field degraded silently to a single value and still passed validation. Oversized steps are refused at input time only, bounded by the count of distinct values a field holds, so day of week rejects */8 while */7 stays legal. Runtime parsing stays lenient so rows saved before the gate keep running the cadence they have. isValidAutomationSchedule now answers only 'acceptable as new input'; a new isRunnableAutomationSchedule answers 'can Orca still run this', and the editor uses it so a legacy row opens intact and can be renamed without re-authoring a schedule that is still firing. Verified: 34/34 corpus expressions fire identically to main. Fixes #16303 Fixes #15895 |
||
|
|
6c1d95b0da |
perf(tooling): reuse directory entry types in source scans (#20212)
* perf(tooling): reuse directory entry types in source scans
* fix(source-scan): stat DT_UNKNOWN dirents so untyped directories are still walked
`readdirSync(..., { withFileTypes: true })` can hand back a Dirent whose
type the filesystem did not report. For that entry every predicate is
false, so the readdir-type fast path treated a real directory as a file
and silently dropped its subtree from every ratchet guard. Fall back to
`statSync` whenever the entry is neither conclusively a file nor a
directory, keeping the no-stat fast path for ordinary entries.
Also make the two readdir-order assertions in the walk test
order-independent; `scanSourceTree` returns raw readdir order, which
differs on tmpfs.
* test(source-scan): unit-test the stat fallback via an extracted helper
The fabricated-Dirent readdir mock could not satisfy both gates at once:
vi.mocked(readdirSync) resolves to Node's Dirent<NonSharedBuffer> overload, so
the mock needed a type assertion, and #19462's casting gate rejects new ones on
changed lines. Removing the cast then failed tsc.
Extract directoryEntryNeedsStat and test it directly with a structural probe.
No mock, no cast, no top-level await, and the DT_UNKNOWN case is pinned:
removing the fallback fails 'stats an entry whose type readdir could not report'.
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: Neil <neil@stably.ai>
|