mirror of
https://github.com/nexu-io/open-design.git
synced 2026-09-20 06:15:06 +08:00
3f99a248b6
* refactor daemon project routes * refactor daemon bootstrap routes * refactor daemon run routes * type daemon run routes * fix(daemon): keep process probes in server Generated-By: looper 0.9.11 (runner=fixer, agent=codex) * feat(daemon): guard against agents looping on failing tool calls Autonomous chat agents had no bound on repeated tool failures. An agent asked to move a button into a titlebar element it believed was called `titlebar-left` (the real class was `tb-left`) wrote a shell assertion for `titlebar-left`, it failed, the agent concluded the edit had not landed, re-read, and retried the same wrong assumption 80+ times with ~19 errors. The daemon streamed every attempt without noticing. The BYOK proxy path already bounds its tool loop at MAX_BYOK_TOOL_LOOPS; the autonomous agents had no equivalent. Add a run-scoped tool-loop guard that observes the normalized tool_use/tool_result events every agent path emits, so one instance covers Claude, Codex/OpenCode, Copilot, and ACP. Two triggers: N consecutive errored results (reset by any success), and K repeats of the same failing action signature (the fixation case). Two tiers: a one-shot `tool_loop` warning surfaced in the chat, then a hard-ceiling halt that terminates the run. Mode via OD_TOOL_LOOP_GUARD (halt|warn|off), default halt. - packages/contracts: `tool_loop` SSE payload and a TOOL_LOOP_DETECTED error. - apps/daemon: pure tool-loop-guard.ts plus tests, wired into both agent event paths in server.ts with an abortForToolLoop mirroring abortForRoleMarker. - apps/web: render the warning as a status line in the transcript. * fix(daemon): scope tool-loop repeated-failure to lack of progress, key on full signature Addresses the two blockers in the #3375 review. 1. The repeated-failure counter was cumulative across the whole run, so a progressing workflow (rerun the same check after each successful edit) could reach the halt ceiling even though each retry tested newly-written code. A successful tool result now clears the per-signature tallies, not just the consecutive streak, so repeated-failure means "failures since the last success." I pulled the conversation that motivated the guard: the loop was 11 back-to-back failing Edits with no success between them, so it still trips. 2. The signature was truncated to 160 chars before being used as the dedup key, so two distinct long commands sharing a prefix pooled each other's failures. computeToolSignature now returns the full signature for keying; a new displayToolSignature truncates only the form shown in the tool_loop event. Adds regression tests: same command with intervening successful edits does not halt, and two >160-char inputs sharing a prefix do not collide. * fix(daemon): route every agent runtime through one tool-loop choke point The Copilot (copilot-stream-json) and ACP (acp-json-rpc) branches emitted agent events with a bare send('agent', ev), bypassing observeToolEventForLoop. So a Copilot or ACP run could repeat a failing tool call indefinitely while Claude / OpenCode / Qoder runs were stopped, which contradicted the "every agent path" claim. Add an emitAgentEvent choke point that sends the event and then feeds it to the guard, and route every stream handler through it (sendAgentEvent, the Claude callback, Copilot, ACP). observe runs after the send so a tool_loop warning or halt follows the result that triggered it. A bare send('agent', ...) now only carries guard-internal events (the tool_loop verdict, role-marker warnings), which the guard ignores. Adds a structured-streams regression test that feeds repeated failing Copilot tool executions through the real parser into the real guard and asserts it halts. * fix(daemon): persist tool_loop warnings so they survive transcript replay The tool_loop event was mapped by the live web translator but dropped by the daemon's persisted-event path (daemonAgentPayloadToPersistedAgentEvent had no tool_loop branch), and send() persists every SSE event before emitting it. So the warning was transient: after a reload or history replay it was gone, and in OD_TOOL_LOOP_GUARD=warn there is no terminal TOOL_LOOP_DETECTED error to fall back on, so the user lost the only record that a loop was detected. Mirror the live status mapping in the persist path so the warning is stored as a warning status and replays identically. Export the mapper and add a regression test covering the warn and halt cases plus a malformed-event drop. (PR #3375 review follow-up, non-blocking.) * fix(daemon): only clear the tool-loop tally on progress, not on read-only successes The previous fix cleared failCounts on every successful tool result, which created a false negative: a stuck agent that alternates a failing Bash/Edit with a successful Read (re-reading the file and retrying the same wrong assumption) reset the tally every round and never tripped. That is the exact "re-read, retry the same wrong assumption" shape from the motivating report. A success now clears the repeated-failure tally only when it is real progress: a successful mutating tool (Edit/Write/apply_patch/...), or the failing action itself finally succeeding. A read-only success (Read/Glob/LS/Grep/TodoWrite) still breaks the strictly-consecutive streak but keeps the per-signature tally, so a re-read-between-failures loop still trips. The earlier edit -> rerun-check -> fix workflow stays safe because the edit is a mutating success that clears the tally. Adds regression tests: a failing call interleaved with successful reads still halts, and a mutating success clears the tally while a read does not. * fix(daemon): treat state-changing shell commands as tool-loop progress isMutatingToolName was a name-only allowlist that excluded Bash, but agents change state through the shell (sed -i, pnpm install, git commit, a build). So a failing check like `pnpm test`, fixed by a successful Bash command, then rerun, carried the old `Bash pnpm test` failures forward until the halt ceiling stopped a progressing run. Replace the allowlist with a progress classifier: read-only tools (Read/Glob/LS/Grep/TodoWrite) and pure-inspection shell commands (cat/ls/grep, git status/diff, sed without -i, python3 -c) are not progress; a state-changing shell command (sed -i, mv, install, build, a redirect to a file, git add/commit) and any other tool success are. The classifier is conservative: anything not recognised as a pure inspection counts as progress, so it never halts a run that is fixing things, while a re-read-between-failures loop (Read/cat/grep) still trips. Adds regression tests: a failing check fixed by successful mutating Bash does not halt, plus read-only vs state-changing command classification. * fix(daemon): default the tool-loop guard to warn, make halt opt-in The repeated-failure trigger classifies whether a success is progress, and review kept surfacing edge cases where a misclassification could halt a run that was actually making progress. Defaulting to halt put that risk on every run. Default to warn instead: the guard surfaces a one-shot heads-up when a run looks stuck (which already solves the original silent-grinding problem, since the operator sees it and can stop the run), and halting is opt-in via OD_TOOL_LOOP_GUARD=halt. A heuristic guard should warn before it terminates someone's run. The halt path stays fully implemented and tested; only the default changes. resolveToolLoopMode and createToolLoopGuard now default to warn. The halt-mechanism tests opt into { mode: 'halt' } explicitly, plus a test that the default warns but never halts. * fix(daemon): count inline python/node scripts as tool-loop progress isReadOnlyShellCommand treated every `python -c` / `node -e` snippet as inspection-only, but an inline script can write files (`python3 -c "Path(...).write_text(...)"`). A fix loop that uses one between failing checks would carry the old failure count forward toward the halt ceiling. Drop the interpreter special case so inline scripts fall through to the state-change default (progress), matching how any other non-inspection shell command is treated. Adds a regression test: a failing check fixed by a successful inline python write does not halt. Updates the classification matrix. * fix(daemon): unwrap env-prefixed shell commands in tool-loop classifier isReadOnlyShellCommand only looked at each segment's head, and `env` was in the read-only binary set, so `env CI=1 sed -i ...` and `env NODE_ENV=production pnpm install` were classified as read-only inspections. A real fix run that way between failing checks never cleared the repeated-failure tally, so a healthy run could still halt at repeatCount 8, the false positive this heuristic exists to avoid. Unwrap a leading `env [flags] KEY=VALUE...` prefix before classifying the real subcommand, and drop `env` from the read-only set so a bare `env` unwraps to nothing and stays an inspection. Adds the classification cases and an integration test that alternates a failing check with a successful `env ... sed -i` / `env ... pnpm install` fix and asserts no halt. Addresses the #3375 review note on the shell progress heuristic. * fix(daemon): count mutating find and unparseable shell segments as tool-loop progress isReadOnlyShellCommand treated find as a pure inspection and silently skipped any segment whose head it could not parse, so find ... -delete, find ... -exec rm, a subshell like (sed -i ...) && cat x, and quoted heads all read as read-only. In OD_TOOL_LOOP_GUARD=halt those successful fixes never cleared the repeated-failure tally, so a run actually changing files could still hit haltRepeat and be terminated as a false positive. Move find to its own branch that stays read-only only without a mutating action (-delete/-exec/-execdir/-ok/-okdir/-fprint/-fprintf/-fls), and treat a non-empty segment with no parseable head as a state change. Empty segments from trailing separators still skip. Adds matrix rows for these Bash-fix paths. Addresses review feedback on #3375. * fix(daemon): match the whole find -fprint* family in the tool-loop classifier The mutating-action matcher listed -fprint and -fprintf but missed -fprint0, because fprint\b has no word boundary before the digit. A successful find ... -fprint0 out.bin writes a file but read as a read-only inspection, so in OD_TOOL_LOOP_GUARD=halt it left failCounts intact and could false-halt a run making progress. Use -fprint\w* to cover -fprint / -fprintf / -fprint0 in one matcher. Adds matrix rows for -fprint0 and -fprintf. Addresses review feedback on #3375. * fix(daemon): route the tool-loop halt through signalChild so it kills the process group abortForToolLoop used a bare child.kill('SIGTERM'), unlike abortForRoleMarker, cancel, and the inactivity watchdog, which all go through design.runs.signalChild. signalChild escalates to the whole process group when one exists, while child.kill only signals the direct child. In OD_TOOL_LOOP_GUARD=halt that left Bash/build grandchildren alive to keep mutating the workspace until the forced-shutdown timer fired. Route the halt through signalChild to match the sibling shutdown paths. Addresses review feedback on #3375. * fix(daemon): drop duplicate ACP first-token stamp * fix(daemon): cover run route deps in context guard Generated-By: looper 0.9.11 (runner=fixer, agent=codex) --------- Co-authored-by: portseif <chris@portseif.io>
504 lines
16 KiB
TypeScript
504 lines
16 KiB
TypeScript
// Contract test for the `od templates` CLI surface. Keeps the
|
|
// UI / API / CLI triple wired together (AGENTS.md "Capability exposure"):
|
|
// the CLI must drive the same /api/templates endpoints the web UI uses
|
|
// (NewProjectPanel / ExamplesTab), with --json support for headless
|
|
// agents that pipe through jq / xargs.
|
|
//
|
|
// We spin up a tiny stub HTTP server to capture requests instead of
|
|
// booting the full daemon: `execFile` is enough to prove that
|
|
// SUBCOMMAND_MAP routes `templates`, parseFlags accepts the documented
|
|
// flags, and the right HTTP call is emitted for each sub-verb.
|
|
|
|
import http from 'node:http';
|
|
import { execFile } from 'node:child_process';
|
|
import { fileURLToPath } from 'node:url';
|
|
import { dirname, resolve as pathResolve } from 'node:path';
|
|
import { promisify } from 'node:util';
|
|
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
|
|
|
const execFileP = promisify(execFile);
|
|
const __dirname = dirname(fileURLToPath(import.meta.url));
|
|
const DAEMON_ROOT = pathResolve(__dirname, '..');
|
|
const REPO_ROOT = pathResolve(__dirname, '../../..');
|
|
const CLI_SRC = pathResolve(__dirname, '../src/cli.ts');
|
|
const TSX_CLI = pathResolve(REPO_ROOT, 'node_modules/tsx/dist/cli.mjs');
|
|
|
|
interface CapturedRequest {
|
|
method: string;
|
|
url: string;
|
|
body: string;
|
|
}
|
|
|
|
interface StubServer {
|
|
baseUrl: string;
|
|
requests: CapturedRequest[];
|
|
setResponder: (
|
|
fn: (req: CapturedRequest) => { status: number; body: unknown } | null,
|
|
) => void;
|
|
close: () => Promise<void>;
|
|
}
|
|
|
|
async function startStubServer(): Promise<StubServer> {
|
|
const requests: CapturedRequest[] = [];
|
|
let responder:
|
|
| ((req: CapturedRequest) => { status: number; body: unknown } | null)
|
|
| null = null;
|
|
|
|
const server = http.createServer((req, res) => {
|
|
let raw = '';
|
|
req.on('data', (chunk) => {
|
|
raw += chunk;
|
|
});
|
|
req.on('end', () => {
|
|
const captured: CapturedRequest = {
|
|
method: req.method ?? '',
|
|
url: req.url ?? '',
|
|
body: raw,
|
|
};
|
|
requests.push(captured);
|
|
const response = responder?.(captured) ?? { status: 200, body: { ok: true } };
|
|
res.statusCode = response.status;
|
|
res.setHeader('content-type', 'application/json');
|
|
res.end(JSON.stringify(response.body));
|
|
});
|
|
});
|
|
|
|
await new Promise<void>((resolveListen) => server.listen(0, '127.0.0.1', resolveListen));
|
|
const addr = server.address();
|
|
if (!addr || typeof addr === 'string') throw new Error('stub server has no address');
|
|
const baseUrl = `http://127.0.0.1:${addr.port}`;
|
|
|
|
return {
|
|
baseUrl,
|
|
requests,
|
|
setResponder: (fn) => {
|
|
responder = fn;
|
|
},
|
|
close: () =>
|
|
new Promise<void>((resolveClose, rejectClose) => {
|
|
server.close((err) => (err ? rejectClose(err) : resolveClose()));
|
|
}),
|
|
};
|
|
}
|
|
|
|
async function runCli(
|
|
args: string[],
|
|
options: { env?: NodeJS.ProcessEnv } = {},
|
|
): Promise<{ stdout: string; stderr: string; code: number | null }> {
|
|
const env: NodeJS.ProcessEnv = {
|
|
...process.env,
|
|
...options.env,
|
|
};
|
|
delete env.NODE_OPTIONS;
|
|
try {
|
|
const { stdout, stderr } = await execFileP(
|
|
process.execPath,
|
|
[TSX_CLI, CLI_SRC, ...args],
|
|
{
|
|
cwd: DAEMON_ROOT,
|
|
env,
|
|
timeout: 15_000,
|
|
maxBuffer: 4 * 1024 * 1024,
|
|
},
|
|
);
|
|
return { stdout, stderr, code: 0 };
|
|
} catch (err) {
|
|
const failed = err as { stdout?: string; stderr?: string; code?: number | null };
|
|
return {
|
|
stdout: failed.stdout ?? '',
|
|
stderr: failed.stderr ?? '',
|
|
code: failed.code ?? 1,
|
|
};
|
|
}
|
|
}
|
|
|
|
// Reserve a port the OS just told us is free, then close it before
|
|
// any test runs so connections to it fail with ECONNREFUSED. The
|
|
// previous `port + 1` heuristic only assumed the stub's ephemeral port
|
|
// was free — `+1` could already be bound by an unrelated process on a
|
|
// shared CI host, which would silently route the "unreachable daemon"
|
|
// tests at a real server and let them pass (or fail) for the wrong
|
|
// reason (#2428 round-4 reviewer-correctness fix).
|
|
async function reserveAndReleasePort(): Promise<{ host: string; port: number }> {
|
|
return new Promise((resolveListen, rejectListen) => {
|
|
const probe = http.createServer();
|
|
probe.unref();
|
|
probe.once('error', rejectListen);
|
|
probe.listen(0, '127.0.0.1', () => {
|
|
const addr = probe.address();
|
|
if (!addr || typeof addr === 'string') {
|
|
probe.close();
|
|
rejectListen(new Error('probe server has no address'));
|
|
return;
|
|
}
|
|
const { port } = addr;
|
|
probe.close((err) => {
|
|
if (err) rejectListen(err);
|
|
else resolveListen({ host: '127.0.0.1', port });
|
|
});
|
|
});
|
|
});
|
|
}
|
|
|
|
describe('od templates CLI', () => {
|
|
let stub: StubServer;
|
|
let unreachableDaemonUrl: string;
|
|
|
|
beforeAll(async () => {
|
|
stub = await startStubServer();
|
|
const reserved = await reserveAndReleasePort();
|
|
unreachableDaemonUrl = `http://${reserved.host}:${reserved.port}`;
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await stub.close();
|
|
});
|
|
|
|
beforeEach(() => {
|
|
stub.requests.length = 0;
|
|
stub.setResponder(() => ({ status: 200, body: { ok: true } }));
|
|
});
|
|
|
|
afterEach(() => {
|
|
stub.setResponder(() => ({ status: 200, body: { ok: true } }));
|
|
});
|
|
|
|
it('prints usage on `od templates help` and exits 0', async () => {
|
|
const result = await runCli(['templates', 'help']);
|
|
expect(result.code).toBe(0);
|
|
expect(result.stdout).toMatch(/od templates/);
|
|
expect(result.stdout).toMatch(/save/);
|
|
expect(result.stdout).toMatch(/delete/);
|
|
expect(stub.requests).toHaveLength(0);
|
|
});
|
|
|
|
it('lists user-saved templates as `id\\tname` lines by default', async () => {
|
|
stub.setResponder((req) => {
|
|
if (req.method === 'GET' && req.url === '/api/templates') {
|
|
return {
|
|
status: 200,
|
|
body: {
|
|
templates: [
|
|
{ id: 't-1', name: 'Cards', sourceProjectId: 'proj-1' },
|
|
{ id: 't-2', name: 'Pricing', sourceProjectId: 'proj-2' },
|
|
],
|
|
},
|
|
};
|
|
}
|
|
return { status: 404, body: { error: 'unexpected' } };
|
|
});
|
|
|
|
const result = await runCli(['templates', 'list', '--daemon-url', stub.baseUrl]);
|
|
|
|
expect(result.code).toBe(0);
|
|
expect(stub.requests).toHaveLength(1);
|
|
expect(stub.requests[0]).toMatchObject({
|
|
method: 'GET',
|
|
url: '/api/templates',
|
|
});
|
|
expect(result.stdout).toContain('t-1\tCards');
|
|
expect(result.stdout).toContain('t-2\tPricing');
|
|
});
|
|
|
|
it('emits the raw GET /api/templates body under --json', async () => {
|
|
const payload = {
|
|
templates: [{ id: 't-1', name: 'Cards', sourceProjectId: 'proj-1', files: [] }],
|
|
};
|
|
stub.setResponder(() => ({ status: 200, body: payload }));
|
|
|
|
const result = await runCli([
|
|
'templates',
|
|
'list',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
'--json',
|
|
]);
|
|
|
|
expect(result.code).toBe(0);
|
|
expect(JSON.parse(result.stdout)).toEqual(payload);
|
|
});
|
|
|
|
it('POSTs /api/templates with name + sourceProjectId on `templates save`', async () => {
|
|
stub.setResponder((req) => {
|
|
if (req.method === 'POST' && req.url === '/api/templates') {
|
|
return {
|
|
status: 200,
|
|
body: { template: { id: 't-new', name: 'Cards', sourceProjectId: 'proj-1' } },
|
|
};
|
|
}
|
|
return { status: 404, body: { error: 'unexpected' } };
|
|
});
|
|
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'proj-1',
|
|
'--name',
|
|
'Cards',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
|
|
expect(result.code).toBe(0);
|
|
expect(stub.requests).toHaveLength(1);
|
|
const req = stub.requests[0]!;
|
|
expect(req.method).toBe('POST');
|
|
expect(req.url).toBe('/api/templates');
|
|
expect(JSON.parse(req.body)).toEqual({
|
|
name: 'Cards',
|
|
sourceProjectId: 'proj-1',
|
|
});
|
|
expect(result.stdout).toContain('t-new');
|
|
});
|
|
|
|
it('forwards an optional --description into the POST body', async () => {
|
|
stub.setResponder(() => ({
|
|
status: 200,
|
|
body: { template: { id: 't-2', name: 'Pricing', description: 'About pricing' } },
|
|
}));
|
|
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'proj-9',
|
|
'--name',
|
|
'Pricing',
|
|
'--description',
|
|
'About pricing',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
|
|
expect(result.code).toBe(0);
|
|
const req = stub.requests[0]!;
|
|
expect(JSON.parse(req.body)).toEqual({
|
|
name: 'Pricing',
|
|
description: 'About pricing',
|
|
sourceProjectId: 'proj-9',
|
|
});
|
|
});
|
|
|
|
it('exits 2 and emits no HTTP request when `templates save` lacks --name', async () => {
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'proj-1',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
|
|
expect(result.code).toBe(2);
|
|
expect(stub.requests).toHaveLength(0);
|
|
expect(result.stderr).toMatch(/--name/);
|
|
});
|
|
|
|
it('exits 2 and emits no HTTP request when `templates save` lacks a projectId', async () => {
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'--name',
|
|
'Cards',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
|
|
expect(result.code).toBe(2);
|
|
expect(stub.requests).toHaveLength(0);
|
|
expect(result.stderr).toMatch(/projectId/i);
|
|
});
|
|
|
|
it('DELETEs /api/templates/:id on `templates delete <id>`', async () => {
|
|
stub.setResponder((req) => {
|
|
if (req.method === 'DELETE' && req.url === '/api/templates/t-1') {
|
|
return { status: 200, body: { ok: true } };
|
|
}
|
|
return { status: 404, body: { error: 'unexpected' } };
|
|
});
|
|
|
|
const result = await runCli([
|
|
'templates',
|
|
'delete',
|
|
't-1',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
|
|
expect(result.code).toBe(0);
|
|
expect(stub.requests).toHaveLength(1);
|
|
expect(stub.requests[0]).toMatchObject({
|
|
method: 'DELETE',
|
|
url: '/api/templates/t-1',
|
|
});
|
|
expect(result.stdout).toContain('t-1');
|
|
});
|
|
|
|
it('exits 2 and emits no HTTP request when `templates delete` lacks an id', async () => {
|
|
const result = await runCli([
|
|
'templates',
|
|
'delete',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
|
|
expect(result.code).toBe(2);
|
|
expect(stub.requests).toHaveLength(0);
|
|
expect(result.stderr).toMatch(/<id>/);
|
|
});
|
|
|
|
it('prints usage on `od templates` with no sub-verb and exits 2', async () => {
|
|
const result = await runCli(['templates']);
|
|
expect(result.code).toBe(2);
|
|
expect(result.stdout).toMatch(/od templates/);
|
|
});
|
|
|
|
it('exits non-zero on unknown sub-verb', async () => {
|
|
const result = await runCli([
|
|
'templates',
|
|
'frobnicate',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
expect(result.code).not.toBe(0);
|
|
expect(result.stderr).toMatch(/unknown subcommand/i);
|
|
});
|
|
|
|
// Reviewer-correctness fix from #2428: when the daemon isn't
|
|
// running, every sub-verb must surface the same clean
|
|
// "failed to reach daemon at <url>: <code>" error that the rest of
|
|
// the CLI emits via `surfaceFetchError`, not a raw
|
|
// `TypeError: fetch failed`. The CLI is pointed at a port the OS
|
|
// just told us was free and which we immediately released — see
|
|
// `reserveAndReleasePort` above — so the connection fails with
|
|
// ECONNREFUSED rather than silently routing to whatever happens to
|
|
// be bound nearby.
|
|
|
|
it('surfaces a clean fetch error from `templates list` when the daemon is unreachable', async () => {
|
|
const result = await runCli(['templates', 'list', '--daemon-url', unreachableDaemonUrl]);
|
|
expect(result.code).toBe(3);
|
|
expect(result.stderr).toMatch(/failed to reach daemon at http:\/\/127\.0\.0\.1:/);
|
|
expect(result.stderr).not.toMatch(/TypeError: fetch failed/);
|
|
});
|
|
|
|
it('surfaces a clean fetch error from `templates save` when the daemon is unreachable', async () => {
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'proj-1',
|
|
'--name',
|
|
'Cards',
|
|
'--daemon-url',
|
|
unreachableDaemonUrl,
|
|
]);
|
|
expect(result.code).toBe(3);
|
|
expect(result.stderr).toMatch(/failed to reach daemon at http:\/\/127\.0\.0\.1:/);
|
|
expect(result.stderr).not.toMatch(/TypeError: fetch failed/);
|
|
});
|
|
|
|
it('surfaces a clean fetch error from `templates delete` when the daemon is unreachable', async () => {
|
|
const result = await runCli([
|
|
'templates',
|
|
'delete',
|
|
't-1',
|
|
'--daemon-url',
|
|
unreachableDaemonUrl,
|
|
]);
|
|
expect(result.code).toBe(3);
|
|
expect(result.stderr).toMatch(/failed to reach daemon at http:\/\/127\.0\.0\.1:/);
|
|
expect(result.stderr).not.toMatch(/TypeError: fetch failed/);
|
|
});
|
|
|
|
// Reviewer-correctness fixes from #2428 round 3.
|
|
// (1) positional id must work even when shared flags come first.
|
|
// (2) `templates save` 404 / 400 must map to project-not-found /
|
|
// missing-input — not the default `daemon-not-running`, which
|
|
// would send agents down the wrong recovery branch.
|
|
it('accepts positional projectId after --daemon-url for `templates save` (flag-before-id)', async () => {
|
|
stub.setResponder(() => ({
|
|
status: 200,
|
|
body: { template: { id: 't-after-flag', name: 'Cards' } },
|
|
}));
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
'proj-1',
|
|
'--name',
|
|
'Cards',
|
|
]);
|
|
expect(result.code).toBe(0);
|
|
expect(stub.requests).toHaveLength(1);
|
|
expect(stub.requests[0]?.method).toBe('POST');
|
|
expect(JSON.parse(stub.requests[0]?.body ?? '{}')).toMatchObject({
|
|
sourceProjectId: 'proj-1',
|
|
name: 'Cards',
|
|
});
|
|
});
|
|
|
|
it('accepts positional id after --daemon-url for `templates delete` (flag-before-id)', async () => {
|
|
stub.setResponder(() => ({ status: 200, body: { ok: true } }));
|
|
const result = await runCli([
|
|
'templates',
|
|
'delete',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
't-after-flag',
|
|
]);
|
|
expect(result.code).toBe(0);
|
|
expect(stub.requests).toHaveLength(1);
|
|
expect(stub.requests[0]?.method).toBe('DELETE');
|
|
expect(stub.requests[0]?.url).toBe('/api/templates/t-after-flag');
|
|
});
|
|
|
|
it('reports project-not-found (not daemon-not-running) when `templates save` gets 404', async () => {
|
|
// POST /api/templates returns a flat `{ error: '<message>' }` body
|
|
// (routes/project/index.ts), so the stub mirrors that exact shape
|
|
// rather than the nested `{ error: { code, message } }` envelope.
|
|
// structuredHttpFailure normalises the flat string into the
|
|
// structured envelope, mapping the 404 to project-not-found via
|
|
// the fallback code passed at the call site and surfacing the
|
|
// daemon's literal message so headless callers don't lose the
|
|
// only diagnostic the route emitted.
|
|
stub.setResponder(() => ({
|
|
status: 404,
|
|
body: { error: 'source project not found' },
|
|
}));
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'missing-project',
|
|
'--name',
|
|
'Cards',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
expect(result.code).toBe(68);
|
|
const envelope = JSON.parse(result.stderr.trim());
|
|
expect(envelope.error.code).toBe('project-not-found');
|
|
expect(envelope.error.message).toBe('source project not found');
|
|
});
|
|
|
|
it('reports missing-input (not daemon-not-running) when `templates save` gets 400', async () => {
|
|
// 400 from POST /api/templates is also a flat `{ error: '<msg>' }`
|
|
// body (routes/project/index.ts). Same normalisation contract
|
|
// as the 404 case above.
|
|
stub.setResponder(() => ({
|
|
status: 400,
|
|
body: { error: 'name required' },
|
|
}));
|
|
const result = await runCli([
|
|
'templates',
|
|
'save',
|
|
'proj-1',
|
|
'--name',
|
|
'Cards',
|
|
'--daemon-url',
|
|
stub.baseUrl,
|
|
]);
|
|
expect(result.code).toBe(67);
|
|
const envelope = JSON.parse(result.stderr.trim());
|
|
expect(envelope.error.code).toBe('missing-input');
|
|
expect(envelope.error.message).toBe('name required');
|
|
});
|
|
});
|