Files
orca/src/shared/pty-session-id-format.ts
T
Brennan BensonandOrca 1f43346c5f fix(resource-usage): hydrate pty-registry at boot; render · remote only for SSH repos (#1667)
* WIP: Changes before auto-review fixes

Co-authored-by: Orca <help@stably.ai>

* fix: address auto-review-fix-multi-agent findings

- Replace local ORCA_WORKTREE_ID_SEPARATOR with shared WORKTREE_ID_SEPARATOR
- Make hydrateLocalPtyRegistryAtBoot idempotent (one-shot per process,
  but stays retry-eligible until daemon provider is available)
- Strengthen daemon-pty-adapter strict-parser test to actually exercise
  the new short-circuit (test would have passed under the old loose
  parser too without the change)
- Add eslint-disable max-lines directive to oversized merge test file

Co-authored-by: Orca <help@stably.ai>

* chore: archive auto-review context to .context/

Co-authored-by: Orca <help@stably.ai>

* fix: address auto-review-fix findings

Drop the destructive reconcileOnStartup call from boot-time PTY registry
hydration: a transient listRepoWorktrees failure (returns [] and only
warns) would otherwise let the reconcile pass kill live local sessions.
The boot path is now read-only against the daemon — listSessions() only.

Also: tighten parsePtySessionId to reject degenerate `::` halves; replace
stale pty.ts:1005 references and a misleading local-unknown comment in
the hydrate module; narrow Store dependency to Pick<Store, 'getRepos'>;
log adapter listSessions failures instead of silently swallowing them;
re-anchor design-doc references on stable symbols and align §1b/§1c/§1d
with the implementation.

Co-authored-by: Orca <help@stably.ai>

* docs(resource-usage): update remote badge spec

Co-authored-by: Orca <help@stably.ai>

* test(resource-usage): cover boot hydration failure modes + warm-reattach e2e

Adds the regression coverage flagged in PR #1667's test plan that wasn't
already locked down.

vitest (`hydrate-local-pty-registry.test.ts`):
  - daemon offline at first call → no-op, hasHydrated stays false so a
    later macOS dock re-activation can retry.
  - listSessions rejection caught and logged, does not throw.
  - pid-write ordering: a pre-existing registry entry with pid=12345 is
    not clobbered by a stale `pid: null` from listSessions (§1d).
  - SSH-gate: a session whose repo has a non-null connectionId stays out
    of the registry, mirroring the spawn-time gate in pty.ts.
  - Happy-path: a local session is registered with the daemon's pid.

Playwright e2e (`resource-usage-warm-reattach.spec.ts`):
  Full quit→relaunch cycle against the same userDataDir; asserts that
  on the second launch the snapshot includes the warm-reattached PTY
  with a real pid before any pane mount, and that the seeded repo
  resolves as local (no connectionId). Mirrors the existing
  terminal-restart-persistence pattern.

Co-authored-by: Orca <help@stably.ai>

* fix(test): satisfy Pick<Store, 'getRepos'> in hydrator vitest

CI typecheck failed because FakeStore's getRepos returned objects missing
Repo's required fields (path, displayName, badgeColor, addedAt). Fill with
placeholder values; the hydrator only reads id + connectionId, but the
type signature still has to line up.

Co-authored-by: Orca <help@stably.ai>

* chore(resource-usage): drop bug-doc files; strip dead doc refs from comments

Remove docs/resource-usage-remote-mislabel.md (new in this PR) and revert
docs/resource-usage-merge-spec.md to the PR-base state. Strip the
matching `docs/...md §N` pointers from code/test comments, keeping the
surrounding "why" explanations intact so readers still get the
warm-reattach mislabel context.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
2026-05-10 17:17:52 -07:00

42 lines
1.7 KiB
TypeScript

/**
* Shared helpers for the minted PTY session id format.
*
* Why split out of `src/main/daemon/pty-session-id.ts`: the renderer-side
* merge in `mergeSnapshotAndSessions.ts` and the boot-time hydration in
* `attach-main-window-services.ts` both need to recover the owning
* worktreeId from a session id. Three call sites silently re-implementing
* the same parser (one of them looser than the others) was the seed of
* the resource-usage REMOTE-mislabel bug. Centralising the format here
* keeps a single definition that both the main process and the renderer
* can import.
*/
export const PTY_SESSION_ID_SEPARATOR = '@@'
export const WORKTREE_ID_SEPARATOR = '::'
/**
* Recover the owning worktreeId from a minted session id.
*
* Why stricter than `lastIndexOf('@@')`: callers that drive memory
* attribution must not synthesize a worktreeId for a sessionId that was
* not minted by us — e.g. a bare UUID. Requiring both the `@@` separator
* AND the `${repoId}::${path}` shape rejects those imposters cleanly.
* Returns `{ worktreeId: null }` when the id does not match the minted
* format.
*/
export function parsePtySessionId(sessionId: string): { worktreeId: string | null } {
const idx = sessionId.lastIndexOf(PTY_SESSION_ID_SEPARATOR)
if (idx <= 0) {
return { worktreeId: null }
}
const candidate = sessionId.slice(0, idx)
// Why: require non-empty halves on both sides of `::` so degenerate
// ids like `::@@…`, `repo::@@…`, or `::path@@…` don't synthesize a
// phantom worktreeId for memory attribution.
const sepIdx = candidate.indexOf(WORKTREE_ID_SEPARATOR)
if (sepIdx <= 0 || sepIdx + WORKTREE_ID_SEPARATOR.length >= candidate.length) {
return { worktreeId: null }
}
return { worktreeId: candidate }
}