diff --git a/docs/MULTIAGENT.md b/docs/MULTIAGENT.md index bac4c63..1f800ba 100644 --- a/docs/MULTIAGENT.md +++ b/docs/MULTIAGENT.md @@ -42,6 +42,20 @@ it. Planner memory is not read by the coder. This introduces no new store, shared raw history, model-learning claim or autonomous permission grant. These boundaries implement FEATURE_SPEC sections 5.2, 5.4 and 5.6. +Acceptance checks are bounded to 240 characters after whitespace normalization. +New slice JSON, written task/plan proposals, child outlines and planning receipts +reject oversized checks explicitly rather than cutting off requirements. +Reading existing task documents preserves complete checks, including legacy +long checks, so source-validated acceptance continuation remains compatible. +Model validation feedback requests concise complete wording within the existing +turn budget. Oversized written checks receive the single validation correction +immediately, rather than repeating rewrites until the general budget expires. +A still-invalid correction stops before coding and publication. +Title shortening is unchanged. This implements FEATURE_SPEC +sections 5.3-5.5; neither the original Ask nor its safety obligations are waived. +Directory-document checks also recognize content phrasing such as "covering", +"gives instructions" and "indexes"; operation-success assertions stay denied. + Both ordinary minimum and full coder packs include the complete configured [coder principal](../principals/coder.md) and root repository coding rules. The existing rules filter still omits harness-only publication, credentials diff --git a/src/planner/plan-mode.mjs b/src/planner/plan-mode.mjs index 9104bfe..e1a3d9e 100644 --- a/src/planner/plan-mode.mjs +++ b/src/planner/plan-mode.mjs @@ -35,6 +35,7 @@ export async function planSlice(ask, { { role: 'system', content: 'You are the read-only plan-mode planner. Explore with read_file, list_dir, or search_text. ' + 'You may write only PLAN.md, never TASK.md, RECIPE.yml, product code, tests, or publishing operations. ' + 'PLAN.md must contain a title, Original Ask, Acceptance checks, explicit Files allowed, and task metadata. ' + + 'Each acceptance check is at most 240 characters; shorten wording without dropping requirements. ' + 'Alternatively return JSON with title, acceptance_checks, files_allowed, and optional difficulty, estimate_min, task_class, model. ' + 'Stay inside the files named by the human Ask. The human must accept before coding.' }, { role: 'user', content: ask }, diff --git a/src/planner/plan.mjs b/src/planner/plan.mjs index 63bace5..610f1eb 100644 --- a/src/planner/plan.mjs +++ b/src/planner/plan.mjs @@ -2,7 +2,7 @@ import { mergeUsage } from '../metrics/run.mjs'; import { isAllowedFile } from '../runtime/tools.mjs'; import { redactSecrets } from '../runtime/memory.mjs'; import { askRequirements, cleanAskText } from './stub.mjs'; -import { allowedFile, checkedList, oneLine } from './task.mjs'; +import { acceptanceCheck, allowedFile, checkedList, oneLine } from './task.mjs'; import { selectReasoning } from '../llm/reasoning.mjs'; function object(value, required, optional = []) { @@ -56,7 +56,7 @@ export function validatePlan(value, { kind, filesAllowed = [], repositoryFiles, } return { title, outcome: oneLine(issue.outcome, 'Child issue outcome'), acceptance_checks: checkedList(issue.acceptance_checks, 'Child issue checks', - (check) => oneLine(check, 'Acceptance check')), + acceptanceCheck), wave: issue.wave, files_allowed: files }; }); const waves = [...new Set(issues.map(({ wave }) => wave))].sort((a, b) => a - b); @@ -152,6 +152,7 @@ export async function planOutline(ask, { 'Do not implement, create issues, or grant policy permissions. Return only JSON with outcomes (1-8 short lines) ' + `and issues (${kind === 'feature' ? '2-5' : '2-12'} child issue drafts). Each issue has title, outcome, ` + 'acceptance_checks (1-8 short verifiable lines), wave (integer1-8 starting at1 without gaps), ' + + 'Each acceptance check is at most 240 characters; shorten wording without dropping requirements. ' + (proposeScope ? 'and files_allowed (1-32 paths). The human named no files: propose each draft\'s planned files from ' + 'repository_files, or new files inside their directories. Planned files are scope the coder may expand ' + diff --git a/src/planner/receipt.mjs b/src/planner/receipt.mjs index bd1d894..be7b820 100644 --- a/src/planner/receipt.mjs +++ b/src/planner/receipt.mjs @@ -1,4 +1,4 @@ -import { applicationFiles, normalizeAsk, oneLine } from './task.mjs'; +import { acceptanceCheck, applicationFiles, normalizeAsk } from './task.mjs'; import { estimateTask } from '../runtime/estimate.mjs'; const fields = new Set(['task', 'title', 'task_class', 'difficulty', 'estimate_min', 'model', 'ask', @@ -46,7 +46,7 @@ export function validatePlanningReceipt(source, task) { !Array.isArray(receipt.files_allowed)) { throw new TypeError('Planning receipt must match the task title and contain checks and allowed files'); } - receipt.acceptance_checks.forEach((check) => oneLine(check, 'Receipt acceptance check')); + receipt.acceptance_checks.forEach(acceptanceCheck); const files = applicationFiles(receipt.files_allowed); if (files.length !== task.files_allowed.length || files.some((file) => !task.files_allowed.includes(file))) { throw new TypeError('Planning receipt application paths differ from TASK.md'); diff --git a/src/planner/stub.mjs b/src/planner/stub.mjs index c658376..49bb85b 100644 --- a/src/planner/stub.mjs +++ b/src/planner/stub.mjs @@ -7,7 +7,7 @@ import { isAllowedFile, plannerToolDefinitions } from '../runtime/tools.mjs'; import { redactSecrets } from '../runtime/memory.mjs'; import { applyFeedback } from './feedback.mjs'; import { parsePlannerToolCalls } from './tool-calls.mjs'; -import { allowedFile, checkedList, ensureAcceptanceChecks, ensureAllowedFiles, ensureOriginalAsk, ensureTitleHeading, isDirectoryDocumentAsk, oneLine, outputDirectoryFromAsk, parseTaskDocument, validateDirectoryDocumentPlan, validateDirectoryFiles } from './task.mjs'; +import { acceptanceCheck, allowedFile, checkedList, ensureAcceptanceChecks, ensureAllowedFiles, ensureOriginalAsk, ensureTitleHeading, isDirectoryDocumentAsk, oneLine, outputDirectoryFromAsk, parseTaskDocument, validateDirectoryDocumentPlan, validateDirectoryFiles } from './task.mjs'; import { selectReasoning } from '../llm/reasoning.mjs'; import { isLlmTimeout, localRequestTimeoutMs, resolveRequestTimeout } from '../llm/request.mjs'; import { issueWave } from '../lib/wave-labels.mjs'; @@ -89,7 +89,7 @@ export function buildPlan(ask, { reference, title, acceptanceChecks, filesAllowe const files = checkedList(filesAllowed, 'Files allowed', allowedFile, 32); const docsOnly = files.length > 0 && files.every((file) => file.endsWith('.md')); const checks = checkedList(acceptanceChecks, 'Acceptance checks', - (check) => oneLine(check, 'Acceptance check')) + acceptanceCheck) .filter((check) => !(docsOnly && /node --test/.test(check))); const requirements = scope ?? askRequirements(cleanAsk); if (requirements.outputDirectory && !checks.length) { @@ -145,6 +145,7 @@ export function planStub(ask, { reference = 'local:draft', title, metadata } = { export function planFromTask(task, ask, { issueTitle, issueBody } = {}) { const { title, acceptance_checks, files_allowed } = parseTaskDocument(task, { expectedAsk: ask, issueTitle, issueBody }); + acceptance_checks.forEach(acceptanceCheck); checkAskScope(files_allowed, askRequirements(issueTitle ? `${issueTitle}\n${ask}` : ask, { allowMissing: true })); const { difficulty, estimate_min, task_class, model } = readTaskMetadata(task); return { title, acceptance_checks, files_allowed, difficulty, estimate_min, task_class, model, design: parseDesign(task) }; @@ -222,6 +223,7 @@ export async function planAsk(ask, { : 'You have no tools in this draft-only planning context. ') + 'Plan one software task. JSON shape: {"title":"Task title","acceptance_checks":["check"],"files_allowed":["path"]}. ' + 'Use 1-8 acceptance checks in JSON or TASK.md; group related checks without dropping requirements. ' + + 'Each acceptance check must be at most 240 characters; shorten wording without dropping requirements. ' + 'A code task needs acceptance_checks including node --test exits 0 and files_allowed. ' + 'Optional fields: difficulty (1-5), estimate_min, task_class (feat|fix|docs|test), model, steps, notes. ' + (lockedModel ? `Keep model ${lockedModel}; do not choose another model. ` : '') + @@ -427,6 +429,16 @@ export async function planAsk(ask, { }); } catch (error) { if (!(error instanceof Error)) throw error; + if (!/^[{\[]/.test(plannerPlanText(message.content)) && + /^Acceptance check must be at most 240 characters/.test(error.message)) { + if (validationRepairUsed) return fallback(error.message, turn); + validationRepairUsed = true; + messages.push({ role: 'user', content: + `The written TASK.md is invalid (${redactSecrets(error.message, { env, apiKeyEnv: config.llm.api_key_env })}). ` + + 'Use this single validation correction to return the complete required JSON plan. ' + + 'Keep every requirement and the original Ask; the harness will write the validated artifacts.' }); + continue; + } if (!/^[{\[]/.test(plannerPlanText(message.content)) && turn >= budget + Number(repairUsed)) { if (validationRepairUsed) return fallback(error.message, turn); if (!/^Acceptance checks must contain 1-8 entries/.test(error.message)) { diff --git a/src/planner/task.mjs b/src/planner/task.mjs index 02fdf22..3bea8e1 100644 --- a/src/planner/task.mjs +++ b/src/planner/task.mjs @@ -1,10 +1,22 @@ import { splitTaskFrontmatter } from '../runtime/skills.mjs'; import { isForbiddenWrite, planArtifactFiles, plannerArtifactFiles } from '../runtime/tools.mjs'; -export function oneLine(value, label) { +function normalizedLine(value, label) { const collapsed = typeof value === 'string' ? value.replace(/[\x00-\x1f\x7f]+/g, ' ').replace(/\s+/g, ' ').trim() : ''; if (!collapsed) throw new TypeError(`${label} must be one nonempty line (at most 240 characters)`); - return collapsed.slice(0, 240); + return collapsed; +} + +export function oneLine(value, label) { + return normalizedLine(value, label).slice(0, 240); +} + +export function acceptanceCheck(value) { + const check = normalizedLine(value, 'Acceptance check'); + if (check.length > 240) { + throw new TypeError('Acceptance check must be at most 240 characters; shorten it without dropping requirements'); + } + return check; } export function allowedFile(value) { @@ -67,7 +79,7 @@ export function validateDirectoryDocumentPlan(ask, files, checks) { const commands = /\b(?:npm|npx|pnpm|yarn|pip|apt|docker|kubectl|terraform|curl|wget|ssh|bash|powershell|Invoke-WebRequest)\b|\b(?:git\s+(?:push|clone)|gh\s+(?:pr|release)|node\s+--test)\b/i; const action = /\b(?:install(?:ation|ed|s)?|deploy(?:ment|ed|s)?|publish(?:ed|es|ing)?|purchase(?:d|s)?|buy|account|network|request|fetch|download(?:ed|s)?|upload(?:ed|s)?|serve(?:d|s)?|hosting)\b/i; const assertion = /\b(?:run|execute|succeeds?|successful(?:ly)?|exits?|passes?|created|activated|completed|live|reachable)\b|\b(?:is|are|was|were)\s+(?:installed|deployed|published|purchased|serving|running)\b/i; - const content = /\b(?:documents?|describes?|explains?|outlines?|contains?|includes?|covers?|lists?|mentions?|states?|warns?|contrasts?|shows?|compares?|defines?|presents?)\b/i; + const content = /\b(?:documents?|describes?|explains?|outlines?|contains?|includes?|cover(?:s|ing)?|lists?|mentions?|states?|warns?|contrasts?|shows?|compares?|defines?|presents?|gives?|indexes)\b/i; const activeClause = /(?:[;:]|\b(?:and|then))\s+(?:run|execute|install|deploy|publish|purchase|buy|register|create an? account|fetch|download|upload|request)(?:s|ed)?\b/i; for (const [index, check] of checks.entries()) { if (!content.test(check) || activeClause.test(check) || commands.test(check) && assertion.test(check) || @@ -266,7 +278,7 @@ export function parseTaskDocument(task, { expectedAsk, issueTitle, issueBody } = } const acceptance_checks = checkedList(sectionList(parsed.sections.find(({ name }) => name === 'acceptance checks'), 'Acceptance Checks (or acceptance_checks)', { continuations: true }), - 'Acceptance checks', (line) => oneLine(line, 'Acceptance check')); + 'Acceptance checks', (line) => normalizedLine(line, 'Acceptance check')); const files_allowed = taskFilesAllowed(task); if (outputDirectory) { validateDirectoryFiles(files_allowed, outputDirectory); diff --git a/src/runtime/checklist.mjs b/src/runtime/checklist.mjs index 424ab6b..1395f4c 100644 --- a/src/runtime/checklist.mjs +++ b/src/runtime/checklist.mjs @@ -50,7 +50,7 @@ const keysAre = (value, keys) => value && typeof value === 'object' && !Array.is Object.keys(value).sort().join(',') === keys.split(',').sort().join(','); const hash = (value) => typeof value === 'string' && /^[a-f0-9]{64}$/.test(value); -// The planner's display parser caps each check at 240 characters; continuity must not lose obligations. +// Existing task checks are authoritative data; continuity must not lose obligations. export function authoritativeAcceptanceChecks(task) { const parsed = parseTaskDocument(task); const content = taskSections(task).sections.find(({ name }) => name === 'acceptance checks').content; diff --git a/tests/planner-task.test.mjs b/tests/planner-task.test.mjs index e155b8b..a6d0753 100644 --- a/tests/planner-task.test.mjs +++ b/tests/planner-task.test.mjs @@ -64,11 +64,15 @@ test('observed plain-language document contrasts and examples validate without a 'The guide shows one read-only PowerShell example that lists branches, `git branch -a`, and one that lists worktrees, `git worktree list`.', 'The guide compares branches with worktrees.', 'The guide defines the terms and presents illustrative examples.', + 'The design is a one-page design covering layout, sample copy and visual style.', + 'SETUP gives step-by-step instructions for obtaining a domain and serving the page publicly.', + 'README indexes the two documents and describes their purpose.', ]; const plan = buildPlan(directoryAsk, { ...directoryOptions, acceptanceChecks: checks }); assert.deepEqual(parseTaskDocument(plan.task, { expectedAsk: directoryAsk }).acceptance_checks, checks); for (const check of ['The guide shows deployment completed', 'The guide presents account creation and the account is activated', - 'The guide compares instructions and run npm install', 'The guide defines setup; execute powershell']) { + 'The guide compares instructions and run npm install', 'The guide defines setup; execute powershell', + 'The guide gives instructions and run npm install', 'The guide is covering deployment completed']) { assert.throws(() => buildPlan(directoryAsk, { ...directoryOptions, acceptanceChecks: [check] }), /document-content checks.*check 1/i); } diff --git a/tests/planner.acceptance.test.mjs b/tests/planner.acceptance.test.mjs new file mode 100644 index 0000000..8382b01 --- /dev/null +++ b/tests/planner.acceptance.test.mjs @@ -0,0 +1,110 @@ +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import test from 'node:test'; +import { parseConfig } from '../src/lib/config.mjs'; +import { validatePlan } from '../src/planner/plan.mjs'; +import { validatePlanningReceipt } from '../src/planner/receipt.mjs'; +import { buildPlan, planAsk, planFromTask } from '../src/planner/stub.mjs'; +import { parseTaskDocument } from '../src/planner/task.mjs'; + +const ask = 'Write a Markdown guide. Output it in a new "guide" directory'; +const check = (length) => `The guide contains ${'x'.repeat(length - 23)} end`; +const options = { reference: 'local:acceptance', filesAllowed: ['guide/README.md'] }; +const outline = (entry) => ({ + outcomes: ['Document safe setup'], + issues: [1, 2].map((wave) => ({ title: `Guide ${wave}`, outcome: 'Explain setup', wave, + files_allowed: ['guide/README.md'], acceptance_checks: [entry] })), +}); + +test('all planner ingress paths reject oversized checks and preserve the exact 240-character boundary', () => { + const full = check(240); + assert.equal(full.length, 240); + const task = buildPlan(ask, { ...options, acceptanceChecks: [full] }).task; + assert.equal(parseTaskDocument(task, { expectedAsk: ask }).acceptance_checks[0], full); + assert.equal(parseTaskDocument(buildPlan(ask, { ...options, + acceptanceChecks: [full.replace(' contains ', ' contains \n')] }).task).acceptance_checks[0], full); + assert.equal(validatePlan(outline(full), { kind: 'feature', filesAllowed: options.filesAllowed }) + .issues[0].acceptance_checks[0], full); + const receipt = (entry) => `title: ${JSON.stringify(parseTaskDocument(task).title)}\n` + + `acceptance_checks:\n - ${JSON.stringify(entry)}\nfiles_allowed:\n - guide/README.md\n`; + validatePlanningReceipt(receipt(full), parseTaskDocument(task)); + const long = check(241); + for (const operation of [ + () => buildPlan(ask, { ...options, acceptanceChecks: [long] }), + () => planFromTask(task.replace(full, long), ask), + () => validatePlan(outline(long), { kind: 'feature', filesAllowed: options.filesAllowed }), + () => validatePlanningReceipt(receipt(long), parseTaskDocument(task)), + ]) assert.throws(operation, /Acceptance check.*240.*shorten/i); + assert.equal(parseTaskDocument(task.replace(full, long), { expectedAsk: ask }).acceptance_checks[0], long); +}); + +test('an oversized written TASK stays invalid until a complete concise JSON correction', async () => { + const config = parseConfig(readFileSync(new URL('../roster.config.example.yml', import.meta.url), 'utf8') + .replace('base_url: ""', 'base_url: "http://localhost:1234/v1"').replace('model: ""', 'model: test-model')); + const concise = 'The guide contains setup steps and the required safety warning'; + const valid = buildPlan(ask, { ...options, acceptanceChecks: [concise] }).task; + let calls = 0; + const result = await planAsk(ask, { config, env: {}, + tools: { write_file: async () => ({ ok: true }) }, + fetchImpl: async (_url, request) => { + const body = JSON.parse(request.body); + calls += 1; + assert.equal(body.messages[1].content, ask); + if (calls === 1) return Response.json({ choices: [{ finish_reason: 'tool_calls', message: { + role: 'assistant', tool_calls: [{ id: 'oversized', type: 'function', function: { + name: 'write_file', arguments: JSON.stringify({ path: 'TASK.md', content: valid.replace(concise, check(241)) }), + } }], + } }] }); + assert.match(JSON.stringify(body.messages), /240.*shorten/i); + return Response.json({ choices: [{ finish_reason: 'stop', message: { role: 'assistant', + content: JSON.stringify({ title: 'Guide', acceptance_checks: [concise], files_allowed: options.filesAllowed }), + } }] }); + } }); + assert.equal(calls, 2); + assert.deepEqual(parseTaskDocument(result.task, { expectedAsk: ask }).acceptance_checks, [concise]); +}); + +test('a repeatedly invalid written TASK stops after one correction even with a large general turn budget', async () => { + const config = parseConfig(readFileSync(new URL('../roster.config.example.yml', import.meta.url), 'utf8') + .replace('base_url: ""', 'base_url: "http://localhost:1234/v1"').replace('model: ""', 'model: test-model')); + const valid = buildPlan(ask, { ...options, acceptanceChecks: ['The guide contains setup instructions'] }).task; + const invalid = valid.replace('The guide contains setup instructions', check(241)); + let calls = 0; + const result = await planAsk(ask, { config: { ...config, planner: { ...config.planner, turn_budget: 20 } }, env: {}, + tools: { write_file: async () => ({ ok: true }) }, + fetchImpl: async () => { + calls += 1; + return Response.json({ choices: [{ finish_reason: 'tool_calls', message: { + role: 'assistant', tool_calls: [{ id: `invalid-${calls}`, type: 'function', function: { + name: 'write_file', arguments: JSON.stringify({ path: 'TASK.md', content: invalid }), + } }], + } }] }); + } }); + assert.equal(calls, 2); + assert.equal(result.mode, 'stub'); + assert.match(result.error, /240.*shorten/); + assert.equal(parseTaskDocument(result.task, { expectedAsk: ask }).ask, ask); +}); + +test('slice JSON correction retains the original Ask and does not accept a truncated requirement', async () => { + const config = parseConfig(readFileSync(new URL('../roster.config.example.yml', import.meta.url), 'utf8') + .replace('base_url: ""', 'base_url: "http://localhost:1234/v1"').replace('model: ""', 'model: test-model')); + let calls = 0; + const concise = 'The guide contains setup steps and ends with the required safety warning'; + const result = await planAsk(ask, { config, env: {}, + fetchImpl: async (_url, request) => { + calls += 1; + const body = JSON.parse(request.body); + assert.equal(body.messages[1].content, ask); + if (calls === 2) assert.match(body.messages.at(-1).content, /240.*shorten/i); + return Response.json({ choices: [{ finish_reason: 'stop', message: { + role: 'assistant', + content: JSON.stringify({ title: 'Guide', acceptance_checks: [calls === 1 ? check(241) : concise], + files_allowed: options.filesAllowed }), + } }] }); + } }); + assert.equal(calls, 2); + const parsed = parseTaskDocument(result.task, { expectedAsk: ask }); + assert.equal(parsed.ask, ask); + assert.deepEqual(parsed.acceptance_checks, [concise]); +});