mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-12 10:19:42 +02:00
fix(cli): review — 8-char id floor, marker echo, wake answers on send
From the review of #557: - An id shorter than 8 characters refuses with exit 4 before any request, on every verb. `rm 9` resolved to whichever session was alone with that first character (the user's own tab included) and deleted it. Same floor as the server's PARENT_SESSION_ID_MIN_PREFIX. `rm` no longer claims a lineage check: "Delete any session except this one". - `wait --match` help and the README example say the marker must not appear verbatim in the prompt (its echo matches at once) and show the split form. - `send` reads the route's wake-on-LAN answers: `buffered` gets its own line (exit 0), `dropped` exits 1 instead of printing "accepted". - `--` for a prompt that starts with "-", in the `send` description and in the one-argument refusal. - `stripAnsi` builds on the shared one (OSC sequences go too); the inputRefusal JSDoc sits above inputRefusal again. - docs/wiki/Driving-Codeman-From-An-Agent.md gets a `codeman agent` section. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
0481db569d
commit
24e51a3b8e
+66
-1
@@ -8,6 +8,7 @@ import http from 'node:http';
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { Command } from 'commander';
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
|
||||
import {
|
||||
AgentGuardError,
|
||||
@@ -29,7 +30,9 @@ import {
|
||||
httpRequest,
|
||||
inputRefusal,
|
||||
isSelfSession,
|
||||
MIN_ID_PREFIX_LENGTH,
|
||||
parsePositiveInt,
|
||||
registerAgentCommands,
|
||||
resolveAgentContext,
|
||||
stripAnsi,
|
||||
waitExitCode,
|
||||
@@ -300,6 +303,24 @@ describe('agent send', () => {
|
||||
expect(deps.out.join('')).not.toMatch(/delivered to/);
|
||||
});
|
||||
|
||||
it('a sleeping remote host: `buffered` gets its own line and exit 0, never "accepted"', async () => {
|
||||
const deps = fakeDeps([ok({ buffered: true })]);
|
||||
expect(await agentSend(deps, { id: OTHER, text: 'go', enter: true })).toBe(EXIT.ok);
|
||||
expect(deps.out.join('')).toMatch(/buffered for .*asleep/);
|
||||
expect(deps.out.join('')).not.toMatch(/accepted for/);
|
||||
});
|
||||
|
||||
it('`dropped` (over the wake buffer cap) is a failure: exit 1, nothing claims success', async () => {
|
||||
const deps = fakeDeps([ok({ buffered: true, dropped: true })]);
|
||||
expect(await agentSend(deps, { id: OTHER, text: 'go', enter: true })).toBe(EXIT.error);
|
||||
expect(deps.err.join('')).toMatch(/dropped: .*nothing will be typed/);
|
||||
expect(deps.out).toEqual([]);
|
||||
|
||||
const json = fakeDeps([ok({ buffered: true, dropped: true })], true);
|
||||
expect(await agentSend(json, { id: OTHER, text: 'go', enter: true })).toBe(EXIT.error);
|
||||
expect(JSON.parse(json.out.join(''))).toEqual({ buffered: true, dropped: true });
|
||||
});
|
||||
|
||||
it('reports a tagged duplicate instead of claiming delivery', async () => {
|
||||
const deps = fakeDeps([ok({ delivered: false, duplicate: true })]);
|
||||
await agentSend(deps, { id: OTHER, text: 'go', enter: true });
|
||||
@@ -370,7 +391,7 @@ describe('agent read', () => {
|
||||
});
|
||||
|
||||
it('--tail fetches the terminal and strips ANSI', async () => {
|
||||
const deps = fakeDeps([ok({ terminalBuffer: '\u001b[32m❯\u001b[0m ready \u001b(B' })]);
|
||||
const deps = fakeDeps([ok({ terminalBuffer: '\u001b]0;w1 title\u0007\u001b[32m❯\u001b[0m ready \u001b(B' })]);
|
||||
expect(await agentRead(deps, { id: OTHER, tail: 500 })).toBe(EXIT.ok);
|
||||
expect(deps.calls[0]).toMatchObject({ path: `/api/v1/sessions/${OTHER}/terminal`, query: { tail: 500 } });
|
||||
expect(deps.out).toEqual(['❯ ready ']);
|
||||
@@ -414,6 +435,8 @@ describe('send takes the prompt as ONE argument', () => {
|
||||
expect(sendPromptFromArgs(['line', 'one', 'line', 'two'])).toMatchObject({
|
||||
error: expect.stringMatching(/ONE argument, got 4/),
|
||||
});
|
||||
// A leading "-" is commander's option syntax, so the hint names the escape.
|
||||
expect(sendPromptFromArgs(['a', 'b'])).toMatchObject({ error: expect.stringContaining('send <id> -- "- fix') });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -549,6 +572,29 @@ describe('session id prefixes', () => {
|
||||
expect(deps.calls.map((c) => c.method)).toEqual(['GET']);
|
||||
});
|
||||
|
||||
it('a prefix shorter than 8 characters refuses (exit 4) before any request, on every verb', async () => {
|
||||
expect(MIN_ID_PREFIX_LENGTH).toBe(8);
|
||||
// The maintainer's repro: `rm 9` with one other session starting with 9 deleted it.
|
||||
const rm = fakeDeps([ok([{ id: SELF }, { id: OTHER }]), ok({})]);
|
||||
expect(await agentRm(rm, { id: '9' })).toBe(EXIT.refused);
|
||||
expect(rm.calls).toEqual([]);
|
||||
expect(rm.err.join('')).toMatch(/"9" is shorter than 8 characters.*8-character id `agent ls` prints/);
|
||||
|
||||
const short = OTHER.slice(0, 7);
|
||||
const verbs: Array<[string, (deps: AgentDeps) => Promise<number>]> = [
|
||||
['send', (d) => agentSend(d, { id: short, text: 'go', enter: true })],
|
||||
['wait', (d) => agentWait(d, { id: short, until: 'idle', timeoutMs: 1000 })],
|
||||
['read', (d) => agentRead(d, { id: short })],
|
||||
['interrupt', (d) => agentInterrupt(d, { id: short })],
|
||||
['rm', (d) => agentRm(d, { id: short })],
|
||||
];
|
||||
for (const [verb, call] of verbs) {
|
||||
const deps = fakeDeps([ok([{ id: SELF }, { id: OTHER }]), ok({ delivered: true })]);
|
||||
expect(await call(deps), verb).toBe(EXIT.refused);
|
||||
expect(deps.calls, verb).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
it('rm runs the self guard before the list and again on the resolved id', async () => {
|
||||
const first = fakeDeps([ok([{ id: SELF }])]);
|
||||
expect(await agentRm(first, { id: SELF.slice(0, 8) })).toBe(EXIT.refused);
|
||||
@@ -559,6 +605,25 @@ describe('session id prefixes', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('help texts carry the traps the skill documents', () => {
|
||||
const agent = registerAgentCommands(new Command());
|
||||
const sub = (name: string) => agent.commands.find((c) => c.name() === name)!;
|
||||
|
||||
it('--match says the prompt must not contain the marker verbatim, and how to split it', () => {
|
||||
const match = sub('wait').options.find((o) => o.long === '--match')!;
|
||||
expect(match.description).toMatch(/never put the marker verbatim in the prompt/);
|
||||
expect(match.description).toMatch(/WORKDONE followed by _4711.*WORKDONE_4711/);
|
||||
});
|
||||
|
||||
it('send names the -- escape for a prompt that starts with "-"', () => {
|
||||
expect(sub('send').description()).toContain('send <id> -- "- fix the bug"');
|
||||
});
|
||||
|
||||
it('rm does not claim a lineage check it does not make', () => {
|
||||
expect(sub('rm').description()).toMatch(/^Delete any session except this one/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('option parsing', () => {
|
||||
it('positive integers only, the server rejects the rest', () => {
|
||||
expect(parsePositiveInt(undefined, 60000)).toBe(60000);
|
||||
|
||||
Reference in New Issue
Block a user