mirror of
https://github.com/qwibitai/nanoclaw.git
synced 2026-07-15 19:06:18 +08:00
Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 9d08ecd57d |
@@ -4,7 +4,6 @@ All notable changes to NanoClaw will be documented in this file.
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
- **Claude tool allowlist reconciled with the pinned CLI, plus a tool-surface drift guard.** `TOOL_ALLOWLIST` in the agent runner named five tools that don't exist on claude-code 2.1.197 (`Task` — renamed `Agent` upstream — plus `TodoWrite`, `TeamCreate`, `TeamDelete`, `ToolSearch`); the phantom entries are removed and the comment corrected: `allowedTools` is a permission auto-approve list, **not an availability filter** — interleaved wire captures show no allowlist effect on the stable tool surface, and `bypassPermissions` moots its permission role. (The surface itself shows per-query variance in conditional tools on this CLI pin — the drift fixture records the flicker set separately so regeneration cannot flake on it — so neither `allowedTools` nor `disallowedTools` alone should be relied on to shape what the model sees — the runner's PreToolUse hook is the deterministic block.) No wire-visible behavior change. A new wire-captured fixture (`container/agent-runner/src/providers/sdk-tools-baseline.json`) and `claude.tools.test.ts` now fail on any future claude-code CLI **or SDK** pin bump until the fixture is regenerated with `dump-sdk-tools.ts` and the lists re-verified.
|
||||
- [BREAKING] **`whatsapp-formatting` and `slack-formatting` container skills moved from trunk to the `channels` branch.** They now install with their channel — `/add-whatsapp` / `/add-slack` and the setup installers copy them in — so installs without those channels stop carrying channel-specific formatting instructions in every agent's context. **Migration — only if this install has the channel wired** (check `src/channels/whatsapp.ts` / `src/channels/slack.ts`): updating removes both skills from the working tree — WhatsApp agents lose the formatting fragment from their composed CLAUDE.md on next spawn, Slack agents lose the mrkdwn skill from `~/.claude/skills`. Re-run the matching skill, `/add-whatsapp` or `/add-slack` (idempotent), to restore. Installs without the channel need nothing — do NOT run the add-skill just in case; it installs the full channel adapter.
|
||||
- **Pre-task script failures back their series off instead of spinning.** A `--script` that errors lands the occurrence as a failed run (`script-skip:error` ack → `failed` status); recurrence reads the series' trailing failed streak and re-arms at `max(cron next, now + 2·2^(n−1) min, cap 60)`; after 8 consecutive failures the series is auto-paused with a host-written note in its run log (`ncl tasks resume` revives it). A deliberate `wakeAgent:false` gate is a normal run and never backs off. Also fixed: an explicitly-addressed `<message to>` in a task fire's final text now delivers as a deliberate send (previously suppressed as a turn-reply echo → zero delivery when the agent skipped the MCP tool); identical echoes of an MCP send are dropped in the runner, where the duplication originates.
|
||||
- [BREAKING] **Scheduled tasks moved from MCP tools to `ncl tasks`.** The six scheduling MCP tools are no longer exposed to agent containers; agents and operators manage tasks with `ncl tasks list/get/create/update/cancel/pause/resume/delete`. New tasks run from a per-agent-group system session rather than waking the chat session that created them, and task writes are not approval-gated inside the owning group. **Migration:** [docs/ncl-tasks-migration.md](docs/ncl-tasks-migration.md).
|
||||
|
||||
@@ -109,7 +109,7 @@ ncl help
|
||||
| tasks | list, get, create, update, cancel, pause, resume, delete, run, append-log | Scheduled tasks for an agent group |
|
||||
| user-dms | list | Cold-DM cache (read-only) |
|
||||
| dropped-messages | list | Messages from unregistered senders (read-only) |
|
||||
| approvals | list, get | Pending approval requests (read-only) |
|
||||
| approvals | list, get, approve, reject, reject-with-reason | Pending approval requests; `approve`/`reject`/`reject-with-reason` resolve one from the host CLI (operator-only, `--as-user <approver>`) via the same auth + resolution as a channel button |
|
||||
|
||||
Key files: `src/cli/dispatch.ts` (dispatcher + approval handler), `src/cli/crud.ts` (generic CRUD registration), `src/cli/resources/` (per-resource definitions).
|
||||
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
"": {
|
||||
"name": "nanoclaw-agent-runner",
|
||||
"dependencies": {
|
||||
"@anthropic-ai/claude-agent-sdk": "0.3.197",
|
||||
"@anthropic-ai/claude-agent-sdk": "^0.3.197",
|
||||
"@anthropic-ai/sdk": "^0.108.0",
|
||||
"@modelcontextprotocol/sdk": "^1.29.0",
|
||||
"cron-parser": "^5.0.0",
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
"test": "bun test"
|
||||
},
|
||||
"dependencies": {
|
||||
"@anthropic-ai/claude-agent-sdk": "0.3.197",
|
||||
"@anthropic-ai/claude-agent-sdk": "^0.3.197",
|
||||
"@anthropic-ai/sdk": "^0.108.0",
|
||||
"@modelcontextprotocol/sdk": "^1.29.0",
|
||||
"cron-parser": "^5.0.0",
|
||||
|
||||
@@ -1,82 +0,0 @@
|
||||
/**
|
||||
* Drift guard for the harness tool surface. sdk-tools-baseline.json is a wire
|
||||
* capture of every tool the pinned CLI can offer under our configuration
|
||||
* (regenerate with dump-sdk-tools.ts — instructions in its header). These
|
||||
* tests catch upstream renames/removals when the claude-code pin moves:
|
||||
* bumping container/cli-tools.json fails the version assertion until the
|
||||
* fixture is regenerated and the lists below are re-verified.
|
||||
*/
|
||||
import fs from 'fs';
|
||||
|
||||
import { describe, expect, it } from 'bun:test';
|
||||
|
||||
import cliTools from '../../../cli-tools.json';
|
||||
import { SDK_DISALLOWED_TOOLS, TOOL_ALLOWLIST } from './claude.js';
|
||||
import baseline from './sdk-tools-baseline.json';
|
||||
|
||||
/**
|
||||
* Disallow entries that do NOT exist on the pinned CLI in headless SDK mode
|
||||
* (wire-verified: never offered, in both string and streaming input modes).
|
||||
* Kept in SDK_DISALLOWED_TOOLS as drift insurance — if an upstream version
|
||||
* starts offering one, the fixture regeneration surfaces it here and the
|
||||
* entry moves out of this set.
|
||||
*/
|
||||
const KNOWN_ABSENT_DISALLOWED = ['AskUserQuestion', 'EnterPlanMode', 'ExitPlanMode'];
|
||||
|
||||
const installedSdkVersion = (
|
||||
JSON.parse(
|
||||
fs.readFileSync(new URL('../../node_modules/@anthropic-ai/claude-agent-sdk/package.json', import.meta.url), 'utf8'),
|
||||
) as { version: string }
|
||||
).version;
|
||||
|
||||
// Membership checks run against stable ∪ variant: a tool that flickers on
|
||||
// this pin (see dump-sdk-tools.ts header) is still a real tool.
|
||||
const baselineTools = new Set<string>([...baseline.tools, ...baseline.variantTools]);
|
||||
|
||||
describe('sdk tool-surface drift guard', () => {
|
||||
it('fixture matches the pinned claude-code CLI version', () => {
|
||||
const pin = cliTools.find((t) => t.name === '@anthropic-ai/claude-code')?.version;
|
||||
expect(baseline.cliVersion).toBe(pin);
|
||||
});
|
||||
|
||||
it('fixture matches the installed Agent SDK version', () => {
|
||||
// The SDK is a caret-free pinned dep; a bump must be captured deliberately.
|
||||
expect(baseline.sdkVersion).toBe(installedSdkVersion);
|
||||
});
|
||||
|
||||
it('allowedTools has no surface effect: the stable cores match across interleaved rounds', () => {
|
||||
// `tools`/`toolsBare` are STABLE cores over N interleaved captures per
|
||||
// mode; per-query flicker (see the dump-sdk-tools.ts header) lands in
|
||||
// `variantTools` instead of here. Passing TOOL_ALLOWLIST has never been
|
||||
// observed to filter or promote a stable tool under bypassPermissions —
|
||||
// if a future CLI makes allowedTools an availability filter, the cores
|
||||
// diverge in every round and this fails deterministically.
|
||||
expect([...baseline.tools].sort()).toEqual([...baseline.toolsBare].sort());
|
||||
});
|
||||
|
||||
it('every allowlist entry names a real tool on this surface', () => {
|
||||
for (const name of TOOL_ALLOWLIST) {
|
||||
expect(baselineTools.has(name), `allowlist entry '${name}' not in captured surface`).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('every disallow entry is either a real tool or documented drift insurance', () => {
|
||||
for (const name of SDK_DISALLOWED_TOOLS) {
|
||||
const real = baselineTools.has(name);
|
||||
const insurance = KNOWN_ABSENT_DISALLOWED.includes(name);
|
||||
expect(
|
||||
real || insurance,
|
||||
`disallow entry '${name}' is neither on the surface nor in KNOWN_ABSENT_DISALLOWED`,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('drift-insurance entries are still absent from the surface', () => {
|
||||
for (const name of KNOWN_ABSENT_DISALLOWED) {
|
||||
expect(
|
||||
baselineTools.has(name),
|
||||
`'${name}' now exists on the surface — move it out of KNOWN_ABSENT_DISALLOWED and re-verify its disposition`,
|
||||
).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -24,7 +24,7 @@ function log(msg: string): void {
|
||||
// the question and blocks on the real reply.
|
||||
// - EnterPlanMode / ExitPlanMode / EnterWorktree / ExitWorktree: Claude
|
||||
// Code UI affordances; in a headless container they'd appear stuck.
|
||||
export const SDK_DISALLOWED_TOOLS = [
|
||||
const SDK_DISALLOWED_TOOLS = [
|
||||
'CronCreate',
|
||||
'CronDelete',
|
||||
'CronList',
|
||||
@@ -36,33 +36,30 @@ export const SDK_DISALLOWED_TOOLS = [
|
||||
'ExitWorktree',
|
||||
];
|
||||
|
||||
// Pre-approved tool set for NanoClaw agent containers. `allowedTools` is a
|
||||
// permission auto-approve list, NOT an availability filter: interleaved wire
|
||||
// captures show no allowlist effect on the stable tool surface (the fixture's
|
||||
// stable cores are equal; claude.tools.test.ts asserts it), and its permission
|
||||
// role is moot under this runner's `bypassPermissions`. The surface itself has
|
||||
// per-query variance in conditional tools (see dump-sdk-tools.ts header), so
|
||||
// never rely on this list — or on `disallowedTools` alone — to shape what the
|
||||
// model sees. Kept as the accurate pre-approval set for a
|
||||
// hypothetical non-bypass mode, and because the per-server `mcpAllowPattern`
|
||||
// entries derived at the call site are retained (MCP invocation-gating under
|
||||
// non-bypass modes is unverified). Exported for the fixture regenerator and
|
||||
// tests.
|
||||
export const TOOL_ALLOWLIST = [
|
||||
'Agent',
|
||||
// Tool allowlist for NanoClaw agent containers. MCP-tool entries are derived
|
||||
// at the call site from the registered `mcpServers` map so that any server
|
||||
// added via `add_mcp_server` (or wired in container.json directly) is
|
||||
// reachable to the agent — without this, the SDK's allowedTools filter
|
||||
// silently drops every MCP namespace not listed here.
|
||||
const TOOL_ALLOWLIST = [
|
||||
'Bash',
|
||||
'Read',
|
||||
'Write',
|
||||
'Edit',
|
||||
'Glob',
|
||||
'Grep',
|
||||
'NotebookEdit',
|
||||
'Read',
|
||||
'SendMessage',
|
||||
'Skill',
|
||||
'WebSearch',
|
||||
'WebFetch',
|
||||
'Task',
|
||||
'TaskOutput',
|
||||
'TaskStop',
|
||||
'WebFetch',
|
||||
'WebSearch',
|
||||
'Write',
|
||||
'TeamCreate',
|
||||
'TeamDelete',
|
||||
'SendMessage',
|
||||
'TodoWrite',
|
||||
'ToolSearch',
|
||||
'Skill',
|
||||
'NotebookEdit',
|
||||
];
|
||||
|
||||
// MCP server names are sanitized by the SDK when forming tool prefixes:
|
||||
|
||||
@@ -1,182 +0,0 @@
|
||||
/**
|
||||
* Regenerates sdk-tools-baseline.json — the bare SDK tool-surface fixture
|
||||
* asserted by claude.tools.test.ts.
|
||||
*
|
||||
* Must run INSIDE the agent container image (the pinned CLI binary only
|
||||
* exists there). From the repo root:
|
||||
*
|
||||
* docker run --rm --network none \
|
||||
* -v "$PWD/container/agent-runner/src":/app/src:ro \
|
||||
* --entrypoint bun <nanoclaw-agent image> /app/src/providers/dump-sdk-tools.ts \
|
||||
* > container/agent-runner/src/providers/sdk-tools-baseline.json
|
||||
*
|
||||
* MEASURED NONDETERMINISM on the pinned CLI (2.1.197): across query
|
||||
* invocations with byte-identical options — even inside one process — (a)
|
||||
* conditional tools (Glob, Grep) flicker in and out of the surface, and (b)
|
||||
* `disallowedTools` sometimes strips flag-gated tools (Workflow, DesignSync,
|
||||
* EnterWorktree) and sometimes ignores them entirely; each single query is
|
||||
* internally coherent. Consequences: schema-stripping via disallowedTools is
|
||||
* BEST-EFFORT — the deterministic enforcement is the runner's PreToolUse
|
||||
* hook, which blocks the invocation regardless of whether the schema shipped.
|
||||
*
|
||||
* Because the variance is per-query, a single capture pair cannot isolate the
|
||||
* allowlist's effect. So: ROUNDS interleaved captures per mode (allowlist /
|
||||
* bare), then split each mode into its STABLE core (tools present in every
|
||||
* capture of that mode) and the VARIANT set (present in some but not all
|
||||
* captures across both modes):
|
||||
* - `tools` : stable core WITH the production TOOL_ALLOWLIST.
|
||||
* - `toolsBare` : stable core with no allowedTools.
|
||||
* - `variantTools` : the flicker set, recorded for membership checks.
|
||||
* - `toolsDisallowProbe` : one capture with disallowedTools=[DISALLOW_PROBE]
|
||||
* — a diagnostic of which disallow behavior this regen observed (no test
|
||||
* asserts stripping; a fixed assertion on a nondeterministic mechanism
|
||||
* would be a coin-flip).
|
||||
* The drift test asserts tools === toolsBare on the STABLE cores: no allowlist
|
||||
* effect has ever been observed there, so the list is permission-layer only
|
||||
* (moot under bypassPermissions). If a CLI/SDK bump makes the allowlist shape
|
||||
* the surface, the stable cores diverge across every round by construction
|
||||
* and the assertion fails deterministically instead of flaking.
|
||||
*
|
||||
* Agent-teams is enabled via a temp settings.json (wire-verified: settings
|
||||
* env strictly beats SDK options env).
|
||||
*
|
||||
* Zero API traffic: ANTHROPIC_BASE_URL points at an in-process stub answering
|
||||
* 401; the full tools array rides on the first /v1/messages request, captured
|
||||
* before the run dies on the auth error. The fixture records WIRE tool names
|
||||
* (the SDK init message reports legacy aliases, e.g. `Task` for wire `Agent` —
|
||||
* do not swap this to an init capture).
|
||||
*/
|
||||
import { execFileSync } from 'child_process';
|
||||
import fs from 'fs';
|
||||
|
||||
import { query } from '@anthropic-ai/claude-agent-sdk';
|
||||
|
||||
import { TOOL_ALLOWLIST } from './claude.js';
|
||||
|
||||
let requests: string[] = [];
|
||||
let captured: (() => void) | null = null;
|
||||
|
||||
const server = Bun.serve({
|
||||
hostname: '127.0.0.1',
|
||||
port: 0,
|
||||
async fetch(req) {
|
||||
const url = new URL(req.url);
|
||||
const body = await req.text();
|
||||
if (url.pathname.includes('/messages')) {
|
||||
requests.push(body);
|
||||
captured?.();
|
||||
}
|
||||
return new Response(
|
||||
JSON.stringify({ type: 'error', error: { type: 'authentication_error', message: 'fixture-capture-stub' } }),
|
||||
{ status: 401, headers: { 'content-type': 'application/json' } },
|
||||
);
|
||||
},
|
||||
});
|
||||
|
||||
const HOME = '/tmp/dump-sdk-tools-home';
|
||||
const CWD = '/tmp/dump-sdk-tools-ws';
|
||||
fs.mkdirSync(`${HOME}/.claude`, { recursive: true });
|
||||
fs.mkdirSync(CWD, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
`${HOME}/.claude/settings.json`,
|
||||
JSON.stringify({ env: { CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS: '1' } }, null, 2),
|
||||
);
|
||||
|
||||
/**
|
||||
* Diagnostic probe for disallowedTools: a flag-gated tool whose stripping is
|
||||
* nondeterministic on the current pin (see header). The fixture records what
|
||||
* this regen run observed; future pins can be compared against it.
|
||||
*/
|
||||
export const DISALLOW_PROBE = 'Workflow';
|
||||
|
||||
/** Run one capture and return the sorted wire tool names. */
|
||||
async function capture(opts?: { allowedTools?: string[]; disallowedTools?: string[] }): Promise<string[]> {
|
||||
requests = [];
|
||||
const firstRequest = new Promise<void>((resolve) => {
|
||||
captured = resolve;
|
||||
});
|
||||
const q = query({
|
||||
prompt: 'fixture capture: reply with one word',
|
||||
options: {
|
||||
cwd: CWD,
|
||||
pathToClaudeCodeExecutable: '/pnpm/claude',
|
||||
systemPrompt: { type: 'preset' as const, preset: 'claude_code' as const },
|
||||
env: {
|
||||
...process.env,
|
||||
HOME,
|
||||
ANTHROPIC_BASE_URL: `http://127.0.0.1:${server.port}`,
|
||||
ANTHROPIC_API_KEY: 'fixture-dummy-key',
|
||||
ANTHROPIC_AUTH_TOKEN: undefined,
|
||||
},
|
||||
permissionMode: 'bypassPermissions',
|
||||
allowDangerouslySkipPermissions: true,
|
||||
settingSources: ['user'],
|
||||
...(opts?.allowedTools ? { allowedTools: opts.allowedTools } : {}),
|
||||
...(opts?.disallowedTools ? { disallowedTools: opts.disallowedTools } : {}),
|
||||
},
|
||||
});
|
||||
void (async () => {
|
||||
try {
|
||||
for await (const _m of q) {
|
||||
/* drain until the auth error kills the run */
|
||||
}
|
||||
} catch {
|
||||
/* expected: 401 from the stub */
|
||||
}
|
||||
})();
|
||||
await Promise.race([firstRequest, Bun.sleep(75_000)]);
|
||||
await Bun.sleep(1_500); // let retries land so we can pick the largest body
|
||||
if (requests.length === 0) {
|
||||
console.error('[dump-sdk-tools] no /v1/messages request captured');
|
||||
process.exit(1);
|
||||
}
|
||||
const biggest = requests.reduce((a, b) => (b.length > a.length ? b : a));
|
||||
const parsed = JSON.parse(biggest) as { tools?: Array<{ name: string }> };
|
||||
return [...new Set((parsed.tools ?? []).map((t) => t.name))].sort();
|
||||
}
|
||||
|
||||
// Interleaved so a drifting gate state biases both modes equally (see header).
|
||||
const ROUNDS = 3;
|
||||
const withRuns: string[][] = [];
|
||||
const bareRuns: string[][] = [];
|
||||
for (let i = 0; i < ROUNDS; i++) {
|
||||
withRuns.push(await capture({ allowedTools: TOOL_ALLOWLIST }));
|
||||
bareRuns.push(await capture());
|
||||
}
|
||||
const toolsDisallowProbe = await capture({ disallowedTools: [DISALLOW_PROBE] });
|
||||
|
||||
const stable = (runs: string[][]): string[] => runs[0].filter((t) => runs.every((r) => r.includes(t))).sort();
|
||||
const union = (runs: string[][]): Set<string> => new Set(runs.flat());
|
||||
|
||||
const tools = stable(withRuns);
|
||||
const toolsBare = stable(bareRuns);
|
||||
const stableSet = new Set([...tools, ...toolsBare]);
|
||||
const variantTools = [...union([...withRuns, ...bareRuns])].filter((t) => !stableSet.has(t)).sort();
|
||||
|
||||
const cliVersionRaw = execFileSync('/pnpm/claude', ['--version'], { encoding: 'utf8' }).trim();
|
||||
const cliVersion = cliVersionRaw.split(/\s+/)[0];
|
||||
const sdkVersion = (
|
||||
JSON.parse(fs.readFileSync('/app/node_modules/@anthropic-ai/claude-agent-sdk/package.json', 'utf8')) as {
|
||||
version: string;
|
||||
}
|
||||
).version;
|
||||
|
||||
console.log(
|
||||
JSON.stringify(
|
||||
{
|
||||
cliVersion,
|
||||
sdkVersion,
|
||||
capturedAt: new Date().toISOString(),
|
||||
capture: `wire names; stable cores over ${ROUNDS} interleaved rounds per mode (tools=production allowlist, toolsBare=no allowedTools); variantTools=flicker set; toolsDisallowProbe=single capture with disallowedTools:[probe]; teams on`,
|
||||
rounds: ROUNDS,
|
||||
disallowProbe: DISALLOW_PROBE,
|
||||
tools,
|
||||
toolsBare,
|
||||
variantTools,
|
||||
toolsDisallowProbe,
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
);
|
||||
process.exit(0);
|
||||
@@ -1,96 +0,0 @@
|
||||
{
|
||||
"cliVersion": "2.1.197",
|
||||
"sdkVersion": "0.3.197",
|
||||
"capturedAt": "2026-07-12T11:43:58.342Z",
|
||||
"capture": "wire names; stable cores over 3 interleaved rounds per mode (tools=production allowlist, toolsBare=no allowedTools); variantTools=flicker set; toolsDisallowProbe=single capture with disallowedTools:[probe]; teams on",
|
||||
"rounds": 3,
|
||||
"disallowProbe": "Workflow",
|
||||
"tools": [
|
||||
"Agent",
|
||||
"Bash",
|
||||
"CronCreate",
|
||||
"CronDelete",
|
||||
"CronList",
|
||||
"DesignSync",
|
||||
"Edit",
|
||||
"EnterWorktree",
|
||||
"ExitWorktree",
|
||||
"Glob",
|
||||
"Grep",
|
||||
"NotebookEdit",
|
||||
"Read",
|
||||
"ReportFindings",
|
||||
"ScheduleWakeup",
|
||||
"SendMessage",
|
||||
"Skill",
|
||||
"TaskCreate",
|
||||
"TaskGet",
|
||||
"TaskList",
|
||||
"TaskOutput",
|
||||
"TaskStop",
|
||||
"TaskUpdate",
|
||||
"WebFetch",
|
||||
"WebSearch",
|
||||
"Workflow",
|
||||
"Write"
|
||||
],
|
||||
"toolsBare": [
|
||||
"Agent",
|
||||
"Bash",
|
||||
"CronCreate",
|
||||
"CronDelete",
|
||||
"CronList",
|
||||
"DesignSync",
|
||||
"Edit",
|
||||
"EnterWorktree",
|
||||
"ExitWorktree",
|
||||
"Glob",
|
||||
"Grep",
|
||||
"NotebookEdit",
|
||||
"Read",
|
||||
"ReportFindings",
|
||||
"ScheduleWakeup",
|
||||
"SendMessage",
|
||||
"Skill",
|
||||
"TaskCreate",
|
||||
"TaskGet",
|
||||
"TaskList",
|
||||
"TaskOutput",
|
||||
"TaskStop",
|
||||
"TaskUpdate",
|
||||
"WebFetch",
|
||||
"WebSearch",
|
||||
"Workflow",
|
||||
"Write"
|
||||
],
|
||||
"variantTools": [],
|
||||
"toolsDisallowProbe": [
|
||||
"Agent",
|
||||
"Bash",
|
||||
"CronCreate",
|
||||
"CronDelete",
|
||||
"CronList",
|
||||
"DesignSync",
|
||||
"Edit",
|
||||
"EnterWorktree",
|
||||
"ExitWorktree",
|
||||
"Glob",
|
||||
"Grep",
|
||||
"NotebookEdit",
|
||||
"Read",
|
||||
"ReportFindings",
|
||||
"ScheduleWakeup",
|
||||
"SendMessage",
|
||||
"Skill",
|
||||
"TaskCreate",
|
||||
"TaskGet",
|
||||
"TaskList",
|
||||
"TaskOutput",
|
||||
"TaskStop",
|
||||
"TaskUpdate",
|
||||
"WebFetch",
|
||||
"WebSearch",
|
||||
"Workflow",
|
||||
"Write"
|
||||
]
|
||||
}
|
||||
@@ -148,12 +148,9 @@ class ClaudeProvider implements AgentProvider {
|
||||
systemPrompt: input.systemContext?.instructions
|
||||
? { type: 'preset', preset: 'claude_code', append: input.systemContext.instructions }
|
||||
: undefined,
|
||||
// Permission auto-approve list (NOT an availability filter — wire
|
||||
// captures show no allowlist effect on the offered tool surface, and
|
||||
// bypassPermissions moots its permission role). Kept accurate for a
|
||||
// hypothetical non-bypass mode; the `mcp__<server>__*` patterns are
|
||||
// retained because MCP invocation-gating under non-bypass modes is
|
||||
// unverified. See sdk-tools-baseline.json + claude.tools.test.ts.
|
||||
// Base tools plus one `mcp__<server>__*` pattern per registered MCP
|
||||
// server — without the explicit MCP patterns the SDK's allowedTools
|
||||
// filter silently drops every MCP namespace.
|
||||
allowedTools: [...TOOL_ALLOWLIST, ...Object.keys(this.mcpServers).map(mcpAllowPattern)],
|
||||
disallowedTools: SDK_DISALLOWED_TOOLS,
|
||||
env: this.env,
|
||||
@@ -198,7 +195,7 @@ SDK message (so the idle timer stays honest) and maps recognized messages to `Pr
|
||||
**Claude-specific behavior inside the provider:**
|
||||
- `MessageStream` for async iterable input (push-based follow-ups)
|
||||
- Resume via the SDK `resume` option keyed on the stored `continuation` (the SDK session ID) — no separate resume-at cursor
|
||||
- `TOOL_ALLOWLIST` (Agent, Bash, Read, Write, Edit, Glob, Grep, WebSearch, WebFetch, Skill, …) — a permission auto-approve list, not an availability filter (moot under `bypassPermissions`; wire-verified, see `claude.tools.test.ts`) — extended at the call site with a `mcp__<server>__*` pattern per registered MCP server; `SDK_DISALLOWED_TOOLS` blocks SDK builtins that collide with NanoClaw's own scheduling/interaction model (CronCreate/Delete/List, ScheduleWakeup, AskUserQuestion, Enter/ExitPlanMode, Enter/ExitWorktree)
|
||||
- `TOOL_ALLOWLIST` (Bash, Read, Write, Edit, Glob, Grep, WebSearch, WebFetch, Task, Skill, …) extended at the call site with a `mcp__<server>__*` pattern per registered MCP server; `SDK_DISALLOWED_TOOLS` blocks SDK builtins that collide with NanoClaw's own scheduling/interaction model (CronCreate/Delete/List, ScheduleWakeup, AskUserQuestion, Enter/ExitPlanMode, Enter/ExitWorktree)
|
||||
- **PreToolUse hook** records the current tool + its declared timeout to `container_state` (so the host sweep widens its stuck tolerance while a long Bash runs) and, as defense-in-depth, blocks any `SDK_DISALLOWED_TOOLS` call that slips through. It does **not** sanitize bash env vars — there is no such hook.
|
||||
- **PostToolUse / PostToolUseFailure** hooks clear the in-flight tool
|
||||
- **PreCompact** hook archives the transcript to `conversations/` before compaction
|
||||
|
||||
@@ -0,0 +1,163 @@
|
||||
/**
|
||||
* Operator approval-resolution verbs (`ncl approvals approve|reject|
|
||||
* reject-with-reason`). Drives `resolveApprovalFromCli` directly with a
|
||||
* fabricated caller context + seeded DB state, asserting the host-only guard,
|
||||
* the authorization check, and that each decision routes to the real
|
||||
* resolution (handler run on approve; row consumed; reason relayed).
|
||||
*/
|
||||
import * as fs from 'fs';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { createAgentGroup } from '../../db/agent-groups.js';
|
||||
import { closeDb, initTestDb, runMigrations } from '../../db/index.js';
|
||||
import { createPendingApproval, createSession, getPendingApproval } from '../../db/sessions.js';
|
||||
import { registerApprovalHandler } from '../../modules/approvals/primitive.js';
|
||||
import { grantRole } from '../../modules/permissions/db/user-roles.js';
|
||||
import { upsertUser } from '../../modules/permissions/db/users.js';
|
||||
import { initSessionFolder } from '../../session-manager.js';
|
||||
import type { CallerContext } from '../frame.js';
|
||||
import { resolveApprovalFromCli } from './approvals-resolve.js';
|
||||
|
||||
vi.mock('../../container-runner.js', () => ({
|
||||
wakeContainer: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
vi.mock('../../config.js', async () => {
|
||||
const actual = await vi.importActual('../../config.js');
|
||||
return { ...actual, DATA_DIR: '/tmp/nanoclaw-test-approvals-resolve' };
|
||||
});
|
||||
|
||||
const TEST_DIR = '/tmp/nanoclaw-test-approvals-resolve';
|
||||
const HOST: CallerContext = { caller: 'host' };
|
||||
const AGENT: CallerContext = { caller: 'agent', sessionId: 'sess-1', agentGroupId: 'ag-1', messagingGroupId: 'mg-1' };
|
||||
|
||||
const now = () => new Date().toISOString();
|
||||
|
||||
function seedApproval(approvalId: string, action: string): void {
|
||||
createPendingApproval({
|
||||
approval_id: approvalId,
|
||||
session_id: 'sess-1',
|
||||
request_id: approvalId,
|
||||
action,
|
||||
payload: JSON.stringify({}),
|
||||
created_at: now(),
|
||||
title: 'Test approval',
|
||||
options_json: JSON.stringify([]),
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DIR, { recursive: true });
|
||||
const db = initTestDb();
|
||||
runMigrations(db);
|
||||
|
||||
createAgentGroup({ id: 'ag-1', name: 'Agent', folder: 'agent', agent_provider: null, created_at: now() });
|
||||
createSession({
|
||||
id: 'sess-1',
|
||||
agent_group_id: 'ag-1',
|
||||
messaging_group_id: null,
|
||||
thread_id: null,
|
||||
agent_provider: null,
|
||||
status: 'active',
|
||||
container_status: 'stopped',
|
||||
last_active: now(),
|
||||
created_at: now(),
|
||||
});
|
||||
initSessionFolder('ag-1', 'sess-1');
|
||||
|
||||
upsertUser({ id: 'cli:owner', kind: 'cli', display_name: 'Owner', created_at: now() });
|
||||
grantRole({ user_id: 'cli:owner', role: 'owner', agent_group_id: null, granted_by: null, granted_at: now() });
|
||||
upsertUser({ id: 'cli:stranger', kind: 'cli', display_name: 'Stranger', created_at: now() });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
closeDb();
|
||||
if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('resolveApprovalFromCli — operator approval resolution', () => {
|
||||
it('rejects an agent caller (self-approval guard) and leaves the row', async () => {
|
||||
seedApproval('appr-1', 'cli_command');
|
||||
await expect(resolveApprovalFromCli({ id: 'appr-1', as_user: 'cli:owner' }, AGENT, 'approve')).rejects.toThrow(
|
||||
/operator \(host\)/,
|
||||
);
|
||||
expect(getPendingApproval('appr-1')).toBeTruthy();
|
||||
});
|
||||
|
||||
it('errors on a missing approval', async () => {
|
||||
await expect(resolveApprovalFromCli({ id: 'nope', as_user: 'cli:owner' }, HOST, 'approve')).rejects.toThrow(
|
||||
/no pending approval/,
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects an unauthorized approver and leaves the row', async () => {
|
||||
seedApproval('appr-2', 'cli_command');
|
||||
await expect(resolveApprovalFromCli({ id: 'appr-2', as_user: 'cli:stranger' }, HOST, 'approve')).rejects.toThrow(
|
||||
/not authorized/,
|
||||
);
|
||||
expect(getPendingApproval('appr-2')).toBeTruthy();
|
||||
});
|
||||
|
||||
it('requires --id and --as-user', async () => {
|
||||
await expect(resolveApprovalFromCli({ as_user: 'cli:owner' }, HOST, 'approve')).rejects.toThrow(/--id is required/);
|
||||
await expect(resolveApprovalFromCli({ id: 'x' }, HOST, 'approve')).rejects.toThrow(/--as-user is required/);
|
||||
});
|
||||
|
||||
it('approve runs the registered action handler and consumes the row', async () => {
|
||||
const calls: string[] = [];
|
||||
registerApprovalHandler('cli_test_approve', async () => {
|
||||
calls.push('ran');
|
||||
});
|
||||
seedApproval('appr-3', 'cli_test_approve');
|
||||
|
||||
const res = await resolveApprovalFromCli({ id: 'appr-3', as_user: 'cli:owner' }, HOST, 'approve');
|
||||
|
||||
expect(calls).toEqual(['ran']);
|
||||
expect(res).toMatchObject({ approval_id: 'appr-3', action: 'cli_test_approve', resolved: 'approve' });
|
||||
expect(getPendingApproval('appr-3')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('reject consumes the row without running the action handler', async () => {
|
||||
const calls: string[] = [];
|
||||
registerApprovalHandler('cli_test_reject', async () => {
|
||||
calls.push('ran');
|
||||
});
|
||||
seedApproval('appr-4', 'cli_test_reject');
|
||||
|
||||
const res = await resolveApprovalFromCli({ id: 'appr-4', as_user: 'cli:owner' }, HOST, 'reject');
|
||||
|
||||
expect(calls).toEqual([]);
|
||||
expect(res.resolved).toBe('reject');
|
||||
expect(getPendingApproval('appr-4')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('reject-with-reason consumes the row and carries the trimmed reason', async () => {
|
||||
seedApproval('appr-5', 'cli_command');
|
||||
|
||||
const res = await resolveApprovalFromCli(
|
||||
{ id: 'appr-5', as_user: 'cli:owner', reason: ' not this quarter ' },
|
||||
HOST,
|
||||
'reject-with-reason',
|
||||
);
|
||||
|
||||
expect(res).toMatchObject({ approval_id: 'appr-5', resolved: 'reject', reason: 'not this quarter' });
|
||||
expect(getPendingApproval('appr-5')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('reject-with-reason requires a reason and leaves the row on omission', async () => {
|
||||
seedApproval('appr-6', 'cli_command');
|
||||
await expect(
|
||||
resolveApprovalFromCli({ id: 'appr-6', as_user: 'cli:owner' }, HOST, 'reject-with-reason'),
|
||||
).rejects.toThrow(/--reason is required/);
|
||||
expect(getPendingApproval('appr-6')).toBeTruthy();
|
||||
});
|
||||
|
||||
it('namespaces a bare --as-user against the cli channel', async () => {
|
||||
seedApproval('appr-7', 'cli_command');
|
||||
// 'owner' → 'cli:owner', the seeded owner
|
||||
const res = await resolveApprovalFromCli({ id: 'appr-7', as_user: 'owner' }, HOST, 'reject');
|
||||
expect(res.resolved).toBe('reject');
|
||||
expect(getPendingApproval('appr-7')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,88 @@
|
||||
/**
|
||||
* Operator-side approval resolution — the host-CLI equivalent of a channel
|
||||
* button click.
|
||||
*
|
||||
* `ncl approvals approve|reject|reject-with-reason` routes through the SAME
|
||||
* authorization (`isAuthorizedApprovalClick`) and resolution the click path
|
||||
* uses — `handleApprovalsResponse` for approve/plain-reject, `finalizeReject`
|
||||
* with an inline reason for reject-with-reason (the operator supplies the reason
|
||||
* directly, so the DM prompt-and-capture flow is skipped). Only the ingress
|
||||
* differs from a rendered card button.
|
||||
*
|
||||
* Host-only: an agent can never resolve an approval — that would let it
|
||||
* self-approve its own held action. `--as-user` is the approver identity the
|
||||
* operator resolves as, checked against the pending row exactly as a click's
|
||||
* user id would be. `--as-user` is asserted, not authenticated — which is sound
|
||||
* because the ncl socket is owner-only and a host caller already bypasses the
|
||||
* approval gate outright.
|
||||
*/
|
||||
import { getPendingApproval, getSession } from '../../db/sessions.js';
|
||||
import { finalizeReject } from '../../modules/approvals/finalize.js';
|
||||
import { handleApprovalsResponse, isAuthorizedApprovalClick } from '../../modules/approvals/response-handler.js';
|
||||
import type { ResponsePayload } from '../../response-registry.js';
|
||||
import type { CallerContext } from '../frame.js';
|
||||
|
||||
export type ResolveDecision = 'approve' | 'reject' | 'reject-with-reason';
|
||||
|
||||
export interface ResolveResult {
|
||||
approval_id: string;
|
||||
action: string;
|
||||
resolved: 'approve' | 'reject';
|
||||
reason?: string;
|
||||
}
|
||||
|
||||
/** Matches reason-capture's cap so an operator reason and a chat reason relay identically. */
|
||||
const MAX_REASON_LEN = 280;
|
||||
|
||||
function clampReason(raw: string): string {
|
||||
const trimmed = raw.trim();
|
||||
return trimmed.length <= MAX_REASON_LEN ? trimmed : trimmed.slice(0, MAX_REASON_LEN - 1) + '…';
|
||||
}
|
||||
|
||||
export async function resolveApprovalFromCli(
|
||||
args: Record<string, unknown>,
|
||||
ctx: CallerContext,
|
||||
decision: ResolveDecision,
|
||||
): Promise<ResolveResult> {
|
||||
if (ctx.caller !== 'host') {
|
||||
throw new Error('approvals can only be resolved by an operator (host), not an agent');
|
||||
}
|
||||
|
||||
const id = String(args.id ?? '').trim();
|
||||
const asUserRaw = String(args.as_user ?? '').trim();
|
||||
if (!id) throw new Error('--id is required');
|
||||
if (!asUserRaw) throw new Error('--as-user is required (the approver identity, e.g. cli:local)');
|
||||
const approver = asUserRaw.includes(':') ? asUserRaw : `cli:${asUserRaw}`;
|
||||
|
||||
const approval = getPendingApproval(id);
|
||||
if (!approval) throw new Error(`no pending approval: ${id}`);
|
||||
|
||||
const payload: ResponsePayload = {
|
||||
questionId: id,
|
||||
value: decision === 'approve' ? 'approve' : 'reject',
|
||||
userId: approver,
|
||||
channelType: 'cli',
|
||||
platformId: '',
|
||||
threadId: null,
|
||||
};
|
||||
if (!isAuthorizedApprovalClick(approval, payload)) {
|
||||
throw new Error(
|
||||
`${approver} is not authorized to resolve approval ${id} — must be its named approver, or an admin/owner of the requesting agent group`,
|
||||
);
|
||||
}
|
||||
|
||||
if (decision === 'reject-with-reason') {
|
||||
const reason = clampReason(String(args.reason ?? ''));
|
||||
if (!reason) throw new Error('--reason is required for reject-with-reason');
|
||||
if (!approval.session_id) throw new Error(`approval ${id} has no session to notify`);
|
||||
const session = getSession(approval.session_id);
|
||||
if (!session) throw new Error(`approval ${id} session not found`);
|
||||
await finalizeReject(approval, session, approver, reason);
|
||||
return { approval_id: id, action: approval.action, resolved: 'reject', reason };
|
||||
}
|
||||
|
||||
// approve / plain reject → the same path a card click drives (handler +
|
||||
// grant-carrying replay on approve; finalizeReject on plain reject).
|
||||
await handleApprovalsResponse(payload);
|
||||
return { approval_id: id, action: approval.action, resolved: decision === 'approve' ? 'approve' : 'reject' };
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
import { registerResource } from '../crud.js';
|
||||
import { resolveApprovalFromCli } from './approvals-resolve.js';
|
||||
|
||||
registerResource({
|
||||
name: 'approval',
|
||||
@@ -50,4 +51,74 @@ registerResource({
|
||||
{ name: 'options_json', type: 'json', description: 'Card button options as JSON array.' },
|
||||
],
|
||||
operations: { list: 'open', get: 'open' },
|
||||
// Operator resolution verbs — the host-CLI equivalent of a channel button
|
||||
// click. Host-only (enforced in resolveApprovalFromCli); an agent can never
|
||||
// resolve an approval. Same auth + resolution as a real click.
|
||||
customOperations: {
|
||||
approve: {
|
||||
access: 'open',
|
||||
description:
|
||||
'Approve a pending approval and run its action (operator only). Runs the same authorization and resolution as a channel button click.',
|
||||
args: [
|
||||
{ name: 'id', type: 'string', description: 'Approval id (from `ncl approvals list`).', required: true },
|
||||
{
|
||||
name: 'as_user',
|
||||
type: 'string',
|
||||
description:
|
||||
'Approver identity to resolve as — a namespaced user id (e.g. cli:local). Must be an authorized approver of the row.',
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
examples: ['ncl approvals approve --id appr-… --as-user cli:local'],
|
||||
handler: (args, ctx) => resolveApprovalFromCli(args, ctx, 'approve'),
|
||||
formatHuman: (data) => {
|
||||
const r = data as { action: string; approval_id: string };
|
||||
return `Approved ${r.action} (${r.approval_id}) and ran its action.`;
|
||||
},
|
||||
},
|
||||
reject: {
|
||||
access: 'open',
|
||||
description: 'Reject a pending approval (operator only).',
|
||||
args: [
|
||||
{ name: 'id', type: 'string', description: 'Approval id (from `ncl approvals list`).', required: true },
|
||||
{
|
||||
name: 'as_user',
|
||||
type: 'string',
|
||||
description: 'Approver identity to resolve as (e.g. cli:local). Must be authorized.',
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
examples: ['ncl approvals reject --id appr-… --as-user cli:local'],
|
||||
handler: (args, ctx) => resolveApprovalFromCli(args, ctx, 'reject'),
|
||||
formatHuman: (data) => {
|
||||
const r = data as { action: string; approval_id: string };
|
||||
return `Rejected ${r.action} (${r.approval_id}).`;
|
||||
},
|
||||
},
|
||||
'reject-with-reason': {
|
||||
access: 'open',
|
||||
description: 'Reject a pending approval and relay a one-line reason to the requesting agent (operator only).',
|
||||
args: [
|
||||
{ name: 'id', type: 'string', description: 'Approval id (from `ncl approvals list`).', required: true },
|
||||
{
|
||||
name: 'as_user',
|
||||
type: 'string',
|
||||
description: 'Approver identity to resolve as (e.g. cli:local). Must be authorized.',
|
||||
required: true,
|
||||
},
|
||||
{
|
||||
name: 'reason',
|
||||
type: 'string',
|
||||
description: 'One-line reason relayed to the agent (trimmed to 280 chars).',
|
||||
required: true,
|
||||
},
|
||||
],
|
||||
examples: ['ncl approvals reject-with-reason --id appr-… --as-user cli:local --reason "not this quarter"'],
|
||||
handler: (args, ctx) => resolveApprovalFromCli(args, ctx, 'reject-with-reason'),
|
||||
formatHuman: (data) => {
|
||||
const r = data as { action: string; approval_id: string };
|
||||
return `Rejected ${r.action} (${r.approval_id}) with reason.`;
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
@@ -131,7 +131,7 @@ function namespacedUserId(payload: ResponsePayload): string | null {
|
||||
return payload.userId.includes(':') ? payload.userId : `${payload.channelType}:${payload.userId}`;
|
||||
}
|
||||
|
||||
function isAuthorizedApprovalClick(approval: PendingApproval, payload: ResponsePayload): boolean {
|
||||
export function isAuthorizedApprovalClick(approval: PendingApproval, payload: ResponsePayload): boolean {
|
||||
const userId = namespacedUserId(payload);
|
||||
if (!userId) return false;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user