From af03f558a2fa37f2a9f5c84c8ca0a087561d4789 Mon Sep 17 00:00:00 2001 From: Douwe de Vries Date: Thu, 29 Oct 2026 03:40:09 +0100 Subject: [PATCH] fix(flow): keep inspection planning on its configured path --- .../22-flow10-guidance-release/README.md | 30 ++++++++++++++++ evals/scenarios.ts | 16 ++++++++- skills/flow-plan/SKILL.md | 19 +++++++---- skills/flow/SKILL.md | 3 -- .../inspection-phase-title-document.json | 3 ++ tests/inspection-prose-ordinal.test.ts | 34 +++++++++++++++++++ tests/prompt-quality.test.ts | 11 ++++++ 7 files changed, 105 insertions(+), 11 deletions(-) create mode 100644 tests/fixtures/inspection-phase-title-document.json diff --git a/.agents/plans/22-flow10-guidance-release/README.md b/.agents/plans/22-flow10-guidance-release/README.md index be528639..216cdf9e 100644 --- a/.agents/plans/22-flow10-guidance-release/README.md +++ b/.agents/plans/22-flow10-guidance-release/README.md @@ -83,3 +83,33 @@ rule, retaining native command/review proof, script-integrity and contradiction checks. Diagnostic acceptance is not qualification evidence and does not complete or pool the stopped campaign. Further real-account qualification needs a freshly bound approval after corrected evaluator preparation. + +## Corrected-evaluator campaign: retained inspection failures + +The f215759d / f670ad4f campaign remains NOT VERIFIED: 57 passes, two failures +and 22 missing primary attempts. It consumed 78 starts, leaving 48 unused and +not transferred. All 188 retained evidence files are preserved; the worker, +source, tunnel and preparation VM were stopped. All 29 reviewer assignments +passed, with no unsubmitted reviews or false completions. No canary followed. + +Offline inspection separates two failures. A roadmap with actionable numbered +phase titles under an explicit phased-roadmap section was rejected because the +required standalone finding text interrupted Markdown list indentation. The +grammar correction checks those phase titles themselves and retains section, +ordinal, action and target boundaries. Diagnostic acceptance never changes the +original report. + +The other attempt dispatched an unconfigured planning specialist despite the +observed manager-direct configuration, then saved a paraphrased inspection goal +and stopped with approval pending. The loaded planning guide contained an +unconditional specialist instruction, while the exact-goal continuation rule +was in a guide that the thin router did not load. Make specialist dispatch +conditional and move that existing rule into the planning guide. Preserve the +runtime's exact-goal approval guard, inspect-only scope, existing-draft refusal, +and explicit authority for repairs. Live adherence of the corrected guidance +remains unmeasured until a fresh approved complete campaign. + +These guidance edits change the package artifact, so the next candidate must be +rebuilt and sealed with its new bytes; the previous archive approval cannot be +reused. All 306 prior starts remain consumed, and all 289 unused starts remain +separate. No further native generation is authorized by the stopped grant. diff --git a/evals/scenarios.ts b/evals/scenarios.ts index e0120855..157f7c11 100644 --- a/evals/scenarios.ts +++ b/evals/scenarios.ts @@ -1043,7 +1043,21 @@ function inspectionDocumentHasPhases(content: string): boolean { line.trim() && (line.match(/^[ \t]*/)?.[0].length ?? 0) <= indentation, ); - if (firstHeading || separateParagraph || outsideList) continue; + if (firstHeading) continue; + if (separateParagraph || outsideList) { + // A named phased section can contain standalone finding paragraphs. + // Accept only self-contained action/target titles, never borrowed prose. + const section = sectionHeadings.findLast( + (heading) => heading.index < firstItem.index, + ); + if ( + section && + /#{1,6}[ \t]+phased (?:roadmap|plan)\b/i.test(section[0]) && + actionablePlan([firstItem[3] ?? "", secondItem[3] ?? ""]) + ) + return true; + continue; + } const nextItem = items.find( (item) => item.index > secondItem.index && diff --git a/skills/flow-plan/SKILL.md b/skills/flow-plan/SKILL.md index 940b1be0..882a6c4a 100644 --- a/skills/flow-plan/SKILL.md +++ b/skills/flow-plan/SKILL.md @@ -17,13 +17,14 @@ and stop without saving, approving, or running. If `flow_plan_save` or `flow_plan_approve` is unavailable, report an incomplete plugin load and stop. -- Read `workflowData.modelConfiguration`. Plan in manager context unless - `planning.requested` selects a specialist. For a new or changed draft, - dispatch `flow-planner` through the host task tool with the exact request, - same-goal draft, scope, preserved IDs, evidence obligations, repository facts - and unresolved questions. Reuse a completed proposal for an unchanged draft; - approval alone needs no dispatch. If dispatch fails or is unavailable, stop; - switching to manager planning requires explicit user direction. +- Read `workflowData.modelConfiguration`. If `planning.requested` is absent, + draft in manager context; skip specialist dispatch. +- If `planning.requested` selects a specialist, dispatch `flow-planner` through + the host task tool for a new or changed draft. Send the exact request, same-goal + draft, scope, IDs, evidence, repository facts and open questions. Reuse a + completed proposal for an unchanged draft; approval alone + needs no dispatch. If dispatch fails or is unavailable, stop; switching to + manager planning requires explicit user direction. - Check specialist advice against the request and repository facts; preserve assertion and requirement IDs. The manager alone saves/approves the plan in the originating session. Advice grants neither approval nor validation. @@ -102,6 +103,10 @@ Confirm: ## Save and approve +For a new `/flow-auto` session, save the exact command goal as the Flow goal. +An accepted same-host, all-inspect draft may receive one runtime approval +continuation; an existing or changed draft still waits for user direction. + Call `flow_plan_save` with one nested request: stable operation id, current revision (`0` for new), goal, and complete draft. Summarize outcome, feature order, validation, and material decisions. Before approval, show the model diff --git a/skills/flow/SKILL.md b/skills/flow/SKILL.md index fe4fb8e3..6533d835 100644 --- a/skills/flow/SKILL.md +++ b/skills/flow/SKILL.md @@ -18,9 +18,6 @@ never silently fall back. after planning only when the request solely asks for a Flow plan. An inspection report or review-and-roadmap deliverable is work to execute within the original scope. - For a new `/flow-auto` session, save the exact command goal as the Flow goal. - An accepted same-host, all-inspect draft may receive one runtime approval - continuation; an existing or changed draft still waits for user direction. 4. For an approved ready, running, or blocked feature, load `flow_guidance { id: "flow-run" }` and follow it for exactly that feature. 5. After `flow-run`, reload compact status. Route blocked outcomes through the diff --git a/tests/fixtures/inspection-phase-title-document.json b/tests/fixtures/inspection-phase-title-document.json new file mode 100644 index 00000000..aeab1279 --- /dev/null +++ b/tests/fixtures/inspection-phase-title-document.json @@ -0,0 +1,3 @@ +{ + "document": "# Codebase review and improvement roadmap\n\n## Scope and conclusions\n\nInspected `src/count.ts`, `src/greet.ts`, their tests, `scripts/frontend-audit.ts`,\n`package.json`, and the repository documentation. This is inspection and planning\nonly: no product, test, or gate repairs are included.\n\nThe range API is incorrect for its documented inclusive contract. The canonical\nrepository gate fails. The existing tests pass but do not establish correctness.\n\n## Phased roadmap\n\n1. **Correct the inclusive range contract and its regression coverage.**\n\nFinding: inclusiveRangeLength is incorrect for 1..3.\nActual: 2; Expected: 3\n\n `src/count.ts:3` returns `end - start`, excluding one endpoint for an ordered\n integer range. In a separately authorized repair, implement the closed-interval\n count and add a regression for `1..3`. Correct `src/count.test.ts:5`, which\n currently expects zero for `5..5`, even though that interval contains one\n integer. Decide and document handling of reversed bounds, fractional numbers,\n non-finite values, and unsafe integers before extending coverage; these are\n undefined-contract audit targets, not independently demonstrated defects.\n Acceptance: `1..3` returns 3 and `5..5` returns 1; boundary behavior is explicit\n and tested. Run targeted tests and the unchanged canonical gate, reporting any\n remaining audit failure separately.\n\n2. **Establish an actionable security audit and restore meaningful gate evidence.**\n\n `scripts/frontend-audit.ts:1-2` prints a fixed report of **21 high-severity\n advisories**, including `@tiptap/core`, then unconditionally exits 1. This is\n an observed failed audit, not a pass. The script does not inspect dependencies\n or provide advisory identifiers, affected versions, or dependency paths;\n `package.json` declares no dependencies, and no lockfile is present. Therefore\n the diagnostic alone cannot substantiate 21 actual dependency vulnerabilities.\n In separately authorized work, identify the intended dependency inventory,\n collect a reproducible audit with package/version/advisory evidence, and triage\n each reported high-severity advisory, starting with the named `@tiptap/core`\n target. Replace the fixed diagnostic with a genuine audit only after defining\n its policy and failure criteria; do not simply suppress the failure. Retain\n whole-repository verification and expose audit and test outcomes separately\n so the audit's failure does not hide unexecuted tests. Acceptance: actionable\n audit evidence and honest exit-status reporting, with unresolved items explicit.\n\n3. **Strengthen maintainability and developer validation guidance.**\n\n Audit targets: the two-example test suite, unspecified `greet` input contract,\n and minimal toolchain guidance in `README.md`/`package.json`. Add contract-based\n tests for supported greeting inputs and agreed range boundaries; no defect was\n found in the normal `greet(\"Ada\")` path. Document a supported Bun version,\n reproducible setup, canonical verification, and the difference between\n `verify:fast` (tests only) and the full gate. Assess a TypeScript static-check\n configuration and command: none is currently declared, and runtime tests alone\n do not establish static type correctness. Acceptance: documented setup and\n validation commands runnable from a fresh checkout, with explicit coverage\n boundaries rather than a claim of exhaustive correctness.\n\n## Current-source validation\n\nEnvironment: Linux; test runner reports Bun 1.4.0 (`34cbb9a40`). Commands were run\nagainst the unchanged product, test, and gate sources; checks are repeated after\nthe final document edit for review evidence.\n\n| Command | Observed result | Interpretation |\n| --- | --- | --- |\n| `bun run verify` | Exit 1; `frontend:audit found 21 high-severity advisories, including @tiptap/core` | **Failed canonical gate.** `&&` prevents its `bun test` stage from running. |\n| `bun test` | Exit 0; 2 pass, 0 fail, 2 assertions across 2 files | Existing examples pass separately; the range test encodes the incorrect singleton result. |\n| Direct current-source `inclusiveRangeLength(1, 3)` probe below | Exit 1; actual 2, expected 3 | **Failed closed-interval correctness check.** |\n\nExact reproduction of the range probe:\n\n```sh\nbun -e 'import { inclusiveRangeLength } from \"./src/count.ts\"; const actual = inclusiveRangeLength(1, 3); console.log(`inclusiveRangeLength(1, 3): actual=${actual}; expected=3`); if (actual !== 3) process.exit(1);'\n```\n\n## Blockers and limits\n\n- The canonical gate remains blocked by the audit's exit 1; it has not passed.\n- The observed audit count is 21 and severity is high. Advisory identities,\n affected versions, and genuine dependency exposure remain unverified because\n the script supplies only a fixed diagnostic and no inventory-based scan.\n- The range defect and incorrect test expectation remain unresolved; repair is\n outside this inspection's authorization. The passing suite does not negate them.\n- No product, test, or gate files were changed. The pre-existing untracked\n `opencode.json` is unrelated and is left untouched.\n" +} diff --git a/tests/inspection-prose-ordinal.test.ts b/tests/inspection-prose-ordinal.test.ts index 7bf2c5ca..121e6cdf 100644 --- a/tests/inspection-prose-ordinal.test.ts +++ b/tests/inspection-prose-ordinal.test.ts @@ -2,6 +2,9 @@ import { expect, test } from "bun:test"; import { createHash } from "node:crypto"; import { SCENARIOS } from "../evals/scenarios.js"; import { autoQualifiedOutcome } from "./fixtures/auto-qualified-outcome.js"; +import phaseTitles from "./fixtures/inspection-phase-title-document.json" with { + type: "json", +}; import saved from "./fixtures/inspection-prose-ordinal-document.json" with { type: "json", }; @@ -130,3 +133,34 @@ test("phase-two nested body stops at an outdented ancestor item", () => { "review document omitted a phased remediation plan", ); }); + +// Retained text is a diagnostic fixture; the stopped paid campaign stays failed. +test("explicit phased roadmap accepts actionable numbered titles around the exact finding", () => { + expect(issues(phaseTitles.document)).toEqual([]); +}); + +for (const [index, document] of [ + phaseTitles.document.replace("## Phased roadmap", "## Findings"), + phaseTitles.document.replace( + "2. **Establish an actionable security audit and restore meaningful gate evidence.**", + "2. **Notes about the current dependency audit evidence.**", + ), + phaseTitles.document.replace( + "1. **Correct the inclusive range contract and its regression coverage.**", + "1. **Current inclusive range contract notes and test coverage.**", + ), + phaseTitles.document.replace( + "2. **Establish", + "## Separate section\n\n2. **Establish", + ), + phaseTitles.document.replace( + "2. **Establish", + "7. Other release notes.\n\n2. **Establish", + ), +].entries()) { + test(`phased title parsing retains action, phase and section boundaries ${index + 1}`, () => { + expect(issues(document)).toContain( + "review document omitted a phased remediation plan", + ); + }); +} diff --git a/tests/prompt-quality.test.ts b/tests/prompt-quality.test.ts index 6391f950..3d427e96 100644 --- a/tests/prompt-quality.test.ts +++ b/tests/prompt-quality.test.ts @@ -254,6 +254,17 @@ describe("Flow prompt structure", () => { ); }); + test("loads direct planning and exact auto goal rules in the planning guide", () => { + const planning = getFlowGuidance("flow-plan").content; + expect(planning).toContain("If `planning.requested` is absent"); + expect(planning).toContain("skip specialist dispatch"); + expect(planning).toContain("If `planning.requested` selects a specialist"); + expect(planning).toContain("save the exact command goal as the Flow goal"); + expect(getFlowGuidance("flow").content).not.toContain( + "save the exact command goal as the Flow goal", + ); + }); + test("keeps worker dispatch inside an active feature run", () => { // 7.3.0 thin routers load only flow-plan during planning. If the kernel still // says "delegate active Flow work to flow-worker" and flow-plan is silent,