mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(daemon): answer the per-pty snapshot predicate for the pty it was asked about (#21381)
canProvideAuthoritativeBufferSnapshot is contracted as "whether this exact PTY can return a sequence-safe provider snapshot" (pty-provider-contract.ts), and two of the three layers already route it per id: DaemonPtyRouter forwards to adapterFor(id), and DegradedDaemonPtyProvider forwards to the provider that owns the session. The daemon adapter was the leaf that discarded the id and returned supportsAuthoritativeBufferSnapshots — a negotiated protocol version, which is a fact about the connection, not about a pty. That is reachable, not theoretical. getProviderForPty falls back to the local provider for any id it cannot place, so a remote-runtime id (whose pty lives on another machine) resolves to the local daemon adapter, and pty:getAuthoritativeBufferSnapshotCapabilities answered `true` for a session this daemon has never owned. The renderer caches that as a definitive per-pty verdict, and because the leaf discarded the id it could not tell it had been asked about something it does not own. Today the wrong answer is masked: allowOrdinaryParkRestore short-circuits remote and SSH ptys before the cached verdict is read, so nothing consults it. This closes the gap before something relies on it — a caller reaching for a per-pty answer should not be handed a confident one that is wrong. Not touching that short-circuit. It is deliberate: SSH bytes transit the client's own main process into its headless mirror, so those panes have a local copy the predicate says nothing about, and the direct-SSH lane was confirmed to repaint from a daemon-backed restore with the park capture disabled entirely. Routing SSH around a daemon-snapshot predicate is correct, and removing the short-circuit would disable SSH parking for no correctness gain. The existing protocol-compatibility test asserted `true` for a made-up session id, which encoded the bug. It now spawns a real session, so it still proves the protocol-version gate without depending on an unowned id reading as supported.
This commit is contained in:
@@ -243,16 +243,29 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
})
|
||||
|
||||
describe('background stream thinning compatibility', () => {
|
||||
it('reports authoritative snapshot support only for the corrected serializer protocol', () => {
|
||||
it('reports authoritative snapshot support only for the corrected serializer protocol', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 80, rows: 24 })
|
||||
const legacy = new DaemonPtyAdapter({ socketPath, tokenPath, protocolVersion: 31 })
|
||||
try {
|
||||
expect(legacy.canProvideAuthoritativeBufferSnapshot('legacy-session')).toBe(false)
|
||||
expect(adapter.canProvideAuthoritativeBufferSnapshot('current-session')).toBe(true)
|
||||
expect(legacy.canProvideAuthoritativeBufferSnapshot(id)).toBe(false)
|
||||
expect(adapter.canProvideAuthoritativeBufferSnapshot(id)).toBe(true)
|
||||
} finally {
|
||||
legacy.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
// Why this matters beyond tidiness: getProviderForPty falls back to the local provider for
|
||||
// any id it cannot place, so these ids reach this adapter for real. Answered from the
|
||||
// protocol flag alone they came back `true`, which the renderer caches as a definitive
|
||||
// per-pty licence to unmount a pane whose bytes this daemon never held.
|
||||
it('refuses an authoritative snapshot claim for a session it does not own', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
expect(adapter.canProvideAuthoritativeBufferSnapshot(id)).toBe(true)
|
||||
expect(adapter.canProvideAuthoritativeBufferSnapshot('remote:env-1:pty-1')).toBe(false)
|
||||
expect(adapter.canProvideAuthoritativeBufferSnapshot('never-spawned-session')).toBe(false)
|
||||
})
|
||||
|
||||
it('reports background state on the authoritative-snapshot protocol', () => {
|
||||
const notifySpy = vi.spyOn(DaemonClient.prototype, 'notify')
|
||||
try {
|
||||
|
||||
@@ -237,8 +237,12 @@ export abstract class DaemonPtyRuntimeState {
|
||||
return this.protocolVersion >= GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION
|
||||
}
|
||||
|
||||
canProvideAuthoritativeBufferSnapshot(_id: string): boolean {
|
||||
return this.supportsAuthoritativeBufferSnapshots
|
||||
// Why the id is read rather than ignored: the contract promises a fact about THIS pty, and
|
||||
// getProviderForPty falls back to the local provider for any id it cannot place. A
|
||||
// remote-runtime id therefore reaches this adapter, and answering from the protocol flag
|
||||
// alone returned `true` for a session this daemon has never owned.
|
||||
canProvideAuthoritativeBufferSnapshot(id: string): boolean {
|
||||
return this.supportsAuthoritativeBufferSnapshots && this.activeSessionIds.has(id)
|
||||
}
|
||||
|
||||
protected get canDelegateBackgroundToDaemon(): boolean {
|
||||
|
||||
Reference in New Issue
Block a user