mirror of
https://github.com/stablyai/orca.git
synced 2026-10-25 00:03:59 +00:00
fix(ssh): revoke stale hydration and keep authority unverifiable when unplaced
Two holes in the previous commit, both found in review. The hydrated set is add-only (ssh.ts), so withholding hydration only protects a target that never synced. A target that synced cleanly and then reconnected with a degraded lineage kept its flag, and hydration is what authorises uploads - so it would still send a replace-session patch built from the incomplete picture and delete the host tabs it had just failed to place. Hydration is now revoked, not merely withheld. The status phase was 'error'. workspace-terminal-host-authority.ts treats 'offline'/'error' on an un-hydrated target as its bounded floor and resolves them to 'none' - which authorises seeding AND sleeping-agent resume, the exact double-resume this gate exists to prevent. 'conflict' is the phase that actually describes the situation, is excluded from uploads by use-app-session-persistence, and is deliberately outside that floor set, so authority stays 'unverifiable'. Both invariants are pinned by tests verified to fail when either fix is reverted individually; the pre-existing tests passed with both reverted.
This commit is contained in:
@@ -160,16 +160,20 @@ export async function applyDirectSshRemoteWorkspaceSnapshot({
|
||||
lastSyncedAt: Date.now()
|
||||
})
|
||||
} else {
|
||||
// Why not hydrated: the host listed tabs on paths this client cannot place, so adopting zero
|
||||
// of them is not the host's picture. Hydrating would authorise uploads
|
||||
// (use-app-session-persistence.ts), and an upload is a `replace-session` patch
|
||||
// (remote-workspace-relay-sync.ts) that wholesale replaces the host snapshot — deleting the
|
||||
// very tabs we failed to adopt. It would also flip the seeding gate in
|
||||
// workspace-terminal-host-authority.ts from `unverifiable` to `none`, authorising a fresh
|
||||
// default tab beside the host's orphaned ones (STA-3593). The next connect or host push
|
||||
// re-pulls, so leaving this unresolved costs a retry, not the data.
|
||||
// The host listed tabs on paths this client cannot place, so adopting zero of them is not
|
||||
// the host's picture. `conflict` is the phase that says exactly that, and it is load-bearing
|
||||
// twice over:
|
||||
// - use-app-session-persistence.ts filters conflicted targets out of uploads, and an
|
||||
// upload is a `replace-session` patch (remote-workspace-relay-sync.ts) that wholesale
|
||||
// replaces the host snapshot - it would delete the very tabs we failed to adopt;
|
||||
// - workspace-terminal-host-authority.ts treats `offline`/`error` on an un-hydrated target
|
||||
// as `none`, its bounded floor, which authorises seeding AND sleeping-agent resume.
|
||||
// `conflict` is deliberately not in that set, so authority stays `unverifiable`.
|
||||
// Hydration is cleared, not merely withheld: the set is add-only, so a target that synced
|
||||
// cleanly before would otherwise keep uploading from this incomplete picture (STA-3593).
|
||||
currentStore.clearRemoteWorkspaceHydrated(authority.targetId)
|
||||
currentStore.setRemoteWorkspaceSyncStatus(authority.targetId, {
|
||||
phase: 'error',
|
||||
phase: 'conflict',
|
||||
direction: 'pull',
|
||||
revision: snapshot.revision,
|
||||
updatedAt: snapshot.updatedAt,
|
||||
|
||||
@@ -24,6 +24,7 @@ import type { RemoteWorkspaceSnapshot } from '../../../shared/remote-workspace-t
|
||||
import type { DirectSshAuthority, SshProviderEpoch } from '../../../shared/ssh-types'
|
||||
import { createTestStore, makeWorktree } from '../store/slices/store-test-helpers'
|
||||
import { applyDirectSshRemoteWorkspaceSnapshot } from './remote-workspace-snapshot-apply'
|
||||
import { resolveWorkspaceTerminalHostAuthority } from '../lib/workspace-terminal-host-authority'
|
||||
import type { DirectSshSnapshotApplyToken } from './direct-ssh-reconnect-coordinator-types'
|
||||
|
||||
vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } }))
|
||||
@@ -208,6 +209,38 @@ describe('a host snapshot whose terminal tabs cannot be placed locally', () => {
|
||||
).not.toBe('synced')
|
||||
})
|
||||
|
||||
it('revokes a hydration it granted earlier when a later snapshot cannot be placed', async () => {
|
||||
const store = createStore()
|
||||
landHostLineage(store)
|
||||
await applySnapshot(store, snapshot(1))
|
||||
expect(isHydrated(store)).toBe(true)
|
||||
|
||||
// The lineage degrades on a later reconnect: the catalog rows this client had are gone.
|
||||
store.setState({ worktreesByRepo: {}, detectedWorktreesByRepo: {} })
|
||||
await applySnapshot(store, snapshot(2))
|
||||
|
||||
// Why revoke rather than merely withhold: the hydrated set is add-only, and hydration is what
|
||||
// authorises uploads. A stale flag would keep uploading this incomplete picture, and an upload
|
||||
// wholesale replaces the host snapshot — deleting the tabs we just failed to place.
|
||||
expect(
|
||||
isHydrated(store),
|
||||
'a stale hydration still authorises a replace-session upload from an incomplete picture'
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('leaves terminal authority unverifiable, so nothing seeds or resumes over the host', async () => {
|
||||
const store = createStore()
|
||||
await applySnapshot(store, snapshot(1))
|
||||
|
||||
// `offline`/`error` on an un-hydrated target are the authority resolver's bounded floor and
|
||||
// resolve to `none`, which authorises seeding AND sleeping-agent resume. An unplaced snapshot
|
||||
// must not land in that set: the host's tabs are live, we simply could not place them.
|
||||
expect(
|
||||
resolveWorkspaceTerminalHostAuthority(store.getState(), ALPHA_ID),
|
||||
'an unplaced snapshot authorised seeding over live host terminals'
|
||||
).toBe('unverifiable')
|
||||
})
|
||||
|
||||
it('adopts every host tab and declares the target hydrated once the catalog is present', async () => {
|
||||
const store = createStore()
|
||||
landHostLineage(store)
|
||||
@@ -221,6 +254,27 @@ describe('a host snapshot whose terminal tabs cannot be placed locally', () => {
|
||||
expect(syncPhase(store)).toBe('synced')
|
||||
})
|
||||
|
||||
it('recovers on the next snapshot once the catalog lands', async () => {
|
||||
const store = createStore()
|
||||
|
||||
// First pass: the lineage read was degraded, so nothing places and the target is left
|
||||
// un-hydrated on purpose. With the retry chain gone, this is the only way back.
|
||||
await applySnapshot(store, snapshot(1))
|
||||
expect(adoptedTabIds(store)).toEqual([])
|
||||
expect(isHydrated(store)).toBe(false)
|
||||
|
||||
// A later connect or host push arrives after the catalog landed.
|
||||
landHostLineage(store)
|
||||
await applySnapshot(store, snapshot(2))
|
||||
|
||||
expect(
|
||||
adoptedTabIds(store),
|
||||
'an un-hydrated target never recovered the host tabs it declined to guess at'
|
||||
).toEqual(['T1', 'T2', 'T3'])
|
||||
expect(isHydrated(store)).toBe(true)
|
||||
expect(syncPhase(store)).toBe('synced')
|
||||
})
|
||||
|
||||
it('still hydrates when the host genuinely reported no terminals', async () => {
|
||||
const store = createStore()
|
||||
expect(store.getState().worktreesByRepo).toEqual({})
|
||||
|
||||
@@ -120,6 +120,7 @@ function appState(overrides: Record<string, unknown> = {}): AppState {
|
||||
hydrateEditorSession: vi.fn(),
|
||||
hydrateBrowserSession: vi.fn(),
|
||||
markRemoteWorkspaceHydrated: vi.fn(),
|
||||
clearRemoteWorkspaceHydrated: vi.fn(),
|
||||
setRemoteWorkspaceSyncStatus: vi.fn(),
|
||||
reconnectPersistedTerminals: vi.fn(async () => {}),
|
||||
...overrides
|
||||
|
||||
Reference in New Issue
Block a user