mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
fix(ssh): stamp the hoist instead of deleting, and normalize what it folds in
Replaces the partition delete with a one-shot settings stamp. The delete broke five deliberate tests: ssh:<target> partitions are legitimately owned by the headless and CLI planes, and destroying them also made a rollback unable to recover those tabs. The stamp stops the re-hoist without touching the data. Moves the hoist BEFORE pane-identity normalization. A partition can carry a legacy non-UUID leaf; injecting one raw skips the normalizer that exists for exactly that, the renderer then mints a fresh UUID on hydrate, the binding lookup misses, and that pane hits the same tab loss this change exists to fix. The earlier claim that normalization prunes partitions was false — those attempts failed because the test fixture was schema-invalid, not because of ordering. Also narrows isPtyProvenGoneForReplacement to the minted expiry (P1-a): a raw SSH_PTY_EXITED is one the reattach could NOT tie to this shell, and honouring it retired the owner and respawned onto a shell still in use. Also prunes expired SSH leases older than 7 days at load (P1-d): they are unreachable from every reattach path and previously grew for the life of the profile. Retention sits far above the 30s pane-recovery grace window, the only remaining reader.
This commit is contained in:
+5
-3
@@ -1121,10 +1121,12 @@ function isPtyProvenGoneForReplacement(err: unknown, ptyId: string): boolean {
|
||||
if (!parseAppSshPtyId(ptyId)) {
|
||||
return isPtyAlreadyGoneError(err)
|
||||
}
|
||||
// For SSH, the two proving answers are the relay's own observed exit and the expiry the reattach
|
||||
// mints only after verifying that proof names this shell. A bare not-found is neither.
|
||||
// The ONLY proving answer is the expiry the reattach mints after verifying the relay's proof
|
||||
// names this shell and this incarnation. Accepting a bare SSH_PTY_EXITED too was over-acceptance:
|
||||
// the reattach deliberately rethrows one it could NOT tie to us, and honouring that here retires
|
||||
// the owner and respawns onto a shell somebody else is still using.
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
return isSshPtyExitedError(err) || message.includes(SSH_SESSION_EXPIRED_ERROR)
|
||||
return message.includes(SSH_SESSION_EXPIRED_ERROR)
|
||||
}
|
||||
|
||||
function isPtyAlreadyGoneError(err: unknown): boolean {
|
||||
|
||||
+33
-14
@@ -2850,6 +2850,10 @@ export type PtyBindingSourceExpectation = {
|
||||
}
|
||||
|
||||
|
||||
/** Expired leases are unreachable from reattach; kept only long enough to outlive any in-flight
|
||||
* recovery window by orders of magnitude. */
|
||||
const EXPIRED_SSH_LEASE_RETENTION_MS = 7 * 24 * 60 * 60 * 1000
|
||||
|
||||
/**
|
||||
* Fold SSH pane membership left in `workspaceSessionsByHostId["ssh:<target>"]` into the local
|
||||
* session, which is the only plane the binding writer maintains.
|
||||
@@ -2864,6 +2868,9 @@ export function hoistSshPartitionsIntoLocalSession(
|
||||
/** Passed explicitly: pane-identity normalization prunes partitions before this runs. */
|
||||
sourcePartitions?: PersistedState['workspaceSessionsByHostId']
|
||||
): boolean {
|
||||
if (state.settings?.sshPartitionHoistSeed === 'done') {
|
||||
return false
|
||||
}
|
||||
const partitions = sourcePartitions ?? state.workspaceSessionsByHostId
|
||||
if (!partitions) {
|
||||
return false
|
||||
@@ -2913,16 +2920,12 @@ export function hoistSshPartitionsIntoLocalSession(
|
||||
local.activeTabIdByWorktree = { ...local.activeTabIdByWorktree, [worktreeId]: tabId }
|
||||
changed = true
|
||||
}
|
||||
// Consume the source. A migration that leaves it behind runs again every launch and
|
||||
// resurrects panes the user has since closed, because "local has no such tab" is exactly
|
||||
// what this function treats as "needs hoisting".
|
||||
if (state.workspaceSessionsByHostId?.[hostId]) {
|
||||
const remaining = { ...state.workspaceSessionsByHostId }
|
||||
delete remaining[hostId]
|
||||
state.workspaceSessionsByHostId = remaining
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
// Mark it done even when nothing moved. The partition itself must SURVIVE — headless and CLI
|
||||
// own panes in that plane — so the only way to stop re-hoisting is to record that we have run.
|
||||
// Without this, "local has no such tab" is both the hoist condition and the state produced by
|
||||
// the user closing a tab, so closed panes came back on the next launch.
|
||||
state.settings = { ...state.settings, sshPartitionHoistSeed: 'done' }
|
||||
return changed
|
||||
}
|
||||
|
||||
@@ -2984,13 +2987,15 @@ export class Store {
|
||||
fallbackSnapshotRoot: legacySnapshotRoot === profileSnapshotRoot ? null : legacySnapshotRoot
|
||||
}
|
||||
const loaded = this.load()
|
||||
const normalized = normalizePersistedPaneIdentityState(loaded)
|
||||
this.state = normalized.state
|
||||
// Why: earlier builds wrote SSH pane membership into `ssh:<target>` partitions; this build's
|
||||
// binding writer is local-only and its reattach refuses to create, so an unhoisted partition
|
||||
// means every pane is refused and the user's tabs are discarded. Runs after pane-identity
|
||||
// normalization, which rebuilds the session and would otherwise drop the hoisted panes.
|
||||
if (hoistSshPartitionsIntoLocalSession(this.state, this.sshPartitionsAtLoad)) {
|
||||
// means every pane is refused and the user's tabs are discarded. Runs BEFORE pane-identity
|
||||
// normalization so hoisted panes are normalized with everything else — a partition can carry
|
||||
// a legacy non-UUID leaf, and injecting one raw re-creates the same tab loss for that pane.
|
||||
const hoisted = hoistSshPartitionsIntoLocalSession(loaded, this.sshPartitionsAtLoad)
|
||||
const normalized = normalizePersistedPaneIdentityState(loaded)
|
||||
this.state = normalized.state
|
||||
if (hoisted) {
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
// Why: activeView is a frequent, tiny preference; keeping it beside the
|
||||
@@ -4002,6 +4007,20 @@ export class Store {
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
|
||||
// Why: an expired lease is unreachable from every reattach path, and only host deletion ever
|
||||
// removed one, so the list grew for the life of the profile. Retention sits far above the
|
||||
// pane-recovery grace window, which is the only remaining reader.
|
||||
const retainedLeases = (result.sshRemotePtyLeases ?? []).filter(
|
||||
(lease) =>
|
||||
lease.state !== 'expired' ||
|
||||
!lease.updatedAt ||
|
||||
Date.now() - lease.updatedAt <= EXPIRED_SSH_LEASE_RETENTION_MS
|
||||
)
|
||||
if (retainedLeases.length !== (result.sshRemotePtyLeases ?? []).length) {
|
||||
result = { ...result, sshRemotePtyLeases: retainedLeases }
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
|
||||
if (gcStaleWorktreeMeta(result) > 0) {
|
||||
this.loadNeedsSave = true
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
* survivors blank", and no oracle covered it because every test writes both planes consistently.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
|
||||
import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { getDefaultPersistedState, getDefaultWorkspaceSession } from '../shared/constants'
|
||||
@@ -44,6 +44,17 @@ async function createStore(state: Record<string, unknown> = {}) {
|
||||
return new Store()
|
||||
}
|
||||
|
||||
/** A pre-upgrade profile has the partition and no hoist marker; the marker only exists because
|
||||
* this build writes it. Stripping it models the on-disk state an upgrading user actually has. */
|
||||
function stripHoistMarker(): void {
|
||||
const file = join(testState.dir, 'orca-data.json')
|
||||
const state = JSON.parse(readFileSync(file, 'utf-8')) as {
|
||||
settings?: { sshPartitionHoistSeed?: string }
|
||||
}
|
||||
delete state.settings?.sshPartitionHoistSeed
|
||||
writeFileSync(file, JSON.stringify(state), 'utf-8')
|
||||
}
|
||||
|
||||
/** Reopen the SAME profile — the upgrade launch. The hoist migration runs at load, so a test that
|
||||
* writes the partition into an already-loaded store would never exercise it. */
|
||||
async function reopenStore() {
|
||||
@@ -100,6 +111,7 @@ describe('an SSH pane whose membership was persisted into the ssh partition', ()
|
||||
).toHaveLength(1)
|
||||
expect(before.getWorkspaceSession().tabsByWorktree?.[WORKTREE] ?? []).toHaveLength(0)
|
||||
before.flushOrThrow()
|
||||
stripHoistMarker()
|
||||
|
||||
// The upgrade launch.
|
||||
const store = await reopenStore()
|
||||
@@ -131,6 +143,7 @@ describe('an SSH pane whose membership was persisted into the ssh partition', ()
|
||||
const before = await createStore()
|
||||
before.setWorkspaceSession(paneSession() as never, SSH_PARTITION)
|
||||
before.flushOrThrow()
|
||||
stripHoistMarker()
|
||||
const store = await reopenStore()
|
||||
|
||||
const bound = store.persistPtyBinding({
|
||||
@@ -164,6 +177,7 @@ describe('an SSH pane whose membership was persisted into the ssh partition', ()
|
||||
const before = await createStore()
|
||||
before.setWorkspaceSession(paneSession() as never, SSH_PARTITION)
|
||||
before.flushOrThrow()
|
||||
stripHoistMarker()
|
||||
|
||||
// The upgrade launch hoists the pane.
|
||||
const upgraded = await reopenStore()
|
||||
|
||||
@@ -3210,6 +3210,8 @@ export type GlobalSettings = {
|
||||
/** One-shot cohort marker for the tab-switch keybinding swap. 'pending' =
|
||||
* pre-existing install (seed pins old chords, then flips to 'done'); 'done' = fresh install. */
|
||||
tabSwitchKeybindingSeed?: 'pending' | 'done'
|
||||
/** One-shot: SSH host session partitions have been folded into the local session. */
|
||||
sshPartitionHoistSeed?: 'done'
|
||||
/** Local voice/dictation config. Optional for pre-voice profiles; getDefaultSettings() hydrates defaults via the persistence merge. */
|
||||
voice?: VoiceSettings
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user