mirror of
https://github.com/stablyai/orca.git
synced 2026-10-26 00:03:55 +00:00
Brennan BensonandQA
64de8dd637
* 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>
52 lines
2.1 KiB
TypeScript
52 lines
2.1 KiB
TypeScript
import type { ExecutionHostId } from '../execution-host'
|
|
import type { Worktree } from './types'
|
|
|
|
/**
|
|
* Separator between the host and the workspace id.
|
|
*
|
|
* Printable on purpose: these identities become React keys and DOM attribute
|
|
* values, and HTML attribute parsing replaces U+0000 with U+FFFD — a NUL
|
|
* separator would not survive the round trip an anchor comparison depends on.
|
|
*
|
|
* `|` is safe as a delimiter because an execution host id cannot contain one:
|
|
* it is `local`, or a fixed `ssh:`/`runtime:` prefix followed by an
|
|
* `encodeURIComponent`-escaped part (see `toSshExecutionHostId`), which encodes
|
|
* `|` as `%7C`. Putting the host first therefore makes the split unambiguous
|
|
* even though the workspace id — a repo id and a filesystem path — may contain
|
|
* anything.
|
|
*/
|
|
const HOST_SEPARATOR = '|'
|
|
|
|
/**
|
|
* Stable key for one workspace on one host (STA-4343).
|
|
*
|
|
* `worktreeId` is `repoId::path` with no host component, so a repo registered on
|
|
* two execution hosts publishes the same id twice for two different workspaces.
|
|
* Any map, set or React key that must keep them apart keys on this instead.
|
|
*
|
|
* An unqualified row gets its own bucket rather than being folded into a host:
|
|
* it may well BE one of them, but nothing here can prove which.
|
|
*/
|
|
export function getWorktreeHostIdentity(worktree: Pick<Worktree, 'id' | 'hostId'>): string {
|
|
return composeWorktreeHostIdentity(worktree.hostId, worktree.id)
|
|
}
|
|
|
|
export function composeWorktreeHostIdentity(
|
|
hostId: ExecutionHostId | undefined,
|
|
worktreeId: string
|
|
): string {
|
|
return `${hostId ?? ''}${HOST_SEPARATOR}${worktreeId}`
|
|
}
|
|
|
|
/**
|
|
* The workspace id back out of an identity.
|
|
*
|
|
* Exact, not best-effort: the host cannot contain the separator (see above), so
|
|
* everything after the first one is the id. That lets an index recover the id
|
|
* from its own key instead of re-reading `worktree.id`, which matters because
|
|
* retained selectors assert that getter is read exactly once per snapshot.
|
|
*/
|
|
export function getWorktreeIdFromHostIdentity(identity: string): string {
|
|
return identity.slice(identity.indexOf(HOST_SEPARATOR) + 1)
|
|
}
|