mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 08:01:56 +00:00
fix(runtime): keep the retraction boundary out of the receipt bound
Bounding the removal watermark alongside the receipt ledger reintroduced the defect the watermark exists to prevent: past 512 retracted worktrees, evicting a boundary readmits every pre-close frame it was fencing, and a delayed list resurrects the closed tab. A boundary is not a cache. One number per worktree ever retracted on an environment is the cheaper price, and environment teardown drains it; only the receipt ledger stays bounded, by frame age. Split the orphan-adoption port by provenance so the last writer that disagreed with the surface-standing rule stops disagreeing. `adoptRuntimeTerminalOrphans` replays the persisted binding when the claim already matches it and writes a new one otherwise, and both went through a single `recordSurface` that stamped the current graph sequence — so re-adopting an already-adopted orphan lifted a dropped pane's stale stamp and reported it attached, in a quiet workspace possibly forever. The replay now names the pane without standing and the fresh claim takes spawn standing, like every other writer. Replace a receipt-count assertion that was vacuous for a map keyed by environment and worktree with the mirror state and freshness it was standing in for.
This commit is contained in:
@@ -1,5 +1,9 @@
|
||||
// @ts-nocheck -- mechanically split from OrcaRuntimeService; behavior is covered by AST equivalence and characterization tests.
|
||||
import { recordPtySurface } from './pty-recorded-surface-topology'
|
||||
import {
|
||||
recordPtySurface,
|
||||
spawnSurfaceClaimSequence,
|
||||
SURFACE_CLAIM_WITHOUT_STANDING
|
||||
} from './pty-recorded-surface-topology'
|
||||
import {
|
||||
observeStructuredWorker,
|
||||
resolveStructuredWorkerAuthority
|
||||
@@ -61,8 +65,10 @@ export class OrcaRuntimeWithAdoptTerminalOrphansFromInventory extends OrcaRuntim
|
||||
getPty: (handle) => this.getLivePtyForHandle(handle)?.pty ?? null,
|
||||
getLeaves: (ptyId) => this.getLeavesForPty(ptyId),
|
||||
getLeaf: (tabId, leafId) => this.leaves.get(this.getLeafKey(tabId, leafId)),
|
||||
recordSurface: (pty, tabId, paneKey) =>
|
||||
recordPtySurface(pty, tabId, paneKey, this.graphSequence),
|
||||
replayPersistedSurface: (pty, tabId, paneKey) =>
|
||||
recordPtySurface(pty, tabId, paneKey, SURFACE_CLAIM_WITHOUT_STANDING),
|
||||
recordAdoptedSurface: (pty, tabId, paneKey) =>
|
||||
recordPtySurface(pty, tabId, paneKey, spawnSurfaceClaimSequence(this.graphSequence)),
|
||||
getMobileSnapshots: () => this.mobileSessionTabsByWorktree.values(),
|
||||
getSession: (worktreeId) => this.getWorkspaceSessionForWorktree(worktreeId),
|
||||
setSession: (worktreeId, next) => this.setWorkspaceSessionForWorktree(worktreeId, next),
|
||||
|
||||
@@ -19,7 +19,10 @@ type RuntimeTerminalOrphanAdoptionPorts = {
|
||||
getPty: (handle: string) => RuntimePtyWorktreeRecord | null
|
||||
getLeaves: (ptyId: string) => readonly RuntimeLeafRecord[]
|
||||
getLeaf: (tabId: string, leafId: string) => RuntimeLeafRecord | undefined
|
||||
recordSurface: (pty: RuntimePtyWorktreeRecord, tabId: string, paneKey: string) => void
|
||||
/** Replays a binding the session already held: names the pane without claiming the graph holds it. */
|
||||
replayPersistedSurface: (pty: RuntimePtyWorktreeRecord, tabId: string, paneKey: string) => void
|
||||
/** Names a pane this adoption just wrote, ahead of the graph statement that will carry it. */
|
||||
recordAdoptedSurface: (pty: RuntimePtyWorktreeRecord, tabId: string, paneKey: string) => void
|
||||
getMobileSnapshots: () => Iterable<RuntimeMobileSessionTabsSnapshot>
|
||||
getSession: (worktreeId: string) => WorkspaceSessionState | null
|
||||
setSession: (worktreeId: string, session: WorkspaceSessionState) => void
|
||||
@@ -135,7 +138,7 @@ export async function adoptRuntimeTerminalOrphansFromInventory(args: {
|
||||
})
|
||||
if (isExactPersisted && sessionWorktreeId === workspace.id) {
|
||||
for (const { claim, pty, paneKey } of validated) {
|
||||
ports.recordSurface(pty, claim.tabId, paneKey)
|
||||
ports.replayPersistedSurface(pty, claim.tabId, paneKey)
|
||||
}
|
||||
return {
|
||||
adopted: false,
|
||||
@@ -235,7 +238,7 @@ export async function adoptRuntimeTerminalOrphansFromInventory(args: {
|
||||
throw error
|
||||
}
|
||||
for (const { claim, pty, paneKey } of validated) {
|
||||
ports.recordSurface(pty, claim.tabId, paneKey)
|
||||
ports.recordAdoptedSurface(pty, claim.tabId, paneKey)
|
||||
}
|
||||
ports.hydrateSession(workspace.id)
|
||||
ports.notifySessionChanged(workspace.id)
|
||||
|
||||
+33
@@ -148,6 +148,39 @@ describe('a removal frame must not retire the publisher that is still live', ()
|
||||
}
|
||||
})
|
||||
|
||||
/**
|
||||
* The boundary must outlive the bound. A ledger entry may be dropped once nothing can be ranked
|
||||
* against it, but dropping a retraction boundary readmits every pre-close frame it was fencing —
|
||||
* so a long-lived session that has seen many worktrees must not lose the one thing standing
|
||||
* between a stale list and a resurrected tab.
|
||||
*/
|
||||
it('keeps a worktree fence after enough other worktrees to evict its receipt', () => {
|
||||
const liveReceived = recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, liveFrame(1))
|
||||
expect(admits(liveFrame(1), liveReceived)).toBe(true)
|
||||
|
||||
const delayedReceived = nextReceivedSessionTabsFrame()
|
||||
const removedReceived = recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, removalFrame())
|
||||
expect(admits(removalFrame(), removedReceived)).toBe(true)
|
||||
|
||||
// Churn other worktrees through the same open-and-close cycle, past the bound and past the
|
||||
// frame-age horizon, so both ledgers are over capacity when the delayed list finally lands.
|
||||
for (let index = 0; index < MAX_TRACKED_SESSION_TABS_RECEIPTS + 16; index += 1) {
|
||||
const worktree = `repo::/churn-${index}`
|
||||
recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, { ...liveFrame(1), worktree })
|
||||
recordReceivedWebSessionTabsSnapshot(ENVIRONMENT_ID, { ...removalFrame(), worktree })
|
||||
}
|
||||
|
||||
const delayed = liveFrame(9)
|
||||
recordReceivedWebSessionTabsSnapshot(
|
||||
ENVIRONMENT_ID,
|
||||
delayed,
|
||||
delayedReceived,
|
||||
undefined,
|
||||
'bootstrap'
|
||||
)
|
||||
expect(admits(delayed, delayedReceived)).toBe(false)
|
||||
})
|
||||
|
||||
/**
|
||||
* A worktree the host has published nothing for still answers a forced list, with a synthesized
|
||||
* `none`/v0 frame that means "ask me later" (host-session-snapshot-authority.ts). Every
|
||||
|
||||
@@ -696,16 +696,18 @@ describe('useWebSessionTabsSync visibility collision recovery', () => {
|
||||
...makeTerminalSnapshot(index === 0 ? '-a' : '-b', index + 1)
|
||||
})
|
||||
}
|
||||
expect(_getWebSessionTabsReceiptTrackingCountsForTest().receipts).toBe(1)
|
||||
|
||||
for (const [index, recovery] of recoveries.entries()) {
|
||||
recovery.resolve(makeTerminalSnapshot(index === 0 ? '-a' : '-b', index + 1))
|
||||
}
|
||||
await act(settle)
|
||||
// The receipt slot is per worktree, so what repeated frames could grow is the tracking behind
|
||||
// it; the watermark stays absent because nothing retracted, and the mirror holds one tab.
|
||||
expect(_getWebSessionTabsReceiptTrackingCountsForTest()).toEqual({
|
||||
receipts: 1,
|
||||
removalWatermarks: 0
|
||||
})
|
||||
expect(_getWebSessionTabsTrackingCountsForTest().freshness).toBe(1)
|
||||
expect(useAppStore.getState().tabsByWorktree[WORKTREE]?.length).toBe(1)
|
||||
hook.unmount()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -127,6 +127,11 @@ export const latestReceivedSessionTabsInventoryFrameByEnvironment = new Map<stri
|
||||
* Highest `receivedFrame` at which this worktree was retracted. Raise-only: a frame reserved before
|
||||
* the retraction is stale evidence no matter what arrived since, so the boundary cannot be a slot a
|
||||
* later frame overwrites, nor conditional on a recovery happening to be in flight when it landed.
|
||||
*
|
||||
* Deliberately not size-bounded, unlike the receipt ledger beside it. Evicting a boundary readmits
|
||||
* every pre-close frame it was fencing, which is the defect this map exists to prevent; one number
|
||||
* per worktree ever retracted on an environment is a cheaper price, and the environment teardown
|
||||
* below drains it.
|
||||
*/
|
||||
export const sessionTabsRemovalWatermarkByWorktree = new Map<string, number>()
|
||||
export const trackedSessionTabsWorktreeIdsByEnvironment = new Map<string, Set<string>>()
|
||||
|
||||
@@ -178,12 +178,7 @@ export function recordReceivedWebSessionTabsRemoval(
|
||||
}
|
||||
const watermark = sessionTabsRemovalWatermarkByWorktree.get(key) ?? 0
|
||||
if (receivedFrame > watermark) {
|
||||
setBoundedSessionTabsReceipt(
|
||||
sessionTabsRemovalWatermarkByWorktree,
|
||||
key,
|
||||
receivedFrame,
|
||||
(entry) => entry
|
||||
)
|
||||
sessionTabsRemovalWatermarkByWorktree.set(key, receivedFrame)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user