From 91f5e85903977e38032d4d2e2c7aa8ca83aad537 Mon Sep 17 00:00:00 2001 From: omartelo Date: Sat, 10 Oct 2026 12:30:13 -0300 Subject: [PATCH] fix(mod): open a subagent card only once Claude Code allowed the Agent call The agent-cards mod answered the Agent tool.call before calling next, which is where Claude Code applies its permission rules, mode, dialog and PreToolUse hooks. A deny or ask rule never applied, and a call the user rejected could still open a lich session that ran the task. The mod now passes a call it takes on beneath it and opens the worker at agent.spawn, which Claude Code raises only once the call was allowed and before the native agent or its worktree exist; it denies that spawn and answers the Agent call with lich's outcome. Both orders measured on 2.1.296. Fixes #74 --- CHANGELOG.md | 9 ++++ docs/agent-cards.md | 9 ++++ hooks/agent-cards.js | 62 +++++++++++++++++++------ tests/agent-cards.engine.test.ts | 80 +++++++------------------------- tests/agent-cards.test.mjs | 54 +++++++++++++++++++-- tests/prompt-text.test.mjs | 18 +++++-- 6 files changed, 148 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c67138..132edaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **A subagent card opens only once Claude Code allowed the Agent call.** The + mod answered the call above Claude Code's permission check, so a deny or ask + rule, the permission mode and the user's "no" at the dialog never applied, + and a refused call could still open a lich session that ran the task. It now + passes the call on to that check and opens the worker at `agent.spawn`, + which only an allowed call reaches (#74). + ## [0.20.0] - 2026-10-10 ### Added diff --git a/docs/agent-cards.md b/docs/agent-cards.md index 3cbb6b1..d7faee5 100644 --- a/docs/agent-cards.md +++ b/docs/agent-cards.md @@ -91,6 +91,15 @@ for an isolated call. ## How it behaves +- **The call is put to Claude Code's own permission check first.** The mod + passes a call it takes on beneath it (`next` in `tool.call`), where Claude + Code applies its rules, the mode, the dialog and PreToolUse hooks, and opens + the worker only at `agent.spawn`, which Claude Code raises once the call was + allowed and before the native agent or its worktree exist. The mod denies + that spawn and answers the Agent call with lich's outcome. A call refused + there, the user's "no" at the dialog included, never reaches `agent.spawn`: + its refusal stands and lich opens nothing. Both orders measured on Claude + Code 2.1.296. - **The call runs in the background**, as a background subagent does: once lich has the task it answers `async_launched` and the asking turn goes on. Parallel Agent calls run their hooks concurrently, so each opens its own card at once. diff --git a/hooks/agent-cards.js b/hooks/agent-cards.js index d63d8b1..f95ae29 100644 --- a/hooks/agent-cards.js +++ b/hooks/agent-cards.js @@ -6,6 +6,13 @@ // the lich CLI (`$LICH_BIN open`), whose output and exit codes are docs/cli.md // in the lich repository. // +// The call goes through Claude Code's own permission check first (its rules, +// mode, dialog and PreToolUse hooks, all beneath `next` in tool.call), and lich +// opens the worker only at `agent.spawn`, which fires once the call was allowed +// and before the native agent or its worktree exist; the spawn is then denied +// and the Agent call answered with lich's outcome. A call refused there never +// reaches `agent.spawn`, so it opens nothing (measured on 2.1.296). +// // Once lich has the task the Agent call answers at once, as a background // subagent does, and nothing here waits on the worker: `--subagent` makes lich // type the worker's whole report at this session's prompt as a [lich] note. @@ -69,7 +76,9 @@ const SUFFIX_CHARS = 4 * @typedef {{ ticket: string, target: string, status: string, answer: string }} Report * @typedef {{ label: string, name: string, path: string, delivery?: Report }} Opened * @typedef {{ label: string, name: string, description: string, shared: boolean }} Worker - * @typedef {{ interactive: boolean, workers: Map, watching: boolean, lich: string }} State + * @typedef {{ call: AgentCall, lich: string, base: string, lang: string, answer?: object }} Claim + * @typedef {{ interactive: boolean, workers: Map, watching: boolean, lich: string, + * claims: Map }} State * @typedef {{ label: string, name: string, state: string }} Peer * @typedef {{ tool: "Agent", tool_use_id: string, agentId?: string, description: string, prompt: string, * subagent_type?: string, model?: string, team_name?: string, isolation?: string }} AgentCall @@ -329,25 +338,49 @@ async function runAsSession($, state, e, next) { const isolated = e.isolation === "worktree" const base = isolated ? await currentBranch($) : "" if (depth === undefined && base.startsWith(WORKER_BRANCH_PREFIX)) return next(e) + /** @type {Claim} */ + const claim = { call: e, lich, base, lang } + state.claims.set(e.tool_use_id, claim) + try { + const native = await next(e) + return claim.answer ?? native + } finally { + state.claims.delete(e.tool_use_id) + } +} + +/** + * The spawn of an Agent call `runAsSession` claimed, reached only once Claude + * Code allowed the call: opens the worker in its place and keeps the answer for + * the call. Before lich has the task the native agent starts instead. + * + * @param {Engine} $ + * @param {State} state + * @param {{ tool_use_id: string }} e + * @param {any} next + */ +async function openInPlaceOfSpawn($, state, e, next) { + const claim = state.claims.get(e.tool_use_id) + if (claim === undefined) return next(e) + const { call, lich, base, lang } = claim const startedMs = await $.clock.now() - const branch = isolated ? branchFor(e) : "" + const branch = call.isolation === "worktree" ? branchFor(call) : "" /** @type {Opened & { delivery: Report }} */ let opened try { - opened = await openWorker($, lich, e, branch, base) + opened = await openWorker($, lich, call, branch, base) } catch (error) { - if (next.signal.aborted) { - return { - deny: say(lang, "denyInterrupted", { - branchPart: branch ? ` ${say(lang, "onBranch", { branch })}` : "", - }), - } + if (!next.signal.aborted) return runNatively($, call, () => next(e), messageOf(error)) + claim.answer = { + deny: say(lang, "denyInterrupted", { + branchPart: branch ? ` ${say(lang, "onBranch", { branch })}` : "", + }), } - return runNatively($, e, next, messageOf(error)) + return claim.answer } - const answer = answerFor(e, opened, branch, opened.delivery, (await $.clock.now()) - startedMs, lang) - if (opened.delivery.status === "pending") trackWorker($, state, lich, opened, e, branch) - return answer + claim.answer = answerFor(call, opened, branch, opened.delivery, (await $.clock.now()) - startedMs, lang) + if (opened.delivery.status === "pending") trackWorker($, state, lich, opened, call, branch) + return { deny: `lich runs this agent as the session "${opened.label}"` } } /** @@ -475,7 +508,7 @@ async function stopWorker($, state, e, next) { /** @param {import('claude-code').On} on */ export function register(on) { /** @type {State} */ - const state = { interactive: false, workers: new Map(), watching: false, lich: "" } + const state = { interactive: false, workers: new Map(), watching: false, lich: "", claims: new Map() } // A `claude -p` started from a tool inside a lich session inherits its // variables; its subagents are its own and stay inside it. The matcher is @@ -487,5 +520,6 @@ export function register(on) { }) on("tool.call", { tool: "Agent" }, ($, e, next) => runAsSession($, state, e, next)) + on("agent.spawn", ($, e, next) => openInPlaceOfSpawn($, state, e, next)) on("tool.call", { tool: "TaskStop" }, ($, e, next) => stopWorker($, state, e, next)) } diff --git a/tests/agent-cards.engine.test.ts b/tests/agent-cards.engine.test.ts index ea41725..9978596 100644 --- a/tests/agent-cards.engine.test.ts +++ b/tests/agent-cards.engine.test.ts @@ -6,10 +6,12 @@ // // tests/agent-cards.test.mjs is the suite CI runs; this one needs a claude // binary, and proves the module loads beside mod-control.js and its hooks -// chain in the engine that runs it: a backgrounded result and a fallback to the -// native agent reach the caller as the engine relays them. The kit does not -// check a `tool.call` result against the Agent tool's output schema (measured -// on 2.1.289), so the `async_launched` shape is pinned by a live run, not here. +// chain in the engine that runs it: a call the mod takes goes on to Claude +// Code's permission check, and one refused there runs no lich. The open itself +// happens at `agent.spawn`, which Claude Code raises beneath an allowed call; +// the kit refuses `$.agent.spawn` from a test's hook (measured on 2.1.296), so +// the open, the `async_launched` result and the worker count are pinned by +// tests/agent-cards.test.mjs and a live run, not here. import { expect, mock, test } from 'claude-code/testing' import type { On, ProcessRunResult } from 'claude-code' @@ -82,43 +84,21 @@ async function footerLine($: { ui: { render: (input: object) => Promise return w.footer.modes.at(-1) } -test('a task lich took comes back at once as a backgrounded agent', async ($, on) => { - const w = world( - on, - exited(2, { ...OPENED, delivery: { ticket: 't1', target: BRANCH, status: 'pending', answer: '' } }), - ) +test('a call the mod takes goes on to the permission check, and a refusal there runs no lich', async ($, on) => { + const w = world(on) await $.session.start({ cwd: '/w', surface: 'terminal', isInteractive: true }) const ran = await $.tool.call(CALL) - expect(ran.deny).toBe(undefined) - expect(ran.isError).toBe(undefined) - expect(ran.result).toEqual( - expect.objectContaining({ status: 'async_launched', agentId: OPENED.name, outputFile: '' }), - ) - expect(ran.context).toEqual([expect.stringContaining('in this same checkout')]) - expect(w.native).toEqual([]) - expect(w.argvs).toEqual([['/opt/lich/bin/lich', 'open', '--kind', 'claude', '--subagent', '--prompt', CALL.prompt, '--json']]) -}) - -test('an isolated task opens in a worktree off the current branch', async ($, on) => { - const w = world( - on, - exited(2, { ...OPENED, delivery: { ticket: 't1', target: BRANCH, status: 'pending', answer: '' } }), - ) - await $.session.start({ cwd: '/w', surface: 'terminal', isInteractive: true }) - const ran = await $.tool.call({ ...CALL, isolation: 'worktree' }) - expect(ran.context).toEqual([expect.stringContaining(`on branch ${BRANCH}`)]) - expect(w.argvs.map((argv) => argv.slice(1, 9))).toEqual([ - ['branch', '--show-current'], - ['open', '--kind', 'claude', '--subagent', '--worktree', BRANCH, '--base', 'main'], - ]) + expect(ran.deny).toBe('the native agent ran') + expect(w.native).toEqual(['Fix the auth flow']) + expect(w.argvs).toEqual([]) }) -test('a failed open runs the native agent, with a toast saying why', async ($, on) => { - const w = world(on, exited(1, '', 'lich: no lich is running\n')) +test('a refused isolated call reads its base branch and opens no worktree', async ($, on) => { + const w = world(on) await $.session.start({ cwd: '/w', surface: 'terminal', isInteractive: true }) - await $.tool.call(CALL) + await $.tool.call({ ...CALL, isolation: 'worktree' }) expect(w.native).toEqual(['Fix the auth flow']) - expect(w.toasts).toEqual(['lich: ran "Fix the auth flow" as a Claude Code subagent: lich: no lich is running']) + expect(w.argvs).toEqual([['git', 'branch', '--show-current']]) }) test('a non-interactive run keeps its subagents native', async ($, on) => { @@ -154,34 +134,10 @@ test('a worker at lich\'s depth limit keeps its subagents native, with a toast s expect(w.toasts).toEqual([expect.stringContaining('this session is itself a lich subagent')]) }) -test('a running worker shows in the footer until lich closes it, and TaskStop closes one', async ($, on) => { - const pending = { ...OPENED, delivery: { ticket: 't1', target: BRANCH, status: 'pending', answer: '' } } - const peer = (name: string, state: string) => ({ label: name, name, project: 'lich', kind: 'claude', state }) - const w = world( - on, - exited(2, pending), - exited(2, { ...pending, label: 'second', name: 'second-1a2b' }), - exited(0, [peer(OPENED.name, 'busy'), peer('second-1a2b', 'busy')]), - exited(0, ''), - exited(0, []), - ) +test('TaskStop on a task this mod did not open goes to Claude Code', async ($, on) => { + const w = world(on) await $.session.start({ cwd: '/w', surface: 'terminal', isInteractive: true }) - await $.tool.call(CALL) - await $.tool.call({ ...CALL, tool_use_id: 'toolu_second' }) - expect(await footerLine($, w)).toBe('lich: 2 workers') - - await w.clock.advance(5000) - expect(w.argvs.at(-1)).toEqual(['/opt/lich/bin/lich', 'sessions', '--json']) - expect(await footerLine($, w)).toBe('lich: 2 workers') - - const stopped = await $.tool.call({ tool: 'TaskStop', task_id: 'second-1a2b' }) - expect(stopped.result).toEqual(expect.objectContaining({ task_id: 'second-1a2b', task_type: 'local_agent' })) - expect(w.argvs.at(-1)).toEqual(['/opt/lich/bin/lich', 'close', 'second-1a2b']) - expect(await footerLine($, w)).toBe('lich: 1 worker') - - await w.clock.advance(5000) - expect(await footerLine($, w)).toBe(undefined) - await $.tool.call({ tool: 'TaskStop', task_id: 'a712043a56e1aafb1' }) expect(w.native).toEqual(['TaskStop a712043a56e1aafb1']) + expect(await footerLine($, w)).toBe(undefined) }) diff --git a/tests/agent-cards.test.mjs b/tests/agent-cards.test.mjs index 6475e50..94d4197 100644 --- a/tests/agent-cards.test.mjs +++ b/tests/agent-cards.test.mjs @@ -44,6 +44,7 @@ const ISOLATED = { ...AGENT, isolation: 'worktree' } // A worker without isolation opens in the asking session's directory. const SHARED = { ...OPENED, label: 'claude-2', name: 'claude-2-9f8e', path: '/w' } const NATIVE = { result: 'the native agent ran' } +const REJECTED = { result: "Error: The user doesn't want to proceed with this tool use.", isError: true } const report = (status, answer = '') => ({ ticket: TICKET, target: BRANCH, status, answer }) const opened = (status, answer) => ({ ...OPENED, delivery: report(status, answer) }) @@ -117,13 +118,34 @@ function load({ env = ENV, runs = [], times = [1000, 4000] } = {}) { }, start: (isInteractive = true) => hooks.get('session.start')($, { cwd: '/w', surface: isInteractive ? 'terminal' : null, isInteractive }, async (e) => e), - /** Raises one Agent call; `passed` is what reached the native agent. */ - async call(e = AGENT, { plugin = 'engine', signal = new AbortController().signal } = {}) { + /** + * Raises one tool call; `passed` is what reached the native tool. Beneath + * an Agent call's `next` the engine decides its permission and, only once + * `permission` is allowed, raises `agent.spawn` before the native agent + * starts, as measured on Claude Code 2.1.296. + */ + async call(e = AGENT, { plugin = 'engine', signal = new AbortController().signal, permission = 'allow' } = {}) { const passed = [] + const spawn = async (forwarded) => { + const input = { tool_use_id: forwarded.tool_use_id, prompt: forwarded.prompt, description: forwarded.description } + const spawnNext = Object.assign( + async () => { + passed.push(forwarded) + return { model: 'claude-opus-5-5' } + }, + { signal }, + ) + const unhooked = (_$, spawning, nextSpawn) => nextSpawn(spawning) + const spawned = await (hooks.get('agent.spawn') ?? unhooked)($, input, spawnNext) + return spawned.deny === undefined ? NATIVE : { result: `Error: ${spawned.deny}`, text: spawned.deny, isError: true } + } const next = Object.assign( async (forwarded) => { - passed.push(forwarded) - return NATIVE + if (forwarded.tool !== 'Agent') { + passed.push(forwarded) + return NATIVE + } + return permission === 'allow' ? spawn(forwarded) : REJECTED }, { origin: { plugin, tier: plugin === 'engine' ? 'core' : 'user' }, signal }, ) @@ -489,6 +511,30 @@ test('an open the user interrupted is denied, never run natively', async () => { }) }) +// ------------------------------------------------------------ permission -- + +// Claude Code refuses the call beneath the mod's `next` (a deny rule, the +// mode, the user's "no" in the dialog, a PreToolUse hook) and never raises +// `agent.spawn`: the refusal stands and lich opens nothing. +test('a call Claude Code refused opens no lich session', async () => { + for (const e of [AGENT, ISOLATED]) { + const runs = e === ISOLATED ? [onBranch('main')] : [] + const { mod, answer, passed } = await delegate(runs, e, { permission: 'reject' }) + assert.equal(answer, REJECTED) + assert.deepEqual(passed, []) + assert.equal(mod.ran.some(({ argv }) => argv[0] === LICH_BIN), false) + assert.deepEqual(mod.statuses, []) + } +}) + +test('a refused call leaves the next allowed one to open its session', async () => { + const mod = load({ runs: [exits(2, openedHere('pending'))] }) + await mod.start() + await mod.call(AGENT, { permission: 'reject' }) + const { answer } = await mod.call(AGENT) + assert.deepEqual(answer, backgrounded({ worker: SHARED })) +}) + // ------------------------------------------------ never native after delegation -- test('a status this mod does not know is denied, never run natively', async () => { diff --git a/tests/prompt-text.test.mjs b/tests/prompt-text.test.mjs index 7083cd8..b9f2f3d 100644 --- a/tests/prompt-text.test.mjs +++ b/tests/prompt-text.test.mjs @@ -76,6 +76,17 @@ function hooksOf(register) { const pass = (value) => async () => value const exits = (code, stdout = '', stderr = '') => ({ exitCode: code, stdout, stderr }) +// Beneath an Agent call it allowed, Claude Code raises `agent.spawn` before the +// native agent starts (measured on 2.1.296), which is where the mod opens lich. +const allowedAgent = (hooks, $) => { + const signal = new AbortController().signal + const spawn = async (e) => { + const spawned = await hooks.get('agent.spawn')($, { tool_use_id: e.tool_use_id }, Object.assign(pass({ model: 'm' }), { signal })) + return spawned.deny === undefined ? { result: 'native' } : { result: `Error: ${spawned.deny}`, isError: true } + } + return Object.assign(spawn, { origin: { plugin: 'engine' }, signal }) +} + test('agent-cards: the background note, the completed addendum and the denies speak pt-BR', async () => { const OPENED = { name: 'w-1', label: 'w', path: '/wt/w', id: 'x' } const env = { ...PT, LICH_BIN: '/lich', LICH_SESSION_ID: 'lich-1' } @@ -89,9 +100,8 @@ test('agent-cards: the background note, the completed addendum and the denies sp ui: { toast() {}, invalidate() {} }, } await hooks.get('session.start')($, { isInteractive: true }, async (x) => x) - const next = Object.assign(pass({ result: 'native' }), { origin: { plugin: 'engine' }, signal: new AbortController().signal }) const e = { tool: 'Agent', tool_use_id: 't1', description: 'd', prompt: 'p', subagent_type: 'general-purpose' } - return hooks.get('tool.call Agent')($, e, next) + return hooks.get('tool.call Agent')($, e, allowedAgent(hooks, $)) } const background = await call('pending') @@ -120,7 +130,7 @@ test('agent-cards: TaskStop failure speaks pt-BR, the success message stays Clau } await hooks.get('session.start')($, { isInteractive: true }, async (x) => x) const next = Object.assign(pass({ result: 'native' }), { origin: { plugin: 'engine' }, signal: new AbortController().signal }) - await hooks.get('tool.call Agent')($, { tool: 'Agent', tool_use_id: 't1', description: 'd', prompt: 'p', subagent_type: 'general-purpose' }, next) + await hooks.get('tool.call Agent')($, { tool: 'Agent', tool_use_id: 't1', description: 'd', prompt: 'p', subagent_type: 'general-purpose' }, allowedAgent(hooks, $)) const failed = await hooks.get('tool.call TaskStop')($, { tool: 'TaskStop', task_id: 'w-1' }, next) assert.equal(failed.deny, 'o lich não conseguiu parar "w": boom') @@ -287,7 +297,7 @@ for (const [tag, want] of Object.entries(EXPECTED)) { await hooks.get('session.start')($, { isInteractive: true }, async (x) => x) const next = Object.assign(pass({ result: 'native' }), { origin: { plugin: 'engine' }, signal: new AbortController().signal }) const e = { tool: 'Agent', tool_use_id: 't1', description: 'd', prompt: 'p', subagent_type: 'general-purpose' } - const first = await hooks.get('tool.call Agent')($, e, next) + const first = await hooks.get('tool.call Agent')($, e, allowedAgent(hooks, $)) if (tool === 'Agent') return first return hooks.get('tool.call TaskStop')($, { tool: 'TaskStop', task_id: 'w-1' }, next) }