diff --git a/.changeset/subagent-model-deny-containment.md b/.changeset/subagent-model-deny-containment.md new file mode 100644 index 0000000..7b200b3 --- /dev/null +++ b/.changeset/subagent-model-deny-containment.md @@ -0,0 +1,5 @@ +--- +"@pythoughts/pythinker-code": patch +--- + +Model permission deny rules now also apply to subagent model overrides coming from agent profiles and from resume or retry, not only to models named in tool arguments; a denied override falls back to the parent agent's model. diff --git a/.changeset/workflow-event-correlation.md b/.changeset/workflow-event-correlation.md new file mode 100644 index 0000000..9b026d8 --- /dev/null +++ b/.changeset/workflow-event-correlation.md @@ -0,0 +1,5 @@ +--- +"@pythoughts/pythinker-code": patch +--- + +Subagent lifecycle events now carry the workflow name on start, completion and failure, and suspension events carry both the workflow run id and name, so clients can correlate every event without caching the spawn event. diff --git a/packages/agent-core/src/agent/permission/index.ts b/packages/agent-core/src/agent/permission/index.ts index 86f2aec..62146ad 100644 --- a/packages/agent-core/src/agent/permission/index.ts +++ b/packages/agent-core/src/agent/permission/index.ts @@ -1,6 +1,8 @@ import type { Agent } from '..'; import type { PrepareToolExecutionResult } from '../../loop'; import { createHookIfMatcher } from '../../session/hooks'; +import { matchesGlobRuleSubjects, modelRuleSubject } from '../../tools/support/rule-match'; +import { matchPermissionRule } from './matches-rule'; import { createPermissionDecisionPolicies } from './policies'; import type { ApprovalResponse, @@ -57,6 +59,40 @@ export class PermissionManager { }; } + /** + * Whether a deny rule forbids running a subagent on `modelAlias`, asked + * outside the tool-approval path. + * + * Deny rules with an argument pattern fire at approval only when their + * subject appears in the tool arguments. A model resolved after approval — + * a subagent profile's override, or a resume/retry that re-resolves it — + * never comes back through approval, so the spawn path re-checks it here + * against the same rules. + * + * Only rules whose argument pattern targets the `model:` namespace are + * consulted. Approval evaluates every rule against the call's full subject + * set (profile name, plan digest, model); this check sees only the model, so + * a rule keyed on another subject — `Agent(!reviewer)`, a workflow plan + * digest — must not be re-interpreted here: its negation would match any + * model-only subject list and strip an override approval already allowed. + */ + deniesModelOverride(toolName: string, modelAlias: string): boolean { + const subjects = modelRuleSubject(modelAlias); + if (subjects.length === 0) return false; + return this.effectiveRules.some( + (rule) => + rule.decision === 'deny' && + matchPermissionRule({ + rule, + toolName, + execution: { + matchesRule: (ruleArgs) => + targetsModelSubject(ruleArgs) && matchesGlobRuleSubjects(ruleArgs, subjects), + }, + })?.hasRuleArgs === true, + ); + } + setMode(mode: PermissionMode): void { this.agent.records.logRecord({ type: 'permission.set_mode', @@ -368,3 +404,9 @@ export class PermissionManager { return prefix; } } + +/** Whether a rule argument pattern (optionally negated) targets the `model:` subject namespace. */ +function targetsModelSubject(ruleArgs: string): boolean { + const positive = ruleArgs.startsWith('!') ? ruleArgs.slice(1) : ruleArgs; + return positive.startsWith('model:'); +} diff --git a/packages/agent-core/src/session/subagent-host.ts b/packages/agent-core/src/session/subagent-host.ts index 1c09238..16c97fd 100644 --- a/packages/agent-core/src/session/subagent-host.ts +++ b/packages/agent-core/src/session/subagent-host.ts @@ -361,6 +361,8 @@ export class SessionSubagentHost { // All subagent lifecycle events carry the launching tool call id so // consumers can correlate them to a workflow and drop stale events. parentToolCallId: event.task.parentToolCallId, + workflowRunId: event.task.workflowRunId, + workflowName: event.task.workflowName, reason: event.reason, }); } @@ -437,16 +439,28 @@ export class SessionSubagentHost { * SingleModelProvider) falls back to the parent's model instead of failing at * generate time. fastMode stays a straight inherit: it is a preference the * provider layer already drops when the active model cannot serve it. + * + * A `model:` deny rule is re-checked here as well — approval only sees a + * model that was in the tool arguments, so a profile-sourced override (or a + * resume/retry re-resolution) would otherwise ride past `Agent(model:x)` / + * `DynamicWorkflow(model:x)`. Every override lands in this method, making it + * the one containment point; a denied override falls back to the parent's + * model rather than failing the spawn. */ private childModelConfig( parent: Agent, child: Agent, profile: ResolvedAgentProfile | undefined, - options: Pick, + options: Pick, ): { modelAlias: string | undefined; thinkingLevel: string | undefined; fastMode: boolean } { const requested = options.modelAlias ?? profile?.model; const modelAlias = - requested !== undefined && child.config.canResolveModel(requested) + requested !== undefined && + child.config.canResolveModel(requested) && + !parent.permission.deniesModelOverride( + options.workflowRunId === undefined ? 'Agent' : 'DynamicWorkflow', + requested, + ) ? requested : parent.config.modelAlias; return { @@ -562,6 +576,7 @@ export class SessionSubagentHost { subagentId: childId, parentToolCallId: options.parentToolCallId, workflowRunId: options.workflowRunId, + workflowName: options.workflowName, resultSummary: result, usage, contextTokens: child.context.tokenCount, @@ -712,6 +727,7 @@ export class SessionSubagentHost { subagentId: childId, parentToolCallId: options.parentToolCallId, workflowRunId: options.workflowRunId, + workflowName: options.workflowName, }); } @@ -727,6 +743,7 @@ export class SessionSubagentHost { subagentId: childId, parentToolCallId: options.parentToolCallId, workflowRunId: options.workflowRunId, + workflowName: options.workflowName, error: error instanceof Error ? error.message : String(error), }); } diff --git a/packages/agent-core/test/session/subagent-host.test.ts b/packages/agent-core/test/session/subagent-host.test.ts index 680d3c2..65fe52b 100644 --- a/packages/agent-core/test/session/subagent-host.test.ts +++ b/packages/agent-core/test/session/subagent-host.test.ts @@ -1583,6 +1583,55 @@ describe('SessionSubagentHost', () => { }), }), ); + // Every lifecycle event carries the correlation pair, not only spawned — + // a consumer that joins on completion must not need a spawned-event cache. + for (const event of ['subagent.started', 'subagent.completed']) { + expect(parent.allEvents).toContainEqual( + expect.objectContaining({ + type: '[rpc]', + event, + args: expect.objectContaining({ + subagentId: 'agent-0', + workflowRunId: 'wfr-test-001', + workflowName: 'Review files', + }), + }), + ); + } + }); + + it('carries the workflow correlation pair on subagent.suspended', () => { + const parent = testAgent(); + parent.configure(); + parent.newEvents(); + + const child = testAgent({ type: 'sub' }); + child.configure(); + const session = fakeSession(parent.agent, child.agent); + const host = new SessionSubagentHost(session, 'main'); + + host.suspended({ + task: { + ...queuedTask(1), + workflowRunId: 'wfr-test-001', + workflowName: 'Review files', + }, + agentId: 'agent-7', + reason: 'rate_limited', + }); + + expect(parent.allEvents).toContainEqual( + expect.objectContaining({ + type: '[rpc]', + event: 'subagent.suspended', + args: expect.objectContaining({ + subagentId: 'agent-7', + workflowRunId: 'wfr-test-001', + workflowName: 'Review files', + reason: 'rate_limited', + }), + }), + ); }); it('retries a rate-limited child turn without appending the original prompt again', async () => { @@ -1846,6 +1895,216 @@ describe('SessionSubagentHost', () => { expect(child.agent.config.modelAlias).not.toBe(parent.agent.config.modelAlias); expect(child.agent.config.thinkingLevel).toBe('medium'); }); + + it('an Agent(model:) deny rule strips a profile-routed model override', async () => { + const parent = testAgent(); + parent.configure(); + parent.agent.permission.setMode('yolo'); + // The rule fires at tool approval only when the model appears in the tool + // arguments. A profile-sourced override never goes back through approval, + // so the spawn-time containment check is what has to strip it. + parent.agent.permission.rules.push({ + decision: 'deny', + scope: 'session-runtime', + pattern: 'Agent(model:implementer-model)', + }); + + const child = testAgent(); + child.configure({ tools: ['Read'] }); + child.configureRuntimeModel({ type: 'pythinker', apiKey: 'test-key', model: 'implementer-model' }); + child.agent.context.appendUserMessage([{ type: 'text', text: 'Earlier context' }]); + child.mockNextResponse({ + type: 'text', + text: 'Resumed the subagent on the contained model and carried the assigned task through to completion, then reported a full and detailed technical summary of every change so the parent agent can continue without repeating any prior work.', + }); + + const implementerProfile: ResolvedAgentProfile = { + name: 'implementer', + description: 'Cheap implementer routed to another model.', + systemPrompt: () => 'implementer system prompt', + tools: ['Read'], + model: 'implementer-model', + effort: 'medium', + }; + child.agent.useProfile(implementerProfile); + + const session = Object.assign( + fakeSession(parent.agent, child.agent, { + 'agent-0': { type: 'sub', parentAgentId: 'main' }, + }), + { agentProfiles: { implementer: implementerProfile } }, + ); + const host = new SessionSubagentHost(session, 'main'); + + const handle = await host.resume('agent-0', { + parentToolCallId: 'call_agent', + prompt: 'Continue from context', + description: 'Continue work', + runInBackground: false, + signal, + }); + await handle.completion; + + expect(child.agent.config.modelAlias).toBe(parent.agent.config.modelAlias); + expect(child.agent.config.modelAlias).not.toBe('implementer-model'); + }); + + it('scopes model deny rules to the spawning surface', async () => { + const parent = testAgent(); + parent.configure(); + parent.agent.permission.setMode('yolo'); + // A workflow child answers to DynamicWorkflow(model:...) rules; an + // Agent-scoped rule must not strip its override. + parent.agent.permission.rules.push({ + decision: 'deny', + scope: 'session-runtime', + pattern: 'Agent(model:implementer-model)', + }); + + const child = testAgent(); + child.configure({ tools: ['Read'] }); + child.configureRuntimeModel({ type: 'pythinker', apiKey: 'test-key', model: 'implementer-model' }); + child.agent.context.appendUserMessage([{ type: 'text', text: 'Earlier context' }]); + child.mockNextResponse({ + type: 'text', + text: 'Resumed the routed workflow subagent from its earlier context and carried the assigned task through to completion, then reported a full and detailed technical summary of every change so the parent agent can continue without repeating any prior work.', + }); + + const implementerProfile: ResolvedAgentProfile = { + name: 'implementer', + description: 'Cheap implementer routed to another model.', + systemPrompt: () => 'implementer system prompt', + tools: ['Read'], + model: 'implementer-model', + effort: 'medium', + }; + child.agent.useProfile(implementerProfile); + + const session = Object.assign( + fakeSession(parent.agent, child.agent, { + 'agent-0': { type: 'sub', parentAgentId: 'main' }, + }), + { agentProfiles: { implementer: implementerProfile } }, + ); + const host = new SessionSubagentHost(session, 'main'); + + const handle = await host.resume('agent-0', { + parentToolCallId: 'call_agent', + prompt: 'Continue from context', + description: 'Continue work', + runInBackground: false, + workflowRunId: 'wfr-scope-test', + signal, + }); + await handle.completion; + + expect(child.agent.config.modelAlias).toBe('implementer-model'); + }); + + it('ignores negated non-model deny rules when containing a model override', async () => { + const parent = testAgent(); + parent.configure(); + parent.agent.permission.setMode('yolo'); + // `Agent(!reviewer)` is a profile policy ("only reviewer may spawn"). + // Approval evaluates it against the call's full subject set; the spawn-time + // model re-check sees only `model:`, where the negation would match + // anything — it must not be re-interpreted as a model rule. + parent.agent.permission.rules.push({ + decision: 'deny', + scope: 'session-runtime', + pattern: 'Agent(!implementer)', + }); + + const child = testAgent(); + child.configure({ tools: ['Read'] }); + child.configureRuntimeModel({ type: 'pythinker', apiKey: 'test-key', model: 'implementer-model' }); + child.agent.context.appendUserMessage([{ type: 'text', text: 'Earlier context' }]); + child.mockNextResponse({ + type: 'text', + text: 'Resumed the routed subagent past the negated profile rule and carried the assigned task through to completion, then reported a full and detailed technical summary of every change so the parent agent can continue without repeating any prior work.', + }); + + const implementerProfile: ResolvedAgentProfile = { + name: 'implementer', + description: 'Cheap implementer routed to another model.', + systemPrompt: () => 'implementer system prompt', + tools: ['Read'], + model: 'implementer-model', + effort: 'medium', + }; + child.agent.useProfile(implementerProfile); + + const session = Object.assign( + fakeSession(parent.agent, child.agent, { + 'agent-0': { type: 'sub', parentAgentId: 'main' }, + }), + { agentProfiles: { implementer: implementerProfile } }, + ); + const host = new SessionSubagentHost(session, 'main'); + + const handle = await host.resume('agent-0', { + parentToolCallId: 'call_agent', + prompt: 'Continue from context', + description: 'Continue work', + runInBackground: false, + signal, + }); + await handle.completion; + + expect(child.agent.config.modelAlias).toBe('implementer-model'); + }); + + it('applies a negated model deny rule to the override it excepts', async () => { + const parent = testAgent(); + parent.configure(); + parent.agent.permission.setMode('yolo'); + // "Deny every model except implementer-model" — the excepted model must + // survive containment, proving negated model rules are evaluated rather + // than skipped along with the non-model patterns. + parent.agent.permission.rules.push({ + decision: 'deny', + scope: 'session-runtime', + pattern: 'Agent(!model:implementer-model)', + }); + + const child = testAgent(); + child.configure({ tools: ['Read'] }); + child.configureRuntimeModel({ type: 'pythinker', apiKey: 'test-key', model: 'implementer-model' }); + child.agent.context.appendUserMessage([{ type: 'text', text: 'Earlier context' }]); + child.mockNextResponse({ + type: 'text', + text: 'Resumed the routed subagent on the excepted model and carried the assigned task through to completion, then reported a full and detailed technical summary of every change so the parent agent can continue without repeating any prior work.', + }); + + const implementerProfile: ResolvedAgentProfile = { + name: 'implementer', + description: 'Cheap implementer routed to another model.', + systemPrompt: () => 'implementer system prompt', + tools: ['Read'], + model: 'implementer-model', + effort: 'medium', + }; + child.agent.useProfile(implementerProfile); + + const session = Object.assign( + fakeSession(parent.agent, child.agent, { + 'agent-0': { type: 'sub', parentAgentId: 'main' }, + }), + { agentProfiles: { implementer: implementerProfile } }, + ); + const host = new SessionSubagentHost(session, 'main'); + + const handle = await host.resume('agent-0', { + parentToolCallId: 'call_agent', + prompt: 'Continue from context', + description: 'Continue work', + runInBackground: false, + signal, + }); + await handle.completion; + + expect(child.agent.config.modelAlias).toBe('implementer-model'); + }); }); describe('Session resume permission parent chain', () => { diff --git a/packages/protocol/src/events.ts b/packages/protocol/src/events.ts index 4b88fc5..51785e6 100644 --- a/packages/protocol/src/events.ts +++ b/packages/protocol/src/events.ts @@ -531,6 +531,8 @@ export interface SubagentStartedEvent { readonly parentToolCallId?: string; /** Identifies the Dynamic Workflow run this subagent belongs to; absent outside a workflow. */ readonly workflowRunId?: string; + /** The workflow's user-facing description, repeated on each subagent for correlation. */ + readonly workflowName?: string; } export interface SubagentSuspendedEvent { @@ -538,6 +540,10 @@ export interface SubagentSuspendedEvent { readonly subagentId: string; /** Tool call in the parent agent that spawned the subagent; absent when spawned outside a tool call. */ readonly parentToolCallId?: string; + /** Identifies the Dynamic Workflow run this subagent belongs to; absent outside a workflow. */ + readonly workflowRunId?: string; + /** The workflow's user-facing description, repeated on each subagent for correlation. */ + readonly workflowName?: string; readonly reason: string; } @@ -548,6 +554,8 @@ export interface SubagentCompletedEvent { readonly parentToolCallId?: string; /** Identifies the Dynamic Workflow run this subagent belongs to; absent outside a workflow. */ readonly workflowRunId?: string; + /** The workflow's user-facing description, repeated on each subagent for correlation. */ + readonly workflowName?: string; readonly resultSummary: string; readonly usage?: TokenUsage; readonly contextTokens?: number; @@ -560,6 +568,8 @@ export interface SubagentFailedEvent { readonly parentToolCallId?: string; /** Identifies the Dynamic Workflow run this subagent belongs to; absent outside a workflow. */ readonly workflowRunId?: string; + /** The workflow's user-facing description, repeated on each subagent for correlation. */ + readonly workflowName?: string; readonly error: string; } @@ -1208,12 +1218,15 @@ export const subagentStartedEventSchema = z.object({ subagentId: z.string(), parentToolCallId: z.string().optional(), workflowRunId: z.string().optional(), + workflowName: z.string().optional(), }) satisfies z.ZodType; export const subagentSuspendedEventSchema = z.object({ type: z.literal('subagent.suspended'), subagentId: z.string(), parentToolCallId: z.string().optional(), + workflowRunId: z.string().optional(), + workflowName: z.string().optional(), reason: z.string(), }) satisfies z.ZodType; @@ -1222,6 +1235,7 @@ export const subagentCompletedEventSchema = z.object({ subagentId: z.string(), parentToolCallId: z.string().optional(), workflowRunId: z.string().optional(), + workflowName: z.string().optional(), resultSummary: z.string(), usage: tokenUsageSchema.optional(), contextTokens: z.number().optional(), @@ -1232,6 +1246,7 @@ export const subagentFailedEventSchema = z.object({ subagentId: z.string(), parentToolCallId: z.string().optional(), workflowRunId: z.string().optional(), + workflowName: z.string().optional(), error: z.string(), }) satisfies z.ZodType;