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. 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/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/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/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/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/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/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/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": { 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..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 @@ -75,6 +76,13 @@ 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 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: - 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..5ecde56 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 @@ -72,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. @@ -87,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/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/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/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/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/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/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/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/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 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", 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/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/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/prompt-quality.test.ts b/tests/prompt-quality.test.ts index f194cff..1a5e6a7 100644 --- a/tests/prompt-quality.test.ts +++ b/tests/prompt-quality.test.ts @@ -253,6 +253,24 @@ 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", + ); + expect(getFlowGuidance("flow-plan").content).toContain('kind: "inspect"'); + }); + test("retires stale projection vocabulary", () => { for (const surface of SURFACES) { expect(compileFlowPromptSurface(surface)).not.toContain( 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..96e0498 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, + }), + ], }); }); @@ -1157,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; 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();