fix: verify JSON-RPC child ownership
This commit is contained in:
@@ -4,7 +4,7 @@ The **SDK server plugin** (`jsonrpc`): mounting it serves a stdio JSON-RPC serve
|
||||
|
||||
## Wiring
|
||||
|
||||
`inject: ['agents']` — the server creates one agent per SDK `sessionId` (get-or-create on `session/prompt`). A local subagent's shared agent/session id supplies `subagent.finished.childSessionId` directly; the server counts local starts by provider/id and the exact delegating-parent carrier, because a continuation may reuse one child and the child may be disposed before a later `subagent/end`. The paired event carrier preserves parent correlation even when reused ids settle out of order. Runs from remote providers are not reported because they create no local agent. The LLM seam is read opportunistically via `ctx.get('llm')` (not injected): when `initialize.model` has no registered adapter, the plugin mounts `dsh-llm-deepseek` for it (credentials from `$DEEPSEEK_API_KEY` / `$DEEPSEEK_BASE_URL`) — a config-registered adapter for the model wins. Everything else — persistence, the tool stacks, the adapter set — comes from the surrounding `cordis.yml`.
|
||||
`inject: ['agents']` — the server creates one agent per SDK `sessionId` (get-or-create on `session/prompt`). A local subagent's shared agent/session id supplies `subagent.finished.childSessionId` directly; the server verifies that the live child is owned by the exact delegating parent, then counts local starts by provider/id and that parent carrier because a continuation may reuse one child and the child may be disposed before a later `subagent/end`. The paired event carrier preserves parent correlation even when reused ids settle out of order. Runs from remote providers are not reported even when their parent-scoped run id collides with an unrelated local agent. The LLM seam is read opportunistically via `ctx.get('llm')` (not injected): when `initialize.model` has no registered adapter, the plugin mounts `dsh-llm-deepseek` for it (credentials from `$DEEPSEEK_API_KEY` / `$DEEPSEEK_BASE_URL`) — a config-registered adapter for the model wins. Everything else — persistence, the tool stacks, the adapter set — comes from the surrounding `cordis.yml`.
|
||||
|
||||
## Config
|
||||
|
||||
|
||||
@@ -109,8 +109,8 @@ export class HarnessSdkServer {
|
||||
// child disposal and reused ids need no settlement-order assumption.
|
||||
const localRuns = this.localRuns
|
||||
this.disposers.push(ctx.on('subagent/start', function (this: Scoped<SubagentService>, info: SubagentRunInfo) {
|
||||
if (ctx.agents.get(info.id) === undefined) return
|
||||
const parent = subagentParentOf(this)
|
||||
if (!ctx.agents.isOwnedBy(info.id, parent)) return
|
||||
const providerRuns = localRuns.get(info.provider) ?? new Map<SessionId, Map<Agent, number>>()
|
||||
const parentRuns = providerRuns.get(info.id) ?? new Map<Agent, number>()
|
||||
parentRuns.set(parent, (parentRuns.get(parent) ?? 0) + 1)
|
||||
@@ -118,7 +118,6 @@ export class HarnessSdkServer {
|
||||
localRuns.set(info.provider, providerRuns)
|
||||
}))
|
||||
this.disposers.push(ctx.on('subagent/end', function (this: Scoped<SubagentService>, info: SubagentRunEndInfo) {
|
||||
const agent = ctx.agents.get(info.id)
|
||||
const parent = subagentParentOf(this)
|
||||
const providerRuns = localRuns.get(info.provider)
|
||||
const parentRuns = providerRuns?.get(info.id)
|
||||
@@ -130,10 +129,10 @@ export class HarnessSdkServer {
|
||||
if (providerRuns?.size === 0) localRuns.delete(info.provider)
|
||||
}
|
||||
// This protocol reports LOCAL child sessions. A lineage-bearing child
|
||||
// has the session/created-driven start notification above; a parentless
|
||||
// local provider still gets its terminal notification. A remote provider
|
||||
// has neither a pending local start nor a live local agent and is ignored.
|
||||
if (pendingCount === undefined && agent === undefined) return
|
||||
// has the session/created-driven start notification above. A remote run
|
||||
// has neither a cached owned start nor a live child owned by this exact
|
||||
// parent; an unrelated local agent with the same id never makes it local.
|
||||
if (pendingCount === undefined && !ctx.agents.isOwnedBy(info.id, parent)) return
|
||||
transport.notify('subagent.finished', {
|
||||
provider: info.provider,
|
||||
agentId: String(info.id),
|
||||
|
||||
@@ -277,12 +277,12 @@ describe('HarnessSdkServer', () => {
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
const handle = await ctx.agents.create({
|
||||
const handle = await parentHandle.agent.ctx.agents.create({
|
||||
sessionId: SessionId('child-session'),
|
||||
meta: { cwd: storageDir, parentSession: SessionId('main') },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
const parentlessHandle = await ctx.agents.create({
|
||||
const parentlessHandle = await parentHandle.agent.ctx.agents.create({
|
||||
sessionId: SessionId('parentless-child-session'),
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
@@ -331,6 +331,43 @@ describe('HarnessSdkServer', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('ignores a remote run id that collides with an unrelated local agent', async () => {
|
||||
const storageDir = await mkdtemp(join(tmpdir(), 'dsh-jsonrpc-subagent-remote-collision-'))
|
||||
const ctx = await makeHarness(storageDir)
|
||||
try {
|
||||
const transport = new FakeTransport()
|
||||
const server = new HarnessSdkServer(ctx, transport)
|
||||
const parentHandle = await ctx.agents.create({
|
||||
sessionId: SessionId('collision-parent'),
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
const unrelatedHandle = await ctx.agents.create({
|
||||
sessionId: SessionId('remote-run-id'),
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
|
||||
await settleSubagent(ctx, parentHandle.agent, {
|
||||
provider: 'remote',
|
||||
id: SessionId('remote-run-id'),
|
||||
stopReason: 'completed',
|
||||
lastAssistantMessage: [],
|
||||
}, () => unrelatedHandle.dispose())
|
||||
|
||||
expect(transport.notifications.some(notification =>
|
||||
notification.method === 'subagent.finished'
|
||||
&& notification.params?.agentId === 'remote-run-id',
|
||||
)).toBe(false)
|
||||
|
||||
await parentHandle.dispose()
|
||||
await server.shutdown()
|
||||
} finally {
|
||||
await ctx.fiber.dispose()
|
||||
await rm(storageDir, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('retains locality across continuation runs on one live child', async () => {
|
||||
const storageDir = await mkdtemp(join(tmpdir(), 'dsh-jsonrpc-subagent-continuation-'))
|
||||
const ctx = await makeHarness(storageDir)
|
||||
@@ -342,7 +379,7 @@ describe('HarnessSdkServer', () => {
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
const childHandle = await ctx.agents.create({
|
||||
const childHandle = await parentHandle.agent.ctx.agents.create({
|
||||
sessionId: SessionId('continuation-child'),
|
||||
meta: { cwd: storageDir, parentSession: SessionId('continuation-parent') },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
@@ -385,7 +422,7 @@ describe('HarnessSdkServer', () => {
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
const oldChild = await ctx.agents.create({
|
||||
const oldChild = await oldParent.agent.ctx.agents.create({
|
||||
sessionId: SessionId('reused-child'),
|
||||
meta: { cwd: storageDir, parentSession: SessionId('old-parent') },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
@@ -425,7 +462,7 @@ describe('HarnessSdkServer', () => {
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
const newChild = await ctx.agents.create({
|
||||
const newChild = await newParent.agent.ctx.agents.create({
|
||||
sessionId: SessionId('reused-child'),
|
||||
meta: { cwd: storageDir, parentSession: SessionId('new-parent') },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
@@ -483,12 +520,12 @@ describe('HarnessSdkServer', () => {
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
handle = await ctx.agents.create({
|
||||
handle = await parentHandle.agent.ctx.agents.create({
|
||||
sessionId: SessionId('fallback-child-session'),
|
||||
meta: { cwd: storageDir, parentSession: SessionId('fallback-parent') },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
})
|
||||
failedHandle = await ctx.agents.create({
|
||||
failedHandle = await parentHandle.agent.ctx.agents.create({
|
||||
sessionId: SessionId('failed-child-session'),
|
||||
meta: { cwd: storageDir },
|
||||
agentOptions: { model: 'deepseek' },
|
||||
|
||||
Reference in New Issue
Block a user