From 8a293f79a3933767ef6fc4afc1225fd842940106 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 19 Aug 2026 16:45:32 +0000 Subject: [PATCH 1/7] Plan a derived findings digest for /flow-auto review UX. Codebase-review goals currently invent a change DAG, stall on the first blocker, and only emit a findings list after close. This plan derives a digest from existing runs, shows it at every handback, and defers an inspect kind until evals show the first eight phases still leak. Co-authored-by: vriesd --- .agents/plans/01-review-workflow/overview.md | 152 ++++++++++++++++++ .../phase-1-findings-digest.md | 38 +++++ .../phase-2-compact-digest.md | 39 +++++ .../phase-3-delivery-digest.md | 36 +++++ .../phase-4-checkpoint-utterance.md | 40 +++++ .../phase-5-autodrive-handback.md | 41 +++++ .../phase-6-reviewer-budget.md | 40 +++++ .../phase-7-inspect-planning.md | 38 +++++ .../plans/01-review-workflow/phase-8-eval.md | 39 +++++ .../phase-9-inspect-kind.md | 43 +++++ .agents/plans/01-review-workflow/testing.md | 34 ++++ 11 files changed, 540 insertions(+) create mode 100644 .agents/plans/01-review-workflow/overview.md create mode 100644 .agents/plans/01-review-workflow/phase-1-findings-digest.md create mode 100644 .agents/plans/01-review-workflow/phase-2-compact-digest.md create mode 100644 .agents/plans/01-review-workflow/phase-3-delivery-digest.md create mode 100644 .agents/plans/01-review-workflow/phase-4-checkpoint-utterance.md create mode 100644 .agents/plans/01-review-workflow/phase-5-autodrive-handback.md create mode 100644 .agents/plans/01-review-workflow/phase-6-reviewer-budget.md create mode 100644 .agents/plans/01-review-workflow/phase-7-inspect-planning.md create mode 100644 .agents/plans/01-review-workflow/phase-8-eval.md create mode 100644 .agents/plans/01-review-workflow/phase-9-inspect-kind.md create mode 100644 .agents/plans/01-review-workflow/testing.md diff --git a/.agents/plans/01-review-workflow/overview.md b/.agents/plans/01-review-workflow/overview.md new file mode 100644 index 0000000..71cd1f4 --- /dev/null +++ b/.agents/plans/01-review-workflow/overview.md @@ -0,0 +1,152 @@ +# Review-workflow UX + +Back-link. This directory is the implementation plan. Do not start until the +user says to. + +## Context + +A user ran `/flow-auto` on the newest Flow (8.0.0) with a codebase-review +goal. The run felt too thorough. Blockers stopped it before the rest of the +survey finished. At the end there was no clear list of findings. + +That is the architecture, not a missed prompt. Flow's aggregate is a change +lifecycle. A finding exists so the next attempt can prove a repair. Compact +status carries routing flags and no finding text. Auto-drive continues only +`ready` plus `flow_run_start`, and `completed` or `closed` plus +`flow_session_close`. The one runtime-owned list is `delivery.report`, and +`deliveryProjection` throws unless a closure is already recorded. `/flow-status` +is the only compiled prompt that dumps findings at a checkpoint. `/flow-auto` +does not load it. + +Four independent critiques agreed on that map. ADR 0005 already deleted the +audit ledger. This plan does not bring it back. It derives a findings digest +from `session.runs` that already exist, shows that digest whenever Flow hands +control back, and stops the planner from turning an inspect goal into a fake +implementation DAG. + +## Scope + +Included. + +- A derived findings digest over every attempt of every feature, not only the + last passing review. +- That digest on compact status and on close delivery. +- A checkpoint and lease-stop utterance that pastes the digest. +- Auto-drive behavior so a first allowed reset or a blocked wait cannot go + silent. +- Planner and reviewer prompt cuts so inspect-shaped goals do not invent + fixes or run an unbounded change checklist. +- An eval that fails when `/flow-auto` on an inspect goal produces no + user-visible findings list. + +Excluded. + +- Restoring `flow_audit_render`, `audit-ledger.ts`, or any persisted findings + ledger. +- A persisted intent classifier. +- A new required-at-save plan field. `tests/documentation-contract.test.ts` + pins today's optional plan fields until a major. +- Changing `adjacent-defect-refused`. Change reviews must still catch a + planted out-of-scope defect. +- Making Flow a team orchestrator or a multi-repo auditor. + +## Constraints + +Session v5 stays the only active schema. New complexity must remove or replace +an existing concept, or be a derived projection of canonical run data (ADR +0005). Auto-drive stays process-local and fail-closed (ADR 0008). Prompt +bytes stay under `MAX_TOTAL_PROMPT_BYTES` in `tests/prompt-quality.test.ts`. +Maintained docs stay under the prose budget in +`tests/documentation-contract.test.ts`. Put this plan under `.agents/plans/` +so it does not spend that budget. + +A new required plan field is a major. An optional compact field that is +derived at projection time is a widening of an existing view, the same kind +as `blockedFeature.scopeBlocker`. + +## Alternatives + +1. **Prompt-only.** Tell `/flow-auto` to read detail and narrate findings. + `/flow-status` already does this. Models following `/flow-auto` still go + mute. Rejected. Routing that a model must spot in prose is the pattern + `scopeBlocker` replaced. + +2. **Restore the audit ledger.** A second durable document next to Session v5. + ADR 0005 named that subtraction. Rejected. + +3. **Derived digest plus handback paste, then a later inspect kind.** Build + the report from runs that already exist. Bind it to "Flow is handing + control back", not only to close. Keep inspect-as-a-kind for a major if + evals still show the planner inventing a fix DAG. Chosen. It matches + `deliveryProjection` (derived, not persisted) and does not need a new + ledger. + +## Applicable skills + +- `how` before each unfamiliar subsystem (`session-projection`, `auto-drive`, + eval harness). +- Cursor `create-skill` for any SKILL.md edit. +- `unslop` and `/deslop` on every prose and diff. +- `flow-contribution-check` before commit and push. +- `show-me-your-work` if a phase amends an ADR. + +## Phases + +1. [Derived findings digest](phase-1-findings-digest.md) +2. [Compact digest field](phase-2-compact-digest.md) +3. [Delivery uses the same digest](phase-3-delivery-digest.md) +4. [Checkpoint paste](phase-4-checkpoint-utterance.md) +5. [Auto-drive handback](phase-5-autodrive-handback.md) +6. [Reviewer budget](phase-6-reviewer-budget.md) +7. [Inspect-shaped planning](phase-7-inspect-planning.md) +8. [Eval](phase-8-eval.md) +9. [Inspect kind, major, only if phases 1 to 8 leak](phase-9-inspect-kind.md) + +[Verification commands](testing.md). + +## Verification + +Project-level. + +```bash +bun run check +bun run replay +``` + +After phase 8 also run the new scenario's unit checks in +`tests/eval-scenario-checks.test.ts`. Paid matrix is not required to land a +phase. It is required before promoting an inspect kind. + +## Implementation guidance + +- Run the **how** skill over `session-projection`, `auto-drive`, and the eval + harness before changing them. +- Run **interrogate** before phase 9. Phases 1 to 8 are not a contested design + once this overview is accepted. Phase 9 is. +- `/deslop` each diff. **unslop** every skill, ADR, and plan edit. +- **show-me-your-work** for phase 5 (ADR 0008) and phase 9 (ADR 0005). +- Cursor **babysit** after the PR that lands each phase. This plan PR is + documents only. + +## Lead judgment + +Four critics. Act on the shared structural claims. Do not restore deleted +ledgers. + +**Act on.** Findings are retry state, not a report. Compact has no finding +text. Delivery exists only after close and then usually prints `terminal +findings: none` because a pass cannot carry blockers. Auto-drive parks or +dies at the moment a survey has something to say. The planner has no inspect +shape, so it invents a change DAG. The reviewer is measured for misses, not +for a readable list. + +**Consider.** Making `flow_feature_reset` mechanical, a reviewer step cap, and +an inspect plan kind. Those wait on phases 1 to 8 and on evals. + +**Noted.** Untyped `nextAction` strings in auto-drive. Cross-cutting findings +cannot reopen a passed feature. Positioning.md never says "do not use Flow +to audit a tree." + +**Dismissed.** Prompt-only dumps. Rebuilding `audit-ledger.ts`. A persisted +intent classifier. Shipping "just don't use Flow for reviews" as the whole +product answer while `/flow-auto` still accepts that goal. diff --git a/.agents/plans/01-review-workflow/phase-1-findings-digest.md b/.agents/plans/01-review-workflow/phase-1-findings-digest.md new file mode 100644 index 0000000..3f9f1f6 --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-1-findings-digest.md @@ -0,0 +1,38 @@ +# Phase 1. Derived findings digest + +Back-link. [Overview](overview.md). + +## Goal + +One pure function turns a `Session` into the findings list a human can read. +It walks every run of every planned feature. It does not persist. It does not +require a closure. + +## Changes + +- Add `src/application/findings-digest.ts`. Export `findingsDigest(session)`. +- Add `tests/findings-digest.test.ts`. Cover a failed then passing retry, a + deferred close of a blocked run, an untouched feature, and a pass that omits + prior ids. + +Do not wire the function into compact or delivery yet. + +## Data structures + +`FindingsDigest` is a readonly array of rows. + +Each row. `featureId`, `findingId`, `severity`, `summary`, `evidence` if +present, `attempt`, `verdict` of the review that last stated it, `live` +boolean. + +`live` is true when `livePriorFindings` still holds that id for the feature. +Historical blockers that a later pass dropped stay in the digest with +`live: false`. That is the survey record the last-review strip throws away. + +## Verification + +**Static.** `bun test tests/findings-digest.test.ts`. `bun run typecheck`. + +**Runtime.** No host surface yet. The unit tests are the check. Fixture a +session with two attempts, first failed with F1 blocking, second passed with +no findings. Digest must still list F1 with `live: false`. diff --git a/.agents/plans/01-review-workflow/phase-2-compact-digest.md b/.agents/plans/01-review-workflow/phase-2-compact-digest.md new file mode 100644 index 0000000..0456140 --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-2-compact-digest.md @@ -0,0 +1,39 @@ +# Phase 2. Compact digest field + +Back-link. [Overview](overview.md). + +## Goal + +Compact status, the view `/flow-auto` and auto-drive actually read, carries +the digest. A checkpoint no longer requires a detail dump to know what was +found. + +## Changes + +- `src/application/session-projection.ts`. Add `findingsDigest` to + `CompactProjection`. Populate it from `findingsDigest(session)`. +- `tests/runtime-gates.test.ts`. Assert the field on a first failed review + and on a scope-blocker checkpoint. +- Public compact shape. Update `docs/maintainer-contract.md` and any compact + inventory test if the contract lists compact keys. Do not add a second + status view. + +Empty digest is `[]`, never omitted as a missing key, so callers can branch +on length. + +## Data structures + +`CompactProjection.findingsDigest` uses the phase 1 type. No Session v5 +field. Derived at project time, same family as `blockedFeature` and +`nextAction`. + +## Verification + +**Static.** `bun test tests/runtime-gates.test.ts tests/findings-digest.test.ts`. +`bun run typecheck`. Documentation contract if the maintainer contract +changed. + +**Runtime.** No OpenCode session required. The compact fixture is the +surface. After a failed `flow_feature_complete` in the in-memory harness, +`flow_status { view: "compact" }` returns the blocking summary in +`findingsDigest`. diff --git a/.agents/plans/01-review-workflow/phase-3-delivery-digest.md b/.agents/plans/01-review-workflow/phase-3-delivery-digest.md new file mode 100644 index 0000000..e69b3a5 --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-3-delivery-digest.md @@ -0,0 +1,36 @@ +# Phase 3. Delivery uses the same digest + +Back-link. [Overview](overview.md). + +## Goal + +Close delivery stops pretending the last passing review is the survey. +`formatReport` prints the phase 1 digest. Historical blockers survive a +later pass. + +## Changes + +- `src/application/delivery.ts`. `DeliveryProjection` includes the same + digest. `formatReport` lists live rows first, then historical rows. Keep + per-feature attempt and latest state lines. +- `tests/runtime-close.test.ts`. A completed close after a failed then + passing retry must not print only `terminal findings: none`. A deferred + close of a blocked run must list the blockers. Untouched features stay + empty. + +Do not persist delivery. Recompute from the closed session, as today. + +## Data structures + +Reuse `FindingsDigest`. Drop the per-feature `terminalFindings` strip if +every caller can read the digest. If a test still names +`terminalFindings`, keep it as a view over live rows of that feature so +this phase stays two files plus tests, not a caller migration. + +## Verification + +**Static.** `bun test tests/runtime-close.test.ts`. `bun run typecheck`. + +**Runtime.** In-memory close harness is the surface. Completed close after +retry. Deferred close while blocked. Both reports include the blocking +summary from the failed attempt. diff --git a/.agents/plans/01-review-workflow/phase-4-checkpoint-utterance.md b/.agents/plans/01-review-workflow/phase-4-checkpoint-utterance.md new file mode 100644 index 0000000..68a706c --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-4-checkpoint-utterance.md @@ -0,0 +1,40 @@ +# Phase 4. Checkpoint paste + +Back-link. [Overview](overview.md). + +## Goal + +When Flow hands control back, the manager pastes the compact digest. This +is a typed field, not a marker in prose. + +## Changes + +- `skills/flow-run/SKILL.md` **Blocked review**. After the one detail read + for routing, print compact `findingsDigest` as the user-facing list, then + checkpoint or reset as today. +- `src/guidance/catalog.ts` `FLOW_MANAGER_KERNEL`. One short line. On + `await-user-direction` or when the lease will stop, report + `findingsDigest` from compact status. Do not invent ids. +- `skills/flow/SKILL.md` Recovery. Same paste rule for checkpoints, not + only for `delivery.report`. +- `tests/prompt-quality.test.ts`. Lock that `/flow-auto` and `/flow-run` + name `findingsDigest`. Do not reintroduce bracket markers. + +Stay inside the prompt byte ceiling. Pay for the kernel line by cutting a +sentence that restates retry policy already on `nextAction`. + +Use Cursor `create-skill` for the SKILL.md edits. + +## Data structures + +None. The compact field from phase 2 is the payload. + +## Verification + +**Static.** `bun test tests/prompt-quality.test.ts`. Prompt byte ceiling. +`bun run check` if skills changed. + +**Runtime.** No control-cli for OpenCode chat in this repo. Replay +`adjacent-defect-refused` still fails a silent pass. Read the compiled +`flow-run` and `flow-auto` surfaces and confirm they name `findingsDigest` +and still omit routing markers. diff --git a/.agents/plans/01-review-workflow/phase-5-autodrive-handback.md b/.agents/plans/01-review-workflow/phase-5-autodrive-handback.md new file mode 100644 index 0000000..4c215b8 --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-5-autodrive-handback.md @@ -0,0 +1,41 @@ +# Phase 5. Auto-drive handback + +Back-link. [Overview](overview.md). + +## Goal + +A blocked or resetting `/flow-auto` does not go silent. The lease either +prompts one handback turn that pastes the digest, or it warns before it +dies. + +## Changes + +- `src/platform/opencode/auto-drive.ts`. When `onIdle` would deactivate on + a non-mechanical projection that is blocked, `flow_feature_reset`, or + `dispatch-flow-reviewer`, prompt once. The prompt says to call compact + `flow_status`, print `findingsDigest`, then follow `nextAction` or stop + at `await-user-direction`. After that prompt, park or stop as now. Do + not auto-reset. Do not auto-approve. +- `tests/auto-drive.test.ts`. First failed review at idle produces that + prompt. A second idle at the same revision does not loop. Scope-blocker + checkpoint still waits, but the first park is preceded by the handback + prompt if the manager has not already spoken on that revision. +- `docs/adr/0008-bounded-auto-continuation.md`. Record the extra prompt as + a conversational handback, not a new mechanical route. Keep start and + close as the only mechanical continuations. + +Do not add a status view. Compact already has the digest after phase 2. + +## Data structures + +Lease may hold `handbackPromptedRevision: number | null` so the one +handback cannot loop. Process-local, like the rest of the lease. Not +Session v5. + +## Verification + +**Static.** `bun test tests/auto-drive.test.ts`. Typecheck. + +**Runtime.** The auto-drive harness in that test file is the surface. Idle +at `blocked` plus `flow_feature_reset` used to deactivate with no prompt. +It must prompt once, then not prompt again at the same revision. diff --git a/.agents/plans/01-review-workflow/phase-6-reviewer-budget.md b/.agents/plans/01-review-workflow/phase-6-reviewer-budget.md new file mode 100644 index 0000000..20fae53 --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-6-reviewer-budget.md @@ -0,0 +1,40 @@ +# Phase 6. Reviewer budget + +Back-link. [Overview](overview.md). + +## Goal + +Ordinary feature review stays a change gate. It does not run an unbounded +concurrency matrix unless the packet asked for one. Uncertainty stays +advisory. `adjacent-defect-refused` still fails a rubber-stamp pass. + +## Changes + +- `skills/flow-review/SKILL.md`. Continue the matrix after one blocker only + when `assignment.packet.riskLenses` is non-empty or the feature packet + summary includes a matrix. Otherwise inspect the changed artifacts, the + supplied validation, and live `priorFindings`. Still report every + problem. Still fail unprovable outcome claims. +- `tests/prompt-quality.test.ts`. Lock the conditional matrix sentence. Do + not lower the reviewer absolute-rule budget in a way that drops + `scopeBlocker` or `findingId`. + +Use Cursor `create-skill`. Do not set a default `OPENCODE_FLOW_REVIEWER_STEPS` +in this phase. That is host policy and fights evals until phase 8 has a +baseline. + +## Data structures + +None. Packet remains `{ summary, riskLenses }`. The runtime still does not +parse a matrix. + +## Verification + +**Static.** `bun test tests/prompt-quality.test.ts`. Replay +`evals/cassettes` that pin `flow_feature_complete` on +`adjacent-defect-refused`. + +**Runtime.** Cassette replay is the surface. A passing review of the +planted adjacent defect must still fail the scenario. The skill must no +longer tell every reviewer to finish a matrix that the packet did not +supply. diff --git a/.agents/plans/01-review-workflow/phase-7-inspect-planning.md b/.agents/plans/01-review-workflow/phase-7-inspect-planning.md new file mode 100644 index 0000000..7e2368e --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-7-inspect-planning.md @@ -0,0 +1,38 @@ +# Phase 7. Inspect-shaped planning + +Back-link. [Overview](overview.md). + +## Goal + +A goal that asks for findings and no code change does not become a fake +implementation DAG. The planner either saves inspect slices that promise +no edits, or it stops and says Flow will treat the request as a change. + +## Changes + +- `skills/flow-plan/SKILL.md`. If the request is inspect-only (review, + audit, survey, no promised edit), do not invent repair features. Save + at most a small set of inspect features whose `validation` is reviewer + inspection, whose `targets` are existing paths, and whose `decisions` + state that no source edit is authorized. The gate may be the repo's + existing check. Ask before turning an inspect request into repairs. +- `docs/positioning.md`. One "Do not use" bullet is not enough by itself. + Add one sentence under Use or Do not use. Flow's independent review is a + gate on a change. A codebase survey needs the inspect path in this plan, + or an ordinary non-Flow chat. Stay inside the maintained-docs byte + budget. Pay with a deletion if needed. + +Use Cursor `create-skill`. No new plan schema field. + +## Data structures + +None. Inspect is a planning convention in `decisions` and feature +summaries, not a stored kind. Phase 9 adds a kind only if this leaks. + +## Verification + +**Static.** `bun test tests/prompt-quality.test.ts tests/documentation-contract.test.ts`. + +**Runtime.** No OpenCode control skill in-repo. Phase 8's scenario is the +behavior check. Until then, read the compiled `flow-plan` surface and +confirm it forbids repair features for inspect-only requests. diff --git a/.agents/plans/01-review-workflow/phase-8-eval.md b/.agents/plans/01-review-workflow/phase-8-eval.md new file mode 100644 index 0000000..8f29695 --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-8-eval.md @@ -0,0 +1,39 @@ +# Phase 8. Eval + +Back-link. [Overview](overview.md). + +## Goal + +A scenario fails when `/flow-auto` on an inspect goal ends with no +user-visible findings list. The suite starts measuring the workflow the +user actually ran. + +## Changes + +- `evals/scenarios.ts`. Add `inspect-goal-delivers-findings`. Fixture a + small repo with one planted, observable defect. Command is `/flow-auto` + asking for a review of that area, no implementation authority to fix it. + The check fails if `finalText` and any close `delivery.report` both omit + the defect, and if compact `findingsDigest` is empty at stop. Asking the + user how to close after listing the finding is an accepted end. +- `tests/eval-scenario-checks.test.ts`. Unit-test that check against + synthetic outcomes. Pass with a digest that names the plant. Fail with + a completed close and `terminal findings: none`. Fail with a checkpoint + and empty compact digest. +- `evals/README.md`. One row in the scenario table. Ungated until a + matrix exists, same pattern as other new scenarios. + +Do not weaken `adjacent-defect-refused`. + +## Data structures + +Reuse compact `findingsDigest` and `delivery.report`. The grader reads +those plus `finalText`. No new Session field. + +## Verification + +**Static.** `bun test tests/eval-scenario-checks.test.ts`. `bun run check`. + +**Runtime.** The scenario check function on fixtures is the surface. A +paid cassette can wait. Land a hand-written cassette only if replay needs +one to pin a refusal, as `plan-only-stops` does for workers. diff --git a/.agents/plans/01-review-workflow/phase-9-inspect-kind.md b/.agents/plans/01-review-workflow/phase-9-inspect-kind.md new file mode 100644 index 0000000..9472ef3 --- /dev/null +++ b/.agents/plans/01-review-workflow/phase-9-inspect-kind.md @@ -0,0 +1,43 @@ +# Phase 9. Inspect kind + +Back-link. [Overview](overview.md). + +## Goal + +Only if phases 1 to 8 still leak in evals. A stored inspect kind so a blocking +finding can be recorded without stopping the rest of the survey, without a +second ledger. + +## Changes + +This phase is a major. It replaces "every feature is an implementation +slice" with "a feature is an outcome slice that is `change` or `inspect`." +Default hydrate to `change` so old Session v5 documents keep today's +rules. + +Expect edits in `src/domain/session.ts`, `src/application/schema.ts`, +`src/domain/transitions.ts`, compact `nextAction`, reviewer skill, ADR +0005, and `tests/documentation-contract.test.ts` (optional plan fields). + +Do not start this phase in the same PR as 1 to 8. Interrogate first. +`completed` for inspect means the survey finished, not that the tree is +clean. Blocking findings on inspect features stay in the digest and do +not force `flow_feature_reset` unless the user asked to fix them. + +## Data structures + +`PlanFeature.kind?: "change" | "inspect"`. Absent means `change`. That is +a new optional plan field and a documented major, even if save does not +require it, because `documentation-contract` currently pins the optional +set to `evidence` only. + +## Verification + +**Static.** Full `bun run check`. Documentation contract. Schema pin test +updated on purpose. + +**Runtime.** Phase 8 scenario plus a second feature after a blocking +inspect finding must still be startable without reset. Paid matrix before +release. + +Skip this phase if phase 8 stays green on inspect goals with phases 1 to 8 alone. diff --git a/.agents/plans/01-review-workflow/testing.md b/.agents/plans/01-review-workflow/testing.md new file mode 100644 index 0000000..4c260b4 --- /dev/null +++ b/.agents/plans/01-review-workflow/testing.md @@ -0,0 +1,34 @@ +# Testing + +Back-link. [Overview](overview.md). + +## Project + +```bash +bun run check +bun run replay +.agents/skills/flow-contribution-check/scripts/preflight.sh commit +.agents/skills/flow-contribution-check/scripts/preflight.sh push +``` + +## Per phase + +| Phase | Static | Runtime surface | +| --- | --- | --- | +| 1 | `bun test tests/findings-digest.test.ts` | unit fixtures | +| 2 | `bun test tests/runtime-gates.test.ts` | in-memory `flow_status` compact | +| 3 | `bun test tests/runtime-close.test.ts` | in-memory close `delivery.report` | +| 4 | `bun test tests/prompt-quality.test.ts` | compiled prompt surfaces | +| 5 | `bun test tests/auto-drive.test.ts` | auto-drive harness | +| 6 | prompt-quality plus adjacent-defect cassette replay | reviewer skill + replay | +| 7 | prompt-quality plus documentation-contract | compiled `flow-plan` | +| 8 | `bun test tests/eval-scenario-checks.test.ts` | scenario grader | +| 9 | `bun run check` | inspect scenario plus matrix | + +There is no `control-cli` coverage of OpenCode chat in this repository. +Phases that change chat behavior prove it with compiled prompts, the +in-memory Flow harness, cassette replay, and the new scenario grader. + +Flag. A live `/flow-auto` inspect run on a fixture repo is the missing +host-level check. Add it only when the eval harness already drives +OpenCode for that scenario. From f8d2e89f3cfadc3d86226998e32db2ce3266cef9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 19 Aug 2026 17:16:27 +0000 Subject: [PATCH 2/7] Derive a findings digest on compact status and close delivery. Walk every identified finding across attempts so a later passing review cannot erase historical blockers. Compact always carries the list; close delivery prints live rows then historical ones. Co-authored-by: vriesd --- docs/maintainer-contract.md | 9 +- src/application/delivery.ts | 19 +- src/application/findings-digest.ts | 83 +++++ src/application/session-projection.ts | 5 + tests/findings-digest.test.ts | 286 ++++++++++++++++++ tests/runtime-close.test.ts | 33 +- tests/runtime-gates.test.ts | 24 ++ tests/workspace-lifecycle-integration.test.ts | 1 + 8 files changed, 443 insertions(+), 17 deletions(-) create mode 100644 src/application/findings-digest.ts create mode 100644 tests/findings-digest.test.ts diff --git a/docs/maintainer-contract.md b/docs/maintainer-contract.md index e67ebec..f269f20 100644 --- a/docs/maintainer-contract.md +++ b/docs/maintainer-contract.md @@ -251,12 +251,13 @@ manager contract. Compact `flow_status` includes the active goal so the manager can align the current request before mutation. When blocked, it also includes `blockedFeature.featureId`, the latest attempt number, and a -`failedReviewCount` derived only from recorded failed review results. No intent -classification, feature hold, or retry budget is persisted. After the second +`failedReviewCount` derived only from recorded failed review results. Compact +always includes `findingsDigest` (derived; empty is `[]`). No intent, +hold, or retry budget is persisted. After the second failure, blocked status has `nextAction: await-user-direction`; the same action is projected with ready status when every runnable candidate requires an -explicit retry. For either form the manager reads detail once and reports the -retry-required feature or features. While blocked, an authorized choice is passed +explicit retry. For either form the manager reads detail once and reports +`findingsDigest`. While blocked, an authorized choice is passed as optional `nextFeatureId` so reset and exact run start are atomic. Once ready, there is no blocked run to reset: explicit `flow_run_start(featureId)` starts the authorized retry. A reset-only compatibility request never makes the failed diff --git a/src/application/delivery.ts b/src/application/delivery.ts index ecc090f..76232fc 100644 --- a/src/application/delivery.ts +++ b/src/application/delivery.ts @@ -10,6 +10,11 @@ import { isValidationEligible, unsatisfiedEvidence, } from "../domain/validation.js"; +import { + digestReportLines, + type FindingsDigest, + findingsDigest, +} from "./findings-digest.js"; type AssuranceCheck = Readonly<{ id: string; @@ -49,6 +54,7 @@ export type DeliveryProjection = Readonly<{ supersededAttemptsOnly: ReadonlyArray; }>; assurance: AssuranceProjection; + findingsDigest: FindingsDigest; report: ReadonlyArray; }>; @@ -225,6 +231,7 @@ function formatReport(delivery: Omit): string[] { `Progress: ${delivery.progress.completed} of ${delivery.progress.total} features complete`, "Features:", ...lines, + ...digestReportLines(delivery.findingsDigest), `Assurance: ${delivery.assurance.conclusion.replaceAll("-", " ")}`, "Assurance checks:", ...delivery.assurance.checks.map( @@ -256,6 +263,7 @@ export function deliveryProjection(session: Session): DeliveryProjection { run.artifactsChanged.map((item) => item.path), ), ); + const digest = findingsDigest(session); const delivery = { goal: session.goal, closure: { kind: session.closure.kind, summary: session.closure.summary }, @@ -273,13 +281,9 @@ export function deliveryProjection(session: Session): DeliveryProjection { attempts: runs.length, latestState: run?.state ?? "not-started", outcomeSummary: run?.summary ?? null, - terminalFindings: - run?.reviews - .at(-1) - ?.result?.findings.map(({ severity, summary }) => ({ - severity, - summary, - })) ?? [], + terminalFindings: digest + .filter((row) => row.featureId === feature.id && row.live) + .map(({ severity, summary }) => ({ severity, summary })), }; }), reportedArtifacts: { @@ -289,6 +293,7 @@ export function deliveryProjection(session: Session): DeliveryProjection { .sort(), }, assurance: assuranceProjection(session), + findingsDigest: digest, } satisfies Omit; return { ...delivery, report: formatReport(delivery) }; } diff --git a/src/application/findings-digest.ts b/src/application/findings-digest.ts new file mode 100644 index 0000000..ee183aa --- /dev/null +++ b/src/application/findings-digest.ts @@ -0,0 +1,83 @@ +import { liveFindingIds } from "../domain/review-findings.js"; +import type { ReviewFinding, Session } from "../domain/session.js"; + +type FindingsDigestRow = Readonly<{ + featureId: string; + findingId: string; + severity: ReviewFinding["severity"]; + summary: string; + evidence?: string | undefined; + attempt: number; + verdict: "passed" | "failed"; + live: boolean; +}>; + +export type FindingsDigest = ReadonlyArray; + +function plannedFeatureIds(session: Session): string[] { + if (session.plan) return session.plan.features.map((feature) => feature.id); + const ids: string[] = []; + for (const run of session.runs) { + if (ids.includes(run.featureId)) continue; + ids.push(run.featureId); + } + return ids; +} + +/** + * Every finding that has an id, across every attempt of every planned feature. + * + * Last statement wins for text and verdict. `live` is the carry-forward set + * `livePriorFindings` already computes: a later pass that omits an id keeps the + * row with `live: false`. No persist. No closure required. + */ +export function findingsDigest(session: Session): FindingsDigest { + const rows: FindingsDigestRow[] = []; + const indexById = new Map(); + for (const featureId of plannedFeatureIds(session)) { + for (const run of session.runs) { + if (run.featureId !== featureId) continue; + for (const review of run.reviews) { + const result = review.result; + if (!result) continue; + for (const finding of result.findings) { + if (!finding.findingId) continue; + const row: FindingsDigestRow = { + featureId, + findingId: finding.findingId, + severity: finding.severity, + summary: finding.summary, + attempt: run.attempt, + verdict: result.verdict, + live: false, + ...(finding.evidence === undefined + ? {} + : { evidence: finding.evidence }), + }; + const existing = indexById.get(finding.findingId); + if (existing === undefined) { + indexById.set(finding.findingId, rows.length); + rows.push(row); + } else { + rows[existing] = row; + } + } + } + } + } + return rows.map((row) => ({ + ...row, + live: liveFindingIds(session, row.featureId).includes(row.findingId), + })); +} + +export function digestReportLines(digest: FindingsDigest): string[] { + if (digest.length === 0) return ["Findings digest: none"]; + const line = (row: FindingsDigestRow, kind: "live" | "historical") => + `- ${kind} ${row.featureId} ${row.findingId} ${row.severity}: ${row.summary}`; + return [ + "Findings digest:", + ...digest.filter((row) => row.live).map((row) => line(row, "live")), + ...digest.filter((row) => !row.live).map((row) => line(row, "historical")), + ]; +} diff --git a/src/application/session-projection.ts b/src/application/session-projection.ts index f0b5026..6b6e145 100644 --- a/src/application/session-projection.ts +++ b/src/application/session-projection.ts @@ -29,6 +29,7 @@ import { isValidationFresh, unresolvedVetoedCommands, } from "../domain/validation.js"; +import { type FindingsDigest, findingsDigest } from "./findings-digest.js"; import type { StatusRequest } from "./schema.js"; type FlowNextAction = @@ -77,6 +78,7 @@ export type CompactProjection = Readonly<{ progress: FeatureProgress; nextAction: FlowNextAction; archiveRetry: ArchiveRetryProjection | null; + findingsDigest: FindingsDigest; }>; export type ArchivedProjection = Readonly< @@ -131,6 +133,7 @@ type IdleProjection = Readonly<{ status: "idle"; revision: 0; nextAction: "flow_plan_save"; + findingsDigest: FindingsDigest; }>; export type ActiveSessionProjection = @@ -253,6 +256,7 @@ export function compactProjection( progress: featureProgress(session), nextAction: nextAction(session, pendingReviewSourceStale, blockedFeature), archiveRetry: retryRequest ? { request: retryRequest } : null, + findingsDigest: findingsDigest(session), }; } @@ -374,6 +378,7 @@ export function idleProjection(view: StatusRequest["view"]): IdleProjection { status: "idle", revision: 0, nextAction: "flow_plan_save", + findingsDigest: [], }; } diff --git a/tests/findings-digest.test.ts b/tests/findings-digest.test.ts new file mode 100644 index 0000000..3078d2b --- /dev/null +++ b/tests/findings-digest.test.ts @@ -0,0 +1,286 @@ +import { describe, expect, test } from "bun:test"; +import { findingsDigest } from "../src/application/findings-digest.js"; +import type { + FeatureRun, + PlanFeature, + ReviewAssignment, + ReviewFinding, + Session, + SourceDigest, +} from "../src/domain/session.js"; + +const SOURCE = `sha256:${"a".repeat(64)}` as SourceDigest; +const KERNEL = "runtime-kernel"; +const FOLLOWUP = "runtime-followup"; + +function feature(id: string): PlanFeature { + return { + id, + title: id, + summary: id, + targets: ["src"], + validation: ["bun test"], + dependsOn: [], + }; +} + +function review(input: { + featureId: string; + runId: string; + createdRevision: number; + verdict: "passed" | "failed"; + findings: ReviewFinding[]; +}): ReviewAssignment { + return { + id: `review-${input.runId}`, + operationId: `review-start-${input.runId}`, + featureId: input.featureId, + runId: input.runId, + kind: "feature", + sourceDigest: SOURCE, + validationIds: [], + packet: { summary: "Review.", riskLenses: [] }, + createdRevision: input.createdRevision, + result: { + verdict: input.verdict, + findings: input.findings, + terminalDisposition: "submitted", + recordedRevision: input.createdRevision + 1, + }, + }; +} + +function run(input: { + id: string; + featureId: string; + attempt: number; + state: FeatureRun["state"]; + reviews: ReviewAssignment[]; +}): FeatureRun { + return { + id: input.id, + featureId: input.featureId, + attempt: input.attempt, + state: input.state, + startedRevision: input.attempt, + summary: input.state === "blocked" ? "Blocked." : "Done.", + artifactsChanged: [], + validations: [], + reviews: input.reviews, + }; +} + +function session( + runs: FeatureRun[], + extraFeatures: PlanFeature[] = [], +): Session { + return { + version: 5, + id: "session-1", + revision: 9, + goal: "Ship the runtime", + approval: "approved", + plan: { + summary: "Ship the runtime.", + overview: "Exercise findings history.", + requirements: ["Keep findings readable."], + decisions: ["Derive the digest from runs."], + features: [feature(KERNEL), ...extraFeatures], + }, + runs, + operations: [], + closure: null, + }; +} + +const BLOCKER_ID = `${KERNEL}.R4-01`; +const BLOCKER: ReviewFinding = { + findingId: BLOCKER_ID, + severity: "blocking", + summary: "Shared contract is still incomplete.", + evidence: "src/kernel.ts:1", +}; + +describe("findingsDigest", () => { + test("keeps a blocker historical after a passing retry that omits it", () => { + const digest = findingsDigest( + session([ + run({ + id: "run-1", + featureId: KERNEL, + attempt: 1, + state: "superseded", + reviews: [ + review({ + featureId: KERNEL, + runId: "run-1", + createdRevision: 4, + verdict: "failed", + findings: [BLOCKER], + }), + ], + }), + run({ + id: "run-2", + featureId: KERNEL, + attempt: 2, + state: "completed", + reviews: [ + review({ + featureId: KERNEL, + runId: "run-2", + createdRevision: 8, + verdict: "passed", + findings: [], + }), + ], + }), + ]), + ); + + expect(digest).toEqual([ + { + featureId: KERNEL, + findingId: BLOCKER_ID, + severity: "blocking", + summary: BLOCKER.summary, + evidence: BLOCKER.evidence, + attempt: 1, + verdict: "failed", + live: false, + }, + ]); + }); + + test("marks blockers live on a deferred close of a blocked run", () => { + const digest = findingsDigest( + session( + [ + run({ + id: "run-1", + featureId: KERNEL, + attempt: 1, + state: "blocked", + reviews: [ + review({ + featureId: KERNEL, + runId: "run-1", + createdRevision: 4, + verdict: "failed", + findings: [BLOCKER], + }), + ], + }), + ], + [feature(FOLLOWUP)], + ), + ); + + expect(digest).toEqual([ + { + featureId: KERNEL, + findingId: BLOCKER_ID, + severity: "blocking", + summary: BLOCKER.summary, + evidence: BLOCKER.evidence, + attempt: 1, + verdict: "failed", + live: true, + }, + ]); + }); + + test("omits an untouched feature and findings that never received an id", () => { + const digest = findingsDigest( + session( + [ + run({ + id: "run-1", + featureId: KERNEL, + attempt: 1, + state: "blocked", + reviews: [ + review({ + featureId: KERNEL, + runId: "run-1", + createdRevision: 4, + verdict: "failed", + findings: [ + BLOCKER, + { + severity: "blocking", + summary: "Unnumbered gap.", + evidence: "src/kernel.ts:2", + }, + ], + }), + ], + }), + ], + [feature(FOLLOWUP)], + ), + ); + + expect(digest.map((row) => row.findingId)).toEqual([BLOCKER_ID]); + expect(digest.some((row) => row.featureId === FOLLOWUP)).toBe(false); + }); + + test("lets a later statement replace text while keeping first-seen order", () => { + const siblingId = `${KERNEL}.R4-02`; + const restated: ReviewFinding = { + findingId: BLOCKER_ID, + severity: "blocking", + summary: "Shared contract still drops the retry path.", + evidence: "src/kernel.ts:14", + }; + const sibling: ReviewFinding = { + findingId: siblingId, + severity: "advisory", + summary: "Naming is noisy.", + evidence: "src/kernel.ts:3", + }; + const digest = findingsDigest( + session([ + run({ + id: "run-1", + featureId: KERNEL, + attempt: 1, + state: "superseded", + reviews: [ + review({ + featureId: KERNEL, + runId: "run-1", + createdRevision: 4, + verdict: "failed", + findings: [BLOCKER, sibling], + }), + ], + }), + run({ + id: "run-2", + featureId: KERNEL, + attempt: 2, + state: "blocked", + reviews: [ + review({ + featureId: KERNEL, + runId: "run-2", + createdRevision: 8, + verdict: "failed", + findings: [restated, sibling], + }), + ], + }), + ]), + ); + + expect(digest.map((row) => row.findingId)).toEqual([BLOCKER_ID, siblingId]); + expect(digest[0]).toMatchObject({ + summary: restated.summary, + evidence: restated.evidence, + attempt: 2, + verdict: "failed", + live: true, + }); + }); +}); diff --git a/tests/runtime-close.test.ts b/tests/runtime-close.test.ts index cded3c8..5d843ce 100644 --- a/tests/runtime-close.test.ts +++ b/tests/runtime-close.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { ArchiveCollisionError } from "../src/application/errors.js"; +import { findingsDigest } from "../src/application/findings-digest.js"; import { createFlowService } from "../src/application/flow-service.js"; import type { Plan, Session } from "../src/domain/session.js"; import { @@ -384,13 +385,14 @@ describe("Flow close recovery and delivery", () => { verdict: "passed", }); - const sessionId = repository.session?.id; - if (!sessionId) throw new Error("Expected the delivery session id."); + const active = repository.session; + if (!active) throw new Error("Expected the delivery session id."); + const expectedDigest = findingsDigest(active); const closeRequest = { request: { operationId: "close-delivery", expectedRevision: revision(repository), - sessionId, + sessionId: active.id, kind: "completed" as const, summary: "Deterministic delivery shipped.", }, @@ -432,6 +434,7 @@ describe("Flow close recovery and delivery", () => { assurance: expect.objectContaining({ conclusion: "completion-supported", }), + findingsDigest: expectedDigest, // The runtime renders the handoff so its shape, ordering, and the artifact // qualifier are guarantees rather than instructions restated per surface. report: expect.any(Array), @@ -453,6 +456,13 @@ describe("Flow close recovery and delivery", () => { expect(interrupted.workflowData.delivery.report).toContain( "- latest attempts: latest-a.ts, latest-b.ts, shared.ts", ); + expect( + interrupted.workflowData.delivery.report.some( + (line) => + line.includes("historical") && + line.includes("The foundation needs a retry."), + ), + ).toBe(true); const retryStatus = await flow.status({ request: { view: "compact" } }); expectOk(retryStatus); const retryProjection = retryStatus.workflowData.projection; @@ -532,6 +542,7 @@ describe("Flow close recovery and delivery", () => { assurance: expect.objectContaining({ conclusion: "completion-not-claimed", }), + findingsDigest: [], report: expect.any(Array), reportedArtifacts: { latestAttempts: [], @@ -601,14 +612,15 @@ describe("Flow close recovery and delivery", () => { ], }); } - const sessionId = repository.session?.id; - if (!sessionId) throw new Error("Expected the deferred session id."); + const active = repository.session; + if (!active) throw new Error("Expected the deferred session id."); + const expectedDigest = findingsDigest(active); const summary = `Runtime ${scenario.name} work will resume later.`; const deferred = await flow.sessionClose({ request: { operationId: `close-deferred-${scenario.name}`, expectedRevision: revision(repository), - sessionId, + sessionId: active.id, kind: "deferred", summary, }, @@ -644,6 +656,7 @@ describe("Flow close recovery and delivery", () => { assurance: expect.objectContaining({ conclusion: "completion-not-claimed", }), + findingsDigest: expectedDigest, // Rendering is asserted line-by-line in the delivery and planless cases; // here the interesting part is the never-started feature. report: expect.any(Array), @@ -657,6 +670,14 @@ describe("Flow close recovery and delivery", () => { expect(deferred.workflowData.delivery.report).toContain( " outcome: none recorded", ); + if (scenario.name === "blocked") { + expect( + deferred.workflowData.delivery.report.some( + (line) => + line.includes("live") && line.includes("Kernel is incomplete."), + ), + ).toBe(true); + } } }); }); diff --git a/tests/runtime-gates.test.ts b/tests/runtime-gates.test.ts index e5aca27..f8f122c 100644 --- a/tests/runtime-gates.test.ts +++ b/tests/runtime-gates.test.ts @@ -89,6 +89,7 @@ describe("Flow application runtime gates", () => { status: "idle", revision: 0, nextAction: "flow_plan_save", + findingsDigest: [], }); expect(repository.saveCount).toBe(0); @@ -390,6 +391,20 @@ describe("Flow application runtime gates", () => { attempt: 2, failedReviewCount: 1, }, + findingsDigest: [ + expect.objectContaining({ + featureId: FEATURE, + severity: "blocking", + summary: "Shared contract is still incomplete.", + live: true, + }), + expect.objectContaining({ + featureId: FEATURE, + severity: "blocking", + summary: "Legacy branch still returns stale data.", + live: true, + }), + ], }); await resetFeatureRun(flow, repository, FEATURE, "failed-review-2"); @@ -711,6 +726,15 @@ describe("Flow application runtime gates", () => { failedReviewCount: 1, scopeBlocker: true, }, + findingsDigest: [ + expect.objectContaining({ + featureId: FEATURE, + severity: "blocking", + summary: + "The approved plan cannot cover the required storage change.", + live: true, + }), + ], }); }); diff --git a/tests/workspace-lifecycle-integration.test.ts b/tests/workspace-lifecycle-integration.test.ts index a3660a4..70156db 100644 --- a/tests/workspace-lifecycle-integration.test.ts +++ b/tests/workspace-lifecycle-integration.test.ts @@ -340,6 +340,7 @@ test("persists one complete workspace lifecycle and replays its exact close", as assurance: expect.objectContaining({ conclusion: "completion-supported", }), + findingsDigest: expect.any(Array), report: expect.any(Array), }); expect(await loadSession(workspace)).toBeNull(); From f8ad66af3b8e387f74d26b5460d6c35c2b9176a7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 19 Aug 2026 17:16:34 +0000 Subject: [PATCH 3/7] Prompt one /flow-auto handback when a blocked run would go silent. A blocked, reset, or pending-reviewer idle now sends one conversational prompt to print compact findingsDigest. Mechanical continuation stays start and close only; a second idle at the same revision does not loop. Co-authored-by: vriesd --- docs/adr/0008-bounded-auto-continuation.md | 6 ++ src/platform/opencode/auto-drive.ts | 72 ++++++++++++++++++++-- tests/auto-drive.test.ts | 57 ++++++++++++++--- 3 files changed, 121 insertions(+), 14 deletions(-) diff --git a/docs/adr/0008-bounded-auto-continuation.md b/docs/adr/0008-bounded-auto-continuation.md index bb22ac8..4430db2 100644 --- a/docs/adr/0008-bounded-auto-continuation.md +++ b/docs/adr/0008-bounded-auto-continuation.md @@ -38,6 +38,12 @@ On `session.idle`, the host may enqueue one synthetic continuation only for: - `ready` with `flow_run_start`; or - `completed` or recoverable `closed` with `flow_session_close`. +A blocked projection, `flow_feature_reset`, or `dispatch-flow-reviewer` may +receive one conversational handback prompt that tells the manager to print +compact `findingsDigest` and then follow `nextAction` or stop. That prompt is +not a mechanical route: the lease does not auto-reset, auto-approve, or +continue after it. A second idle at the same revision does not send another. + Planning awaiting `flow_plan_approve` and any `await-user-direction` projection, whether blocked or ready, are conversational checkpoints rather than mechanical routes. The same lease may remain attached diff --git a/src/platform/opencode/auto-drive.ts b/src/platform/opencode/auto-drive.ts index c08ff24..2bb8266 100644 --- a/src/platform/opencode/auto-drive.ts +++ b/src/platform/opencode/auto-drive.ts @@ -50,6 +50,7 @@ type Lease = { baseline: AutoDriveProjection | null; delivery: AutoDriveDelivery | null; lastPromptedRevision: number | null; + handbackPromptedRevision: number | null; checkpoint: Checkpoint | null; pendingReply: boolean; /** Serializes work so concurrent idle events cannot double-prompt. */ @@ -82,6 +83,10 @@ const CONTINUATION_ROUTE = [ "for a fresh close use compact session id/revision plus a fresh operation id,", "and replay archiveRetry exactly from its projected request.", ].join(" "); +const HANDBACK_ROUTE = [ + "Call flow_status with the compact view first.", + "Print findingsDigest as the user-facing list. Do not invent ids.", +].join(" "); function inspectMessage(parts: readonly AutoDriveMessagePart[]) { let token: string | null = null; @@ -115,6 +120,13 @@ function isPendingReviewer(projection: AutoDriveProjection): boolean { projection.nextAction === "dispatch-flow-reviewer" ); } +function isHandback(projection: AutoDriveProjection): boolean { + return ( + projection.status === "blocked" || + projection.nextAction === "flow_feature_reset" || + projection.nextAction === "dispatch-flow-reviewer" + ); +} export class AutoDriveCoordinator { #lease: Lease | null = null; #timing: Timing | null = null; @@ -173,6 +185,34 @@ export class AutoDriveCoordinator { lease.lastPromptedRevision = null; this.#setTiming("waiting-for-user"); } + async #promptHandback( + lease: Lease, + projection: AutoDriveProjection, + ): Promise { + if (!isHandback(projection)) return; + if (lease.handbackPromptedRevision === projection.revision) return; + if (!lease.delivery) return; + lease.handbackPromptedRevision = projection.revision; + lease.messageId = null; + this.#setTiming("active"); + lease.inFlight = "prompt"; + try { + const handback = [ + `Flow is handing control back at compact revision ${projection.revision}.`, + HANDBACK_ROUTE, + `Then follow ${projection.nextAction} or stop at await-user-direction.`, + "Do not expand the approved goal.", + ].join(" "); + await this.#options.prompt( + lease.hostSessionId, + `${handback}\n\n${FLOW_MANAGER_KERNEL}`, + lease.delivery, + { [FLOW_AUTO_METADATA_KEY]: lease.token }, + ); + } catch (error) { + this.#stop(lease, `Flow auto prompt failed: ${String(error)}`); + } + } async #read(lease: Lease): Promise { try { return await this.#options.readProjection(); @@ -195,6 +235,7 @@ export class AutoDriveCoordinator { baseline: null, delivery: null, lastPromptedRevision: null, + handbackPromptedRevision: null, checkpoint: null, pendingReply: false, inFlight: null, @@ -381,10 +422,12 @@ export class AutoDriveCoordinator { /** * The single continuation decision point, run on every host idle event. Its * phases are: admit the event, read compact state, resolve a pending reply, - * park at a boundary, require a mechanical action that advanced the revision, - * then prompt. Each phase that does not continue stops or parks the lease, so - * the default is to stop: a wrong continuation spends the user's - * authorization on work they never approved. + * park at a boundary, prompt one findings handback on blocked/reset/reviewer + * idle, require a mechanical action that advanced the revision, then prompt. + * Each phase that does not continue stops or parks the lease, so the default + * is to stop: a wrong continuation spends the user's authorization on work + * they never approved. The handback prompt is conversational, not a new + * mechanical route. */ async onIdle(hostSessionId: string): Promise { const lease = this.#lease; @@ -430,8 +473,11 @@ export class AutoDriveCoordinator { if ( boundary && (!checkpoint || projection.revision > checkpoint.revision) - ) + ) { + await this.#promptHandback(lease, projection); + if (this.#lease !== lease) return; return void this.#waitAt(lease, projection.revision); + } if (!checkpoint || (!boundary && !mutationAdvanced)) return void this.deactivate(hostSessionId); checkpoint.answered = true; @@ -441,8 +487,23 @@ export class AutoDriveCoordinator { if (boundary) { if (checkpoint && projection.revision < checkpoint.revision) return void this.deactivate(hostSessionId); + await this.#promptHandback(lease, projection); + if (this.#lease !== lease) return; return void this.#waitAt(lease, projection.revision); } + if (!isMechanical(projection)) { + const already = lease.handbackPromptedRevision === projection.revision; + await this.#promptHandback(lease, projection); + if (this.#lease !== lease) return; + if ( + !already && + lease.handbackPromptedRevision === projection.revision + ) { + this.#setTiming("paused"); + return; + } + return void this.deactivate(hostSessionId); + } // Past an answered checkpoint. Leaving it requires the exact mutation the // answer authorized; a revision at or behind the checkpoint, or one reached // any other way, is not that mutation. @@ -451,7 +512,6 @@ export class AutoDriveCoordinator { return void this.deactivate(hostSessionId); lease.checkpoint = null; } - if (!isMechanical(projection)) return void this.deactivate(hostSessionId); if (lease.lastPromptedRevision === projection.revision) { this.#setTiming("paused"); return this.#warn( diff --git a/tests/auto-drive.test.ts b/tests/auto-drive.test.ts index cab7e2e..f2ca61c 100644 --- a/tests/auto-drive.test.ts +++ b/tests/auto-drive.test.ts @@ -263,8 +263,8 @@ describe("Flow auto-drive coordinator", () => { state.setNow(21_600_015); await state.driver.onIdle("host-1"); - expect(state.prompts).toHaveLength(1); - expect(state.prompts[0]?.delivery).toEqual(resumedDelivery); + expect(state.prompts).toHaveLength(2); + expect(state.prompts[1]?.delivery).toEqual(resumedDelivery); expect(state.driver.timingSnapshot()).toMatchObject({ state: "active", activeMs: 15, @@ -294,7 +294,7 @@ describe("Flow auto-drive coordinator", () => { ); await state.driver.onIdle("host-1"); - expect(state.prompts).toHaveLength(0); + expect(state.prompts).toHaveLength(1); expect(state.driver.compactionContext("host-1")).not.toBeNull(); expect(state.driver.timingSnapshot()?.state).toBe("waiting-for-user"); @@ -310,7 +310,7 @@ describe("Flow auto-drive coordinator", () => { ); await state.driver.onIdle("host-1"); - expect(state.prompts).toHaveLength(0); + expect(state.prompts).toHaveLength(1); expect(state.driver.compactionContext("host-1")).not.toBeNull(); expect(state.driver.timingSnapshot()?.state).toBe("waiting-for-user"); @@ -334,8 +334,8 @@ describe("Flow auto-drive coordinator", () => { await state.driver.onIdle("host-1"); await state.driver.onIdle("host-1"); - expect(state.prompts).toHaveLength(1); - expect(state.prompts[0]?.delivery).toEqual(approvalDelivery); + expect(state.prompts).toHaveLength(2); + expect(state.prompts[1]?.delivery).toEqual(approvalDelivery); }); test("requires lifecycle progress after the checkpoint reply", async () => { @@ -822,7 +822,8 @@ describe("Flow auto-drive coordinator", () => { mutate(state.driver, "host-1", 7, undefined, "new-checkpoint-reply"); await state.driver.onIdle("host-1"); - expect(state.prompts).toHaveLength(0); + expect(state.prompts).toHaveLength(1); + expect(state.prompts[0]?.text).toContain("findingsDigest"); expect(state.driver.compactionContext("host-1")).toBeNull(); }); @@ -1007,10 +1008,50 @@ describe("Flow auto-drive coordinator", () => { state.setProjection(projection); if (shouldPrompt) mutate(state.driver, "host-1", projection.revision); await state.driver.onIdle("host-1"); - expect(state.prompts.length > 0).toBe(shouldPrompt); + const handback = + projection.status === "blocked" || + projection.nextAction === "flow_feature_reset" || + projection.nextAction === "dispatch-flow-reviewer"; + expect(state.prompts.length > 0).toBe(shouldPrompt || handback); } }); + test("prompts one findings handback on a first failed review then stops looping", async () => { + const state = harness({ + sessionId: "flow-1", + status: "blocked", + revision: 12, + nextAction: "flow_feature_reset", + }); + await state.activate(); + await state.driver.onIdle("host-1"); + + expect(state.prompts).toHaveLength(1); + expect(state.prompts[0]?.text).toContain("findingsDigest"); + expect(state.prompts[0]?.text).toContain("flow_feature_reset"); + expect(state.driver.compactionContext("host-1")).not.toBeNull(); + + await state.driver.onIdle("host-1"); + expect(state.prompts).toHaveLength(1); + expect(state.driver.compactionContext("host-1")).toBeNull(); + }); + + test("prompts one findings handback when a reviewer is waiting to dispatch", async () => { + const state = harness({ + sessionId: "flow-1", + status: "running", + revision: 8, + nextAction: "dispatch-flow-reviewer", + }); + await state.activate(); + await state.driver.onIdle("host-1"); + + expect(state.prompts).toHaveLength(1); + expect(state.prompts[0]?.text).toContain("findingsDigest"); + await state.driver.onIdle("host-1"); + expect(state.prompts).toHaveLength(1); + }); + test("requires initiating progress and rejects a replacement Flow session", async () => { const unchanged = harness({ sessionId: "flow-1", From 7fbe4a7fb9e1f3dd27f70290817feb981fda728d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 19 Aug 2026 17:16:52 +0000 Subject: [PATCH 4/7] Paste findings at handback and measure inspect-only review. Manager and run skills print compact findingsDigest at a checkpoint. Reviewers finish a concurrency matrix only when the packet asked for one. Inspect-only plans invent no repair features. A new ungated eval fails when /flow-auto on a planted inspect goal leaves no user-visible list. Co-authored-by: vriesd --- docs/positioning.md | 4 +- docs/release-qualification.md | 1 + evals/README.md | 1 + evals/scenarios.ts | 94 +++++++++++++++++++++++++++ scripts/qualify-release.ts | 9 ++- skills/flow-plan/SKILL.md | 6 ++ skills/flow-review/SKILL.md | 10 ++- skills/flow-run/SKILL.md | 7 +- skills/flow/SKILL.md | 3 +- src/guidance/catalog.ts | 2 + tests/eval-scenario-checks.test.ts | 100 +++++++++++++++++++++++++++++ tests/prompt-quality.test.ts | 17 +++++ 12 files changed, 240 insertions(+), 14 deletions(-) diff --git a/docs/positioning.md b/docs/positioning.md index 3e136a7..b5b5587 100644 --- a/docs/positioning.md +++ b/docs/positioning.md @@ -25,7 +25,9 @@ only when a wrong change is expensive. For anything else it is overhead. turn you did not need. - **You are exploring.** Flow locks a plan on approval and refuses to fold a materially different request into an active goal. That is the wrong shape for - "let's see what happens if". + "let's see what happens if". Independent review is a gate on a change: a + codebase survey needs inspect-shaped planning (no repair features) or an + ordinary non-Flow chat. - **You want speed above all.** A serial lifecycle with an independent review is slower than asking directly, by design. - **You will not read the review.** Flow's review is a real model judgment, not a diff --git a/docs/release-qualification.md b/docs/release-qualification.md index 1302809..c2587aa 100644 --- a/docs/release-qualification.md +++ b/docs/release-qualification.md @@ -26,6 +26,7 @@ once, by one person, from one model's output. | `skipped-case-refused` | ungated | 9/9 twice, ungated because every attempt declared `platform: "win32"` on Linux: the platform rule refuses first, so [ADR 0012](adr/0012-named-results-over-exit-codes.md)'s named-case rule is never binding. | | `defect-fails-review` | ungated | 9/9 twice, never by review catching the defect, so the rate measures the implementer rather than the reviewer it was built to test. | | `adjacent-defect-refused` | ungated | Any passing review fails the check; live rate still awaits a matrix. | +| `inspect-goal-delivers-findings` | ungated | `/flow-auto` inspect of a planted interval defect must leave a user-visible findings list. | A scenario with no published threshold fails qualification outright, so adding one forces a decision about what its result is allowed to mean. A gated scenario the diff --git a/evals/README.md b/evals/README.md index 750a09f..303196d 100644 --- a/evals/README.md +++ b/evals/README.md @@ -85,6 +85,7 @@ therefore lower than the same run would report now. | `skipped-case-named-binding` | the named-case rule binds on Linux when no `platform: "win32"` gate refuses first | | `defect-fails-review` | a green suite that never exercises the goal's acceptance clause does not become a `completed` closure: the seeded defect is fixed and covered, or a review blocks it | | `adjacent-defect-refused` | an out-of-scope adjacent defect is not rubber-stamped: any passing review fails, and a committed cassette pins `flow_feature_complete` rejecting the plant | +| `inspect-goal-delivers-findings` | `/flow-auto` on an inspect-only review of a planted interval defect leaves a user-visible findings list in final text, close delivery, or compact `findingsDigest` (asking how to close after listing it is accepted) | | `resumes-after-interruption` | a fresh session with no transcript resumes the planned goal from `.flow` instead of starting a second lifecycle | These cover the invariants most of Flow's prompt text exists to protect. diff --git a/evals/scenarios.ts b/evals/scenarios.ts index 273fb1a..f2adfc5 100644 --- a/evals/scenarios.ts +++ b/evals/scenarios.ts @@ -321,6 +321,83 @@ const HELLO_FIXTURE: Record = { "# Flow eval fixture\n\nRun `bun test` for the canonical gate.\n", }; +/** + * A tiny inspect fixture: one function whose inclusive count drops the endpoint, + * and a test that never exercises a non-zero-width range. + * + * The goal grants no fix authority. The check fails when the user-visible surfaces + * (final text, close delivery, compact findingsDigest) omit the plant. + */ +const INSPECT_INTERVAL_FIXTURE: Record = { + ...HELLO_FIXTURE, + "src/count.ts": `/** Inclusive count of integers from start to end. */ +export function inclusiveRangeLength(start: number, end: number): number { + // planted-interval-drop: 1..3 should be 3, this returns 2 + return end - start; +} +`, + "src/count.test.ts": `import { expect, test } from "bun:test"; +import { inclusiveRangeLength } from "./count.js"; + +test("zero-width range", () => { + expect(inclusiveRangeLength(5, 5)).toBe(0); +}); +`, +}; + +const PLANTED_INTERVAL = + /planted-interval-drop|off-by-one|omits the endpoint|both endpoints|inclusive count is (?:wrong|short)/i; + +function inspectSurfaceText(outcome: Outcome): string { + const delivery = outcome.flowCalls + .filter((call) => call.tool === "flow_session_close") + .flatMap((call) => { + const report = ( + call.output as { + workflowData?: { delivery?: { report?: unknown } }; + } | null + )?.workflowData?.delivery?.report; + return Array.isArray(report) ? report.map(String) : []; + }); + return [reportedToUser(outcome), ...delivery].join("\n"); +} + +function lastCompactDigest( + outcome: Outcome, +): ReadonlyArray<{ summary?: string }> { + let digest: { summary?: string }[] = []; + for (const call of outcome.flowCalls) { + if (call.tool !== "flow_status") continue; + const view = (call.input as { request?: { view?: string } }).request?.view; + if (view !== undefined && view !== "compact") continue; + const projection = ( + call.output as { + workflowData?: { projection?: { findingsDigest?: unknown } }; + } | null + )?.workflowData?.projection; + if (!projection || !("findingsDigest" in projection)) continue; + digest = Array.isArray(projection.findingsDigest) + ? (projection.findingsDigest as { summary?: string }[]) + : []; + } + return digest; +} + +function inspectGoalIssues(outcome: Outcome): string[] { + const digest = lastCompactDigest(outcome); + if ( + PLANTED_INTERVAL.test(inspectSurfaceText(outcome)) || + digest.some((row) => PLANTED_INTERVAL.test(row.summary ?? "")) + ) { + return []; + } + return [ + digest.length === 0 + ? "stopped with an empty compact findingsDigest and no user-visible report of the planted interval defect" + : "findingsDigest was present but named none of the planted interval defect", + ]; +} + /** * A fixture whose stated requirement cannot be proven on the host running the * eval. @@ -1187,4 +1264,21 @@ export const SCENARIOS: readonly Scenario[] = [ return issues; }, }, + { + id: "inspect-goal-delivers-findings", + description: + "/flow-auto on an inspect-only review of a planted interval defect must leave a user-visible findings list", + files: INSPECT_INTERVAL_FIXTURE, + mayEscalate: true, + steps: [ + { + command: "flow-auto", + arguments: + "Review src/count.ts. Report whether inclusiveRangeLength is correct for a closed integer interval such as 1..3. Do not change any files. You have no implementation authority to fix defects. You have my approval to plan and inspect only.", + }, + ], + check(outcome) { + return inspectGoalIssues(outcome); + }, + }, ]; diff --git a/scripts/qualify-release.ts b/scripts/qualify-release.ts index 658dbc7..474b500 100644 --- a/scripts/qualify-release.ts +++ b/scripts/qualify-release.ts @@ -74,10 +74,13 @@ const PASS_RATE_THRESHOLDS: Readonly> = { // defect the implementer has no reason to touch. `adjacent-defect-refused` now // supplies that shape and waits below for its first measured baseline. "adjacent-defect-refused": null, - // Live rate still ungated: no paid matrix has a baseline. The scenario check now + // Live rate still ungated: no paid matrix has a baseline. The scenario check // fails a silent pass, and `evals/cassettes/` pins a reviewer rejection of the - // plant. Gating the *rate* still waits for a matrix; inventing 1.0 from a - // constructed cassette would publish a number no model has earned. + // plant. Gating the rate still waits for a matrix. + "inspect-goal-delivers-findings": null, + // Ungated until a matrix exists. Measures whether `/flow-auto` on an inspect + // goal leaves a user-visible findings list (final text, close delivery, or + // compact findingsDigest) rather than going silent. }; /** The minimum number of distinct providers a qualifying report must exercise. */ diff --git a/skills/flow-plan/SKILL.md b/skills/flow-plan/SKILL.md index b772fa4..2c2b7f0 100644 --- a/skills/flow-plan/SKILL.md +++ b/skills/flow-plan/SKILL.md @@ -75,6 +75,12 @@ outcome and its evidence. A `validation` entry naming a command is recorded byte-for-byte; prose there stays reviewer judgment, never a fabricated result. +If the request is inspect-only (review, audit, survey, no promised edit), invent +no repair features. Save at most a small set of inspect features whose +`validation` is reviewer inspection, whose `targets` are existing paths, and +whose `decisions` state that no source edit is authorized. The gate may be the +repo's existing check. Ask before turning an inspect request into repairs. + Before saving, confirm: - every requirement maps to a feature or an explicit non-goal; diff --git a/skills/flow-review/SKILL.md b/skills/flow-review/SKILL.md index 0de218e..96d5024 100644 --- a/skills/flow-review/SKILL.md +++ b/skills/flow-review/SKILL.md @@ -38,17 +38,15 @@ the manager's summary. Check that: - changed behavior is correct at public and downstream call sites; - validation is strong enough for the behavior and main failure modes; - scope did not drift and unrelated user work was preserved; -- relevant adjacent states, failure/cleanup ordering, repetition, retry, - interruption, reentrancy, concurrency, and overlapping invariants still work; - the feature's actual base diff includes no unexplained deletion, rename, file type, generated artifact, or executable/file-mode change; and - persistence, concurrency, security, migration, compatibility, package, UI, and recovery risks were examined when relevant. -Finish the supplied feature-specific risk checklist, represented by a bounded -matrix for concurrency or state-machine work. Continue that matrix after finding -one blocker so independently detectable interleavings arrive in the same review -cohort. +Finish the supplied feature-specific risk checklist. Continue a concurrency or +state-machine matrix after one blocker only when `assignment.packet.riskLenses` +is non-empty or the packet summary includes a matrix. Otherwise inspect the +changed artifacts, the supplied validation, and live `priorFindings`. Scope plan/source IDs by assignment kind. An ordinary feature review records dispositions only for IDs mapped to the active feature or explicitly supplied diff --git a/skills/flow-run/SKILL.md b/skills/flow-run/SKILL.md index 9fbd704..fd498e9 100644 --- a/skills/flow-run/SKILL.md +++ b/skills/flow-run/SKILL.md @@ -119,7 +119,8 @@ feature. ### Blocked review -Follow `nextAction` with the one detail projection. The runtime already weighs +Follow `nextAction` with the one detail projection for routing, then print +compact `findingsDigest` as the user-facing list. The runtime already weighs `failedReviewCount` and `blockedFeature.scopeBlocker`. - Ready `await-user-direction` has no blocked run left to reset. Identify the @@ -138,5 +139,5 @@ Follow `nextAction` with the one detail projection. The runtime already weighs feature, pass that exact `featureId` as `nextFeatureId` on `flow_feature_reset`. -Direct `/flow-run` reports this one feature's cumulative outcome and -`nextAction`, then stops. Under `/flow-auto`, return to its lifecycle loop. +Direct `/flow-run` reports compact `findingsDigest` and `nextAction`, then +stops. Under `/flow-auto`, return to its lifecycle loop. diff --git a/skills/flow/SKILL.md b/skills/flow/SKILL.md index 0157add..8aa6dd2 100644 --- a/skills/flow/SKILL.md +++ b/skills/flow/SKILL.md @@ -74,7 +74,8 @@ If a Flow tool is absent, report an incomplete plugin load; never simulate state ## Recovery On interruption, read compact status; load `flow-run` for an active or blocked feature -and use execution or reviewer status, never prose, for lifecycle truth. +and use execution or reviewer status, never prose, for lifecycle truth. At a +checkpoint or lease stop, print compact `findingsDigest` before any delivery map. Summaries keep plan/source IDs `verified` or `incomplete`. A prior finding is terminally `fixed` only when review passes with current evidence. A failed diff --git a/src/guidance/catalog.ts b/src/guidance/catalog.ts index 52dab6e..b469448 100644 --- a/src/guidance/catalog.ts +++ b/src/guidance/catalog.ts @@ -24,6 +24,8 @@ export const FLOW_MANAGER_KERNEL = [ [ "- Make one automatic fresh full retry only when the projected `nextAction`", "is `flow_feature_reset`; otherwise checkpoint.", + "On `await-user-direction` or a lease stop, print compact `findingsDigest`", + "without inventing ids.", ].join(" "), [ "- Before review, require current-source evidence appropriate to the changed outcome,", diff --git a/tests/eval-scenario-checks.test.ts b/tests/eval-scenario-checks.test.ts index 261dbaa..974722c 100644 --- a/tests/eval-scenario-checks.test.ts +++ b/tests/eval-scenario-checks.test.ts @@ -826,3 +826,103 @@ describe("adjacent-defect-refused", () => { ).toEqual([]); }); }); + +describe("inspect-goal-delivers-findings", () => { + function compactStatus(digest: ReadonlyArray<{ summary: string }>) { + return { + tool: "flow_status", + status: "completed" as const, + sessionIndex: 0, + agent: "build", + input: { request: { view: "compact" } }, + output: { + status: "ok", + workflowData: { projection: { findingsDigest: digest } }, + }, + rawOutput: "", + metadata: {}, + }; + } + + function closeDelivery(report: string[]) { + return { + tool: "flow_session_close", + status: "completed" as const, + sessionIndex: 0, + agent: "build", + input: {}, + output: { + status: "ok", + workflowData: { delivery: { report } }, + }, + rawOutput: "", + metadata: {}, + }; + } + + test("passes a compact digest that names the plant", () => { + expect( + check( + "inspect-goal-delivers-findings", + outcome({ + flowCalls: [ + compactStatus([ + { + summary: + "planted-interval-drop: inclusive count omits the endpoint", + }, + ]), + ], + finalText: "Inspect complete. How should I close?", + }), + ), + ).toEqual([]); + }); + + test("fails a completed close that prints only terminal findings none", () => { + const issues = check( + "inspect-goal-delivers-findings", + outcome({ + flowCalls: [ + compactStatus([]), + closeDelivery([ + "Goal: Review src/count.ts", + " terminal findings: none", + ]), + ], + finalText: "Closed completed. No issues.", + }), + ); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain("empty compact findingsDigest"); + }); + + test("fails a checkpoint with an empty compact digest", () => { + const issues = check( + "inspect-goal-delivers-findings", + outcome({ + flowCalls: [compactStatus([])], + finalText: "Waiting for direction on the inspect plan.", + }), + ); + expect(issues).toHaveLength(1); + expect(issues[0]).toContain("empty compact findingsDigest"); + }); + + test("passes asking how to close after listing the finding", () => { + expect( + check( + "inspect-goal-delivers-findings", + outcome({ + allCalls: [ + question( + "inclusiveRangeLength omits the endpoint for 1..3 (off-by-one). Defer or abandon?", + ), + ], + finalText: + "Inspect found the interval bug. How do you want to close?", + }), + ), + ).toEqual([]); + }); +}); diff --git a/tests/prompt-quality.test.ts b/tests/prompt-quality.test.ts index f194cff..5f39cc8 100644 --- a/tests/prompt-quality.test.ts +++ b/tests/prompt-quality.test.ts @@ -253,6 +253,23 @@ describe("Flow prompt structure", () => { expect(compileFlowPromptSurface("flow-reviewer")).toContain("scopeBlocker"); }); + test("names findingsDigest on auto and run handback surfaces", () => { + expect(compileFlowPromptSurface("flow-auto")).toContain("findingsDigest"); + expect(compileFlowPromptSurface("flow-run")).toContain("findingsDigest"); + expect(getFlowGuidance("flow-run").content).toContain("findingsDigest"); + expect(getFlowGuidance("flow").content).toContain("findingsDigest"); + }); + + test("continues a reviewer matrix only when the packet asked for one", () => { + const reviewer = compileFlowPromptSurface("flow-reviewer"); + expect(reviewer).toContain("riskLenses"); + expect(reviewer).toContain("packet summary includes a matrix"); + expect(getFlowGuidance("flow-plan").content).toContain("inspect-only"); + expect(getFlowGuidance("flow-plan").content).toContain( + "no repair features", + ); + }); + test("retires stale projection vocabulary", () => { for (const surface of SURFACES) { expect(compileFlowPromptSurface(surface)).not.toContain( From 69c8c8925bdd6b97a7420c8eca1c39a23f15fea0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 19 Aug 2026 17:45:10 +0000 Subject: [PATCH 5/7] Complete inspect features with blockers so later slices start without reset. Optional PlanFeature.kind hydrates absent to change. A failed inspect review finishes that survey slice, so the next feature can start without reset. Co-authored-by: vriesd --- ...005-flow-v6-session-v5-simplicity-first.md | 9 +++- skills/flow-plan/SKILL.md | 12 +++-- skills/flow-review/SKILL.md | 8 +-- src/application/schema.ts | 1 + src/domain/session-invariants.ts | 39 +++++++++------ src/domain/session.ts | 16 ++++++ src/domain/transitions.ts | 17 +++++-- tests/documentation-contract.test.ts | 9 ++++ tests/domain-transitions.test.ts | 50 +++++++++++++++++++ tests/prompt-quality.test.ts | 1 + tests/runtime-gates.test.ts | 2 +- 11 files changed, 135 insertions(+), 29 deletions(-) diff --git a/docs/adr/0005-flow-v6-session-v5-simplicity-first.md b/docs/adr/0005-flow-v6-session-v5-simplicity-first.md index 87cedc6..08e1898 100644 --- a/docs/adr/0005-flow-v6-session-v5-simplicity-first.md +++ b/docs/adr/0005-flow-v6-session-v5-simplicity-first.md @@ -4,7 +4,8 @@ Date: 2026-07-20 ## Status -Accepted. Supersedes ADR 0003 and ADR 0004; amended by ADR 0008. +Accepted. Supersedes ADR 0003 and ADR 0004; amended by ADR 0008 and by optional +`PlanFeature.kind`. ## Context @@ -57,7 +58,11 @@ implicitly. From blocked status, reset may atomically start one explicitly chosen retry or untouched dependency-independent feature through optional `nextFeatureId`. Once that failed run is superseded and status is ready, explicit `flow_run_start(featureId)` starts its authorized retry; this adds no -hold or retry ledger. Every accepted close returns a deterministic delivery +hold or retry ledger. `PlanFeature.kind` is optional `change` or `inspect`; +absent is `change`. An inspect feature completes after review even with +blockers, so a survey can continue without reset. `completed` then means the +survey finished, not that the tree is clean. Repair is a later change feature. +Every accepted close returns a deterministic delivery derived from canonical Session data instead of asking the conversation to reconstruct the result. Exact replay re-confirms existing active bytes and the archive/cleanup durability boundaries without rewriting Session v5. A true diff --git a/skills/flow-plan/SKILL.md b/skills/flow-plan/SKILL.md index 2c2b7f0..5015366 100644 --- a/skills/flow-plan/SKILL.md +++ b/skills/flow-plan/SKILL.md @@ -60,7 +60,8 @@ Save one plan with: gate is the command every broad observation must run; a failed or claimed-broad gate still vetoes review. - `features`: ordered outcome slices, each with a stable `id`, `title`, - `summary`, bounded `targets`, concrete `validation`, and `dependsOn` ids. + `summary`, bounded `targets`, concrete `validation`, `dependsOn` ids, and + optional `kind`. Each feature needs one observable outcome judgeable from bounded evidence and focused validation. Split only independent failures or true dependencies; file @@ -76,10 +77,11 @@ A `validation` entry naming a command is recorded byte-for-byte; prose there sta reviewer judgment, never a fabricated result. If the request is inspect-only (review, audit, survey, no promised edit), invent -no repair features. Save at most a small set of inspect features whose -`validation` is reviewer inspection, whose `targets` are existing paths, and -whose `decisions` state that no source edit is authorized. The gate may be the -repo's existing check. Ask before turning an inspect request into repairs. +no repair features. Save at most a small set of inspect features with +`kind: "inspect"`, whose `validation` is reviewer inspection, whose `targets` +are existing paths, and whose `decisions` state that no source edit is +authorized. The gate may be the repo's existing check. Ask before turning an +inspect request into repairs. Before saving, confirm: diff --git a/skills/flow-review/SKILL.md b/skills/flow-review/SKILL.md index 96d5024..5ecde56 100644 --- a/skills/flow-review/SKILL.md +++ b/skills/flow-review/SKILL.md @@ -70,8 +70,8 @@ not a failure; a missing or conflicting baseline fact, or a material mode, platform, race, or failure-path claim without proof, is. Flow deliberately projects no raw command output; use the durable command, exit -code, completeness, digest, source binding, and your workspace inspection. A weak or -unclear coverage claim is an evidence gap. +code, completeness, digest, source binding, and your workspace inspection. A weak +coverage claim is an evidence gap. For a final assignment, also inspect broad validation and confirm docs, commands, package surfaces, and remaining gaps are consistent with completion. @@ -85,8 +85,8 @@ IDs in summary or evidence. Report every problem you find. Severity is a routing decision the runtime acts on, not a filter on what to mention: `blocking` when the issue invalidates the -approved outcome, `advisory` otherwise. When you are unsure, report it as -`advisory` rather than omitting it. +approved outcome, `advisory` otherwise. If unsure, report it as +`advisory` rather than omitting it. Inspect kind completes with blockers. Set `scopeBlocker: true` on a blocking finding whose repair requires material work outside the approved plan, and identify the boundary in `evidence`. The diff --git a/src/application/schema.ts b/src/application/schema.ts index e35b188..452b0b8 100644 --- a/src/application/schema.ts +++ b/src/application/schema.ts @@ -80,6 +80,7 @@ const PlanFeatureSchema = z .max(MAX_PLAN_FEATURES) .default([]), dependsOn: z.array(FeatureIdSchema).max(MAX_PLAN_FEATURES).default([]), + kind: z.enum(["change", "inspect"]).optional(), }) .strict(); diff --git a/src/domain/session-invariants.ts b/src/domain/session-invariants.ts index e4f433e..a033cf7 100644 --- a/src/domain/session-invariants.ts +++ b/src/domain/session-invariants.ts @@ -3,31 +3,34 @@ import { MAX_REVIEW_FINDINGS, MAX_VALIDATIONS_PER_RUN } from "./limits.js"; import { closureOperationIssue } from "./operation.js"; import { planIssue } from "./plan.js"; import type { Session } from "./session.js"; -import { reviewResultSemanticIssues } from "./session.js"; +import { featureKind, reviewResultSemanticIssues } from "./session.js"; import { isFeatureComplete } from "./transitions.js"; import { isValidationEligible } from "./validation.js"; /** - * Whether the feature already held a passing review before the given revision. + * Whether the feature was already settled before the given revision. * - * Read only to re-derive a review's `kind`. A review is `final` when it is the - * last feature still outstanding, which is a fact about the whole session at the - * moment the review was created -- so checking a stored `kind` means reconstructing - * that moment from revision numbers rather than from the session as it stands now. + * Used only to re-derive a review's `kind`. A review is `final` when it is the + * last feature still outstanding. Change features settle on a passing review. + * Inspect features settle on any recorded result, including a failed survey. */ -function featurePassedBefore( +function featureSettledBefore( session: Session, featureId: string, revision: number, ): boolean { + const inspect = + featureKind( + session.plan?.features.find((feature) => feature.id === featureId), + ) === "inspect"; return session.runs.some( (run) => run.featureId === featureId && - run.reviews.some( - (review) => - review.result?.verdict === "passed" && - review.result.recordedRevision < revision, - ), + run.reviews.some((review) => { + const result = review.result; + if (!result || result.recordedRevision >= revision) return false; + return result.verdict === "passed" || inspect; + }), ); } @@ -220,7 +223,7 @@ export function sessionInvariantIssues(session: Session): string[] { const expectedKind = session.plan.features.every( (feature) => feature.id === run.featureId || - featurePassedBefore(session, feature.id, review.createdRevision), + featureSettledBefore(session, feature.id, review.createdRevision), ) ? "final" : "feature"; @@ -252,7 +255,15 @@ export function sessionInvariantIssues(session: Session): string[] { issues.push(`Active run '${run.id}' contains a recorded outcome.`); } if (run.state === "completed" && last?.result?.verdict !== "passed") { - issues.push(`Completed run '${run.id}' lacks a passing review.`); + const inspect = + featureKind( + session.plan?.features.find( + (feature) => feature.id === run.featureId, + ), + ) === "inspect"; + if (!(inspect && last?.result?.verdict === "failed")) { + issues.push(`Completed run '${run.id}' lacks a passing review.`); + } } if (run.state === "blocked" && last?.result?.verdict !== "failed") { issues.push(`Blocked run '${run.id}' lacks a failed review.`); diff --git a/src/domain/session.ts b/src/domain/session.ts index cc0e58d..42f89ff 100644 --- a/src/domain/session.ts +++ b/src/domain/session.ts @@ -12,6 +12,8 @@ export type SourceDigest = `sha256:${string}`; export type Artifact = Readonly<{ path: string }>; +export type FeatureKind = "change" | "inspect"; + export type PlanFeature = Readonly<{ id: FeatureId; title: string; @@ -19,6 +21,13 @@ export type PlanFeature = Readonly<{ targets: string[]; validation: string[]; dependsOn: FeatureId[]; + /** + * Outcome slice kind. Absent hydrates to `change`. + * + * `inspect` records findings without a repair loop. A failed review completes + * the survey slice so later features can start without reset. + */ + kind?: FeatureKind | undefined; }>; type EvidenceScope = "gate" | "extra"; @@ -69,6 +78,13 @@ export function planGate(plan: Plan | null | undefined): string | undefined { return planEvidence(plan).find((entry) => entry.scope === "gate")?.command; } +/** Absent `kind` is `change`, so existing Session v5 documents keep the repair loop. */ +export function featureKind( + feature: PlanFeature | undefined | null, +): FeatureKind { + return feature?.kind === "inspect" ? "inspect" : "change"; +} + export type ValidationScope = "focused" | "broad"; /** diff --git a/src/domain/transitions.ts b/src/domain/transitions.ts index 65510f8..61adbe3 100644 --- a/src/domain/transitions.ts +++ b/src/domain/transitions.ts @@ -21,7 +21,11 @@ import type { SessionStatus, SourceDigest, } from "./session.js"; -import { planEvidence, reviewResultSemanticIssues } from "./session.js"; +import { + featureKind, + planEvidence, + reviewResultSemanticIssues, +} from "./session.js"; import { FlowTransitionError } from "./transition-error.js"; import { evidenceRefusal, @@ -603,6 +607,10 @@ export function completeFeature( input.result.findings, findingIdPrefix(run.featureId, assignment.createdRevision), ); + const inspect = + featureKind( + session.plan?.features.find((feature) => feature.id === run.featureId), + ) === "inspect"; const next = commit( session, "feature-complete", @@ -614,7 +622,10 @@ export function completeFeature( if (item.id !== run.id) return item; return { ...item, - state: input.result.verdict === "passed" ? "completed" : "blocked", + state: + input.result.verdict === "passed" || inspect + ? "completed" + : "blocked", summary: input.summary, reviews: item.reviews.map((review) => review.id === assignment.id @@ -754,7 +765,7 @@ export function closeSession( fail("sessionId does not match active state."); assertMutable(session); if (input.kind === "completed" && sessionStatus(session) !== "completed") { - fail("A completed close requires every planned feature to pass review."); + fail("A completed close requires every planned feature to be complete."); } if (input.kind === "completed") { const unsatisfied = unsatisfiedExtraEvidence(session); diff --git a/tests/documentation-contract.test.ts b/tests/documentation-contract.test.ts index d7f245c..ce2070d 100644 --- a/tests/documentation-contract.test.ts +++ b/tests/documentation-contract.test.ts @@ -514,6 +514,15 @@ describe("Flow v6 documentation contract", () => { ); expect(optionalPlanFields).toEqual(["evidence"]); + const featureBlock = sessionSource.slice( + sessionSource.indexOf("export type PlanFeature ="), + sessionSource.indexOf("type EvidenceScope"), + ); + const optionalFeatureFields = [ + ...featureBlock.matchAll(/^\t(\w+)\?:/gm), + ].map((match) => match[1]); + expect(optionalFeatureFields).toEqual(["kind"]); + const entryBlock = sessionSource.slice( sessionSource.indexOf("export type EvidenceEntry ="), sessionSource.indexOf("export type ObservedAssertion"), diff --git a/tests/domain-transitions.test.ts b/tests/domain-transitions.test.ts index db836a5..4cadba6 100644 --- a/tests/domain-transitions.test.ts +++ b/tests/domain-transitions.test.ts @@ -10,6 +10,7 @@ import type { EvidenceEntry, EvidencePlatform, FeatureId, + FeatureKind, ObservedAssertion, Plan, ReviewAssignment, @@ -18,6 +19,7 @@ import type { ValidationScope, } from "../src/domain/session.js"; import { planGate } from "../src/domain/session.js"; +import { sessionInvariantIssues } from "../src/domain/session-invariants.js"; import { approvePlan, closeSession, @@ -778,6 +780,54 @@ describe("Session v5 domain state machine", () => { ).toBe(false); }); + test("an inspect feature completes with blockers so the next feature can start", () => { + const kind: FeatureKind = "inspect"; + const environment = deterministicEnvironment(); + const inspectPlan: Plan = { + ...plan, + features: plan.features.map((feature) => ({ ...feature, kind })), + }; + let session = begin( + approve(saveDraft(environment, { plan: inspectPlan })), + FOUNDATION, + environment, + ); + session = validate(session, { + id: "inspect-foundation-validation", + featureId: FOUNDATION, + scope: "focused", + }); + const first = requestReview(session, FOUNDATION, environment); + session = rejectReview(first.session, FOUNDATION, first.assignment); + + expect(session.runs[0]?.state).toBe("completed"); + expect(sessionStatus(session)).toBe("ready"); + expect(compactProjection(session).nextAction).toBe("flow_run_start"); + expect(sessionInvariantIssues(session)).toEqual([]); + expect(SessionSchema.safeParse(structuredClone(session)).success).toBe( + true, + ); + + session = begin(session, DELIVERY, environment, "start-after-inspect"); + expect(session.runs[0]?.state).toBe("completed"); + expect(session.runs[1]).toMatchObject({ + featureId: DELIVERY, + state: "active", + }); + expect(sessionStatus(session)).toBe("running"); + + session = validate(session, { + id: "inspect-delivery-validation", + featureId: DELIVERY, + }); + const second = requestReview(session, DELIVERY, environment); + session = rejectReview(second.session, DELIVERY, second.assignment); + expect(session.runs[1]?.state).toBe("completed"); + expect(sessionStatus(session)).toBe("completed"); + expect(compactProjection(session).nextAction).toBe("flow_session_close"); + expect(sessionInvariantIssues(session)).toEqual([]); + }); + test("requires explicit retries while independent untouched work continues", () => { const environment = deterministicEnvironment(); const templateFeature = plan.features[0]; diff --git a/tests/prompt-quality.test.ts b/tests/prompt-quality.test.ts index 5f39cc8..1a5e6a7 100644 --- a/tests/prompt-quality.test.ts +++ b/tests/prompt-quality.test.ts @@ -268,6 +268,7 @@ describe("Flow prompt structure", () => { expect(getFlowGuidance("flow-plan").content).toContain( "no repair features", ); + expect(getFlowGuidance("flow-plan").content).toContain('kind: "inspect"'); }); test("retires stale projection vocabulary", () => { diff --git a/tests/runtime-gates.test.ts b/tests/runtime-gates.test.ts index f8f122c..96e0498 100644 --- a/tests/runtime-gates.test.ts +++ b/tests/runtime-gates.test.ts @@ -1181,7 +1181,7 @@ describe("Flow application runtime gates", () => { }); expectError(prematureClose); expect(prematureClose.summary).toContain( - "requires every planned feature to pass review", + "requires every planned feature to be complete", ); repository.sourceDigest = SOURCE_B; From 8c15fce04e26e2b93ba3350d96534da983840d0f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 19 Aug 2026 17:46:01 +0000 Subject: [PATCH 6/7] Expose optional PlanFeature.kind on the OpenCode plan-save schema. The host schema is a separate zod tree from the application schema. Without this field the host rejected inspect plans the runtime already accepted. Co-authored-by: vriesd --- src/platform/opencode/tools.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/platform/opencode/tools.ts b/src/platform/opencode/tools.ts index 0241025..7f3ea7a 100644 --- a/src/platform/opencode/tools.ts +++ b/src/platform/opencode/tools.ts @@ -95,6 +95,7 @@ const planFeature = host targets: host.array(text).max(MAX_PLAN_FEATURES).default([]), validation: host.array(text).max(MAX_PLAN_FEATURES).default([]), dependsOn: host.array(featureId).max(MAX_PLAN_FEATURES).default([]), + kind: host.enum(["change", "inspect"]).optional(), }) .strict(); const plan = host From 791b9629600687f63c2f4ea4c9185599379aecff Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 19 Aug 2026 20:31:07 +0000 Subject: [PATCH 7/7] Prepare 8.1.0 so merge to main cuts the release. Bump the package and install pins, and write dated notes. An untagged package version on main creates v8.1.0 and publishes. Pushing that tag from this PR would release before merge. Co-authored-by: vriesd --- CHANGELOG.md | 19 +++++++++++++++++++ README.md | 4 ++-- docs/troubleshooting.md | 4 ++-- package.json | 2 +- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e47748..07b7df4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ One short entry per release, written for users deciding whether to upgrade. +## [8.1.0] - 2026-08-19 + +Inspect surveys can finish with blockers, and `/flow-auto` hands back a findings list. + +- Compact `flow_status` and close delivery now carry a derived `findingsDigest` + over every identified finding across attempts. `/flow-auto` prints that list + when it parks or stops instead of going silent after a blocked review. +- **Session v5 schema:** `PlanFeature.kind` is optional `change` or `inspect`. + Absent is `change`. A failed inspect review completes that slice so later + features can start without `flow_feature_reset`. `completed` then means the + survey finished, not that the tree is clean. Existing documents keep the + repair loop. + +Install or update: + +```bash +opencode plugin opencode-plugin-flow@8.1.0 --global --force +``` + ## [8.0.0] - 2026-08-18 One evidence record, a thinner run loop, and a freeze on further declarations. diff --git a/README.md b/README.md index c4d7272..8ef605c 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ expensive, and it is overhead when it is not. Install the exact npm release through OpenCode: ```bash -opencode plugin opencode-plugin-flow@8.0.0 --global --force +opencode plugin opencode-plugin-flow@8.1.0 --global --force ``` Omit `--global` for project scope. Version pins are exact and never update on @@ -51,7 +51,7 @@ The equivalent manual project configuration is: ```json { "$schema": "https://opencode.ai/config.json", - "plugin": ["opencode-plugin-flow@8.0.0"] + "plugin": ["opencode-plugin-flow@8.1.0"] } ``` diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b529532..f04c848 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5,7 +5,7 @@ Rerun OpenCode's exact-version npm plugin command: ```bash -opencode plugin opencode-plugin-flow@8.0.0 --global --force +opencode plugin opencode-plugin-flow@8.1.0 --global --force ``` Or confirm that the relevant `opencode.json` contains the exact npm plugin @@ -14,7 +14,7 @@ entry: ```json { "$schema": "https://opencode.ai/config.json", - "plugin": ["opencode-plugin-flow@8.0.0"] + "plugin": ["opencode-plugin-flow@8.1.0"] } ``` diff --git a/package.json b/package.json index f67c8d4..c5c027c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "opencode-plugin-flow", - "version": "8.0.0", + "version": "8.1.0", "description": "Small durable planning, validation, and review workflow for OpenCode", "type": "module", "repository": {