diff --git a/cli/__tests__/task.test.mjs b/cli/__tests__/task.test.mjs new file mode 100644 index 0000000000..4c47929980 --- /dev/null +++ b/cli/__tests__/task.test.mjs @@ -0,0 +1,328 @@ +/** + * `commonly task` against a fake API. The routes are the ones the commonly_* task + * tools call, and every fake body is the shape backend/routes/tasksApi.ts sends, + * error text included, so a failure here means the CLI and the server disagree. + */ +import { Command } from 'commander'; +import { + registerTask, + runTaskClaim, + runTaskClose, + runTaskCreate, + runTaskList, + runTaskUpdate, +} from '../src/commands/task.js'; +import { TaskRefusal, checkTaskId, checkUpdateStatus, resolveTaskToken } from '../src/lib/task.js'; + +const POD = 'aaaaaaaaaaaaaaaaaaaaaaaa'; +const ENV = { COMMONLY_AGENT_TOKEN: 'cm_agent_test', COMMONLY_API_URL: 'https://api.example.test' }; + +// `handler(method, path, body)` answers each request with { status, json }. Every request is recorded. +const fakeApi = (handler = () => ({ json: {} })) => { + const calls = []; + const fetchImpl = async (url, init) => { + const { pathname, search } = new URL(url); + const body = init.body === undefined ? undefined : JSON.parse(init.body); + calls.push({ method: init.method, path: pathname, search, body, auth: init.headers.Authorization }); + const { status = 200, json = {} } = handler(init.method, pathname, body) || {}; + return { ok: status >= 200 && status < 300, status, text: async () => JSON.stringify(json) }; + }; + return { fetchImpl, calls }; +}; + +const sink = () => { + const lines = []; + const errors = []; + return { lines, errors, log: (line) => lines.push(line), error: (line) => errors.push(line) }; +}; + +const depsFor = (fetchImpl, out) => ({ env: ENV, fetchImpl, log: out.log, error: out.error }); + +const row = (over = {}) => ({ + taskId: 'TASK-669', title: 'D214 gap', status: 'pending', priority: 'P1', assignee: 'wren', claimedByMe: false, ...over, +}); + +describe('task ids and status words', () => { + test.each(['TASK-669', 'task-669', '669'])('%s names TASK-669', (input) => { + expect(checkTaskId(input)).toBe('TASK-669'); + }); + + test.each([ + ['1', 'TASK-001'], + ['001', 'TASK-001'], + ['task-7', 'TASK-007'], + ['706', 'TASK-706'], + ])('%s names %s, padded as the server stores it', (input, expected) => { + expect(checkTaskId(input)).toBe(expected); + }); + + test('anything else is refused before a request', () => { + expect(() => checkTaskId('TASK-')).toThrow(TaskRefusal); + expect(() => checkTaskId('wren/669')).toThrow(TaskRefusal); + }); + + test('update takes only pending or blocked; claim and close have their own verbs', () => { + expect(checkUpdateStatus('Blocked')).toBe('blocked'); + expect(checkUpdateStatus('pending')).toBe('pending'); + expect(() => checkUpdateStatus('claimed')).toThrow(/commonly task claim/); + expect(() => checkUpdateStatus('done')).toThrow(/commonly task close/); + }); +}); + +describe('identity', () => { + test('the token comes from the agent env var, and the saved login is never read', () => { + expect(resolveTaskToken({}, ENV)).toBe('cm_agent_test'); + expect(() => resolveTaskToken({}, { COMMONLY_TOKEN: 'operator-login' })).toThrow(/no token/); + }); +}); + +describe('list', () => { + test('reads the board with the status filter and prints one line per row', async () => { + const api = fakeApi(() => ({ + json: { tasks: [row(), row({ taskId: 'TASK-670', status: 'claimed', claimedByMe: true })] }, + })); + const out = sink(); + const code = await runTaskList({ pod: POD, status: 'pending,claimed' }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(0); + expect(api.calls[0]).toMatchObject({ method: 'GET', path: `/api/v1/tasks/${POD}`, auth: 'Bearer cm_agent_test' }); + expect(api.calls[0].search).toBe('?status=pending%2Cclaimed'); + expect(out.lines).toHaveLength(2); + expect(out.lines[0]).toBe('TASK-669 pending P1 wren D214 gap'); + }); + + test('--mine keeps only the rows this identity holds', async () => { + const api = fakeApi(() => ({ + json: { tasks: [row(), row({ taskId: 'TASK-670', status: 'claimed', claimedByMe: true })] }, + })); + const out = sink(); + await runTaskList({ pod: POD, mine: true }, depsFor(api.fetchImpl, out)); + + expect(out.lines).toHaveLength(1); + expect(out.lines[0]).toMatch(/^TASK-670 claimed/); + }); + + test('--json prints the rows as JSON for agents', async () => { + const api = fakeApi(() => ({ json: { tasks: [row()] } })); + const out = sink(); + await runTaskList({ pod: POD, json: true }, depsFor(api.fetchImpl, out)); + + expect(JSON.parse(out.lines[0])).toEqual([row()]); + }); +}); + +describe('create', () => { + test('creates the row, then sets its notes in a second request', async () => { + const api = fakeApi((method) => (method === 'POST' + ? { status: 201, json: { task: row({ taskId: 'TASK-690' }) } } + : { json: { task: row({ taskId: 'TASK-690', notes: 'Repro in staging' }) } })); + const out = sink(); + const code = await runTaskCreate( + { pod: POD, title: 'Fix the board', priority: 'p1', notes: 'Repro in staging' }, + depsFor(api.fetchImpl, out), + ); + + expect(code).toBe(0); + expect(api.calls.map((c) => [c.method, c.path])).toEqual([ + ['POST', `/api/v1/tasks/${POD}`], + ['PATCH', `/api/v1/tasks/${POD}/TASK-690`], + ]); + expect(api.calls[0].body).toEqual({ title: 'Fix the board', priority: 'P1' }); + expect(api.calls[1].body).toEqual({ notes: 'Repro in staging' }); + }); + + test('without --notes it is one request', async () => { + const api = fakeApi(() => ({ status: 201, json: { task: row({ taskId: 'TASK-691' }) } })); + const out = sink(); + await runTaskCreate({ pod: POD, title: 'Plain row' }, depsFor(api.fetchImpl, out)); + + expect(api.calls).toHaveLength(1); + expect(api.calls[0].body).toEqual({ title: 'Plain row' }); + }); + + test('a failed notes write says the row exists, so nobody retries the create into a twin', async () => { + const api = fakeApi((method) => (method === 'POST' + ? { status: 201, json: { task: row({ taskId: 'TASK-692' }) } } + : { status: 500, json: { error: 'database unavailable' } })); + const out = sink(); + const code = await runTaskCreate({ pod: POD, title: 'x', notes: 'n' }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(1); + expect(out.errors).toEqual([ + 'Created TASK-692, but --notes did not save. Failed (HTTP 500): database unavailable', + ]); + }); +}); + +describe('claim', () => { + test('takes the lease with a POST to /claim', async () => { + const api = fakeApi(() => ({ json: { task: row({ status: 'claimed', claimedByMe: true }) } })); + const out = sink(); + const code = await runTaskClaim('669', { pod: POD }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(0); + expect(api.calls[0]).toMatchObject({ method: 'POST', path: `/api/v1/tasks/${POD}/TASK-669/claim`, body: {} }); + }); + + test('--continue-anyway is sent as the body flag the route reads', async () => { + const api = fakeApi(() => ({ json: { task: row({ status: 'claimed' }) } })); + await runTaskClaim('TASK-669', { pod: POD, continueAnyway: true }, depsFor(api.fetchImpl, sink())); + + expect(api.calls[0].body).toEqual({ continueAnyway: true }); + }); + + test('a live lease held by someone else is one plain line naming the holder and the time', async () => { + // The claim route's compare-and-set loss answers with this text (tasksApi.ts, "Task already claimed"). + const api = fakeApi(() => ({ + status: 409, + json: { + error: 'Task already claimed', + claimedBy: 'alpha', + status: 'claimed', + claimExpiresAt: '2026-10-31T22:04:00.000Z', + }, + })); + const out = sink(); + const code = await runTaskClaim('669', { pod: POD }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(1); + expect(out.errors).toEqual([ + 'Failed (HTTP 409): Task already claimed (held by alpha, lease until 22:04Z)', + ]); + }); + + test('an unseen-messages hold is named by its count, with the way out', async () => { + // The hold carries code 'unseen_messages' (agentSendFreshness.ts), which is the branch the CLI takes. + const api = fakeApi(() => ({ + status: 409, + json: { code: 'unseen_messages', unseenCount: 3, message: 'There are 3 unseen messages.', messages: [] }, + })); + const out = sink(); + const code = await runTaskClaim('669', { pod: POD }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(1); + expect(out.errors).toEqual([ + 'Failed (HTTP 409): 3 unseen messages in this pod. Read them and retry, or pass --continue-anyway to go ahead.', + ]); + }); +}); + +describe('update', () => { + test('a status change is a PATCH of that field only', async () => { + const api = fakeApi(() => ({ json: { task: row({ status: 'blocked' }) } })); + const out = sink(); + const code = await runTaskUpdate('669', { pod: POD, status: 'blocked' }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(0); + expect(api.calls[0]).toMatchObject({ method: 'PATCH', path: `/api/v1/tasks/${POD}/TASK-669`, body: { status: 'blocked' } }); + }); + + test('an empty --assignee is sent as an empty string, which the server stores as unassigned', async () => { + const api = fakeApi(() => ({ json: { task: row({ assignee: null }) } })); + await runTaskUpdate('669', { pod: POD, assignee: '' }, depsFor(api.fetchImpl, sink())); + + expect(api.calls[0].body).toEqual({ assignee: '' }); + }); + + test('--note is a POST to the updates route', async () => { + const api = fakeApi(() => ({ status: 201, json: { task: row() } })); + await runTaskUpdate('669', { pod: POD, note: 'PR is up' }, depsFor(api.fetchImpl, sink())); + + expect(api.calls[0]).toMatchObject({ method: 'POST', path: `/api/v1/tasks/${POD}/TASK-669/updates`, body: { text: 'PR is up' } }); + }); + + test('claiming through update is refused before any request', async () => { + const api = fakeApi(); + const out = sink(); + const code = await runTaskUpdate('669', { pod: POD, status: 'claimed' }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(1); + expect(api.calls).toEqual([]); + expect(out.errors[0]).toMatch(/^refusing: --status claimed is refused/); + }); + + test('nothing to change is refused before any request', async () => { + const api = fakeApi(); + const out = sink(); + const code = await runTaskUpdate('669', { pod: POD }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(1); + expect(api.calls).toEqual([]); + expect(out.errors[0]).toMatch(/^refusing: nothing to change/); + }); + + test('a failed note after a field change says the field did save', async () => { + const api = fakeApi((method) => (method === 'PATCH' + ? { json: { task: row({ title: 'Renamed' }) } } + : { status: 500, json: { error: 'database unavailable' } })); + const out = sink(); + const code = await runTaskUpdate( + '669', + { pod: POD, title: 'Renamed', note: 'context' }, + depsFor(api.fetchImpl, out), + ); + + expect(code).toBe(1); + expect(out.errors).toEqual([ + 'Updated TASK-669, but --note did not post. Failed (HTTP 500): database unavailable', + ]); + }); +}); + +describe('close', () => { + test('completes the row with the PR when one is given', async () => { + const api = fakeApi(() => ({ json: { task: row({ status: 'done' }) } })); + const out = sink(); + const pr = 'https://github.com/Team-Commonly/commonly/pull/3089'; + const code = await runTaskClose('669', { pod: POD, pr }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(0); + expect(api.calls[0]).toMatchObject({ method: 'POST', path: `/api/v1/tasks/${POD}/TASK-669/complete`, body: { prUrl: pr } }); + expect(out.lines[0]).toBe('TASK-669 done P1 wren D214 gap'); + }); + + test('a seat that is neither creator nor holder gets the 409 as one plain line', async () => { + // Complete answers with the lease helper's refusal (tasksApi.ts, leaseRefusalForWrite). + const api = fakeApi(() => ({ + status: 409, + json: { + error: 'Task is claimed by another seat', + status: 'claimed', + claimedBy: 'alpha', + claimExpiresAt: '2026-10-31T22:04:00.000Z', + }, + })); + const out = sink(); + const code = await runTaskClose('669', { pod: POD }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(1); + expect(out.errors).toEqual([ + 'Failed (HTTP 409): Task is claimed by another seat (held by alpha, lease until 22:04Z)', + ]); + }); + + test('a non-member gets one plain 403 line', async () => { + // requirePodMember's write gate answers a non-member with this text (podMembershipService.ts). + const api = fakeApi(() => ({ status: 403, json: { error: 'Write access denied' } })); + const out = sink(); + const code = await runTaskClose('669', { pod: POD }, depsFor(api.fetchImpl, out)); + + expect(code).toBe(1); + expect(out.errors).toEqual(['Failed (HTTP 403): Write access denied']); + expect(out.errors[0]).not.toContain('\n'); + }); +}); + +describe('registration', () => { + test('wires the five subcommands, each with a required --pod', () => { + const program = new Command(); + program.exitOverride(); + registerTask(program); + const task = program.commands.find((c) => c.name() === 'task'); + + expect(task.commands.map((c) => c.name())).toEqual(['list', 'create', 'claim', 'update', 'close']); + for (const sub of task.commands) { + expect(sub.options.some((o) => o.long === '--pod' && o.mandatory)).toBe(true); + } + }); +}); diff --git a/cli/src/commands/task.js b/cli/src/commands/task.js new file mode 100644 index 0000000000..d0f642c2dc --- /dev/null +++ b/cli/src/commands/task.js @@ -0,0 +1,278 @@ +/** + * commonly task + * + * list — the pod's board, optionally narrowed by status, or to rows this identity holds + * create — a new row; --notes is a second request, because create does not take notes + * claim — take the lease on a row + * update — change a row's fields, and/or append a history note + * close — mark a row done, as its creator or its holder + * + * Identity and base URL come from ../lib/task.js. Every subcommand goes through + * connect() below, so none can grow its own fallback to the saved login. Flags are + * checked before the first request, so a refusal costs no network round trip. + */ + +import { TOKEN_FILE_ENV } from '../lib/inbox.js'; +import { + TASK_PRIORITIES, + TASK_STATUSES, + TASK_TOKEN_ENV, + TaskRefusal, + UPDATE_STATUSES, + checkPriority, + checkStatusFilter, + checkTaskId, + checkUpdateStatus, + createTaskClient, + describeFailure, + formatTaskLine, + requireText, + resolveTaskBaseUrl, + resolveTaskToken, + taskPath, +} from '../lib/task.js'; + +export const TASK_TOKEN_HELP = `Agent token file (or ${TOKEN_FILE_ENV}, or ${TASK_TOKEN_ENV})`; + +const connect = (opts, env, fetchImpl) => createTaskClient({ + baseUrl: resolveTaskBaseUrl(opts, env), + token: resolveTaskToken(opts, env), + fetchImpl, +}); + +const podOf = (opts) => requireText(opts.pod, '--pod'); + +const report = (err, error) => { + error(err instanceof TaskRefusal ? `refusing: ${err.message}` : describeFailure(err)); + return 1; +}; + +// A row goes out as JSON for agents, and as the line a person scans otherwise. +const emitRow = (opts, log, task) => { + log(opts.json ? JSON.stringify(task, null, 2) : formatTaskLine(task)); +}; + +export const runTaskList = async (opts = {}, deps = {}) => { + const { + env = process.env, fetchImpl = fetch, log = console.log, error = console.error, + } = deps; + try { + const pod = podOf(opts); + const status = opts.status === undefined ? undefined : checkStatusFilter(opts.status); + const { tasks } = await connect(opts, env, fetchImpl).get(taskPath(pod), { status }); + // The server marks each row claimedByMe for this identity, so --mine needs no second request. + const rows = opts.mine ? tasks.filter((task) => task.claimedByMe) : tasks; + if (opts.json) { + log(JSON.stringify(rows, null, 2)); + } else if (rows.length === 0) { + log('(no tasks match)'); + } else { + for (const task of rows) log(formatTaskLine(task)); + } + return 0; + } catch (err) { + return report(err, error); + } +}; + +export const runTaskCreate = async (opts = {}, deps = {}) => { + const { + env = process.env, fetchImpl = fetch, log = console.log, error = console.error, + } = deps; + try { + const pod = podOf(opts); + const body = { title: requireText(opts.title, '--title') }; + if (opts.priority !== undefined) body.priority = checkPriority(opts.priority); + if (opts.assignee !== undefined) body.assignee = requireText(opts.assignee, '--assignee'); + if (opts.dep !== undefined) body.dep = checkTaskId(opts.dep); + const notes = opts.notes === undefined ? undefined : requireText(opts.notes, '--notes'); + const client = connect(opts, env, fetchImpl); + const { task } = await client.post(taskPath(pod), body); + if (notes === undefined) { + emitRow(opts, log, task); + return 0; + } + // The row exists once the create returns. If the notes fail, say so, so nobody + // retries the create and makes a twin. + try { + const { task: noted } = await client.patch(taskPath(pod, task.taskId), { notes }); + emitRow(opts, log, noted); + return 0; + } catch (err) { + error(`Created ${task.taskId}, but --notes did not save. ${describeFailure(err)}`); + return 1; + } + } catch (err) { + return report(err, error); + } +}; + +export const runTaskClaim = async (taskArg, opts = {}, deps = {}) => { + const { + env = process.env, fetchImpl = fetch, log = console.log, error = console.error, + } = deps; + try { + const pod = podOf(opts); + const taskId = checkTaskId(taskArg); + const body = opts.continueAnyway ? { continueAnyway: true } : {}; + const { task } = await connect(opts, env, fetchImpl).post(`${taskPath(pod, taskId)}/claim`, body); + emitRow(opts, log, task); + return 0; + } catch (err) { + return report(err, error); + } +}; + +export const runTaskUpdate = async (taskArg, opts = {}, deps = {}) => { + const { + env = process.env, fetchImpl = fetch, log = console.log, error = console.error, + } = deps; + try { + const pod = podOf(opts); + const taskId = checkTaskId(taskArg); + const fields = {}; + if (opts.status !== undefined) fields.status = checkUpdateStatus(opts.status); + // An empty value unassigns. The server stores that as null, not as '', so the row + // reads as unassigned to the router. + if (opts.assignee !== undefined) fields.assignee = String(opts.assignee).trim(); + if (opts.title !== undefined) fields.title = requireText(opts.title, '--title'); + if (opts.priority !== undefined) fields.priority = checkPriority(opts.priority); + // "none" clears the dependency. Sending '' would store a dependency that names nothing. + if (opts.dep !== undefined) fields.dep = opts.dep === 'none' ? null : checkTaskId(opts.dep); + const note = opts.note === undefined ? undefined : requireText(opts.note, '--note'); + if (Object.keys(fields).length === 0 && note === undefined) { + throw new TaskRefusal('nothing to change. Pass --status, --assignee, --title, --priority, --dep or --note.'); + } + const client = connect(opts, env, fetchImpl); + const path = taskPath(pod, taskId); + let task = null; + if (Object.keys(fields).length > 0) ({ task } = await client.patch(path, fields)); + if (note !== undefined) { + try { + const body = opts.continueAnyway ? { text: note, continueAnyway: true } : { text: note }; + ({ task } = await client.post(`${path}/updates`, body)); + } catch (err) { + // With no field change before it, a failed note is the whole failure. + if (task === null) throw err; + error(`Updated ${taskId}, but --note did not post. ${describeFailure(err)}`); + return 1; + } + } + emitRow(opts, log, task); + return 0; + } catch (err) { + return report(err, error); + } +}; + +export const runTaskClose = async (taskArg, opts = {}, deps = {}) => { + const { + env = process.env, fetchImpl = fetch, log = console.log, error = console.error, + } = deps; + try { + const pod = podOf(opts); + const taskId = checkTaskId(taskArg); + const prUrl = opts.pr === undefined ? undefined : requireText(opts.pr, '--pr'); + // No notes key here. /complete replaces the row's notes whenever one is sent. + const body = prUrl === undefined ? {} : { prUrl }; + const { task } = await connect(opts, env, fetchImpl).post(`${taskPath(pod, taskId)}/complete`, body); + emitRow(opts, log, task); + return 0; + } catch (err) { + return report(err, error); + } +}; + +export const registerTask = (program) => { + const task = program + .command('task') + .description('Read and write the task board as an agent (list, create, claim, update, close)'); + + task.addHelpText('after', ` +The board is read and written as the identity in a token file, and nowhere else: + + $ commonly task list --pod --token-file + $ COMMONLY_AGENT_TOKEN= commonly task claim TASK-123 --pod + +The identity is --token-file, then COMMONLY_TOKEN_FILE, then COMMONLY_AGENT_TOKEN. +The saved login and COMMONLY_TOKEN are never read: a write made under an +operator's login lands on the row under that operator's name. + +Each verb does what it says. \`claim\` takes the lease. \`close\` stamps the +completion time and the PR. \`update --status\` sets only pending or blocked, so it +cannot claim or close a row. \`update --note\` appends a history line. The row's +notes are set by \`create --notes\` and are not editable here. + +A claim or a note can refuse with unseen messages in the pod. Read them and retry, +or pass --continue-anyway to go ahead without reading them. + +Examples: + $ commonly task list --pod --status pending,claimed + $ commonly task list --pod --mine --json + $ commonly task create --pod --title "Fix the board" --priority P1 --notes "Repro in staging" + $ commonly task claim TASK-123 --pod + $ commonly task update TASK-123 --pod --note "PR is up" + $ commonly task close TASK-123 --pod --pr https://github.com/Team-Commonly/commonly/pull/ +`); + + task + .command('list') + .description("List a pod's task board") + .requiredOption('--pod ', 'Pod whose board to read') + .option('--status ', `Only these statuses, comma-separated (${TASK_STATUSES.join('|')})`) + .option('--mine', 'Only rows this identity holds a claim on') + .option('--token-file ', TASK_TOKEN_HELP) + .option('--instance ', 'Target Commonly instance') + .option('--json', 'Emit the rows as JSON') + .action(async (opts) => { process.exit(await runTaskList(opts)); }); + + task + .command('create') + .description('Create a row, then set its --notes (create does not take notes)') + .requiredOption('--pod ', 'Pod to create the row in') + .requiredOption('--title ', 'Row title') + .option('--notes ', 'Notes to set once the row exists') + .option('--priority

', `${TASK_PRIORITIES.join('|')} (default P2)`) + .option('--assignee ', 'Assign the row at creation') + .option('--dep ', 'Row this one waits on') + .option('--token-file ', TASK_TOKEN_HELP) + .option('--instance ', 'Target Commonly instance') + .option('--json', 'Emit the row as JSON') + .action(async (opts) => { process.exit(await runTaskCreate(opts)); }); + + task + .command('claim ') + .description('Take the lease on a row (TASK-n)') + .requiredOption('--pod ', 'Pod the row is in') + .option('--continue-anyway', 'Claim even with unseen messages in the pod') + .option('--token-file ', TASK_TOKEN_HELP) + .option('--instance ', 'Target Commonly instance') + .option('--json', 'Emit the row as JSON') + .action(async (taskId, opts) => { process.exit(await runTaskClaim(taskId, opts)); }); + + task + .command('update ') + .description("Change a row's fields, and/or append a history note") + .requiredOption('--pod ', 'Pod the row is in') + .option('--status ', `${UPDATE_STATUSES.join('|')} (claim and close have their own commands)`) + .option('--assignee ', 'Reassign the row; an empty value unassigns it') + .option('--title ', 'New title') + .option('--priority

', TASK_PRIORITIES.join('|')) + .option('--dep ', 'Row this one waits on; "none" clears it') + .option('--note ', 'Append a history note') + .option('--continue-anyway', 'Post the note even with unseen messages in the pod') + .option('--token-file ', TASK_TOKEN_HELP) + .option('--instance ', 'Target Commonly instance') + .option('--json', 'Emit the row as JSON') + .action(async (taskId, opts) => { process.exit(await runTaskUpdate(taskId, opts)); }); + + task + .command('close ') + .description('Mark a row done, as its creator or its holder') + .requiredOption('--pod ', 'Pod the row is in') + .option('--pr ', 'Pull request that delivered the row') + .option('--token-file ', TASK_TOKEN_HELP) + .option('--instance ', 'Target Commonly instance') + .option('--json', 'Emit the row as JSON') + .action(async (taskId, opts) => { process.exit(await runTaskClose(taskId, opts)); }); +}; diff --git a/cli/src/index.js b/cli/src/index.js index 8b933fd250..415b8f2db1 100755 --- a/cli/src/index.js +++ b/cli/src/index.js @@ -23,6 +23,7 @@ import { registerDev } from './commands/dev.js'; import { registerVerify } from './commands/verify.js'; import { registerDoctor } from './commands/doctor.js'; import { registerMcp } from './commands/mcp.js'; +import { registerTask } from './commands/task.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); const pkg = JSON.parse(readFileSync(join(__dirname, '../package.json'), 'utf8')); @@ -60,6 +61,9 @@ registerVerify(program); registerDoctor(program); registerMcp(program); +// The task board as an agent: list, create, claim, update, close (TASK-703) +registerTask(program); + // Each subcommand owns its own `--instance ` flag; a program-level // duplicate shadowed the subcommand value on commander v12, causing // `commonly login --instance …` to silently fall back to the default URL. diff --git a/cli/src/lib/task.js b/cli/src/lib/task.js new file mode 100644 index 0000000000..3a6c26005e --- /dev/null +++ b/cli/src/lib/task.js @@ -0,0 +1,234 @@ +/** + * The task board from the CLI: identity, the request layer, and one-line + * rendering. The routes are the ones the commonly_* task tools call + * (backend/routes/tasksApi.ts), so the CLI and the MCP cannot disagree about + * what a write means. + * + * Identity is an explicit token and nothing else: --token-file, then + * COMMONLY_TOKEN_FILE, then COMMONLY_AGENT_TOKEN. The saved login and + * COMMONLY_TOKEN are never read. A write made under an operator's saved login + * lands on the row under that operator's name. + */ + +import { readFileSync } from 'fs'; +import { DEFAULT_URL, resolveInstance } from './config.js'; +import { extractToken, resolveTokenFilePath, TOKEN_FILE_ENV } from './inbox.js'; + +export const API_URL_ENV = 'COMMONLY_API_URL'; +export const TASK_TOKEN_ENV = 'COMMONLY_AGENT_TOKEN'; +export const TASK_STATUSES = ['pending', 'claimed', 'done', 'blocked']; +// `update` writes only these. A claim writes the lease, which /claim owns, and a +// close stamps completedAt and the PR, which PATCH does not. +export const UPDATE_STATUSES = ['pending', 'blocked']; +export const TASK_PRIORITIES = ['P0', 'P1', 'P2', 'P3']; +const REQUEST_TIMEOUT_MS = 30_000; + +/** A refusal made before any request: a bad flag, or no identity to send. */ +export class TaskRefusal extends Error {} + +/** A failed call. `status` is 0 when the server was never reached. */ +export class TaskRequestError extends Error { + constructor(message, { status = 0, code = null, body = null } = {}) { + super(message); + this.name = 'TaskRequestError'; + this.status = status; + this.code = code; + this.body = body; + } +} + +export const requireText = (value, flag) => { + const text = typeof value === 'string' ? value.trim() : ''; + if (!text) throw new TaskRefusal(`${flag} needs a value.`); + return text; +}; + +export const checkPriority = (value) => { + const priority = String(value).trim().toUpperCase(); + if (!TASK_PRIORITIES.includes(priority)) { + throw new TaskRefusal(`--priority must be ${TASK_PRIORITIES.join(', ')} (got '${value}').`); + } + return priority; +}; + +export const checkStatusFilter = (value) => { + const statuses = String(value).split(',').map((s) => s.trim()).filter(Boolean); + if (statuses.length === 0 || statuses.some((s) => !TASK_STATUSES.includes(s))) { + throw new TaskRefusal(`--status takes ${TASK_STATUSES.join(', ')}, comma-separated (got '${value}').`); + } + return statuses.join(','); +}; + +export const checkUpdateStatus = (value) => { + const status = String(value).trim().toLowerCase(); + if (status === 'claimed') { + throw new TaskRefusal('--status claimed is refused: `commonly task claim` writes the lease, and a bare status does not.'); + } + if (status === 'done') { + throw new TaskRefusal('--status done is refused: `commonly task close` stamps completedAt and the PR, and PATCH does not.'); + } + if (!UPDATE_STATUSES.includes(status)) { + throw new TaskRefusal(`--status must be ${UPDATE_STATUSES.join(' or ')} (got '${value}').`); + } + return status; +}; + +/** TASK-669, task-669 and 669 name one row. Anything else is refused before a request. */ +export const checkTaskId = (value) => { + const match = String(value ?? '').trim().match(/^(?:task-)?(\d+)$/i); + if (!match) throw new TaskRefusal(`task id must look like TASK-123 (got '${value ?? ''}').`); + return `TASK-${String(Number(match[1])).padStart(3, '0')}`; +}; + +/** + * Resolve the token. A token file that exists but holds nothing is refused by + * its path. The message never suggests `commonly login`, because pointing back + * at the saved login is the thing this rule exists to prevent. + */ +export const resolveTaskToken = (opts = {}, env = process.env) => { + const tokenFile = resolveTokenFilePath(opts, env); + if (tokenFile) { + let raw; + try { + raw = readFileSync(tokenFile, 'utf8'); + } catch (err) { + throw new TaskRefusal(`cannot read the token file ${tokenFile} (${err?.code || err?.message}).`); + } + const token = extractToken(raw); + if (!token) throw new TaskRefusal(`no token in ${tokenFile}. Expected a raw token, or JSON with a token field.`); + return token; + } + const fromEnv = extractToken(env[TASK_TOKEN_ENV]); + if (fromEnv) return fromEnv; + throw new TaskRefusal(`no token. Pass --token-file , or set ${TOKEN_FILE_ENV} or ${TASK_TOKEN_ENV}.`); +}; + +/** + * --instance takes a URL or a saved key, and an unknown key is refused. Falling + * back to the default host would send this token to an instance it was not + * minted on. + */ +export const resolveTaskBaseUrl = (opts = {}, env = process.env) => { + if (opts.instance) { + const inst = resolveInstance(opts.instance); + if (!inst?.url) { + throw new TaskRefusal(`unknown instance '${opts.instance}'. Pass its URL, or a saved instance key.`); + } + return inst.url.replace(/\/$/, ''); + } + const fromEnv = String(env[API_URL_ENV] || '').trim().replace(/\/$/, ''); + return fromEnv || DEFAULT_URL; +}; + +/** `/api/v1/tasks/` for the board, and `…/` for one row. */ +export const taskPath = (podId, taskId = null) => [ + '/api/v1/tasks', + encodeURIComponent(podId), + ...(taskId === null ? [] : [encodeURIComponent(taskId)]), +].join('/'); + +const messageFor = (status, body, text) => { + if (body && typeof body.error === 'string' && body.error) return body.error; + if (body && typeof body.message === 'string' && body.message) return body.message; + const trimmed = String(text || '').trim(); + if (!trimmed) return `HTTP ${status}`; + if (/^<(?:!doctype|html)/i.test(trimmed)) { + const title = trimmed.match(/]*>([\s\S]*?)<\/title>/i)?.[1]; + return title ? title.replace(/\s+/g, ' ').trim() : 'HTML error page'; + } + return trimmed.replace(/\s+/g, ' ').slice(0, 200); +}; + +/** + * One attempt per call. The writes here are POSTs, and a retried create or note + * lands twice, so a failure is reported and the caller decides what to do next. + */ +export const createTaskClient = ({ baseUrl, token, fetchImpl = fetch }) => { + const send = async (method, path, body) => { + let res; + try { + res = await fetchImpl(`${baseUrl}${path}`, { + method, + headers: { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json', + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); + } catch (err) { + throw new TaskRequestError(`could not reach ${baseUrl} (${err?.message || err}).`); + } + const text = await res.text(); + let parsed = null; + try { + parsed = text ? JSON.parse(text) : null; + } catch { + parsed = null; + } + if (!res.ok) { + throw new TaskRequestError(messageFor(res.status, parsed, text), { + status: res.status, + code: parsed?.code || null, + body: parsed, + }); + } + return parsed; + }; + + const get = (path, params = {}) => { + const query = new URLSearchParams( + Object.entries(params).filter(([, value]) => value !== undefined && value !== ''), + ).toString(); + return send('GET', query ? `${path}?${query}` : path); + }; + + return { + get, + post: (path, body = {}) => send('POST', path, body), + patch: (path, body = {}) => send('PATCH', path, body), + }; +}; + +const clockOf = (iso) => { + if (!iso) return null; + const at = new Date(iso); + return Number.isNaN(at.getTime()) ? null : `${at.toISOString().slice(11, 16)}Z`; +}; + +/** One row, as a human reads it. A claimed row also says whether its lease holds. */ +export const formatTaskLine = (task) => { + const line = [task.taskId, task.status, task.priority || '-', task.assignee || '-', task.title].join(' '); + if (task.status !== 'claimed') return line; + const at = clockOf(task.claimExpiresAt); + if (!at) return line; + const lapsed = new Date(task.claimExpiresAt).getTime() < Date.now(); + return `${line} (${lapsed ? 'lease lapsed' : 'held until'} ${at})`; +}; + +/** + * One line for a failed call. The unseen-messages hold and a lease conflict get + * their own wording, because a seat acts on each differently. Everything else is + * the server's own text. + */ +export const describeFailure = (err) => { + if (!(err instanceof TaskRequestError)) return `Failed: ${err?.message || err}`; + if (err.status === 0) return `Failed: ${err.message}`; + const body = err.body || {}; + let line; + if (err.code === 'unseen_messages') { + const count = Number(body.unseenCount) || 0; + line = `Failed (HTTP ${err.status}): ${count} unseen message${count === 1 ? '' : 's'} in this pod. ` + + 'Read them and retry, or pass --continue-anyway to go ahead.'; + } else { + const detail = [ + body.claimedBy ? `held by ${body.claimedBy}` : null, + clockOf(body.claimExpiresAt) ? `lease until ${clockOf(body.claimExpiresAt)}` : null, + ].filter(Boolean).join(', '); + line = `Failed (HTTP ${err.status}): ${err.message}${detail ? ` (${detail})` : ''}`; + if (err.status === 401) { + line += '. Check the agent token: --token-file, COMMONLY_TOKEN_FILE or COMMONLY_AGENT_TOKEN.'; + } + } + return line.replace(/\s+/g, ' '); +};