mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 00:02:35 +00:00
b5ee1b09dfd2dbad5abe761b284b236fdcf7d90c
11004
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Neil
|
b5ee1b09df |
fix(windows): repair what running the node-pty suites on Windows exposed
Putting these files on the Windows job turned four assertions red on the first run. Three of them were in tests that carried `skipIf(platform !== 'win32')` and had therefore never executed anywhere, on any branch. - `writeFakeElectronRebuild` emitted the `windows-process-tree` addon a real rebuild leaves but never node-pty's, so every Windows test of the rebuild path ran against a tree no real rebuild can produce: node-pty "rebuilt" with nothing in `build/Release`. The new same-host check reads that state correctly and said so. The fake rebuild now writes `build/Release/conpty.node` when it was asked to rebuild node-pty for win32, with the marker and the target machine. - `mkTempProject` never staged `windows-process-tree-creation-time.cjs`. The rebuild script reaches it through `projectRequire`, which resolves against the project root, so the module walker cannot follow it and must not try. Staged by name, with a comment saying which of the two it is. Without it the windows-process-tree probe failed to load its own checker and the module joined `modulesToRebuild`, which is the second and third red assertion. - the two `nodePtyAddonPath` cases compared against a literal POSIX string. `resolve` returns a drive letter and backslashes on Windows, so they could only ever pass off it. Built from segments now, which still pins the `..` traversal that is the point of the test. Verified on macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime -- 109 passed, 6 skipped. The 6 are the Windows-gated rebuild tests, which is the job this change is aimed at; Windows CI is the arbiter. |
||
|
Neil
|
2d623d704f |
docs(windows): put the superseded export-only gate in the past tense
It describes what used to pass a broken addon, so present tense reads as a description of the gate the same document then explains replacing it. |
||
|
Neil
|
ad0ccf83a8 |
fix(windows): run the node-pty addon gates on the Windows job that can
`rebuild-native-deps-node-pty.test.mjs` carries four `skipIf(platform !== 'win32')`
tests. The full suite runs on ubuntu, and the Windows PR job runs an explicit
file list that never named this file -- so those tests were skipped on Linux and
never reached anywhere else. Three of them predate this branch. The Windows job
is added the four node-pty addon suites plus the module-walker one; the comment
above that list already says why it is the right place, which is that the addon
assertions only hold once natives have been rebuilt. Running the path-joining
suites there also covers the separator this gate's candidate list is built from.
The rest is round-three review:
- the rebuild-time arch assertion told a reader "node-gyp did not honour --arch"
about a file that was not a PE image at all, which is a truncated or
quarantined artifact and a different command to run. The two now read
differently, and neither claims the other's cause. Same fix the packaged gate
had one commit ago, in the place that had not had it yet.
- the missing-addon error said node-pty "would load" a prebuild without checking
it is there. It says "fall through to" now, which is true either way.
- `isLoadableByArch` had no caller left once the packaged gate started needing
the raw machine field for its message. Removed rather than kept warm.
- each candidate's header is read once instead of up to three times.
- the module walker's comment claimed every shape that reaches a co-located
module; it does not follow `projectRequire`/`requireLocal`, and it must not --
those specifiers resolve against the project root, so following one stages the
wrong path and the copy fails. Proven by trying: widening the pattern to
require-shaped names broke nine tests on
`projectRequire('./config/scripts/...')`. The comment now says what it follows
and why it stops there.
- a new test resolved a file URL with `.pathname`, which keeps the drive-letter
slash on Windows -- the very job this commit adds it to.
|
||
|
Neil
|
2a73e6b03a |
fix(windows): say why the packaged ConPTY fell back, not just that it did
The previous commit resolved the addon by architecture but still had one message
for every way the resolution could land on the published prebuild. Those ways
want opposite remedies, and the one it printed was the remedy the commit before
it had just called wrong:
- no source build in the package at all — the slice has to be built somewhere
that can build node-pty for the target arch.
- a source build that is there but is the packaging host's architecture, because
the cross-arch rebuild did not honour `--arch` — re-running that rebuild is the
fix, and "package on a Windows arm64 host" is neither necessary nor possible.
The second is the common one, since node-pty publishes a prebuild for both
Windows arches and prune keeps the target's on every cross-arch package. So the
old text fired mostly on the case it described least. It now reports which source
builds were skipped and the machine field each carried, and names the rebuild
command.
"Nothing the target can load" had the same problem in reverse: a zero-length or
truncated `conpty.node` got a cross-architecture diagnosis. Every candidate is
now named with what was actually read, including "not a PE image".
The rebuild path asserts the architecture too. A rebuild that ignored `--arch`
was otherwise only visible at packaging, two steps from the command that fixes
it. Arches with no known machine value are left unjudged rather than guessed at.
Two things the extraction broke or nearly broke, both found by mutation:
- the shared PE reader answers `null` where the relay builder's private copy
returned a number, which would have turned its "node-gyp ignored --arch" error
into a `TypeError`. Both callers now go through `describePeMachine`.
- the rebuild fixtures stage a script's co-located modules by walking its
imports, and the walker only understood `from '...'` — so the gate's new
`require('./windows-pe-machine.cjs')` was left behind and every subprocess test
failed with a resolution error, which is the exact failure its own comment
warns about. It now follows `require` and bare side-effect `import` as well,
and has tests; the fixture stages the gate by walking it rather than by naming
one file.
Fixtures write real PE headers through one shared builder instead of three
hand-rolled ones.
|
||
|
Neil
|
5d9697b6ba |
fix(windows): resolve the packaged ConPTY the way its loader does
Sweeping every candidate and demanding the marker on all of them was wrong in
the one case it was meant to make safe. `beforeBuild` runs
`rebuild-native-deps.mjs --platform=win32 --arch=<target>`, so a cross-arch slice
normally does get a patched `build/Release` for the target; `prunePackagedNodePty`
keeps the prebuild anyway because its guard is `electronArch === process.arch`
rather than the arch of the binary. That package is correct and its leftover
prebuild is never reached, and the sweep failed it -- telling whoever ran it to
package on a Windows arm64 host, which is both the wrong remedy and one no runner
here can offer.
Presence cannot separate that package from the one whose cross-arch rebuild
quietly emitted the host's architecture, because the only difference is the arch
of `build/Release`. So the gate now resolves the addon the way `loadNativeModule`
does -- first candidate whose PE `IMAGE_FILE_HEADER.Machine` matches the target,
walking root-then-lib for each layout in node-pty's own order -- and checks the
marker on the one that will actually run. A package with no candidate, or none of
the target's architecture, is refused: it has no ConPTY backend either way, and
the second is exactly what a silently host-arch cross-build looks like.
The PE machine reader already existed, privately, in the relay addon builder that
needed the same "a cross-build cannot silently emit host arch" guarantee. It is
now shared rather than copied.
Two seams were unreachable from anything but Windows, so nothing tested them:
- the afterPack hook's win32 block was an inline if/else that only a source-text
assertion could inspect, and that assertion could not tell the difference
between the check running and the check being wrapped in `try {} catch {}`. It
is now `verifyPackagedWindowsNodePty`, and "the marker check runs even where
the export check cannot" is four spied assertions instead of a string match.
- the rebuild path's verdict read `process` directly, so the branch that fires
only on the host being rebuilt for was dead on every other host. It now takes
the host as arguments, and the fs checks, the warning and the failure are all
exercised from macOS.
Fixtures write a real PE header rather than `MZ fake addon`, since the gate now
reads one. The machine table is pinned to the documented IMAGE_FILE_MACHINE
values, because every fixture builds its header from that table and a table wrong
in both entries would otherwise agree with itself.
|
||
|
Neil
|
2de0829da6 |
fix(windows): check the marker on every ConPTY path the packaged app can load
The packaged marker check read one hard-coded path, `build/Release/conpty.node`, and warned when it was absent. `loadNativeModule` tries `build/Release`, then `build/Debug`, then `prebuilds/win32-<arch>`, swallowing each failure, and `prunePackagedNodePty` drops the published prebuild only when a same-arch `build/Release` exists to replace it. So the two packages the check was added for were the two it could not see: - cross-host: no host but Windows can build conpty.node, so there is no `build/Release` and the prebuild is what ships. The check warned and returned. - cross-arch: `build/Release` is the packaging host's own arch, patched and marked, so the check printed OK -- while the target app cannot load it and falls through to the unmarked prebuild underneath. Measured, not assumed: both published Windows prebuilds in the node-pty tarball contain neither `msys-2.0.dll` nor `cygwin1.dll` in any encoding. They are the binary that leaks every MSYS pane child out of its job. It now sweeps every candidate present for the *target* arch and refuses a package with no candidate at all, which is a package with no ConPTY backend rather than a layout to shrug at. It runs for every Windows slice instead of only the branch the export check skips, so deleting the export check cannot silently take it too. A stale source build keeps the rebuild advice; the prebuild gets the advice that actually works, which is to package the slice on a Windows host of that arch. Also: the marker constant was re-typed in four places and was tied to the C++ literal that produces it by nothing at all, so editing the patch would have left a gate that fails every correctly rebuilt addon and tells the developer to do the one thing that cannot help. The fixtures now take the constant from the gate, and a test asserts the patch still adds `L"msys-2.0.dll"` to conpty.cc. And the rebuild path treated a missing addon as a warning even on the host that will run the install, where node-pty would fall through to that same prebuild. The verdict is now a value, so it is tested without a platform gate. |
||
 Neil
|
c28a0a4f46 |
fix(windows): check the MSYS breakaway denial on the rebuild path too
The Electron probe carried the marker check, but it lives inside probeElectronNativeModules, which returns early whenever the Electron package binary is unusable. Covered by another path is not this path checks -- and the defect this whole change closes was a gate that looked like it checked. Reading the binary needs neither a loadable Electron nor an executable target arch, so assert it after the rebuild, beside the windows-process-tree assertion that exists for the same reason: this is the addon copied into the packaged app. Absent warns (a cross-platform rebuild need not leave a win32 addon on this disk); present and unmarked is fatal. The fixtures now write a real addon file, because the gate reads the binary it was told about rather than trusting the exports. Verified against the two real binaries measured on the Windows host: the pre-#19068 build fails this path, the build from current patched source passes. |
||
|
Neil
|
e47e00649b |
fix(windows): reject a node-pty addon that predates the MSYS breakaway denial
The native-runtime gate asserted only that terminateJob, listJobProcessIds and assignCurrentProcessToJob were exported. All three predate the Cygwin/MSYS breakaway denial, so an addon built before it passes every gate, isPtyJobOwnershipAvailable() returns true, and windows-pty-job.win32.test.ts passes 6/6 -- while every Git Bash child is created outside its pane's job and survives terminatePtyJob. Read the resolved .node and require the wide msys-2.0.dll literal that usesCygwinRuntime holds, the way stagedRelayAddonIsUnpatched() already tells a patched windows-process-tree addon from a published one. An addon the caller cannot name is refused rather than skipped: a gate that cannot see its subject is not a gate. Verified against real binaries on a Windows 11 host: the shared checkout's pre-#19068 build errors, a build from current patched source passes, a missing path errors. Also closes the cross-host packaging skip. The export half has to load the addon so it cannot run when the packaging host is not the target, which is how a Windows release built elsewhere could ship this. The marker is a file read and needs neither; an unrecognised layout warns rather than fails a release that was packaging fine. |
||
|
Neil
|
0b37de7cd4 |
docs(windows): record the measured MSYS job-breakaway mechanism
The per-PTY job already denies JOB_OBJECT_LIMIT_BREAKAWAY_OK for Cygwin/MSYS shells (#19068), but nothing records why, and a conpty.node built before that commit fails windows-msys-job.win32.test.ts in a way that reads as a source defect. Measured on a real Windows 11 host: both the plain and the exec- replacement Git Bash shapes leak, the escape is the MSYS runtime's own spawn/exec (fork keeps membership), and a single-variable A/B on usesCygwinRuntime flips the result 0/2 -> 4/4. Also names the gap the failure hid behind: node-pty-job-ownership.cjs asserts symbol presence, which cannot distinguish patch revisions. |
||
|
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 | ||
|
Neil
|
bbd808a63d |
fix(lint): keep root postinstall as the sole Electron binary install owner (#20788)
#20726 appended the anti-slop plugin sync to postinstall, which breaks the contract asserted by package-electron-runtime-contract.test.mjs and is failing on main. The sync is not needed there: audit:anti-slop already runs it before linting, so a cached install that skips postinstall still works. |
||
 Brennan Benson
|
ab6b86dd5c | fix(orchestration): require registered structured worker pane key (#20664) | ||
|
Neil
|
4a5b0583b2 |
fix(runtime): keep listed handles when graph sync learns a PTY incarnation (#20779)
reconcilePtyIncarnationHandles compared a null retained incarnation against the learned one and staled the handle. Daemon-hosted PTYs are recorded from first output before the spawn commit reports an incarnation, so on Windows `orca terminal create` returned a handle that was stale by the next graph publish. Treat null-to-known as un-fenced like every other site; keep the known-to-different and preallocated-handle invalidations. |
||
|
Neil
|
b79206533e |
chore(lint): enable anti-slop no-reduce-accumulator-copy and no-widen-then-assert (#20780)
Both rules already report zero violations, so this only locks in the current state as a ratchet. No source changes. |
||
|
Neil
|
11180fa532 |
chore(lint): add anti-slop oxlint plugin (pinned, all rules off) (#20726)
* chore(lint): add anti-slop oxlint plugin (all rules off) Vendors dmmulroy/anti-slop (MIT) plus no-call-only-assertions and no-pass-through-type-alias from maharshi365/deslop (MIT). Every rule starts "off"; each follow-up PR fixes one rule's violations and flips it to "error". * fix(lint): actually exclude the vendored plugin from the anti-slop audit oxlint does not honour ignorePatterns supplied via --config, so the config/oxlint-plugins/anti-slop/** entry never matched and the vendored rule source was being linted as first-party code (505 violations). Move the exclusion to the --ignore-pattern CLI flag in audit:anti-slop, which does work, and drop the entry that gave a false sense of coverage. Keeping vendored source unlinted matters because anti-slop is updated by three-way merge against the upstream snapshot; reformatting it locally would conflict on every update. * chore(lint): pin anti-slop instead of vendoring it; drop deslop Replaces the ~5k vendored lines with a git-pinned devDependency: oxlint-plugin-anti-slop: github:dmmulroy/anti-slop#c44ef22 anti-slop ships raw .ts with no build step, and Node refuses to type-strip anything under node_modules (ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING), so oxlint cannot load it from there -- which is why upstream says to vendor it. A postinstall step copies the pinned package's source to .anti-slop-plugin/ (gitignored), which Node will type-strip because it sits outside node_modules. Upgrading is now a SHA bump rather than a re-vendor and three-way merge. Verified byte-identical rule output to the vendored copy across all 16 rules that fire. Drops maharshi365/deslop and its two rules (no-call-only-assertions, no-pass-through-type-alias). It is not on npm either, so it would need a second git pin and copy step, and it is a 5-star single-maintainer repo that is itself a re-namespaced copy of anti-slop. One upstream is enough. * ci(lint): run audit:anti-slop in PR CI config/scripts/pr-workflow-lint-parity.test.mjs requires every step in `pnpm lint` to have a matching step in .github/workflows/pr.yml; adding audit:anti-slop to lint without the workflow step failed that ratchet. Also makes audit:anti-slop sync the plugin itself before linting. The generated .anti-slop-plugin/ directory is gitignored and otherwise only created by postinstall, so a cached install that skips postinstall would leave oxlint unable to load the plugin. |
||
|
Neil
|
ef39f32d4f |
test(native-chat): split the windowing test harness out of the suite (#20773)
#20719 grew NativeChatMessageList.windowing.test.tsx to 897 effective lines, past the 800 ceiling for test files, so oxlint fails on main. Moves the shared layout/ResizeObserver stubs into native-chat-windowing-test-harness.tsx. No test was changed, split or dropped: still 5 describes and 23 it() blocks, 29 assertions passing. The stubs' mutable knobs become one exported `layout` object because an imported binding cannot be reassigned across modules. AGENTS.md forbids a max-lines disable, so extraction is the fix. |
||
|
Jinjing
|
99062ed80b |
fix(worktrees): preserve unverifiable disk witness (#20713)
* fix(worktrees): preserve unverifiable disk witness * fix(worktrees): follow gitdir/commondir markers in disk witness The disk witness validates created worktrees by reading the repo's common directory from disk. Previously it only checked for a direct .git directory and returned a status object that conflated different failure modes. Now it properly follows .gitdir and commondir pointer files to locate the true common directory, fixing detection on repos with linked git directories (worktrees, submodules) and WSL scenarios. Error handling is simplified: definitive absence returns undefined, other read failures throw with proper cause chains, eliminating the ambiguous "unverifiable" state that would mask real errors. * fix: validate gitdir marker targets are directories When a .git marker points to a missing or non-directory path, that's unverifiable—not the same as an absent .git file (bare repo). Validate accessibility before reading commondir to catch these errors clearly. |
||
|
Brennan Benson
|
438603f9e7 |
feat(native-chat): add a message rail for jumping between your prompts (#20719)
* feat(native-chat): add a message rail for jumping between your prompts A vertical rail down the right edge of the transcript, one bar per user message, with the bar for the turn you are reading highlighted once scrolling settles. Hovering the rail opens a panel that previews every prompt and jumps to it on click. Bars are capped at 20 and sampled evenly across the thread, always keeping both ends and the active bar, so the rail stays readable at a glance on a long conversation. The active bar is resolved from virtualizer offsets rather than by scanning rendered rows: the transcript is windowed, so an off-window row has no element to measure. The row at the scroll fold resolves to its owning prompt through turnKey, which is what keeps your own message lit while you read a long reply instead of going dark. Jumps reuse the existing reveal/pin path and scrollMessageToTop, which releases the bottom pin. Scrolling through the virtualizer directly would leave a reader snapped back down by the next streamed token. Ticks cover loaded history only; older prompts gain a bar once "Load earlier messages" pages them in. * fix(native-chat): service a rail jump once and give its pin back The rail borrowed the diff reveal's pin to reach a row the window had left behind, but copied only its state shape, not its consumption. The request was never cleared and the effect depended on `slots`, which is rebuilt on every render, so three things went wrong at once: - every later render re-scrolled to the jumped message, dragging a reader back there for the rest of the pane's life, and forcing the bottom pin off each time; - the standing request outranked `revealedDiff` in the shared pin, so revealing a diff outside the window silently stopped mounting its row; - the pinned row stayed mounted and measured indefinitely. The request now carries a monotonic id, is serviced once, and is released as soon as the scroll is issued, which hands the pin back. The rail's scroll listener had the same churn: it listed `items` in its deps, so a streaming turn tore the listener down and cancelled the pending idle timer on every frame and the highlight never settled. It now subscribes once and re-reads on a key built from the prompt ids. Also: the hover trigger is a real button, because `asChild` discards the primitive's focusable trigger and the panel is the only way to reach these messages; the wheel forwarder honours line and page delta modes rather than treating every delta as pixels; and the e2e panel assertion is exact, since a loose bound passed at 20 rows against 20 ticks. * fix(native-chat): make prompt rail accessible and reuse previews * fix(native-chat): supersede prior navigation when selecting a prompt |
||
|
Brennan Benson
|
ff5b1a5a05 |
fix(native-chat): preserve detached transcript position during growth (#20710)
* fix(native-chat): stop the transcript following an end it measured short The virtualizer compensates a row's measured size change by moving scrollTop whenever it believes the view was already at the end. It decides that from the spacer's own height minus a container-absolute offset, so the distance it computes is short by everything in the document outside the spacer: the transcript's top gutter, the "load earlier" block while older history is still pageable, and the trailing chrome. A reader sitting ~100px above the bottom therefore measured as "at the end", and every row that settled below them dragged them down to it. Measured in the windowing harness with a 92px gutter and 24px of trailing chrome: a reader parked 96px above the end is pulled to the end on the first growth frame, scrollTop 9261 to 9357. The same option gates following an append, but that path measures the true document distance, so it was never wrong, only redundant. The transcript already decides whether to follow the end from the scroll container's real geometry, and it re-pins once the growth is in the document rather than before it, where the library's own write is clamped. Both library end behaviours are retired by a threshold no finite distance can meet; the prepend anchoring that shares the option is kept. overflow-anchor:none is restated as structural: the engine's anchoring writes never pass through the scrollToFn adapter that attributes this pane's own scrolls, so they would arrive unmarked and read as the reader leaving. * fix(native-chat): preserve visible rows on first measurement |
||
|
github-actions[bot]
|
6f4e4bfa22 | Update README downloads badge | ||
|
Neil
|
b61a2347b9 |
feat(design-system): gate renderer UI with @shadcn/lint (#20731)
* feat(design-system): gate renderer UI with @shadcn/lint Wires shadcn-ui/lint's Oxlint plugin into the two places this repo already ratchets: the changed-lines PR gate for rules the renderer can't satisfy today, and `pnpm lint` for the one that is already at zero. - config/oxlint-design-system.json: no-restyle (layout allowed), no-raw-colors, require-static-classes -- scoped to src/renderer/**/*.tsx, run over added lines only. Measured at 10 findings across the last 60 commits (771 changed files), so it holds the line without a migration. - config/oxlint-dead-classes.json: no-unknown-classes repo-wide, with the renderer's plain-CSS hook namespaces allow-listed. Now at zero. - no-inline-styles and no-arbitrary-values stay off; STYLEGUIDE says why. Fixes the three live bugs the linter found: - `--editor-surface` never reached `@theme inline`, so `bg-editor-surface` generated no CSS -- 12 editor/artifact/notebook panes fell through to the page background instead of #1e1e1e in dark mode. - `scrollbar-none` is not a Tailwind utility and was declared nowhere, so the remote file browser breadcrumbs showed the scrollbar they meant to hide. Declared as a real `@utility`. - Notebook markdown cells used `markdown-preview-body`, which no stylesheet defines; the styled class is `markdown-body`. They rendered unstyled. * ci: run the dead-class gate in PR CI `pnpm lint` gained check:dead-classes, and pr-workflow-lint-parity requires every `pnpm lint` step to have a matching step in pr.yml. * fix(notebook): keep markdown theme selectors working |
||
|
Brennan Benson
|
db09a7bd50 |
fix(native-chat): let a reader park just above the latest message (#20709)
* fix(native-chat): let a reader park just above the latest message A reader who scrolled up by less than the bottom threshold was still classified as being at the end, so follow stayed armed and the next chunk of stream carried them back down. One constant was answering two different questions: how close to the end still counts as pinned, and whether a reader's own scroll meant to stay there. The first wants slack, because a streaming last message jitters in height by tens of pixels. The second wants almost none, because it is a statement of intent. Give it its own, far stricter band, and move the choice of band into the decision rather than leaving it to the call site, which is where the two got conflated. Re-arming follow now requires the reader to be within 4px of the end: enough for fractional-pixel and zoom rounding, well inside one line of prose. The pin and the jump-to-latest affordance keep their 48px band. * fix(native-chat): make transcript intent own end following |
||
|
Neil
|
2b34255d96 |
fix(ci): stop defining pilot mutant tests inside a conditional (#20755)
`vitest/no-conditional-tests` fires on the `if (mutation) { it(...) }` inside
the pilot loop, and `audit:code-quality:native` runs oxlint with
`--deny-warnings`, so main's "Enforce focused code-quality plugins" step exits
1 and blocks every open PR.
Pair each pilot with its pinned mutant and reference state before the loops, so
every iteration defines exactly one test unconditionally. Same 14 tests, same
names: 11 mutant-kill tests and the 3 reference tests that `skipIf` still gates
on RPC_FOUNDATION_REFERENCE_ROOT.
|
||
|
Neil
|
20794ee785 |
ci: keep the baseline build off the compatibility matrix lanes (#20733)
The compatibility gate started the pinned 2.25.5 source build inside the same step that runs the three measured lanes, so `make -j$(nproc)` competed with two container lanes whose wall clock is container starts, not Git. A boundary case that costs ~1.5s stretched past Vitest's 30s timeout and failed the job. Build the binary in its own step before the matrix, and pull both images before any lane starts so a lazy pull cannot stall whichever test its sibling is timing. |
||
|
Jinjing
|
ffc331212c |
Fix PTY child process verdict to preserve unverifiable state (#20729)
* 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. |
||
 Jinwoo Hong
|
6a11a0b8e6 |
test(mobile): pin each RPC golden to the recorder inputs that can reach it, not the whole directory (#20662)
* 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
|
b07c4032ea | Log bounded PostgreSQL acquisition and execution failure diagnostics (#20749) | ||
|
Jinjing
|
4bcdc67369 |
Distinguish pane load failures from empty states (#20735)
* refactor(renderer): give the IPC error reader a clamped and an unclamped shape * fix(settings): tell a failed load apart from a genuinely empty pane * refactor: consolidate import types and simplify failure handling - Move filesystem import types to shared for renderer use - Add compactIpcErrorMessage for single-line error display - Consolidate entry failure toasts to single global slot - Simplify account tracking and discard retry logic * fix type * fix: clear stale state when pane loads fail Credential reads, account fetches, and skill scans can fail, leaving stale data on screen. This change clears previous state when a load fails, distinguishing load failures from genuinely empty results, and prevents stale controls from appearing after failed re-checks. Use readIpcErrorMessage for consistent error handling and track runtime targets to invalidate results from old targets. * fix(settings): show credential action when bitbucket status read fails When the credential-read operation fails, allow users to retry by showing "Add or replace credentials" button. Initialize the credentials dialog with the current (confirmed) connection state instead of stale data from a failed read, preventing outdated information from pre-populating the form. |
||
  
|
6cb5643241 |
fix(deps): migrate Tiptap security updates with Markdown compatibility guards (#19376)
* chore(deps): evaluate coordinated Tiptap security migration * fix(editor): adapt link ranking and initialization for Tiptap 3.31 * fix(editor): preserve literal Markdown through Tiptap serialization * test(editor): cover literal saves in local folder and paired workspaces * test(editor): reselect folder after closing its final tab * perf(editor): avoid repeated inline source-marker lookahead scans * refactor(editor): inline redundant HTML match wrapper * test(chat): await Tiptap React skill-pill rendering --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local> Co-authored-by: m4air <m4air@Mac.localdomain> |
||
|
Jinjing
|
b8554f1c59 |
fix(composer): clarify failed attachment drops (#20704)
* refactor(renderer): give the IPC error reader a clamped and an unclamped shape * fix(composer): name the attachments a drop could not add, in one toast * fix(composer, source-control): use one stable failure toast slot - Replace per-worktree toast IDs with single slot that replaces on each failure - Remove destructive retry actions; discard must confirm in dialog - Consolidate filesystem import types to shared location - Add compactIpcErrorMessage for string error handling * refactor: centralize filesystem import types and clarify failure naming Move import result types from main/ipc to shared layer so they're available across preload and renderer. Rename uniformFailure → commonFailure and skippedOrFailed → failureCount for clarity. Simplify preload/API type definitions by reusing shared types directly instead of duplicating inlined union shapes. * Reuse single toast slot for composer drop failures Multiple drop failures now replace the previous toast instead of stacking, preventing notification clutter. Uses a dedicated toast ID separate from Source Control's stage/discard notifications. |
||
|
Neil
|
767b7c14f1 |
fix(ai-vault): expand nested OMP session history (#20663)
Expand saved OMP descendants lazily while preserving exact child targets for Resume and View Log. Retain expanded branches across virtual scrolling and reject late responses/cycles. Includes the independently reviewed child-workspace correction from #20629. 61 combined target/map/nesting tests and actual OMP child/grandchild storage/CLI smoke pass. Earlier hidden Electron proof covers eight generations and narrow sidebar layout. Folder-only unresolved child targets remain disabled. No live delegation or full terminal-launch proof claimed. Addresses #12885 Scope 2. |
||
|
Neil
|
742a7ad842 |
fix(omp): resume independent child sessions from history (#20629)
Add Resume to eligible local OMP child history rows. Resolve lazy child targets from their own cwd and host, never an unrelated active workspace. Unresolved folder-only targets stay disabled; copy-command remains available. Verified production map/resume resolver regression before/after; 50 focused tests and independent 40-test review, web types and code quality passed. Actual OMP storage/CLI smoke confirms distinct child/grandchild sessions. No native Windows or live SSH launch claim. Addresses #12885 Scope 1. |
||
 Brennan BensonandMerge Sim
|
f55b7ba680 |
fix(native-chat): cancel pending prompts precisely (#20601)
* fix(native-chat): hide activity while awaiting input * fix(native-chat): keep approval turns cancellable * test(native-chat): satisfy split PR quality gate * fix(native-chat): catalog approval cancellation label * fix(native-chat): include approval cancellation runtime label * fix(codex): settle prompts when cancelled turns complete * fix(codex): settle prompt registry fallbacks * test(native-chat): cover pending interaction fallbacks * test(native-chat): split prompt state coverage * test(native-chat): keep prompt state isolated * fix(native-chat): bound prompt turn backfill * refactor(codex): centralize prompt registry bounds * fix(native-chat): cancel pending prompts precisely * fix(native-chat): consolidate capability imports * fix(native-chat): harden precise prompt cancellation * fix claude cancellation teardown races * retry claude prompt lifecycle admission * bound claude prompt cancellation retry work * fix(codex): bound prompt turn identity on registration * fix(native-chat): route rejected late dispatch settlements * fix(codex): retain exact cancellable prompt turn ids --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
dd85e5fc81 |
fix: keep OMP terminals when folder workspaces become Git repos (#20653)
Preserve the original folder locator through Git upgrade and subsequent listing, persistence, and removal decisions after proving it still names the same checkout. Independently reviewed with 60 focused persistence/listing/removal tests and six native Windows real-Git/NTFS cases covering case/slashes, junction retention and retargeting, remote-host isolation and unrelated checkout preservation. Prior source-connected native OMP proof confirms process survival. Full PR CI passed; no rebuilt full-app after-proof claimed. |