Commit Graph
5 Commits
Author SHA1 Message Date
Brennan BensonandQA 64de8dd637 fix(workspaces): delete on the confirmed host, and make both hosts' rows selectable (STA-4343) (#15013)
* fix(workspaces): host-qualified workspace deletion (STA-4343, STA-4448)

Squashed integration of PR #14606 + the codex review-loop output, replayed
onto current main. Granular history preserved on brennanb2025/sta-4343-review-full.

Fixes the regression from #13413: a workspace id is repoId::path with no host
component, so the same repo at the same path on two hosts published one id for
two workspaces, and deletion routed by that id landed on whichever host routing
preferred - usually the ACTIVE one, not the row the user confirmed.

- removeWorktree takes a REQUIRED host-qualified WorktreeRemovalTarget; omitting
  the host is a type error. All destructive callers migrated.
- Projections dedup on (host, id), so two hosts render as two selectable rows
  while the createWorktree/fetchWorktrees race duplicate still collapses.
- Ephemeral VM cleanup is host-scoped. It matched on bare workspaceId, so the
  host-scoped delete path destroyed the SURVIVING host's VM and its unpushed
  filesystem - a leak fix that had become data destruction.
- Selection, keyboard routing, lineage grouping and Space rows carry host
  identity end to end; fixing the executor dedupe alone would have turned
  one-row intent into deleting both hosts.

Files split to stay under max-lines rather than raising any cap.

* refactor: split files that crossed max-lines

The review-loop commits used --no-verify, so the pre-commit hook never
enforced the caps. Extracted cohesive units rather than raising any limit:
renderer teardown, delete-with-toast, pinned-group rows, host-scope helpers,
workspace-kind predicates, filter actions, kanban drag selection, the
renderer removal result type, and the native-chat persistence tests.

* refactor(workspaces): extract cleanup deletion-phase selector

Clears the last max-lines violation and the import-type side effect the
changed-code gate flagged.

* refactor(sidebar): track the delete-dialog extraction modules

* fix(workspaces): preserve host identity across remaining surfaces

* fix(sidebar): re-carry host through the rewritten palette result model

#15170 replaced PaletteSearchResult while this PR was open. Re-applied the
host qualification on top of the new model instead of taking either side:
results carry worktreeHostId again, and the board filter keys its matched
set on host identity rather than the bare id.

Known gap, documented in the board test rather than deleted: searchWorktrees
resolves evidence through a `documents` map keyed by BARE worktree id, so two
same-id host rows collapse before this code sees them. Closing that belongs
with the palette work.

* test(cmd-j): pin the palette collision gap instead of asserting the old model

The palette collision test asserted two host-qualified rows, which #15170's
rewrite made unreachable: item ids are bare again and worktreeMap is id-keyed.

Rewritten to assert what holds — activation always names a host — and to pin
the defect it exposes: two same-id rows render on ONE command value, so React
sees duplicate keys and a click on the first row activates the second row's
host. That reproduces on main, so it is pre-existing, not from this PR. Pinned
rather than deleted so fixing it must update this test.

---------

Co-authored-by: QA <qa@local>
2026-08-17 15:57:07 -07:00
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.
2026-08-13 22:48:24 -07:00
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).
2026-08-13 20:44:16 -07:00
Jinwoo HongandJinwoo-H 5e3a2d25f7 Filter remote workspaces by creating device (#13718)
* Filter remote workspaces by creating device

* Fix workspace origin filter reconnect behavior

* Shorten workspace origin filter label

* Refine remote workspace filter UX

---------

Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com>
2026-08-11 01:05:12 -07:00
Jinjing f5d934a3f5 3c5f0060 (#13496)
* refactor(runtime): extract pure path-candidate, review-branch, and folder-workspace helpers from orca-runtime.ts

Mechanical move of three closed, pure module-scope clusters out of
orca-runtime.ts (37,608 -> 37,207 lines) into domain-named siblings:

- terminal-output-path-candidates.ts: PTY output path harvesting and the
  recent-candidate history bound (3 entry points + 15 private callees).
- selected-review-branch.ts: forge-agnostic selected-review predicates and
  lookup hints (GitHub/GitLab/Bitbucket/Azure DevOps/Gitea).
- runtime-folder-workspace.ts: folder-workspace id math and the repo+meta
  -> Worktree projection.

Bodies are token-identical to their previous form; the only production
changes are the moves, the new import statements, and `export` keywords.
The no-control-regex suppression travels with the path-candidate scanning
that needs it. No max-lines suppression was added and the ratchet is
unchanged.

Adds characterization tests for the two clusters that had no direct
coverage; the path-candidate cluster keeps its existing tests, repointed
at the new module.

* fix flaky timer on CI
2026-08-09 23:37:33 -07:00