mirror of
https://github.com/stablyai/orca.git
synced 2026-10-03 08:02:12 +00:00
6c4625d7ff178badefa38527c3d6e69ea5c8e88e
8
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo Hong
|
4b876758d3 |
refactor(mobile): checked reply readers for the session domain (step 7) (#21089)
* test(mobile): record main's session reply behaviour at every unrecorded read site Step 7 for the session domain changes how 51 RPC readers read a *malformed* reply. Eleven of the session read sites had no recording family, so main's answer to a malformed reply at those sites was undocumented and the reader change would have had nothing to move. This commit is the before picture, taken from main's own tree with no product edit in it. Ten new families, twelve scenarios, twenty-five goldens: - `session.review-file-diff` / `session.review-branch-diff` — `git.diff` and `git.branchDiff` read through the review projection, which the Changes screen's verbatim readers do not cover. - `session.review-git-mutations` — the single-file `git.stage` / `git.discard` and the bulk stage sweep's second `git.stage`. - `session.review-send-sheet` — `session.tabs.list` read for the agent terminals the send sheet lists, the third reader on that method. Needs an `open-send-sheet` action on the review-action adapter, which re-digests that family's eight goldens on `adapterSha256` and nothing else. - `session.browser-tab-create` — `browser.tabCreate`. - `agentSession.structured-create` — `agentSession.create`, whose family base only ever covered the support probe. - `session.tab-rename` / `session.tab-close-session` — `terminal.rename` and `session.tabs.close`. - `settings.new-tab-local-agents` — `preflight.detectAgents`, the arm the new-tab loader takes for a workspace with no connection. `baseline` is repinned to main's tip because two commits (#20659, #21004) touched a fenced path after the pilot's pin, so `--record` refuses on main's own tree until it moves. The repin is what rewrites `baseline` on all 705 existing goldens; nothing else about them moves. Decoded against origin/main through the value pool: 705 header-only (`baseline` on every one, `adapterSha256` on the eight review-action goldens), 0 body-moved, 25 added, 0 deleted. Not covered, with the reason: the chunked clipboard upload's `appendImageUploadChunk`, `commitImageUpload` and `abortImageUpload` cannot be matrixed, because `replyMatrixSites` takes every completion in the base scenario and the chain's later params carry the `uploadId` the start reply named. Driving `clipboard.startImageUpload#1` therefore makes main send an append whose params no scripted step matches, and the recorder raises `Request params mismatch: clipboard.appendImageUploadChunk#1` instead of recording. The two families were written, probed and removed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): checked reply readers for the session domain (step 7) Fifty-one unchecked reply readers across nine files become checked zod readers, so a malformed host reply surfaces as one readable error at the operation boundary instead of a downstream `TypeError`, a rendered `undefined`, or a screen left ready over garbage. Deliberately a behaviour change on malformed replies only. Eight schema modules, one per reply family, each recording the consumer line behind every requirement and the host handler that publishes it: - `clipboard-image-reply-schema.ts` — the upload slot's `uploadId`, the commit and single-frame path strings, and the two legs whose body nothing reads. - `github-pr-mutation-reply-schema.ts` — the `{ ok, error }` status envelope as two variants, and the bare-boolean confirmation. - `github-pr-entity-reply-schema.ts` / `github-pr-read-reply-schema.ts` — the seven PR sidebar reads. Every identity requirement the hand parsers had is kept, so a payload that degraded to null still degrades to null; what changes is a payload that is not the declared container at all. - `diff-review-reply-schema.ts` — the normalized branch compare, the review notes on the worktree record, the three file-diff arms, and the file-level git mutations. - `review-terminal-reply-schema.ts`, `session-launch-reply-schema.ts`, `session-read-reply-schema.ts`, `session-write-reply-schema.ts` — the review send sheet, the launch paths, the session screen's reads and its writes. Requirements are exactly the members a consumer reads unguarded, everything else is a salvaged optional with main's own default applied in the transform, and no schema is `.strict()`: a member a newer host adds passes through untouched. Enum arm sets that a reader compares against pass through or degrade to the arm the reader handles most conservatively; the two closed sets — the committed change status and the diff kind — are closed because main *dropped* an arm it did not know rather than passing it through, and degrading them would draw a row or render a diff main never did. No member is coerced on the way back to the host. `github-pr-parsers.ts`, `github-pr-comment-parsers.ts` and `github-pr-value-readers.ts` are gone; their suite is now the parity record for the schemas that replaced them, with the four cases that refuse rather than degrade marked as such. Twelve call-site casts are deleted, and three dead "response was invalid" branches with them: the reader refuses those replies now, so the error names its method. The nine session files come off `unchecked-rpc-reader-inventory.ts` entirely rather than being lowered. `git show --stat` on this commit touches nothing under `mobile/rpc-foundation`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): unit-pin every session reply schema's decision Three kinds of case, one per kind of decision the schemas encode: a member a consumer reads unguarded is required and its absence refuses, an arm set a reader compares against degrades to the arm that reader handles most conservatively, and a reply whose arms need different members is declared as variants and each arm is read. The last suite is the wire-compatibility claim: a member no reader knows passes straight through, on the markdown document, the upload slot and the terminal inventory alike, so a newer host is never refused for a field mobile does not read. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh the corpus for the session domain's checked readers Repins `baseline` to the last commit touching a fenced path and re-records all 730 goldens, which is the disclosed behaviour change taken as an observation. Decoded through the value pool against the pre-refactor tree on this branch: 688 header-only with `baseline` the only key that moved, 42 body-moved, 0 added, 0 deleted. The 42 are seven named scenarios and thirty-five matrix goldens, and every moved checkpoint's own reply is malformed or refused. Three `normal` partitions appear in the list and none of them reads a well-formed reply differently: the review file-diff family's base scenario drives three legs and its third is scripted `{ kind: 'unknown' }`, so that leg's checkpoint moves in every variant, the varied leg included. The same append-only-history effect puts `pr-read-upstream-error`'s `no-pr` checkpoint in the list for the malformed PR recorded before it. What the corpus now records, in one sentence: a property read on null, a V8 destructuring message shown to the user, and four hand-written "response was invalid" strings are replaced by one message that names the method, and four screens that published a malformed payload as ready state now show an error instead. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): split the expanded check run out of the PR read schemas `github-pr-read-reply-schema.ts` was 328 code lines against the 300-line cap. The expanded check run and the annotations, jobs and steps listed under it are one reply with no reader in common with the other six, so they move to `github-pr-check-reply-schema.ts` whole. A move, not an edit: no schema changes and no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the branch's last fenced-path commit The schema-module split touched `mobile/src`, so `--record` refuses on the pin the previous refresh left behind. Repins to that commit and re-records. Decoded against the previous corpus: 730 header-only with `baseline` the only key that moved, 0 body-moved, 0 added, 0 deleted — the split is a move, and the corpus says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the worktree display-name cast's type import The live-title read is typed by its schema now, so the cast it annotated is gone and the import it needed with it. oxlint flags the leftover. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the branch tip The unused-import removal touched a fenced path, so the pin moves with it. Decoded against the previous corpus: 730 header-only on `baseline` alone, 0 body-moved, 0 added, 0 deleted. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): contain a refused prChecks reply to the checks section The checks read was the one phase-1 dependency that could take the whole PR sidebar down. `loadPrSidebarData` routed `!checksOutcome.ok` through `failureState`, so a host whose `github.prChecks` shape drifted cost the user the title, body, comments, reviewers and merge controls — everything they opened the sidebar for — over a section that renders a row of icons. Main never noticed because its unchecked reader answered `[]` for the same reply; this branch's reader refuses it, which is correct, and which is what makes the containment necessary. Contained the way phase 2 already is: a failed read keeps `kind: 'ready'`, empties `checks`, and carries the message in a new `checksError` so the checks section can say what happened. The sidebar can no longer reach `error` or `blocked` on the checks read alone. Also pins the enum departure this PR makes deliberately. The degrading arm sets go through `salvagedOptional(name, z.enum(...))` rather than `openEnum` because `openEnum` refuses a non-string where main mapped it to the conservative arm; nothing held that, and all 2477 tests stayed green against the swap. Six cases now hold both halves: a non-string degrades on the three open sets, and an unknown arm drops the row on the closed ones. Four deletions the reviewer found: a reaction-token alias with no importers, the `errorType`/`fetchedAt` the branch-lookup reader fabricated to satisfy a type whose only consumer reads neither, two bare schema aliases, and a quick-commands pass-through with two callers. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the containment commit `--record` refuses unless the product tree equals `baseline`, so the fix above moves the pin. The corpus re-recorded in place against it: 730 goldens, every one header-only on `baseline`, no observation moved. No observation moved because no family reaches the code the fix changed. The `github.pr-read` family calls the seven wrapper reads directly and records their `{ ok, error }` outcomes; `loadPrSidebarData` sits a layer above that and no scenario mounts it. The prChecks outcome is identical before and after — what changed is what the sidebar does with it — so the unit suite is the only oracle for the containment. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the PR sidebar's checks containment The containment landed with no golden: no scenario mounted `loadPrSidebarData`, so the row in the delta table rested on unit tests alone. `PrSidebarLoadDeps` is five client-taking functions, so a new adapter drives phase 1 directly and records the `PrSidebarState` it resolves to — no React host, and no edit to an existing adapter, so no recorded golden moves. Two scenarios: a normal load, and one whose checks leg answers a shape the reader refuses. The matrix over the base then drives all eleven partitions at `github.prChecks#1`, and every one of them records `ready` with a `checksError` where main took the whole sidebar to `error`. `pr-sidebar-checks-failure-state` is the mutant that routes the refusal back through `failureState`; it moves both `pr-sidebar-checks-refused` and the prChecks matrix golden. Also pins two closed-and-required enum decisions that were free to become defaults — an unknown check-summary state drops the summary block, an unknown reaction content drops the reaction — deletes four exported type aliases and five enum constants with no reader outside their own file, makes `PRChecksSection`'s `checksError` required so a second caller cannot silently lose the message, and stops the header reading "No checks" when the checks were unreadable rather than absent. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the pr-sidebar family commit Six new goldens — two pilots and the four matrix sites the base scenario scripts — and `baseline` on the 730 that already existed. No body moved and no `adapterSha256`: the family is a new adapter module, so nothing recorded through another one re-digests. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the corpus against the merged main Repins `baseline` to the merge commit and re-records all 736 goldens in place. Against `origin/main` the 705 shared goldens move only on `baseline` (672 of them header-only), leaving the same 33 body moves and the same partitions the branch carried before the merge, plus its 31 added goldens. Every body also takes main's recorder shape from #21088: `sent` becomes `ordinal` over one interleaved write counter, subscriptions record a cleanup checkpoint, and a salvaging read now reports a `reply-salvage` effect naming what it dropped. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep an explicit null on the two tri-state PR flags `autoMergeAllowed` and `mergeQueueRequired` carry three answers, not two: `null` is GitHub saying auto-merge is not allowed, `undefined` is the host not carrying the member at all. The readers coalesced the null away, so a well-formed reply read differently from the parsers they replaced, which preserved it explicitly. Both shared types already declare `boolean | null`. No consumer separates the two today — `pull-request-auto-merge-availability` compares with `=== true` and `!== false` — so this is parity, not a visible fix, which is exactly why it needed a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the tri-state flag commit All 736 goldens move on `baseline` alone: no scenario scripts an explicit null on either flag, so preserving it changes no recorded screen. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): check the two session-write readers #21083 brought Step 7 empties the session block of the unchecked-reader inventory, and #21083 landed two readers into it after that: the New Tab create's member read of `tab`, and the display-mode toggle's payload. Converting them is what keeps the claim true — a session line reappearing would mean the domain is not migrated. `created-terminal-tab` requires `tab.id` and `tab.type === 'terminal'`, because the strip keys the new tab on the id and spreads the rest into a union whose arm `type` picks. `terminal`, `title` and `terminalTheme` stay optional behind main's own guards, and unknown members pass through. `terminal-display-mode-set` reads nothing, so it takes the same `z.unknown()` the other five unread writes take. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record over #21083's corpus All 736 goldens this branch already had move on `baseline` alone, and #21083's 22 arrive beside them. One of the 22 moves against main's own recording: `matrix-session.create-terminal-session.tabs.createterminal-1`, where the New Tab create's five malformed partitions read `Cannot read properties of undefined (reading 'tab')` and now read the method's own message. Two of them also stop unsubscribing the terminal the user was watching before the property read threw, so a create that never happened no longer costs the live pane its subscription. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what carries a refused create reply to the catch Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
44268d9616 |
refactor(mobile): send the github.* PR surface and the diff-review loaders through typed RpcOperations (#20668)
* test(mobile): record main's github.* PR and diff-review loaders before migrating them
Scenarios and goldens for the step-4 `src/session/` first half, recorded
against main's unmigrated product code so the migration that follows has a
frozen parity oracle instead of an assertion.
- 20 scenarios over seven new families: the seven `github.*` PR reads, the
twelve PR mutations split by their three reply contracts (`{ok}` envelope,
bare boolean, slug-addressed comment edit), the triage createTerminal+send
launch, the PR branch-context chain and the review screen's three loaders.
- Two new sender-style mount adapters. Both mount exported async functions
taking a client, so no React host is needed and the recorded state is each
wrapper's own outcome.
- 50 new goldens: 20 pilot, 30 reply-matrix sites. `recorderSha256` moved on
all 153 existing goldens because the adapters are in the whole-recorder
digest; no other line in any of them changed.
Text diffs are deliberately unscripted: highlighting one reaches `lowlight`,
which the module loader refuses as an unspecified native dependency.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): send the github.* PR surface and the review loaders through RpcOperation
The step-4 first half for `src/session/`: eight files, 38 references to the raw
request port, all replaced with declared operations. No behaviour change — the
50 goldens recorded in the previous commit do not move, which is the claim.
- 21 operations over 21 methods. The seven PR reads keep their defensive
parsers as readers; the ten status-envelope mutations share one reader
because the `{ok, error}` convention is one host convention, not ten; the two
bare-boolean mutations read the payload unchecked because `=== true` is the
caller's confirmation rule.
- Four second readers, each justified in place: git.status and git.branchCompare
for the PR branch context (a refusal costs a fallback, not the screen),
git.branchCompare and git.branchDiff for review (the projection is not a
superset of the verbatim payload), and worktree.show for the review notes the
summary reader drops.
- Every failure text is preserved, including the two main kept apart: a refusal
with no message falls back to the screen's copy, a transport drop with no
message surfaces its empty message verbatim. `sendRaw`'s callers replaced
theirs a second time, so those fall back on both paths.
- No retry, and no operation reads a dropped reply as a failed mutation: the
rejection reaches each wrapper's catch as the original object.
- `github-pr-mutations.ts` split along the action/comment seam it already had
in its consumers, so no file needs a max-lines bump.
Inventory: src/session/ 47 files / 114 references -> 39 / 76.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): record the review snapshot answering its notes leg first
The barrier mutation census found one survivor: moving
`reviewWorktreeMetadataRead.interpret` inside the `Promise.all` in
`loadMobileDiffReviewSnapshot` changed nothing any golden observed. The base
scenario answers the branch-base legs before the notes leg, so by the time the
notes reply lands the compare leg has already sent `git.branchCompare` and the
two orders record the same sender list.
This scenario answers the notes leg first, while the compare leg is still
resolving its base ref, and checkpoints before the rest. At that checkpoint the
barrier is the whole difference: the correct order has nothing settled, the
early interpretation has already rejected the action. The mutation now fails it.
Recorded from a detached checkout of the previous commit, which carries main's
unmigrated product code with this branch's recorder over it, so the parity claim
stays non-circular. One new golden; no existing golden moved, because the family
base is unchanged and `scenarioSha256` is per golden.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): bind one git.status projection reader, not a copy per domain
The branch-context read declared its own `statusProjectionReader` with the same
parser, the same 'normalized-status' variant and the same empty salvage as
source-control's `gitStatusProjectionReader`, while its doc block claimed "one
reader serves both". Export the source-control reader and bind it here so the
claim is true; the doc now names the reader and keeps the part that is actually
different, which is what a refusal means on each policy.
No wire change and no golden moves: the reader is the same function value the
copy computed.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): undo the github-pr-mutations split, which max-lines no longer forces
The split was made when the migrated file measured 319 lines. It does not any
more: `sendRaw`, `sendGithubPrMutation` and `extractMutationError` moved to
github-pr-mutation-outcome.ts and the prRepo/headSha allow-lists to
github-pr-repo-slug.ts, so the merged file is 293 lines against the 300 limit
and oxlint is clean.
Nothing imported github-pr-comment-mutations directly — every consumer went
through the re-export hub in github-pr-mutations — so the seam bought a reader
one extra file to open and nothing else. Merge it back and drop the hub.
Product-only: same wrappers, same params, same settle shapes, no golden moves.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): one settleable-operation type for the PR reads and mutations
`GitHubPrMutationOperation` and the private `GitHubPrReadOperation` declared the
same two members for the same reason: a settle shape needs a bound operation's
method and its interpret, nothing else. Keep one, `GitHubPrSettleableOperation`,
and import it into the read settle. `extractMutationError` goes back to private,
as it was on main; it never had an importer outside its own file.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* docs(mobile): drop the key-order claim from the PR param builder
The oracle does not observe param key order: `captureValue` in recording-values.ts
sorts keys, and no golden carries a raw frame string, so "the sender recordings
pin the bytes" was not a fact the evidence supports. The assertion stays for the
reason already in the doc — the builder is method-generic and returns a record.
`GitHubPrParamOptions` goes back to private; nothing outside the module names it.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): read the bare-boolean mutations with the shared unchecked reader
`mutationConfirmationReader` spelled out what `rpcUncheckedPayloadReader` already
returns, under the same 'pr-mutation-confirmation' variant that eleven other
operations in this tree get from the helper. Same function value, same variant,
so no golden moves. The comment explaining why the payload is left unread stays.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* refactor(mobile): one RpcOperationSender for both domains, not one alias each
`MobileSessionRpcSender` and `MobileSourceControlRpcSender` were the same type
with the same doc, each derived from whichever operation its domain happened to
own. Replace both with `RpcOperationSender` in transport, derived from
`settingsRead` there, and name it for what it is: what a bound operation needs
to send with.
Still derived rather than restated, so no module names the raw request port to
accept a client; the port inventory and its ratchet are untouched.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): point the moved PR and diff-review adapters at the seam and register them
The merge commit carried the two adapter files into adapters/ with their old
specifiers and left the register untouched, so this completes the move: the
relative imports climb one more level, and both modules are registered in
adapters/mounted-operation-modules.ts as identifiers imported from their own
source, which is what adapter-seam.test.ts checks.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): re-record the session goldens against #20662's adapter seam
The merge brought #20568's per-golden scenario digest and #20662's per-golden
adapter digest, so the 51 goldens this PR owns move on four header fields and
nothing else: baseline, goldenFormatVersion, recorderSha256, and the newly
added adapterSha256. No recorded byte outside those headers changed.
baseline stays at main's own pin
|
||
 Neil
|
77f23b013f |
refactor(shared): drop the shared/types barrel and import from the real modules (#14447)
#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. |
||
|
Neil
|
583ab1601b |
refactor(shared): group worktree, github, and linear modules into folders (#14437)
`src/shared` is a flat directory of ~1,150 entries. The worktree, github, and
linear domains accounted for 71 of them, so finding the module you wanted meant
scanning a wall of same-prefixed filenames.
Move each domain into its own folder and drop the now-redundant prefix:
src/shared/github-pr-types.ts -> src/shared/github/pull-request-types.ts
src/shared/worktree-id.ts -> src/shared/worktree/id.ts
src/shared/linear-links.ts -> src/shared/linear/links.ts
This follows the existing `network/` and `new-workspace/` convention in the
same directory, which also drop the prefix inside the folder.
Whole clusters move, including tests. Foldering only part of a domain would be
worse than flat: a reader would have to check both `github/` and the flat
directory, and `github-auth-types.ts` / `github-project-types.ts` are type
modules that belong with the rest. No files with these prefixes remain flat.
Import specifiers were rewritten by resolving each one to an absolute path and
recomputing it, not by string substitution, so the `@/../../shared/...` alias
forms are handled correctly. 501 specifiers across 298 files.
Two things `tsc` cannot catch, handled explicitly:
- `github-project-types.ts` carries its own `max-lines` bypass, so its baseline
entry is REPOINTED to the new path rather than pruned. Pruning would drop the
bypass and then flag the new path as a fresh violation. Ratchet stays at 345.
- `mobile/` is outside `pnpm typecheck` and cannot be typechecked here
(`mobile/node_modules` is empty). Instead every relative specifier in the repo
was resolved against the filesystem: 174 unresolved before this change and 174
after — identical, so nothing broke in mobile either.
The pinned `tests/e2e/.cross-version-checkouts` fixtures are deliberately NOT
rewritten; they are a snapshot of an older release and still reference the old
paths.
Verified: cold `tsc --noEmit` green on node, cli, and web (buildinfo deleted
first — these projects are `composite: true` and reuse stale caches).
|
||
 Brennan Benson
|
86b878cfd6 | fix(mobile): parse classified PR lookup outcomes (#12659) | ||
 Jinjing
|
2f104d8713 |
Tier GitHub PR lookup polling to prevent quota exhaustion (#12013)
* Tier GitHub PR lookup polling to prevent quota exhaustion The selected worktree (O(1)) checks per-minute; card list (O(N)) per-15-minutes. Introduce process-wide cache to collapse concurrent polling and gate lookups on available rate-limit budget with exponential backoff on failure. - Preserve last-known review during backoff - Invalidate cache when Orca opens a PR - Stop coordinator from double-charging * Tier GitHub PR lookup polling to prevent quota exhaustion - Return the latest reset time when both GitHub API buckets are rate-limited, preventing premature retries against still-blocked buckets. - Serve the last known review on transient lookup failures, preventing reviews from blinking out on temporary errors. - Discard in-flight lookups that predate an invalidation so stale answers cannot overwrite newly opened reviews. * fix: give rate-limit reset tests unique titles oxlint vitest/no-identical-title was failing static analysis because two cases shared the same describe title. |
||
 
|
971b167548 |
fix(github): load PR diffs for Enterprise remotes (#8932)
* fix(github): load PR diffs for Enterprise remotes * fix(github): encode PR content paths by segment * Fix PR review actions failing on GitHub Enterprise remotes - Threads GitHub host identity (not just owner/repo) through the client, work-item-details, issues, and RPC layers so gh commands target the correct Enterprise server instead of silently falling back to github.com - Adds a shared github-api-repository helper to resolve/host-qualify repo identity consistently across REST, GraphQL, and CLI shorthand calls - Scopes the gh rate-limit breaker and singleton rate-limit snapshot by host/runtime so a github.com block or probe can't affect GHES or WSL - Coalesces concurrent host-auth probes and paginates PR file fetching beyond 100 results - Propagates `host` through renderer PR caches, checks-panel keys, and preload IPC types so Enterprise and github.com data never collide * Route gh host qualification through runner options instead of argv sniff Move GHES/GH_HOST resolution from parsing --hostname/--repo out of gh argv to an explicit options.host passed through ghExecFileAsync, since SSH-backed repos spawn gh with no cwd and argv sniffing couldn't reliably detect the target host. The runner now injects --hostname and qualifies --repo/-R at spawn time from options.host, and rate-limit scoping/guards use the same explicit host instead of inferring it. Also adds a shared githubRepoIdentityKey helper to keep cache/store keys consistent with the new host-aware repository identity. * Fix gh CLI GHES host pinning and rate-limit scope leaks - Pin `--host` on every gh call site so a process-level GH_HOST can't silently redirect requests, and qualify `-R`/`-R=` repo shorthand alongside the existing `--repo=` handling. - Check the target scope for an active rate-limit block before each WSL/native or host fallback retry, not just on the initial attempt, so a blocked scope can't be hit again through a fallback path. - Compute idempotency once per call instead of re-deriving it after fallback reassigns args. * Fix GitHub Enterprise host identity loss across PR/work-item paths - Thread `host` through mobile PR RPC params, IPC work-item lookups, and RPC schemas so GHES identity survives the renderer/mobile/main boundary instead of silently falling back to a same-named github.com repo. - Qualify `--repo`/`-R` args for github.com too (not just GHES), since gh resolves bare shorthand against a process-level GH_HOST that can redirect pinned github.com commands. - Cache `getOriginGitHubApiRepository` to avoid a per-call uncached `git remote get-url` round trip on connection-backed repos. - Add a local-fork fallback in `getWorkItemDetails` so PRs living on a base repo (not visible via the origin slug) still resolve via cwd. - Centralize the github.com-vs-GHES host predicate in `isDefaultGitHubHost` so cache keys, quota scoping, and identity checks can't drift out of sync. * Make repository identity host-aware across all GitHub surfaces Generalize the auth-gated enterprise resolver to any remote and build a cached hosted-identity family (origin/issue/candidates/source) on top of it, then migrate every github.com-only consumer: Tasks listing/counting, branch-to-PR discovery, push targets, fork upstream, issue operations, Projects, web links, avatars, and PR-link facts. Scope the rate-limit breaker probe per runtime:host and classify WSL UNC cwds correctly. Co-authored-by: Orca <help@stably.ai> * Fix expected slug to include host field in GitHub PR link test Updates the smart-source paste-intent test fixture to match the repository slug shape that now carries a `host` field, keeping GHES host identity intact through the paste-intent parsing path. * Surface per-host gh auth state for GitHub Enterprise diagnoseGhAuth accepts the host a surface needs credentials for, scopes the account/scope diagnosis to that host, and reports whether gh has any login there; GhAuthErrorHelp renders host-qualified login/refresh commands so an unauthenticated GHES host stops masquerading as a github.com scope problem. Also fixes the mobile paste-intent expectation for host-carrying parsed links. Co-authored-by: Orca <help@stably.ai> * Bound GHES identity caches and preserve non-default ports in host identity Cap the origin-repo and host-auth caches like ownerRepoCache; keep ports from remote/link URLs so GHES on a non-default port is a distinct identity; make positional github.com slugs explicit against GH_HOST; compare work-item sources by host-aware identity key; bail cwd-less branch lookups when no repository candidate resolved; thread host through the renderer work-item slug lookup. Co-authored-by: Orca <help@stably.ai> * Thread GitHub host through issue detail requests Incorporates ghes-issue-host-support (ed6bb96ef): one hosted issue repository identity is resolved before the details fan-out so comments, timeline, participants, and mention lookups cannot drift across hosts, with SSH guards so unresolved issue/PR repositories never fall through to gh's default host. Co-authored-by: Orca <help@stably.ai> * Scope remaining GitHub rate-limit accounting * Resolve typed PR lookups across hosted repository candidates getWorkItem's PR path probes upstream-then-origin hosted candidates instead of origin alone, so fork checkouts resolve the base repo's PR with the right host; issue detail resolution reuses the up-front hosted identity and keeps the SSH unresolved-host guards. Co-authored-by: Orca <help@stably.ai> * Refactor GitHub repository execution setup * Carry host on smart-submit link intents Co-authored-by: Orca <help@stably.ai> * Carry the project host on GitHub item dialog origins Co-authored-by: Orca <help@stably.ai> * Keep GHES web ports but drop SSH transport ports in host identity Supersedes PR #9118 on this branch: http(s) remote ports identify the Enterprise web/API endpoint and are preserved, while ssh/git transport ports (including ssh.github.com:443) never leak into gh's host identity. Replaces the ssh.github.com:443 special case with the structural protocol split and ports the PR's parsing test suite. Co-authored-by: Orca <help@stably.ai> * Support GitHub Enterprise diffs and mutations with host-scoped caches Parse GitHub host identity from work-item URLs and carry it through PR/issue mutations, labels, and assignments. Bound rate-limit and scope-probe caches (1024 and 512 entries) to prevent unbounded growth when interacting with multiple GHES instances. Normalize repository identity keys to include host so github.com and GHES slugs don't collide in cache and equality checks. * Support GitHub Enterprise diffs and mutations with host-scoped caches - Carry host identity through PR mutations and reads so fork PRs on different GHES instances don't collide in cache or state tracking. - Validate host authentication before routing requests to unconfigured Enterprise servers; ambient credentials must never reach untrusted hosts. - Scope rate-limit guards and spend tracking per host so GHES quota stays independent from github.com quota. - Respect explicit --hostname arguments in gh CLI calls ahead of GH_HOST or ambient defaults, so breaker state follows the actual request target. - Detect implicit WSL runtimes from UNC paths for consistent host auth and execution-options scoping across mobile and desktop clients. * Support GitHub Enterprise work-item diffs with host-scoped execution Enterprise PRs must use their selected host consistently across diff, comments, and file-content loads. Validate repository slugs before authenticated execution to prevent path-injection via renderer overrides. Scope project browsing cache and rate-limit tracking by host to prevent cross-host pollution. Use parsed URLs as authoritative over ambient hosts for project resolution. * Support GitHub Enterprise work-item diffs with host-scoped execution Preserve host identity on PR/issue work items throughout the mutation and diff pipeline so Enterprise instances (including ported endpoints like github.acme.test:8443) can execute mutations without ambiguity. Rate-limit gh commands by the pre-qualified --repo host, cache auth state per ported host, and surface Enterprise hosts in project metadata and error messages. * fix(review): drop dead rateLimitGuard/noteRateLimitSpend re-export Both callers (project-view.ts, mutations.ts) moved to the host-scoped repositoryRateLimitGuard/noteRepositoryRateLimitSpend; the bucket-only re-export in internals.ts had zero importers left. Co-authored-by: Orca <help@stably.ai> * fix(ci): split Enterprise host work-item tests under max-lines Move GHES/SSH host-routing cases out of work-item-details.test.ts so the suite stays within the 800-line test max-lines budget. * test(github): align mocks with host-scoped repository resolution - Route origin repository resolution through getOwnerRepoForRemote, not getOwnerRepo, to match production path - Pin github.com host on origin results so host-less fixtures pass host gate in resolveGitHubApiRepository - Add generation-based invalidation to prevent stale slug-cache writes from in-flight resolutions - Fix ref-sync race in ProjectPicker: use useLayoutEffect so committed tree owns browse cache key - Defer handledCrossRepoUrlRef assignment in SmartWorkspaceNameField until resolution succeeds - Update Enterprise host routing: found work items must not silently fall back to default host when unresolved - Normalize GHES avatar URLs: accept explicit port 443 as canonical form, not a fallback trigger --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai> |
||
 
|
c9bd61376f |
feat(mobile): combine PR sidebar and checks parity (#5641)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Orca <help@stably.ai> Co-authored-by: gsxdsm <gsxdsm@users.noreply.github.com> |