From 3d6ec666c26324a403ab5e04a8d324d6ae970942 Mon Sep 17 00:00:00 2001 From: cookerpapa Date: Thu, 10 Sep 2026 15:32:26 +0800 Subject: [PATCH] fix(runner): reject tasks without parseable steps --- docs/reference/status-format.md | 6 + docs/reference/task-format.md | 4 + extensions/taskplane/lane-runner.ts | 49 ++++- .../tests/lane-runner-empty-steps.test.ts | 207 ++++++++++++++++++ 4 files changed, 258 insertions(+), 8 deletions(-) create mode 100644 extensions/tests/lane-runner-empty-steps.test.ts diff --git a/docs/reference/status-format.md b/docs/reference/status-format.md index 7766a087..ef97bfe6 100644 --- a/docs/reference/status-format.md +++ b/docs/reference/status-format.md @@ -106,6 +106,12 @@ Generated file includes: Orchestrator and runner use `.DONE` as the authoritative completion marker. +If Runtime V2 finds no parseable steps in `PROMPT.md`, it records `❌ Failed` and +an actionable error in the execution log instead of marking the task complete. +It quarantines any existing `.DONE` as `.DONE.unauthorized-` and does +not create a new completion marker. Existing step progress remains available +after the prompt is corrected. + --- ## Editing guidelines diff --git a/docs/reference/task-format.md b/docs/reference/task-format.md index f305a5c3..aa6e593d 100644 --- a/docs/reference/task-format.md +++ b/docs/reference/task-format.md @@ -65,6 +65,10 @@ Canonical folder: - At least one step in `### Step N: ...` format - Checkbox items (`- [ ]`) inside steps +Runtime V2 rejects a task with no parseable steps before launching a worker. +For example, `## Step N: ...` headings do not satisfy the required `### Step N: ...` format. +Correct the headings before retrying the task. + ### Optional but strongly recommended - `## Review Level: N` diff --git a/extensions/taskplane/lane-runner.ts b/extensions/taskplane/lane-runner.ts index 00bbb243..a0e249ec 100644 --- a/extensions/taskplane/lane-runner.ts +++ b/extensions/taskplane/lane-runner.ts @@ -1100,11 +1100,46 @@ export async function executeTaskV2( } } - // ── 1. Ensure STATUS.md exists ────────────────────────────────── - if (!existsSync(statusPath)) { + let totalIterations = 0; + let cumulativeCostUsd = 0; + let cumulativeTokens = 0; + // TP-115: carry latest worker telemetry into terminal snapshots, including invalid tasks. + let lastTelemetry: Partial = {}; + + const invalidPromptResult = (): LaneRunnerTaskResult => { + const reason = + "Invalid PROMPT.md: no parseable steps. Expected at least one '### Step N: ...' heading."; + quarantineUnauthorizedDone(reason); + updateStatusField(statusPath, "Status", "❌ Failed"); + updateStatusField(statusPath, "Last Updated", new Date().toISOString().slice(0, 10)); + logExecution(statusPath, "Invalid task", reason); + return makeResult( + taskId, + segmentId, + workerAgentId, + "failed", + startTime, + reason, + false, + totalIterations, + cumulativeCostUsd, + cumulativeTokens, + config, + statusPath, + reviewerStatePath, + lastTelemetry, + ); + }; + + // ── 1. Ensure STATUS.md exists and validate the task ───────────── + { const content = readFileSync(promptPath, "utf-8"); const parsed = parsePromptMd(content, promptPath); - writeFileSync(statusPath, generateStatusMd(parsed)); + if (!existsSync(statusPath)) { + writeFileSync(statusPath, generateStatusMd(parsed)); + } + // An empty step list is an invalid task, not an already-completed task (#614). + if (parsed.steps.length === 0) return invalidPromptResult(); } updateStatusField(statusPath, "Status", "🟡 In Progress"); @@ -1151,11 +1186,6 @@ export async function executeTaskV2( let rulingsInFlight: HoldRecord[] = []; /** #629: gates the CURRENT iteration was spawned to remediate (empty = normal iteration). */ let remediationGates: BlockingReviewGate[] = []; - let totalIterations = 0; - let cumulativeCostUsd = 0; - let cumulativeTokens = 0; - // TP-115: carry latest worker telemetry across iterations and into post-loop terminal snapshots - let lastTelemetry: Partial = {}; // TP-174: Build segment context once for emitSnapshot calls. // Available outside the loop so it can be passed to makeResult too. @@ -1533,6 +1563,7 @@ export async function executeTaskV2( // Determine remaining steps const currentStatus = parseStatusMd(readFileSync(statusPath, "utf-8")); const parsed = parsePromptMd(readFileSync(promptPath, "utf-8"), promptPath); + if (parsed.steps.length === 0) return invalidPromptResult(); // TP-174: Resolve segment-scoped step filtering. // Use config.repoId (structured identity) instead of parsing opaque segmentId. @@ -2769,6 +2800,8 @@ export async function executeTaskV2( const finalStatusContent = readFileSync(statusPath, "utf-8"); const finalStatus = parseStatusMd(finalStatusContent); const parsed = parsePromptMd(readFileSync(promptPath, "utf-8"), promptPath); + // A worker can edit PROMPT.md, including on the last permitted iteration. + if (parsed.steps.length === 0) return invalidPromptResult(); // TP-174: Segment-scoped post-loop check. Re-derive repo scoping since // the iteration loop variables are out of scope here. diff --git a/extensions/tests/lane-runner-empty-steps.test.ts b/extensions/tests/lane-runner-empty-steps.test.ts new file mode 100644 index 00000000..6a555c3e --- /dev/null +++ b/extensions/tests/lane-runner-empty-steps.test.ts @@ -0,0 +1,207 @@ +import assert from "node:assert/strict"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, it, mock } from "node:test"; + +import type { AgentHostResult } from "../taskplane/agent-host.ts"; +import type { LaneRunnerConfig } from "../taskplane/lane-runner.ts"; +import type { ExecutionUnit } from "../taskplane/types.ts"; +import { readLaneSnapshot } from "../taskplane/process-registry.ts"; +import { generateStatusMd, parsePromptMd } from "../taskplane/task-executor-core.ts"; +import { resolvePacketPaths } from "../taskplane/types.ts"; + +let onSpawn: (() => void) | undefined; +const workerResult: AgentHostResult = { + exitCode: 0, + signal: null, + durationMs: 500, + killed: false, + inputTokens: 5, + outputTokens: 5, + cacheReadTokens: 0, + cacheWriteTokens: 0, + costUsd: 0.01, + toolCalls: 1, + lastTool: "edit", + retries: 0, + compactions: 0, + contextUsage: null, + error: null, + agentEnded: true, + stderrTail: "", +}; +const realAgentHost = await import("../taskplane/agent-host.ts"); +const spawnAgent = mock.fn(() => { + onSpawn?.(); + return { promise: Promise.resolve(workerResult), kill: () => {} }; +}); +mock.module("../taskplane/agent-host.ts", { + namedExports: { ...realAgentHost, spawnAgent }, +}); +const { executeTaskV2 } = await import("../taskplane/lane-runner.ts"); + +const VALID_PROMPT = `# Task: TP-614 — Empty steps regression + +## Review Level: 0 + +## Steps + +### Step 0: Implement +- [ ] Do the work +`; +const INVALID_PROMPTS = [ + ["h2 step headings", VALID_PROMPT.replace("### Step", "## Step")], + ["an empty prompt", ""], + [ + "a mission without steps", + "# Task: TP-614 — Empty steps regression\n\n## Mission\nDo the work.\n", + ], +]; + +describe("Runtime V2 rejects tasks without parseable steps (#614)", () => { + let root: string; + let unit: ExecutionUnit; + let config: LaneRunnerConfig; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "taskplane-empty-steps-")); + const taskFolder = join(root, "task"); + const worktreePath = join(root, "worktree"); + mkdirSync(taskFolder); + mkdirSync(worktreePath); + const packet = resolvePacketPaths(taskFolder); + unit = { + id: "TP-614", + taskId: "TP-614", + segmentId: null, + executionRepoId: "default", + packetHomeRepoId: "default", + worktreePath, + packet, + task: { + taskId: "TP-614", + taskName: "Empty steps regression", + reviewLevel: 0, + size: "S", + dependencies: [], + fileScope: [], + taskFolder, + promptPath: packet.promptPath, + areaName: "test", + status: "pending", + }, + }; + config = { + batchId: "empty-steps", + agentIdPrefix: "orch-test", + laneNumber: 1, + worktreePath, + branch: "test-branch", + repoId: "default", + stateRoot: root, + workerModel: "", + workerTools: "", + workerThinking: "", + workerSystemPrompt: "", + workerSegmentPrompt: "", + reviewerModel: "", + reviewerThinking: "", + reviewerTools: "", + maxIterations: 2, + noProgressLimit: 2, + maxWorkerMinutes: 5, + warnPercent: 80, + killPercent: 95, + }; + spawnAgent.mock.resetCalls(); + onSpawn = undefined; + }); + + afterEach(() => rmSync(root, { recursive: true, force: true })); + + function writeStatus(complete = false): void { + const status = generateStatusMd(parsePromptMd(VALID_PROMPT, unit.packet.promptPath)); + writeFileSync(unit.packet.statusPath, complete ? status.replaceAll("- [ ]", "- [x]") : status); + } + + function assertFailed(result: Awaited>): void { + assert.equal(result.outcome.status, "failed"); + assert.match(result.outcome.exitReason ?? "", /no parseable steps/i); + assert.match(result.outcome.exitReason ?? "", /### Step N:/); + assert.equal(result.outcome.doneFileFound, false); + assert.equal(existsSync(unit.packet.donePath), false); + const status = readFileSync(unit.packet.statusPath, "utf-8"); + assert.match(status, /\*\*Status:\*\* ❌ Failed/); + assert.match(status, /no parseable steps/i); + assert.equal(readLaneSnapshot(root, config.batchId, config.laneNumber)?.status, "failed"); + } + + for (const [name, prompt] of INVALID_PROMPTS) { + for (const existingStatus of [false, true]) { + it(`rejects ${name} ${existingStatus ? "with" : "without"} an existing STATUS.md`, async () => { + writeFileSync(unit.packet.promptPath, prompt); + if (existingStatus) writeStatus(); + const result = await executeTaskV2(unit, config, { paused: false }); + assertFailed(result); + assert.equal(spawnAgent.mock.callCount(), 0); + assert.equal(result.iterations, 0); + assert.equal(result.costUsd, 0); + assert.equal(result.totalTokens, 0); + if (existingStatus) { + assert.match(readFileSync(unit.packet.statusPath, "utf-8"), /- \[ \] Do the work/); + } + }); + } + } + + it("quarantines an existing completion marker for an invalid task", async () => { + writeFileSync(unit.packet.promptPath, INVALID_PROMPTS[0][1]); + writeStatus(true); + writeFileSync(unit.packet.donePath, "previous false completion"); + assertFailed(await executeTaskV2(unit, config, { paused: false })); + const quarantined = readdirSync(unit.packet.taskFolder).find((f) => + f.startsWith(".DONE.unauthorized-"), + ); + assert.ok(quarantined); + assert.equal( + readFileSync(join(unit.packet.taskFolder, quarantined), "utf-8"), + "previous false completion", + ); + assert.equal(spawnAgent.mock.callCount(), 0); + }); + + for (const maxIterations of [1, 2]) { + it(`rejects steps removed by a worker with an iteration budget of ${maxIterations}`, async () => { + writeFileSync(unit.packet.promptPath, VALID_PROMPT); + writeStatus(); + config.maxIterations = maxIterations; + onSpawn = () => writeFileSync(unit.packet.promptPath, INVALID_PROMPTS[0][1]); + const result = await executeTaskV2(unit, config, { paused: false }); + assertFailed(result); + assert.equal(spawnAgent.mock.callCount(), 1); + assert.equal(result.iterations, 1); + assert.equal(result.costUsd, workerResult.costUsd); + assert.equal(result.totalTokens, 10); + }); + } + + it("still completes a valid task whose steps were finished before resuming", async () => { + writeFileSync(unit.packet.promptPath, VALID_PROMPT); + writeStatus(true); + const result = await executeTaskV2(unit, config, { paused: false }); + assert.equal(result.outcome.status, "succeeded"); + assert.equal(result.outcome.doneFileFound, true); + assert.equal(existsSync(unit.packet.donePath), true); + assert.equal(spawnAgent.mock.callCount(), 0); + assert.equal(result.iterations, 0); + }); +});