mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
Harden checkpoint cooldown against crash-boundary and reuse edges
From adversarial review of the cooldown commit: - Warm reattach by an adapter that was not already managing the session now forces a full-snapshot re-anchor: the previous adapter may have drained daemon records it never persisted (deferred hot-session tick), and appending past that unknown drain point would put a seq gap in the log, which the restore reader rejects wholesale. This also closes the pre-existing (previously ~ms-wide) drain-vs-persist crash window. - Cold-restore re-anchor clears any cooldown inherited from the pre-crash generation (daemon respawn within one adapter), so the revived session's first snapshot is never deferred. - Exit-event routing clears lastFullCheckpointAt (reused ptyIds must not inherit a dead session's cooldown); fanoutSyntheticExits now clears sessionsNeedingFullCheckpoint for consistency. - Backward wall-clock jumps count as expired instead of extending the deferral. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -1183,6 +1183,62 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('clears a stale snapshot cooldown when the cold-restore re-anchor is flagged', async () => {
|
||||
// Simulate a previous daemon crash with recoverable history on disk.
|
||||
const sessionId = 'cold-restore-stale-cooldown'
|
||||
const sessionDir = join(historyDir, getHistorySessionDirName(sessionId))
|
||||
mkdirSync(sessionDir, { recursive: true })
|
||||
writeFileSync(
|
||||
join(sessionDir, 'meta.json'),
|
||||
JSON.stringify({
|
||||
cwd: '/tmp',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
startedAt: '2026-04-15T10:00:00Z',
|
||||
endedAt: null,
|
||||
exitCode: null
|
||||
})
|
||||
)
|
||||
writeFileSync(join(sessionDir, 'scrollback.bin'), 'pre-crash output\r\n')
|
||||
|
||||
historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
const internals = historyAdapter as unknown as {
|
||||
lastFullCheckpointAt: Map<string, number>
|
||||
sessionsNeedingFullCheckpoint: Set<string>
|
||||
}
|
||||
// A daemon respawn inside one adapter keeps this map: seed a fresh
|
||||
// cooldown as if the pre-crash generation just snapshotted.
|
||||
internals.lastFullCheckpointAt.set(sessionId, Date.now())
|
||||
|
||||
await historyAdapter.spawn({ cols: 80, rows: 24, sessionId })
|
||||
|
||||
// The revived generation has no checkpoint of its own — the re-anchor
|
||||
// must not inherit the previous generation's cooldown.
|
||||
expect(internals.sessionsNeedingFullCheckpoint.has(sessionId)).toBe(true)
|
||||
expect(internals.lastFullCheckpointAt.has(sessionId)).toBe(false)
|
||||
})
|
||||
|
||||
it('re-anchors a warm reattach the adapter was not already managing', async () => {
|
||||
const sessionId = 'warm-reattach-reanchor'
|
||||
const first = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
await first.spawn({ cols: 80, rows: 24, sessionId })
|
||||
first.dispose()
|
||||
|
||||
// A fresh adapter (app relaunch) attaches to the still-live daemon
|
||||
// session. The old adapter may have drained records it never persisted
|
||||
// (deferred hot-session tick), so appends must not resume until a full
|
||||
// snapshot re-anchors the log.
|
||||
historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
const internals = historyAdapter as unknown as {
|
||||
sessionsNeedingFullCheckpoint: Set<string>
|
||||
lastFullCheckpointAt: Map<string, number>
|
||||
}
|
||||
const result = await historyAdapter.spawn({ cols: 80, rows: 24, sessionId })
|
||||
expect(result.isReattach).toBe(true)
|
||||
expect(internals.sessionsNeedingFullCheckpoint.has(sessionId)).toBe(true)
|
||||
expect(internals.lastFullCheckpointAt.has(sessionId)).toBe(false)
|
||||
})
|
||||
|
||||
it('returns same cold restore on StrictMode double-mount (sticky cache)', async () => {
|
||||
const sessionId = 'sticky-cache-test'
|
||||
const sessionDir = join(historyDir, getHistorySessionDirName(sessionId))
|
||||
|
||||
@@ -208,6 +208,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
}
|
||||
}
|
||||
|
||||
const wasAlreadyManaged = this.activeSessionIds.has(sessionId)
|
||||
this.activeSessionIds.add(sessionId)
|
||||
|
||||
// Cold restore: daemon created a new session but disk history shows
|
||||
@@ -231,6 +232,10 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
if (this.historyManager) {
|
||||
this.historyManager.registerWriter(sessionId)
|
||||
this.sessionsNeedingFullCheckpoint.add(sessionId)
|
||||
// Why: the revived generation has no valid checkpoint of its own; a
|
||||
// cooldown inherited from the pre-crash generation (daemon respawn
|
||||
// within one adapter) must not defer this re-anchor.
|
||||
this.lastFullCheckpointAt.delete(sessionId)
|
||||
}
|
||||
if (scrollback) {
|
||||
const coldRestore = { scrollback, cwd: restoreInfo.cwd, oscLinks: restoreInfo.oscLinks }
|
||||
@@ -254,6 +259,15 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
// without overwriting meta.json or deleting the existing checkpoint
|
||||
// (which is the only valid recovery data until the next tick).
|
||||
this.historyManager.registerWriter(sessionId)
|
||||
if (!wasAlreadyManaged) {
|
||||
// Why: a previous adapter may have drained daemon records it never
|
||||
// persisted (a deferred hot-session tick) before the app died.
|
||||
// Appending increments past that unknown drain point would put a seq
|
||||
// gap in the log, which the restore reader rejects wholesale. Force a
|
||||
// full snapshot to re-anchor before any further appends.
|
||||
this.sessionsNeedingFullCheckpoint.add(sessionId)
|
||||
this.lastFullCheckpointAt.delete(sessionId)
|
||||
}
|
||||
}
|
||||
|
||||
const isReattach = !result.isNew
|
||||
@@ -514,6 +528,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
this.activeSessionIds.clear()
|
||||
this.dirtySessionVersions.clear()
|
||||
this.lastFullCheckpointAt.clear()
|
||||
this.sessionsNeedingFullCheckpoint.clear()
|
||||
this.stopCheckpointTimer()
|
||||
for (const id of ids) {
|
||||
this.coldRestoreCache.delete(id)
|
||||
@@ -760,7 +775,13 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
// cold restore would find nothing.
|
||||
private isFullCheckpointCoolingDown(sessionId: string): boolean {
|
||||
const last = this.lastFullCheckpointAt.get(sessionId)
|
||||
return last !== undefined && Date.now() - last < DaemonPtyAdapter.FULL_CHECKPOINT_COOLDOWN_MS
|
||||
if (last === undefined) {
|
||||
return false
|
||||
}
|
||||
const elapsed = Date.now() - last
|
||||
// Why elapsed < 0 counts as expired: a backward wall-clock jump must not
|
||||
// extend the deferral window.
|
||||
return elapsed >= 0 && elapsed < DaemonPtyAdapter.FULL_CHECKPOINT_COOLDOWN_MS
|
||||
}
|
||||
|
||||
// Why 'deferred' exists: a cap/overflow-triggered full snapshot inside the
|
||||
@@ -960,6 +981,9 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
// full-checkpoint flag is dead state. Without this, a cold-restored
|
||||
// session that exits before its first checkpoint leaks a permanent entry.
|
||||
this.sessionsNeedingFullCheckpoint.delete(event.sessionId)
|
||||
// Why: a reused sessionId (renderer respawns a persisted ptyId) must
|
||||
// not inherit the dead session's snapshot cooldown.
|
||||
this.lastFullCheckpointAt.delete(event.sessionId)
|
||||
this.stopCheckpointTimerIfIdle()
|
||||
if (this.historyManager) {
|
||||
void this.historyManager
|
||||
|
||||
@@ -50,8 +50,9 @@ export class HistoryReader {
|
||||
}
|
||||
|
||||
// Why log replay is preferred over the checkpoint alone: the log carries
|
||||
// byte-exact output up to ~5s before the crash, while the checkpoint can
|
||||
// be a full log-cap (~5MB of output) stale.
|
||||
// byte-exact output up to ~5s before the crash (up to the full-snapshot
|
||||
// cooldown, ~45s, for a streaming session mid-deferral), while the
|
||||
// checkpoint can be a full log-cap (~5MB of output) stale.
|
||||
const logRestore = this.restoreFromIncrementalLog(sessionDir, meta, checkpoint)
|
||||
if (logRestore) {
|
||||
return logRestore
|
||||
|
||||
Reference in New Issue
Block a user