fix(agent): address initial review findings
This commit is contained in:
@@ -87,7 +87,11 @@ Sources: [`packages/core/session/src/types.ts:258`](../packages/core/session/src
|
|||||||
#### `agent/inbox/spliced` — log-only
|
#### `agent/inbox/spliced` — log-only
|
||||||
|
|
||||||
```ts persistence-catalog
|
```ts persistence-catalog
|
||||||
/** One normalized mutation of an agent's durable pending-message lists. */
|
/**
|
||||||
|
* One normalized mutation of an agent's durable pending-message lists.
|
||||||
|
* Live dispatch precedes projection mutation, so synchronous observers may
|
||||||
|
* read the pre-splice inbox to recover the removed messages.
|
||||||
|
*/
|
||||||
'agent/inbox/spliced': {
|
'agent/inbox/spliced': {
|
||||||
target: InboxTarget
|
target: InboxTarget
|
||||||
start: number
|
start: number
|
||||||
@@ -97,7 +101,7 @@ Sources: [`packages/core/session/src/types.ts:258`](../packages/core/session/src
|
|||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
Source: [`packages/core/agent/src/types.ts:268`](../packages/core/agent/src/types.ts)
|
Source: [`packages/core/agent/src/types.ts:272`](../packages/core/agent/src/types.ts)
|
||||||
|
|
||||||
### `approval/*`
|
### `approval/*`
|
||||||
|
|
||||||
|
|||||||
@@ -144,10 +144,10 @@ export class ReactLoopAgent implements Agent {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private async admit(onTurnBoundary: boolean): Promise<Admission> {
|
private async admit(onTurnBoundary: boolean): Promise<Admission> {
|
||||||
if (this.phase.kind !== 'running') throw new Error()
|
if (this.phase.kind !== 'running') throw new Error(`agent "${this.id}": admit outside running phase`)
|
||||||
const signal = this.phase.abort.signal
|
const signal = this.phase.abort.signal
|
||||||
const claimed = [...this.inbox.nextStep]
|
const claimed = [...this.inbox.nextStep]
|
||||||
const outboxLength = this.inbox.nextStep.length
|
const outboxLength = claimed.length
|
||||||
const queued = onTurnBoundary ? this.inbox.nextTurn[0] : undefined
|
const queued = onTurnBoundary ? this.inbox.nextTurn[0] : undefined
|
||||||
if (queued !== undefined) claimed.push(queued)
|
if (queued !== undefined) claimed.push(queued)
|
||||||
if (claimed.length === 0) return { kind: 'empty' }
|
if (claimed.length === 0) return { kind: 'empty' }
|
||||||
@@ -167,7 +167,7 @@ export class ReactLoopAgent implements Agent {
|
|||||||
|
|
||||||
/** Admitted input stays unowned until `turn/start` commits. */
|
/** Admitted input stays unowned until `turn/start` commits. */
|
||||||
private async turn(): Promise<boolean> {
|
private async turn(): Promise<boolean> {
|
||||||
if (this.phase.kind === 'idle') throw new Error()
|
if (this.phase.kind === 'idle') throw new Error(`agent "${this.id}": turn without driver reservation`)
|
||||||
const abort = this.phase.kind === 'collecting' ? this.phase.abort : new AbortController()
|
const abort = this.phase.kind === 'collecting' ? this.phase.abort : new AbortController()
|
||||||
const { signal } = abort
|
const { signal } = abort
|
||||||
const lastTurn = this.phase.kind === 'collecting' ? this.phase.lastTurn : this.phase.turn
|
const lastTurn = this.phase.kind === 'collecting' ? this.phase.lastTurn : this.phase.turn
|
||||||
@@ -227,7 +227,7 @@ export class ReactLoopAgent implements Agent {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private async step(): Promise<TurnEndReason | null> {
|
private async step(): Promise<TurnEndReason | null> {
|
||||||
if (this.phase.kind !== 'running') throw new Error()
|
if (this.phase.kind !== 'running') throw new Error(`agent "${this.id}": step outside running phase`)
|
||||||
const { turn, step, abort: { signal } } = this.phase
|
const { turn, step, abort: { signal } } = this.phase
|
||||||
signal.throwIfAborted()
|
signal.throwIfAborted()
|
||||||
await this.loopCtx.serial(agentCarrier(this), 'agent/step', this, turn, step, signal)
|
await this.loopCtx.serial(agentCarrier(this), 'agent/step', this, turn, step, signal)
|
||||||
|
|||||||
@@ -44,6 +44,9 @@ export class Inbox {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Apply standard splice semantics and durably record the normalized result.
|
* Apply standard splice semantics and durably record the normalized result.
|
||||||
|
* The durable event commits before the live projection mutates, so synchronous
|
||||||
|
* `session/event` observers see the pre-splice lists and can reconstruct the
|
||||||
|
* removed messages from the normalized coordinates.
|
||||||
* @param target - pending list to mutate.
|
* @param target - pending list to mutate.
|
||||||
* @param start - splice position.
|
* @param start - splice position.
|
||||||
* @param deleteCount - maximum number of messages to remove.
|
* @param deleteCount - maximum number of messages to remove.
|
||||||
@@ -59,12 +62,14 @@ export class Inbox {
|
|||||||
outcome?: 'admitted' | 'canceled',
|
outcome?: 'admitted' | 'canceled',
|
||||||
): UserMessage[] {
|
): UserMessage[] {
|
||||||
const inbox = this.state[target]
|
const inbox = this.state[target]
|
||||||
const offset = Math.trunc(start) || 0
|
const truncatedStart = Math.trunc(start)
|
||||||
|
const offset = Number.isNaN(truncatedStart) ? 0 : truncatedStart
|
||||||
const actualStart = offset < 0
|
const actualStart = offset < 0
|
||||||
? Math.max(inbox.length + offset, 0)
|
? Math.max(inbox.length + offset, 0)
|
||||||
: Math.min(offset, inbox.length)
|
: Math.min(offset, inbox.length)
|
||||||
|
const truncatedDeleteCount = Math.trunc(deleteCount)
|
||||||
const actualDeleteCount = Math.min(
|
const actualDeleteCount = Math.min(
|
||||||
Math.max(Math.trunc(deleteCount) || 0, 0),
|
Math.max(Number.isNaN(truncatedDeleteCount) ? 0 : truncatedDeleteCount, 0),
|
||||||
inbox.length - actualStart,
|
inbox.length - actualStart,
|
||||||
)
|
)
|
||||||
if (actualDeleteCount === 0 && inserted.length === 0) return []
|
if (actualDeleteCount === 0 && inserted.length === 0) return []
|
||||||
|
|||||||
@@ -264,7 +264,11 @@ declare module 'cordis' {
|
|||||||
|
|
||||||
declare module '@deepseek-ai/dsh-session' {
|
declare module '@deepseek-ai/dsh-session' {
|
||||||
interface SessionEventMap {
|
interface SessionEventMap {
|
||||||
/** One normalized mutation of an agent's durable pending-message lists. */
|
/**
|
||||||
|
* One normalized mutation of an agent's durable pending-message lists.
|
||||||
|
* Live dispatch precedes projection mutation, so synchronous observers may
|
||||||
|
* read the pre-splice inbox to recover the removed messages.
|
||||||
|
*/
|
||||||
'agent/inbox/spliced': {
|
'agent/inbox/spliced': {
|
||||||
target: InboxTarget
|
target: InboxTarget
|
||||||
start: number
|
start: number
|
||||||
|
|||||||
@@ -418,6 +418,7 @@ export function apply(ctx: Context): void {
|
|||||||
attempt.stale = true
|
attempt.stale = true
|
||||||
if (attempt.phase === 'admitted' && state.agent.status === 'running') {
|
if (attempt.phase === 'admitted' && state.agent.status === 'running') {
|
||||||
state.agent.cancel({ kind: 'parent' })
|
state.agent.cancel({ kind: 'parent' })
|
||||||
|
waits.push(state.agent.whenIdle())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (state.run !== undefined) waits.push(state.run)
|
if (state.run !== undefined) waits.push(state.run)
|
||||||
|
|||||||
@@ -735,7 +735,7 @@ describe('same-session goal driving', () => {
|
|||||||
activation: 'disarmed',
|
activation: 'disarmed',
|
||||||
roundsStarted: 1,
|
roundsStarted: 1,
|
||||||
})
|
})
|
||||||
await test.agent.whenIdle()
|
expect(test.agent.status).toBe('idle')
|
||||||
expect(test.adapter.requests).toHaveLength(1)
|
expect(test.adapter.requests).toHaveLength(1)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -1502,6 +1502,20 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro
|
|||||||
const view = viewFor(ctx, event, callId =>
|
const view = viewFor(ctx, event, callId =>
|
||||||
openCalls.get(session.id)?.get(callId) ?? backscanArgs(session.events, callId))
|
openCalls.get(session.id)?.get(callId) ?? backscanArgs(session.events, callId))
|
||||||
queue.push(frame({ type: 'session/event', sessionId: session.id, event, ...view === undefined ? {} : { view } }))
|
queue.push(frame({ type: 'session/event', sessionId: session.id, event, ...view === undefined ? {} : { view } }))
|
||||||
|
if (event.type === 'agent/inbox/spliced' && event.data.target === 'next-turn') {
|
||||||
|
const agent = ctx.agents.get(session.id)
|
||||||
|
if (agent?.session === session) {
|
||||||
|
queue.push(frame({
|
||||||
|
type: 'session/queue',
|
||||||
|
sessionId: session.id,
|
||||||
|
items: agent.inbox.nextTurn.toSpliced(
|
||||||
|
event.data.start,
|
||||||
|
event.data.removedCount ?? 0,
|
||||||
|
...event.data.inserted,
|
||||||
|
),
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
}
|
||||||
}),
|
}),
|
||||||
ctx.on('session/created', (session: Session) => {
|
ctx.on('session/created', (session: Session) => {
|
||||||
subscribeSession(queue, session)
|
subscribeSession(queue, session)
|
||||||
|
|||||||
@@ -62,9 +62,9 @@ export type MuxFrame =
|
|||||||
| { type: 'question/requested'; sessionId: SessionId; questions: AskUserQuestionItem[] }
|
| { type: 'question/requested'; sessionId: SessionId; questions: AskUserQuestionItem[] }
|
||||||
| { type: 'question/resolved'; sessionId: SessionId; questionRpcId: RpcId; outcome: 'answered' | 'cancelled' }
|
| { type: 'question/resolved'; sessionId: SessionId; questionRpcId: RpcId; outcome: 'answered' | 'cancelled' }
|
||||||
/**
|
/**
|
||||||
* Complete next-turn queue baseline emitted when a mux stream opens. Live
|
* Complete next-turn queue snapshot emitted when a mux stream opens and
|
||||||
* mutations arrive through durable `agent/inbox/spliced` session events.
|
* after every live next-turn mutation. Pending next-step input is outside
|
||||||
* Pending next-step input is outside this Web queue projection.
|
* this Web queue projection.
|
||||||
*/
|
*/
|
||||||
| { type: 'session/queue'; sessionId: SessionId; items: UserMessage[] }
|
| { type: 'session/queue'; sessionId: SessionId; items: UserMessage[] }
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ import ToolRegistry from '@deepseek-ai/dsh-tools'
|
|||||||
import UserInteractionService from '@deepseek-ai/dsh-user-interaction'
|
import UserInteractionService from '@deepseek-ai/dsh-user-interaction'
|
||||||
import CommandService from '@deepseek-ai/dsh-commands'
|
import CommandService from '@deepseek-ai/dsh-commands'
|
||||||
import SkillService from '@deepseek-ai/dsh-skill'
|
import SkillService from '@deepseek-ai/dsh-skill'
|
||||||
import type { HostFrame, MuxFrame } from '../src/api/index.ts'
|
import type { HostFrame } from '../src/api/index.ts'
|
||||||
import type { RpcRequest, RpcResponse } from '../src/api/rpc.ts'
|
import type { RpcRequest, RpcResponse } from '../src/api/rpc.ts'
|
||||||
import { RpcId } from '../src/api/rpc.ts'
|
import { RpcId } from '../src/api/rpc.ts'
|
||||||
import { createApiProxy } from '../src/api-proxy.ts'
|
import { createApiProxy } from '../src/api-proxy.ts'
|
||||||
@@ -85,6 +85,13 @@ async function collect<F>(iterable: AsyncIterable<RpcRequest<F>>, count: number,
|
|||||||
return frames
|
return frames
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Read the next payload from an open stream. */
|
||||||
|
async function nextFrame<F>(iterator: AsyncIterator<RpcRequest<F>>): Promise<F> {
|
||||||
|
const result = await iterator.next()
|
||||||
|
if (result.done) throw new Error('stream ended')
|
||||||
|
return result.value.payload
|
||||||
|
}
|
||||||
|
|
||||||
describe('command.list', () => {
|
describe('command.list', () => {
|
||||||
it('serves the addressed agent\'s name-sorted catalog', async () => {
|
it('serves the addressed agent\'s name-sorted catalog', async () => {
|
||||||
const ctx = await harness()
|
const ctx = await harness()
|
||||||
@@ -332,24 +339,41 @@ describe('session.updateQueue', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe('session/queue frames', () => {
|
describe('session/queue frames', () => {
|
||||||
it('publishes the durable next-turn baseline without duplicating message identity', async () => {
|
it('publishes authoritative next-turn snapshots without duplicating message identity', async () => {
|
||||||
const ctx = await harness()
|
const ctx = await harness()
|
||||||
const api = createApiProxy(ctx, DEFAULTS)
|
const api = createApiProxy(ctx, DEFAULTS)
|
||||||
const agent = stubAgent(ctx)
|
const agent = stubAgent(ctx)
|
||||||
const queued = inboxMessage('m-1', 'queued prompt')
|
const queued = inboxMessage('m-1', 'queued prompt')
|
||||||
|
const edited = inboxMessage('m-1', 'edited prompt')
|
||||||
const steering = inboxMessage('m-2', 'steering prompt')
|
const steering = inboxMessage('m-2', 'steering prompt')
|
||||||
agent.inbox.splice('next-turn', 0, 0, [queued])
|
agent.inbox.splice('next-turn', 0, 0, [queued])
|
||||||
agent.inbox.splice('next-step', 0, 0, [steering])
|
agent.inbox.splice('next-step', 0, 0, [steering])
|
||||||
|
|
||||||
const abort = new AbortController()
|
const abort = new AbortController()
|
||||||
const frames = await collect<MuxFrame>(
|
const iterator = api.events.mux({
|
||||||
api.events.mux({ rpcId: RpcId('t-mux-baseline'), payload: {} }, abort.signal), 2, abort)
|
rpcId: RpcId('t-mux-baseline'),
|
||||||
|
payload: {},
|
||||||
|
}, abort.signal)[Symbol.asyncIterator]()
|
||||||
|
const frames = [
|
||||||
|
await nextFrame(iterator),
|
||||||
|
await nextFrame(iterator),
|
||||||
|
]
|
||||||
|
agent.inbox.splice('next-turn', 0, 1, [edited])
|
||||||
|
frames.push(await nextFrame(iterator), await nextFrame(iterator))
|
||||||
|
abort.abort()
|
||||||
|
await iterator.return?.()
|
||||||
|
|
||||||
expect(frames.filter(frame => frame.type === 'session/queue')).toEqual([
|
expect(frames.filter(frame => frame.type === 'session/queue')).toEqual([
|
||||||
{
|
{
|
||||||
type: 'session/queue',
|
type: 'session/queue',
|
||||||
sessionId: agent.id,
|
sessionId: agent.id,
|
||||||
items: [queued],
|
items: [queued],
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
type: 'session/queue',
|
||||||
|
sessionId: agent.id,
|
||||||
|
items: [edited],
|
||||||
|
},
|
||||||
])
|
])
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -10,7 +10,7 @@
|
|||||||
import { describe, expect, it } from 'vitest'
|
import { describe, expect, it } from 'vitest'
|
||||||
import { Context } from 'cordis'
|
import { Context } from 'cordis'
|
||||||
import { z } from 'zod'
|
import { z } from 'zod'
|
||||||
import AgentRegistry from '@deepseek-ai/dsh-agent'
|
import AgentRegistry, { Inbox } from '@deepseek-ai/dsh-agent'
|
||||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||||
import { createUserMessage } from '@deepseek-ai/dsh-llm'
|
import { createUserMessage } from '@deepseek-ai/dsh-llm'
|
||||||
import SessionStore, { SessionId } from '@deepseek-ai/dsh-session'
|
import SessionStore, { SessionId } from '@deepseek-ai/dsh-session'
|
||||||
@@ -53,9 +53,8 @@ async function harness(withRegistry: boolean): Promise<{ ctx: Context; session:
|
|||||||
await ctx.plugin(AgentRegistry)
|
await ctx.plugin(AgentRegistry)
|
||||||
if (withRegistry) await ctx.plugin(SessionProjectionRegistry)
|
if (withRegistry) await ctx.plugin(SessionProjectionRegistry)
|
||||||
const session = ctx.sessions.create()
|
const session = ctx.sessions.create()
|
||||||
// history resolves the agent first; a live structural stub is enough (only
|
// The gateway reads both the session and durable inbox baseline.
|
||||||
// .session is read on this path).
|
ctx.agents.register({ id: session.id, session, inbox: new Inbox(session), status: 'idle', ctx } as Agent)
|
||||||
ctx.agents.register({ id: session.id, session, status: 'idle', ctx } as Agent)
|
|
||||||
return { ctx, session }
|
return { ctx, session }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user