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) }