From 42ee4e22debcbfcc1234c89b5c710ecece8d9ae5 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:27:55 +0800 Subject: [PATCH] fix(subagent): validate setup transactions before agent publication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `materialize` ran `setupTransaction.assertIntact()` only after `ctx.agents.create()/resume()` resolved — but the factory publishes `session/created` (and the persistence backend writes the descriptor seed) inside that call, and `rollbackUnpublished()` only disposes the live handle; the persistence seam has no delete. A setup contribution revoked during construction therefore left a durable ghost: `startContinuable()` rejected with `ACTIVATION_SETUP_REVOKED` and returned no child id, yet `list_agents` surfaced a persisted `continuable` child whose log carries a valid descriptor — so a later `send_message` could cold-resume a child the deployment had explicitly refused to establish. Move the validation into the creation callback, before the factory can publish: `assertIntact()` then rejects the create/resume call itself, so no session is ever persisted for a rejected child. Commit the batch in the same callback so a later contribution removal releases the installation instead of invalidating a child already being established (live revocation, matching the resident semantics). Pins the rollback regression test to assert that no `session/created` is ever announced for the rejected child (the parent is created before the listener registers), in addition to the existing registry assertion. --- packages/subagent/subagent/src/continuation.ts | 16 +++++++++++++--- .../tests/tool-subagent-report.spec.ts | 11 +++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/subagent/subagent/src/continuation.ts b/packages/subagent/subagent/src/continuation.ts index d21f235589..bc4833bbd0 100644 --- a/packages/subagent/subagent/src/continuation.ts +++ b/packages/subagent/subagent/src/continuation.ts @@ -804,6 +804,16 @@ export class SubagentContinuationManager { const setup = (childCtx: Context): void => { applyChildComposition(childCtx, inputs.composition) setupTransaction = this.setupRegistry.apply(childCtx) + // Validate and freeze the batch inside the creation callback, before the + // factory can publish the session: a revoked contribution must reject + // the create/resume call pre-publication, so no persisted session is + // ever left behind for a child the manager rejects — rollback only + // disposes the live handle, and the persistence seam has no delete, so + // a post-publication rejection would leave a resumable ghost child. + // Committing here also means a later contribution removal releases the + // installation instead of invalidating a child already being established. + setupTransaction.assertIntact() + setupTransaction.commit() } const observer = this.host.observeActivation(provider, childId, parent) const { create } = inputs @@ -842,7 +852,6 @@ export class SubagentContinuationManager { try { inputs.signal.throwIfAborted() this.assertAdmitting(parent) - setupTransaction.assertIntact() this.acquireOwnership(parent, childId) // Every accepted id leaves the inbox exactly once, through dequeue or // discard. Clearing it there is what lets `stateOf()` distinguish a truly @@ -860,8 +869,9 @@ export class SubagentContinuationManager { for (const item of items) activation.accepted.delete(item.message.id) this.wake(activation) }) - // Resident setup revokes live from here instead of invalidating creation. - setupTransaction.commit() + // Setup already validated and committed inside the creation callback; + // revocations from here on are immediate live revocation, never + // creation invalidation. // Publish the start edge before any turn can run, so observers see this // epoch before its first request. observer.start(handle.agent) diff --git a/packages/subagent/tool-subagent-report/tests/tool-subagent-report.spec.ts b/packages/subagent/tool-subagent-report/tests/tool-subagent-report.spec.ts index 31837afa9d..20e751530f 100644 --- a/packages/subagent/tool-subagent-report/tests/tool-subagent-report.spec.ts +++ b/packages/subagent/tool-subagent-report/tests/tool-subagent-report.spec.ts @@ -327,6 +327,15 @@ describe('dsh-tool-subagent-report', () => { return dispose }) + // No session may be announced for the rejected child: the setup + // validation must reject inside the creation callback, before the factory + // publishes — a post-publication rejection would persist a resumable + // ghost that `list_agents` surfaces and `send_message` can resurrect. + // The parent was created inside setup(), so any later announcement is the + // rejected child's. + const announced: SessionId[] = [] + const listener = (session: { id: SessionId }): void => { announced.push(session.id) } + const removeListener = ctx.on('session/created', listener) await expect(ctx.subagents.startContinuable({ provider: 'spawn', label: 'racing child', @@ -336,6 +345,8 @@ describe('dsh-tool-subagent-report', () => { }, signal: testSignal, })).rejects.toMatchObject({ code: 'ACTIVATION_SETUP_REVOKED' }) + removeListener() + expect(announced).toEqual([]) expect(ctx.agents.list().map(agent => agent.id)).toEqual([parent.id]) })