From 106c61e6eebdf4d1bf99c27fb6c607b402ba79e5 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 25 Jul 2026 23:44:56 -0700 Subject: [PATCH] fix(agent-tools): close the fn_delegate_task Deny bypass and the store's window clamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 13a2b2a9d, from a multi-agent review of that commit. Three of its claims did not hold. 1. fn_delegate_task bypassed the gate entirely (P0). It reaches the same createAgentTask primitive, was registered unconditionally in both session lanes, and validated only that the TARGET agent is non-ephemeral — never the caller. Under Deny an ephemeral worker could enumerate agents and delegate unlimited tasks. It is now withheld under Deny, and also under upon_validation: delegation has no proposal channel, so leaving it available would launder a create past the operator review that policy requires. 2. The widened dedupe window was capped at 5 minutes. The store query in branch-and-pr-entities.ts carried its own independent `?? 60_000` / `min(300_000, …)` pair, so widening only duplicate-guard.ts under-delivered and made the new ceiling unreachable. Both sites now share FINGERPRINT_WINDOW_DEFAULT_MS / FINGERPRINT_WINDOW_MAX_MS. 3. The pi-extension gate does not fire at all. pi's ExtensionContext carries no agentId — the read is a speculative cast and only tests supply one, so every real call short-circuits as a human caller. The fail-closed direction is kept for the day an identity signal exists, but the limitation is now documented instead of implied to be enforcement. Also: the session prompt now states when creation is disabled and names fn_task_log as the fallback (the base prompt still taught fn_task_create, which is the same instruction/capability mismatch that fed the retry storm); suppression emits an `agent:task-create-withheld` run-audit event; and the two source-text ratchet tests are replaced with behavioral assertions on the tool list the executor actually hands the model — verified to fail when the guard is broken, which the string assertions did not. Co-Authored-By: Claude Opus 5 (1M context) --- .../ephemeral-task-create-deny-hides-tool.md | 4 +- docs/settings-reference.md | 8 +- packages/cli/src/extension.ts | 25 +-- .../src/__tests__/duplicate-guard.test.ts | 63 +++++++- packages/core/src/duplicate-guard.ts | 13 +- .../src/task-store/branch-and-pr-entities.ts | 12 +- .../ephemeral-task-create-gate.test.ts | 152 ++++++++++++++++-- packages/engine/src/agent-tools.ts | 46 ++++++ packages/engine/src/executor.ts | 79 ++++++++- packages/engine/src/step-session-executor.ts | 13 +- 10 files changed, 372 insertions(+), 43 deletions(-) diff --git a/.changeset/ephemeral-task-create-deny-hides-tool.md b/.changeset/ephemeral-task-create-deny-hides-tool.md index 9990b22267..f7b5684c7d 100644 --- a/.changeset/ephemeral-task-create-deny-hides-tool.md +++ b/.changeset/ephemeral-task-create-deny-hides-tool.md @@ -2,6 +2,6 @@ "@runfusion/fusion": patch --- -summary: Deny now hides fn_task_create from agents, and retried task creates no longer duplicate. +summary: Deny now withholds task-creating tools from agent sessions, and retried creates no longer duplicate. category: fix -dev: Adds `isAgentTaskCreateToolAvailable(settings, callerIsEphemeral)` in `@fusion/engine` agent-tools; the outer execution session (`executor.ts`) and per-step workflow sessions (`step-session-executor.ts`) omit `fn_task_create` from the tool list when the project policy resolves to `deny`. `isEphemeralCallerAgent` in the pi extension now fails closed: a caller id that is present but unresolvable counts as ephemeral, so the policy still applies. `upon_validation` keeps the tool (it proposes to the mailbox); permanent-agent and human/chat callers are unaffected. Separately, the deterministic content-fingerprint duplicate window in `duplicate-guard.ts` goes 60s -> 10m (clamp ceiling 5m -> 1h) so an agent that retries a create after a tool timeout links the existing task instead of filing a second one. +dev: Adds `isAgentTaskCreateToolAvailable` and `isAgentDelegateTaskToolAvailable` in `@fusion/engine` agent-tools. The outer execution session (`executor.ts`) and per-step workflow sessions (`step-session-executor.ts`) omit `fn_task_create` under `deny` and `fn_delegate_task` under both `deny` and `upon_validation` (delegation reaches the same `createAgentTask` primitive but has no proposal channel, so leaving it available would bypass operator validation). Suppression emits an `agent:task-create-withheld` run-audit event and appends a prompt section naming `fn_task_log` as the fallback, so the withheld tool reads as policy rather than malfunction. Execute-time refusals are retained as defense in depth. The pi extension's `isEphemeralCallerAgent` now fails closed, but that lane remains unenforced because pi's extension context carries no agent identity — documented as a known gap. Separately, the deterministic content-fingerprint duplicate window goes 60s -> 10m; the store query in `branch-and-pr-entities.ts` carried its own independent `60s`/`5m` clamp that capped the effective window, so both sites now share `FINGERPRINT_WINDOW_DEFAULT_MS`/`FINGERPRINT_WINDOW_MAX_MS`. diff --git a/docs/settings-reference.md b/docs/settings-reference.md index 5217781dd8..fb6f4d9ac9 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -1785,11 +1785,13 @@ Project-scoped default permission policy for agent runtime action gates. It appl ### `ephemeralAgentTaskCreationPolicy` -Project-scoped policy for ephemeral/runtime-managed task workers calling `fn_task_create`. +Project-scoped policy for ephemeral/runtime-managed task workers calling `fn_task_create` or `fn_delegate_task` (delegation creates a task through the same path, so the policy governs both). - `allow` creates follow-up tasks immediately. -- `upon_validation` sends a structured proposal to the operator mailbox. The operator can create the proposed task from the message; repeated requests reuse a durable proposal key so one proposal materializes at most one task. -- `deny` rejects ephemeral follow-up creation. Permanent agents and human/dashboard callers are unaffected. +- `upon_validation` sends a structured proposal to the operator mailbox. The operator can create the proposed task from the message; repeated requests reuse a durable proposal key so one proposal materializes at most one task. `fn_delegate_task` is withheld under this policy — delegation has no proposal channel, so allowing it would bypass the operator review this policy exists to require. +- `deny` withholds both tools from the agent's tool list entirely: an ephemeral session never sees `fn_task_create` or `fn_delegate_task`, and the session prompt states that creation is disabled and points the agent at `fn_task_log` instead. An execute-time refusal is retained as defense in depth. Permanent agents and human/dashboard callers are unaffected. + - Suppression emits an `agent:task-create-withheld` run-audit event (ids/policy/outcomes only) so an operator can tell "policy suppressed the tool" apart from "the agent had nothing to file". + - Known gap: the `fn_task_create` registered by the pi extension is gated only at execute time, and that gate does not currently fire because pi's extension context carries no agent identity. Enforcement today comes from the engine session lanes, which withhold the tools outright. - The setting deliberately has no materialized default. The resolver falls back to `allow`; legacy persisted `ephemeralAgentsCanCreateTasks: false` still resolves to `deny` (and legacy `true` resolves to `allow`). - The unified runtime policy still applies: `defaultAgentPermissionPolicy.toolRules.fn_task_create = "block"` blocks ephemeral and permanent agents, and `"require-approval"` creates an approval request before the tool can run. diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 4062c6e3f8..4105f99357 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -728,19 +728,24 @@ async function validateAssignableAgentId( FNXC:EphemeralAgentTaskCreation 2026-07-01-00:00: fn_task_create runs inside whatever agent loaded the pi extension. When the caller is an ephemeral/runtime task-worker (executor-FN-XXXX and friends), the project setting `ephemeralAgentsCanCreateTasks` decides whether it may open new tasks. Human/dashboard/CLI callers have no `ctx.agentId`, so they are never gated here — the setting only constrains runtime-managed agents. -Resolution is fail-open on lookup errors: a missing/unresolvable caller is treated as non-ephemeral so a store hiccup never blocks legitimate task creation. FNXC:EphemeralAgentTaskCreation 2026-07-26-06:20: -Fail-open was too generous for the identity signal itself. A runtime task-worker session always -carries a caller id; only a human/dashboard/CLI caller has none. So an id that is PRESENT but does -not resolve to an agent row (deleted ephemeral row, cross-project store, transient read failure) is -a runtime caller with an unknown identity, and is now classified ephemeral so the project policy -still applies. An absent id keeps the old human pass-through, and a resolved permanent agent is -still never gated. +Lookup resolution is now fail-CLOSED (superseding the original fail-open rule stated here): a +runtime task-worker session carries a caller id, only a human/dashboard/CLI caller has none, so an +id that is PRESENT but does not resolve to an agent row (deleted ephemeral row, cross-project +store, transient read failure) is a runtime caller with unknown identity and is classified +ephemeral so the project policy still applies. An absent id keeps the human pass-through, and a +resolved permanent agent is still never gated. -Incident: with the project policy on Deny, an executing agent still filed ten follow-up tasks — -an execute-time gate that answers "not ephemeral" whenever identity resolution comes up empty is -indistinguishable from no gate at all on exactly the sessions the setting exists to constrain. +FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: +KNOWN LIMITATION — this gate does not currently fire in production, and must not be counted as +enforcement of the Deny policy. pi's `ExtensionContext` (pi-coding-agent, core/extensions/types) +carries no `agentId`; the read at the fn_task_create execute site is a speculative cast, and only +tests ever supply one. Every real call therefore short-circuits at `!callerAgentId` and passes +through as a human caller. The fail-closed direction above is correct for the day an identity +signal exists, but making this lane genuinely enforce Deny needs the engine to thread the session's +agent/task identity into the extension context (env or an augmented context) — a plumbing decision, +not a local fix. Enforcement today lives in the engine lanes, which withhold the tool outright. */ async function isEphemeralCallerAgent(cwd: string, callerAgentId: string | undefined): Promise { if (!callerAgentId) return false; diff --git a/packages/core/src/__tests__/duplicate-guard.test.ts b/packages/core/src/__tests__/duplicate-guard.test.ts index b67104b937..d1f77426e6 100644 --- a/packages/core/src/__tests__/duplicate-guard.test.ts +++ b/packages/core/src/__tests__/duplicate-guard.test.ts @@ -3,7 +3,10 @@ import { describe, expect, it, vi } from "vitest"; import type { Column, Task } from "../types.js"; import type { TaskStore } from "../store.js"; import { computeContentFingerprint } from "../duplicate-detection.js"; +import { findRecentTasksByContentFingerprintImpl } from "../task-store/branch-and-pr-entities.js"; import { + FINGERPRINT_WINDOW_DEFAULT_MS, + FINGERPRINT_WINDOW_MAX_MS, __getDeterministicGuardMutexSize, reconcileDeterministicDuplicate, runDeterministicDuplicateGuard, @@ -32,8 +35,12 @@ function makeStore(seed: Task[] = []): { tasks: Task[]; store: TaskStore } { const tasks = [...seed]; const store = { findRecentTasksByContentFingerprint: vi.fn().mockImplementation(async (fp: string, options?: { windowMs?: number; includeArchived?: boolean }) => { - // Mirrors clampWindowMs in duplicate-guard.ts (default 10m, ceiling 1h). - const windowMs = Math.max(1, Math.min(3_600_000, Math.trunc(options?.windowMs ?? 600_000))); + /* + FNXC:TaskCreationDeduplication 2026-07-26-07:40: + Import the real bounds instead of hand-mirroring them. A hardcoded copy here is what let the + widened window look correct in tests while the production store clamped it back to 5 minutes. + */ + const windowMs = Math.max(1, Math.min(FINGERPRINT_WINDOW_MAX_MS, Math.trunc(options?.windowMs ?? FINGERPRINT_WINDOW_DEFAULT_MS))); const cutoff = Date.now() - windowMs; return tasks .filter((task) => task.source?.sourceMetadata?.contentFingerprint === fp) @@ -313,3 +320,55 @@ describe("reconcileDeterministicDuplicate", () => { expect(warn).toHaveBeenCalled(); }); }); + +/* +FNXC:TaskCreationDeduplication 2026-07-26-07:40: +The store query owns a SECOND clamp on the same window. Code review found that widening only +duplicate-guard.ts capped the effective window at the store's own 5-minute ceiling, and no test +caught it because the guard tests stub the query. These assertions pin the real cutoff the SQL +receives, so the two clamps cannot drift apart again. +*/ +describe("findRecentTasksByContentFingerprintImpl window", () => { + function stubStore(): { store: TaskStore; cutoffs: string[] } { + const cutoffs: string[] = []; + const store = { + backendMode: false, + getTaskSelectClause: () => "t.*", + rowToTask: (row: unknown) => row as Task, + db: { + prepare: () => ({ + all: (_fingerprint: string, cutoffIso: string) => { + cutoffs.push(cutoffIso); + return []; + }, + }), + }, + } as unknown as TaskStore; + return { store, cutoffs }; + } + + it("defaults to the shared 10-minute window, not the store's old 60s/5m pair", async () => { + const { store, cutoffs } = stubStore(); + const before = Date.now(); + await findRecentTasksByContentFingerprintImpl(store, "fp"); + const windowMs = before - Date.parse(cutoffs[0]!); + expect(windowMs).toBeGreaterThanOrEqual(FINGERPRINT_WINDOW_DEFAULT_MS - 5_000); + expect(windowMs).toBeLessThanOrEqual(FINGERPRINT_WINDOW_DEFAULT_MS + 5_000); + }); + + it("honors an explicit window above the old 5-minute ceiling", async () => { + const { store, cutoffs } = stubStore(); + const before = Date.now(); + await findRecentTasksByContentFingerprintImpl(store, "fp", { windowMs: 20 * 60_000 }); + const windowMs = before - Date.parse(cutoffs[0]!); + expect(windowMs).toBeGreaterThan(300_000); + }); + + it("still clamps to the shared ceiling", async () => { + const { store, cutoffs } = stubStore(); + const before = Date.now(); + await findRecentTasksByContentFingerprintImpl(store, "fp", { windowMs: 24 * 60 * 60_000 }); + const windowMs = before - Date.parse(cutoffs[0]!); + expect(windowMs).toBeLessThanOrEqual(FINGERPRINT_WINDOW_MAX_MS + 5_000); + }); +}); diff --git a/packages/core/src/duplicate-guard.ts b/packages/core/src/duplicate-guard.ts index cfff030bbc..4e4214072b 100644 --- a/packages/core/src/duplicate-guard.ts +++ b/packages/core/src/duplicate-guard.ts @@ -17,9 +17,18 @@ cheap and rare: this is an EXACT normalized title+description hash, and a legiti byte-identical content inside ten minutes is a double-submit, not distinct work. Near-duplicate (paraphrase) matching is unaffected and keeps its own thresholds/windows. The clamp ceiling rises with it so an explicit caller-supplied window is not silently cut back to five minutes. + +FNXC:TaskCreationDeduplication 2026-07-26-07:40: +Exported because the STORE query clamps the window independently. Code review caught that +findRecentTasksByContentFingerprintImpl carried its own `?? 60_000` / `Math.min(300_000, …)` +pair, so widening only this module capped the effective window at five minutes and made the +new ceiling unreachable. Two clamps for one policy is how a window silently under-delivers; +both sites now read these constants. */ -const DEFAULT_WINDOW_MS = 600_000; -const MAX_WINDOW_MS = 3_600_000; +export const FINGERPRINT_WINDOW_DEFAULT_MS = 600_000; +export const FINGERPRINT_WINDOW_MAX_MS = 3_600_000; +const DEFAULT_WINDOW_MS = FINGERPRINT_WINDOW_DEFAULT_MS; +const MAX_WINDOW_MS = FINGERPRINT_WINDOW_MAX_MS; export const deterministicGuardLocks = new Map>(); // Test-only compatibility hook used by dashboard deterministic-dedup route tests. diff --git a/packages/core/src/task-store/branch-and-pr-entities.ts b/packages/core/src/task-store/branch-and-pr-entities.ts index 8750709b95..8c44568011 100644 --- a/packages/core/src/task-store/branch-and-pr-entities.ts +++ b/packages/core/src/task-store/branch-and-pr-entities.ts @@ -13,6 +13,7 @@ import { filterTasksByBranchGroup } from "../branch-assignment.js"; import { BUILTIN_WORKFLOW_SETTINGS } from "../builtin-workflow-settings.js"; import { isBuiltinWorkflowId } from "../builtin-workflows.js"; import { fromJson } from "../db.js"; +import { FINGERPRINT_WINDOW_DEFAULT_MS, FINGERPRINT_WINDOW_MAX_MS } from "../duplicate-guard.js"; import * as schema from "../postgres/schema/index.js"; import { taskProjectScope } from "../postgres/data-layer.js"; import { ensureBranchGroupForSource as ensureBranchGroupForSourceAsync, ensurePrEntityForSource as ensurePrEntityForSourceAsync, getActivePrEntityBySource as getActivePrEntityBySourceAsync, getBranchGroup as getBranchGroupAsync, getBranchGroupByBranchName as getBranchGroupByBranchNameAsync, getBranchGroupBySource as getBranchGroupBySourceAsync, getPrEntity as getPrEntityAsync, getPrThreadState as getPrThreadStateAsync, listActivePrEntities as listActivePrEntitiesAsync, listBranchGroups as listBranchGroupsAsync, listPrThreadStates as listPrThreadStatesAsync, recordPrThreadOutcome as recordPrThreadOutcomeAsync } from "./async-branch-groups.js"; @@ -682,8 +683,15 @@ export async function findRecentTasksByContentFingerprintImpl(store: TaskStore, return []; } - const requestedWindowMs = options?.windowMs ?? 60_000; - const windowMs = Math.max(1, Math.min(300_000, Math.trunc(requestedWindowMs))); + /* + FNXC:TaskCreationDeduplication 2026-07-26-07:40: + Share the duplicate-guard's window constants. This query previously carried its own + `?? 60_000` / `Math.min(300_000, …)` pair, so widening the guard alone capped the effective + window at five minutes and made its ceiling unreachable — the guard asked for ten minutes + and silently got five. One policy, one pair of bounds. + */ + const requestedWindowMs = options?.windowMs ?? FINGERPRINT_WINDOW_DEFAULT_MS; + const windowMs = Math.max(1, Math.min(FINGERPRINT_WINDOW_MAX_MS, Math.trunc(requestedWindowMs))); const cutoffIso = new Date(Date.now() - windowMs).toISOString(); const includeArchived = options?.includeArchived ?? false; diff --git a/packages/engine/src/__tests__/ephemeral-task-create-gate.test.ts b/packages/engine/src/__tests__/ephemeral-task-create-gate.test.ts index 6c532e5340..c2006e79c4 100644 --- a/packages/engine/src/__tests__/ephemeral-task-create-gate.test.ts +++ b/packages/engine/src/__tests__/ephemeral-task-create-gate.test.ts @@ -1,8 +1,14 @@ -import { describe, expect, it } from "vitest"; -import { readFileSync } from "node:fs"; -import { fileURLToPath } from "node:url"; +import { describe, expect, it, vi, beforeEach } from "vitest"; import type { TaskStore } from "@fusion/core"; -import { createTaskCreateTool, isAgentTaskCreateToolAvailable } from "../agent-tools.js"; +import "./executor-test-helpers.js"; +import { createMockStore, mockedCreateFnAgent, resetExecutorMocks } from "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { + createTaskCreateTool, + createDelegateTaskTool, + isAgentDelegateTaskToolAvailable, + isAgentTaskCreateToolAvailable, +} from "../agent-tools.js"; /* FNXC:EphemeralAgentTaskCreation 2026-07-26-06:20: @@ -23,8 +29,48 @@ function settings(policy?: "allow" | "upon_validation" | "deny", legacy?: boolea } as Parameters[0]; } -function readEngineSource(relativePath: string): string { - return readFileSync(fileURLToPath(new URL(`../${relativePath}`, import.meta.url)), "utf8"); +/* +FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: +Behavioral capture of the session tool list. This replaced a pair of source-text ratchets that +asserted the call expression appeared in executor.ts/step-session-executor.ts: code review noted +they assert spelling, not behavior — an inverted condition or an unconditional include would +still pass them. Asserting on the tools the executor actually hands the model is the invariant. +*/ +async function captureExecutorSession( + policy?: "allow" | "upon_validation" | "deny", +): Promise<{ toolNames: string[]; systemPrompt: string }> { + const store = createMockStore(); + store.getSettings.mockResolvedValue({ + maxConcurrent: 2, + maxWorktrees: 4, + pollIntervalMs: 15_000, + groupOverlappingFiles: false, + autoMerge: false, + ...(policy ? { ephemeralAgentTaskCreationPolicy: policy } : {}), + }); + + let toolNames: string[] = []; + let systemPrompt = ""; + mockedCreateFnAgent.mockImplementation((async (opts: { customTools?: Array<{ name: string }>; systemPrompt?: string }) => { + toolNames = (opts.customTools ?? []).map((tool) => tool.name); + systemPrompt = opts.systemPrompt ?? ""; + return { session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn() } }; + }) as never); + + const executor = new TaskExecutor(store, "/tmp/test"); + await executor.execute({ + id: "FN-001", + title: "Test", + description: "Test", + column: "in-progress", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }); + return { toolNames, systemPrompt }; } describe("isAgentTaskCreateToolAvailable", () => { @@ -76,14 +122,94 @@ describe("fn_task_create execute-time gate (defense in depth)", () => { }); }); -describe("engine lanes that register fn_task_create", () => { - it("guards the outer execution session registration with the availability predicate", () => { - const source = readEngineSource("executor.ts"); - expect(source).toContain("isAgentTaskCreateToolAvailable(settings, executionCallerIsEphemeral)"); +describe("isAgentDelegateTaskToolAvailable", () => { + /* + FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: + fn_delegate_task reaches the same createAgentTask primitive, so Deny must cover it. It is + stricter than fn_task_create: upon_validation also withholds it, because delegation has no + proposal channel and would otherwise launder a create past the operator review. + */ + it("withholds delegation from an ephemeral caller under deny", () => { + expect(isAgentDelegateTaskToolAvailable(settings("deny"), true)).toBe(false); }); - it("guards the per-step workflow session registration with the availability predicate", () => { - const source = readEngineSource("step-session-executor.ts"); - expect(source).toContain("isAgentTaskCreateToolAvailable(settings, this.options.callerIsEphemeral)"); + it("withholds delegation under upon_validation (no proposal channel to validate through)", () => { + expect(isAgentDelegateTaskToolAvailable(settings("upon_validation"), true)).toBe(false); + }); + + it("allows delegation under allow, and always for non-ephemeral callers", () => { + expect(isAgentDelegateTaskToolAvailable(settings("allow"), true)).toBe(true); + expect(isAgentDelegateTaskToolAvailable(settings("deny"), false)).toBe(true); + }); + + it("refuses at execute time without creating a task", async () => { + let createCalls = 0; + const taskStore = { + getSettings: async () => settings("deny"), + createTask: async () => { createCalls += 1; throw new Error("createTask must not run under deny"); }, + } as unknown as TaskStore; + const agentStore = { + getAgent: async () => { throw new Error("target lookup must not run before the caller gate"); }, + } as never; + + const tool = createDelegateTaskTool(agentStore, taskStore, { callerIsEphemeral: true }); + const result = await (tool.execute as unknown as ( + id: string, + params: unknown, + ) => Promise<{ isError?: boolean; details?: unknown }>)( + "call-1", + { agent_id: "agent-permanent", description: "Delegated follow-up" }, + ); + + expect(result.isError).toBe(true); + expect((result.details as { rule?: string }).rule).toBe("ephemeral-agents-cannot-create-tasks"); + expect(createCalls).toBe(0); + }); +}); + +describe("executor session tool list (behavioral)", () => { + beforeEach(() => { + resetExecutorMocks(); + }); + + it("hands the model no task-creating tool under deny", async () => { + const { toolNames } = await captureExecutorSession("deny"); + expect(toolNames).not.toContain("fn_task_create"); + expect(toolNames).not.toContain("fn_delegate_task"); + // Sanity: the session still has its other tools, so this is suppression, not an empty list. + expect(toolNames).toContain("fn_task_done"); + }); + + it("keeps fn_task_create under upon_validation but still withholds delegation", async () => { + const { toolNames } = await captureExecutorSession("upon_validation"); + expect(toolNames).toContain("fn_task_create"); + expect(toolNames).not.toContain("fn_delegate_task"); + }); + + it("keeps both tools when the policy allows creation", async () => { + const { toolNames } = await captureExecutorSession("allow"); + expect(toolNames).toContain("fn_task_create"); + }); + + it("defaults to allow when no policy is persisted", async () => { + const { toolNames } = await captureExecutorSession(); + expect(toolNames).toContain("fn_task_create"); + }); + + /* + FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: + The base prompt teaches fn_task_create in several places. Withholding the tool without + correcting the prompt recreates the instruction/capability mismatch behind the incident, so + the session prompt must state the absence and name the fallback. + */ + it("tells the agent the tool is withheld by policy and what to do instead", async () => { + const { systemPrompt } = await captureExecutorSession("deny"); + expect(systemPrompt).toContain("Follow-up task creation is disabled for this session"); + expect(systemPrompt).toContain("fn_task_log"); + }); + + it("adds no withheld-tool guidance when creation is allowed", async () => { + const { systemPrompt } = await captureExecutorSession("allow"); + expect(systemPrompt).not.toContain("Follow-up task creation is disabled for this session"); }); }); diff --git a/packages/engine/src/agent-tools.ts b/packages/engine/src/agent-tools.ts index be038bab51..eddf4fd4f4 100644 --- a/packages/engine/src/agent-tools.ts +++ b/packages/engine/src/agent-tools.ts @@ -1001,6 +1001,29 @@ export function isAgentTaskCreateToolAvailable( return fusionCore.resolveEphemeralTaskCreationPolicy(settings ?? {}) !== "deny"; } +/** + * FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: + * `fn_delegate_task` creates a task through the same `createAgentTask` primitive as + * `fn_task_create`, so the follow-up-task policy must govern both or it governs neither. + * Code review of the first Deny fix found the gap: the tool validated only that the TARGET + * agent is non-ephemeral and never checked the CALLER, so under Deny an ephemeral worker + * could enumerate agents and delegate unlimited tasks to any permanent one — reproducing the + * ten-duplicate incident through a sibling tool name. + * + * Delegation is withheld under BOTH non-allow policies, which is stricter than the + * `fn_task_create` rule. `upon_validation` means "an operator approves before work is filed"; + * delegation has no proposal channel of its own, so honoring it as an allow would launder a + * create past the very validation the operator asked for. Under `upon_validation` the agent + * still has the sanctioned path: `fn_task_create` remains registered and mails a proposal. + */ +export function isAgentDelegateTaskToolAvailable( + settings: Pick | undefined | null, + callerIsEphemeral: boolean | undefined, +): boolean { + if (!callerIsEphemeral) return true; + return fusionCore.resolveEphemeralTaskCreationPolicy(settings ?? {}) === "allow"; +} + type AgentTaskCreationOptions = { rootDir?: string; bypassDuplicateCheck?: boolean; @@ -4839,6 +4862,29 @@ export function createDelegateTaskTool( "fn_workflow_list to discover valid IDs.", parameters: delegateTaskParams, execute: async (_id: string, params: Static) => { + /* + FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: + Caller-side policy gate, mirroring fn_task_create. The target-agent check below is a + routing rule, not an authorization one — it never asked whether the CALLER may create + work at all. Fail open only on a settings read error so a store hiccup cannot strand + delegation for permanent agents. + */ + if (options?.callerIsEphemeral) { + const settings = typeof (taskStore as { getSettings?: unknown }).getSettings === "function" + ? await taskStore.getSettings().catch(() => ({} as Settings)) + : ({} as Settings); + if (!isAgentDelegateTaskToolAvailable(settings as Settings, true)) { + const policy = fusionCore.resolveEphemeralTaskCreationPolicy(settings as Settings); + const message = policy === "deny" + ? "Ephemeral task-worker agents are not allowed to create tasks (ephemeral agent task creation is denied for this project), and delegation creates a task." + : "Ephemeral task-worker agents must route new work through fn_task_create for operator validation; delegation cannot bypass that review."; + return { + content: [{ type: "text" as const, text: `ERROR: ${message}` }], + details: { error: message, rule: "ephemeral-agents-cannot-create-tasks", policy }, + isError: true, + }; + } + } // Validate target agent exists const agent = await agentStore.getAgent(params.agent_id); if (!agent) { diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 219ae9184a..a3066a018e 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -98,7 +98,7 @@ import { Type, type Static } from "@earendil-works/pi-ai"; import { describeModel, formatModelMarkerDetails, promptWithFallback, compactSessionContext } from "./pi.js"; import { buildAgentGatedActionSummary } from "./permanent-agent-gating.js"; import { accumulateSessionTokenUsage, captureSessionTokenBaseline, mergeTokenUsagePerModel, resetSessionTokenBaseline } from "./session-token-usage.js"; -import { finalizePlanningSegment, startPlanningSegment } from "@fusion/core"; +import { finalizePlanningSegment, startPlanningSegment, resolveEphemeralTaskCreationPolicy } from "@fusion/core"; import { enforceTaskTokenBudgetForPersist } from "./token-budget-enforcer.js"; import { createResolvedAgentSession, @@ -257,6 +257,7 @@ import { createArtifactViewTool as sharedCreateArtifactViewTool, createTaskCreateTool as sharedCreateTaskCreateTool, isAgentTaskCreateToolAvailable, + isAgentDelegateTaskToolAvailable, createTaskDocumentReadTool as sharedCreateTaskDocumentReadTool, createTaskDocumentWriteTool as sharedCreateTaskDocumentWriteTool, createTaskPromptWriteTool as sharedCreateTaskPromptWriteTool, @@ -1577,13 +1578,50 @@ The tool prevents your session from being killed by the inactivity watchdog duri - Introducing new patterns when existing local patterns should be reused - Marking a step done before required review/tooling gates are satisfied`; +/* +FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: +The base prompt teaches fn_task_create/fn_delegate_task in several places ("Out-of-scope work +found during execution", the Guardrails follow-up rule, the completion checklist). When the +project policy withholds those tools, an unmodified prompt instructs the agent to call a tool +that is not in its tool list — the same instruction/capability mismatch that produced the +original retry storm, just from the other direction. + +This override states the absence and names what to do instead, so a withheld tool reads as +policy rather than malfunction. It is appended last so it wins over the base text, and it +applies to a custom operator prompt too (an operator who overrode the prompt still gets a +truthful statement of what this session may do). +*/ +function getWithheldTaskCreationGuidance(taskCreateWithheld: boolean, delegateWithheld: boolean): string { + if (!taskCreateWithheld && !delegateWithheld) return ""; + const withheld = [ + ...(taskCreateWithheld ? ["`fn_task_create`"] : []), + ...(delegateWithheld ? ["`fn_delegate_task`"] : []), + ].join(" and "); + return `## Follow-up task creation is disabled for this session + +This project's "Ephemeral agent follow-up tasks" policy withholds ${withheld}. ${ + taskCreateWithheld && delegateWithheld ? "Those tools are" : "That tool is" + } deliberately absent from your tool list — this is an operator setting, not a malfunction or a transient error. Do not attempt to call ${ + taskCreateWithheld && delegateWithheld ? "them" : "it" + }, and do not retry. + +Ignore any instruction above that tells you to file follow-up work with ${withheld}. When you find out-of-scope work, record it instead with \`fn_task_log(message="follow-up: ...")\` and include it in your \`fn_task_done\` summary so the operator sees it. If the work genuinely blocks this task, use \`fn_task_done(outcome="blocked", reason="...")\` rather than trying to create a task for it.`; +} + /** Resolve the executor system prompt from settings, falling back to the hardcoded constant. */ -export function getExecutorSystemPrompt(settings: Settings): string { +export function getExecutorSystemPrompt( + settings: Settings, + toolAvailability?: { taskCreateWithheld?: boolean; delegateWithheld?: boolean }, +): string { const customPrompt = resolveAgentPrompt("executor", settings.agentPrompts); const basePrompt = customPrompt || EXECUTOR_SYSTEM_PROMPT; const sections = [ basePrompt, isResearchToolSurfaceEnabled(settings) ? getResearchGuidanceForSurface("executor") : "", + getWithheldTaskCreationGuidance( + toolAvailability?.taskCreateWithheld === true, + toolAvailability?.delegateWithheld === true, + ), ].filter((section) => section.trim()); return sections.join("\n\n"); } @@ -12647,15 +12685,40 @@ export class TaskExecutor { FNXC:EphemeralAgentTaskCreation 2026-07-26-06:20: A `deny` project policy removes fn_task_create from the session's tool list instead of registering a tool that only refuses at execute time; see isAgentTaskCreateToolAvailable. + + FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: + fn_delegate_task is withheld by the same policy (it creates a task through the same + primitive), and the suppression emits a run-audit event. Without the event an operator + cannot distinguish "the policy suppressed the tool" from "the agent had nothing to file" — + every other policy decision in this engine leaves that trail. */ const executionCallerIsEphemeral = !identityAgent || isEphemeralAgent(identityAgent); + const taskCreateWithheld = !isAgentTaskCreateToolAvailable(settings, executionCallerIsEphemeral); + const delegateWithheld = !isAgentDelegateTaskToolAvailable(settings, executionCallerIsEphemeral); + if (taskCreateWithheld || delegateWithheld) { + await this.store.recordRunAuditEvent?.({ + taskId: task.id, + agentId: identityAgent?.id ?? "executor", + runId: this.getRunContextFor(task.id)?.runId ?? generateSyntheticRunId("task-create-withheld", task.id), + domain: "database", + mutationType: "agent:task-create-withheld", + target: task.id, + metadata: { + taskId: task.id, + policy: resolveEphemeralTaskCreationPolicy(settings), + withheldTaskCreate: taskCreateWithheld, + withheldDelegateTask: delegateWithheld, + lane: "execution-session", + }, + }).catch(() => undefined); + } const customTools = [ this.createTaskUpdateTool(task.id, codeReviewVerdicts, sessionRef, stuckDetector), this.createTaskLogTool(task.id), this.createTaskLogsReadTool(task.id), - ...(isAgentTaskCreateToolAvailable(settings, executionCallerIsEphemeral) - ? [this.createTaskCreateTool(executionCallerIsEphemeral, task.id, identityAgent?.id)] - : []), + ...(taskCreateWithheld + ? [] + : [this.createTaskCreateTool(executionCallerIsEphemeral, task.id, identityAgent?.id)]), this.createTaskAddDepTool(task.id), this.createTaskDoneTool(task.id, worktreePath, detail.prompt ?? "", codeReviewVerdicts, () => { taskDone = true; }, audit), createRunVerificationTool({ @@ -12736,7 +12799,9 @@ export class TaskExecutor { // Agent delegation tools — discover and delegate work to other agents. ...(this.options.agentStore ? [ createListAgentsTool(this.options.agentStore), - createDelegateTaskTool(this.options.agentStore, this.store, { rootDir: this.rootDir, sourceTaskId: task.id, sourceAgentId: assignedAgentId }), + ...(delegateWithheld + ? [] + : [createDelegateTaskTool(this.options.agentStore, this.store, { rootDir: this.rootDir, sourceTaskId: task.id, sourceAgentId: assignedAgentId, callerIsEphemeral: executionCallerIsEphemeral })]), createTaskAssignTool(this.options.agentStore, this.store), ...(assignedAgentId ? [ createGetAgentConfigTool(this.options.agentStore, assignedAgentId), @@ -12908,7 +12973,7 @@ export class TaskExecutor { const executorGoalContext = executorGoalResolution.goalContext; const executorLayers = buildPromptLayers({ - basePrompt: getExecutorSystemPrompt(settings), + basePrompt: getExecutorSystemPrompt(settings, { taskCreateWithheld, delegateWithheld }), goalContext: executorGoalContext, agentInstructions: executorInstructions, pluginContributions: executorPluginContributions, diff --git a/packages/engine/src/step-session-executor.ts b/packages/engine/src/step-session-executor.ts index cc9d26895f..540524ce36 100644 --- a/packages/engine/src/step-session-executor.ts +++ b/packages/engine/src/step-session-executor.ts @@ -52,6 +52,7 @@ import { createSendMessageTool, createTaskCreateTool, isAgentTaskCreateToolAvailable, + isAgentDelegateTaskToolAvailable, createTaskDocumentReadTool, createTaskDocumentWriteTool, createTaskLogTool, @@ -1340,11 +1341,19 @@ export class StepSessionExecutor { ? [createTaskCreateTool(this.options.store, undefined, { rootDir: this.options.rootDir, callerIsEphemeral: this.options.callerIsEphemeral, sourceTaskId: this.options.sourceTaskId ?? taskDetail.id, sourceAgentId: this.options.sourceAgentId ?? taskDetail.assignedAgentId, messageStore: this.options.messageStore })] : []; - // Agent delegation tools — discover and delegate work to other agents. + /* + FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: + fn_delegate_task creates a task through the same primitive as fn_task_create, so the + follow-up-task policy withholds it on this lane too. Withheld under `deny` AND + `upon_validation`: delegation has no proposal channel, so leaving it available under + upon_validation would launder a create past the operator review that policy requires. + */ const delegationTools = this.options.agentStore ? [ createListAgentsTool(this.options.agentStore), - createDelegateTaskTool(this.options.agentStore, this.options.store!, { rootDir: this.options.rootDir, sourceTaskId: this.options.sourceTaskId ?? taskDetail.id, sourceAgentId: this.options.sourceAgentId ?? taskDetail.assignedAgentId }), + ...(isAgentDelegateTaskToolAvailable(settings, this.options.callerIsEphemeral) + ? [createDelegateTaskTool(this.options.agentStore, this.options.store!, { rootDir: this.options.rootDir, sourceTaskId: this.options.sourceTaskId ?? taskDetail.id, sourceAgentId: this.options.sourceAgentId ?? taskDetail.assignedAgentId, callerIsEphemeral: this.options.callerIsEphemeral })] + : []), createTaskAssignTool(this.options.agentStore, this.options.store!), ] : [];