diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index a38d8a3..65f351e 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -8,6 +8,11 @@ "name": "specflow", "source": "./plugins/specflow", "description": "Grid Dynamics SpecFlow Marketplace plugin" + }, + { + "name": "specflow2", + "source": "./plugins/specflow2", + "description": "Refine specifications before you build. Independent subagents produce ID-keyed readings under different adversarial lenses; a local CLI compares them without using the SpecFlow backend. Your coding-agent and model-provider data policies still apply." } ] } diff --git a/.gitignore b/.gitignore index cb8b9e2..b959c0d 100644 --- a/.gitignore +++ b/.gitignore @@ -10,7 +10,7 @@ gain.json # Python __pycache__/ -*.py[cod +*.py[cod] *$py.class *.so .Python @@ -20,8 +20,8 @@ dist/ downloads/ eggs/ .eggs/ -lib/ -lib64/ +/lib/ +/lib64/ parts/ sdist/ var/ diff --git a/agents/IMPLEMENTATION.md b/agents/IMPLEMENTATION.md index fd8ae9b..3cc39a1 100644 --- a/agents/IMPLEMENTATION.md +++ b/agents/IMPLEMENTATION.md @@ -2,6 +2,26 @@ **Updated**: June 23, 2026 | **Branch**: (current) | **Tests**: 1228+ passing (`make unit-tests`) +## Active Work + +- **PR 59 refinement-plugin presentability fixes (Aug 4)**: Discovery and reviewed + implementation proposal are in + `agents/plans/pr59-presentability/pr59-presentability-{SPECS,PLAN}.md`. + Implementation now uses manifested lens expectations, required grid IDs for + deterministic comparison, exact-id resolution validation, branch-local + marketplace installation, and accurate model-provider data-flow language. + Non-test validation passed: direct CLI flow reported a missing expected lens, + compared only shared cell IDs, ignored case/whitespace-only answer differences, + kept same-anchor findings separate, rejected unknown/invalid resolutions, and + accepted a valid one; local marketplace dry-run used the checkout path. A fresh + wheel contained all three `services/refine_*.py` modules (release workflow + replaces the placeholder package version from the tag). Validation: 81 focused + refinement tests and 834 full unit tests pass; Ruff passes on every changed + Python file. Repository-wide `make check` remains blocked by two unchanged + backend Ruff findings (`claude_code.py` duplicate `run_git` import and + `test_workspace_git_hooks.py` unused `pytest` import); user chose to keep PR 59 + scoped rather than add unrelated cleanup. + ## Core Systems (Production Ready) - **State**: `backend/app/state/` — estimation_state_machine, workspace_state_machine, transitions, workflow_orchestrator diff --git a/docs/specflow-2.0/refinement-loop.html b/docs/specflow-2.0/refinement-loop.html new file mode 100644 index 0000000..25686aa --- /dev/null +++ b/docs/specflow-2.0/refinement-loop.html @@ -0,0 +1,284 @@ +SpecFlow 2.0 — how spec gaps get found + + + +
+ +
+

SpecFlow 2.0 · /specflow-refine · local artifacts, no SpecFlow backend

+

Read one spec six ways at once. Where ID-keyed readings diverge, inspect the spec.

+
+ +
+ +
+

One round

+
+ +
+ model + List the decisions the spec implies + Answers none of them. This is the exam paper. +
+
+ +
+ ×6 + Six agents answer it, in parallel, blind to each other + One failure mode each. Every guess marked as a guess. + + concurrencyorderingidempotencyauthpartial failuredata lifecycle + + +
+
+ +
+ each + Also draws its own grid, then fills every cell + Rows × columns from its own angle — held resources × collisions, + entities × lifecycle events. Axes first, answers second. A cell it + cannot answer, with the reason, is the best output there is. +
+
+ +
+ ×1 + Can all six answers be true at once? + Reads everything, so it reports problems but never votes. +
+
+ +
+ code + Compare answers carrying the same grid id + No fuzzy prose matching. Same input, same output, every time. +
+
+ +
+ model + Decide what is worth your attention + Ask, assume, or drop. Nothing ranks it for you. +
+
+ +
+ code + Write the decision into the spec file + Recorded, so that exact blocker id is suppressed later. +
+
+ +

↻ Another round, or stop. Round two runs against the spec your + decisions changed, so it reaches questions that only exist downstream of them.

+ +

+ code — reproducible + model — judgment +

+
+ +
+

What comes out

+
+ +
+

Disagreement

+

Two agents answered the same shared grid cell differently. + Evidence to inspect, not proof of a defect. File and section attached.

+
+ +
+

The spec cannot answer this

+

A lens reached a cell in its own grid and could not fill it. + The question existed because the axes forced it. Closest thing to + what building used to surface.

+
+ +
+

Nobody answered

+

A decision no reading even reached. + The one real coverage number: cells filled / cells listed.

+
+ +
+

All agreed, all guessing

+

Consensus over a spec that said nothing. + A shared blind spot. Disagreement alone cannot see it.

+
+ +
+

Never produced

+

readiness score · pass/fail gate · “you have converged” · + “the spec is complete”

+
+
+
+
+ + + +
diff --git a/docs/specflow-2.0/testing-the-refine-loop.md b/docs/specflow-2.0/testing-the-refine-loop.md new file mode 100644 index 0000000..ea538e4 --- /dev/null +++ b/docs/specflow-2.0/testing-the-refine-loop.md @@ -0,0 +1,374 @@ +# Testing the refinement loop + +Four levels, cheapest first. They test different things, and only the last one +tests the product hypothesis — so read level 3 before concluding that a green +level 0–2 means the loop works. + +| Level | What it proves | Cost | +|---|---|---| +| 0 — unit tests | the comparison is correct | seconds | +| 1 — CLI by hand | the plumbing works, with no model involved | ~5 minutes | +| 2 — the full loop | six lenses, a grid, coherence, a second round | a real run | +| 3 — planted ambiguity | **that the loop finds spec defects at all** | a real run + a spec you know | + +Levels 0 and 1 are deterministic and belong in every change. Levels 2 and 3 +involve a model, so they are observations rather than tests, and each one needs a +human to read the result. + +There is no level for "did it converge". The loop has no stop rule and reports no +verdict — see the plugin README for why that inference was removed rather than +fixed. + +--- + +## Level 0 — the unit tests + +```bash +make unit-tests +``` + +That runs the backend suite and then the MCP-server suite. For just this feature: + +```bash +cd mcp_server && uv run pytest tests/test_refine.py tests/test_refine_commands.py -v +``` + +Before the branch is released to PyPI/default branch, exercise its install path +from the checkout: + +```bash +cd mcp_server +uv tool install --force . +specflow plugin install --target claude \ + --marketplace "$(git rev-parse --show-toplevel)" --dry-run +``` + +The dry run must point the marketplace-add command at this checkout. The released +default continues to use `griddynamics/specflow`. + +- `tests/test_refine.py` — comparison: what counts as a disagreement, how blockers + merge, grid coverage, coherence attribution, and how malformed readings are + handled. +- `tests/test_refine_commands.py` — the command layer: exit codes, the payload keys + the skills read, `--root-path` resolution, and (in `TestNoStopRule`) that no + convergence signal has crept back in. + +There is deliberately no test that a reading is "complete", that a spec is "ready", +or that a round converged. Those are judgments the skill makes out loud; a test +asserting them would only pin down an arbitrary threshold. + +--- + +## Level 1 — drive the CLI by hand, with no model + +**This is the level worth knowing.** It separates the deterministic half of the +product from the model half: if a round looks wrong, running it here tells you +whether the CLI or the subagents produced the wrong answer. + +Everything below is copy-pasteable, in bash or zsh. From a repo checkout the CLI is +`uv run python -m cli` from inside `mcp_server/`; installed from PyPI it is +`specflow`. The examples call `sf`, which you define as whichever you have — a +function rather than a variable, because zsh does not word-split `$SF`. + +### Set up a throwaway project + +```bash +export DEMO=$(mktemp -d) +mkdir -p "$DEMO/specs" "$DEMO/docs" +cat > "$DEMO/specs/booking.md" <<'EOF' +# Booking + +## Holds +A user may hold a seat while completing payment. A hold has a timer. +Payment is taken after the hold is confirmed. +EOF + +# from a checkout: +cd mcp_server && sf() { uv run python -m cli "$@"; } +# or, installed: sf() { specflow "$@"; } +``` + +That spec is deliberately underdetermined. It never says what happens when the +timer expires, what the losing caller sees on a contended seat, or which wins when +expiry and payment settlement race. + +### Allocate a round + +```bash +sf --root-path "$DEMO" refine new-round --outputs docs --lens concurrency ordering +``` + +It prints the round directory and the exact filename each lens must write. Note +that `--root-path` is a **global** flag, so it comes before `refine`. + +### Write the grid + +Normally one subagent writes this. By hand: + +```bash +R="$DEMO/docs/refine/round-01" +cat > "$R/grid.json" <<'EOF' +{ + "cells": [ + {"id": "hold.timeout", "question": "A hold's timer expires — what happens to the seat?", "where": "specs/booking.md#Holds"}, + {"id": "hold.timeout.paid", "question": "The timer expires while payment is in flight — what happens?", "where": "specs/booking.md#Holds"}, + {"id": "hold.contended", "question": "Two users hold the same seat — what does the loser see?", "where": "specs/booking.md#Holds"} + ] +} +EOF +``` + +### Write two readings that disagree + +```bash +cat > "$R/reading.concurrency.json" <<'EOF' +{ + "lens": "concurrency", + "matrices": [ + { + "name": "held resource × collision", + "rows": ["seat hold", "seat inventory"], + "cols": ["second claim", "cancel", "timer expiry"], + "cells": [ + {"row": "seat hold", "col": "second claim", "value": "rejected 409", "guessed": true}, + {"row": "seat hold", "col": "cancel", "value": "seat released"}, + {"row": "seat hold", "col": "timer expiry", + "unanswerable": "spec never says who owns the timer"} + ] + } + ], + "cells": [ + {"id": "hold.timeout", "value": "seat returns to the pool", "guessed": true}, + {"id": "hold.contended", "value": "rejected with 409", "guessed": true} + ], + "decisions": [ + {"question": "Does the hold expire before the payment settles?", "value": "yes", + "where": "specs/booking.md#Holds", "guessed": true} + ], + "blockers": [ + {"id": "contended-seat-loser", "title": "What the losing caller sees", + "question": "Reject with 409, or queue?", "where": "specs/booking.md#Holds", + "options": [{"label": "reject-409", "consequence": "caller retries"}, + {"label": "queue", "consequence": "unbounded wait"}], + "recommended": "reject-409", "impact": "changes_behaviour", "reversible": false} + ] +} +EOF + +cat > "$R/reading.ordering.json" <<'EOF' +{ + "lens": "ordering", + "cells": [ + {"id": "hold.timeout", "value": "seat stays held until payment resolves", "guessed": true}, + {"id": "hold.contended", "value": "rejected with 409", "guessed": true} + ], + "decisions": [ + {"question": "does a hold expire before payment settles", "value": "no", + "where": "specs/booking.md#Holds", "guessed": true} + ], + "blockers": [] +} +EOF +``` + +Note that the free-form `decisions` word the same question differently. The CLI +does not guess that they are equivalent; deterministic comparison uses the shared +`hold.timeout` cell id. The orchestrating model may still use `decisions` as +evidence when judging the round. + +### Optionally, the coherence pass + +```bash +cat > "$R/coherence.json" <<'EOF' +{ + "blockers": [ + {"id": "timeout-races-payment", + "title": "Expiry and payment settlement have no defined order", + "question": "Which wins when both fire?", "where": "specs/booking.md#Holds", + "options": [{"label": "expiry-wins", "consequence": "paid user loses the seat"}, + {"label": "payment-wins", "consequence": "hold outlives its timer"}], + "recommended": "payment-wins", "impact": "changes_architecture", "reversible": false} + ] +} +EOF +``` + +### Compare + +```bash +sf --root-path "$DEMO" refine round --outputs docs +``` + +Eight things in that output are worth checking, because each is a behaviour that has +been wrong at some point: + +1. **`2 lenses: concurrency, ordering`** — the count is of readings that could be + compared, not files on disk. Break one file and it becomes `1 of 2 readings + compared`. + +2. **`concurrency · held resource × collision — 2/6 answered`** — the lens's own + cross-product. Six intersections exist because it named 2 rows × 3 columns, and + it accounted for three of them. + +3. **`spec cannot say seat hold × timer expiry`** with the reason attached — the + payoff. That question exists only because the axes forced it, and it is listed + apart from `never filled`, which marks cells the lens enumerated and abandoned. +4. **`hold.timeout` is one disagreement keyed by its grid id.** Case and + surrounding-whitespace-only answer differences would not count; semantic + equivalence remains for the model to judge. +5. **Three open blockers** — the lens-authored contention blocker, the grid + disagreement, and the coherence blocker. Sharing the same `where` never merges + unrelated findings. +6. **`hold.timeout.paid` is listed as answered by nobody.** A cell no reading + reached never shows up as a disagreement — this is the only place it appears. +7. **`hold.contended` is listed as agreed-but-guessed.** Consensus over a silent + spec is not evidence. +8. **The coherence blocker is attributed to `coherence`** and is *not* counted as + a third lens. + +### Re-run it + +```bash +sf --root-path "$DEMO" refine round --outputs docs --json > /tmp/a.json +sf --root-path "$DEMO" refine round --outputs docs --json > /tmp/b.json +diff /tmp/a.json /tmp/b.json && echo "identical" +``` + +Byte-identical. Re-running a round replaces its findings and keeps no history of +having been run, so nothing about the *number of times you ran it* can leak into +what it reports. + +### Record a decision + +```bash +sf --root-path "$DEMO" refine resolve --outputs docs \ + --id timeout-races-payment --choice payment-wins \ + --applied-to specs/booking.md --source user +sf --root-path "$DEMO" refine status --outputs docs +``` + +`Open blockers` drops to 2 immediately, without re-running the round. Running the +same `resolve` twice, resolving an unknown id, or choosing a label outside the +blocker's options is refused with exit code 2. + +### Check the failure paths + +None of these may produce a traceback. Two of them exit `2` with a message naming +the file; the middle one deliberately does not. + +```bash +# no such round → exit 2 +sf --root-path "$DEMO" refine round --outputs docs --round 9; echo "exit=$?" + +# one broken reading → exit 0, "1 of 2 readings compared", the other lens still counted +echo '{not json' > "$R/reading.ordering.json" +sf --root-path "$DEMO" refine round --outputs docs; echo "exit=$?" + +# broken grid → exit 2 +echo '{"cells": 1}' > "$R/grid.json" +sf --root-path "$DEMO" refine round --outputs docs; echo "exit=$?" +``` + +The asymmetry is deliberate. A missing or malformed **reading** — bad JSON or the +wrong container shape — is reported under `Readings that could not be compared` +and the round continues, because six subagents write those concurrently and one +bad file must not throw away five good ones. Only a round where *nothing* is +comparable refuses. A missing, empty, or malformed **grid** refuses outright for +manifested rounds, because its ids are the exam every lens sat: quietly treating a +broken one as absent would report a round with no comparison identity as a round +with no disagreement. + +### Tear down + +```bash +rm -rf "$DEMO" +``` + +--- + +## Level 2 — the full loop + +``` +/specflow-refine specs docs +``` + +What to look at afterwards, in order: + +1. **`docs/refine/round-01/` — did every lens write a file?** A missing file is a + subagent that failed, and the round saw proportionally less. The output says + `N of M readings compared` when they differ. +2. **Did each lens declare a matrix, and does it look like its own?** If all six + chose near-identical axes, they were not reading from their own angle — check + nothing leaked the grid or another lens's output into their prompts. And check + the axes were written *before* the answers: a matrix where every cell is filled + confidently is the signature of a lens that enumerated only what it could + already handle. +3. **Are `guessed: true` markers present in the readings?** A reading with no + guesses on an underdetermined spec is a lens that filled in confident + inventions, and every count downstream inherits that. +4. **Do the blockers carry a `where` pointing at a real section?** Location is what + groups a disagreement with the blocker it belongs to; a vague `where` degrades + the grouping. +5. **The `Worth knowing about this round` section.** Two files claiming the same + lens name means the fan-out sent one lens twice, so there are fewer independent + readings than the header suggests. +6. **Did the lenses actually diverge?** Zero disagreements across six lenses on a + real spec is more likely a fan-out that leaked shared context than a spec that + is unambiguous. Check that no subagent prompt contained another's output. +7. **Round two.** Run it after resolving something substantial. It should reach + questions that exist *because* of how you resolved round one; if it returns the + same blockers unchanged, check that the resolutions were recorded (they drop out + of `refine status` when they were). + +Independence is the load-bearing claim and **nothing in the CLI can check it** — a +round where six subagents shared context produces artifacts identical to a round +where they did not. It is guaranteed only by the skill's prose, and the only way +to verify it is to read the subagent prompts. + +--- + +## Level 3 — does it find anything real? + +Levels 0–2 all test whether the machinery runs. None of them tests the product +hypothesis: **that disagreement between independent readings tracks real spec +defects.** That is unproven, and it is the thing worth measuring. + +The procedure, which needs no new code: + +1. **Take a spec you consider unambiguous** — ideally one you have already built + from, so you know where the real gaps were. +2. **Run the loop and record what it finds.** These are your false positives: + findings on a spec you believe is determinate. +3. **Plant one ambiguity.** Delete a sentence that settles something, or change + one requirement so it contradicts another elsewhere. Write down exactly what + you changed and where. +4. **Run the loop again on the mutated spec.** +5. **Score two things separately:** + - **Detected** — did any blocker or disagreement correspond to the planted + defect? + - **Localized** — did it point at the requirement you actually mutated, or + somewhere adjacent? A finding that says "the spec is unclear about holds" for + a mutation in the payment section is a miss dressed as a hit. +6. **Repeat with a mutation of a different class** — a removed constraint, a + contradiction between two files, an unstated ordering assumption. The lenses + are built around failure classes, so per-class detection is what matters, not + an overall rate. + +Until step 5 has been run several times, the honest claim about this loop is "it +surfaces places independent readings diverged", not "it finds spec defects". The +`plans/specflow-2.0/` plan calls this gate P7 and says nothing should be deleted +until it is green; see `plans/specflow-2.0/status.md` for what has and has not +been done against that plan. + +--- + +## What none of these levels test + +- **Anything that only appears when code runs.** Six agents reading a spec is not + building from it. Integration defects, performance, and anything that depends on + a library actually behaving as documented are all outside what this can see. +- **Whether a resolution was any good.** The loop records the decision you made; + nothing checks it was the right one. +- **Cost.** Lens count is the cost dial and nothing measures it. If you care, + record token spend per round yourself — the plan's P2 gate asked for that + measurement and it has not been taken. diff --git a/mcp_server/cli.py b/mcp_server/cli.py index f138072..8751bc7 100644 --- a/mcp_server/cli.py +++ b/mcp_server/cli.py @@ -14,6 +14,8 @@ download-outputs Download and extract completed generation outputs clear-workspace Free a CLEANING workspace set early sessions List active generation sessions + refine Spec refinement (see services/refine_commands.py) + plugin Install the Claude Code plugin Local-only invariant: refuses to connect to non-localhost URLs unless --force is passed. No API key is ever sent in local mode. @@ -22,15 +24,18 @@ import argparse import asyncio import datetime +import inspect import json import logging import os +import shutil +import subprocess import sys from pathlib import Path from typing import Any from urllib.parse import urlparse -from services import local_env +from services import local_env, refine_commands from tui import mcp_clients logger = logging.getLogger(__name__) @@ -172,9 +177,7 @@ def _configure_env(backend_url: str, user_email: str | None) -> None: def resolve_root(root_path_arg: str | None) -> Path: """Return absolute project root. Defaults to cwd; --root-path overrides.""" - if root_path_arg: - return Path(root_path_arg).expanduser().resolve() - return Path.cwd().resolve() + return local_env.resolve_project_root(root_path_arg) # --------------------------------------------------------------------------- @@ -492,6 +495,77 @@ async def cmd_init(args: argparse.Namespace) -> int: return rc +# --------------------------------------------------------------------------- +# Plugin installation +# --------------------------------------------------------------------------- + +# The plugin ships through the marketplace, not through this package. The wheel +# carries the commands; the marketplace carries the skills that call them. So +# install means "point the tool at the published marketplace" — never copying +# files out of this install, which would fork the skills from the ones the +# marketplace serves and let the two drift. +_PLUGIN_MARKETPLACE_REPO = "griddynamics/specflow" +# Name declared by .claude-plugin/marketplace.json, not the repo name. +_PLUGIN_MARKETPLACE_NAME = "specflow-marketplace" +# Of the marketplace's two plugins, only specflow2's skills need this CLI present. +_PLUGIN_NAME = "specflow2" +_DEFAULT_PLUGIN_TARGET = "claude" +_PLUGIN_TARGETS = {_DEFAULT_PLUGIN_TARGET} + + +def _plugin_install_steps( + target: str, + marketplace_source: str = _PLUGIN_MARKETPLACE_REPO, +) -> list[list[str]]: + """Commands that install the plugin from a marketplace source into ``target``.""" + if target != "claude": # pragma: no cover - argparse restricts the choices + raise ValueError(f"Unsupported plugin target: {target!r}") + return [ + [target, "plugin", "marketplace", "add", marketplace_source], + # Qualified: the user may well have other marketplaces installed. + [target, "plugin", "install", f"{_PLUGIN_NAME}@{_PLUGIN_MARKETPLACE_NAME}"], + ] + + +def cmd_plugin_install(args: argparse.Namespace) -> int: + """Install SpecFlow by driving the target tool's own plugin CLI.""" + target = args.target + steps = _plugin_install_steps(target, args.marketplace) + + if args.dry_run: + for step in steps: + print(" ".join(step)) + return 0 + + if shutil.which(target) is None: + print( + f"'{target}' was not found on PATH, so the plugin cannot be installed.\n" + f"Install {target.capitalize()} Code first, then re-run: " + f"specflow plugin install --target {target}", + file=sys.stderr, + ) + return 1 + + for step in steps: + print(f"$ {' '.join(step)}") + result = subprocess.run(step) + if result.returncode != 0: + print( + f"'{' '.join(step)}' failed with exit code {result.returncode}. " + "Nothing further was attempted.", + file=sys.stderr, + ) + return result.returncode + + print( + "\nInstalled. The skills call this CLI, which is already on your PATH — " + "that is why they are distributed separately.\n" + "Run /specflow-refine on a spec directory; /specflow-resolve settles what " + "it finds." + ) + return 0 + + # --------------------------------------------------------------------------- # Argument parser # --------------------------------------------------------------------------- @@ -564,6 +638,8 @@ def _build_parser() -> argparse.ArgumentParser: help="Print planned actions without starting services or seeding", ) + p_init.set_defaults(func=cmd_init) + # run-generation p_run = subparsers.add_parser("run-generation", help="Upload specs and start code generation") p_run.add_argument("--spec-dir", default="specs", help="Spec directory (default: specs)") @@ -579,7 +655,10 @@ def _build_parser() -> argparse.ArgumentParser: ) # check-status - subparsers.add_parser("check-status", help="Check progress of a running generation") + p_status = subparsers.add_parser("check-status", help="Check progress of a running generation") + p_status.set_defaults(func=cmd_check_status) + + p_run.set_defaults(func=cmd_run_generation) # retry-generation p_retry = subparsers.add_parser("retry-generation", help="Retry a failed generation") @@ -590,6 +669,8 @@ def _build_parser() -> argparse.ArgumentParser: help="Generation ID (default: from specflow_session.json)", ) + p_retry.set_defaults(func=cmd_retry_generation) + # download-outputs p_dl = subparsers.add_parser( "download-outputs", help="Download and extract completed generation outputs" @@ -606,6 +687,8 @@ def _build_parser() -> argparse.ArgumentParser: help="Local directory to extract outputs into (default: docs)", ) + p_dl.set_defaults(func=cmd_download_outputs) + # clear-workspace p_clear = subparsers.add_parser( "clear-workspace", @@ -614,6 +697,8 @@ def _build_parser() -> argparse.ArgumentParser: p_clear.add_argument("--set", type=int, required=True, dest="set", help="Set number to clear") p_clear.add_argument("--yes", action="store_true", help="Skip confirmation prompt") + p_clear.set_defaults(func=cmd_clear_workspace) + # sessions p_sessions = subparsers.add_parser("sessions", help="List active generation sessions") p_sessions.add_argument( @@ -629,6 +714,8 @@ def _build_parser() -> argparse.ArgumentParser: help="Polling interval in seconds for --watch mode (default: 15)", ) + p_sessions.set_defaults(func=cmd_sessions) + # tui p_tui = subparsers.add_parser("tui", help="Launch the interactive terminal UI") p_tui.add_argument( @@ -645,6 +732,44 @@ def _build_parser() -> argparse.ArgumentParser: help="Status poll interval in seconds (default: 3)", ) + p_tui.set_defaults(func=cmd_tui) + + # refine — the whole group registers itself + refine_commands.register(subparsers) + + # plugin + p_plugin = subparsers.add_parser( + "plugin", help="Install the spec-refinement plugin into an AI coding tool" + ) + plugin_actions = p_plugin.add_subparsers(dest="plugin_command", metavar="ACTION") + plugin_actions.required = True + p_plugin_install = plugin_actions.add_parser( + "install", help=f"Install the {_PLUGIN_NAME} plugin" + ) + p_plugin_install.add_argument( + "--target", + default=_DEFAULT_PLUGIN_TARGET, + choices=sorted(_PLUGIN_TARGETS), + help=f"Which tool to install into (default: {_DEFAULT_PLUGIN_TARGET})", + ) + p_plugin_install.add_argument( + "--dry-run", + action="store_true", + dest="dry_run", + help="Print the commands that would run, without running them", + ) + p_plugin_install.add_argument( + "--marketplace", + default=_PLUGIN_MARKETPLACE_REPO, + metavar="REPO_OR_PATH", + help=( + "Marketplace repository or local checkout " + f"(default: {_PLUGIN_MARKETPLACE_REPO})" + ), + ) + + p_plugin_install.set_defaults(func=cmd_plugin_install) + return parser @@ -652,24 +777,26 @@ def _build_parser() -> argparse.ArgumentParser: # Entry point # --------------------------------------------------------------------------- -_COMMAND_MAP = { - "init": cmd_init, - "run-generation": cmd_run_generation, - "check-status": cmd_check_status, - "retry-generation": cmd_retry_generation, - "download-outputs": cmd_download_outputs, - "clear-workspace": cmd_clear_workspace, - "sessions": cmd_sessions, - "tui": cmd_tui, -} - -# Commands that operate on files and print the project root -_FILE_OPERATING_COMMANDS = { - "run-generation", - "check-status", - "retry-generation", - "download-outputs", -} +# Commands with no backend to talk to. They skip config resolution, the +# localhost guard and the env push — not as exceptions to the normal path, but +# because none of that means anything to them. `init` brings the backend up, so +# it cannot require one; `refine` and `plugin` never touch it at all. +_LOCAL_COMMANDS = {"init", "refine", "plugin"} + + +def _dispatch(handler: Any, args: argparse.Namespace) -> int: + """Run a command handler, awaiting it only if it is actually async. + + Backend commands are coroutines; the local ones are plain functions. Which + it is belongs to the handler, not to the caller, so this asks rather than + keeping a second list to fall out of step with the first. + """ + try: + result = handler(args) + return asyncio.run(result) if inspect.isawaitable(result) else result + except KeyboardInterrupt: + print("\nStopped.") + return 0 def main() -> None: @@ -682,17 +809,15 @@ def main() -> None: parser = _build_parser() args = parser.parse_args() - # `init` brings the backend UP — it must not require a reachable/localhost - # backend, and it needs no env-pushed runtime config. Dispatch it directly, - # before the localhost guard and config resolution that every other command - # runs through. - if args.command == "init": - try: - exit_code = asyncio.run(cmd_init(args)) - except KeyboardInterrupt: - print("\nStopped.") - exit_code = 0 - sys.exit(exit_code) + # Every parser carries its own handler. Resolved here rather than from a + # module-level table because the table binds function objects at import + # time, which silently defeats patching them in tests. + handler = args.func + + # Local commands run before any backend config is resolved or guarded — + # see _LOCAL_COMMANDS for why that is not an exception but the correct path. + if args.command in _LOCAL_COMMANDS: + sys.exit(_dispatch(handler, args)) # Determine root early so config resolution can read mcp-config.json root = resolve_root(getattr(args, "root_path", None)) @@ -716,13 +841,7 @@ def main() -> None: # Push config into env before importing service singletons _configure_env(backend_url, user_email) - handler = _COMMAND_MAP[args.command] - try: - exit_code = asyncio.run(handler(args)) - except KeyboardInterrupt: - print("\nStopped.") - exit_code = 0 - sys.exit(exit_code) + sys.exit(_dispatch(handler, args)) if __name__ == "__main__": diff --git a/mcp_server/services/bundled_skills.py b/mcp_server/services/bundled_skills.py index 6e0c9cf..688d36d 100644 --- a/mcp_server/services/bundled_skills.py +++ b/mcp_server/services/bundled_skills.py @@ -9,6 +9,11 @@ Source of truth: services/skills/{name}/SKILL.md — included as package data so skills are available after pip install, not just from the repo. +Two consumers, one copy: served as MCP tool responses *and* symlinked into the +`plugins/specflow/` plugin. Keep the symlink. The separate `specflow2` plugin must +never ship an analysis or planning skill, nor write to `analysis/` / `planning/` — +this flow's contract owns those names. See `plans/specflow-2.0/status.md`. + Only user-facing skills are bundled here. Developer skills (specflow-backport, deploy-requirements) live in .claude/skills/ as repo-local slash commands for SpecFlow engineers and are not distributed via the MCP server. diff --git a/mcp_server/services/local_env.py b/mcp_server/services/local_env.py index b720d22..9f54331 100644 --- a/mcp_server/services/local_env.py +++ b/mcp_server/services/local_env.py @@ -169,6 +169,13 @@ def resolve_repo_root(start: Path | None = None) -> Path | None: return repo_root(start) or installed_repo_root() +def resolve_project_root(root_path_arg: str | Path | None) -> Path: + """The user's project root from ``--root-path``, default cwd. Not ``resolve_repo_root``.""" + if root_path_arg: + return Path(root_path_arg).expanduser().resolve() + return Path.cwd().resolve() + + def env_file_path(root: Path) -> Path: return root / _ENV_FILENAME diff --git a/mcp_server/services/refine_artifacts.py b/mcp_server/services/refine_artifacts.py new file mode 100644 index 0000000..8fed7f6 --- /dev/null +++ b/mcp_server/services/refine_artifacts.py @@ -0,0 +1,225 @@ +from __future__ import annotations + +import json +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +REFINE_SUBDIR = "refine" +RESOLUTIONS_FILE = "resolutions.json" +FINDINGS_FILE = "findings.json" +GRID_FILE = "grid.json" +COHERENCE_FILE = "coherence.json" +MANIFEST_FILE = "manifest.json" +READING_PREFIX = "reading." + +@dataclass(frozen=True) +class Layout: + outputs_dir: Path + + @property + def root(self) -> Path: + return self.outputs_dir / REFINE_SUBDIR + + @property + def resolutions_path(self) -> Path: + return self.root / RESOLUTIONS_FILE + + @property + def findings_path(self) -> Path: + return self.root / FINDINGS_FILE + + def manifest_path(self, number: int) -> Path: + return self.round_dir(number) / MANIFEST_FILE + + def round_dir(self, number: int) -> Path: + return self.root / f"round-{number:02d}" + + def reading_path(self, number: int, lens: str) -> Path: + return self.round_dir(number) / f"{READING_PREFIX}{lens}.json" + + def grid_path(self, number: int) -> Path: + return self.round_dir(number) / GRID_FILE + + def coherence_path(self, number: int) -> Path: + return self.round_dir(number) / COHERENCE_FILE + + def rounds(self) -> list[int]: + if not self.root.is_dir(): + return [] + numbers = [] + for entry in self.root.iterdir(): + if entry.is_dir() and entry.name.startswith("round-"): + suffix = entry.name.removeprefix("round-") + if suffix.isdigit(): + numbers.append(int(suffix)) + return sorted(numbers) + + def latest_round(self) -> int | None: + rounds = self.rounds() + return rounds[-1] if rounds else None + + def readings(self, number: int) -> list[Path]: + directory = self.round_dir(number) + if not directory.is_dir(): + return [] + return sorted(directory.glob(f"{READING_PREFIX}*.json")) + + def has_grid(self, number: int) -> bool: + return self.grid_path(number).exists() + +def layout_for(outputs_dir: str | Path) -> Layout: + return Layout(Path(outputs_dir)) + +def read_json(path: Path) -> Any: + try: + return json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError: + raise FileNotFoundError(f"Not found: {path}") from None + except json.JSONDecodeError as exc: + raise ValueError(f"{path} is not valid JSON: {exc}") from None + +def write_json(path: Path, payload: Any) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps(payload, indent=2, ensure_ascii=False) + "\n", encoding="utf-8" + ) + return path + +def _require_object(path: Path, data: Any) -> dict[str, Any]: + if not isinstance(data, dict): + raise ValueError(f"{path} should contain an object, got {type(data).__name__}") + return data + +def _require_object_list(path: Path, data: dict[str, Any], key: str) -> None: + value = data.get(key) + if value is None: + return + if not isinstance(value, list) or not all(isinstance(i, dict) for i in value): + raise ValueError( + f"{path}: '{key}' should be a list of objects, got " + f"{type(value).__name__}" + ) + +def load_manifest(layout: Layout, number: int) -> dict[str, Any]: + path = layout.manifest_path(number) + if not path.exists(): + return {} + data = _require_object(path, read_json(path)) + lenses = data.get("lenses") + if ( + not isinstance(lenses, list) + or not lenses + or not all(isinstance(lens, str) and lens.strip() for lens in lenses) + or any( + not all(character.isalnum() or character in "-_" for character in lens) + for lens in lenses + ) + or len({lens.casefold() for lens in lenses}) != len(lenses) + ): + raise ValueError( + f"{path}: 'lenses' should be a non-empty list of unique safe names" + ) + return data + +def load_readings( + layout: Layout, + number: int, + expected_lenses: list[str] | None = None, +) -> list[dict[str, Any]]: + loaded = [] + paths = ( + [layout.reading_path(number, lens) for lens in expected_lenses] + if expected_lenses is not None + else layout.readings(number) + ) + for path in paths: + from_filename = path.stem.removeprefix(READING_PREFIX) + if not path.exists(): + loaded.append({ + "lens": from_filename, + "_path": str(path), + "_unreadable": f"missing expected reading: {path}", + }) + continue + try: + data = _require_object(path, read_json(path)) + except (ValueError, OSError) as exc: + loaded.append({ + "lens": from_filename, + "_path": str(path), + "_unreadable": str(exc), + }) + continue + declared = data.get("lens") + data["lens"] = from_filename + if declared and str(declared) != from_filename: + data["_lens_declared"] = str(declared) + data["_path"] = str(path) + loaded.append(data) + return loaded + +def load_grid( + layout: Layout, + number: int, + *, + required: bool = False, +) -> dict[str, Any]: + path = layout.grid_path(number) + if not path.exists(): + if required: + raise ValueError(f"missing required grid: {path}") + return {} + data = _require_object(path, read_json(path)) + _require_object_list(path, data, "cells") + cells = data.get("cells") or [] + ids = [cell.get("id") for cell in cells] + if required and not cells: + raise ValueError(f"{path}: 'cells' should contain at least one grid cell") + if any(not isinstance(cell_id, str) or not cell_id.strip() for cell_id in ids): + raise ValueError(f"{path}: every grid cell should have a non-empty string 'id'") + invalid_ids = [ + cell_id + for cell_id in ids + if isinstance(cell_id, str) + and not all(character.isalnum() or character in "._-" for character in cell_id) + ] + if invalid_ids: + raise ValueError( + f"{path}: grid cell ids may contain only letters, numbers, '.', '-' and '_'" + ) + normalized_ids = [str(cell_id).casefold() for cell_id in ids] + if len(set(normalized_ids)) != len(normalized_ids): + raise ValueError(f"{path}: grid cell ids should be unique") + return data + +def load_coherence(layout: Layout, number: int) -> dict[str, Any]: + path = layout.coherence_path(number) + if not path.exists(): + return {} + data = _require_object(path, read_json(path)) + _require_object_list(path, data, "blockers") + return data + +def load_findings(layout: Layout) -> dict[str, Any]: + if not layout.findings_path.exists(): + return {} + return read_json(layout.findings_path) + +def load_resolutions(layout: Layout) -> list[dict[str, Any]]: + path = layout.resolutions_path + if not path.exists(): + return [] + data = read_json(path) + if isinstance(data, dict): + _require_object_list(path, data, "resolved") + return data.get("resolved", []) + if not isinstance(data, list) or not all(isinstance(i, dict) for i in data): + raise ValueError( + f"{path} should contain a list of objects or an object with a " + f"'resolved' list, got {type(data).__name__}" + ) + return data + +def resolved_ids(layout: Layout) -> set[str]: + return {r["blocker_id"] for r in load_resolutions(layout) if "blocker_id" in r} diff --git a/mcp_server/services/refine_commands.py b/mcp_server/services/refine_commands.py new file mode 100644 index 0000000..6614cd8 --- /dev/null +++ b/mcp_server/services/refine_commands.py @@ -0,0 +1,395 @@ +from __future__ import annotations + +import argparse +import functools +import json +from collections.abc import Callable +from pathlib import Path +from typing import Any + +from services import local_env +from services import refine_artifacts as artifacts +from services import refine_compare as compare + +EXIT_OK, EXIT_USAGE = 0, 2 + +def _emit(payload: dict[str, Any], as_json: bool, human: str) -> None: + if as_json: + print(json.dumps(payload, indent=2, ensure_ascii=False)) + else: + print(human) + +def _reporting_usage_errors( + handler: Callable[[argparse.Namespace], int], +) -> Callable[[argparse.Namespace], int]: + @functools.wraps(handler) + def wrapper(args: argparse.Namespace) -> int: + try: + return handler(args) + except (ValueError, OSError) as exc: + _emit({"error": str(exc)}, getattr(args, "json", False), str(exc)) + return EXIT_USAGE + + return wrapper + +def _layout(args: argparse.Namespace) -> artifacts.Layout: + root = local_env.resolve_project_root(getattr(args, "root_path", None)) + return artifacts.layout_for(root / args.outputs) + +def _round_context(args: argparse.Namespace) -> tuple[artifacts.Layout, int]: + layout = _layout(args) + number = args.round or layout.latest_round() + if number is None: + raise ValueError("no rounds found — run new-round first") + return layout, number + +def cmd_new_round(args: argparse.Namespace) -> int: + layout = _layout(args) + if not args.lens: + raise ValueError("at least one --lens is required") + invalid_lenses = [ + lens + for lens in args.lens + if not lens + or not all(character.isalnum() or character in "-_" for character in lens) + ] + if invalid_lenses: + raise ValueError( + "--lens names may contain only letters, numbers, '-' and '_': " + + ", ".join(invalid_lenses) + ) + if len({lens.casefold() for lens in args.lens}) != len(args.lens): + raise ValueError("--lens names should be unique") + + number = (layout.latest_round() or 0) + 1 + directory = layout.round_dir(number) + directory.mkdir(parents=True, exist_ok=True) + artifacts.write_json(layout.manifest_path(number), {"lenses": args.lens}) + + write_to = [str(layout.reading_path(number, lens)) for lens in args.lens] + payload = { + "round": number, + "dir": str(directory), + "manifest": str(layout.manifest_path(number)), + "write_to": write_to, + "grid": str(layout.grid_path(number)), + "coherence": str(layout.coherence_path(number)), + } + lines = [f"Round {number} -> {directory}"] + lines.append(f" contract {artifacts.MANIFEST_FILE} (expected lenses)") + lines.append(f" grid {artifacts.GRID_FILE} (required; write before readings)") + lines += [f" expects {Path(p).name}" for p in write_to] + lines.append(f" then {artifacts.COHERENCE_FILE} (optional)") + _emit(payload, args.json, "\n".join(lines)) + return EXIT_OK + +def cmd_round(args: argparse.Namespace) -> int: + layout, number = _round_context(args) + manifest = artifacts.load_manifest(layout, number) + expected_lenses = manifest.get("lenses") + + readings = artifacts.load_readings(layout, number, expected_lenses=expected_lenses) + if not readings: + raise ValueError( + f"no readings in {layout.round_dir(number)} — " + f"each lens writes {artifacts.READING_PREFIX}.json there" + ) + + grid = artifacts.load_grid(layout, number, required=bool(manifest)) + result = compare.compare( + readings, + grid=grid, + coherence=artifacts.load_coherence(layout, number), + ) + if not result.lens_count: + raise ValueError( + f"none of the {result.readings_total} reading(s) in " + f"{layout.round_dir(number)} could be compared:\n " + + "\n ".join(result.incomplete) + ) + + resolved = artifacts.resolved_ids(layout) + open_blockers = [ + b for b in result.blockers if b.get("id") not in resolved + ] + + coverage = result.coverage + payload = { + "round": number, + "lenses": result.lenses, + "lens_count": result.lens_count, + "readings_total": result.readings_total, + "counts": { + "open": len(open_blockers), + "already_decided": len(result.blockers) - len(open_blockers), + "disagreements": len(result.disagreements), + "uncovered_cells": len(coverage.uncovered) if coverage else 0, + "agreed_guesses": len(coverage.agreed_guesses) if coverage else 0, + "matrix_unanswerable": sum(len(m.unanswerable) for m in result.matrices), + "matrix_skipped": sum(len(m.missing) for m in result.matrices), + }, + "disagreements": [d.as_dict() for d in result.disagreements], + "blockers": open_blockers, + "coverage": coverage.as_dict() if coverage else None, + "matrices": [m.as_dict() for m in result.matrices], + "incomplete_readings": result.incomplete, + "notes": result.notes, + "findings_path": str(layout.findings_path), + } + + artifacts.write_json(layout.findings_path, payload) + + _emit(payload, args.json, _render_round(payload)) + return EXIT_OK + +def _render_round(payload: dict[str, Any]) -> str: + counts = payload["counts"] + compared, total = payload["lens_count"], payload["readings_total"] + head = ( + f"{compared} lenses" + if compared == total + else f"{compared} of {total} readings compared" + ) + lines = [ + f"Round {payload['round']} — {head}: {', '.join(payload['lenses'])}", + f" open {counts['open']} already decided {counts['already_decided']} " + f"disagreements {counts['disagreements']}", + "", + ] + + if payload["incomplete_readings"]: + lines.append("Readings that could not be compared:") + lines += [f" {item}" for item in payload["incomplete_readings"]] + lines.append("") + + if payload["notes"]: + lines.append("Worth knowing about this round:") + lines += [f" {item}" for item in payload["notes"]] + lines.append("") + + if payload.get("matrices"): + lines.append("Each lens's own matrix:") + for matrix in payload["matrices"]: + lines.append( + f" {matrix['lens']} · {matrix['name']} — " + f"{matrix['answered']}/{matrix['declared']} answered" + + (f", {matrix['guessed']} guessed" if matrix["guessed"] else "") + ) + for cell in matrix["unanswerable"]: + lines.append( + f" spec cannot say {cell['row']} × {cell['col']}" + f" — {cell['why']}" + ) + for cell in matrix["missing"]: + lines.append(f" never filled {cell['row']} × {cell['col']}") + lines.append("") + + coverage = payload.get("coverage") + if coverage: + lines.append( + f"Grid: {coverage['cells_filled']}/{coverage['cells_total']} cells " + "answered by at least one lens" + ) + if coverage["uncovered"]: + lines.append(" no lens answered these:") + lines += [ + f" {cell['id']} — {cell['question']}" + for cell in coverage["uncovered"] + ] + if coverage["agreed_guesses"]: + lines.append(" agreed, but every lens was guessing:") + lines += [ + f" {cell['id']} — {cell['value']} " + f"({', '.join(cell['lenses'])})" + for cell in coverage["agreed_guesses"] + ] + lines.append("") + + if payload["blockers"]: + lines.append("Open blockers:") + for blocker in payload["blockers"]: + found = ", ".join(blocker.get("found_by", [])) or "unknown" + lines.append(f" {blocker['id']} — {blocker.get('title', '')}") + lines.append( + f" raised by {found}" + + (f" in {blocker['where']}" if blocker.get("where") else "") + ) + if blocker.get("question"): + lines.append(f" {blocker['question']}") + for option in blocker.get("options", []): + consequence = option.get("consequence") + lines.append( + f" - {option.get('label', '?')}" + + (f" — {consequence}" if consequence else "") + ) + for item in blocker.get("disagreements", []): + lines.append(f" readings disagree — {item['question']}") + for lens, answer in item["answers"].items(): + lines.append(f" {lens}: {answer}") + lines.append("") + + if counts["uncovered_cells"] or counts["agreed_guesses"]: + lines.append( + f"{counts['uncovered_cells']} cell(s) no lens answered and " + f"{counts['agreed_guesses']} answered only by agreeing guesses. " + "Neither shows up as a disagreement." + ) + + if counts["matrix_unanswerable"] or counts["matrix_skipped"]: + lines.append( + f"{counts['matrix_unanswerable']} matrix cell(s) a lens reached and " + f"reported the spec cannot answer; {counts['matrix_skipped']} it " + "enumerated and never came back to. The first is a finding, the second " + "is an incomplete reading — re-run that lens." + ) + + if payload["incomplete_readings"]: + lines.append( + f"{len(payload['incomplete_readings'])} of {payload['readings_total']} " + "readings could not be compared, so this round saw less than a whole " + "one. Fix those and re-run before reading anything into the counts." + ) + return "\n".join(lines) + +def cmd_resolve(args: argparse.Namespace) -> int: + layout = _layout(args) + existing = artifacts.load_resolutions(layout) + if any(r.get("blocker_id") == args.id for r in existing): + _emit( + {"error": "already resolved", "blocker_id": args.id}, + args.json, + f"{args.id} is already resolved.", + ) + return EXIT_USAGE + + findings = artifacts.load_findings(layout) + blocker = next( + (item for item in findings.get("blockers", []) if item.get("id") == args.id), + None, + ) + if blocker is None: + _emit( + {"error": "unknown open blocker", "blocker_id": args.id}, + args.json, + f"{args.id} is not an open blocker in the latest findings.", + ) + return EXIT_USAGE + + option_labels = [ + str(option["label"]) + for option in blocker.get("options", []) + if option.get("label") is not None + ] + if option_labels and args.choice not in option_labels: + _emit( + { + "error": "invalid choice", + "blocker_id": args.id, + "choice": args.choice, + "allowed": option_labels, + }, + args.json, + f"{args.choice!r} is not a choice for {args.id}; choose one of: " + + ", ".join(option_labels), + ) + return EXIT_USAGE + + record = { + "blocker_id": args.id, + "choice": args.choice, + "applied_to_spec": args.applied_to or [], + "source": args.source, + } + existing.append(record) + artifacts.write_json(layout.resolutions_path, {"resolved": existing}) + + _emit( + {"recorded": record, "total_resolved": len(existing)}, + args.json, + f"Recorded {args.id} -> {args.choice} ({len(existing)} resolved in total)", + ) + return EXIT_OK + +def cmd_status(args: argparse.Namespace) -> int: + layout = _layout(args) + findings = artifacts.load_findings(layout) + resolutions = artifacts.load_resolutions(layout) + + resolved = artifacts.resolved_ids(layout) + open_blockers = [ + b for b in findings.get("blockers", []) if b.get("id") not in resolved + ] + counts = dict(findings.get("counts", {})) + counts["open"] = len(open_blockers) + + payload = { + "rounds_run": len(layout.rounds()), + "resolved": len(resolutions), + "resolutions": resolutions, + "counts": counts, + "blockers": open_blockers, + "disagreements": findings.get("disagreements", []), + "coverage": findings.get("coverage"), + "matrices": findings.get("matrices", []), + } + + lines = [ + f"Rounds run {payload['rounds_run']}", + f"Resolved {payload['resolved']}", + f"Open blockers {payload['counts'].get('open', 0)}", + f"Disagreements {payload['counts'].get('disagreements', 0)}", + f"Unanswered {payload['counts'].get('uncovered_cells', 0)} grid cell(s)", + f"Agreed guesses {payload['counts'].get('agreed_guesses', 0)}", + f"Spec cannot say {payload['counts'].get('matrix_unanswerable', 0)} matrix cell(s)", + f"Lens skipped {payload['counts'].get('matrix_skipped', 0)} matrix cell(s)", + ] + if payload["blockers"]: + lines.append("") + lines.append("Still open:") + lines += [ + f" {b['id']} — {b.get('title', '')}" for b in payload["blockers"] + ] + _emit(payload, args.json, "\n".join(lines)) + return EXIT_OK + +def register(subparsers: argparse._SubParsersAction) -> None: + refine = subparsers.add_parser( + "refine", + help="Spec refinement — compare independent readings of a spec", + description=( + "Local spec refinement. Independent lenses read the spec; these " + "commands compare what they disagree about and remember what you " + "have already decided." + ), + ) + commands = refine.add_subparsers(dest="refine_command", metavar="COMMAND") + commands.required = True + + def leaf(name: str, help_text: str) -> argparse.ArgumentParser: + sub = commands.add_parser(name, help=help_text) + sub.add_argument("--outputs", default="docs", help="outputs dir (default: docs)") + sub.add_argument("--json", action="store_true", help="machine-readable output") + return sub + + def bind(parser: argparse.ArgumentParser, handler: Any) -> None: + parser.set_defaults(func=_reporting_usage_errors(handler)) + + new_round = leaf("new-round", "allocate the next round directory") + new_round.add_argument("--lens", nargs="*", default=[], help="lens names this round") + bind(new_round, cmd_new_round) + + round_cmd = leaf("round", "compare this round's readings") + round_cmd.add_argument("--round", type=int) + bind(round_cmd, cmd_round) + + resolve = leaf("resolve", "record a decision") + resolve.add_argument("--id", required=True, help="blocker id") + resolve.add_argument("--choice", required=True, help="the chosen option label") + resolve.add_argument("--applied-to", nargs="*", help="spec files updated") + resolve.add_argument( + "--source", default="user", choices=["user", "assumed"], + help="whether the user decided or the skill applied a default", + ) + bind(resolve, cmd_resolve) + + bind(leaf("status", "current refinement state"), cmd_status) diff --git a/mcp_server/services/refine_compare.py b/mcp_server/services/refine_compare.py new file mode 100644 index 0000000..aabd69d --- /dev/null +++ b/mcp_server/services/refine_compare.py @@ -0,0 +1,321 @@ +from __future__ import annotations + +import re +from dataclasses import dataclass, field +from typing import Any + +_NON_ID = re.compile(r"[^a-z0-9._-]+") + +def _normalized_answer(value: str) -> str: + return " ".join(value.split()).casefold() + +@dataclass +class Disagreement: + cell_id: str + question: str + where: str + answers: dict[str, str] = field(default_factory=dict) + + def as_dict(self) -> dict[str, Any]: + return { + "cell_id": self.cell_id, + "question": self.question, + "where": self.where, + "answers": dict(sorted(self.answers.items())), + "distinct": len({_normalized_answer(value) for value in self.answers.values()}), + } + +@dataclass +class Coverage: + cells_total: int = 0 + cells_filled: int = 0 + uncovered: list[dict[str, Any]] = field(default_factory=list) + agreed_guesses: list[dict[str, Any]] = field(default_factory=list) + + def as_dict(self) -> dict[str, Any]: + return { + "cells_total": self.cells_total, + "cells_filled": self.cells_filled, + "uncovered": self.uncovered, + "agreed_guesses": self.agreed_guesses, + } + +@dataclass +class MatrixCoverage: + lens: str + name: str + declared: int = 0 + answered: int = 0 + guessed: int = 0 + unanswerable: list[dict[str, Any]] = field(default_factory=list) + missing: list[dict[str, Any]] = field(default_factory=list) + + def as_dict(self) -> dict[str, Any]: + return { + "lens": self.lens, + "name": self.name, + "declared": self.declared, + "answered": self.answered, + "guessed": self.guessed, + "unanswerable": self.unanswerable, + "missing": self.missing, + } + +@dataclass +class Comparison: + lens_count: int + readings_total: int = 0 + lenses: list[str] = field(default_factory=list) + blockers: list[dict[str, Any]] = field(default_factory=list) + disagreements: list[Disagreement] = field(default_factory=list) + incomplete: list[str] = field(default_factory=list) + notes: list[str] = field(default_factory=list) + coverage: Coverage | None = None + matrices: list[MatrixCoverage] = field(default_factory=list) + +def _where(item: dict[str, Any]) -> str: + return str(item.get("where") or item.get("spec_anchor") or "") + +def merge_blockers(readings: list[dict[str, Any]]) -> list[dict[str, Any]]: + by_id: dict[str, dict[str, Any]] = {} + for reading in readings: + lens = reading.get("lens", "?") + for blocker in reading.get("blockers", []): + key = blocker.get("id") + if not key: + continue + existing = by_id.get(key) + if existing is None: + merged = dict(blocker) + merged["found_by"] = [lens] + by_id[key] = merged + elif lens not in existing["found_by"]: + existing["found_by"].append(lens) + if len(blocker.get("options", [])) > len(existing.get("options", [])): + existing["options"] = blocker["options"] + existing["recommended"] = blocker.get( + "recommended", existing.get("recommended") + ) + + ordered = sorted( + by_id.values(), + key=lambda b: (-len(b["found_by"]), b.get("id", "")), + ) + for blocker in ordered: + blocker["found_by"] = sorted(blocker["found_by"]) + return ordered + +def _sorted(found: list[Disagreement]) -> list[Disagreement]: + return sorted( + found, + key=lambda d: ( + -len({_normalized_answer(value) for value in d.answers.values()}), + d.cell_id, + ), + ) + +def grid_coverage( + grid: dict[str, Any], readings: list[dict[str, Any]] +) -> tuple[Coverage, list[Disagreement]]: + cells = [cell for cell in grid.get("cells", []) if cell.get("id")] + answered: dict[str, dict[str, tuple[str, bool]]] = {} + for reading in readings: + lens = reading.get("lens", "?") + for entry in reading.get("cells", []): + cell_id, value = entry.get("id"), entry.get("value") + if not cell_id or value is None: + continue + answered.setdefault(cell_id, {})[lens] = ( + str(value), bool(entry.get("guessed")) + ) + + coverage = Coverage(cells_total=len(cells)) + disagreements: list[Disagreement] = [] + for cell in cells: + filled = answered.get(cell["id"], {}) + question = str(cell.get("question") or cell["id"]) + where = _where(cell) + if not filled: + coverage.uncovered.append( + {"id": cell["id"], "question": question, "where": where} + ) + continue + + coverage.cells_filled += 1 + answers = {lens: value for lens, (value, _) in filled.items()} + if len({_normalized_answer(value) for value in answers.values()}) > 1: + disagreements.append( + Disagreement( + cell_id=str(cell["id"]), + question=question, + where=where, + answers=answers, + ) + ) + elif len(filled) > 1 and all(guessed for _, guessed in filled.values()): + coverage.agreed_guesses.append({ + "id": cell["id"], + "question": question, + "where": where, + "value": next(iter(answers.values())), + "lenses": sorted(answers), + }) + return coverage, disagreements + +def matrix_coverage(readings: list[dict[str, Any]]) -> list[MatrixCoverage]: + found: list[MatrixCoverage] = [] + for reading in readings: + lens = str(reading.get("lens", "?")) + for index, matrix in enumerate(reading.get("matrices") or []): + rows = [str(r) for r in matrix.get("rows") or []] + cols = [str(c) for c in matrix.get("cols") or []] + if not rows or not cols: + continue + entries = { + (str(cell.get("row")), str(cell.get("col"))): cell + for cell in matrix.get("cells") or [] + } + report = MatrixCoverage( + lens=lens, + name=str(matrix.get("name") or f"matrix {index + 1}"), + declared=len(rows) * len(cols), + ) + for row in rows: + for col in cols: + cell = entries.get((row, col)) + at = {"row": row, "col": col} + if cell is not None and cell.get("value") is not None: + report.answered += 1 + if cell.get("guessed"): + report.guessed += 1 + elif cell is not None and cell.get("unanswerable"): + report.unanswerable.append( + {**at, "why": str(cell["unanswerable"])} + ) + else: + report.missing.append(at) + found.append(report) + + return sorted( + found, key=lambda m: (-len(m.missing), -len(m.unanswerable), m.lens, m.name) + ) + +_OBJECT_LISTS = ("decisions", "blockers", "cells", "matrices") + +def _matrix_problems(matrix: Any, index: int) -> list[str]: + label = f"matrices[{index}]" + if not isinstance(matrix, dict): # pragma: no cover + return [f"{label} should be an object, got {type(matrix).__name__}"] + problems = [ + f"{label}.{key} should be a list of strings" + for key in ("rows", "cols") + if not isinstance(matrix.get(key), list) + or not all(isinstance(item, str) for item in matrix[key]) + ] + cells = matrix.get("cells") + if not isinstance(cells, list) or not all(isinstance(i, dict) for i in cells): + problems.append(f"{label}.cells should be a list of objects") + return problems + +def _reading_problems(reading: dict[str, Any]) -> list[str]: + unreadable = reading.get("_unreadable") + if unreadable: + return [str(unreadable)] + + problems = [ + f"missing {key}" + for key in ("lens", "decisions", "blockers") + if key not in reading + ] + for key in _OBJECT_LISTS: + if key not in reading: + continue + value = reading[key] + if not isinstance(value, list) or not all( + isinstance(item, dict) for item in value + ): + problems.append( + f"{key} should be a list of objects, got {type(value).__name__}" + ) + continue + if key == "matrices": + for index, matrix in enumerate(value): + problems += _matrix_problems(matrix, index) + return problems + +def compare( + readings: list[dict[str, Any]], + grid: dict[str, Any] | None = None, + coherence: dict[str, Any] | None = None, +) -> Comparison: + usable: list[dict[str, Any]] = [] + incomplete: list[str] = [] + notes: list[str] = [] + for reading in readings: + problems = _reading_problems(reading) + if problems: + incomplete.append( + f"{reading.get('lens', reading.get('_path', '?'))}: " + + "; ".join(problems) + ) + continue + usable.append(reading) + declared = reading.get("_lens_declared") + if declared: + notes.append( + f"{reading['lens']}: file declares lens '{declared}' — used the " + "filename; check the fan-out did not run one lens twice" + ) + + result = Comparison( + lens_count=len(usable), + readings_total=len(readings), + lenses=sorted(str(r.get("lens", "?")) for r in usable), + incomplete=incomplete, + notes=notes, + ) + if not usable: + return result + + sources = list(usable) + if coherence and coherence.get("blockers"): + sources.append({"lens": "coherence", "blockers": coherence["blockers"]}) + + result.blockers = merge_blockers(sources) + result.matrices = matrix_coverage(usable) + + if grid: + result.coverage, result.disagreements = grid_coverage(grid, usable) + result.disagreements = _sorted(result.disagreements) + + _add_disagreement_blockers(result) + return result + +def _chosen_by(lens: str, disagreement: Disagreement) -> str: + return f"chosen independently by {lens}" + +def _add_disagreement_blockers(result: Comparison) -> None: + by_id = {b["id"]: b for b in result.blockers if b.get("id")} + for disagreement in result.disagreements: + safe_cell_id = _NON_ID.sub("-", disagreement.cell_id.casefold()).strip("-") + blocker_id = f"diverged-{safe_cell_id or 'unnamed'}" + clash = by_id.get(blocker_id) + if clash is not None: + clash.setdefault("disagreements", []).append(disagreement.as_dict()) + continue + + synthesized = { + "id": blocker_id, + "title": f"Lenses disagree: {disagreement.question}", + "question": disagreement.question, + "where": disagreement.where, + "options": [ + {"label": value, "consequence": _chosen_by(lens, disagreement)} + for lens, value in sorted(disagreement.answers.items()) + ], + "found_by": sorted(disagreement.answers), + "from_disagreement": True, + } + by_id[blocker_id] = synthesized + result.blockers.append(synthesized) + diff --git a/mcp_server/tests/test_refine.py b/mcp_server/tests/test_refine.py new file mode 100644 index 0000000..d4ab032 --- /dev/null +++ b/mcp_server/tests/test_refine.py @@ -0,0 +1,613 @@ +from __future__ import annotations + +import json +import tempfile +import unittest +from pathlib import Path +from typing import Any + +from services import refine_artifacts as artifacts +from services import refine_compare as compare + +def reading( + lens: str, *, decisions=None, blockers=None, cells=None, matrices=None +) -> dict[str, Any]: + payload = { + "lens": lens, + "spec_root": "specs", + "decisions": decisions if decisions is not None else [], + "blockers": blockers if blockers is not None else [], + } + if cells is not None: + payload["cells"] = cells + if matrices is not None: + payload["matrices"] = matrices + return payload + +def grid(*ids: str) -> dict[str, Any]: + return { + "cells": [ + { + "id": cell_id, + "question": f"what happens on {cell_id}?", + "where": "specs/orders.md#Checkout", + } + for cell_id in ids + ] + } + +def cell(cell_id: str, value: str, *, guessed: bool = False) -> dict[str, Any]: + return {"id": cell_id, "value": value, "guessed": guessed} + +def decision(question: str, value: str, *, guessed: bool = False) -> dict[str, Any]: + return { + "question": question, + "value": value, + "where": "specs/orders.md#Checkout", + "guessed": guessed, + } + +def blocker(identifier: str, *, title: str = "", options: int = 2) -> dict[str, Any]: + return { + "id": identifier, + "title": title or f"decision {identifier}", + "question": f"which way for {identifier}?", + "where": "specs/orders.md#Checkout", + "options": [ + {"label": f"opt{i}", "consequence": "..."} for i in range(options) + ], + "recommended": "opt0", + "impact": "changes_behaviour", + "reversible": True, + } + +class TestDisagreementIdentity(unittest.TestCase): + def test_free_form_decisions_are_evidence_not_deterministic_identity(self): + result = compare.compare([ + reading("a", decisions=[decision("Who owns the hold expiry timer?", "service")]), + reading( + "b", + decisions=[ + decision( + "Which component is responsible for expiring the hold timer?", + "database TTL", + ) + ], + ), + ]) + self.assertEqual(result.disagreements, []) + self.assertEqual(result.blockers, []) + + def test_grid_id_creates_a_located_disagreement_blocker(self): + result = compare.compare( + [ + reading("a", cells=[cell("hold.timeout", "released")]), + reading("b", cells=[cell("hold.timeout", "extended")]), + ], + grid=grid("hold.timeout"), + ) + self.assertEqual(result.disagreements[0].cell_id, "hold.timeout") + self.assertEqual(result.blockers[0]["id"], "diverged-hold.timeout") + + def test_case_and_whitespace_only_answer_differences_are_equal(self): + result = compare.compare( + [ + reading("a", cells=[cell("hold.timeout", " Seat Released ")]), + reading("b", cells=[cell("hold.timeout", "seat released")]), + ], + grid=grid("hold.timeout"), + ) + self.assertEqual(result.disagreements, []) + + def test_grid_question_rewording_does_not_change_the_blocker_id(self): + readings = [ + reading("a", cells=[cell("hold.timeout", "released")]), + reading("b", cells=[cell("hold.timeout", "extended")]), + ] + first = compare.compare( + readings, + grid={"cells": [{"id": "hold.timeout", "question": "What happens?"}]}, + ) + second = compare.compare( + readings, + grid={"cells": [{"id": "hold.timeout", "question": "Where does it go?"}]}, + ) + self.assertEqual(first.blockers[0]["id"], second.blockers[0]["id"]) + + def test_same_where_never_merges_unrelated_findings(self): + result = compare.compare( + [ + reading( + "a", + blockers=[blocker("seat-map-source")], + cells=[cell("refund.window", "30 days")], + ), + reading("b", cells=[cell("refund.window", "90 days")]), + ], + grid=grid("refund.window"), + ) + self.assertEqual( + {item["id"] for item in result.blockers}, + {"seat-map-source", "diverged-refund.window"}, + ) + self.assertNotIn("disagreements", result.blockers[0]) + + def test_exact_stable_id_can_explicitly_join_authored_and_grid_findings(self): + result = compare.compare( + [ + reading( + "a", + blockers=[blocker("diverged-hold.timeout")], + cells=[cell("hold.timeout", "released")], + ), + reading("b", cells=[cell("hold.timeout", "extended")]), + ], + grid=grid("hold.timeout"), + ) + self.assertEqual([item["id"] for item in result.blockers], ["diverged-hold.timeout"]) + self.assertEqual(len(result.blockers[0]["disagreements"]), 1) + +class TestMergeBlockers(unittest.TestCase): + def test_same_id_from_three_lenses_merges_with_attribution(self): + merged = compare.merge_blockers([ + reading("a", blockers=[blocker("expiry")]), + reading("b", blockers=[blocker("expiry")]), + reading("c", blockers=[blocker("expiry")]), + ]) + self.assertEqual(len(merged), 1) + self.assertEqual(merged[0]["found_by"], ["a", "b", "c"]) + + def test_richest_option_set_wins(self): + merged = compare.merge_blockers([ + reading("a", blockers=[blocker("expiry", options=2)]), + reading("b", blockers=[blocker("expiry", options=4)]), + ]) + self.assertEqual(len(merged[0]["options"]), 4) + + def test_widely_raised_blockers_sort_first(self): + merged = compare.merge_blockers([ + reading("a", blockers=[blocker("lonely"), blocker("popular")]), + reading("b", blockers=[blocker("popular")]), + reading("c", blockers=[blocker("popular")]), + ]) + self.assertEqual(merged[0]["id"], "popular") + + def test_blocker_without_id_is_skipped(self): + merged = compare.merge_blockers([reading("a", blockers=[{"title": "no id"}])]) + self.assertEqual(merged, []) + + def test_reading_missing_keys_is_reported_not_crashed(self): + result = compare.compare([{"lens": "broken"}]) + self.assertEqual(len(result.incomplete), 1) + self.assertIn("decisions", result.incomplete[0]) + +class TestMalformedReadings(unittest.TestCase): + def test_blockers_as_an_object_is_reported_not_crashed(self): + result = compare.compare([ + {"lens": "bad", "decisions": [], "blockers": {"expiry": {"id": "x"}}}, + reading("good", blockers=[blocker("expiry")]), + ]) + self.assertEqual(result.lens_count, 1) + self.assertEqual([b["id"] for b in result.blockers], ["expiry"]) + self.assertIn("blockers should be a list of objects", result.incomplete[0]) + + def test_decisions_as_a_string_is_reported_not_crashed(self): + result = compare.compare([{"lens": "bad", "decisions": "oops", "blockers": []}]) + self.assertEqual(result.lens_count, 0) + self.assertIn("decisions should be a list of objects", result.incomplete[0]) + + def test_cells_as_a_string_is_reported_not_crashed(self): + result = compare.compare( + [{"lens": "bad", "decisions": [], "blockers": [], "cells": "oops"}], + grid=grid("hold.timeout"), + ) + self.assertEqual(result.lens_count, 0) + self.assertIn("cells should be a list of objects", result.incomplete[0]) + + def test_a_list_with_a_non_object_entry_is_still_rejected(self): + result = compare.compare([ + {"lens": "bad", "decisions": [{"question": "q", "value": "v"}, "oops"], + "blockers": []}, + ]) + self.assertEqual(result.lens_count, 0) + + def test_uncomparable_readings_are_not_counted_as_lenses(self): + result = compare.compare([ + reading("a"), + {"lens": "bad", "decisions": "oops", "blockers": []}, + ]) + self.assertEqual(result.lens_count, 1) + self.assertEqual(result.readings_total, 2) + self.assertEqual(result.lenses, ["a"]) + +class TestLayout(unittest.TestCase): + def test_round_allocation_and_reading_round_trip(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + self.assertIsNone(layout.latest_round()) + + path = layout.reading_path(1, "concurrency") + artifacts.write_json(path, reading("concurrency")) + self.assertEqual(layout.latest_round(), 1) + + loaded = artifacts.load_readings(layout, 1) + self.assertEqual([r["lens"] for r in loaded], ["concurrency"]) + + def test_manifest_round_trip_records_expected_lenses(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.manifest_path(1), + {"lenses": ["concurrency", "ordering"]}, + ) + self.assertEqual( + artifacts.load_manifest(layout, 1)["lenses"], + ["concurrency", "ordering"], + ) + + def test_manifest_rejects_unsafe_or_case_duplicate_lenses(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + for lenses in (["../escape"], ["Ordering", "ordering"]): + artifacts.write_json(layout.manifest_path(1), {"lenses": lenses}) + with self.assertRaisesRegex(ValueError, "unique safe names"): + artifacts.load_manifest(layout, 1) + + def test_missing_expected_reading_is_loaded_as_incomplete(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "a"), reading("a")) + loaded = artifacts.load_readings(layout, 1, expected_lenses=["a", "b"]) + result = compare.compare(loaded) + self.assertEqual(result.lens_count, 1) + self.assertEqual(result.readings_total, 2) + self.assertIn("missing expected reading", result.incomplete[0]) + + def test_legacy_round_without_manifest_uses_observed_files(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "a"), reading("a")) + self.assertEqual(artifacts.load_manifest(layout, 1), {}) + self.assertEqual(len(artifacts.load_readings(layout, 1)), 1) + + def test_lens_name_is_derived_from_the_filename(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + path = layout.reading_path(1, "ordering") + artifacts.write_json(path, {"decisions": [], "blockers": []}) + self.assertEqual(artifacts.load_readings(layout, 1)[0]["lens"], "ordering") + + def test_the_filename_wins_over_a_lens_field_that_disagrees(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.reading_path(1, "ordering"), + reading("concurrency", decisions=[decision("what store", "pg")]), + ) + artifacts.write_json( + layout.reading_path(1, "concurrency"), + reading("concurrency", decisions=[decision("what store", "ddb")]), + ) + loaded = artifacts.load_readings(layout, 1) + self.assertEqual( + sorted(r["lens"] for r in loaded), ["concurrency", "ordering"] + ) + + result = compare.compare(loaded) + self.assertEqual(result.disagreements, []) + self.assertEqual(len(result.notes), 1) + self.assertIn("fan-out", result.notes[0]) + + def test_a_matching_lens_field_raises_no_note(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "ordering"), reading("ordering")) + result = compare.compare(artifacts.load_readings(layout, 1)) + self.assertEqual(result.notes, []) + + def test_a_grid_with_the_wrong_shape_names_the_file(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.grid_path(1), {"cells": "oops"}) + with self.assertRaises(ValueError) as ctx: + artifacts.load_grid(layout, 1) + self.assertIn(artifacts.GRID_FILE, str(ctx.exception)) + + def test_required_grid_must_exist_and_contain_a_cell(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + with self.assertRaisesRegex(ValueError, "missing required grid"): + artifacts.load_grid(layout, 1, required=True) + + artifacts.write_json(layout.grid_path(1), {"cells": []}) + with self.assertRaisesRegex(ValueError, "at least one"): + artifacts.load_grid(layout, 1, required=True) + + def test_grid_ids_must_be_unique_and_path_safe(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.grid_path(1), + {"cells": [{"id": "Hold.Timeout"}, {"id": "hold.timeout"}]}, + ) + with self.assertRaisesRegex(ValueError, "unique"): + artifacts.load_grid(layout, 1) + + artifacts.write_json(layout.grid_path(1), {"cells": [{"id": "../escape"}]}) + with self.assertRaisesRegex(ValueError, "may contain"): + artifacts.load_grid(layout, 1) + + def test_a_coherence_file_with_the_wrong_shape_names_the_file(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.coherence_path(1), {"blockers": {"a": {}}}) + with self.assertRaises(ValueError) as ctx: + artifacts.load_coherence(layout, 1) + self.assertIn(artifacts.COHERENCE_FILE, str(ctx.exception)) + + def test_hand_edited_resolutions_of_the_wrong_shape_name_the_file(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.resolutions_path, {"resolved": "expiry"}) + with self.assertRaises(ValueError) as ctx: + artifacts.load_resolutions(layout) + self.assertIn(artifacts.RESOLUTIONS_FILE, str(ctx.exception)) + + def test_everything_lives_under_the_loop_s_own_subdirectory(self): + layout = artifacts.layout_for("docs") + for path in ( + layout.resolutions_path, + layout.findings_path, + layout.grid_path(1), + layout.reading_path(1, "ordering"), + ): + self.assertIn(artifacts.REFINE_SUBDIR, path.parts) + self.assertNotIn("analysis", path.parts) + self.assertNotIn("planning", path.parts) + + def test_broken_json_names_the_file(self): + with tempfile.TemporaryDirectory() as tmp: + bad = Path(tmp) / "x.json" + bad.write_text("{not json") + with self.assertRaises(ValueError) as ctx: + artifacts.read_json(bad) + self.assertIn("x.json", str(ctx.exception)) + + def test_resolutions_round_trip(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.resolutions_path, + {"resolved": [{"blocker_id": "expiry", "choice": "refund"}]}, + ) + self.assertEqual(artifacts.resolved_ids(layout), {"expiry"}) + + def test_a_fresh_project_is_empty_not_an_error(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + self.assertEqual(layout.rounds(), []) + self.assertEqual(artifacts.load_resolutions(layout), []) + self.assertEqual(artifacts.load_findings(layout), {}) + +class TestGridCoverage(unittest.TestCase): + def test_cell_no_lens_answered_is_reported(self): + coverage, _ = compare.grid_coverage( + grid("hold.timeout", "hold.cancel"), + [reading("a", cells=[cell("hold.timeout", "seat released")])], + ) + self.assertEqual(coverage.cells_total, 2) + self.assertEqual(coverage.cells_filled, 1) + self.assertEqual([c["id"] for c in coverage.uncovered], ["hold.cancel"]) + + def test_two_lenses_filling_a_cell_differently_is_a_disagreement(self): + _, disagreements = compare.grid_coverage( + grid("hold.timeout"), + [ + reading("a", cells=[cell("hold.timeout", "seat released")]), + reading("b", cells=[cell("hold.timeout", "hold extended")]), + ], + ) + self.assertEqual(len(disagreements), 1) + self.assertEqual( + disagreements[0].answers, {"a": "seat released", "b": "hold extended"} + ) + + def test_agreement_reached_by_guessing_is_reported_not_silent(self): + coverage, disagreements = compare.grid_coverage( + grid("hold.timeout"), + [ + reading("a", cells=[cell("hold.timeout", "released", guessed=True)]), + reading("b", cells=[cell("hold.timeout", "released", guessed=True)]), + ], + ) + self.assertEqual(disagreements, []) + self.assertEqual(coverage.agreed_guesses[0]["lenses"], ["a", "b"]) + + def test_one_guess_is_not_mislabeled_as_agreement(self): + coverage, disagreements = compare.grid_coverage( + grid("hold.timeout"), + [reading("a", cells=[cell("hold.timeout", "released", guessed=True)])], + ) + self.assertEqual(disagreements, []) + self.assertEqual(coverage.agreed_guesses, []) + + def test_agreement_one_lens_read_from_the_spec_is_not_flagged(self): + coverage, _ = compare.grid_coverage( + grid("hold.timeout"), + [ + reading("a", cells=[cell("hold.timeout", "released", guessed=True)]), + reading("b", cells=[cell("hold.timeout", "released")]), + ], + ) + self.assertEqual(coverage.agreed_guesses, []) + + def test_cell_conflict_becomes_a_blocker_through_the_normal_path(self): + result = compare.compare( + [ + reading("a", cells=[cell("hold.timeout", "released")]), + reading("b", cells=[cell("hold.timeout", "extended")]), + ], + grid=grid("hold.timeout"), + ) + self.assertTrue(any(b.get("from_disagreement") for b in result.blockers)) + + def test_cell_conflict_does_not_attach_by_free_form_location(self): + result = compare.compare( + [ + reading("a", blockers=[blocker("expiry")], + cells=[cell("hold.timeout", "released")]), + reading("b", cells=[cell("hold.timeout", "extended")]), + ], + grid=grid("hold.timeout"), + ) + self.assertEqual( + [b["id"] for b in result.blockers], + ["expiry", "diverged-hold.timeout"], + ) + self.assertNotIn("disagreements", result.blockers[0]) + + def test_a_round_without_a_grid_still_compares(self): + result = compare.compare([reading("a"), reading("b")]) + self.assertIsNone(result.coverage) + +def matrix(name="ops × collisions", rows=("hold", "seat"), cols=("claim", "cancel"), + cells=()) -> dict[str, Any]: + return {"name": name, "rows": list(rows), "cols": list(cols), "cells": list(cells)} + +class TestMatrixCoverage(unittest.TestCase): + def test_every_intersection_is_counted_whether_or_not_it_was_filled(self): + report = compare.matrix_coverage([ + reading("a", matrices=[matrix(cells=[ + {"row": "hold", "col": "claim", "value": "409"}, + ])]), + ]) + self.assertEqual(report[0].declared, 4) + self.assertEqual(report[0].answered, 1) + self.assertEqual(len(report[0].missing), 3) + + def test_a_cell_the_lens_could_not_answer_is_a_finding_with_a_reason(self): + report = compare.matrix_coverage([ + reading("a", matrices=[matrix(cells=[ + {"row": "hold", "col": "claim", "unanswerable": "nobody owns the timer"}, + ])]), + ]) + self.assertEqual(report[0].answered, 0) + self.assertEqual( + report[0].unanswerable, + [{"row": "hold", "col": "claim", "why": "nobody owns the timer"}], + ) + self.assertEqual(len(report[0].missing), 3) + + def test_guessed_answers_are_counted_separately_from_answered(self): + report = compare.matrix_coverage([ + reading("a", matrices=[matrix(cells=[ + {"row": "hold", "col": "claim", "value": "409", "guessed": True}, + {"row": "hold", "col": "cancel", "value": "released"}, + ])]), + ]) + self.assertEqual(report[0].answered, 2) + self.assertEqual(report[0].guessed, 1) + + def test_matrices_are_never_merged_across_lenses(self): + report = compare.matrix_coverage([ + reading("a", matrices=[matrix(name="same title")]), + reading("b", matrices=[matrix(name="same title")]), + ]) + self.assertEqual([m.lens for m in report], ["a", "b"]) + + def test_the_least_complete_reading_is_reported_first(self): + report = compare.matrix_coverage([ + reading("thorough", matrices=[matrix(name="x", cells=[ + {"row": r, "col": c, "value": "v"} + for r in ("hold", "seat") for c in ("claim", "cancel") + ])]), + reading("thin", matrices=[matrix(name="y")]), + ]) + self.assertEqual([m.lens for m in report], ["thin", "thorough"]) + + def test_a_matrix_with_no_axes_is_not_a_cross_product(self): + report = compare.matrix_coverage([ + reading("a", matrices=[matrix(rows=(), cols=())]), + ]) + self.assertEqual(report, []) + + def test_axes_of_the_wrong_shape_are_reported_not_counted(self): + result = compare.compare([ + {"lens": "a", "decisions": [], "blockers": [], + "matrices": [{"name": "x", "rows": "hold", "cols": ["claim"], "cells": []}]}, + ]) + self.assertEqual(result.lens_count, 0) + self.assertIn("matrices[0].rows", result.incomplete[0]) + + def test_a_round_without_matrices_still_compares(self): + result = compare.compare([reading("a"), reading("b")]) + self.assertEqual(result.matrices, []) + + def test_nothing_about_a_matrix_blocks_or_fails_the_round(self): + result = compare.compare([ + reading("a", matrices=[matrix()]), + ]) + self.assertEqual(result.blockers, []) + self.assertEqual(len(result.matrices[0].missing), 4) + +class TestCoherence(unittest.TestCase): + def test_coherence_blockers_reach_the_user_attributed(self): + result = compare.compare( + [reading("a"), reading("b")], + coherence={"blockers": [blocker("locking-contradicts-retry")]}, + ) + found = {b["id"]: b["found_by"] for b in result.blockers} + self.assertEqual(found["locking-contradicts-retry"], ["coherence"]) + + def test_coherence_does_not_count_as_an_independent_reading(self): + result = compare.compare( + [reading("a"), reading("b")], + coherence={"blockers": [blocker("x")]}, + ) + self.assertEqual(result.lens_count, 2) + self.assertEqual(result.lenses, ["a", "b"]) + + def test_coherence_never_votes_in_a_disagreement(self): + result = compare.compare( + [reading("a", decisions=[decision("what store", "postgres")])], + coherence={ + "blockers": [], + "decisions": [decision("what store", "mysql")], + }, + ) + self.assertEqual(result.disagreements, []) + +class TestEndToEnd(unittest.TestCase): + def test_resolved_blocker_drops_out_of_the_next_round(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + for lens in ("a", "b"): + artifacts.write_json( + layout.reading_path(1, lens), + reading(lens, blockers=[blocker("expiry")]), + ) + first = compare.compare(artifacts.load_readings(layout, 1)) + self.assertIn("expiry", [b["id"] for b in first.blockers]) + + artifacts.write_json( + layout.resolutions_path, + {"resolved": [{"blocker_id": "expiry", "choice": "refund"}]}, + ) + resolved = artifacts.resolved_ids(layout) + still_open = [b for b in first.blockers if b["id"] not in resolved] + self.assertEqual(still_open, []) + + def test_findings_file_is_valid_json_for_the_reporting_skill(self): + result = compare.compare([ + reading("a", decisions=[decision("what store", "postgres")], + blockers=[blocker("expiry")]), + reading("b", decisions=[decision("what store", "mysql")]), + ]) + payload = { + "disagreements": [d.as_dict() for d in result.disagreements], + "blockers": result.blockers, + } + self.assertEqual(json.loads(json.dumps(payload))["blockers"][0]["id"], "expiry") + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/mcp_server/tests/test_refine_commands.py b/mcp_server/tests/test_refine_commands.py new file mode 100644 index 0000000..7bef374 --- /dev/null +++ b/mcp_server/tests/test_refine_commands.py @@ -0,0 +1,391 @@ +from __future__ import annotations + +import argparse +import contextlib +import io +import json +import tempfile +import unittest +from pathlib import Path +from typing import Any + +import cli as specflow_cli +from services import refine_artifacts as artifacts +from services import refine_commands + +def run(*argv: str) -> tuple[int, str]: + parser = argparse.ArgumentParser() + parser.add_argument("--root-path", default=None) + subparsers = parser.add_subparsers(dest="command") + subparsers.required = True + refine_commands.register(subparsers) + + args = parser.parse_args(argv) + buffer = io.StringIO() + with contextlib.redirect_stdout(buffer): + code = args.func(args) + return code, buffer.getvalue() + +def reading(lens: str, **fields: Any) -> dict[str, Any]: + return {"lens": lens, "decisions": [], "blockers": [], **fields} + +def blocker(identifier: str, *, where: str = "specs/orders.md#Checkout") -> dict[str, Any]: + return { + "id": identifier, + "title": f"decision {identifier}", + "question": f"which way for {identifier}?", + "where": where, + "options": [{"label": "opt0", "consequence": "..."}], + "recommended": "opt0", + } + +class TestUsageErrors(unittest.TestCase): + def test_new_round_requires_at_least_one_safe_unique_lens(self): + with tempfile.TemporaryDirectory() as tmp: + code, output = run("refine", "new-round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("at least one --lens", output) + + code, output = run( + "refine", + "new-round", + "--outputs", + tmp, + "--lens", + "../escape", + ) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("may contain only", output) + + def test_round_with_no_rounds_yet_exits_two_with_a_message(self): + with tempfile.TemporaryDirectory() as tmp: + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("new-round", output) + + def test_round_with_no_readings_exits_two_with_a_message(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + layout.round_dir(1).mkdir(parents=True) + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("reading.", output) + + def test_a_round_of_only_unparseable_readings_exits_two_naming_the_file(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + path = layout.reading_path(1, "ordering") + path.parent.mkdir(parents=True) + path.write_text("{not json") + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("reading.ordering.json", output) + + def test_one_unparseable_reading_does_not_cost_the_other_lenses(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.reading_path(1, "a"), reading("a", blockers=[blocker("expiry")]) + ) + layout.reading_path(1, "b").write_text('{"lens": "b", "decis') + + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertIn("1 of 2 readings compared", output) + self.assertIn("reading.b.json", output) + self.assertIn("expiry", output) + + def test_a_round_where_nothing_could_be_compared_exits_two(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.reading_path(1, "ordering"), {"lens": "ordering"} + ) + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("could be compared", output) + + def test_manifested_round_requires_a_nonempty_grid(self): + with tempfile.TemporaryDirectory() as tmp: + run( + "refine", + "new-round", + "--outputs", + tmp, + "--lens", + "a", + ) + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "a"), reading("a")) + + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("missing required grid", output) + + def test_manifested_round_reports_an_absent_expected_lens(self): + with tempfile.TemporaryDirectory() as tmp: + run( + "refine", + "new-round", + "--outputs", + tmp, + "--lens", + "a", + "b", + ) + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.grid_path(1), + {"cells": [{"id": "hold.timeout", "question": "What happens?"}]}, + ) + artifacts.write_json(layout.reading_path(1, "a"), reading("a")) + + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertIn("1 of 2 readings compared", output) + self.assertIn("missing expected reading", output) + + def test_the_error_is_json_when_json_was_asked_for(self): + with tempfile.TemporaryDirectory() as tmp: + code, output = run("refine", "round", "--outputs", tmp, "--json") + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("error", json.loads(output)) + +class TestRootPath(unittest.TestCase): + def test_outputs_is_resolved_against_root_path(self): + with tempfile.TemporaryDirectory() as tmp: + code, output = run( + "--root-path", + tmp, + "refine", + "new-round", + "--outputs", + "docs", + "--lens", + "a", + ) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertIn(str(Path(tmp) / "docs" / "refine" / "round-01"), output) + + def test_status_reads_the_root_path_tree_not_the_cwd(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(Path(tmp) / "docs") + artifacts.write_json( + layout.resolutions_path, {"resolved": [{"blocker_id": "expiry"}]} + ) + code, output = run( + "--root-path", tmp, "refine", "status", "--outputs", "docs", "--json" + ) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertEqual(json.loads(output)["resolved"], 1) + + def test_an_absolute_outputs_dir_still_wins(self): + with tempfile.TemporaryDirectory() as tmp: + code, output = run( + "--root-path", + "/nonexistent", + "refine", + "new-round", + "--outputs", + tmp, + "--lens", + "a", + ) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertIn(tmp, output) + +class TestNoStopRule(unittest.TestCase): + def test_the_payload_carries_no_novelty_or_stop_signal(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.reading_path(1, "a"), reading("a", blockers=[blocker("expiry")]) + ) + payload = json.loads(run("refine", "round", "--outputs", tmp, "--json")[1]) + self.assertNotIn("novelty", payload) + for absent in ("new", "repeat"): + self.assertNotIn(absent, payload["counts"]) + + def test_no_round_ledger_is_written(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "a"), reading("a")) + run("refine", "round", "--outputs", tmp, "--json") + self.assertEqual( + sorted(p.name for p in layout.root.iterdir()), + ["findings.json", "round-01"], + ) + + def test_rounds_are_counted_from_the_directories_on_disk(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + for number in (1, 2): + artifacts.write_json(layout.reading_path(number, "a"), reading("a")) + payload = json.loads(run("refine", "status", "--outputs", tmp, "--json")[1]) + self.assertEqual(payload["rounds_run"], 2) + + def test_rerunning_a_round_is_idempotent(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.reading_path(1, "a"), reading("a", blockers=[blocker("expiry")]) + ) + first = json.loads(run("refine", "round", "--outputs", tmp, "--json")[1]) + second = json.loads(run("refine", "round", "--outputs", tmp, "--json")[1]) + self.assertEqual(first, second) + +class TestPayload(unittest.TestCase): + def test_round_payload_carries_the_keys_the_skills_read(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "a"), reading("a")) + payload = json.loads(run("refine", "round", "--outputs", tmp, "--json")[1]) + for key in ( + "round", "lenses", "lens_count", "readings_total", "counts", + "disagreements", "blockers", "coverage", "matrices", + "incomplete_readings", "notes", "findings_path", + ): + self.assertIn(key, payload) + + def test_a_matrix_cell_the_spec_cannot_answer_reaches_the_output(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "concurrency"), reading( + "concurrency", + matrices=[{ + "name": "held resource × collision", + "rows": ["seat hold"], + "cols": ["second claim", "timer expiry"], + "cells": [{ + "row": "seat hold", "col": "second claim", + "unanswerable": "nobody owns the timer", + }], + }], + )) + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertIn("0/2 answered", output) + self.assertIn("spec cannot say", output) + self.assertIn("nobody owns the timer", output) + self.assertIn("never filled", output) + + payload = json.loads(run("refine", "round", "--outputs", tmp, "--json")[1]) + self.assertEqual(payload["counts"]["matrix_unanswerable"], 1) + self.assertEqual(payload["counts"]["matrix_skipped"], 1) + + def test_status_reports_the_matrix_counts_back(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "a"), reading("a", matrices=[{ + "name": "m", "rows": ["r"], "cols": ["c"], "cells": [], + }])) + run("refine", "round", "--outputs", tmp, "--json") + code, output = run("refine", "status", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertIn("Lens skipped 1 matrix cell(s)", output) + + def test_a_partial_round_says_so_in_the_headline_and_at_the_end(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json(layout.reading_path(1, "a"), reading("a")) + artifacts.write_json(layout.reading_path(1, "b"), {"lens": "b"}) + code, output = run("refine", "round", "--outputs", tmp) + self.assertEqual(code, refine_commands.EXIT_OK) + self.assertIn("1 of 2 readings compared", output) + self.assertIn("could not be compared", output) + + def test_findings_are_written_for_status_to_read_back(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.reading_path(1, "a"), reading("a", blockers=[blocker("expiry")]) + ) + run("refine", "round", "--outputs", tmp, "--json") + findings = artifacts.load_findings(layout) + self.assertEqual([b["id"] for b in findings["blockers"]], ["expiry"]) + + def test_status_drops_a_blocker_resolved_since_the_last_round(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.reading_path(1, "a"), + reading("a", blockers=[blocker("expiry"), blocker("contention")]), + ) + run("refine", "round", "--outputs", tmp, "--json") + run("refine", "resolve", "--outputs", tmp, "--id", "expiry", + "--choice", "opt0") + + payload = json.loads(run("refine", "status", "--outputs", tmp, "--json")[1]) + self.assertEqual(payload["counts"]["open"], 1) + self.assertEqual([b["id"] for b in payload["blockers"]], ["contention"]) + + def test_resolving_twice_is_refused_not_recorded_twice(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.findings_path, + {"blockers": [blocker("expiry")]}, + ) + args = ("refine", "resolve", "--outputs", tmp, "--id", "expiry", + "--choice", "opt0") + self.assertEqual(run(*args)[0], refine_commands.EXIT_OK) + code, output = run(*args) + self.assertEqual(code, refine_commands.EXIT_USAGE) + self.assertIn("already resolved", output) + + def test_unknown_blocker_and_invalid_choice_do_not_mutate_resolutions(self): + with tempfile.TemporaryDirectory() as tmp: + layout = artifacts.layout_for(tmp) + artifacts.write_json( + layout.findings_path, + {"blockers": [blocker("expiry")]}, + ) + + unknown = run( + "refine", + "resolve", + "--outputs", + tmp, + "--id", + "missing", + "--choice", + "opt0", + ) + invalid = run( + "refine", + "resolve", + "--outputs", + tmp, + "--id", + "expiry", + "--choice", + "not-an-option", + ) + + self.assertEqual(unknown[0], refine_commands.EXIT_USAGE) + self.assertIn("not an open blocker", unknown[1]) + self.assertEqual(invalid[0], refine_commands.EXIT_USAGE) + self.assertIn("choose one of", invalid[1]) + self.assertEqual(artifacts.load_resolutions(layout), []) + +class TestPluginInstall(unittest.TestCase): + def test_default_install_uses_the_published_marketplace(self): + steps = specflow_cli._plugin_install_steps("claude") + self.assertEqual( + steps[0], + ["claude", "plugin", "marketplace", "add", "griddynamics/specflow"], + ) + + def test_local_checkout_can_replace_the_marketplace_source(self): + steps = specflow_cli._plugin_install_steps("claude", "/checkout/specflow") + self.assertEqual( + steps[0], + ["claude", "plugin", "marketplace", "add", "/checkout/specflow"], + ) + self.assertEqual( + steps[1], + ["claude", "plugin", "install", "specflow2@specflow-marketplace"], + ) + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/plans/specflow-2.0/specflow-plugin-plan.md b/plans/specflow-2.0/specflow-plugin-plan.md new file mode 100644 index 0000000..f4cfe6b --- /dev/null +++ b/plans/specflow-2.0/specflow-plugin-plan.md @@ -0,0 +1,283 @@ +# SpecFlow 2.0 — Historical Plugin Plan (superseded) + +> ⚠️ **Sections 3 and 5–8 are superseded by the code that shipped.** The oracle +> library, the totality gate, the ranking and saturation scripts, the +> `plugins/specflow/lib/` layout and the Steel Commandments 2.0 proposal were all +> deliberately cut or reversed. So were four of the seven skills: what ships is +> `specflow-refine` and `specflow-resolve`, and there is **no stop rule** — §7's P6 +> ("loop terminates on saturation") encodes an inference the design cannot support. +> Section 4 still holds and §1–§2 describe the shape, but the flow diagram in §1 and +> the skill inventory in §5 do not. See **`status.md`** in this directory for what is +> actually built, what was cut and why, and which gates remain unmet. Do not build +> from §3, §5, §6 or §7. + +**Status**: SUPERSEDED — rationale/history only; `status.md` describes the product +**Date**: 2026-08-03 +**Supersedes**: `PLAN.md` and `PLUGIN-SKILLS.md` in this directory (earlier drafts, safe to delete once this is approved) +**Shape**: A Claude Code marketplace plugin with no SpecFlow backend. The user's +coding agent and model provider may still be hosted and receive specification +context. + +--- + +## 1. The flow + +### Today (README §"Get started") + +``` +specs/ → check_specification_completeness → run_planning → run_generation + (local skill, free) (local skill, free) (backend, 2–8 hrs, ~$400) +``` + +### SpecFlow 2.0 + +``` +specs/ → /specflow-analysis → /specflow-refine → /specflow-planning + (gap detection) (the product) (now trustworthy) + ↕ + you, resolving + ranked blockers +``` + +**Yes — one new user-facing verb.** That is the whole UX change. `run_generation` is replaced by `/specflow-refine`, and everything the backend used to do dissolves into subagents inside that one skill. + +### The one correction: planning moves *after* refine + +You had it as analysis → planning → refine. I'd swap the last two, for the reason that broke 1.0's measurement. + +**A plan is downstream of the spec.** If the spec is ambiguous, the plan is *one arbitrary resolution* of that ambiguity — and once it exists it anchors everything after it. That is precisely what `sync_plan_to_workspaces` (`workflow_steps.py:700`) did: one plan copied to all N workspaces, so the largest interpretation step ran exactly once and its arbitrariness became invisible to the statistics. + +Planning before refining reintroduces that defect: you'd be refining against a spec whose ambiguity has already been silently resolved by the planner. + +So planning takes on **two distinct roles**: + +| Role | When | Why | +|---|---|---| +| **Internal, per-lens** | Inside each refine round | Attempting to sequence work is a strong forcing function — you cannot phase what you don't understand. Divergent phase decomposition across lenses *is* a blocker signal. | +| **Final, user-facing** | After refinement converges | One plan, generated from a spec whose ambiguities are resolved. Now worth trusting. | + +This is also the better product story: *refine until the spec is unambiguous, then the plan you get is reliable.* Planning becomes the reward rather than a prerequisite. + +`/specflow-planning` still runs standalone whenever the user wants — nothing stops them. But the documented happy path puts it last. + +--- + +## 2. What `/specflow-refine` actually does + +One skill, owning a loop. Each round: + +**1. Fan out.** Spawn N lens subagents **in a single message** so they run concurrently. Each gets one adversarial lens and the spec. Blind to each other — no interpreter sees another's output, and there is no shared plan. Fresh context each. + +**2. Each lens produces a *total* artifact.** Not a prose blocker list — a filled structure: + +- the architectural dimensions (Parts A–D, every one "Pick exactly ONE") +- a state transition table +- a failure-mode matrix +- a phase decomposition (the internal planning role) +- blockers, each with a spec anchor + +**Totality is the forcing function that replaces building.** A prose list is partial by nature; a filled matrix is total by construction. An agent filling a state table *cannot skip* the cell for "payment succeeded + reservation expired." + +**3. Run the oracles** (scripts, not prose): schema conformance, totality check, contract validation. + +**4. Triage.** Cross-lens concordance, then rank by cost asymmetry. Concordance is *not* a score shown to the user — it decides what is worth your attention. If 5 of 6 lenses independently ask the same question, it's real. If 1 of 6 asks, it's probably pedantry. + +**5. Gate.** `AskUserQuestion` (native, supports multiSelect and previews). Prefer proposing over asking — "I'll assume X unless you object" clears most items at near-zero cost. Reserve blocking questions for consequential forks. + +**6. Write decisions back into the specs**, with traceability, and record them so later rounds don't re-ask. + +**7. Converge or loop.** Stop when a fresh round produces no new high-concordance blockers. Saturation, not a threshold — directly observable, no scoring, honest completion signal. + +### The lenses + +| Lens | Attacks | +|---|---| +| `concurrency` | simultaneous access, races, lock scope | +| `partial-failure` | half-completed operations, compensations, retries | +| `data-lifecycle` | migration, retention, deletion, backfill | +| `auth-boundaries` | who can do what to whose data | +| `idempotency` | replay, duplicate delivery, at-least-once | +| `ordering` | sequence assumptions, out-of-order arrival | + +These are the failure classes physical building surfaced and that naive "think about blockers" misses. **Lens count is the cost dial.** + +They ship as `lenses/*.md` assets, not as separate skills — nobody types "run the idempotency lens." Marketplace entries should be things a user would actually invoke. + +### On "all the parallelism by subagents?" — yes + +Spawned concurrently in one message, fresh context each, blind to each other. Two honest caveats: + +- **Subagents are Claude-only** (opus/sonnet/haiku/fable). No GPT-5.5, no GLM. The existing `recommended-models: openai/gpt-5.3-codex` frontmatter goes inert. Adversarial lenses replace vendor diversity — deliberate attack angles beat hoping three vendors have different blind spots — but it *is* a real reduction. +- **N is tunable, and practical concurrency has limits.** Treat lens count as the cost/coverage dial and measure actual behavior at P2 rather than assuming all six run truly simultaneously. + +--- + +## 3. Prose for orchestration, code for oracles + +The architecture in one line. + +**Orchestration is prose** — a skill spawns subagents, sequences rounds, decides when to ask you. Few steps, judgment calls, fine for a model. + +**Oracles are code.** An oracle's entire value is that it is *not* a language model. "Verify the state table is complete" as an instruction is advisory; a script that exits non-zero on a blank cell is a forcing function. + +**Code ships with the plugin.** A skill is a directory — `SKILL.md` plus assets and executables. Already proven in this repo: `.claude/skills/pr-loc-breakdown/` ships `count_py_loc.py` and the skill runs it via Bash. + +| Script | Job | +|---|---| +| `validate_artifact.py` | JSON Schema conformance — malformed fails loudly, not silently | +| `check_totality.py` | Every dimension filled, every matrix cell present. **The gate.** | +| `contracts_oracle.py` | Real SQL DDL / OpenAPI / type-def validators | +| `concordance.py` | Anchor-scoped cross-lens agreement | +| `rank_blockers.py` | Cost-asymmetry ordering, dedup against resolved | +| `saturation.py` | The stop rule | + +~1–2k LOC of pure functions over files. No server, no persistent state, no network — assertable in a test. + +### The asset we already have + +`specflow-analysis/SKILL.md` is 488 lines and already contains the total-artifact framework: + +- **Part A** — 6 universal dimensions, each "Pick exactly **ONE**" +- **Part B** — technology-specific dimensions by project type +- **Part C** — project-specific dimensions, headed *"Discover additional variance sources"* +- **Part D** — micro-level consistency locks, *"AGGRESSIVE ENFORCEMENT"*, "Must specify ALL" + +2.0 does not invent this. It (a) replicates the fill across independent lenses, (b) makes the fill machine-checkable, (c) diffs the filled values. **Divergence on a locked dimension is a named, localized spec ambiguity** — no scoring involved. + +--- + +## 4. Why no backend, no MCP server, no Agent SDK + +**The Agent SDK is Claude Code packaged as a library** — built-in tools, agent loop, context management, subagents, permissions. It supplies the **harness only; deployment is yours.** That is exactly what `backend/app/services/claude_code.py` + workspace pool + NFS + K8s exist to do: run the harness *somewhere other than the user's machine*. + +Once the product runs in the user's IDE, **their Claude Code session is the harness.** Nothing to host, so nothing the SDK provides is needed. Same for `mcp_server/` — it exists to precheck and call a backend that won't exist. + +Consequences worth stating plainly: + +- **COGS → ~0.** Runs on the user's own subscription. This shifts the business model from consumption to licensing — a bigger change than the 10x we started from. +- **Zero egress, no server to audit.** Strictly stronger than the compliance story that killed P10Y. +- **State = files in the user's repo.** Git-tracked, human-readable, human-editable. No Firestore, SQLite, or NFS. Better than an opaque database the user can't inspect. +- **HITL becomes possible at all.** 1.0's own constraint was "no opportunity to prompt the user" mid-run. The HITL pivot *requires* the local architecture. + +--- + +## 5. Skill inventory + +**7 published, 4 net new.** + +| Skill | Status | Role | +|---|---|---| +| `specflow-analysis` | extend | Gap detection. Add JSON output + totality gate. Drop Part F (`INTEGRATION_TESTS_READY` — it exists to tell the backend whether to run E2E). | +| `specflow-refine` | **new** | The orchestrator and entry point. §2. | +| `specflow-simulate` | **new** | Single-lens run, no loop. Cheap first touch, natural demo, immediate value. | +| `specflow-resolve` | **new** | Walk ranked blockers, write decisions into the spec files with traceability. | +| `specflow-contracts` | **new** | Emit data model + API contract as real schemas; validate with real validators. Keeps the compiler, drops the application. | +| `specflow-planning` | rework | Per-lens internally; final artifact after convergence. §1. | +| `specflow-report` | repurpose `specflow-compare-variants` (255 lines) | Current state: resolved, open, ranked. **Counts, never a score.** | + +Retired: `specflow-diagnose` (156 lines, reads backend failure state — nothing to salvage). +Internal only: `specflow-mutate` → `.claude/skills/`, not published. It's our QA harness for validating the loop, not a customer feature. + +**`specflow-resolve` is deliberately separate from finding blockers.** Applying decisions to spec files is an edit operation with its own hazards — don't clobber the user's prose, keep traceability, record resolutions for dedup. A loop that only *reports* blockers leaves all the work with the user, which isn't autonomous refinement. + +--- + +## 6. Plugin layout + +``` +plugins/specflow/ + .claude-plugin/plugin.json → v0.2.0, keywords += spec-refinement, blocker-detection + lib/ # shared oracles — ONE copy + schema/ + interpretation.schema.json + dimensions.schema.json # Parts A–D, machine-readable — the source of truth + blocker.schema.json + validate_artifact.py + check_totality.py + contracts_oracle.py + concordance.py + rank_blockers.py + saturation.py + skills/ + specflow-analysis/ SKILL.md + specflow-planning/ SKILL.md + specflow-refine/ SKILL.md lenses/*.md + specflow-simulate/ SKILL.md + specflow-resolve/ SKILL.md + specflow-contracts/ SKILL.md + specflow-report/ SKILL.md +``` + +**Shared `lib/`, not per-skill copies.** `concordance.py` is needed by two skills, `validate_artifact.py` by four. Copies drift — the single-source-of-truth rule in CLAUDE.md applies to shipped scripts too. + +Moving the dimensions framework into `lib/schema/dimensions.schema.json` does two things at once: shrinks the 488-line skill, and makes the framework machine-checkable. + +⚠️ **P0 open item.** I have not verified the supported mechanism for a skill to resolve a path *above* its own directory to reach `lib/`. Do not build on an assumed environment variable — check the plugin docs first. Fallback is a thin per-skill shim over one implementation. 15 minutes, and it shapes the layout. + +--- + +## 7. Build order + +Each phase leaves the plugin installable and prior phases working. + +| Phase | Work | Exit criterion | +|---|---|---| +| **P0** | Verify plugin-root path resolution. Move the four SKILL.md files from `mcp_server/services/skills/` into `plugins/specflow/skills/`. Drop the `<>` substitution layer — skills take arguments directly. | `/specflow-analysis` runs from the installed plugin with no MCP server | +| **P1** | `lib/schema/*.json` + `validate_artifact.py` + `check_totality.py`. Extend `specflow-analysis` to emit JSON and call the gate. | Totality check rejects a deliberately-blank dimension | +| **P2** | `specflow-simulate` + the six lens prompts. Single lens end-to-end on a real spec. | Artifact validates; blockers carry spec anchors; **measured cost and real concurrency confirmed** | +| **P3** | `contracts_oracle.py` + `specflow-contracts`. | Catches a planted contradiction as a schema impossibility | +| **P4** | `concordance.py` + `rank_blockers.py` + `specflow-refine` fan-out (no loop yet). | N lenses run concurrently; blockers ranked and deduped | +| **P5** | `specflow-resolve` + the `AskUserQuestion` gate. | A human resolves ranked blockers; specs updated with traceability | +| **P6** | `saturation.py` + the round loop. `specflow-report`. | Loop terminates on saturation, not a fixed count | +| **P7** | `specflow-mutate` (internal). | Injected ambiguity detected **and localized** to the mutated requirement | +| **P8** | Rework `specflow-planning` for per-lens + final roles. Retire `specflow-diagnose`. Bump to `0.2.0`, update README flow. | Marketplace install delivers the full 2.0 experience | +| **P9** | Delete `backend/`, `mcp_server/`, `server.py`, docker-compose, infra scripts. | No network I/O outside model calls, asserted in a test | + +**P2 and P7 are the gates.** P2 proves the economics and the concurrency assumption on a real spec. P7 proves the loop detects anything real. **Nothing is deleted until P7 is green** — the sequence front-loads cheap reversible work on purpose. + +--- + +## 8. What gets deleted (P9, not before) + +`backend/` (37.6k LOC app + 40.9k LOC tests), `mcp_server/`, `server.py`, `docker-compose.yml`, the K8s/NFS/Firestore/SQLite layer, `Dockerfile`, `scripts/init-mobile-sdk.sh`. + +Everything justified by *"run the harness on our infra"* or *"generated code takes hours and is irreplaceable"* — both premises are now false. Retry = rerun. Crash recovery = rerun. + +### Steel Commandments 2.0 (needs your ratification) + +The constitution rests on those same two premises. + +- **I–VI** (workspace sanctity, no-release-on-fail, archive-as-precondition, retry-reuses-workspace, no-background-touch) — **retire.** There are no workspaces. +- **VII–X** (state machine sole writer, forward-only checkpoints, transitions logged, invalid transitions raise) — **retire.** No state machine; state is files in the user's repo. +- **XI** — **retire**, superseded. + +Proposed replacements, each guarding a property 2.0 actually depends on: + +1. **No step performs network I/O beyond the model call.** Guards the compliance property that is now the product's main asset. +2. **No interpreter observes another interpreter's output.** Guards independence. +3. **Every verdict, count, and validation is produced by a script, never by a model.** Guards auditability — this is what makes output evidence rather than opinion. +4. **Artifacts are total or rejected.** The forcing function that replaces building. +5. **Samples are ephemeral and reproducible; never build machinery to preserve them.** The anti-pattern that produced ~9k LOC of preservation code. + +--- + +## 9. Risks + +| Risk | Severity | Handling | +|---|---|---| +| Simulated-build divergence may not track real spec defects | **High** | P7 mutation harness. This is the core product hypothesis and it is currently unproven. | +| Prose orchestration cannot guarantee a step ran | **High** | Artifact-passing (a stage's input is the prior stage's output file, so a skipped gate shows as a missing file) + non-zero-exit validators + hooks. **Cannot fully close** — this is the honest price of the architecture. | +| Self-reported blockers depend on agent introspection | **High** | An agent that silently assumes something won't report it. Divergence in the *total artifacts* is the objective backstop; do not build the report on self-reported blockers alone. | +| Integration-class defects only real building surfaces | **High** | §2's total artifacts + lenses + contract oracle recover much of it. Not all. Accepted, not solved. | +| Losing Cursor support | Medium | ⚠️ Skills and subagents are Claude Code only; `docs/IDE-SETUP.md` supports Cursor today. Either keep a thin shim or write parallel `.cursor/rules`. **Commercial decision — your call, not a technical blocker.** | +| No telemetry → slower iteration | Medium | For the compliance-sensitive buyer, not collecting telemetry is a feature. Substitute: artifacts live in the user's repo; ask design partners to share them. | +| Plugin-root path resolution for `lib/` | Medium | P0 verification. Per-skill shim as fallback. | +| Sales narrative weakens ("we build 3 prototypes") | Medium | Counter: no SpecFlow backend is used and artifacts stay in the repo; model-provider processing and cost still apply. | + +--- + +## 10. Decisions needed from you + +1. **Ratify the planning/refine order swap** (§1) — or tell me to keep your original order and I'll note why it's weaker. +2. **Ratify Steel Commandments 2.0** (§8) — I proposed retiring all eleven and replacing them with five. That's your constitution; I won't edit it unilaterally. +3. **Cursor: keep or drop** (§9). diff --git a/plans/specflow-2.0/status.md b/plans/specflow-2.0/status.md new file mode 100644 index 0000000..63f9294 --- /dev/null +++ b/plans/specflow-2.0/status.md @@ -0,0 +1,238 @@ +# SpecFlow 2.0 — where the plan and the code actually stand + +**Date**: 2026-08-04 +**Reads with**: `specflow-plugin-plan.md` in this directory, which is **stale in +its second half** — see §2. Read this file first. + +--- + +## 1. What is built + +A local refinement loop, shipped as its **own** Claude Code plugin plus four CLI +commands. + +- **`plugins/specflow2/`** — a second plugin in the existing marketplace, holding + **two** skills, `refine` and `resolve`, plus six lens prompts as data. Prose + orchestration: `refine` fans out independent subagents over the spec and decides + what reaches the user; `resolve` settles the decisions and writes them back into + the spec files. +- **`mcp_server/services/refine_{compare,artifacts,commands}.py`** — ~680 lines of + code that compare a round's readings. + `specflow refine new-round|round|resolve|status`. +- **State** — readable JSON under `/refine/` in the user's own repo. + +The split is one line: **code compares, a model judges.** Nothing in the CLI +scores, gates, or passes verdict. + +**`plugins/specflow/` is untouched — byte-identical to `main`,** symlinks and all. +That is the 1.0 companion plugin: two skills symlinked to +`mcp_server/services/skills/`, which is also what the MCP server serves. The two +plugins are independent products in one marketplace, and `specflow plugin install` +installs `specflow2` because it is the only one whose skills need this CLI present. +Before merge/release, reviewers install the CLI from `mcp_server/` and pass the +local checkout through `specflow plugin install --marketplace`. + +### What was cut, and why + +Everything not essential to *finding what a spec fails to determine* was removed +after the §5 argument below made it clear the loop cannot promise convergence: + +| Cut | Reason | +|---|---| +| `specflow-simulate` | One lens cannot disagree with itself, so it produces none of the signal. It was a demo. | +| `specflow-report` | A renderer over `refine status`. The orchestrator reports inline. | +| `specflow-analysis`, `specflow-planning` (2.0 copies of them) | Analysis is subsumed by six lenses reading the same spec; planning and estimating are downstream of a spec, not part of reading one. The 1.0 originals are unaffected — the PR briefly de-symlinked and forked them, and that is what got reverted. | +| `novelty`, `record_round`, `state.json`, `counts.new`/`counts.repeat` | The round-to-round diff. It existed to feed a stop rule the design cannot justify — see §5. Its output was being read as convergence. | + +What survived the cut is the part that does not depend on a convergence claim: +ID-keyed grid disagreement detection, grid coverage (uncovered cells and +agreed-but-guessed), exact-id blocker merging with attribution, and +`resolutions.json`. An exact blocker id already decided is suppressed; semantically +repeated lens findings with new agent-authored ids are handled by passing the +resolution artifact back to the model, not by pretending code can match prose. + +## 1a. What building gave us, and what actually replaces it + +The question this design has to answer: 1.0 found gaps by **building the app**, +which surfaced decisions nobody knew existed when they read the spec. Reading +cannot do that by default. Building was five separate instruments, and they are +recovered at very different rates — worth keeping separate, because collapsing them +into "we replaced building" is how the claim becomes dishonest. + +| Building gave | What replaces it | Recovered | +|---|---|---| +| **Enumeration by machine.** A compiler never had to be told which decisions existed — it walked the graph and stopped at each one. | **Per-lens matrices** (new here): each lens names its own rows and columns before answering, then must account for every intersection. Six lenses declare six cross-products from six angles. Plus the **shared grid**, which is narrower but comparable across lenses. | ~ mostly | +| **Composition failure.** Two decisions each fine, incompatible once both are instantiated. | The **coherence pass**: reads all six readings, asks which two answers cannot both be true. | ~ partly | +| **Dependency depth.** Hour-six decisions exist only because of hour-two decisions. | **Round two seeded with your resolutions**, so it reaches questions that exist only downstream of the last answers. | ~ partly | +| **Contradiction by impossibility.** You literally cannot write the code. | Nothing. A lens can *argue* two requirements conflict; nothing here fails to compile. | ✗ | +| **Runtime discovery.** Only true when data flows. | Nothing, and nothing short of building recovers it. | ✗ | + +**The matrix is the load-bearing addition, and the distinction from the deleted +gate matters.** The gate scored an agent against axes the agent itself had +declared, so it was gameable — declare fewer axes, pass. The matrix declares axes +too, but nothing passes or fails: unfilled cells are counted and printed, and the +value is in the *asking*. A cross-product cell exists whether or not anyone wants +to answer it, which is precisely the property prose lacks. + +Three outcomes per cell, and the middle one is why the mechanism earns its place: + +- **answered** (with `guessed` where the spec did not supply it), +- **`unanswerable`** — the lens reached the cell, could not answer, and said why. + A question the spec's own vocabulary forced into existence and cannot settle. + This is the closest thing here to what building used to surface, and it needs no + second lens to corroborate it: "this intersection has no answer" is a fact about + the spec, not an opinion about it. +- **missing** — enumerated and never returned to. A hole in the *reading*, not the + spec, reported as such so it is not mistaken for a finding. + +Each lens is also told to work in **three passes**: enumerate the axes answering +nothing, fill every intersection, then re-read its own matrix and account for every +remaining cell. The ordering is the point — a lens that answers while enumerating +only lists cases it can already handle. + +**Bounds worth stating.** The union of six lens-chosen cross-products is wider than +one shared grid but still not machine enumeration: a decision no lens's axes reach +stays invisible, and no count here reveals that. Matrices and free-form decisions +are never merged across lenses, because aligning one lens's "seat hold" with +another's "reservation" would take a similarity threshold — a judgment, and not +one belonging in code. The model may cluster that prose; deterministic cross-lens +comparison uses only shared grid cell ids. The grid is therefore required for new +manifested rounds, not an optional enhancement. + +**Still missing, and cheap.** Contract oracles (`specflow-contracts`, plan §5, +unbuilt) would recover *contradiction by impossibility* without building the app: +generate real SQL DDL, OpenAPI or type definitions from the spec and run real +validators, which reject things prose can express and a schema cannot. Generating +the **acceptance tests** rather than the implementation would force the same +"what does it do when X" enumeration, and a requirement you cannot write a test for +is itself the finding. + +## 2. The plan document is half-superseded + +`specflow-plugin-plan.md` was committed in the same branch as the code that +reverses it. Sections 1, 2 and 4 still describe what was built. These do not: + +| Plan says | Code does | +|---|---| +| §3 — an oracle library: `validate_artifact.py`, `check_totality.py`, `contracts_oracle.py`, `concordance.py`, `rank_blockers.py`, `saturation.py` | All cut (commit `6808a60`). No validator, no ranking, no saturation rule. | +| §3 — "Artifacts are total or rejected" as the forcing function | No totality gate. A reading is compared as far as it goes; the required grid reports unfilled cells instead of rejecting the reading. | +| §5 — `specflow-contracts` skill | Not built. | +| §5 — `specflow-mutate` as the internal QA harness | Not built. This was the P7 gate. | +| §5 — seven published skills | **Two.** See §1. Four were cut as non-essential; `specflow-contracts` was never built. | +| §6 — `plugins/specflow/lib/` with JSON schemas | Does not exist; the code lives in the CLI wheel instead. | +| §6 — `saturation.py` and the round loop's stop rule (P6) | Cut. The inference it encoded is unsupported — §5. | +| §7 — build order P0–P9, with P2 and P7 as the gates | P0/P8-ish done. **P2 and P7 not run.** | +| §8 — Steel Commandments 2.0, five replacements | Not ratified. `CLAUDE.md` still carries all eleven 1.0 commandments. | + +The reversal was deliberate and, on the merits, right: a completeness gate over a +checklist the agent wrote itself and a weighted score deciding what to ask about +were both judgments wearing arithmetic. Cutting them is the strongest decision in +the branch. But the plan of record now contradicts the code, including its own +"decisions needed from you" list — so it must not be read as current. + +**Action**: either rewrite §3/§5–§8 of the plan or mark it superseded. Leaving it +as-is means the next person builds `check_totality.py`. + +## 3. The product hypothesis is untested + +> Disagreement between independent readings of a spec tracks real spec defects. + +There is **no evidence for this in the branch.** Not weak evidence — none. No run +on a real spec is recorded, no cost is measured, and the harness that would have +produced the evidence (§5's `specflow-mutate`, the plan's own P7 gate) was cut. +The plan says "nothing is deleted until P7 is green"; P7 was deleted instead. + +This is the single thing worth doing next, and it needs no new code — the manual +procedure is in `docs/specflow-2.0/testing-the-refine-loop.md` §"Level 3". Plant +one ambiguity in a spec you have already built from, run the loop, and score +*detected* and *localized* separately. A finding that says "the spec is unclear +about holds" for a mutation in the payment section is a miss dressed as a hit. + +Until that has been done a few times, the honest claim is "it surfaces places +independent readings diverged", not "it finds spec defects". The skills are careful +to say exactly that, which is to their credit and is also the problem in §5. + +## 4. Independence is load-bearing and unenforceable + +The entire signal is that lenses read the spec with no knowledge of each other. +Nothing checks it, and nothing can: a round where six subagents shared context +produces artifacts byte-identical in shape to a round where they did not. Proposed +Steel Commandment 2.0 #2 ("no interpreter observes another interpreter's output") +states it as an invariant with no mechanism behind it. + +It is also not **recorded**, which is the cheaper half of the problem. A reading +does not say which model produced it or what its prompt contained, so a leaky +fan-out cannot be caught even after the fact. One provenance field per reading +would make it auditable without pretending to enforce it — worth doing before any +measurement in §3, because otherwise a good result cannot be distinguished from a +lucky one. + +## 5. The commercial tension worth naming + +The engineering integrity here — no score, no gate, no readiness percentage, every +judgment attributed to the model out loud — is directly at odds with having +something to sell. 1.0 could say "we built it three ways and measured the +variance". 2.0's most defensible claim is "here is where independent readings of +your spec diverged, and here is what an agent concluded from that", with an +explicit disclaimer that nothing was executed and nothing was proven. + +That may well be enough, especially with no SpecFlow backend cost in this path. +Model-token cost is unmeasured, and specification data follows the coding agent +and model provider's normal data path — it is not guaranteed to stay on-device. +But it is a *narrower* claim than §1 of the plan implies +("refine until the spec is unambiguous, then the plan you get is reliable" — the +code deliberately refuses to support that sentence), and the gap should be closed +deliberately rather than by a demo that overstates it. §3 is what closes it: a +measured detection rate on planted defects is a claim that survives contact with a +sceptical buyer, and it is the only one on offer. + +## 6. Is it a dead end? + +**No — but it is one experiment away from being one, and that experiment has not +been run.** + +What is genuinely new and worth keeping regardless of how §3 turns out: + +- **Located, attributed findings.** "This question, at this anchor, answered two + ways by these two lenses" is a categorically better artifact than a prose + blocker list, because it can be checked by a human in seconds. +- **The grid.** Enumerating the decisions before anyone answers them turns a gap + from something you must notice into something you can count. Cheap, and it is + the closest thing here to the forcing function a compiler provided. +- **Agreed-but-guessed.** Naming "consensus over a silent spec" as a distinct + finding is the sharpest idea in the branch — it is the failure mode a + disagreement-only design structurally cannot see, and most tools in this space + do not have a name for it. + +What would make it a dead end: + +- §3 comes back negative — divergence turns out to be noise about wording rather + than signal about spec defects. Plausible; the lenses are prompted to be + adversarial, and adversarial readings diverge on determinate specs too. The + false-positive rate on a spec you consider unambiguous is as important as the + detection rate, which is why Level 3 measures both. +- Or the loop never terminates on anything but a judgment call. That is now + **settled, not open**: the stop rule is gone, the loop reports no verdict, and + the skill says out loud that stopping is the user's call. It costs the product + its most natural-sounding claim, and the honest replacement is the measured + detection rate in §3 rather than a convergence promise. + +## 7. Smaller things carried over + +- **`CLAUDE.md` describes only the 1.0 flow.** It has no mention of the local + channel, and nothing there records the two rules that keep the marketplace's two + plugins from colliding: `specflow2` ships no analysis or planning skill, and + nothing in it writes to `analysis/` or `planning/`. Those rules are currently + written only in `plugins/specflow2/README.md` and `bundled_skills.py`. +- **Prose is most of the diff and none of it is testable.** ~680 lines of code + against ~930 of skills and README. The product lives in the prose, so + the review surface is mostly unverifiable by construction. Stated as accepted + risk in the plan (§9, "cannot fully close") and it is the right call — but it + means Level 2–3 human observation is not optional, it is the only coverage the + orchestration has. +- **The plugin path serves 1.0 templates unsubstituted.** Pre-existing on `main` + and out of scope here, but worth recording: the symlinked skills contain + `<>` / `<>` / `<>` placeholders that + `server.py:_make_prompt_text` fills in for the MCP tool responses. Nothing fills + them on the plugin path, so a `specflow` plugin user running `/specflow-analysis` + sees the literal tokens. Unchanged by this PR either way. diff --git a/plugins/specflow2/.claude-plugin/plugin.json b/plugins/specflow2/.claude-plugin/plugin.json new file mode 100644 index 0000000..9cc3c06 --- /dev/null +++ b/plugins/specflow2/.claude-plugin/plugin.json @@ -0,0 +1,18 @@ +{ + "name": "specflow2", + "description": "Refine specifications before you build. Independent subagents produce ID-keyed readings under different adversarial lenses; a local CLI compares them and records decisions without using the SpecFlow backend. Your coding-agent and model-provider data policies still apply.", + "version": "0.1.0", + "author": { + "name": "Grid Dynamics" + }, + "homepage": "https://github.com/griddynamics/specflow", + "repository": "https://github.com/griddynamics/specflow", + "license": "MIT", + "keywords": [ + "specflow", + "spec-refinement", + "spec-ambiguity", + "blocker-detection", + "requirements" + ] +} diff --git a/plugins/specflow2/README.md b/plugins/specflow2/README.md new file mode 100644 index 0000000..37e95b4 --- /dev/null +++ b/plugins/specflow2/README.md @@ -0,0 +1,227 @@ +# specflow2 — the spec-refinement plugin + +Spec refinement orchestrated from the user's Claude Code session, with artifacts +and deterministic comparison kept in the local repository. The SpecFlow backend +is not used; the coding agent and its model provider still receive whatever +specification context their normal operation requires. + +**Separate from the `specflow` plugin in the same marketplace.** That one carries +the two skills of the 1.0 backend flow — `specflow-analysis` and +`specflow-planning`, symlinked to the templates the MCP server serves for +`check_specification_completeness` and `run_planning`. This one carries the local +refinement loop and nothing else. Installing either does not affect the other, and +they write to different places: 1.0 owns `/{analysis,planning}/`, this +plugin owns `/refine/`. + +**This plugin is prose.** Skills do the work: they spawn independent subagents, +sequence rounds, and decide what reaches the user. The SpecFlow CLI — shipped +separately on PyPI as `gd-specflow` — does the part prose is bad at. + +The split follows one rule: + +**Code compares stable IDs.** Which lenses answered the same shared-grid cell +differently. Which cells nobody filled. Which exact blocker ids several lenses +raised independently. Which exact blocker ids you already resolved. It does not +guess that differently worded prose means the same thing; the model judges that. + +**A model judges.** Whether the architecture is sound, whether the spec is ready, +whether a decision is worth interrupting someone over, whether another round is +worth running. None of that is checkable, and earlier versions of this plugin tried +anyway — a completeness gate over a checklist the agent wrote itself, a weighted +score deciding what to ask about, and a round diff read as convergence. All three +were judgments wearing arithmetic. They are gone, and the skills now make those +calls out loud where a user can disagree with them. + +So there is no validator, no readiness score, no stop rule, and nothing that will +tell you your spec passed. What you get is: here is where independent readings of +your spec disagreed, and here is what the agent concluded from that. + +## What replaces building + +SpecFlow 1.0 found gaps by building the system — hours per variant. That worked for +one reason: a compiler never had to be told which decisions existed. It walked the +graph and stopped at every undecided thing. Reading a spec walks nothing, and a +paragraph can omit a case without looking incomplete. + +Four things restore that forcing, in order of how much they recover: + +- **Each lens's own matrix.** Before answering anything, a lens names its rows and + columns — held resources × colliding operations, entities × lifecycle events, + operations × who is calling — and then must account for **every intersection**. A + cell exists whether or not anyone wants to answer it. A cell the lens reaches and + cannot answer, with a reason, is the single most useful thing this loop produces. + Six lenses declare six different cross-products, so a case invisible from one + angle is often forced by another. +- **A shared grid.** One pass enumerates decisions for *all* lenses, so those cells + are comparable across readings — narrower than the matrices, but the only thing + here with a denominator. A cell nobody filled is a countable gap; a cell every + lens filled *by guessing* is agreement that is not evidence. +- **A coherence pass.** After the lenses finish, one agent asks whether their + answers can all be true at once. Disagreement finds gaps but never finds lenses + that agreed and were jointly wrong; building found those when the code did not + run. +- **The next round.** Seeded with your resolutions, so it reaches the questions that + exist only because of how you answered the last ones. + +**None of it executes anything.** A matrix forces the *question* to exist; only +running code proves an answer wrong. So this cannot find what appears when data +flows, and it cannot show a decision is impossible — a lens can argue two +requirements conflict, but nothing here fails to compile. For the one decision that +is expensive, irreversible, and split, the skill tells you to spike it: half an hour +of real code, not eight hours of it. + +## Install + +```bash +uv tool install gd-specflow # the CLI +specflow plugin install --target claude +``` + +The second command points Claude Code at the published marketplace and installs +`specflow2` from it. Installing in that order matters: the skills call `specflow`, +so the CLI has to exist first. If you added the marketplace by hand instead, the +skills will tell you what is missing — and note the plugin name: + +```bash +claude plugin marketplace add griddynamics/specflow +claude plugin install specflow2@specflow-marketplace # not `specflow` +``` + +Before this branch is released, install and test from a checkout instead of +PyPI/default-branch contents: + +```bash +cd mcp_server +uv tool install --force . +specflow plugin install --target claude --marketplace "$(git rev-parse --show-toplevel)" +``` + +## Skills + +Two. + +| Skill | Job | +|---|---| +| `specflow-refine` | the loop — fan out independent lenses over the spec, compare, repeat | +| `specflow-resolve` | settle the decisions it found and write them back into the spec | + +The six adversarial lenses ship as data (`skills/specflow-refine/lenses/*.md`), +not as skills. Nobody types "run the idempotency lens", and a seventh lens is one +markdown file. Lens count is the cost dial. + +Nothing else is here on purpose. Earlier drafts of this plugin shipped a +single-lens pass, a read-only status renderer, a spec-analysis pass and a planning +skill. Each was +either a wrapper over one CLI command, or work this loop is not for: one lens +cannot disagree with itself, so it produces none of the signal, and planning and +estimating are downstream of a spec rather than part of reading one. The +deliverable here is a spec with fewer holes in it and a list of the holes that +remain. + +## Models + +SpecFlow never calls a model. The skills run in your coding agent and every +subagent inherits that agent's model, so the choice is yours and you make it +where you already make it. Any model your agent can run, SpecFlow runs on. + +| Job | Needs | +|---|---| +| the `/specflow-refine` orchestrator — decides what reaches you | best-in-class model | +| the lenses, and `/specflow-resolve` | general purpose | + +Small and cheap under-reports as a lens: sent to attack a spec, it agrees with +it, and a lens that finds nothing still costs a round. + +If your harness can give different subagents different models, spread the lenses +across model families — disagreement between readings is the signal, and one +model disagrees with itself less than several do. + +(The 1.0 backend flow is unchanged: OpenRouter, configured per tier with +`LLM_HIGH` / `LLM_MEDIUM` / `LLM_LOW` in your MCP client.) + +## The commands the skills call + +Four, and each exists only because a model doing the same job by eye would be +less reliable — not more authoritative. + +``` +specflow refine new-round allocate the next round directory and name its files +specflow refine round compare this round's readings against each other and + against the grid +specflow refine resolve validate and record an exact blocker-id decision +specflow refine status read the last round's findings back, minus anything + resolved since +``` + +New rounds carry a manifest naming their expected lenses and require a grid. The +grid supplies the stable cell IDs that make deterministic cross-lens comparison +possible. The coherence file remains optional; when present, its blockers are +folded into the result. + +Exit codes: `0` success, `2` bad usage — a missing round, a round with no +readings, a file that is not the JSON it should be. Every message names the file. +Nothing fails a run on a judgment call, and re-running a round simply replaces its +findings. + +**There is no stop rule, and that is the deliberate part.** An earlier version +diffed each round against the previous ones, and the skill read "nothing new" as +convergence. That inference does not hold: `new == 0` is equally consistent with +this round's lenses finding less, and nothing holds lens effort constant between +rounds. Making it a threshold would need the false-negative rate of a single +round, which is unmeasured — so the diff, its counts and the round ledger behind +them are gone. When to stop is a judgment the skill makes out loud, where you can +disagree with it. + +Source: `mcp_server/services/refine_compare.py` (comparison), +`refine_artifacts.py` (file layout), `refine_commands.py` (the command group). +About 680 lines of code, a third of it argparse wiring and output formatting. If +the comparison module starts growing, check whether a judgment has crept into it. + +## Trying it out + +`docs/specflow-2.0/testing-the-refine-loop.md` walks the whole thing, cheapest +first. Level 1 drives the four commands with hand-written artifacts and **no model +at all**, which is the fastest way to see what the loop does and the only way to +tell a CLI bug from a subagent that read the spec badly. + +## Where the artifacts go + +Everything the loop reads or writes lives under `/refine/`, one +directory the local flow owns outright: + +``` +docs/refine/ + resolutions.json decisions made, cumulative + findings.json the latest round's merged view + round-01/ manifest.json, grid.json, reading..json, + optional coherence.json +``` + +Readable JSON in your own repo — git-tracked, diffable, and editable with the +tools you already have. There is no database and no server to query. + +Two things are absent by design. There is **no round ledger**: it existed only to +feed the stop rule described above, and rounds are simply the directories present. +And the loop writes **no markdown report** — the skill reports to you in the +conversation, which is where you can argue with it. + +## Why this plugin writes no `analysis/` or `planning/` files + +Those two directories belong to the 1.0 flow. Its contract reserves +`analysis/specification_completeness.md` and `planning/IMPLEMENTATION_PLAN.md`, and +its validator rejects an analysis file with no Part F readiness section — a section +that exists only to tell that backend whether to run E2E, and one this loop has no +reason to produce. + +An earlier draft of the 2.0 skills lived in the `specflow` plugin alongside the 1.0 +ones and wrote those same filenames with different content, so a user could run the +local skill and then have `run_generation` reject the file they never meant to hand +it. Splitting the plugins fixed that at the root: `specflow` has the 1.0 skills and +only those, `specflow2` has the loop and writes only under `refine/`, which is +outside the three directories that validator searches. + +Two rules keep it that way. **Don't add an analysis or planning skill here** — if +you want to change what 1.0 produces, edit +`mcp_server/services/skills/specflow-{analysis,planning}/SKILL.md`, which is the +single source for both the MCP tool responses and the `specflow` plugin. And +**don't write into `analysis/` or `planning/`** from any skill in this plugin. diff --git a/plugins/specflow2/skills/specflow-refine/SKILL.md b/plugins/specflow2/skills/specflow-refine/SKILL.md new file mode 100644 index 0000000..040f0eb --- /dev/null +++ b/plugins/specflow2/skills/specflow-refine/SKILL.md @@ -0,0 +1,473 @@ +--- +name: specflow-refine +description: Refine a specification before you build it. Independent subagents read the spec under different adversarial lenses; a local CLI compares their ID-keyed artifacts and records decisions. Uses no SpecFlow backend; model-provider data handling still applies. +argument-hint: "(optional) spec_dir outputs_dir — defaults: specs docs" +--- + +# SpecFlow Refine + +You are orchestrating a specification refinement loop. Several subagents each +read the same spec under one adversarial lens, with no knowledge of each other. +Where independent readings give different answers to the same grid cell, report +the divergence. It is evidence to inspect, not proof by itself that the spec is +defective. + +**You do not write code and you do not commit anything.** The point is to find +what the spec fails to determine, before anyone spends days building one arbitrary +reading of it. + +## Arguments + +- `spec_dir` — specification root. Default `specs`. +- `outputs_dir` — where artifacts are written. Default `docs`. + +Check the CLI is reachable once, at the start: + +```bash +specflow refine --help >/dev/null +``` + +If it is missing, tell the user to install it and stop: + +``` +uv tool install gd-specflow +``` + +--- + +## What the CLI does, and what you do + +Read this before running anything. Getting it backwards is the main way this loop +goes wrong. + +**The CLI compares and remembers.** It compares answers carrying the same grid +cell id, merges blockers carrying the same exact id, reports missing expected +readings, and suppresses exact blocker ids already resolved. It does not infer +that differently worded prose means the same thing. Semantic clustering stays +your judgment. + +**You judge.** Run this skill on a best-in-class model — the calls below are +yours, not the CLI's. Is this spec good enough to build from? Is this architecture +coherent? Is this decision worth interrupting the user over? Has the loop found +everything worth finding? None of that is checkable by a script, and there is +deliberately no gate, no score, and no completeness check pretending otherwise. + +So: never say "the validator confirmed the spec is ready" — nothing validates +that. Say what you concluded and why, and let the user disagree with you. + +**Independence is the one rule you must not weaken.** Subagents never see each +other's output, and there is no shared plan. Two lenses reaching different +conclusions from the same spec is the entire signal; shared context destroys it. + +The rule that makes the grid below legal: **share the questions, never the +answers.** Every lens may know which cells exist, because that came from the +spec. No lens may know what another put in one. + +--- + +## What replaces building + +The predecessor to this loop found gaps by building the system — hours of it. +That worked because code will not compile half a decision: every field needs a +type, every branch a body, every error path a return. Reading a spec forces +nothing, and an agent slides past a gap without noticing. + +Four things below restore that forcing, at a fraction of the cost. They are listed +in order of how much they recover: + +- **The per-lens matrix** (§ the reading format) is the strongest, and the reason + the loop reaches decisions nobody knew to ask about. Each lens names its own rows + and columns *before* answering anything, then must put something in every + intersection. A cell exists whether or not anyone wants to answer it — that is + the property a compiler had and prose does not. Six lenses declare six different + cross-products, so a case invisible from one angle is often forced by another. +- **The grid** (step 2) does the same over one list, shared by every lens, which is + what makes its cells comparable *across* lenses. Narrower than the matrices — + one author, one set of blind spots — but it is the only thing here with a + denominator you can count against. +- **The coherence pass** (step 4) asks whether the answers can all be true at + once. Disagreement finds gaps; it never finds two lenses that agreed and were + jointly wrong. Building found those when the code did not run. +- **The next round** (step 7) is seeded with your resolutions, so it reaches the + gaps that exist *because* of how you resolved the last one. That is the + dependency ordering a build gets for free. + +**What none of them restore is execution.** A matrix forces the *question* to +exist; only running code proves an answer wrong. So this loop cannot find what +appears when data actually flows, and it cannot show that a decision is impossible +— a lens can argue two requirements conflict, but nothing here fails to compile. +See step 7 for the one place you should still spend half an hour building. + +--- + +## The loop + +### Step 1 — allocate a round + +```bash +specflow refine new-round --outputs --lens concurrency partial-failure data-lifecycle auth-boundaries idempotency ordering +``` + +Note the round number and directory it prints, along with the paths for the grid +and the coherence file. + +Six lenses is the default. Fewer is cheaper and finds less; more costs +proportionally. This is the cost dial — start with three on a first pass if the +spec is large. + +### Step 2 — sketch the grid + +Spawn **one** subagent to enumerate the decisions the spec implies, and to answer +none of them. It writes `/grid.json`: + +```json +{ + "cells": [ + { + "id": "hold.timeout", + "question": "A seat hold is open and its timer expires — what happens?", + "where": "specs/booking.md#Holds" + } + ] +} +``` + +Cells come from the spec's own vocabulary, not from any lens: + +- every entity the spec names × every event that can reach it, +- every role × every protected resource × every action, +- every stored field that can be absent, expire, or change. + +Aim for a few dozen. If the spec implies hundreds, narrow this round to a +subsystem and say which one — a grid too large to fill is one nobody fills. + +Two rules make this worth doing. **The enumerator must not answer** — a cell +carrying a suggested value contaminates every lens that reads it. And **the grid +is written once, for all lenses**; a lens that picks its own cells has scoped its +own exam, which is exactly how the deleted completeness gate failed. + +This step is required. The grid ids are the deterministic identity shared across +independent readings; without them, code cannot know that two differently worded +questions mean the same thing. If the grid is too broad, narrow the round rather +than omitting it. + +### Step 3 — fan out + +Read each lens file from `lenses/` in this skill's directory. Then spawn **one +subagent per lens, all in a single message** so they run concurrently. + +Give each subagent: + +- the full contents of its lens file, +- the spec directory to read, +- the grid from step 2, with the instruction to fill every cell its lens has a + view on and to leave the rest alone, +- the reading format below, +- **the three-pass instruction below, verbatim**, +- the exact output path: `/reading..json`. + +**Tell every lens to work in three passes, in this order.** The order is what +makes the decisions appear at all; a lens that answers as it goes only ever finds +what it already thought to look for. + +> **1. Enumerate, answer nothing.** Name your matrix axes first — the rows and +> columns your lens attacks along, from the lens file. Write the axes down before +> you know a single answer. If you answer while enumerating you will only list the +> cases you can already handle, which is the exact failure this pass exists to +> prevent. +> +> **2. Fill every intersection.** Go cell by cell. Say what the system does. Where +> the spec did not tell you, answer anyway and mark it `guessed: true`. Where you +> genuinely cannot answer, write `unanswerable` with one line on why — that is a +> finding, not a failure, and it is the most useful thing you can produce. +> +> **3. Re-read your own matrix and account for every cell.** Any cell still empty +> is one you enumerated and walked away from: go back and either answer it, or say +> why it cannot be answered. Then ask of your filled cells: *which of these did I +> assert more confidently than the spec justifies?* Downgrade those to `guessed`, +> and promote to a blocker any guess that would be expensive to get wrong. + +**Never tell a subagent what another lens found, and never pass it a plan.** Each +one reads the spec cold. If a previous round produced resolutions, pass +`/refine/resolutions.json` — those are now part of the spec's +meaning, so all lenses may see them equally. + +**Which model runs a lens.** SpecFlow does not choose — a subagent inherits +whatever your harness gives it. A lens needs a general-purpose model or better; +small and cheap under-reports, agreeing with the spec it was sent to attack. If +your harness can give different subagents different models, spread the lenses +across model families — one model disagrees with itself less than several do. + +### Step 4 — check coherence + +Once every lens has written its reading, spawn **one** subagent over the whole +round directory. Its question is not "is anything missing" but: + +> Take these answers as given. Which two of them cannot both be true? + +It is looking for the failure disagreement cannot see — a locking rule that makes +the retry policy unreachable, a retention window shorter than the dispute window, +an idempotency key that does not survive the partition the ordering lens assumed. +Building caught these when the code did not run. Nothing else here does. + +It writes `/coherence.json`, blockers only, same format as below: + +```json +{ "blockers": [ { "id": "locking-blocks-retry", "...": "..." } ] } +``` + +This pass reads every lens's output, so it is **not** an independent reading. It +runs after the lenses are done, never before, and it never contributes decisions +— only blockers. The CLI enforces the second half of that: coherence blockers are +attributed to `coherence` and left out of the lens count. + +Skipping it is allowed and costs you this class of finding entirely. + +### Step 5 — compare + +```bash +specflow refine round --outputs +``` + +This prints where readings gave different answers to the same grid ids, the +merged exact-id blockers with attribution — +which lenses independently raised each, and which you have already decided — and, +which cells no lens answered and which were answered by every lens guessing. It +writes `findings.json`, which `refine status` reads back. + +It passes no verdict. There is no score, no readiness call, and no signal that the +loop is done; see step 7. + +Read those last two carefully. **A cell nobody answered is a gap so complete that +no reading even reached it**, and it will never appear as a disagreement. **A cell +every lens guessed at is agreement that is not evidence** — the spec was silent +and they converged anyway. Both are findings; neither is a disagreement. + +**Then read the matrix section, and treat its two lines differently.** + +- **`spec cannot say`** — a lens reached a cell in its own cross-product and + reported that the spec cannot answer it, with a reason. These are the decisions + that only exist because something forced the question, and they are the closest + this loop gets to what building used to surface. Promote the expensive ones to + the user in step 6 even when no other lens mentioned them; a single lens is weak + evidence for a *judgment*, but "this intersection has no answer" is a fact about + the spec, not an opinion about it. +- **`never filled`** — a lens enumerated a cell and never came back to it. That is + a hole in the reading, not in the spec. Re-run that lens before you conclude + anything about the area it covers, and do not report those cells to the user as + findings. + +**Check what the round actually saw before you read anything into it.** Two +sections of the output decide how much the rest is worth: + +- **`Readings that could not be compared`** — a lens whose file is missing or + malformed did not participate. The headline says `N of M readings compared` + when they differ. Re-run that lens before treating the round as a full one; the + gaps it would have found are simply absent, not shown to be absent. +- **`Worth knowing about this round`** — two files claiming the same lens name + means the fan-out likely copied one lens into another file, and you have fewer + independent readings than the manifest says. + +If a command exits non-zero it tells you which file is wrong. Fix the file and +re-run; re-running a round simply replaces its findings. + +### Step 6 — decide what reaches the user + +The command does **not** sort blockers into ask/assume for you. That is your +judgment, and here is how to make it: + +- **Ask** when a wrong guess is expensive to undo — it changes the architecture, + or the data model, or a security boundary. Also ask when the lenses disagreed, + because that is evidence the spec genuinely underdetermines it. +- **Assume** when the choice is cheap to reverse. Apply the recommendation, + record it with `--source assumed`, and tell the user in a batch afterwards. +- **Drop** a lone cosmetic nitpick. One lens being pedantic is not a finding. + +Use the blocker's own `impact` and `reversible` fields as input, not as gospel — +a subagent that marked everything `blocks_build` was being defensive, and you +should say so rather than flooding the user. + +Then hand off to `/specflow-resolve`, or handle it here with `AskUserQuestion`. + +### Step 7 — loop or stop + +**Nothing tells you when to stop. That is deliberate, and it is your call.** + +There used to be a signal here — the command diffed each round against the +previous ones and reported what was new. It was removed, because "this round +raised nothing new" has two causes that leave identical artifacts: the spec has +nothing left to give, or *this round's lenses found less*. Nothing holds lens +effort constant between rounds, so the second reading is always available, and a +loop that stopped on the first was reporting a guess as a measurement. + +So decide out loud, from what you can actually see: + +- **Was the round whole?** Six readings compared, or four? A partial round found + less because it *saw* less. Fix the missing lenses and re-run before concluding + anything — re-running a round is safe and overwrites its findings. +- **Did you resolve anything substantial?** If so, run another round: the spec has + changed, and round two reaches the questions that exist only downstream of how + you answered the last ones — the ones a build would have hit in hour six because + of a decision made in hour two. **Round two is not a retry.** +- **Are the remaining open blockers ones you are willing to build on?** That is + the actual question, and it is a judgment about your risk, not about the spec's + completeness. + +Whatever you conclude, say it as your own assessment and say what it does not +mean. "The last round surfaced nothing I judge worth blocking on" is honest. "The +spec is complete" is not a claim this loop can support, and nothing in it will +tell you otherwise. + +**Spike the one decision worth executing.** If a blocker is expensive and +irreversible and the lenses split on it, no amount of reading settles it — that +is precisely what building was for. Write only that one interface and its state +transitions, half an hour, and let it fail. Recommend this rather than pretending +the round covered it. + +When you stop, report the decisions resolved, what is still open, what you +assumed, and where the readings disagreed — then hand the user back their spec. +Planning, estimating and building are not this skill's job and there is no +follow-on skill to point at; the deliverable is a spec with fewer holes in it and +a list of the holes that remain. + +--- + +## The reading format + +Give this to every subagent verbatim. One JSON object per lens: + +```json +{ + "lens": "concurrency", + "spec_root": "specs", + "matrices": [ + { + "name": "held resource × concurrent operation", + "rows": ["seat hold", "payment intent", "seat inventory"], + "cols": ["second hold attempt", "cancel", "timer expiry"], + "cells": [ + {"row": "seat hold", "col": "second hold attempt", + "value": "rejected with 409", "guessed": true}, + {"row": "seat hold", "col": "timer expiry", + "unanswerable": "spec never says who owns the timer, so two owners give two answers"} + ] + } + ], + "cells": [ + {"id": "hold.timeout", "value": "seat returns to the pool", "guessed": true} + ], + "decisions": [ + { + "question": "What does the second caller see while a seat is held?", + "value": "blocks until the hold is released", + "where": "specs/booking.md#Holds", + "guessed": true + } + ], + "blockers": [ + { + "id": "seat-contention-loser", + "title": "What the losing caller sees on a contended seat", + "question": "Reject with 409, or queue the caller?", + "where": "specs/booking.md#Holds", + "options": [ + {"label": "reject-409", "consequence": "caller must retry"}, + {"label": "queue", "consequence": "unbounded wait under load"} + ], + "recommended": "reject-409", + "impact": "changes_behaviour", + "reversible": false + } + ] +} +``` + +**`matrices`** — the lens's own cross-product, and **the reason this loop finds +anything a careful read would not.** Each lens names its own axes, from its own +angle of attack, and then must put something in every intersection. That is the +one property building had for free: a compiler never needed to be told which +decisions existed, because it walked the graph and stopped at each one. Prose walks +nothing and can omit a case without looking incomplete. A grid of rows × columns +cannot — the cell exists whether anyone wants to answer it or not. + +Three things may go in a cell, and they mean different things: + +- **an answer** — `value`, plus `guessed: true` if the spec did not supply it, +- **`unanswerable`** — you reached the cell, cannot answer it, and say why in one + line. **This is the most valuable output of the whole reading**: a question the + spec's own vocabulary forced into existence and cannot settle, +- **nothing at all** — no entry. Reported as a cell you enumerated and never came + back to, which is a hole in your reading rather than in the spec. + +Nothing gates on filling it. There is no score and no pass mark, so padding a cell +with a confident invention buys you nothing and costs the round a real finding. + +Axes come from the lens file (§ *the matrix to fill*). Keep each matrix small +enough to actually complete — two axes of three to six values each. Two small +matrices beat one 12 × 12 nobody finishes. + +**`lens`** — write it, but **the filename decides**. `reading.ordering.json` is the +`ordering` lens whatever its body says, because a body written by hand across six +concurrent prompts is exactly where a copy-paste puts the same name on two files — +and two readings sharing a lens name collapse into one, taking the disagreement +between them with it. A mismatch is reported, not silently obeyed. + +**`cells`** — the grid, answered. One short value per cell the lens has a view +on, keyed by the id the grid gave it, and `guessed: true` whenever the spec did +not say. Omit a cell rather than filling it from nothing; a concurrency lens has +no business answering an authorization cell, and a lens that fills everything +tells you less than one that fills what it knows. Comparison here needs no word +matching — the id already says two lenses answered the same question. + +**`decisions`** — every question the lens had to answer to proceed, with the +answer it settled on. Write it down even when the spec was silent and set +`guessed: true`. The orchestrating model reads these as evidence, but the CLI does +not compare their free-form wording: doing so would introduce an arbitrary +similarity threshold. Cross-lens deterministic comparison happens only in +`cells`, keyed by the shared grid id. + +**`blockers`** — decisions the lens could not responsibly make alone. Needs a +stable slug `id`, a `question` answerable in one line, at least two `options` with +consequences, a +`recommended`, `impact` (`blocks_build` / `changes_architecture` / +`changes_behaviour` / `cosmetic`), and `reversible`. + +The CLI merges blockers only when their ids are exactly equal. If the blocker is +about a grid cell, derive its id from that cell id. For a lens-only finding, use a +semantic slug and do not assume another independent lens will choose the same +one; the orchestrating model remains responsible for recognizing equivalent +prose. + +**`where`** — the spec file and section. Used to group a disagreement with the +blocker at the same place, so one gap does not show up as two items. + +Tell subagents plainly: **a guessed answer recorded honestly is more useful than a +confident one.** The comparison only works if each lens reports what it actually +concluded. There is no gate rewarding a full-looking artifact, so there is no +reason to pad one. + +--- + +## Asking well + +Human attention is the scarce resource here. + +- **Prefer proposing.** "I'll assume X unless you object" clears most items for + free. Reserve real questions for consequential forks. +- **One line to answer.** Give the scenario, the options with consequences, and + your recommendation. If a question needs a paragraph of setup, the lens did not + finish its work — say so rather than passing the confusion on. +- **Batch.** Present related decisions together. +- **Say who found it, not a number.** "Five of six independent readings hit this" + is useful and true. A ratio or a score is not — there is no calibration behind + one. + +## Reporting + +Report counts and observations: decisions resolved, still open, assumed, and +where the readings disagreed. No composite metric — this loop has no calibration +to justify one, and a number would make a judgment look like a measurement. + +Say plainly what the loop cannot do: several agents reading a spec is not the same +as building it, so it will miss defects that only appear when code runs. That +honesty is what makes the findings it *does* report worth acting on. diff --git a/plugins/specflow2/skills/specflow-refine/lenses/auth-boundaries.md b/plugins/specflow2/skills/specflow-refine/lenses/auth-boundaries.md new file mode 100644 index 0000000..517b54a --- /dev/null +++ b/plugins/specflow2/skills/specflow-refine/lenses/auth-boundaries.md @@ -0,0 +1,63 @@ +# Lens: authorization boundaries + +Simulate building this system while asking, for **every single operation**: who +may call this, and on whose data? + +Specs usually name roles once and then describe features as though the caller is +always entitled. The gap is per-operation, so check them one at a time. + +Work through the spec asking: + +- For each operation, which actors may invoke it? Not "logged-in users" — + which ones, and under what condition? +- Which operations act on a record belonging to someone else? What relationship + must hold between caller and record? An endpoint that takes an id and does not + check ownership is the most common real vulnerability in generated code. +- Where does one actor act on behalf of another (admin, support agent, + automation)? Is that impersonation visible in the audit trail, and are its + limits stated? +- Which reads are as sensitive as writes? Listing and searching leak data even + when the caller cannot change anything. Does a list endpoint filter to the + caller's scope? +- What is visible in an error? "Record not found" versus "not permitted" tells + an attacker whether the record exists. +- Which fields may the caller set, and which are server-controlled? A caller who + can write `role` or `price` has an authorization bug, not a validation bug. + +## The matrix to fill + +Name the axes before you answer anything, then put something in every cell. + +- **rows** — every operation the spec describes. Every one, including the ones + that read like plumbing; this lens exists because specs name roles once and then + describe features as though the caller is always entitled. +- **cols** — the caller's relationship to the data being touched: *owns it*, + *same organisation or team*, *different tenant*, *unauthenticated*, *another + service*, *an operator or support user*. + +Each cell says allowed or refused, **and what a refusal looks like** — 403, 404 to +avoid leaking existence, or a filtered result. Those are different decisions and +the spec usually makes none of them. + +A cell you cannot answer is the finding. Write `unanswerable` with the reason, for +example *the spec never says whether support staff are a role or an escalation, so +this row has no defined answer*. + +## What counts as a blocker here + +The spec is missing a decision wherever an operation touches data it does not +prove the caller owns. Also wherever roles are named but their permissions are +not enumerated — "admins can manage users" is a role, not a rule. + +## Decisions to record + +Write these into `decisions` even where the spec is silent — that is what makes +another lens's different answer visible. Mark a guess `guessed: true`. + +- For every mutating operation: who is allowed to call it, on whose records? +- Which fields are server-controlled, and what happens if a caller sends one? +- Who may act on someone else's data, and under what escalation? +- Where does the spec name a role without saying what it can do? + +Raise a blocker for each operation whose ownership rule you had to infer. These +are cheap to fix in the spec and expensive to discover in production. diff --git a/plugins/specflow2/skills/specflow-refine/lenses/concurrency.md b/plugins/specflow2/skills/specflow-refine/lenses/concurrency.md new file mode 100644 index 0000000..501bf62 --- /dev/null +++ b/plugins/specflow2/skills/specflow-refine/lenses/concurrency.md @@ -0,0 +1,58 @@ +# Lens: concurrency + +Simulate building this system for **two things happening at once**. + +Assume every operation can be invoked simultaneously by different actors, and +that no operation is instantaneous. Work through the spec asking: + +- Which two operations, run at the same moment on the same record, produce a + result neither one intended? +- What must be held while a multi-step operation is in flight? For how long? + What happens to the second caller meanwhile — wait, fail, or proceed? +- Where does the spec assume it is the only writer? Check every read-then-write + sequence: is the value still true when the write lands? +- Which invariants are stated as if they hold continuously ("stock is never + negative", "a seat has one holder") and could be violated in the window + between check and commit? +- What is the unit of atomicity? If a request touches three records, can it + leave two updated and one not? + +## The matrix to fill + +Name the axes before you answer anything, then put something in every cell. + +- **rows** — every resource the spec says can be held, reserved, decremented or + claimed. Take the names from the spec, not from your own model of it. +- **cols** — every operation that can reach one of those resources while another + operation already has it. Include the operation colliding with *itself*. + +Each cell answers one question: **who wins, and what does the loser see?** "The +database handles it" is not a cell value — it names a mechanism, not a behaviour. + +A cell you cannot answer is the finding. Write `unanswerable` with the reason in +one line, for example *the spec never says who owns the timer, so two owners give +two different answers here*. + +## What counts as a blocker here + +The spec is missing a decision if you cannot answer, for any contended +operation: *who wins, and what does the loser see?* "The database handles it" +is not an answer — it names a mechanism, not a behaviour. + +Note that an invariant the spec states without saying how it is enforced under +contention is a real gap even when the happy path is fully specified. + +## Decisions to record + +Write these into `decisions` even where the spec is silent — that is what makes +another lens's different answer visible. Mark a guess `guessed: true`. + +- For every contended operation: *who wins, and what does the loser see?* +- For every operation: is running it twice at once safe? +- For each stated invariant: what enforces it in the window between check and + commit? +- What is the unit of atomicity when one request touches several records? + +Raise a blocker where you would not be willing to pick for the user. An invariant +the spec states without saying how it holds under contention is a real gap even +when the happy path is fully specified. diff --git a/plugins/specflow2/skills/specflow-refine/lenses/data-lifecycle.md b/plugins/specflow2/skills/specflow-refine/lenses/data-lifecycle.md new file mode 100644 index 0000000..eb2aa4c --- /dev/null +++ b/plugins/specflow2/skills/specflow-refine/lenses/data-lifecycle.md @@ -0,0 +1,63 @@ +# Lens: data lifecycle + +Simulate building this system for **the second year of its operation**, not the +first day. Specs describe creation; they rarely describe what happens to data +afterwards. + +Work through the spec asking: + +- For each entity: who creates it, who may change it, and what ends its life? + Is it deleted, archived, anonymised, or kept forever? "Forever" is a valid + answer only if someone chose it. +- What happens to records that reference a deleted record? Cascade, orphan, + refuse the delete, or soft-delete the parent? Every foreign key is one of + these decisions. +- Which fields are historical and must not change retroactively (the price at + time of purchase) versus current (today's price)? Mutating a field that + something historical points at is a common, quiet data bug. +- How does existing data get to the new shape? If the spec changes an entity, + what happens to rows written under the old rules — backfilled, defaulted, or + left mixed? +- Is anything unique, and over what window? Unique forever, or unique among + active records? Reusing an identifier after deletion is a decision. +- What is the retention obligation? If the spec mentions personal data at all, + deletion and export are requirements, not features. + +## The matrix to fill + +Name the axes before you answer anything, then put something in every cell. + +- **rows** — every entity the spec names, including the ones it mentions only in + passing as a field on something else. +- **cols** — the events that reach it after creation: *updated*, *the thing it + references is deleted*, *it is deleted while referenced*, *retention window + expires*, *a subject asks for export or erasure*, *restored from a backup taken + before a schema change*. + +Each cell says what happens to the data. "Kept forever" is a valid answer only +where someone chose it; if you are inferring it from silence, that is a guess. + +A cell you cannot answer is the finding. Write `unanswerable` with the reason, for +example *the spec sets no retention period for this entity and it carries personal +data, so the expiry column has no answer at all*. + +## What counts as a blocker here + +The spec is missing a decision wherever an entity has no defined end of life, or +a reference has no defined behaviour when its target disappears. These surface +in production months after launch, which is exactly why simulating the build +catches them and reading the happy path does not. + +## Decisions to record + +Write these into `decisions` even where the spec is silent — that is what makes +another lens's different answer visible. Mark a guess `guessed: true`. + +- For every relationship: what happens to the children when the parent is + deleted? +- For every computed value: is it recalculated or frozen, and does anything + depend on it staying historically stable? +- What is the retention period, and what does deletion actually mean — removed, + or flagged? +- Are there delete and export paths at all? Include them even when the spec omits + them; their absence is the finding. diff --git a/plugins/specflow2/skills/specflow-refine/lenses/idempotency.md b/plugins/specflow2/skills/specflow-refine/lenses/idempotency.md new file mode 100644 index 0000000..952fd1e --- /dev/null +++ b/plugins/specflow2/skills/specflow-refine/lenses/idempotency.md @@ -0,0 +1,57 @@ +# Lens: idempotency and replay + +Simulate building this system on the assumption that **every message arrives at +least once, and sometimes more than once**. Networks retry, users double-click, +queues redeliver, and clients resend after a timeout they could not interpret. + +Work through the spec asking: + +- For each operation: what happens if it runs twice with identical input? Twice + is the minimum — assume it can run five times. +- Where does the caller supply an idempotency key, and where must the system + derive one? If neither, the operation is not safe to retry, and the spec + should say retries are forbidden. +- Which effects are not naturally idempotent — charging a card, sending an + email, incrementing a counter, appending to a log? Each needs an explicit + dedup story. +- How long is a duplicate recognised as a duplicate? A dedup window is a + decision with a number in it; if the spec has no number, that is the gap. +- What does the second caller receive — the original result, a conflict error, + or a fresh execution? Returning the original result requires storing it. +- Is the *response* replayable? A caller that retried because it lost the + response needs the same answer, not a "already done" error it cannot act on. + +## The matrix to fill + +Name the axes before you answer anything, then put something in every cell. + +- **rows** — every operation the spec describes that changes something. Reads too, + where a read has a side effect such as issuing a token or consuming a quota. +- **cols** — the replay conditions: *same input twice*, *same key with a different + payload*, *retried after a timeout where the caller cannot know if it landed*, + *replayed after a later operation already happened*. + +Each cell says what the second and fifth attempt do — not just the second. Assume +five. + +A cell you cannot answer is the finding. Write `unanswerable` with the reason, for +example *the spec names no key for this operation and no natural one exists in its +input, so nothing can tell a retry from a new request*. + +## What counts as a blocker here + +The spec is missing a decision for every operation whose second execution is +observably different from its first and which the spec does not mark as +non-retryable. This class of defect is invisible in a happy-path read and +routinely reaches production. + +## Decisions to record + +Write these into `decisions` even where the spec is silent — that is what makes +another lens's different answer visible. Mark a guess `guessed: true`. + +- For every operation: is calling it twice with the same input safe, and if so + what makes it safe — a key, a version, a natural uniqueness? +- What happens when an event arrives against a state that already consumed it? +- On a retry that succeeded the first time invisibly, what does the caller see? +- Which side effects are not replayable — mail, payment, external calls? diff --git a/plugins/specflow2/skills/specflow-refine/lenses/ordering.md b/plugins/specflow2/skills/specflow-refine/lenses/ordering.md new file mode 100644 index 0000000..4406d69 --- /dev/null +++ b/plugins/specflow2/skills/specflow-refine/lenses/ordering.md @@ -0,0 +1,58 @@ +# Lens: ordering and sequence + +Simulate building this system on the assumption that **events do not arrive in +the order they happened**. Specs are written as narratives, so they inherit an +implied sequence that nothing enforces. + +Work through the spec asking: + +- Which parts of the spec read as "first this, then that"? For each, what + actually guarantees the order — a transaction, a queue with ordering + guarantees, a timestamp, or nothing? +- What happens if a later event arrives before an earlier one? A cancellation + before the booking it cancels; an update for a record not yet created; a + payment for an order that has not been placed. +- Where are timestamps used to order things? Whose clock produced them? Two + events from different machines can carry impossible relative times. +- Which operations assume a prior operation completed? Is the precondition + checked, or assumed? An unchecked precondition is a decision to trust the + caller. +- If an event arrives that is no longer relevant (superseded, stale, for a + deleted record), is it dropped, queued, or an error? Silence is a choice. +- For anything batched or scheduled: what happens when a run overlaps the + previous one because it took longer than the interval? + +## The matrix to fill + +Name the axes before you answer anything, then put something in every cell. + +- **rows** — every event, message or state change the spec names. If the spec + reads "first this, then that", both halves are rows. +- **cols** — the ways it can arrive out of order: *before the event it depends + on*, *twice, second copy late*, *after a terminal state was reached*, *never*. + +Each cell says what the system does. The narrative order the spec implies is not +an answer — the cell asks what happens when that order does not hold. + +A cell you cannot answer is the finding. Write `unanswerable` with the reason, +for example *nothing in the spec establishes an order between these two, so both +answers are equally supported*. + +## What counts as a blocker here + +The spec is missing a decision wherever it implies a sequence without stating a +mechanism that enforces it, and wherever an out-of-order arrival has no defined +handling. "Events are processed in order" needs to name what provides that +guarantee, or it is an assumption rather than a requirement. + +## Decisions to record + +Write these into `decisions` even where the spec is silent — that is what makes +another lens's different answer visible. Mark a guess `guessed: true`. + +- For every event that can arrive early: what happens if it does? +- Where does an input reference something that may not exist yet? +- What is the ordering guarantee the spec assumes, and what provides it? +- If the build order matters and the spec does not imply one, what order would + you pick? Your sequencing is a hypothesis, and another lens choosing + differently is a signal in itself. diff --git a/plugins/specflow2/skills/specflow-refine/lenses/partial-failure.md b/plugins/specflow2/skills/specflow-refine/lenses/partial-failure.md new file mode 100644 index 0000000..853410d --- /dev/null +++ b/plugins/specflow2/skills/specflow-refine/lenses/partial-failure.md @@ -0,0 +1,60 @@ +# Lens: partial failure + +Simulate building this system on the assumption that **anything can fail halfway +through, including the thing recording the failure**. + +Every operation that touches more than one place — two tables, a table and a +queue, a database and a payment provider — can complete some parts and not +others. Work through the spec asking: + +- For each multi-step operation, what is the state of the world if it stops + after step 1? After step 2? Is that state one the system can recognise and + recover from, or is it silently inconsistent? +- Which external calls can succeed while the caller believes they failed + (timeout after the remote side committed)? What does the spec say to do when + you cannot tell whether the money moved? +- What compensates a completed step when a later step fails? Who runs the + compensation, and what if the compensation itself fails? +- Which failures are retried, how many times, and with what backoff? Which are + terminal? Retrying a non-idempotent operation is a decision, not a detail. +- What does the user see mid-failure? A spinner that never resolves is a + specified behaviour if nobody chose otherwise. + +## The matrix to fill + +Name the axes before you answer anything, then put something in every cell. + +- **rows** — every operation that touches more than one place: two tables, a table + and a queue, a database and an external provider. +- **cols** — where it stopped: *after the first write*, *after the external call + but before recording it*, *after the external call succeeded but the response was + lost*, *while writing the failure record itself*. + +Each cell describes the state of the world and answers one thing: **can the system +recognise it later, and recover?** A state nothing can detect is worse than a +crash. + +A cell you cannot answer is the finding. Write `unanswerable` with the reason, for +example *the spec describes no record of this step having started, so this state is +indistinguishable from never having been attempted*. + +## What counts as a blocker here + +The spec is missing a decision wherever an operation can leave the system in a +state the spec never names. If a state is reachable and unnamed, no +implementer can handle it consistently — two builds will handle it two ways. + +"Roll back the transaction" only closes this when every step is inside the same +transaction. Say so explicitly, or treat it as open. + +## Decisions to record + +Write these into `decisions` even where the spec is silent — that is what makes +another lens's different answer visible. Mark a guess `guessed: true`. + +- For every reachable bad state: what does the spec say happens? Record "nothing" + honestly when it says nothing — that is the finding, and it deserves a blocker. +- What intermediate states do real failures create (`pending_confirmation`, + `partially_applied`) that the spec never names? +- Who cleans up a half-finished operation, and when? +- What does the caller see, and what can they safely retry? diff --git a/plugins/specflow2/skills/specflow-resolve/SKILL.md b/plugins/specflow2/skills/specflow-resolve/SKILL.md new file mode 100644 index 0000000..e1d2a21 --- /dev/null +++ b/plugins/specflow2/skills/specflow-resolve/SKILL.md @@ -0,0 +1,113 @@ +--- +name: specflow-resolve +description: Walk through open specification decisions from a refinement round, write answers back into the spec files, and record exact blocker IDs for traceability. +argument-hint: "(optional) spec_dir outputs_dir — defaults: specs docs" +--- + +# SpecFlow Resolve + +Take the open decisions from a refinement round, settle them with the user, and +**write the answers into the specification**. + +That last part is the job. A loop that only reports blockers leaves all the work +with the user; the value is a spec that no longer has the hole. + +## What to do + +### 1. Read the open decisions + +```bash +specflow refine status --outputs --json +``` + +That gives you the open blockers with attribution — which lenses raised each, and +where the readings disagreed. If the list is empty, say so and stop; do not +manufacture questions. + +**Sorting these is your judgment, not the CLI's.** There is no ranking to defer +to, on purpose: a weighted score would have been invented numbers dressed up as +measurement. Decide it yourself, using: + +- how expensive a wrong guess is to undo — architecture, data model, and security + boundaries are the costly ones, +- whether the lenses disagreed, which is evidence the spec genuinely leaves it + open rather than one agent being pedantic, +- how many independent lenses raised it. + +Say your reasoning out loud when you present the list, so the user can push back +on your ordering rather than trusting it. + +### 2. Handle the cheap ones without asking + +Where a choice is genuinely reversible and low-impact, apply the recommendation, +record it, and mention it in your summary as a batch. Do not put these to the +user one by one. + +```bash +specflow refine resolve --outputs --id --choice "