mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 08:02:31 +00:00
fix(native-chat): capture awaits-user on the marker instead of re-deriving it
The awaits-user clause could never fire. It asked the live projection for
`attention`, which needs a prompt whose resolution is still `pending` -- but
teardown CANCELS that prompt a few phases after it writes the marker. By the next
launch the evidence is gone, for precisely the sessions the clause was written
for. QA measured the injection still being offered and then resumed.
This is the same shape as the leaf-drift bug: state read after teardown is not the
state that justified the marker. The discriminator, now applied across the whole
predicate:
- a fact teardown itself destroys or mutates must be CAPTURED on the marker
while it is still true;
- a fact that evolves on its own must be RE-DERIVED at read time, never
snapshotted.
So `awaitsUser` is now recorded at teardown and the predicate reads the recorded
value. Teardown still declines to mint a marker for such a session, so the
recorded flag is the second line rather than the only one.
Audit of every other clause against the same test:
- turn id (captured) -- teardown rewrites turn STATE but never the id. Correct.
- provider handle root (captured) -- the close path appends a resumed link, and
appendAgentSessionProviderHandleLink refuses one that changes the root, so the
root is invariant under exactly the mutation that broke the key. Correct.
- turn state (re-derived) -- DELIBERATE exception, stated here rather than left
implicit: we are not reading the state that justified the marker, we are
reading teardown's receipt that it settled the turn. A turn still `running`
means eviction never finished, and we refuse. Correct, and intentionally so.
- lease reconciled / released / handoff stage (re-derived) -- these answer a
different, launch-time question: may this host take the lease NOW. The
teardown-time value would be meaningless, and `unreconciled` is cleared by
this launch's own reconciliation. Correct.
- adapter support, marker TTL, marker consumption (re-derived) -- all evolve
independently of teardown. Correct.
Only awaitsUser was on the wrong side.
This commit is contained in:
+1
-7
@@ -9,8 +9,7 @@ import type { AgentSessionRecordStore } from '../../runtime/agent-session-record
|
||||
import type { AgentSessionResumeTrigger } from '../../../shared/agent-session-resume-marker'
|
||||
import {
|
||||
latestStructuredAgentSessionPrompt,
|
||||
newestStructuredAgentSessionTurn,
|
||||
projectStructuredAgentSessionStatus
|
||||
newestStructuredAgentSessionTurn
|
||||
} from '../../../shared/structured-agent-session-projection'
|
||||
import type { AgentSessionResumeMarker } from '../../../shared/agent-session-resume-marker'
|
||||
import type { AgentSessionJournal } from '../agent-session-journal/journal-store'
|
||||
@@ -77,11 +76,6 @@ export function createStructuredAgentSessionRestartResume(
|
||||
getRecord: deps.store.getRecord,
|
||||
supportsRecord: (record) => adapterSupportsRecord(deps.adapter, record),
|
||||
journalTurn: (sessionId) => newestStructuredAgentSessionTurn(itemsFor(sessionId)),
|
||||
// The projection tests for a pending approval or question BEFORE it looks at turn state, so
|
||||
// it still reports `attention` after eviction has rewritten the turn to `interrupted`. That
|
||||
// makes it the durable signal, and the same one teardown gates on.
|
||||
awaitsUser: (sessionId) =>
|
||||
projectStructuredAgentSessionStatus(itemsFor(sessionId)) === 'attention',
|
||||
latestPrompt: (sessionId) => latestStructuredAgentSessionPrompt(itemsFor(sessionId)),
|
||||
now: surfaces.now(),
|
||||
leaseState
|
||||
|
||||
+4
-6
@@ -39,8 +39,6 @@ export type StructuredAgentSessionResumeSetInput = {
|
||||
* not be read. Deliberately not the live-turn reader: eviction has already rewritten that turn
|
||||
* to `interrupted` by the time this runs. */
|
||||
journalTurn: (sessionId: string) => AgentJournalTurnLifecycle | null
|
||||
/** Whether the chat is blocked on the USER — a pending approval or question. */
|
||||
awaitsUser: (sessionId: string) => boolean
|
||||
latestPrompt: (sessionId: string) => string
|
||||
now: number
|
||||
/**
|
||||
@@ -86,10 +84,10 @@ export function structuredAgentSessionResumableSet(
|
||||
if (turn.state !== 'interrupted' && turn.state !== 'unverifiable') {
|
||||
continue
|
||||
}
|
||||
// Symmetric with teardown, which already refuses to MINT a marker for a chat awaiting the user.
|
||||
// Without the same clause here an injected or pre-existing marker would still be offered — the
|
||||
// exact asymmetry the completed-turn case had.
|
||||
if (input.awaitsUser(marker.sessionId)) {
|
||||
// Read off the MARKER, never re-derived. Teardown cancels the pending prompt a few phases after
|
||||
// it writes the marker, so by now the live journal no longer reports `attention` for exactly the
|
||||
// sessions this refuses — which is what made the re-derived version inert.
|
||||
if (marker.awaitsUser) {
|
||||
continue
|
||||
}
|
||||
candidates.push({
|
||||
|
||||
+8
-16
@@ -13,7 +13,6 @@ import {
|
||||
type AgentSessionResumeMarker
|
||||
} from '../../../shared/agent-session-resume-marker'
|
||||
import { newestStructuredAgentSessionTurn } from '../../../shared/structured-agent-session-live-turn'
|
||||
import { projectStructuredAgentSessionStatus } from '../../../shared/structured-agent-session-projection'
|
||||
import { structuredAgentSessionResumableSet } from './structured-agent-session-restart-resume-set'
|
||||
import {
|
||||
resumeStructuredAgentSessionsFromRestart,
|
||||
@@ -137,6 +136,7 @@ function marker(overrides: Partial<AgentSessionResumeMarker> = {}): AgentSession
|
||||
recordedAt: NOW,
|
||||
trigger: 'quit',
|
||||
providerHandleRoot: HANDLE_ROOT,
|
||||
awaitsUser: false,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
@@ -153,9 +153,6 @@ function resumableSet(input: {
|
||||
getRecord: () => record(input.chain === undefined ? {} : { chain: input.chain }),
|
||||
supportsRecord: () => true,
|
||||
journalTurn: () => newestStructuredAgentSessionTurn(items),
|
||||
// Derived from the same items the real host reads, so adding a pending prompt to a fixture
|
||||
// exercises this exactly as production would.
|
||||
awaitsUser: () => projectStructuredAgentSessionStatus(items) === 'attention',
|
||||
latestPrompt: () => 'fix the auth bug',
|
||||
now: input.now ?? NOW
|
||||
})
|
||||
@@ -178,6 +175,7 @@ describe('deriving what was working at teardown', () => {
|
||||
turnId: 'turn-1',
|
||||
recordedAt: NOW,
|
||||
trigger: 'quit',
|
||||
awaitsUser: false,
|
||||
providerHandleRoot: HANDLE_ROOT
|
||||
}
|
||||
])
|
||||
@@ -335,7 +333,6 @@ describe('the resumable set', () => {
|
||||
getRecord: () => claudeRecord('5aed93d6-advanced-leaf'),
|
||||
supportsRecord: () => true,
|
||||
journalTurn: () => ({ turnId: 'turn-1', state: 'interrupted' }),
|
||||
awaitsUser: () => false,
|
||||
latestPrompt: () => '',
|
||||
now: NOW
|
||||
})
|
||||
@@ -350,7 +347,6 @@ describe('the resumable set', () => {
|
||||
getRecord: () => claudeRecord(null, 'prov-session-2'),
|
||||
supportsRecord: () => true,
|
||||
journalTurn: () => ({ turnId: 'turn-1', state: 'interrupted' }),
|
||||
awaitsUser: () => false,
|
||||
latestPrompt: () => '',
|
||||
now: NOW
|
||||
})
|
||||
@@ -371,16 +367,11 @@ describe('the resumable set', () => {
|
||||
)
|
||||
})
|
||||
|
||||
// Teardown already refuses to MINT a marker for this, but the set predicate needs the same clause
|
||||
// or a marker that exists by any other route is offered. The projection still says `attention`
|
||||
// here because it tests for a pending prompt before it looks at turn state.
|
||||
it('refuses a marker for a chat that is blocked on the user', () => {
|
||||
expect(
|
||||
resumableSet({
|
||||
markers: [marker()],
|
||||
items: [turnItem('turn-1', 'interrupted'), pendingApproval()]
|
||||
})
|
||||
).toEqual([])
|
||||
// The flag is CAPTURED at teardown because teardown then cancels the prompt: by the time this
|
||||
// predicate runs, the live journal no longer reports `attention`, so only the recorded value can
|
||||
// still refuse. Re-deriving it here was inert for exactly the sessions it was written for.
|
||||
it('refuses a marker recorded while the chat was blocked on the user', () => {
|
||||
expect(resumableSet({ markers: [marker({ awaitsUser: true })] })).toEqual([])
|
||||
})
|
||||
|
||||
it('offers a turn whose end the host could not verify', () => {
|
||||
@@ -548,6 +539,7 @@ describe('the restart-resume surface', () => {
|
||||
turnId: 'turn-2',
|
||||
recordedAt: NOW,
|
||||
trigger: 'update',
|
||||
awaitsUser: false,
|
||||
providerHandleRoot: HANDLE_ROOT
|
||||
}
|
||||
])
|
||||
|
||||
+6
-1
@@ -43,10 +43,14 @@ export function structuredAgentSessionsWorkingAtTeardown(input: {
|
||||
continue
|
||||
}
|
||||
const snapshot = session.journal.snapshot()
|
||||
const status = projectStructuredAgentSessionStatus(snapshot.items, snapshot.submissions)
|
||||
// Captured HERE, while it is still true. A later teardown phase cancels the pending prompt, so
|
||||
// nothing downstream can re-derive this fact — the marker has to carry it.
|
||||
const awaitsUser = status === 'attention'
|
||||
// The product's own classification, so the marker rule cannot disagree with what the UI calls
|
||||
// working. A turn blocked on an approval or a question projects as `attention`: the agent is
|
||||
// waiting on the USER, and that is not interrupted work to hand back.
|
||||
if (projectStructuredAgentSessionStatus(snapshot.items, snapshot.submissions) !== 'working') {
|
||||
if (status !== 'working') {
|
||||
continue
|
||||
}
|
||||
const turnId = activeStructuredAgentSessionTurnId(snapshot.items)
|
||||
@@ -64,6 +68,7 @@ export function structuredAgentSessionsWorkingAtTeardown(input: {
|
||||
turnId,
|
||||
recordedAt: input.now,
|
||||
trigger: input.trigger,
|
||||
awaitsUser,
|
||||
// Root, not key: the close path advances Claude's leaf moments after this runs, and a key
|
||||
// comparison would then refuse the session forever.
|
||||
providerHandleRoot: agentSessionProviderHandleRoot(head.handle)
|
||||
|
||||
@@ -22,6 +22,7 @@ function marker(overrides: Partial<AgentSessionResumeMarker> = {}): AgentSession
|
||||
recordedAt: NOW,
|
||||
trigger: 'quit',
|
||||
providerHandleRoot: 'codex:"thread-1"',
|
||||
awaitsUser: false,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,6 +34,14 @@ export type AgentSessionResumeMarker = {
|
||||
* preserve — a resume that changes it forked — which is exactly what this guard is for.
|
||||
*/
|
||||
providerHandleRoot: string
|
||||
/**
|
||||
* Whether the chat was blocked on the USER — a pending approval or question — at teardown.
|
||||
*
|
||||
* CAPTURED, never re-derived, because teardown itself destroys the evidence: a later phase
|
||||
* cancels the pending prompt, so by the next launch the projection no longer reports `attention`
|
||||
* for precisely the sessions this exists to refuse. Re-reading it was inert.
|
||||
*/
|
||||
awaitsUser: boolean
|
||||
}
|
||||
|
||||
const MAX_FIELD_LENGTH = 512
|
||||
@@ -51,6 +59,7 @@ export function isAgentSessionResumeMarker(value: unknown): value is AgentSessio
|
||||
isMarkerField(marker.sessionId) &&
|
||||
isMarkerField(marker.turnId) &&
|
||||
isMarkerField(marker.providerHandleRoot) &&
|
||||
typeof marker.awaitsUser === 'boolean' &&
|
||||
Number.isSafeInteger(marker.recordedAt) &&
|
||||
(marker.recordedAt as number) >= 0 &&
|
||||
(marker.trigger === 'quit' || marker.trigger === 'update')
|
||||
|
||||
Reference in New Issue
Block a user