Cwd-less gh/glab calls whose host CLI is missing fell back to the default
WSL distro with no running check, so background work (rate limits, auth
status, Settings, task-source probes) booted a stopped distro. The
fallback now asks the background exec gate first: stopped or
unverifiable refuses with the distro-stopped error, running and user
acts fall back as before.
GitHub issue close/reopen swallowed every gh failure mentioning "already"
(e.g. "API rate limit already exceeded"), and GitLab MR close/reopen did the
same for glab. gh exits 0 when the issue is already in the target state, so
every gh failure is now reported. For glab, only its fixed no-op refusals
("this merge request has been closed" / "this merge request is already open")
count as success; "has already been merged" and all other errors surface.
Fixes#28766
Co-authored-by: Neil <neil@stably.ai>
* fix(gitlab): retain successful hosts from partial auth failures
Read rejected auth-status output without treating failed host headings or
API error advice as successful logins. Publish positive results through
the existing context-owned cache; keep backoff when no login succeeds.
Co-authored-by: David Anderson <cantspamdave@gmail.com>
* fix(gitlab): preserve discovery retries with partial host evidence
* fix(gitlab): escape auth marker for ASCII server bundle
---------
Co-authored-by: David Anderson <cantspamdave@gmail.com>
* Correct GitLab MR check status for failed and manual pipelines
Use the same single-MR read for linked and branch-discovered reviews so a
list summary cannot hide a failed pipeline. Preserve lookup failures for
strict callers and keep detail reads within the existing branch cache.
Set mergeable manual pipelines to neutral without claiming an unrun stage
passed; keep blocking manual pipelines pending.
Co-authored-by: Son of Odin <2700975-son-of-odin@users.noreply.gitlab.com>
Co-authored-by: Emily <hadikcze@gmail.com>
* test: cover GitLab MR check status consumers
* test: type GitLab MR fixtures from mapper input
* chore: document GitLab MR boundary assertion exceptions
* test: share GitLab MR producer and consumer fixtures
---------
Co-authored-by: Son of Odin <2700975-son-of-odin@users.noreply.gitlab.com>
Co-authored-by: Emily <hadikcze@gmail.com>
* fix(gitlab): keep paging merge requests when GitLab omits the totals
When total headers are absent, a full response advertises another page to probe and a short response ends pagination. Keep the existing header precedence and unknown total count.
Tracked in #33800
* test(gitlab): cover MR pagination exhaustion and header precedence
* test(gitlab): verify unknown-total metadata across list consumers
* test(gitlab): cover remaining screen consumers of list metadata
* test: use checked GitLab RPC runtime fixtures
---------
Co-authored-by: Hotragn <103170876+Hotragn@users.noreply.github.com>
Main now tags each Orca-authored PR/MR create failure with a reason code
(the English error is unchanged for mobile, older clients and logs), and
the renderer maps the reason to translated copy in the Source Control
notice, the Checks panel and the pre-create blocked notice. Sign-in steps
are single-sourced across providers.
Fixes#33321
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
* fix: back off failed GitLab authentication probes
* fix(gitlab): use short exponential backoff for failed host-discovery probes
Start at 15s and double up to the existing 5-minute negative TTL, and
clear the backoff on auth refresh (rememberGlabKnownHosts), successful
probes and forced preflight resets so recovered auth is seen promptly.
Keeps the positive host cache shape unchanged.
---------
Co-authored-by: Neil <neil@stably.ai>
* feat(ci): ratchet raw repo connectionId host reads; one drop-in resolver
Codex review 6 class fix (#4/#6/#7): host decisions read off a repo row's
legacy connectionId miss rows that name SSH only as executionHostId.
- getRepoRoutingConnectionId (shared/execution-host.ts): drop-in for a raw
repo.connectionId routing check that also finds executionHostId-only SSH.
- check-repo-host-read-ratchet: per-file count of raw repo connectionId
reads in src/main, src/shared, src/cli; counts may only shrink.
- Migrated 66 cheap reads: runtime GitHub/GitLab commands, PR/MR head ref
fetches, preserved-branch cleanup, ref queries, base-ref IPC, worktree
preparation, trash sweep, scan cache TTL, usage metadata.
* fix(ipc): PR base IPC routes executionHostId-only SSH rows; ratchet is a stated lower bound
Review follow-up (Opus on #31096):
- worktrees:resolvePrBase decided git/gh routing on the raw connectionId while
its head-ref fetches already used the resolver; one resolved connectionId
now drives all of it (test fails on main).
- getRepoRoutingConnectionId's contract is stated exactly: never local when the
raw field is set; on a contradictory ssh row executionHostId wins.
- The ratchet regex no longer matches report.connectionId; header and docs
say it is a lower bound.
- Regression tests for a runtime GitHub and GitLab command site.
* test(ci): pin the repo host-read ratchet step to the static phase
* fix(ci): repo host-read ratchet skips verbatim field copies into projected rows
A copy of connectionId into a projected row decides nothing; migrating it to
the resolver would change what the projection publishes. The sidebar feed
catalog's copy (added after the baseline) and two older projections no longer
count; baseline pruned to 91.
* refactor(providers): delete the SSH git and filesystem providers and relay-only wiring
* refactor(ssh): drop relay-protocol exports only the deleted stream readers used
---------
Co-authored-by: m4air <m4air@Mac.localdomain>
Extract subprocess execution and process contracts into @orca/process-host with explicit public entry points, independent checks, and one Node 18 compatible CommonJS runtime. Migrate application consumers and integrate package builds, dependency selection, import enforcement, and runtime resource staging.
Preserve daemon startup ownership and profile isolation, and qualify development rebuild recovery and concurrent output publication. Remove unnecessary aliases, test forwarding exports, and a duplicate output scan.
Validation: full typechecks, changed-code quality, package and artifact checks, native Windows builds/types plus 306 runtime tests, and independent native reviews.
Matched full E2E controls on original main and unchanged reviewed head both pass all 17 applicable tests (nine skips). Earlier intermittent E2E failures are documented in the PR; no assertions were weakened. The final combined-tree audit preserves both branches with no import-boundary violations.
`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.
* fix(git): skip upstream remote probes when the remote is absent
Issue and PR resolvers listed remotes by probing `git remote get-url
upstream` on every poll, including origin-only clones where that remote
cannot exist. List remotes once, cache against git config, and skip the
probe unless `upstream` is present.
* fix(git): avoid stale remote probe cache entries
* fix(github): observe origin repository probe failures
* fix(github): observe verified origin probe failures
* fix(github): skip missing upstream probe for PR lists
* test(github): scope the #9171 lazy-resolution guard to default-branch commands
The guard asserted that no git command runs for an open PR, using "no git at
all" as a proxy for "no default-branch resolution". Remote-name listing is a
separate concern, so allow it and keep every other command forbidden; the
symbolic-ref/rev-parse resolution this issue is about stays unreachable.
---------
Co-authored-by: Neil <neil@stably.ai>
`ForgeProvider.createReview(repoPath, input, connectionId, options)` and the
`connectionId` on `ForgeProviderRepositoryContext` carried the same collapse the
five prior migrations closed: `string | null` spells "genuinely local", "runtime
host" and "could not resolve" with one value. Because it was decided two layers
up -- `repo.connectionId ?? null` at the `hostedReview:*` IPC handlers and in
`RuntimeHostedReviewCommands` -- a row naming its owner only as
`executionHostId: ssh:<target>` ran the whole review path against this machine's
copy of a remote path (#11163): `git rev-parse`, `git status`, the base-on-remote
ref probe, the upstream divergence read, and `gh`/`glab` with no host flags.
Replace it with a required `ExecutionHostId` threaded from the decision point
through the contract, routed by #18296's `resolveGitRouteForHost`. The parameter
is removed rather than added beside, so all five implementations -- GitLab,
GitHub, Bitbucket, Azure DevOps, Gitea -- and every caller became a compile
error. None of these families carries `@ts-nocheck`, so unlike #18325 that
guarantee is real here; `orca-runtime-file-commands.ts` does, but it only
constructs `RuntimeHostedReviewCommands` with unchanged deps.
Also fixed at the sites:
- The branch cache scoped entries on `connectionId ?? ''`, so two rows at one
path on different hosts shared one cached review, one backoff deadline and one
invalidation. Keyed on the resolved host now, as #18377 did for its probe key.
- `hostedReview:create` resolved shared symlink paths and normalized worktree
paths off the raw field, so an `executionHostId`-only SSH row read `orca.yaml`
and `resolve()`d a remote POSIX path on the client. Those ask the file-holder
question -- `getRepoSshConnectionId` -- not the dialable one.
- An SSH host with no provider now refuses inside the git-state layer instead of
reaching the local branch, keeping "remote and unreachable" distinct from
"local" (docs/reference/ssh-execution-boundary.md).
`runtime:` is a routing mistake inside `hostedReviewSshConnectionId` -- that
environment's server runs its own git, and the SSH target on its repo row is
nested in that server's namespace, so dialing it here reaches a same-named box of
ours. But store-backed callers ask `getRepoHostedReviewExecutionHostId` first,
which is "what may this client dial" and answers `local` for a `runtime:` row.
That is deliberate and matches #18377: the runtime registration controller only
adopts a `runtime:` stamp onto a row with no `connectionId`
(`runtimeRepoMatchesExecutionHost` refuses to match an SSH row), so the checkout
really is in this process and refusing would regress a runtime server creating
reviews for its own rows.
No wire change. `connectionId` on `CreateHostedReviewArgs`,
`CreateStackedHostedReviewArgs` and `HostedReviewCreationEligibilityArgs` in
src/shared/hosted-review.ts is untouched -- every host already ignores it in
favor of the repo row, and removing it from the request types would only churn
the schema older clients still populate. The main-side eligibility input `Omit`s
it so nothing on this side can read the ambiguous field again.
`gh` and `glab` on PATH are routinely shims — mise, asdf, volta, or a
hand-written wrapper — so a timed-out invocation has a chain to stop, not
one process. `execFileCapture`'s POSIX kill path signals only the direct
child; the descendants are orphaned to init and keep running. #18234 is
exactly that shape: `bash ~/.local/bin/gh` -> `mise x gh` -> `gh`, where
the reporter found the tail reparented to `systemd --user` and still at
100% CPU nearly two hours later. The 15s deadline #18239 added bounds
Orca's semaphore slot and its promise; it does not bound the CPU burn.
Route both CLIs through `execFileCaptureToTermination`, the primitive
git's barrier path already uses: POSIX children spawn `detached`, the
deadline signals `-pgid` and escalates to SIGKILL, and the promise waits
for verified termination. Windows behaviour is unchanged (`taskkill /t`
either way).
Switching primitives also swapped execFile's hard maxBuffer failure for
`runProcess`'s silent clipping, which would have turned an oversized gh
response into a shorter valid-looking one. `ProcessResult` now reports
truncation and the capture rejects on it, restoring the old contract and
closing the same latent gap on git's barrier path.
* fix(wsl): name an explicit Windows cwd for wsl.exe spawns
Removing the worktree Orca was launched from broke every wsl.exe spawn for
the rest of the session. The WSL command builders passed `cwd: undefined`
meaning "the directory is inside the command" -- but CreateProcessW reads
NULL as "inherit the parent's", and the parent's was a \\wsl.localhost path
Linux had just deleted.
Fixes#16463
* fix(wsl): name the spawn directory at the six remaining wsl.exe sites
The first commit fixed the WSL command builders. Six spawn sites were left
inheriting the process cwd, which is the same deletable `\\wsl.localhost`
worktree: `wsl-availability` (both probes), the WSL filesystem watcher, the
agent-hook relay launch, the UNC delete, and the local worktree filesystem.
`wsl-availability` is the one that matters most, and it turns the bug into a
latching false negative. `isRetryableWslProbeFailure` returns false for ENOENT,
so a spawn that failed only because the inherited cwd was gone is cached as
"WSL is not installed" on the 10-minute definitive TTL with exponential
backoff up to 30 minutes. Git keeps working and Orca reports WSL unavailable --
worse than the bug being fixed.
ENOENT stays non-retryable. It is answer-shaped for the reason it is meant to
be -- wsl.exe is not on PATH -- and naming the directory is what removes the
one cause that was not. Making it retryable would instead re-probe every
non-WSL Windows machine on the short window, and would leave the false ENOENT
in place for the other five sites, which have no cache to correct.
Three of these are also on the `runWslProcess` W3 migration allowlist; this is
the interim until they move, and matches what #17837 does inside the runner.
On Windows with no host `glab.exe`, the cwd-less `glab auth status` known-hosts
probe fell through to `wsl.exe -d <default distro>`. Probe failures are never
cached, so when that WSL leg also fails (glab absent or logged out inside the
distro) every forge detection re-booted the distro; when it succeeds it cached
that distro's auth hosts under the 'native' execution key, which the comment two
lines above the call already forbids. gitlab-auth-and-rate-limit.ts already
passes allowDefaultWslFallback: false for this exact command; the known-hosts
probe now agrees with it.
Connection-keyed probes keep the fallback: glab has no SSH/relay dispatch, so the
`glab api` calls this gates run the same local CLI with no cwd and would
otherwise disagree with the probe. wsl:<distro>-keyed probes were already
unreachable by the fallback, so passing the flag there is inert.
Counted child_process.execFile calls over 3 sequential probes, process.platform
forced to 'win32', host glab mocked ENOENT (wsl.exe / glab.exe spawns):
native key, glab absent in the distro too: 3/3 -> 0/3
native key, glab logged in in the distro: 1/1 -> 0/3, and that distro's
self-hosted hosts stop reaching the native known-hosts list
connection key: 1/1 -> 1/1, unchanged
Residual risk on that second config: a repo on a \\wsl$\ UNC path can be keyed
'native' (no project runtime match) while its own glab calls still route into
WSL by cwd, so it loses the seeded host and must re-derive it through
`glab auth status --hostname`. A co-resident Windows-path repo on the same host
can now write a shared `native\0<host>` unauthenticated negative that stalls that
recovery for one NEGATIVE_ENTRY_TTL_MS window. Fixing that properly means keying
the cache by the host that actually served the call, which needs an exec-layer
API change and is deliberately out of scope here.
Also splits the getGlabKnownHosts suite out of gl-utils.test.ts (796 counted
lines against the 800-line cap for tests) into gitlab-known-host-probe.test.ts.
* perf(git): bound git subprocess execution with an atomic admission scheduler
Field traces (#16038, #11363) show Windows freeze storms driven by unbounded
concurrent git children (12+ at once, 50-65s status convoys for 25+ minutes).
Admit every main-process git child against atomic per-budget base+headroom
counters (general / network / per-route), with reserved interactive capacity,
ordering-only aging, close-bound permit release, a 120s fail-safe read timeout
that feeds scheduler backoff, tier plumbing through every option carrier, and
coalesced+jittered visibility pollers. Killswitch: ORCA_GIT_ADMISSION_DISABLED=1.
Storm harness A/B: max concurrent children 65 -> 6, interactive p95 791ms -> 88ms;
output-parity battery byte-identical with admission on vs off.
* test(git): run the admission output-parity battery on every platform
Parity needs real git, not the storm harness's PATH stub, so it must not share
that file's POSIX gate - Windows is the platform where parity evidence matters.
* fix(git): preserve interactive admission invariants
* perf(git): keep admission queue drains linear
* fix(git): close final admission gaps
* perf(git): bound eligible route selection
* fix(merge): remove unrelated stale snapshot changes
* fix(git): preserve refresh lifecycle authority
* test(git): align admission lifetime contracts
* fix(git): harden admission across runtime paths
* fix(git): restore freshness for bulk status reads
* test(git): repoint delete-dialog source pins after admission plumbing
The hydration effect now orders its targets through
orderDeleteWorktreeStatusHydrationTargets and passes includeLineStats
alongside the abort signal, so both literal anchors stopped matching.
The invariants are unchanged and still pinned: dropping the signal, the
main-worktree/folder filter, or getState-instead-of-subscribe each
still reddens this test.
* Fix git admission tier propagation and lock ordering
Decode optional Git status tiers permissively and default runtime RPC status reads to the status lane while preserving renderer caller intent.
Acquire the FETCH_HEAD mutex before atomic admission so same-repository fetch waiters hold no global or route permits.
Preserve automatic pull-request refresh reasons, keep explicit hosted-review refreshes interactive, remove the dead candidate tier, and keep relay scheduling unchanged.
Use tier-aware status lease keys because a shared lease cannot be safely promoted after its admission request is queued or granted.
* test: align expectations with admission plumbing
* refactor(child-process): move the process contract types to process-spec
run-process.ts crossed its line cap after gaining the termination observer;
the public types and defaults move out with re-exports so no caller changes.
* chore: restore pnpm-lock.yaml to main (unintended local drift)
---------
Co-authored-by: Merge Sim <sim@local>
* Fix draft review sidebar actions
* Drop unused React import in draft actions test
The automatic JSX runtime makes the default React import dead, and
tsconfig.tc.web.json failed the branch on TS6133.
* Add localization keys for draft review actions
The new Ready for review controls introduced five untranslated keys and
the static analysis job requires them present in en.json.
* Name the draft action for what it does
The button read 'Ready for review', which states a status rather than an
action, directly under a header already showing the PR state. The i18n
key (markReady), the in-flight label ('Marking ready...') and the success
toast ('marked ready for review') all already used the verb.
* rm unused files
* rm unused files
* fix(task-page): clean readiness lint findings
* Add GitLab IPC timeout wrapper and improve error handling
- Extract GitLab timeout logic into reusable `withGitLabIpcTimeout` wrapper to protect all GitLab API calls from hanging indefinitely
- Apply timeout protection to all GitLab list and fetch operations
- Add error handling for GitHub and Linear issue creation operations
- Fix event bubbling in GitHub work item row to prevent nested button clicks from opening detail page
- Remove unused `usePRReviewCellState` hook
- Consolidate redundant imports
* refactor(task-page): extract components and improve provider handling
- Add glab timeout handling (30s) to prevent IPC thread blocking
- Extract GitHub assignee/review components to dedicated files
- Improve GitLab work item row keying (repoId:id) and keyboard event handling
- Add context-aware error handling for Jira creation failures
- Refactor GitHubAssigneeAvatar to use shared GitHubUserAvatar component
* Add timeout support and error handling for GitLab operations
- Admission control times out queued work after 30s to prevent
indefinite queueing behind saturated operations
- Mutation errors now display to users via toast instead of failing
silently
* Consolidate workspace attachment labeling into unified utility
Extract common label-generation logic from GitHub and Linear
work-item components into a single getWorktreeAttachmentLabel
function, removing duplication across attachment types.
* Improve TaskPage accessibility, i18n coverage, and error handling
- Add missing aria-labels, roles, and semantic attributes for improved screen reader support
- Extract hardcoded UI strings into i18n system with translate() calls
- Add error handling and proper abort signal support for async operations
- Use locale-aware date formatting throughout
- Fix pagination disabled state and reviewer suggestion merging logic
- Improve async state management with proper refs and effects
- Add Textarea component import for Jira dialog
* Improve TaskPage accessibility and i18n key naming
- Add DialogTitle/Description with i18n to Linear issue dialog
- Use useId to improve aria-labelledby in GitHub selectors
- Replace hash-based i18n keys with semantic names
- Use Object.hasOwn instead of `in` for safer filter checks
- Fix PR review cell to clear input only on success
* Add missing dependencies to TaskPage hooks and useCallback/useEffect arr
Fixes exhaustive-deps warnings by adding missing setters, refs, and computed
values to dependency arrays. Refactors GitHub and Linear issue state handling
to compute values from pageData where available, with fallback to local state.
Moves imperative ref updates into useEffect to properly track dependencies.
* Fix TaskPage ref timing and null repo selection state
Treat null newIssueRepoId as a valid selection, and use useLayoutEffect to synchronize the provider context ref before paint rather than after.
* Extract Linear issue dialog components and fix popover scroll styling
- Consolidate scroll styling: apply popover-scroll-content and scrollbar-sleek classes to PopoverContent wrappers
- Remove redundant max-h-60 overflow-y-auto styles from inner picker divs
- Fix GitHub new issue repo selection to explicitly target first selected repo on fresh mount
- Correct CacheEntry import paths from store/slices/github to store/github/cache-model
- Update tests to reference extracted dialog components instead of TaskPage.tsx
* Improve GitHub task page i18n and fix issue creation edge cases
- Add i18n support to GitHub work item aria-labels (draft PR, PR, issue)
- Optimize work item row by extracting repeated source context call
- Add safety check to prevent opening detail page when issue URL is missing
- Fix dependency reference in detail opener hook
- Extend GitLab job trace timeouts (60s backend, 65s frontend) for slow logs
* Increase GitLab job trace fetch timeouts
Job traces can outlive the runner's 30-second default timeout.
Extend fetch operations to allow 60–65 seconds to complete.
* Verify sourceContext variable extraction in github row test
Update expectations to check that sourceContext is assigned to a
variable rather than called inline, matching the refactored component
implementation.
The GitLab, GitHub, Jira and Linear integration modules, their two IPC
registrars, and the shared GitHub project types each carried a file-level
`eslint-disable max-lines` and ran 351-614 counted lines against a 300-line
budget. AGENTS.md calls for splitting rather than suppressing, and
config/max-lines-baseline.txt is a shrink-only ratchet, so this removes all
eight suppressions and prunes their entries (341 -> 333).
Pure move, no behavior change. Each client is cut along the seam it already
had: per-operation modules for the issue APIs (create / update / comment /
field options), and for Jira the request queue, site credential store,
authenticated request, and site identity. The two IPC registrars keep their own
handlers and delegate the rest to per-domain sub-registrars, so they remain
real entry points rather than re-export shims.
The IPC surface is proved intact rather than assumed: comparing (method,
channel) multisets between HEAD and the split gives 52 registrations across 52
distinct channels on both sides.
Provider-neutrality is preserved -- GitLab and GitHub keep separate, parallel
module layouts rather than being merged behind a shared abstraction.
Verified: oxlint clean, ratchet passes, typecheck clean, full unit suite green
(the one remaining failure is a pre-existing load flake in an untouched file,
green when re-run serially), no new runtime import cycles among 744 modules,
and no lint suppression added anywhere.
* refactor(tests): split oversized test files off the max-lines suppression list
Every `*.test.ts`/`*.spec.ts` that carried an `eslint/oxlint-disable max-lines`
directive is now split into focused, behavior-scoped suites that fit the 800-line
test budget, with shared setup extracted into co-located `*-test-harness.ts` /
`*-test-fixtures.ts` modules (300-line budget). 83 files became ~930; the largest
output is 797 effective lines. `orca-runtime.test.ts` is intentionally untouched.
Test bodies were moved by scripted line-range slicing rather than retyped, so
assertions are byte-identical. The only permitted body edits were mechanical
rebinding where a shared value moved into a harness (e.g. `tmpHome` ->
`homes.tmpHome`).
Registries that enumerate test files were updated in lockstep:
- config/max-lines-baseline.txt: pruned 341 -> 258 entries (all 83 removed).
- config/reliability-gates.jsonc: 33 gates repointed at the split files, with
assertionRefs split per file where a gate's coverage now spans several.
- .github/workflows/pr.yml: the real-zsh lane now lists the 4 split files that
actually exercise zsh, so they keep running in the dedicated shell lane.
Also renamed agent-hooks `server-test-fixtures.ts` to `server.test-fixtures.ts`
so the global-fetch call-site audit keeps skipping it, and added `.js` extensions
to the CLI suites' dynamic harness imports (node16 resolution) to unbreak
`build:cli`.
Verification: full suite 52,449 passing vs 52,448 at baseline with zero
assertions lost; `pnpm lint`, `pnpm typecheck`, and `pnpm build:cli` all exit 0;
the terminal-pane e2e spec runs 31/31 headless.
* refactor(tests): split hook-idle arbitration suite that oxfmt pushed over budget
The pre-commit oxfmt pass reflowed pty-connection-hook-idle-arbitration.test.ts
to 811 effective lines, 11 over the test budget. Split the hook-completion side
effect and replacement-agent veto cases into their own suite; both files now sit
well under the cap and the 15 tests are unchanged.
* test: port upstream test changes into the split files after rebase
Rebasing onto main surfaced 27 tests that main had added to files this branch
deleted, plus edits to tests that had already moved. Taking the deletion side of
those modify/delete conflicts would have dropped that coverage silently, so each
upstream change is ported into the split file that now owns the behavior — for
example main's six orchestration mailbox tests land across orchestration-runs,
-send, and -check.
Also repoints `orchestration.notification-mailbox-consistency`, a gate main added
after this branch's gate remap, at those same three split files, and re-prunes
the max-lines baseline against main's (257 entries).
Verified: all 27 upstream test titles present; full suite 52,761 passing with the
only diff vs baseline being 12 tests main itself removed and 3 that moved from
skipped to passing; lint and typecheck exit 0.
* fix(test): flush pending continuations before tearing down terminal test globals
CI shard 5/16 failed on both Node 24 and 26 with `ReferenceError: window is not
defined` from pty-connection.ts, surfacing through
pty-connection-daemon-snapshot-replay.test.ts.
The reattach/settle chains `await` a real promise and then touch `window.api`.
Under fake timers those continuations cannot run, so they only become schedulable
once restoreTerminalTestGlobals() switches back to real timers — which previously
happened immediately before `delete globalThis.window`, so a late continuation
threw and failed the whole file. Flush async ticks in that window instead.
This is latent in the source rather than new: the pre-split 25k-line file kept
running other tests after these, which gave the chains time to settle before
teardown. Splitting the file moved teardown directly behind them.
* fix(test): keep an inert window after terminal test teardown instead of deleting it
The async-tick flush was not enough: the reattach/settle chain can resolve after
teardown regardless of how long we drain, so CI shard 5/16 still failed with
`ReferenceError: window is not defined` from pty-connection.ts.
A real renderer never loses `window`, so deleting it was the artificial part.
Swap in an inert proxy whose properties resolve to callables and whose calls
resolve to undefined, making a late `window.api.pty.*` call a harmless no-op.
The next test replaces it wholesale via installTerminalTestGlobals(), and no test
asserts that `window` is absent.
#14397 split `shared/types.ts` into 46 per-domain modules but kept the path as
a re-export barrel so the import sites did not have to change. This removes
the barrel: every consumer now imports from the module that actually declares
the type, and `src/shared/types.ts` is deleted.
Barrels hide where a type lives, make every consumer look like it depends on
the whole domain, and let an unrelated edit invalidate a module that ~2,000
files transitively import.
2,323 import declarations across 2,321 files. Rewritten mechanically: each
specifier was resolved to an absolute path via the TypeScript AST and
recomputed, rather than string-substituted, so alias forms (`@/../../shared/
types`) and per-specifier `type` modifiers survive.
Four cases the mechanical pass had to handle, each found by a gate rather than
by reading the diff:
- Modules inside `src/shared` import the barrel as `./types`, not
`shared/types`. A pre-filter on the latter string skipped 176 of them and
left imports dangling at a deleted file, which surfaced as confusing
`Property 'x' is optional in type 'Repo' but required in Pick<Repo, ...>`
errors rather than "module not found".
- The barrel RENAMED one type on the way through
(`WorkspaceSource as WorkspaceCreateTelemetrySource`), so the original name
in the owning module has to be re-aliased at each consumer.
- Three test files put `;(globalThis as ...)` on the line after the import.
TypeScript parses that `;` as the import statement's terminator, so
replacing through `statement.getEnd()` deletes it and breaks ASI. The
rewrite now stops at the module specifier.
- A file that already imported directly from a module got a SECOND import
from it, because the barrel re-exported those same names — which trips
`import/no-duplicates` under `--deny-warnings`. A post-pass merges
declarations sharing a specifier and type-only-ness; the `import type` plus
`import` pair from one module is left alone, since that form is allowed.
Splitting one barrel import into several genuinely adds lines, which pushed
`terminal-layout-pty-ownership.ts` to 301 counted lines: its 107-character
import must wrap, and neither local type collapses onto one line (101 and 116
characters). Rather than contort a type declaration to fit a line budget,
`collectLeafIds` and `pruneLeaves` move to `terminal-pane-layout-tree.ts` —
they are pure structural operations on the layout tree and independent of PTY
ownership. `visible-worktrees.ts` similarly loses its own mini-barrel
re-export of `isDefaultBranchWorkspace`, with the four real consumers
repointed at the declaring module. No `max-lines` bypass added.
Verified: cold `tsc --noEmit` green on node, cli, and web (buildinfo deleted
first — these projects are `composite: true` and reuse stale caches); the full
`pnpm lint` green, not just bare oxlint — the narrower local check is what let
the duplicate imports reach CI; max-lines ratchet OK at 344.
* fix(gitlab): guard against non-array API responses in MR/issue listing
fetchIssuesAsWorkItems and listMergeRequests parsed glab's JSON output
and called .map straight on it. When the GitLab API returns a JSON
object instead of an array (error body, unexpected shape) on a
successful exit, this crashed with a bare TypeError that got
misclassified as "Failed to load issues: JSON.parse(...).map is not
a function" instead of a useful message.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(gitlab): cover listIssues and keep payloads out of error classification
The guard missed listIssues in issues.ts — the RPC-backed issue list that
produces the reported "Failed to load issues: JSON.parse(...).map is not a
function". Hoist the guard into glab-api-response.ts so both files share it.
The thrown message is fed to classifyGlabError, which substring-matches it.
A response payload is content, not a diagnostic: an MR titled "fix network
timeout" classified as network_error and the canned copy replaced the payload
the user needed. Report a GitLab error envelope by its own message, and mark
an opaque body so classification is skipped.
* test(gitlab): make the list-guard tests fail on the regressions they name
Two assertions were vacuous under mutation. The envelope test used a "403
Forbidden" message whose keyword matches earlier in the classifier chain than
its sibling payload, so leaking the payload into classification still passed;
it now uses a 404 envelope beside a "403 forbidden" sibling. No call-site test
carried a classifier keyword, so deleting the marker-error branch entirely
failed only one unit test; the MR API path now uses a keyword-bearing body.
Also give the non-list branch the same "Failed to load issues" prefix as every
other list error, cover the `{ error }` envelope field, and pin the thrown type.
* test(gitlab): pin the reported-payload bound
Removing the 300-char slice survived the whole suite, and the banner's
break-words now depends on it. Name the limit and assert both branches
truncate, plus the envelope falling through a blank message to `error`.
* test(gitlab): pin message-over-error envelope precedence
Swapping the lookup order passed the whole suite. Anchor the bound regex too
so it cannot match an incidental ": " near the end of a message.
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
* test(repro): demonstrate #7732 GitLab pipeline job details never load in Checks panel
Co-authored-by: Orca <help@stably.ai>
* fix(gitlab): load pipeline job traces in the Checks side panel (#7732)
Expanding a GitLab pipeline job in the Checks panel always showed
"No inline details are available for this check.": the mapper dropped the
numeric job id, `PRCheckDetail` had nowhere to carry it, and every consumer
called the GitHub check-runs API, which returns null for a GitLab job.
- carry `gitlabJobId` on `PRCheckDetail` and add the `gitlab-job:` branch to
all three identity ladders (panel rows, editor tabs, fix-prompt keys) so
same-stage jobs with no web_url stop colliding
- add a runtime-routed trace client so SSH/remote workspaces work, not just
local IPC, and thread the MR's `projectRef` for fork pipelines
- bound the trace in main via the existing `sliceCheckLogTail` (now shared,
not GitHub-only) so a multi-megabyte CI log never crosses the 1 MB
transport frame cap; strip ANSI/section markers up to the CR only, which
keeps each section's visible header and command echo
- render the excerpt inline instead of "Log tail available in full details."
- feed GitLab traces to "Fix with AI", which previously sent bare check names
- skip the fetch for jobs that cannot have a trace (created/manual/skipped)
so GitLab's 404 does not replace the benign empty state, and re-arm a
failed load when the job's state changes since the panel has no retry
Co-authored-by: Orca <help@stably.ai>
* fix(gitlab): treat a missing job log as an empty log, not an error (#7732)
Round-1 review follow-up.
- a job canceled before it started (or whose log was erased/expired) is
`completed`/`cancelled`, so the panel fetched its trace, GitLab answered 404,
and `classifyGlabError`'s issue-edit copy ("Issue not found — it may have been
deleted.") landed verbatim on the auto-expanded check row; main now maps that
404 to an empty trace so the row keeps its benign empty state
- keep a missing project a real error (GitLab masks unauthorized projects as
404) and add `classifyJobLogError` so 403/unknown failures stop borrowing
issue-edit wording on a job-log read
- broaden the empty-log copy in all five catalogs: it now covers erased and
expired logs, not only jobs that never ran
- e2e: derive the repro screenshot dir from `process.cwd()` (or an env
override) instead of a hardcoded POSIX path to a throwaway worktree
- bound the raw trace before the ANSI/section passes so a multi-megabyte log
is not scanned in full on the main-process event loop
- drop the redundant `if (repo)` in `handleFixChecksWithAI` and the now-dead
"Log tail available in full details." catalog entry
Co-authored-by: Orca <help@stably.ai>
* fix(gitlab): address review — project ref on reload, retry re-arm, IPC timeout
- Carry the MR's GitLab project ref on the check-details tab so reloading a
fork/cross-project job tab fetches the trace from the pipeline's own project.
- Re-arm the sidebar retry when a details load resolves to null, not only when
it throws; a detail-less row otherwise never retried after the job moved on.
- Bound the local `gl.jobTrace` IPC call with the same 30s timeout the runtime
RPC path uses — glab runs without a subprocess timeout in main.
- Document that the trace 404 -> empty-log mapping is deliberately broad.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>
GitLab's project-ref cache stored `null` forever and returned any cached
value straight from the map, so a repo probed before `origin` was configured
— or before `glab auth login` ran for its self-hosted host — kept
hosted-review provider detection stale until app restart. The negative-TTL
work that shipped for Azure DevOps / Bitbucket / Gitea skipped it.
Mirror `createRemoteRefProbeCache`'s semantics: negatives expire on the
shared interval, positives stay, the SSH provider generation joins the cache
signature so a reconnect re-asks, and a probe abandoned as stale can no
longer publish over its successor. Transient git/SSH failures stay uncached.
Expiring negatives would otherwise turn `glab auth status --hostname` into
one spawn per repo per interval on the hosted-review poll, since a non-GitLab
remote reaches it too, so remember the unauthenticated answer per host — not
per repo — on the same clock.
* fix(gitlab): count diff lines whose content begins with -- or ++
countDiffLines skipped every line starting with ---/+++ as a file header,
but a removed line whose original text began with -- (SQL/Lua/Haskell
`-- comment`) becomes a diff line `---<content>`, colliding with the
`--- a/file` header — so its deletion was silently dropped from the
+N/-N shown in the GitLab MR dialog. Same collision for an added line
whose content began with ++ (+++ flag).
Track hunk state: ---/+++ are file headers only before the first @@;
inside a hunk every +/- is content, matching the unified-diff rule git
itself uses to disambiguate headers from content.
Co-authored-by: Claude <noreply@anthropic.com>
* fix(gitlab): validate countDiffLines with actual diff format
GitLab's /diffs endpoint returns json_safe_diff starting at @@ without
file headers. Add comprehensive test coverage validating the collision
fix correctly handles this format: content lines beginning with -- or ++
are counted as additions/deletions.
Tests cover binary files, empty diffs, no-newline markers, and content
beginning with @@ or C-style ++. Clarify function contract: requires
hunk headers to distinguish headers from content lines.
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
* chore(dead-code): drop 2k lines of unreachable exports and orphan modules
Ran knip across every build entry (main, preload, renderer, popout, web,
cli, relay, workers, forked sidecars, config scripts) and removed what no
entry graph can reach.
- 11 orphan modules nothing imported, plus one test that only covered them
- 159 unused exports/types, with their now-dead helpers, imports and tests
Each candidate was verified against dynamic references before deletion.
42 knip hits were false positives and are kept: shared modules consumed by
the mobile/ workspace, the src/shared/plugins/** public API, vendored
shadcn primitives, and relay wire-protocol constants held for compatibility.
Adds knip.json + `pnpm audit:dead-code` so this stays measurable.
Verified: pnpm typecheck, pnpm lint, and 2081 tests across the 73 affected
test files all pass.
* chore(dead-code): move knip config under config/
Root-level additions are blocked by the root directory guard.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>
* fix(P1-D): coalesce remote-ref probes, TTL negatives, and bound unsettled keys
Keep forge resolution from stampeding git under worktree fan-out, let
remotes added mid-session be discovered without a restart, and refuse
pathological new-branch waves once the unsettled map is full.
* fix(P1-D): stop abandoned probes publishing, and split capacity refusals
A coalesced probe abandoned as stale kept running and still wrote its answer
to the cache, so a late permanent miss could land over the successor's fresher
one. Probes now publish only while they still own the in-flight key.
The hosted-review capacity refusal told brand-new branches that an earlier
attempt of their own never answered when the refusal was really the unsettled
map or the process-wide detached cap; each cap now says what it is.
Also caches stable "no such remote" SSH misses under the negative TTL instead
of re-spawning the probe on every poll.
Co-authored-by: Orca <help@stably.ai>
* Bound SSH remote URL probe with deadline to prevent hangs
The SSH branch of remote URL probes was unbounded — the relay's bounds
are per-phase and reset on every frame, so a relay dribbling output would
outlive them. Pass AbortSignal.timeout to the SSH provider's exec call to
enforce the same 30s deadline as local probes.
Treat AbortError as a transient probe error: it signals unavailable
infrastructure (deadline or cancellation), not a negative answer about
the remote.
---------
Co-authored-by: Orca <help@stably.ai>
* fix(P1-D): bound hosted-review lookups with a detachable deadline
The `inflight` map in the hosted-review branch cache was only ever cleared
when the lookup settled, and nothing bounded how long that took. One wedged
provider call pinned its branch for the life of the process: every later poll
joined the same dead promise, so the card loaded forever with no in-session
recovery.
Each lookup now runs under a 120s deadline. Nothing below the funnel can be
cancelled, so the deadline detaches instead: the record is released, the
callers get the last known review (or a timeout error), and the branch enters
the existing failure backoff. The lookup keeps running and its answer is still
adopted if it lands, so a slow-but-alive host converges rather than failing
forever. A token identity keeps a detached lookup from evicting the record
that replaced it, and a wall-clock sweep expires records whose timer never
fired — main's timers are suspended across system sleep. `inflight` is capped
independently of the completed cache.
The failure backoff moves to its own module: it has a different lifetime from
the answer cache and is what a deadline records against.
* fix(P1-D): bound `git remote get-url` on the local/WSL path
`getRemoteUrlForRepo` ran the git child with no timeout, which is the one
unbounded step under the hosted-review lookup funnel: `git/runner.ts` only
arms its kill path when a timeout is passed, so a dead network mount or a
stalled WSL interop hangs the call and everything above it. The SSH branch is
already bounded by the relay mux's 30s request timeout, so it is unchanged.
* rm review doc
* rm review doc
* test(P1-D): add probe tests and transient-failure recovery verification
Add tests for coalesced-probe and remote-url-probe infrastructure. Add integration test verifying that transient Bitbucket API failures don't cache as a definitive no-review result, allowing recovery after cache TTL expiration.
* fix(P1-D): track lookups from start, prevent stale scope adoption
- Count unsettled lookups when they start, not after deadline expires: prevents multiple concurrent lookups for the same branch.
- Add evicted generation floor: prevents adopting stale results when scope is invalidated and evicted from the map.
- Consolidate duplicate repository reference cache logic into createRemoteRefProbeCache utility.
- Fix deadline wrapper in git config signature lookup: bound the caller's deadline only, not the coalesced probe itself.
* feat(P1-D): add remote-ref-probe-cache utility
Cache successful remote URL probes per repo/runtime to avoid duplicate work.
Skip caching transient errors and SSH failures so providers can retry on
reconnect, preventing stale scope adoption during the session.
* fix(checks): stop skipped and manual checks reporting as failures
Route every check-classification surface through one shared helper so
desktop renderer, desktop main and mobile agree on the same verdict.
- GitLab `manual` jobs and pipelines are neutral again, not action_required/failure
- `skipped` counts as passed everywhere, including mobile
- a neutral check no longer demotes a summary that has passing checks
* fix(checks): move the check-classification parity test into the renderer project
The parity table lived in src/shared but imported a renderer module, and both
config/tsconfig.node.json and config/tsconfig.cli.json are composite projects
that include src/shared without that renderer path, so `pnpm typecheck` failed
with TS6307 on two of its three projects. Only the web project spans both trees.
Co-authored-by: Orca <help@stably.ai>
* fix(checks): stop the Tasks-grid pill contradicting its own verdict
The checks pill's label, tone and icon all read one ProviderCheckSummary, but
getChecksLabel short-circuited on the raw `neutral` counter while the tone and
icon key off `state`. After the classification fix a PR with 19 success + 1
neutral renders an emerald CheckCircle2 pill that reads "1 unresolved", and
mobile's own label (which keys off `state`) reads "19/20 passed" for the same
summary.
Move the label into src/shared/provider-check-summary.ts so desktop and mobile
cannot fork it again, and key it off `state`.
Also covers deriveWorkItemCheckSummary, the desktop-main producer of the summary
that reaches the Tasks grid and the relay-paired mobile client. It was rewritten
here with no test at all; the parity table stands in derivePRCheckStatusFromRollup,
which is a different normalizer. The new main-process test drives getWorkItem with
a real statusCheckRollup fixture, pinning the StatusContext `state` fallback that
would otherwise be deletable with the whole suite still green.
Co-authored-by: Orca <help@stably.ai>
* fix(gitlab): route the pipeline job-array rollup through the shared check classifier
The array path in derivePipelineStatus kept its own copy of the rollup rules, so
manual-only read green and one unrecognized job status demoted a passing pipeline
to neutral — both disagreeing with every other check surface.
Also retry the packaged-CLI smoke temp cleanup on Windows: the copied Orca.exe can
still be locked by AV/indexers after every assertion passed, failing the package job.
Co-authored-by: Orca <help@stably.ai>
* fix(gitlab): stop the skipped pipeline string diverging from the Checks tab
- classifyPipelineString now counts a skipped pipeline as passing, matching
the per-check classifier; canceled stays neutral and is pinned as an
explicit, sign-off-pending divergence.
- Pin the production string path (head_pipeline.status) in the parity table
and note that the job-array branch has no production caller yet.
- Count skipped checks in the Checks panel's passing header so it agrees
with the checks pill.
- Correct the packaged-CLI smoke retry comment: the EBUSY is the smoke's own
just-exited Electron process, not AV/indexers.
Co-authored-by: Orca <help@stably.ai>
* fix(checks): finish cross-surface check parity and back out the skipped MR-card flip
Review follow-ups on the check-classification PR.
- PullRequestPage and GitHubItemDialog kept private copies of getCheckCounts /
getChecksSummaryLabel that still counted only `success` as passing, so a
2-success/3-skipped PR read "2 passing · 3 skipped" there and "5 passing" in
the sidebar. Both copies move to pr-check-counts.ts, which routes the passing
bucket through classifyCheckOutcome; action_required keeps its own amber
bucket. The summary icon now keys off passing count, so an all-neutral PR
stops painting a green tick above "0 of N checks passing".
- The sidebar checks header and triage strip still called
`{status: completed, conclusion: null}` pending, contradicting the grey
"Unresolved checks" pill. Both now read summarizeProviderChecks and render an
unresolved chip/strip instead of an amber spinner that can never resolve.
- classifyPipelineString('skipped') is reverted to neutral. That flip painted
MR cards green for pipelines that never ran, on the only GitLab path with
production callers, and contradicted the same function's deferral of
`canceled`. Both tone changes stay deferred, pinned by one test.
- classifyPipelineString('manual') resolves to pending rather than neutral: a
blocked pipeline is outstanding, and neutral let the worktree card fall
through to its emerald `open` default while GitLab still refuses the merge.
- TaskPage's checks pill helpers move to task-page-checks-pill.ts so the
"1 unresolved on a green pill" fix is actually pinned by a test.
- smoke-packaged-cli no longer lets an EBUSY cleanup replace the real failure.
* fix(checks): stop completed unknown checks from spinning
---------
Co-authored-by: Orca <help@stably.ai>
* fix(checks): rank successful checks above skipped and neutral
Checks were ordered with `skipped` (4) and `neutral` (3) ahead of
`success` (5), so a PR with a long tail of skipped jobs pushed every
passing check below the fold — you scroll past a wall of "Skipped" to
find out whether anything actually ran.
Rank the no-signal conclusions last (`success` 3, `neutral` 4, `skipped`
5) and pull the order out of its three duplicated copies
(checks-panel-content, PullRequestPage, GitHubItemDialog) into
`src/shared/pr-check-severity-order.ts`. Unknown conclusions now sink to
the bottom instead of silently ranking as `neutral`.
* fix(checks): look up check ranks through a Map, not an object literal
An object-literal rank table resolves `constructor`, `toString`, and
`__proto__` off Object.prototype, so those keys returned a function
instead of falling through to UNKNOWN_CHECK_RANK — the comparator then
subtracted functions, went NaN, and left the list in arbitrary order.
Conclusions come from provider payloads, so keep the lookup on a Map and
cover prototype property names in the test.
* test(checks): cover provider-neutral ordering states
* fix(checks): preserve actionable provider states
* fix(checks): preserve unresolved provider rollups
* fix(checks): keep unknown GitLab rollups neutral
* fix: preserve neutral review check summaries
* fix: complete provider-neutral check ordering remediation
* fix: use provider-neutral mobile review status input
* fix: hydrate GitLab mobile review status
* fix: type mobile GitLab review hydration
---------
Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Brennan Bensonandyasuhiro nishioka
Neil
OrcaWinandOrcaWin
Žan MlakarandNeil
Seiji KoharaandNeil
Naveen ChatlapalliandNeil
Kelvin Amoaba
Jinjing
Wooseong KimandNeil
BingZandNeil
Pongsakorn PaetrakulandJinwoo-H
KyuJoo HanandOrcaWin
余辉andOrcaWin