mirror of
https://github.com/stablyai/orca.git
synced 2026-10-29 08:07:18 +00:00
fix(pty): derive deterministic sessionId from (worktreeId, tabId, leafId)
Why: Issue #217 loses terminal scrollback across unclean shutdowns because the minted sessionId is random — the persist-sessionId write races SIGKILL, so the post-restart PTY maps to a fresh history dir instead of the previous one. mintPtySessionId now hashes (tabId, leafId) when both are present so a crash before persistence still lands on the same history dir after restart. Falls back to randomUUID() when callers don't thread pane identity (non-daemon-host spawns and legacy paths). The origin is surfaced via mintPtySessionIdWithOrigin so ipc/pty can log random-fallback at warn — a daemon-host spawn that hits the fallback indicates a plumbing regression in the renderer. Thread tabId/leafId from renderer through PtyTransport → preload → pty:spawn IPC → daemon-pty-adapter fallback so all daemon-host spawns produce deterministic ids. See DESIGN_DOC_TERMINAL_HISTORY_FIX.md §Deterministic sessionId. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -96,7 +96,14 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
private async doSpawn(opts: PtySpawnOptions): Promise<PtySpawnResult> {
|
||||
await this.ensureConnected()
|
||||
|
||||
const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId)
|
||||
// Why: the renderer should thread tabId/leafId for every daemon-host
|
||||
// spawn so we get a deterministic sessionId. On the main-process
|
||||
// spawn path, `ipc/pty.ts` has already minted `opts.sessionId` with
|
||||
// the deterministic suffix — we arrive here via opts.sessionId in
|
||||
// the common case. This fallback handles direct callers (tests,
|
||||
// non-IPC code paths) that don't pre-mint; they should still pass
|
||||
// tabId/leafId when possible so the id is stable across restarts.
|
||||
const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId, opts.tabId, opts.leafId)
|
||||
|
||||
if (this.killedSessionTombstones.has(sessionId)) {
|
||||
throw new TerminalKilledError(sessionId)
|
||||
|
||||
@@ -1,18 +1,104 @@
|
||||
import { randomUUID } from 'crypto'
|
||||
import { createHash, randomUUID } from 'crypto'
|
||||
import { isAbsolute, join, relative, resolve, sep } from 'path'
|
||||
|
||||
export type MintedSessionIdOrigin = 'deterministic' | 'explicit' | 'random-fallback'
|
||||
|
||||
export type MintSessionIdResult = {
|
||||
sessionId: string
|
||||
/**
|
||||
* How the id was produced:
|
||||
* - `deterministic`: derived from (worktreeId, tabId, leafId) — the happy
|
||||
* path that lets cold-restore survive a crash before persistence.
|
||||
* - `explicit`: caller supplied a pre-computed sessionId (reattach path).
|
||||
* - `random-fallback`: a daemon-host spawn did not receive tabId/leafId
|
||||
* and had to fall back to a random UUID. Persistence still matters for
|
||||
* this branch — it is the old behavior and re-exposes the race this
|
||||
* design is trying to close. Logged at warn level so regressions in
|
||||
* the renderer plumbing surface loudly.
|
||||
*/
|
||||
derivedFrom: MintedSessionIdOrigin
|
||||
}
|
||||
|
||||
/**
|
||||
* Session IDs use the format `${worktreeId}@@${shortUuid}` so that
|
||||
* Session IDs use the format `${worktreeId}@@${suffix}` so that
|
||||
* DaemonPtyAdapter.reconcileOnStartup (see daemon-pty-adapter.ts) can
|
||||
* derive the owning worktree by splitting on the @@ separator.
|
||||
*
|
||||
* Both pty.ts (host-daemon spawn path) and DaemonPtyAdapter.doSpawn
|
||||
* (fallback when opts.sessionId is absent) must use this helper — a
|
||||
* drifted format would break cold-restore mapping and Pi overlay
|
||||
* Deterministic suffix (preferred): when both `tabId` and `leafId` are
|
||||
* provided, the suffix is the first 32 bits of SHA-256(tabId || "\0" ||
|
||||
* leafId), hex-encoded. This lets a restart reach the same history
|
||||
* directory for the same (worktreeId, tabId, leafId) triple *without*
|
||||
* needing `orca-data.json` to have persisted the sessionId — closing
|
||||
* the SIGKILL-between-spawn-and-persist race that Issue #217 describes.
|
||||
*
|
||||
* Random suffix (fallback): when tabId/leafId are absent (e.g. an older
|
||||
* non-pane caller) the suffix is `randomUUID().slice(0, 8)`. This is the
|
||||
* legacy behavior; the renderer is expected to thread tabId+leafId for
|
||||
* every daemon-host spawn, so falling in here from a daemon-host code
|
||||
* path indicates a plumbing bug and is logged at warn level by callers.
|
||||
*
|
||||
* Both `pty.ts` (host-daemon spawn path) and DaemonPtyAdapter.doSpawn
|
||||
* (fallback when opts.sessionId is absent) delegate to this helper —
|
||||
* a drifted format would break cold-restore mapping and Pi overlay
|
||||
* keying.
|
||||
*
|
||||
* Collision bound: with 32 bits of suffix entropy, the birthday bound
|
||||
* for two distinct (tabId, leafId) pairs colliding within one worktree
|
||||
* is ~1 in 2^16 at ~256 pairs. Real worktrees hold O(10) panes. The
|
||||
* bound matches the pre-existing random-UUID path (also 32 bits) — no
|
||||
* regression.
|
||||
*/
|
||||
export function mintPtySessionId(worktreeId?: string): string {
|
||||
return worktreeId ? `${worktreeId}@@${randomUUID().slice(0, 8)}` : randomUUID()
|
||||
export function mintPtySessionId(worktreeId?: string, tabId?: string, leafId?: string): string {
|
||||
if (!worktreeId) {
|
||||
return randomUUID()
|
||||
}
|
||||
if (tabId && leafId) {
|
||||
// Why: NUL separator is belt-and-suspenders. `createHash.update()` is
|
||||
// streaming so `update("a") + update("\0") + update("b")` and
|
||||
// `update("a\0b")` produce identical digests; the explicit separator
|
||||
// is cosmetic against today's NUL-free input alphabet (tabIds are
|
||||
// `crypto.randomUUID()`, leafIds are `pane:<number>`). Callers that
|
||||
// might introduce NUL-containing inputs must switch to a
|
||||
// length-prefixed encoding instead.
|
||||
const h = createHash('sha256')
|
||||
h.update(tabId)
|
||||
h.update('\0')
|
||||
h.update(leafId)
|
||||
const suffix = h.digest('hex').slice(0, 8)
|
||||
return `${worktreeId}@@${suffix}`
|
||||
}
|
||||
return `${worktreeId}@@${randomUUID().slice(0, 8)}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Variant that also reports how the id was produced. Use this at the
|
||||
* single mint call site in `ipc/pty.ts` so the observability log can
|
||||
* split info vs. warn appropriately.
|
||||
*/
|
||||
export function mintPtySessionIdWithOrigin(args: {
|
||||
worktreeId?: string
|
||||
tabId?: string
|
||||
leafId?: string
|
||||
explicitSessionId?: string
|
||||
}): MintSessionIdResult {
|
||||
if (args.explicitSessionId) {
|
||||
return { sessionId: args.explicitSessionId, derivedFrom: 'explicit' }
|
||||
}
|
||||
if (args.worktreeId && args.tabId && args.leafId) {
|
||||
return {
|
||||
sessionId: mintPtySessionId(args.worktreeId, args.tabId, args.leafId),
|
||||
derivedFrom: 'deterministic'
|
||||
}
|
||||
}
|
||||
// Either worktreeId is absent (non-daemon spawn — we return a raw uuid
|
||||
// and the caller is responsible for deciding whether that's expected)
|
||||
// or tabId/leafId weren't threaded through from the renderer. The
|
||||
// second case is the one we want to hear about; callers differentiate
|
||||
// on `!!args.worktreeId` before choosing the log level.
|
||||
return {
|
||||
sessionId: mintPtySessionId(args.worktreeId, args.tabId, args.leafId),
|
||||
derivedFrom: 'random-fallback'
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+51
-3
@@ -15,7 +15,7 @@ import { piTitlebarExtensionService } from '../pi/titlebar-extension-service'
|
||||
import { isPwshAvailable } from '../pwsh'
|
||||
import { LocalPtyProvider } from '../providers/local-pty-provider'
|
||||
import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../providers/types'
|
||||
import { mintPtySessionId, isSafePtySessionId } from '../daemon/pty-session-id'
|
||||
import { mintPtySessionIdWithOrigin, isSafePtySessionId } from '../daemon/pty-session-id'
|
||||
import type { ClaudeRuntimeAuthPreparation } from '../claude-accounts/runtime-auth-service'
|
||||
import { CLAUDE_AUTH_ENV_VARS, hasClaudeAuthEnvConflict } from '../claude-accounts/environment'
|
||||
import {
|
||||
@@ -631,6 +631,13 @@ export function registerPtyHandlers(
|
||||
worktreeId?: string
|
||||
sessionId?: string
|
||||
shellOverride?: string
|
||||
// Why: renderer threads tabId + leafId for every daemon-host spawn so
|
||||
// the main process can mint a deterministic sessionId from
|
||||
// (worktreeId, tabId, leafId). That makes cold-restore survive a
|
||||
// SIGKILL between spawn and the renderer's debounced persistence.
|
||||
// Both fields are optional for back-compat with non-pane callers.
|
||||
tabId?: string
|
||||
leafId?: string
|
||||
}
|
||||
) => {
|
||||
const provider = getProvider(args.connectionId)
|
||||
@@ -680,8 +687,40 @@ export function registerPtyHandlers(
|
||||
// an existing PTY whose state (OpenCode hooks, Pi overlay, agent-hook
|
||||
// pane caches) we must not clobber on a retry/attach failure.
|
||||
const isMintedSessionId = args.sessionId === undefined && isDaemonHostSpawn
|
||||
const effectiveSessionId =
|
||||
args.sessionId ?? (isDaemonHostSpawn ? mintPtySessionId(args.worktreeId) : undefined)
|
||||
// Why: mint deterministically from (worktreeId, tabId, leafId) when
|
||||
// the renderer threads those. Explicit sessionIds (reattach) and
|
||||
// random-fallback (tabId/leafId missing) are recorded so we can
|
||||
// spot renderer plumbing regressions in the logs.
|
||||
const mintResult = isDaemonHostSpawn
|
||||
? mintPtySessionIdWithOrigin({
|
||||
worktreeId: args.worktreeId,
|
||||
tabId: args.tabId,
|
||||
leafId: args.leafId,
|
||||
explicitSessionId: args.sessionId
|
||||
})
|
||||
: args.sessionId
|
||||
? { sessionId: args.sessionId, derivedFrom: 'explicit' as const }
|
||||
: null
|
||||
const effectiveSessionId = mintResult?.sessionId
|
||||
if (mintResult && isDaemonHostSpawn) {
|
||||
const logPayload = {
|
||||
worktreeId: args.worktreeId,
|
||||
tabId: args.tabId,
|
||||
leafId: args.leafId,
|
||||
sessionId: mintResult.sessionId,
|
||||
derivedFrom: mintResult.derivedFrom
|
||||
}
|
||||
if (mintResult.derivedFrom === 'random-fallback') {
|
||||
// Why: a daemon-host spawn arriving without tabId/leafId means the
|
||||
// renderer plumbing didn't thread them — cold restore will only
|
||||
// survive if the renderer later persists this sessionId before a
|
||||
// crash. That's exactly the race Issue #217 documented, so surface
|
||||
// it as a warning.
|
||||
console.warn('[pty] mintPtySessionId random-fallback', logPayload)
|
||||
} else {
|
||||
console.info('[pty] mintPtySessionId', logPayload)
|
||||
}
|
||||
}
|
||||
const baseEnv = claudeAuth ? { ...args.env, ...claudeAuth.envPatch } : args.env
|
||||
let env: Record<string, string> | undefined = baseEnv
|
||||
const preAllocatedHandle =
|
||||
@@ -753,6 +792,15 @@ export function registerPtyHandlers(
|
||||
if (effectiveSessionId !== undefined) {
|
||||
spawnOptions.sessionId = effectiveSessionId
|
||||
}
|
||||
// Why: forward tabId/leafId to the provider so any fallback mint path
|
||||
// inside the adapter (tests, direct callers) can still produce a
|
||||
// deterministic id.
|
||||
if (args.tabId !== undefined) {
|
||||
spawnOptions.tabId = args.tabId
|
||||
}
|
||||
if (args.leafId !== undefined) {
|
||||
spawnOptions.leafId = args.leafId
|
||||
}
|
||||
// Why: on Windows, fall back to the persisted default-shell setting
|
||||
// when the renderer didn't send a per-tab override. Without this, the
|
||||
// daemon path ignores the user's "Default Shell" preference entirely —
|
||||
|
||||
@@ -25,6 +25,15 @@ export type PtySpawnOptions = {
|
||||
/** Daemon session ID for reattach. When provided, the daemon reconnects
|
||||
* to an existing session instead of creating a new one. */
|
||||
sessionId?: string
|
||||
/** Tab identity, forwarded by the renderer for daemon-host spawns so the
|
||||
* main process can derive a deterministic sessionId from
|
||||
* (worktreeId, tabId, leafId). Closes the SIGKILL-between-spawn-and-
|
||||
* persist race that loses scrollback on unclean restart. Optional for
|
||||
* backwards compatibility with non-pane callers; when absent the daemon
|
||||
* path falls back to a random-suffix sessionId and logs at warn level. */
|
||||
tabId?: string
|
||||
/** Pane leaf identity (e.g. "pane:1"). See `tabId` for why. */
|
||||
leafId?: string
|
||||
/** Why: allows the renderer to request a specific shell for a single new
|
||||
* terminal tab (e.g. "open this tab in WSL" from the "+" submenu) without
|
||||
* changing the user's persistent default shell setting. Only consulted on
|
||||
|
||||
@@ -418,6 +418,12 @@ export type PreloadApi = {
|
||||
// Preserved from the deleted index.d.ts PtyApi duplicate during the
|
||||
// single-source-of-truth collapse (see docs/preload-typecheck-hole.md §1).
|
||||
shellOverride?: string
|
||||
// Why: renderer threads these for daemon-host spawns so the main
|
||||
// process can mint a deterministic sessionId from (worktreeId,
|
||||
// tabId, leafId). Closes the SIGKILL-between-spawn-and-persist race
|
||||
// that lost terminal scrollback on unclean restart (Issue #217).
|
||||
tabId?: string
|
||||
leafId?: string
|
||||
}) => Promise<{
|
||||
id: string
|
||||
snapshot?: string
|
||||
|
||||
@@ -328,6 +328,8 @@ const api = {
|
||||
worktreeId?: string
|
||||
sessionId?: string
|
||||
shellOverride?: string
|
||||
tabId?: string
|
||||
leafId?: string
|
||||
}): Promise<{
|
||||
id: string
|
||||
snapshot?: string
|
||||
|
||||
@@ -445,12 +445,25 @@ export function connectPanePty(
|
||||
// has no equivalent mechanism, so the renderer must inject it via sendInput.
|
||||
let pendingStartupCommand = connectionId ? (paneStartup?.command ?? null) : null
|
||||
|
||||
// Why: this must match the leafId the persistence layer uses for
|
||||
// terminalLayoutsByTabId[tabId].ptyIdsByLeafId so a restart reaches
|
||||
// the same history directory. `restoredLeafId` wins when the pane
|
||||
// was materialized from a persisted snapshot (replayTerminalLayout
|
||||
// preserved it); otherwise the newly-assigned paneId is the leaf.
|
||||
const effectiveLeafId = deps.restoredLeafId ?? paneLeafId(pane.id)
|
||||
|
||||
const startFreshSpawn = (): void => {
|
||||
const spawnPromise = Promise.resolve(
|
||||
transport.connect({
|
||||
url: '',
|
||||
cols,
|
||||
rows,
|
||||
// Why: threading tabId/leafId lets the main process derive a
|
||||
// deterministic sessionId (see src/main/daemon/pty-session-id.ts)
|
||||
// so cold-restore works even if the renderer was SIGKILLed
|
||||
// before its debounced ptyId persistence ran.
|
||||
tabId: deps.tabId,
|
||||
leafId: effectiveLeafId,
|
||||
callbacks: {
|
||||
onData: dataCallback,
|
||||
onReplayData: replayDataCallback,
|
||||
@@ -797,6 +810,12 @@ export function connectPanePty(
|
||||
cols,
|
||||
rows,
|
||||
sessionId: deferredReattachSessionId,
|
||||
// Why: tabId/leafId are ignored when sessionId is explicit, but
|
||||
// threading them keeps the audit trail intact and makes a future
|
||||
// fallback (e.g. the explicit id was invalid) produce a
|
||||
// deterministic id instead of a random one.
|
||||
tabId: deps.tabId,
|
||||
leafId: effectiveLeafId,
|
||||
callbacks: {
|
||||
onData: dataCallback,
|
||||
onReplayData: replayDataCallback,
|
||||
|
||||
@@ -153,6 +153,13 @@ export type PtyTransport = {
|
||||
/** Daemon session ID for reattach. When provided, the daemon reconnects
|
||||
* to an existing session instead of creating a new one. */
|
||||
sessionId?: string
|
||||
/** Tab identity, forwarded with every daemon-host spawn so the main
|
||||
* process can mint a deterministic sessionId from (worktreeId, tabId,
|
||||
* leafId). Closes the SIGKILL-between-spawn-and-persist race that
|
||||
* lost terminal scrollback on unclean restart (Issue #217). */
|
||||
tabId?: string
|
||||
/** Pane leaf identity (e.g. "pane:1"). See `tabId` for why. */
|
||||
leafId?: string
|
||||
callbacks: {
|
||||
onConnect?: () => void
|
||||
onDisconnect?: () => void
|
||||
|
||||
@@ -353,7 +353,12 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
|
||||
...(connectionId ? { connectionId } : {}),
|
||||
...(options.sessionId ? { sessionId: options.sessionId } : {}),
|
||||
worktreeId,
|
||||
...(shellOverride ? { shellOverride } : {})
|
||||
...(shellOverride ? { shellOverride } : {}),
|
||||
// Why: thread tabId/leafId so the main process can derive a
|
||||
// deterministic sessionId. Issue #217 regression prevention —
|
||||
// see pty-session-id.ts.
|
||||
...(options.tabId ? { tabId: options.tabId } : {}),
|
||||
...(options.leafId ? { leafId: options.leafId } : {})
|
||||
})
|
||||
const spawnResult = result as PtyConnectResult & { isReattach?: boolean }
|
||||
|
||||
|
||||
Reference in New Issue
Block a user