Merge remote-tracking branch 'origin/master' into codex/rfc-subagent-background-tasks
# Conflicts: # docs/architecture.md # docs/config-catalog.md # docs/cordis-catalog/events.md # docs/cordis-catalog/services.md # docs/event-producer-consumer.md # docs/module-graph.md # docs/tool-catalog.md # packages/bash/tool-bash/tests/integration.spec.ts # packages/bash/tool-bash/tests/tools.spec.ts # packages/core/agent-core/tests/agent-core.spec.ts # packages/core/agent-loop/README.md # packages/core/agent-loop/src/index.ts # packages/core/agent/README.md # packages/core/agent/src/index.ts # packages/core/agent/tests/agent.spec.ts # packages/subagent/subagent/README.md # packages/subagent/subagent/src/index.ts # packages/subagent/tool-subagent/README.md # packages/subagent/tool-subagent/src/index.ts # pnpm-lock.yaml # scripts/doc-budgets.manifest.json
This commit is contained in:
@@ -17,8 +17,8 @@ Every read/kill/wait/get compares the task's owner session (`owner.session.heade
|
||||
## Lifecycle
|
||||
|
||||
- Registrations are NOT effect-scoped to the registering fiber: tasks belong to their owning agent + producing backend, so producer/surface HMR reloads never touch them.
|
||||
- An owned task must name the exact live `Agent` instance currently registered under its id (stale objects are rejected after id reuse), then attaches once per owner an awaited cleanup via `ctx.agents.onCleanup`: on owner disposal the registry cancels live tasks, awaits contract-compliant producers to quiescence, and drops their snapshots. If a teardown cancel throws, it force-fails the record and logs that the underlying work may be orphaned rather than deadlocking `AgentHandle.dispose()`.
|
||||
- Service disposal closes the listener registry first (late teardown settlements stay silent), then applies the same cancellation rule to every live task and awaits terminal records.
|
||||
- An owned task must name the exact live `Agent` instance currently registered under its id (stale objects are rejected after id reuse), then attaches one awaited cleanup through `owner.ctx`: agent-scope disposal cancels live tasks, awaits contract-compliant producers to quiescence, and drops their snapshots. If a teardown cancel throws, it force-fails the record and logs that the underlying work may be orphaned rather than deadlocking `AgentHandle.dispose()`.
|
||||
- Service disposal closes the listener registry first, applies the same cancellation rule to every live task, awaits terminal records, then detaches its effects from still-live agent scopes so a reloaded tasks service is not retained until those agents exit.
|
||||
- A producer whose `cancel` returns but never causes `done` to settle remains indistinguishable from a slow stop and can stall teardown; solving that residual requires an explicit bounded-lifetime or forced-disposal design.
|
||||
|
||||
## Non-goals (v1)
|
||||
|
||||
@@ -31,7 +31,7 @@
|
||||
*/
|
||||
|
||||
import { Context, Service } from 'cordis'
|
||||
import type { Agent, AgentId } from '@deepseek-ai/dsh-agent'
|
||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
import type { SessionId } from '@deepseek-ai/dsh-session'
|
||||
import { deadline, timeoutOf } from '@deepseek-ai/dsh-timeout'
|
||||
import { TaskId } from './types.ts'
|
||||
@@ -100,14 +100,14 @@ export class TaskService extends Service {
|
||||
private surfaces = new Set<symbol>()
|
||||
private listeners = new Set<TaskDoneListener>()
|
||||
private listenersClosed = false
|
||||
/** Owner agents whose cleanup effect is attached, mapped to its self-detacher. */
|
||||
private ownerCleanups = new Map<AgentId, () => void>()
|
||||
/** Owner agents whose scope cleanup is attached, mapped to its exact disposer. */
|
||||
private ownerCleanups = new Map<Agent, () => Promise<void> | void>()
|
||||
/**
|
||||
* The service's OWN construction-time context, for work that outlives the
|
||||
* calling fiber: detached settlement continuations (logging), and the
|
||||
* owner-cleanup registration on `ctx.agents` (which must survive a producer
|
||||
* plugin's HMR reload, unlike the caller-fiber-scoped effects in
|
||||
* {@link onTaskDone}/{@link attachSurface}).
|
||||
* service teardown. Owner cleanup itself is registered through the owning
|
||||
* agent's scope so it survives producer-plugin reloads and participates in
|
||||
* the agent's structural quiescence boundary.
|
||||
*/
|
||||
private readonly selfCtx: Context
|
||||
|
||||
@@ -123,8 +123,7 @@ export class TaskService extends Service {
|
||||
* control-surface fence ({@link attachSurface}; a task the model could
|
||||
* never read or stop must fail loud before it exists), kind/label
|
||||
* validation, exact live owner-instance identity, and the owner's awaited
|
||||
* disposal-cleanup attach (once per owner agent, through
|
||||
* `ctx.agents.onCleanup`) — runs BEFORE
|
||||
* disposal-cleanup attach (once per owner agent, through `owner.ctx`) — runs BEFORE
|
||||
* `spec.run()` starts the actual work, and nothing in the runtime can fail
|
||||
* after it returns: "work started but never got a collectable id" is
|
||||
* structurally impossible, not a producer rollback obligation. The runtime
|
||||
@@ -442,16 +441,13 @@ export class TaskService extends Service {
|
||||
}
|
||||
|
||||
/**
|
||||
* Attach the awaited owner-disposal cleanup for an owner agent, once: when
|
||||
* the agent's disposal chain drains (`ctx.agents.drainCleanups`), the
|
||||
* owner's still-live tasks are cancelled, awaited to settlement, and their
|
||||
* snapshots dropped. Registered through {@link selfCtx} so the cleanup
|
||||
* survives producer-plugin reloads. When the cleanup starts, it detaches its
|
||||
* own effect before awaiting task settlement, so completed owners do not
|
||||
* accumulate effect wrappers (and captured sessions) on the long-lived tasks
|
||||
* fiber. A narrow race remains if new work starts on an agent already being
|
||||
* drained: before this callback clears the owner entry, that start can reuse
|
||||
* the in-flight cleanup after its task snapshot was taken.
|
||||
* Attach the awaited owner-disposal cleanup for an owner agent, once. The
|
||||
* effect is registered through `owner.ctx`, so it belongs to the agent scope
|
||||
* rather than the producer or long-lived tasks fiber: it survives producer
|
||||
* reloads, runs at the structural agent quiescence boundary, and removes its
|
||||
* wrapper automatically when that scope unwinds. The tasks service retains
|
||||
* the exact disposer only so service teardown can detach cross-fiber effects
|
||||
* instead of leaving a dead service captured by still-live agents.
|
||||
* Fails loud when no agent registry is mounted or when `owner` is not the
|
||||
* exact live instance currently registered under its id — accepting a stale
|
||||
* object after id reuse would attach its session's task to another agent's
|
||||
@@ -466,20 +462,15 @@ export class TaskService extends Service {
|
||||
if (agents.get(ownerId) !== owner) {
|
||||
throw new Error(`agent "${ownerId}" is not the registered agent instance (background task owner must be live)`)
|
||||
}
|
||||
if (this.ownerCleanups.has(ownerId)) return
|
||||
if (this.ownerCleanups.has(owner)) return
|
||||
const ownerSession = owner.session.header.id
|
||||
// Attach FIRST, record after: onCleanup throws for an unregistered agent,
|
||||
// and marking the owner as covered before that would make every later
|
||||
// registration for the same owner silently skip the cleanup.
|
||||
const detach = agents.onCleanup(ownerId, async () => {
|
||||
const disposeEffect = this.ownerCleanups.get(ownerId)
|
||||
this.ownerCleanups.delete(ownerId)
|
||||
// A drain racing lifecycle teardown may find that the effect was already
|
||||
// detached; otherwise this removes its wrapper from the tasks fiber now.
|
||||
disposeEffect?.()
|
||||
// Attach FIRST, record after: an already-disposing scope rejects effects,
|
||||
// and marking the owner as covered before that would poison later starts.
|
||||
const detach = owner.ctx.effect(() => async () => {
|
||||
this.ownerCleanups.delete(owner)
|
||||
await this.disposeOwned(ownerSession)
|
||||
})
|
||||
this.ownerCleanups.set(ownerId, detach)
|
||||
}, 'tasks.ownerCleanup()')
|
||||
this.ownerCleanups.set(owner, detach)
|
||||
}
|
||||
|
||||
/** Cancel, await terminal records, and drop every task owned by one session. */
|
||||
@@ -504,6 +495,12 @@ export class TaskService extends Service {
|
||||
this.cancelForTeardown(all, 'tasks service disposed')
|
||||
await Promise.all(all.map(task => task.settled))
|
||||
this.store.clear()
|
||||
// These effects belong to agent scopes, not this service's fiber. Detach
|
||||
// them after the shared store is quiescent so a tasks-service reload cannot
|
||||
// leave old callbacks retaining the dead service until each agent exits.
|
||||
const ownerCleanups = [...this.ownerCleanups.values()]
|
||||
this.ownerCleanups.clear()
|
||||
await Promise.all(ownerCleanups.map(cleanup => Promise.resolve(cleanup())))
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -71,8 +71,8 @@ export interface TaskStart {
|
||||
label: string
|
||||
/**
|
||||
* The spawning agent. Its `session.header.id` becomes the task's owner
|
||||
* token (read/kill/wait/list are fenced to that session), and its disposal
|
||||
* cancels and awaits the task through the `ctx.agents.onCleanup` seam. It
|
||||
* token (read/kill/wait/list are fenced to that session), and its `ctx` scope
|
||||
* owns an async cleanup that cancels and awaits the task during disposal. It
|
||||
* must be the exact live instance currently registered under its agent id;
|
||||
* a stale object whose id has been reused is rejected before work starts.
|
||||
* `undefined` starts an UNOWNED task: open to any caller, alive until the
|
||||
|
||||
@@ -6,19 +6,31 @@ import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
import TaskService, { TaskId } from '@deepseek-ai/dsh-tasks'
|
||||
import type { TaskHooks, TaskOutcome, TaskSnapshot, TaskStart } from '@deepseek-ai/dsh-tasks'
|
||||
|
||||
function stubAgent(rawId: string, rawSessionId = `${rawId}-session`): Agent {
|
||||
const agentScopeDisposers = new WeakMap<Agent, () => Promise<void>>()
|
||||
|
||||
function stubAgent(ctx: Context, rawId: string, rawSessionId = `${rawId}-session`): Agent {
|
||||
const id = AgentId(rawId)
|
||||
return {
|
||||
const scopeFiber = ctx.plugin(() => {})
|
||||
const agent = {
|
||||
id,
|
||||
options: {},
|
||||
session: new Session(SessionId(rawSessionId)),
|
||||
status: 'idle',
|
||||
status: 'idle' as const,
|
||||
ctx: scopeFiber.ctx,
|
||||
send() {},
|
||||
steer() {},
|
||||
inject() {},
|
||||
cancel() {},
|
||||
whenIdle() { return Promise.resolve() },
|
||||
}
|
||||
agentScopeDisposers.set(agent, async () => { await scopeFiber.dispose() })
|
||||
return agent
|
||||
}
|
||||
|
||||
async function disposeAgentScope(agent: Agent): Promise<void> {
|
||||
const dispose = agentScopeDisposers.get(agent)
|
||||
if (dispose === undefined) throw new Error(`missing test scope for agent "${agent.id}"`)
|
||||
await dispose()
|
||||
}
|
||||
|
||||
/** A controllable producer start-spec: settle its `done` on demand, record cancels. */
|
||||
@@ -320,9 +332,9 @@ describe('TaskService.wait', () => {
|
||||
describe('TaskService owner isolation', () => {
|
||||
it('fences read/kill/wait to the owning session and keeps unowned tasks open', async () => {
|
||||
const ctx = await harness()
|
||||
const owner = stubAgent('owner')
|
||||
const owner = stubAgent(ctx, 'owner')
|
||||
ctx.agents.register(owner)
|
||||
const other = stubAgent('other')
|
||||
const other = stubAgent(ctx, 'other')
|
||||
|
||||
const owned = ctx.tasks.start(producer({ owner }).spec)
|
||||
const open = ctx.tasks.start(producer().spec)
|
||||
@@ -340,8 +352,8 @@ describe('TaskService owner isolation', () => {
|
||||
|
||||
it('list() shows only caller-owned plus unowned tasks', async () => {
|
||||
const ctx = await harness()
|
||||
const alice = stubAgent('alice')
|
||||
const bob = stubAgent('bob')
|
||||
const alice = stubAgent(ctx, 'alice')
|
||||
const bob = stubAgent(ctx, 'bob')
|
||||
ctx.agents.register(alice)
|
||||
ctx.agents.register(bob)
|
||||
|
||||
@@ -358,7 +370,7 @@ describe('TaskService owner isolation', () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(TaskService)
|
||||
ctx.tasks.attachSurface('test-surface')
|
||||
expect(() => ctx.tasks.start(producer({ owner: stubAgent('a') }).spec))
|
||||
expect(() => ctx.tasks.start(producer({ owner: stubAgent(ctx, 'a') }).spec))
|
||||
.toThrow('background task ownership requires the agent registry')
|
||||
// The failed registration mutated nothing: no stored task, counter untouched.
|
||||
expect(ctx.tasks.list()).toEqual([])
|
||||
@@ -367,7 +379,7 @@ describe('TaskService owner isolation', () => {
|
||||
|
||||
it('a failed owner-cleanup attach leaves the registry unchanged and does not poison the owner', async () => {
|
||||
const ctx = await harness()
|
||||
const ghost = stubAgent('ghost') // never registered in ctx.agents
|
||||
const ghost = stubAgent(ctx, 'ghost') // never registered in ctx.agents
|
||||
|
||||
// Exact-instance preflight rejects the unregistered agent BEFORE any
|
||||
// registry mutation or owner-cleanup attachment.
|
||||
@@ -390,18 +402,18 @@ describe('TaskService owner isolation', () => {
|
||||
}),
|
||||
})
|
||||
expect(id).toBe('bash-1') // the failed attempt burned no counter
|
||||
await ctx.agents.drainCleanups(ghost.id)
|
||||
await disposeAgentScope(ghost)
|
||||
expect(cancels).toEqual(['owner disposed'])
|
||||
expect(ctx.tasks.list(ghost)).toEqual([])
|
||||
})
|
||||
|
||||
it('rejects a stale owner instance after another agent reuses its id', async () => {
|
||||
const ctx = await harness()
|
||||
const staleOwner = stubAgent('owner', 'stale-session')
|
||||
const staleOwner = stubAgent(ctx, 'owner', 'stale-session')
|
||||
const unregisterStale = ctx.agents.register(staleOwner)
|
||||
unregisterStale()
|
||||
|
||||
const currentOwner = stubAgent('owner', 'current-session')
|
||||
const currentOwner = stubAgent(ctx, 'owner', 'current-session')
|
||||
ctx.agents.register(currentOwner)
|
||||
const current = producer({ owner: currentOwner })
|
||||
ctx.tasks.start(current.spec) // Attach the current owner's cleanup first.
|
||||
@@ -416,14 +428,14 @@ describe('TaskService owner isolation', () => {
|
||||
|
||||
current.settle({ status: 'completed' })
|
||||
await tick()
|
||||
await ctx.agents.drainCleanups(currentOwner.id)
|
||||
await disposeAgentScope(currentOwner)
|
||||
})
|
||||
})
|
||||
|
||||
describe('TaskService owner cleanup', () => {
|
||||
it('drains the owner: cancels live tasks, awaits settlement, drops snapshots', async () => {
|
||||
const ctx = await harness()
|
||||
const owner = stubAgent('owner')
|
||||
const owner = stubAgent(ctx, 'owner')
|
||||
ctx.agents.register(owner)
|
||||
|
||||
// The producer settles only when cancelled — models a child that stops on request.
|
||||
@@ -443,15 +455,15 @@ describe('TaskService owner cleanup', () => {
|
||||
terminal.settle({ status: 'completed' })
|
||||
await tick()
|
||||
|
||||
await ctx.agents.drainCleanups(owner.id)
|
||||
await disposeAgentScope(owner)
|
||||
expect(cancels).toEqual(['owner disposed'])
|
||||
// Snapshots dropped: nothing of the owner's remains, listing is empty.
|
||||
expect(ctx.tasks.list(owner)).toEqual([])
|
||||
})
|
||||
|
||||
it('attaches one cleanup per owner and re-attaches after a drain', async () => {
|
||||
it('attaches one cleanup per owner and drains all owned tasks with the scope', async () => {
|
||||
const ctx = await harness()
|
||||
const owner = stubAgent('owner')
|
||||
const owner = stubAgent(ctx, 'owner')
|
||||
ctx.agents.register(owner)
|
||||
|
||||
const first = producer({ owner })
|
||||
@@ -461,34 +473,28 @@ describe('TaskService owner cleanup', () => {
|
||||
first.settle({ status: 'completed' })
|
||||
second.settle({ status: 'completed' })
|
||||
await tick()
|
||||
await ctx.agents.drainCleanups(owner.id)
|
||||
|
||||
// A fresh task after the drain gets a fresh cleanup (the set was consumed).
|
||||
const third = producer({ owner })
|
||||
ctx.tasks.start(third.spec)
|
||||
third.settle({ status: 'completed' })
|
||||
await tick()
|
||||
expect(ctx.tasks.list(owner)).toHaveLength(1)
|
||||
await ctx.agents.drainCleanups(owner.id)
|
||||
expect(owner.ctx.fiber.getEffects().filter(effect => effect.label === 'tasks.ownerCleanup()')).toHaveLength(1)
|
||||
await disposeAgentScope(owner)
|
||||
expect(ctx.tasks.list(owner)).toEqual([])
|
||||
})
|
||||
|
||||
it('releases the owner-cleanup effect from the tasks fiber after its drain', async () => {
|
||||
it('registers owner cleanup on the agent scope rather than the tasks fiber', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(AgentRegistry)
|
||||
const tasksFiber = await ctx.plugin(TaskService)
|
||||
ctx.tasks.attachSurface('test-surface')
|
||||
const owner = stubAgent('owner')
|
||||
const owner = stubAgent(ctx, 'owner')
|
||||
ctx.agents.register(owner)
|
||||
const ownerCleanupEffects = () => tasksFiber.getEffects()
|
||||
.filter(effect => effect.label === 'agents.onCleanup()')
|
||||
const ownerCleanupEffects = () => owner.ctx.fiber.getEffects()
|
||||
.filter(effect => effect.label === 'tasks.ownerCleanup()')
|
||||
|
||||
const first = producer({ owner })
|
||||
ctx.tasks.start(first.spec)
|
||||
expect(ownerCleanupEffects()).toHaveLength(1)
|
||||
first.settle({ status: 'completed' })
|
||||
await tick()
|
||||
await ctx.agents.drainCleanups(owner.id)
|
||||
expect(tasksFiber.getEffects().some(effect => effect.label === 'tasks.ownerCleanup()')).toBe(false)
|
||||
await disposeAgentScope(owner)
|
||||
|
||||
// Only the owner registration is released; the long-lived tasks service
|
||||
// and its own teardown effect remain active.
|
||||
@@ -496,20 +502,12 @@ describe('TaskService owner cleanup', () => {
|
||||
expect(ctx.get('tasks')).toBeDefined()
|
||||
expect(tasksFiber.getEffects().some(effect => effect.label === 'tasks teardown')).toBe(true)
|
||||
|
||||
// The same still-live owner can attach and release a fresh registration.
|
||||
const second = producer({ owner })
|
||||
ctx.tasks.start(second.spec)
|
||||
expect(ownerCleanupEffects()).toHaveLength(1)
|
||||
second.settle({ status: 'completed' })
|
||||
await tick()
|
||||
await ctx.agents.drainCleanups(owner.id)
|
||||
expect(ownerCleanupEffects()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('force-fails a throwing teardown cancel without awaiting producer done, first outcome wins', async () => {
|
||||
const ctx = await harness()
|
||||
const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => {})
|
||||
const owner = stubAgent('owner')
|
||||
const owner = stubAgent(ctx, 'owner')
|
||||
ctx.agents.register(owner)
|
||||
const seen: TaskSnapshot[] = []
|
||||
ctx.tasks.onTaskDone(snapshot => void seen.push(snapshot))
|
||||
@@ -525,7 +523,7 @@ describe('TaskService owner cleanup', () => {
|
||||
}),
|
||||
})
|
||||
|
||||
const drain = ctx.agents.drainCleanups(owner.id)
|
||||
const drain = disposeAgentScope(owner)
|
||||
let drained = false
|
||||
void drain.then(() => { drained = true })
|
||||
await tick()
|
||||
@@ -618,6 +616,32 @@ describe('TaskService disposal', () => {
|
||||
expect(seen).toEqual([])
|
||||
})
|
||||
|
||||
it('detaches owner effects from still-live agent scopes when the service unloads', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(AgentRegistry)
|
||||
const tasksFiber = await ctx.plugin(TaskService)
|
||||
ctx.tasks.attachSurface('test-surface')
|
||||
const owner = stubAgent(ctx, 'owner')
|
||||
ctx.agents.register(owner)
|
||||
let settle!: (outcome: TaskOutcome) => void
|
||||
ctx.tasks.start({
|
||||
kind: 'bash',
|
||||
label: 'owned work',
|
||||
owner,
|
||||
run: () => ({
|
||||
cancel() { settle({ status: 'killed' }) },
|
||||
done: new Promise<TaskOutcome>((resolve) => { settle = resolve }),
|
||||
}),
|
||||
})
|
||||
const ownerEffects = () => owner.ctx.fiber.getEffects()
|
||||
.filter(effect => effect.label === 'tasks.ownerCleanup()')
|
||||
expect(ownerEffects()).toHaveLength(1)
|
||||
|
||||
await tasksFiber.dispose()
|
||||
|
||||
expect(ownerEffects()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('detaching the last surface re-arms the register fence', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(TaskService)
|
||||
|
||||
@@ -27,7 +27,13 @@ async function setup(config: ToolTasks.Config = {}) {
|
||||
* wrong-field match fails the test).
|
||||
*/
|
||||
function fakeAgent(ctx: Context, sessionId: string, inject: (...args: unknown[]) => void = () => {}): Agent {
|
||||
const agent = { id: `agent-${sessionId}`, inject, session: { header: { version: 0, id: sessionId, createdAt: 0 } } } as unknown as Agent
|
||||
const scopeFiber = ctx.plugin(() => {})
|
||||
const agent = {
|
||||
id: `agent-${sessionId}`,
|
||||
ctx: scopeFiber.ctx,
|
||||
inject,
|
||||
session: { header: { version: 0, id: sessionId, createdAt: 0 } },
|
||||
} as unknown as Agent
|
||||
ctx.agents.register(agent)
|
||||
return agent
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user