mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 16:02:38 +00:00
step4-session-2
11028
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo-H
|
f6833113e1 |
test(mobile): re-record the goldens at the new baseline pin
One re-record of all 641 goldens against pin |
||
|
Jinwoo-H
|
b57cbd3583 |
Merge origin/main into step4-session-2
Brings in #20884 ( |
||
 Jinwoo Hong
|
2fccacadbe |
test(mobile): mount screens, declared device stores and declared OS state in the RPC recorder (#20884)
* test(mobile): mount screens, declared device stores and declared OS state in the RPC recorder Three recorder capabilities, each with a test, plus four holdout sites recorded against main's product code to prove them. No product source changes. - JSX compiles through the automatic runtime, which is what product sources use; the classic `React.createElement` emit threw `React is not defined` on the first render of every screen. - An unlisted package answers `__esModule` as undefined, so a default import loads and the refusal defers to the first real member read instead of killing the module at load. - The substitutes table gains the inert view packages a screen needs, split into `screen-native-substitutes.ts` behind the same rule: only what a screen reads is listed. - A scenario may declare `deviceStore` and `deviceState.notificationTray`. Reads resolve the declaration or null and never a write; writes are recorded as effects. Undeclared is unchanged. - `screenMount` mounts a component with a crash boundary, so a reply partition that takes a screen down is a recording rather than a suite failure. Every pre-existing golden re-records byte-identical except `recorderSha256`, recorded from a detached worktree at the pinned baseline. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): declare the crash boundary's state type instead of asserting it The changed-code casting gate counts `null as string | null` as a type assertion. Re-records every golden from the pinned baseline, because the edit moves `recorderSha256`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refuse an unlisted native package by the member a mount reads Both emitted interop helpers short-circuit on `__esModule`, so answering `true` hands the refusing trap back unwrapped to `__importDefault` and `__importStar`. All three import forms now load the importer and throw the named refusal at the first member read, instead of a namespace import silently yielding `undefined` and failing later at the call. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): run a task deferred past interactions instead of dropping it An inert `runAfterInteractions` swallows whatever send the screen deferred, and the recording then claims the screen sends nothing. It runs the task on a microtask and returns the RN-shaped handle, so a cancel before the task runs still prevents it. `Alert.alert` stays inert. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record every golden for the two recorder engine changes Only `recorderSha256` moves, from c58067de to a59b30e7. Recorded from a detached worktree at the pinned baseline |
||
|
Jinwoo-H
|
39d83bb356 |
refactor(mobile): finish the refusal-interpretation helper across the session hooks
Twelve try/catch blocks across the session hooks now call the generic interpretOrThrowRefusalMessage instead of rethrowing refusedRpcMessageOrFallback by hand. Each one throws the same message on the same inputs, and the request stays outside the catch, so a transport rejection keeps its delivery-unknown identity. Two frozen parity hashes move for that reason alone: - HEAD_CALLBACK_BODY_SHA256, for the one converted block that sits in a useCallback (use-mobile-session-diff-comments.ts) - HEAD_NESTED_FUNCTION_SHA256, for the three that sit in plain nested functions (use-mobile-session-content-create-actions.ts) Every other parity hash and every count is unchanged: hooks 269, callbacks 77, effects 24, nested functions 12, plus the callback identity, effect, main-hook, hook-binding, content-hook, native registration/removal and timer hashes. Copies that stay longhand, by design: - the action-level try blocks that wrap the request as well as the interpret and surface the failure to the UI, in the diff-review comment, git and send hooks - the two catches that call setActionError or setError and return instead of throwing, in use-mobile-diff-review-interactions.ts and use-quick-commands.ts - the call sites in files this branch does not touch (pr-ai-triage-launch.ts, mobile-diff-review-loaders.ts, github-pr-rpc.ts, github-pr-mutation-outcome.ts) No golden moves: this touches no golden, adapter or recorder engine file. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
681d9a3592 |
refactor(mobile): answer round 2 on the session migration
The tapped-path resolve is now a re-export of the preview screen's `terminalArtifactPathResolve` rather than a second definition: same method, same skip, same whole-payload read, so the only thing the old comment could claim was a difference that was not there. `fileOwnershipRuntimeStatusRead` already re-exports for this reason. `worktree.show` has four readers, not three. The one the notes read is closest to is `fileOwnershipWorktreeRead`, which reads the same member whole, and acceptance is all that separates them: a file mutation throws rather than write to the wrong host, a session screen without its notes shows none and keeps working. `interpretOrThrowRefusalMessage` is generic, so a caller keeps the interpretation's own type, and eight longhand copies of its try/catch across five files now call it. Three more copies wait on a frozen source hash; see the report. Three operations sit in the module matching their direction: the quick-command save with the writes, the review-notes read and the markdown read with the reads. The quick-command reader is shared across that line, which is what keeps the save from adopting `[]` on a payload the parser rejects. The native-chat readability probe imports `MobileRuntimeRepoSummary` instead of redeclaring it, and a stray mutant comment that described the race entry is gone. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
005616171d |
test(mobile): record the real worktree catalog snapshot result in the RPC recorder (#20873)
* test(rpc-recording): record the real worktree catalog snapshot result `worktree.catalog-snapshot`'s action returned `WorktreeCatalogSnapshotClient.fetch`'s raw result. That value nests the live `RpcClient` under `pending.client`, so `captureValue` threw `Unsupported observation: function`: both goldens baked an `unhandled-rejection` effect and left the action's settlement `pending`, proving nothing about what the fetch returns. Project the result through `projectObservable` on the settlement path, the same way `state()` already shows it and the same way #20667 fixed `transport.pairing-race`. Rejections still propagate unchanged, so the two transport-rejection matrix partitions keep their recorded errors. The matrix golden now discriminates all eleven reply partitions: a full admission, six invalid ones, three `request_failed` codes and two rejections. `recorderSha256` does not move; `adapterSha256` moves on the six goldens mounted through this adapter module, four of which have no other change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(rpc-recording): fail the suite when a golden records a recorder failure A projection refusal settles as data, not as a failed run: `captureValue` throws inside the action, the recorder captures it as an `unhandled-rejection` effect, and `--record` writes a green golden whose action never settled. The class has landed twice — `transport.pairing-race` in #20667 and `worktree.catalog-snapshot` in the previous commit — and reverting either one plus a re-record would go green with the broken golden back. Read every golden and fail on an `unhandled-rejection` effect or on either of `captureValue`'s refusal texts in any observation field. A positive control in the same case asserts both detectors fire, so the absence claim is load-bearing rather than vacuous. Not a recording driver, so `recorderSha256` excludes it and no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(rpc-recording): ban only the recorder's own refusals, not product bugs The gate also failed on any effect named `unhandled-rejection`, which bans a real observation: detached-rejection capture exists to pin a main bug in a golden, and `unhandled-recording.test.ts` pins the capture precisely because no golden carries one today. Banning the name would make recording a genuine product failure a test failure. Drop that detector. `refusalText` alone catches all seven checkpoints of the worktree-catalog regression, because the refusal is the message of the captured error rather than the effect's name. The positive control now seeds that shape — the refusal inside a recorded error under `effects` — so the surviving detector is still proven to fire. Vacuity guards on golden and checkpoint counts are unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(rpc-recording): re-record the worktree catalog goldens over main's engine digest #20874 re-digested `recorderSha256` on all 509 goldens, so the six goldens mounted through the worktree catalog adapter had to be recorded again from the pin rather than merged. Bodies and `adapterSha256` are unchanged from the pre-merge recording. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
 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 |
||
|
Jinwoo Hong
|
783d8feabb |
fix(lint): merge duplicate type imports in the mobile RPC recorder adapters (#20895)
* fix(lint): merge duplicate type imports in the mobile RPC recorder adapters The native code-quality audit rejects a module imported twice in one file, so main's static-analysis job is red for every open PR. * test(mobile): re-record RPC goldens against the merged adapters The duplicate-import fix changed two mount adapters, so the nine goldens that pin them by adapterSha256 needed re-recording. The recorder fence requires the pinned baseline to match the product tree, so the baseline moves to current main, which rewrites that header in all 509 goldens. Every recording body is identical, which also shows the commits between the two baselines changed no observed behavior. |
||
|
Jinwoo-H
|
8b5a49d827 |
docs(mobile): state what actually gates the native chat paging read
The `nativeChat.readSession` send sits in the paging callback, not in the mount effect. What blocks recording it is that the mount effect's `nativeChat.subscribe` is what arms the offset and generation the callback pages against, and the request-only runner refuses to open a subscription. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
441fa57f98 |
refactor(mobile): answer round 1 on the session migration
Reuses `RpcOperationSender` for the file-tap client instead of respelling it as `Parameters<typeof fileTapPathResolve.request>[0]` at two sites; the type is the same by construction, and the handlers file no longer imports an operation only to name its first parameter. Names why `worktree.set-review-notes` stays separate from source-control's identical `worktree.set-review-link`, and why the new-tab loader's two preflight reads do not share the task drawer's readers on the same methods. Corrects the `callAgentSession` holdout count: five call sites across two hooks plus one inside the module's own mutation wrapper, not seven callers across five files. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Brennan Benson
|
e7206f62a8 |
fix(mobile): retire a structured operation id the host has refused (#20868)
`agentSession.cancel` kept its client operation id whenever the outcome came back unknown. One of those unknowns is not transport doubt: when the host answers `agent_session_operation_unknown` it has decided about that id and will not run it again, because cancel's mutation plan recovers no unknown ledger row. Every later Stop on that turn re-sent the same refused id, so Stop stayed unusable until the row expired. The RPC layer collapsed both cases into a bare `unknown`, discarding the difference between "the effect is in doubt" and "the host answered about this id". It now reports the second case, and cancel spends the id there while still replaying under genuine transport doubt. `agentSession.conversationCommand` deliberately keeps its id: its plan sets `recoverUnknownFromDurableState`, so a reused id can still replay or rerun. |
||
|
Brennan Benson
|
22ca862f76 |
test(native-chat): widen real-timer waitFor budget in agent-session-wire handoff tests (#20880)
vi.waitFor defaults to a 1000ms/50ms real-clock budget on this suite (no useFakeTimers), which is occasionally too tight for host.requestHandoff / handoffStatus to settle under a loaded CI shard. Production behaviour is unchanged; the assertions are correct, just sometimes slow to observe. vi.waitFor's own poll loop always runs on the real clock (vitest resolves its interval/timeout via getSafeTimers, which bypasses vi's faked globals), so the lease-renewer test carries the same real-wall-clock exposure despite calling vi.useFakeTimers() for the simulated renewal interval. 5000ms follows existing repo precedent for explicit vi.waitFor timeouts on real-timer waits (e.g. ssh-relay-session-rejected-delivery.test.ts, daemon/client.test.ts, pty-subprocess-io-failure-native.test.ts, windows-msys-job.win32.test.ts), which range 1500-15000ms. |
||
|
Brennan Benson
|
6c03bb6e82 |
fix(lint): replace Reflect.get with typed property access in mounting substitutes (#20874)
* fix(lint): replace Reflect.get with typed property access in native mounting substitutes main's tip fails `pnpm run audit:anti-slop` (the `static analysis` CI gate) on `no-reflect-get` in mobile/src/test-support/rpc-recording/native-mounting-substitutes.ts, blocking every open PR. The Proxy get trap's key is `string | symbol`; branch on that to keep typed bracket access for strings and a symbol-indexed cast for symbols, preserving the existing throw-on-unsubstituted-member behavior exactly. * test(rpc-recording): re-record goldens for the recorderSha256 shift native-mounting-substitutes.ts changed bytes, so recorderSha256 (which pins every non-adapter file under this directory into every golden's header) moved. Re-recorded all 509 goldens; only recorderSha256 differs in any of them, confirming the checkpoint content is unchanged. |
||
|
Jinwoo-H
|
8dcfac7253 |
fix(mobile): drop three assertions the changed-code casting gate rejects
`pnpm run check:code-quality:changed` reports `consistent-type-assertions` separately from oxlint's own pass, and three sites had no rationale: the resume preparation's payload read, which gets the standard SAFETY line, and two adapter refs whose `null as string | null` is just an annotation written the wrong way round. The adapter is pinned by `adapterSha256`, so its thirteen goldens are re-recorded from the baseline — header-only, no observation moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
eedb153202 |
refactor(mobile): keep the new session readers inside the operation cast fence
`rpc-operation-cast-fence.test.ts` refuses a type assertion anywhere in the region reachable from `rpc-operation-contract.ts`, and four of the new operation modules asserted their own result type. Each of those reads goes back to `rpcUncheckedPayloadReader`/`rpcUncheckedMemberReader`, and the shape the call site expects is named at the call site, which is where every migrated domain already puts it. `extractPaths` and `readQuickCommands` return to their hooks for the same reason — both were only movable by carrying a cast with them. No golden moves: the readers hand back the same values, and all 705 recordings still compare clean. The two frozen source-parity suites over the session route family are refreshed for the migrated text: three hashes, one runtime-string count, and the inventory hook's acceptance check, which now reads `!isCurrent() || !response.accepted`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
085a5e0433 |
test(mobile): matrix every reply the new session families script
Thirty-four reply-matrix goldens, one per scripted reply across the twenty-two families the session migration added, each running the eleven partitions in `reply-matrix.ts`. Recorded from the same pinned baseline as the pilot goldens through the detached worktree, so they freeze main's answer to a result-less success, a null result, an inner refusal envelope, a message-less outer refusal and a transport drop — not the migrated code's. They are laid down after the refactor because they are what found its three remaining divergences, each fixed in the previous commit rather than recorded around: the clipboard upload's destructuring message, and two mutation anchors the rewrite left matching zero sites. Two families matrix only their first request. The clipboard upload chain puts the start reply's `uploadId` into every later call's params, so a partition that answers the start differently moves a downstream assertion instead of a recorded observation; their base scenarios stop there, and the upload family's fallback arm carries its second site. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
43c8df5f26 |
fix(mobile): keep main's malformed-reply message and re-anchor two mutants
Three findings the reply matrices surfaced that the pilot scenarios could not.
The clipboard upload's `uploadId` read goes back to the raw result. A success carrying no result
throws a destructuring TypeError there, and V8 puts the destructured expression's source text in
the message — which the composer shows. Reading the slot off the interpreted payload rewrote that
sentence for every user who hits a malformed reply, on three families' `result-absent` and
`result-null` partitions. The cast is the one main made, kept for the message alone.
`race` and `new-tab-refusal-order` both anchored in text the migration rewrote, so each matched
zero sites. Re-anchored at their new homes; the defect each injects is unchanged, and
`probe-hole-witness.test.ts` still shows the probe killing the reorder while every pre-probe
scenario survives it.
`native-chat-send-delivery-unknown` is added as this domain's own mutant: dropping the
delivery-unknown arm of a chat send makes an ack lost after the frame was written read as a
definite rejection, which invites the user to send the same message twice. A second candidate —
swapping `terminal.list` from skip to throw — survives every golden and is not registered: the
inventory hook wraps its whole read in `catch { return false }`, so a refusal and a throw leave
the same strip.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
|
||
|
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-H
|
c4cd7c253c |
refactor(mobile): send the rest of the session domain through typed RpcOperations
Thirty files and 58 references leave the raw request port. Every send now names a declared operation with a fixed method, one acceptance policy and one reader, and no call site casts a reply payload: the AI Vault resume pair, the clipboard image upload with both its attachment surfaces, the native-chat terminal writes, session tab activation and reconciliation, the terminal-path tap, the structured agent launch, and the session screen's reads, tab creates/closes, review actions, notes, markdown save, quick commands and file search. Five new operation modules, plus two readers added to existing ones. Three methods get a second reader, each argued where it is declared: `files.resolveTerminalPath` (the tap branches on five members the grant refresh hands back whole), `files.open` (the tap's miss is silent, the Changes screen raises the host's message) and `session.tabs.list` (the send sheet keeps only terminal tabs, which the reveal poller and the reconciliation controller both drop). `git.stage` carries two acceptances for the same reason `repo.list` does: a tapped file raises its refusal, a bulk sweep counts it. No new acceptance policy. The AI Vault resume launch and the review send sheet now share `mobile-review-terminal-operations.ts` with the PR triage launch instead of re-deriving the same create/send pair, and the file-search hook's `extractPaths` moves into the reader it belongs to. One latent behaviour is preserved rather than fixed and wants a ticket: `mobile-session-tab-activation.ts` decides whether to replay an activation with `error instanceof LogicalClientCutoverError`, not with the message-matching `isLogicalClientCutoverError` that exists because "instanceof can miss across bundle copies". Under a second copy of the module the retry silently does not happen, which the recorder reproduces. No golden scripts a cutover for this family, so none records the wrong behaviour. The offender floor in the port ratchet comes down from 50 to 20: it is an anti-vacuity guard on a list this migration is driving to zero, and 39 files still reach the port. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
87c63b2905 |
style(mobile): format the session mount adapter and the recording manifest
`pnpm exec oxfmt --check .` from `mobile/` flagged both after the previous commit's type fixes. The adapter is pinned by `adapterSha256`, so the eight goldens recorded through it are re-recorded from the same baseline; the manifest is not pinned by its bytes — `scenarioSha256` canonicalises the parsed scenarios — so no golden moves for it. Header-only either way: no observation changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
5d0c85fca5 |
test(mobile): type the new session mount fixtures against the product model
The ten adapter modules added with the session recordings typed several fixtures structurally — loose maps, a local `Terminal`/`Tab` shape, `unknown` for the review screen state — which `pnpm --dir mobile typecheck` rejects: `tsconfig` covers `src/test-support`, so an adapter is checked like product code even though no test file is. Each one now names the product type it stands in for (`MarkdownDocState`, `TerminalRecord`, `MobileSessionTab`, `ReviewScreenState`, `MobileDiffReviewQueueItem`, `DiffComment`) and supplies its members through `mountFixture`. Only one of those changed a recording: a `DiffComment` requires `side`, so the review actions now put it on the wire and the two `review-mark-reviewed-*` goldens carry it. That is the fixture becoming a real subset of the type it claimed, not a behaviour change — no product source moves in this commit, and the parity claim the next one makes is against these bytes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo-H
|
ded9186468 |
test(mobile): record the session domain's remaining call sites before migrating them
Freezes main's behaviour for what is left of `mobile/src/session/`: the AI Vault resume pair, the clipboard image upload and its two attachment surfaces, the native-chat terminal writes, session tab activation and reconciliation, the terminal-path tap, the structured agent launch, and the session screen's own reads, tab creates/closes, review actions, notes, markdown save and quick commands. 87 scenarios over 22 new families, recorded from the pinned baseline `c6a72169843ececf3a21da370ac50c5c5a4e6462` through a detached worktree, before any product edit. Ten new modules under `adapters/`, so each domain's goldens are pinned by `adapterSha256` and no existing family re-digests. `native-mounting-substitutes.ts` gains `expo-haptics` (inert: every caller is already fire-and-forget), `expo-clipboard` (a per-recording pasteboard cell, because these screens read back what they wrote) and `BackHandler`/`Keyboard`. That is an engine file, so all 509 pre-existing goldens move on `recorderSha256` and on nothing else. Two families cannot be matrixed at their first request: the clipboard upload chain puts the start reply's `uploadId` into the params of every later call, so a partition that answers the start differently changes a downstream assertion rather than a recorded observation. Their base scenarios stop at that first reply instead, and the fallback arm carries the upload family's second site. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
  OrcaWinandm4air
|
22857cd8a0 |
fix(crash-reporting): stop periodic emitters from evicting the crash trail (#20639)
* fix(crash-reporting): stop a once-a-minute sampler from evicting the crash trail The breadcrumb ring is 30 entries and evicts oldest-first, so any emitter that repeats outlasts the whole lifecycle trail. Across 293 field reports three periodic emitters hold 77% of every slot ever shipped and 39% of reports arrive with no lifecycle crumb at all — the "Recent activity" section cannot say what the app was doing. Charge the overflow to the most crowded name instead of the oldest event, so a series is thinned from its oldest end and singletons survive. No allowlist, so a new periodic emitter cannot reopen the hole. * test(crash-reporting): pin coalesced-burst accounting under mid-ring eviction * fix(crash-reporting): scope eviction per origin and spare live coalescing owners Round-1 review found two ways the name-only policy was worse than plain FIFO: - Counting ignored `origin` while the snapshot filters by it, so a busy popout's samples made the main window's singleton look redundant and deleted it. - Names like `renderer_error` carry many independent coalesce keys, so the name became "crowded" out of genuinely distinct errors — and the entry taken was the oldest, i.e. a key still accumulating `suppressedSinceLast`. A crash report is the last snapshot, so an orphaned owner is never re-claimed and the burst count simply vanished. Group by (name, origin), skip an entry a coalesce key still owns unless every candidate is owned, and never consider the crumb that just arrived — its coalesce state is linked after the push, so it would always look unowned. * fix(crash-reporting): trim the report window by the same policy as eviction Round 2 found the fix defeating itself. Fair-share eviction parks one-off crumbs at the ring's HEAD and the repeating series at its tail — and the snapshot then took a plain tail slice of `MAX_BREADCRUMBS - retained.length`, trimming exactly what eviction had just protected. Measured on the previous commit: one retained `renderer_memory_highwater` cost one lifecycle crumb, and three erased the lifecycle trail from the report entirely. That lane fills under the same memory pressure that produces the `renderer_memory` flood, so the two cancelled out precisely when the trail matters most. Trim with `evictionIndex` instead, and route `isCoalescedCrumbStillInEvidence` through the same window — a predicate that disagrees with the snapshot would drop an owner's handle and lose the burst count from the crumb the reader sees. Also strengthens the uncoalesced-burst test, whose only remaining delta against its coalesced twin was the slot count: it now asserts the pane population is absent on the uncoalesced side, which is the signal coalescing exists to keep. --------- Co-authored-by: m4air <m4air@Mac.localdomain> |
||
 Brennan BensonandMerge Sim
|
2eb93206c8 |
refactor(agent-launch): make the launch-mode decision surface-neutral (#19848)
* 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.
* 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.
* 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.
---------
Co-authored-by: Merge Sim <sim@local>
|
||
|
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 |
||
|
Jinwoo Hong
|
d130347993 |
refactor(mobile): send the dictation, terminal, notification and browser domains through typed RpcOperations (#20702)
* refactor(mobile): pin each RPC golden to its own mount adapter, not every domain's `recorderSha256` covered the whole recorder directory, mount adapters included, so a domain PR that adds its adapter module moved the header of all 153 goldens. #20568 did exactly that and its merge with main conflicted on that one line in 153 files; every future domain PR would collide with every other in flight the same way. Split the directory at a real seam instead of a filename convention: `adapters/` holds one module per domain, registered in `adapters/mounted-operation-modules.ts`, and `recorderSha256` now covers the engine only. A new `adapterSha256` covers the source of the module that mounts each operation a golden's scenarios drive, read off the same `mounts` calls that build the table the recording runs against, so the pin cannot name a file the runner did not use. Adding a domain's module now re-digests nothing already recorded; editing one fails exactly the goldens mounted through it. `adapter-seam.test.ts` keeps the split from drifting: an engine file inside `adapters/`, an adapter defined in an engine file, a register entry naming the wrong file, and an adapter importing a sibling each fail. The five adapters that were inline in `pilot-mount-adapters.ts` move into their own modules, which leaves that file as the registry and nothing else. `GOLDEN_FORMAT_VERSION` goes to 5 for the new header field; the goldens re-record in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the split recorder/adapter digest Header-only. Every changed line is `recorderSha256` (the engine digest no longer covers `adapters/`), the new `adapterSha256`, or `goldenFormatVersion` 4 -> 5; `baseline` is unchanged and recording ran against the same pinned product tree. git diff -U0 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256|goldenFormatVersion)":' | wc -l 0 The seven `adapterSha256` values partition the 153 goldens by the module each was recorded through: 58 settings, 37 hosted review, 21 source control, 11 new-tab agents, 9 file inventory, 9 tasks, 8 workspace settings. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop pinning goldens to recorder inputs no recording can read The adapter split left three per-domain edits still moving all 153 headers: the mutant table, the per-family mutant registry beside it, and the probe-hole witness. None can change a recording -- the loader consults a mutant only when a mutant test asks for one, and no suite but the two recording drivers writes a golden -- so pinning them claimed a provenance the goldens do not have and charged every domain a full re-record for it. `mutants/` now holds the table, the registry, the reference states, the mutant suites and the probe-hole witness, and `recorderSha256` skips it. What makes that sound is that no recording can reach it: `operationModuleLoader` takes a resolved mutation spec instead of importing a table by name, so nothing on the recording path names `mutants/` at all. `mutants/mutant-seam.test.ts` checks exactly that, and fails if an engine file names the directory or anything outside imports from it. `recorderSha256` also pins only the suites in `recording-drivers.ts`, which `scripts/rpc-recording.mts` records from, so the two cannot drift. A suite that reads goldens, or writes one to a scratch directory, is no longer provenance for a recorded file. `OPERATION_EXPOSURES` went the other way, because it does change what a recording loads: withhold the resume-metadata exposure and exactly four goldens fail. Each domain module now declares its own exposures and gets its own loader, so `adapterSha256` pins the ones that reached each golden. Two assertions in the digest boundary test were vacuous: `join(root, '.')` normalises back to `root` and hit `recorderSha256`'s per-root cache, so the prose-is-ignored claim never recomputed anything. Each call now spells the root differently. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the mutant and driver exclusions Header-only, and no format bump: the header shape is unchanged. `recorderSha256` moves on all 153 because the engine set shrank, and `adapterSha256` moves on the 58 settings goldens because that module now carries its own exposure declaration. git diff -U0 HEAD~1 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256)":' | wc -l 0 Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): restore the preferences actions the merge resolution dropped #20568 added `resume` and `trust` actions to the `settings.task-preferences` adapter while it still lived in `pilot-mount-adapters.ts`. This branch had already moved that adapter into `adapters/task-mount-adapters.ts`, so resolving the `pilot-mount-adapters.ts` conflict in favour of the registry merge silently discarded them and `tw-task-preferences-resume-write` failed to record at all ("Missing or completed request: ui.set#1"). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens at main's tip after the merge All 208 goldens, header-only. `baseline` moves from |
||
|
Jinwoo Hong
|
2dfdbc8657 |
refactor(mobile): send the task provider, detail and board domains through typed RpcOperations (#20685)
* test(mobile): record main's task provider item, detail and board RPC behaviour 35 scenarios over 22 of the 25 files left in src/tasks/, recorded from main so the step-4 migration of the provider half has a frozen answer to compare against. Every one of the 70 references this branch will migrate reaches a recorded wire here, which is the check the workspace-creation half added after it lost three sites to fixtures that short-circuited before the call. Scenario params are observed, not written: a generator drove each adapter with nothing answered, read the projected sender calls back, and emitted the completion steps from them, so no `params` in the manifest is a guess about what the screen sends. Five adapter modules, split the way the screens are: one item's reads, the list and composer, the item mutations, the board's reads and the board's row mutations. `mountModelHook` holds the mount/dispatch/project boilerplate these twenty-two hooks share, so each adapter is only its fixture, its actions and its projection. Two fixture modules hold the task items and the project rows, shared so the same pull request looks the same to the comment hook, the merge hook and the checks hook — which is what makes their recordings comparable. `baseline` moves from |
||
|
Jinwoo Hong
|
98784820d8 |
refactor(mobile): send the transport pairing and status domain through typed RpcOperations (#20667)
* test(mobile): record the transport pairing and status domain against main Adds seven recording families for `mobile/src/transport/`, recorded from main's unmigrated product code before any refactor: the protocol-gate hook, the retrying capability probe, the pairing candidate race, credential rotation, direct-to-relay upgrade, startup pairing recovery and first pairing. The relay modules build their `defaultDependencies` at module scope, so merely referencing `Platform.OS` or a storage-backed loader threw before an adapter could override it. `native-mounting-substitutes.ts` separates reference from use: react, zod and @noble/hashes are the real libraries, expo-crypto routes through the Web Crypto the scheduler already pins, and the two secret stores throw when called. `baseline` repins to |
||
|
Jinwoo Hong
|
44268d9616 |
refactor(mobile): send the github.* PR surface and the diff-review loaders through typed RpcOperations (#20668)
* test(mobile): record main's github.* PR and diff-review loaders before migrating them
Scenarios and goldens for the step-4 `src/session/` first half, recorded
against main's unmigrated product code so the migration that follows has a
frozen parity oracle instead of an assertion.
- 20 scenarios over seven new families: the seven `github.*` PR reads, the
twelve PR mutations split by their three reply contracts (`{ok}` envelope,
bare boolean, slug-addressed comment edit), the triage createTerminal+send
launch, the PR branch-context chain and the review screen's three loaders.
- Two new sender-style mount adapters. Both mount exported async functions
taking a client, so no React host is needed and the recorded state is each
wrapper's own outcome.
- 50 new goldens: 20 pilot, 30 reply-matrix sites. `recorderSha256` moved on
all 153 existing goldens because the adapters are in the whole-recorder
digest; no other line in any of them changed.
Text diffs are deliberately unscripted: highlighting one reaches `lowlight`,
which the module loader refuses as an unspecified native dependency.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): send the github.* PR surface and the review loaders through RpcOperation
The step-4 first half for `src/session/`: eight files, 38 references to the raw
request port, all replaced with declared operations. No behaviour change — the
50 goldens recorded in the previous commit do not move, which is the claim.
- 21 operations over 21 methods. The seven PR reads keep their defensive
parsers as readers; the ten status-envelope mutations share one reader
because the `{ok, error}` convention is one host convention, not ten; the two
bare-boolean mutations read the payload unchecked because `=== true` is the
caller's confirmation rule.
- Four second readers, each justified in place: git.status and git.branchCompare
for the PR branch context (a refusal costs a fallback, not the screen),
git.branchCompare and git.branchDiff for review (the projection is not a
superset of the verbatim payload), and worktree.show for the review notes the
summary reader drops.
- Every failure text is preserved, including the two main kept apart: a refusal
with no message falls back to the screen's copy, a transport drop with no
message surfaces its empty message verbatim. `sendRaw`'s callers replaced
theirs a second time, so those fall back on both paths.
- No retry, and no operation reads a dropped reply as a failed mutation: the
rejection reaches each wrapper's catch as the original object.
- `github-pr-mutations.ts` split along the action/comment seam it already had
in its consumers, so no file needs a max-lines bump.
Inventory: src/session/ 47 files / 114 references -> 39 / 76.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): record the review snapshot answering its notes leg first
The barrier mutation census found one survivor: moving
`reviewWorktreeMetadataRead.interpret` inside the `Promise.all` in
`loadMobileDiffReviewSnapshot` changed nothing any golden observed. The base
scenario answers the branch-base legs before the notes leg, so by the time the
notes reply lands the compare leg has already sent `git.branchCompare` and the
two orders record the same sender list.
This scenario answers the notes leg first, while the compare leg is still
resolving its base ref, and checkpoints before the rest. At that checkpoint the
barrier is the whole difference: the correct order has nothing settled, the
early interpretation has already rejected the action. The mutation now fails it.
Recorded from a detached checkout of the previous commit, which carries main's
unmigrated product code with this branch's recorder over it, so the parity claim
stays non-circular. One new golden; no existing golden moved, because the family
base is unchanged and `scenarioSha256` is per golden.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): bind one git.status projection reader, not a copy per domain
The branch-context read declared its own `statusProjectionReader` with the same
parser, the same 'normalized-status' variant and the same empty salvage as
source-control's `gitStatusProjectionReader`, while its doc block claimed "one
reader serves both". Export the source-control reader and bind it here so the
claim is true; the doc now names the reader and keeps the part that is actually
different, which is what a refusal means on each policy.
No wire change and no golden moves: the reader is the same function value the
copy computed.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): undo the github-pr-mutations split, which max-lines no longer forces
The split was made when the migrated file measured 319 lines. It does not any
more: `sendRaw`, `sendGithubPrMutation` and `extractMutationError` moved to
github-pr-mutation-outcome.ts and the prRepo/headSha allow-lists to
github-pr-repo-slug.ts, so the merged file is 293 lines against the 300 limit
and oxlint is clean.
Nothing imported github-pr-comment-mutations directly — every consumer went
through the re-export hub in github-pr-mutations — so the seam bought a reader
one extra file to open and nothing else. Merge it back and drop the hub.
Product-only: same wrappers, same params, same settle shapes, no golden moves.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): one settleable-operation type for the PR reads and mutations
`GitHubPrMutationOperation` and the private `GitHubPrReadOperation` declared the
same two members for the same reason: a settle shape needs a bound operation's
method and its interpret, nothing else. Keep one, `GitHubPrSettleableOperation`,
and import it into the read settle. `extractMutationError` goes back to private,
as it was on main; it never had an importer outside its own file.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(mobile): drop the key-order claim from the PR param builder
The oracle does not observe param key order: `captureValue` in recording-values.ts
sorts keys, and no golden carries a raw frame string, so "the sender recordings
pin the bytes" was not a fact the evidence supports. The assertion stays for the
reason already in the doc — the builder is method-generic and returns a record.
`GitHubPrParamOptions` goes back to private; nothing outside the module names it.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): read the bare-boolean mutations with the shared unchecked reader
`mutationConfirmationReader` spelled out what `rpcUncheckedPayloadReader` already
returns, under the same 'pr-mutation-confirmation' variant that eleven other
operations in this tree get from the helper. Same function value, same variant,
so no golden moves. The comment explaining why the payload is left unread stays.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): one RpcOperationSender for both domains, not one alias each
`MobileSessionRpcSender` and `MobileSourceControlRpcSender` were the same type
with the same doc, each derived from whichever operation its domain happened to
own. Replace both with `RpcOperationSender` in transport, derived from
`settingsRead` there, and name it for what it is: what a bound operation needs
to send with.
Still derived rather than restated, so no module names the raw request port to
accept a client; the port inventory and its ratchet are untouched.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): point the moved PR and diff-review adapters at the seam and register them
The merge commit carried the two adapter files into adapters/ with their old
specifiers and left the register untouched, so this completes the move: the
relative imports climb one more level, and both modules are registered in
adapters/mounted-operation-modules.ts as identifiers imported from their own
source, which is what adapter-seam.test.ts checks.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): re-record the session goldens against #20662's adapter seam
The merge brought #20568's per-golden scenario digest and #20662's per-golden
adapter digest, so the 51 goldens this PR owns move on four header fields and
nothing else: baseline, goldenFormatVersion, recorderSha256, and the newly
added adapterSha256. No recorded byte outside those headers changed.
baseline stays at main's own pin
|
||
 github-actions[bot]
|
f742ab88d2 | Update README downloads badge | ||
|
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. |
||
|
Jinwoo Hong
|
36ef93a64f |
refactor(mobile): migrate the small domains onto RpcOperation (step 4) (#20705)
* refactor(mobile): pin each RPC golden to its own mount adapter, not every domain's `recorderSha256` covered the whole recorder directory, mount adapters included, so a domain PR that adds its adapter module moved the header of all 153 goldens. #20568 did exactly that and its merge with main conflicted on that one line in 153 files; every future domain PR would collide with every other in flight the same way. Split the directory at a real seam instead of a filename convention: `adapters/` holds one module per domain, registered in `adapters/mounted-operation-modules.ts`, and `recorderSha256` now covers the engine only. A new `adapterSha256` covers the source of the module that mounts each operation a golden's scenarios drive, read off the same `mounts` calls that build the table the recording runs against, so the pin cannot name a file the runner did not use. Adding a domain's module now re-digests nothing already recorded; editing one fails exactly the goldens mounted through it. `adapter-seam.test.ts` keeps the split from drifting: an engine file inside `adapters/`, an adapter defined in an engine file, a register entry naming the wrong file, and an adapter importing a sibling each fail. The five adapters that were inline in `pilot-mount-adapters.ts` move into their own modules, which leaves that file as the registry and nothing else. `GOLDEN_FORMAT_VERSION` goes to 5 for the new header field; the goldens re-record in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the split recorder/adapter digest Header-only. Every changed line is `recorderSha256` (the engine digest no longer covers `adapters/`), the new `adapterSha256`, or `goldenFormatVersion` 4 -> 5; `baseline` is unchanged and recording ran against the same pinned product tree. git diff -U0 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256|goldenFormatVersion)":' | wc -l 0 The seven `adapterSha256` values partition the 153 goldens by the module each was recorded through: 58 settings, 37 hosted review, 21 source control, 11 new-tab agents, 9 file inventory, 9 tasks, 8 workspace settings. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop pinning goldens to recorder inputs no recording can read The adapter split left three per-domain edits still moving all 153 headers: the mutant table, the per-family mutant registry beside it, and the probe-hole witness. None can change a recording -- the loader consults a mutant only when a mutant test asks for one, and no suite but the two recording drivers writes a golden -- so pinning them claimed a provenance the goldens do not have and charged every domain a full re-record for it. `mutants/` now holds the table, the registry, the reference states, the mutant suites and the probe-hole witness, and `recorderSha256` skips it. What makes that sound is that no recording can reach it: `operationModuleLoader` takes a resolved mutation spec instead of importing a table by name, so nothing on the recording path names `mutants/` at all. `mutants/mutant-seam.test.ts` checks exactly that, and fails if an engine file names the directory or anything outside imports from it. `recorderSha256` also pins only the suites in `recording-drivers.ts`, which `scripts/rpc-recording.mts` records from, so the two cannot drift. A suite that reads goldens, or writes one to a scratch directory, is no longer provenance for a recorded file. `OPERATION_EXPOSURES` went the other way, because it does change what a recording loads: withhold the resume-metadata exposure and exactly four goldens fail. Each domain module now declares its own exposures and gets its own loader, so `adapterSha256` pins the ones that reached each golden. Two assertions in the digest boundary test were vacuous: `join(root, '.')` normalises back to `root` and hit `recorderSha256`'s per-root cache, so the prose-is-ignored claim never recomputed anything. Each call now spells the root differently. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the mutant and driver exclusions Header-only, and no format bump: the header shape is unchanged. `recorderSha256` moves on all 153 because the engine set shrank, and `adapterSha256` moves on the 58 settings goldens because that module now carries its own exposure declaration. git diff -U0 HEAD~1 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256)":' | wc -l 0 Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): restore the preferences actions the merge resolution dropped #20568 added `resume` and `trust` actions to the `settings.task-preferences` adapter while it still lived in `pilot-mount-adapters.ts`. This branch had already moved that adapter into `adapters/task-mount-adapters.ts`, so resolving the `pilot-mount-adapters.ts` conflict in favour of the registry merge silently discarded them and `tw-task-preferences-resume-write` failed to record at all ("Missing or completed request: ui.set#1"). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens at main's tip after the merge All 208 goldens, header-only. `baseline` moves from |
||
 Neil
|
caa465d1da |
fix(automations): stop tick latency counting against the missed-run grace (#20819)
* fix(automations): stop tick latency counting against the missed-run grace The scheduler compared wall-clock lateness straight against the grace budget, but evaluation runs on a fixed 60s interval that is never aligned to an occurrence. With grace 0, any tick arriving after the scheduled instant -- in practice every tick -- recorded skipped_missed and told the user "Orca was unavailable during the missed-run grace window" while Orca had been up the whole time. A zero-grace automation effectively never ran. Grace is a downtime catch-up budget. An occurrence that came due while the scheduler was running was never missed; it is waiting for the next tick. The service now tracks continuous availability and only charges lateness to grace for occurrences that came due while it was stopped. Downtime behaviour is unchanged, and the new test asserts that half too. The missed-run branch moved to dispatch-refusal.ts, which already owns non-dispatch outcomes, keeping service.ts under max-lines without a disable. Fixes #11299 * fix(automations): use a tick-latency tolerance instead of process liveness Review caught two real defects in the first cut: - availableSince is process liveness, not continuous execution. A suspended process (system sleep) keeps its start time, so an occurrence that came due during a multi-hour sleep skipped the grace check entirely and replayed on wake -- exactly the downtime case grace exists for. - The restart edge: an occurrence due after the last tick but before stop() was reclassified as downtime and skipped with zero grace. Elapsed lateness cannot be faked by suspension and needs no restart bookkeeping, so the budget is now grace + two tick intervals. Both edges disappear rather than being special-cased. Also fixes a hollow test: workspaceId 'wt1' has no worktree separator, so the target refused and the run recorded skipped_unavailable -- a 'not skipped_missed' assertion passed without ever dispatching. Tests now use a valid id and assert 'dispatching' directly, and cover the sleep, tolerance boundary and restart cases. * fix(automations): scope to the verified tolerance and document the stall gap Review found three defects, all real: - The 'as never' cast failed the changed-code casting gate. AutomationRendererChannel is a Pick<> precisely so a test can pass the real shape; cast removed. - The restart test never restarted: evaluateAt advanced 60s internally, so the first pass already dispatched and the second was a no-op. It now evaluates exactly once and asserts no run exists before the second pass. - tickMs * 2 does not bound a pass that holds the re-entrancy guard across a slow serve-mode dispatch. I tried a busy-window fix for the third and could not test it honestly -- the case needs a genuinely slow in-pass dispatch, and both attempts passed with the fix disabled. Rather than ship logic I cannot prove, the tolerance stays at the verified shape and the gap is documented where the next reader will find it, with the reason 'time since last pass' is the wrong bound (a suspended process runs no passes either). Not a regression: on main that automation never ran at all. * fix(automations): name the check for what it does and correct its message Two review points, both fair: - missedDuringDowntime consulted nothing about availability once the liveness flag was removed; it is elapsed lateness against grace plus tolerance. Renamed missedBeyondGrace so callers read the real contract. - The run error still claimed 'Orca was unavailable' -- the same false statement #11299 was filed about, now reachable for a genuinely late run rather than a merely tick-delayed one. It states what was actually observed instead. Also documented the deliberate trade CodeRabbit raised: elapsed lateness cannot tell a short outage from a late tick, so a zero-grace run due during an outage shorter than the tolerance dispatches instead of skipping. The alternative got the far worse case wrong -- a multi-hour sleep replayed on wake. |
||
|
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
|
37394e9cb7 |
build(release): compile the Windows relay process-table addon (#20809)
* build(release): compile the Windows relay process-table addon #16598 added build-windows-process-tree-relay-addon.mjs and the ORCA_REQUIRE_RELAY_NATIVE_ADDONS gate, but wired both into dev-channel-win-build.yml only. release-cut.yml was never touched, and stageWindowsProcessTreeAddon merely logs when the addon is absent, so every stable release has shipped Windows relays without windows-process-tree.node. Confirmed by extracting the installers: v1.4.191 (the first stable carrying the feature), v1.4.198 and v1.4.203 all have no windows-process-tree.node in relay/win32-x64 or relay/win32-arm64. Those hosts have been taking the CIM fallback the whole time — 1247ms and a powershell.exe per scan against 57ms native, on #16598's own ~1490-process measurement host. Mirror the dev-channel steps. Same windows-2022 image, so the MSVC ARM64 cross toolset the arm64 leg needs is already proven there, and the addon build runs before the long packaging step so a missing component fails in seconds with MSB8020 naming it. * build(release): keep the Build app env rationale attached to its step The new addon step landed between the ORCA_POSTHOG_WRITE_KEY / BUILD_IDENTITY / DIAGNOSTICS_TOKEN_URL comment block and the Build app step it documents, orphaning it. Move the step above the block and record why it carries no run_attempt guard: Build app is ungated, so a guarded addon step would let a rerun reach the required-addon check with nothing staged. |
||
|
Neil
|
c0fb04c8d2 |
fix(relay): open the real null device when detaching Windows stdio (#20808)
* fix(relay): open the real null device when detaching Windows stdio
`openSync('NUL')` does not reach the null device on Windows. node's fs runs
the path through `toNamespacedPath`, which resolves it against cwd and
prefixes `\\?\` — and that prefix turns off DOS device-name mapping, so
CreateFileW creates a regular file named `NUL` in the relay's install dir
and pins fds 0/1 to it instead of to a discard sink.
Verified on a Windows 11 host: `fs.openSync('NUL', 'w')` + a 5-byte write
produced a 5-byte file named `NUL` in cwd. `\\.\NUL` is passed through
`toNamespacedPath` verbatim; the same write discards and a read answers
EOF, with no file created.
It also escaped into shipped artifacts. release-cut.yml runs the relay
watcher fault harness with cwd = out/relay/win32-x64, so every Windows
installer since v1.4.169 carries `resources/relay/win32-x64/NUL`, which
NSIS extracts as `_NUL`.
* test(relay): prove the `\\?\` rewrite on a drive-letter path
`toNamespacedPath('NUL')` off Windows only resolves against a POSIX cwd and
stops; with no drive letter it never reaches the branch that adds `\\?\`. So
the assertion held for the wrong reason and did not demonstrate the rewrite
the comment describes. Assert it on an absolute drive path, which takes the
same branch on every host.
|
||
|
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
|
37a5b278b3 |
test(package): reject an Electron install takeover by exact command (#20799)
* test(package): reject an Electron install takeover by exact command CodeRabbit was right about #20787. Replacing the pinned postinstall string with a /electron/i keyword check was wrong in both directions, verified: rebuild-native-deps.mjs && rebuild-native-deps.mjs PASSED (should fail) rebuild-native-deps.mjs && check-electron-version FAILED (should pass) The owner's own path contains no "electron", so duplicating it slipped through -- the one case the contract is named for. And a substring match rejects any later step that merely mentions Electron, which is the same over-tightness that broke every open PR in the first place, relocated. Later steps are now checked against the exact owned command plus the known Electron install commands. A second case pins the rejections themselves, because reading the real postinstall cannot show a bad chain would be caught -- that is how #20787 shipped with a guard that did not guard. Split into its own file rather than adding a max-lines disable (AGENTS.md). * test(package): match install commands as tokens and cover the rebuild:electron alias Both review comments were right, verified by running them: && check-install-app-deps-version.mjs rejected by substring match (should pass) && pnpm run rebuild:electron slipped through (should fail) package.json:101 aliases rebuild:electron to the owned script, so invoking it is the same takeover. Matching is now token-based with the owned command still checked as a phrase, and both cases are pinned. |
||
 Jinjing
|
6fe140ded8 |
Report clipboard and composer drop failures (#20795)
* 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. * fix(source-control): surface a failed notes copy instead of swallowing it * Simplify diff comment notes copy error message Replace parameterized translation template with a direct string. Add explicit type annotations in tests to improve type safety. * Sanitize clipboard write error messages for user display - Only user-friendly messages for recognized errors - Native failures logged but not exposed to UI - Prevents information disclosure (CWE-209) |
||
|
Jinjing
|
0569ca4cdc |
Improve microphone permission errors and drop failure reporting (#20801)
* 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. * fix(settings): say when the microphone is blocked and where to grant it * Use generic stream for microphone permission requests - Request generic audio stream instead of saved device to handle stale device IDs (unplugged microphones). This ensures the initial permission grant succeeds even if the previously saved device is no longer available. - Refactor error handling to not require instanceof checks, supporting errors thrown as plain objects and improving robustness across browsers. - Simplify tests with proper typing and add coverage for stale device and permission error edge cases. * fix type check * minor type fix |
||
|
Neil
|
22ce8d69a1 |
fix(lint): enable anti-slop/no-module-mocking (#20783)
The rule rejects `vi.mock` / `vi.doMock` / `vi.unstable_mockModule` and the
`jest` equivalents, on the argument that a test which rewrites the module graph
asserts against a stand-in the production code never sees. It is already off for
`**/*.test.{ts,tsx}`, `**/*.spec.{ts,tsx}`, `tests/**` and `**/__mocks__/**` via
the existing override in config/oxlint-anti-slop.json; that override is
unchanged here. What the rule actually catches is module mocking that has drifted
out of a spec and into a first-party `.ts` support module, where nothing marks it
as test-only.
73 violations at baseline, all of them in test-support code. 9 were relocated
back into spec files the override already exempts; the remaining 64 sit in 10
files that are test-only but do not match the override globs, and carry a
file-level disable naming the rule and the reason.
Relocated:
- terminal-hydration-store-test-bootstrap.ts: the sonner / sync-runtime-graph /
pty-transport `vi.mock` calls moved into the two specs that import it
(terminals-hydration-canonical-rows, terminals-hydration-canonical-pty-overlap).
Vitest hoists `vi.mock` inside a test file, so registration is strictly earlier
than the previous module-eval-time call; the bootstrap keeps only the preload
API proxy. Both importers were updated.
- ipc-events-ssh-authority-test-fixtures.ts: the 6 direct-ssh `vi.doMock` calls
moved into useIpcEvents-agent-status-ssh-authority.test.ts as a local
`stubDirectSshModules()` helper, which also de-duplicates the three copies the
spec already had inline. The fixture now returns the store state and coordinator
doubles it builds, typed via the exported DirectSshReconnectCoordinatorDouble.
Suppressed, with justification (each is `/* oxlint-disable
anti-slop/no-module-mocking -- ... */`, rule named, no blanket disable):
- config/scripts/headless-serve-shutdown-matrix.test.mjs (1) - a genuine Vitest
spec that the override misses only because its globs say {ts,tsx}. The script
under test is a top-level CLI module; the alternative is spawning real docker.
- src/main/codex-accounts/runtime-home-service-test-harness.ts (1) - stubs one
probe predicate in ../pty/shell-startup-env, imported directly by several
main-process readers; 17 specs share it.
- src/main/computer/desktop-script-provider-test-harness.ts (2) - stubs
child_process/fs-promises for a provider that shells out; 8 specs share it.
- src/main/github/work-item-search-test-harness.ts (4) - one consumer lives in
tests/e2e, where the relative mock ids resolve differently, so moving the calls
into the specs would silently stop mocking there.
- src/renderer/src/components/automations/automations-page-test-harness.tsx (14)
- the mount rig for 10 AutomationsPage specs.
- src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts
(1) - stubs refreshWebRuntimeSessionTabsSnapshot, imported directly by several
renderer runtime modules; 18 specs share it.
- src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts (7) -
stubReactSyncEffect/stubAuxiliaryModules, shared by 11 specs.
- src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts (11) - stubs
and hook invocation are one unit; 4 specs share it.
- src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts (13) - its
only spec is at 799 of an 800 max-lines budget.
- src/renderer/src/hooks/ipc-events-test-harness.ts (10) - shared by 8 specs.
No violation was converted to real dependency injection, and no max-lines disable
was added.
Verified: the audit command exits 0 with no output (and reports errors on a
planted probe, so the rule is live); node config/scripts/run-typecheck-projects-in-parallel.mjs
exits 0; 354 spec files / 2506 tests covering every importer of every touched
file pass. No mobile/ file was touched.
The changed-code quality gate's root Oxlint scan runs without --config so it never
loads the anti-slop JS plugin, which made all 10 of those file-level suppressions
read as "Unused oxlint-disable directive". check-changed-code-quality.mjs now
exempts directives naming an anti-slop rule from that unused-directive warning,
the same carve-out isCastingDirectiveUnusedWarning already makes for the casting
suppressions the casting config enforces. Such a directive can never suppress a
root-config rule, so nothing the root scan would otherwise report is hidden;
audit:anti-slop remains the scan that enforces the rule.
|
||
|
Jinjing
|
775a932651 |
fix(git): distinguish binary absence from missing cwd on spawn ENOENT (#20798)
* fix(repos): preserve unknown Git availability * fix(git): distinguish binary absence from missing cwd on spawn ENOENT Node reports ENOENT for both a missing git binary and a missing working directory during spawn. The fix checks specifically for spawn syscall, then verifies the cwd exists to disambiguate. This prevents reporting "no Git" when the error is actually a missing working directory. Centralizes probe logic in a reusable function; other failures cause rejection so callers preserve the unknown status instead of collapsing to false. |
||
|
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.
|
||
|
Neil
|
c9ae17fe3d |
fix(lint): enable anti-slop/no-unknown-type-aliases (#20784)
Flips anti-slop/no-unknown-type-aliases from "off" to "error" and fixes the
3 baseline violations.
The rule rejects a named type alias whose resolved type is `unknown` (directly,
through another alias, through parentheses, or as a member of a union). Such an
alias is strictly worse than writing `unknown`: it reads like a real domain type
at every use site while accepting anything, so the compiler stops helping and
readers are actively misled. `unknown` is fine, but it must stay visible at the
boundary that actually parses it.
Violations fixed (3 at baseline, 5 source files touched):
- src/main/runtime/workspace-session-failed-write-rollback.ts
`type RollbackValue = unknown` -> a real recursive JSON-shaped union
`RollbackSlot` (primitives | null | undefined | typeof MISSING |
readonly RollbackSlot[] | RollbackRecord), with a named
`type RollbackRecord = { readonly [key: string]: RollbackSlot }`.
The record is a named alias rather than an inline index signature because
inline violates typescript/consistent-indexed-object-style, `interface`
violates consistent-type-definitions, and `Readonly<Record<..>>` trips
TS2456 circular-reference. The named alias satisfies all three.
- src/renderer/src/hooks/direct-ssh-reconnect-coordinator-types.ts
`type DirectSshReconnectTimer = unknown` -> `ReturnType<typeof setTimeout>`,
the handle that actually flows. `DirectSshReconnectTargetState.timer` is
widened to `DirectSshReconnectTimer | null` to match the state machine, which
initializes to null and resets to null in the scheduled callback.
- src/renderer/src/hooks/direct-ssh-host-hydration.ts
`type HostReadTimer = unknown` -> `ReturnType<typeof setTimeout>`.
Fix pattern throughout: replace the alias with the type that already flows
through the code, never with `any` and never with a relabelled `unknown`.
Because the timer aliases are now honest, two pre-existing
`as ReturnType<typeof setTimeout>` casts at the clearTimeout boundaries could be
deleted, a net win under the repo's type-assertion policy.
Suppressions added: none. No eslint-disable, oxlint-disable, `any`, or `as`
cast was introduced anywhere in this change.
The diff is type-annotation-only; no runtime statement changed.
|
||
|
Jinjing
|
3ec6193e0f |
fix(pty): preserve child-process inspection uncertainty (#20756)
* fix(pty): preserve unverifiable local child reads * fix(pty): make child-process inspection synchronous Separate foreground and child-process sampling. Sample child processes synchronously after confirming foreground availability, returning unverifiable verdicts when pty reads fail. Handle both transport loss and local read failures uniformly in the completion coordinator. * fix(pty): handle retired masters and pane instance swaps Detect when node-pty retires the master fd (fd == -1) and return unverifiable instead of misreading the spawn file as an idle shell. Guard inspectProcess against PTY replacement mid-read to avoid pairing old foreground with replacement's children. * fix test * fix tests |
||
|
Neil
|
18d0afc918 | test(package): let the postinstall contract allow unrelated chained steps (#20787) | ||
|
github-actions[bot]
|
cf19735a4d | Update README downloads badge |