feat(recce-devloop): review a working tree against a local base

[wcchang1115]

Background

/recce-dev-review only compared against a base held in Recce Cloud. A developer with their own target-base/ and no Cloud account got no review at all: every path led to the Cloud setup questions, and declining ended at "there is no review to give".

Change

A second path through the same skill, taken only when the project has base artifacts and recce-cloud is not installed. The other combinations reach the existing Cloud flow untouched.

Checks created on this path are saved to target/recce_state.json, so recce server can open them after the session ends.

Non-obvious decisions

Both signals must agree before going local. A Cloud user who once built a target-base/ would otherwise be reviewed against a months-old base, with nothing saying so.

The local path checks the server mode instead of setting it. set_backend(mode="local") drops the server's state loader, so checks after that call are created in memory and lost with no error. That is a bug in recce, worked around here.

Cloud users now get a target/recce_state.json, about 2 MB with no checks in it. recce writes it when swapping to cloud, now that a state file path is passed at startup. Gitignored and harmless, but new.

🤖 Generated with Claude Code

[iamcxa]

@wcchang1115 Reviewed at 936699d against base e240e4d. The design reasoning in the PR body holds up — I checked the non-obvious claims against recce main and they are all accurate:

• recce mcp-server <state_file> is a real positional argument (recce/cli.py:1764).
• get_server_info does return mode and single_env in local mode (mcp_server.py:2539-2543).
• set_backend(mode="local") does rebuild the context without a state loader, so export_persistent_state() finds default_context().state_loader is None and silently no-ops. Not calling it is the right call.
• Local create_check replaces on a matching (type, params) (mcp_server.py:2820-2832) while cloud appends — Section 0's table is correct.
• FileIO.write is a bare open(path, "w"), so the mkdir -p target comment is accurate about parent directories.

The full test suite passes (90/90). What blocks this is not the reasoning but two places where the new guard rails do not reach: one dead-ends the user, and one is a regression for people who never touch the new path. Details inline.

Advisory (not blocking — no action required to merge)

#	File	Note
A	plugins/recce-devloop/.claude-plugin/plugin.json, .claude-plugin/marketplace.json	Both descriptions still read "against the team's Recce Cloud base". That is the text a developer without a Cloud account reads when deciding whether to install the plugin this PR just made useful to them. CLAUDE.md's plugin table says the same.
B	skills/recce-dev-review/SKILL.md L2	L2 restates Step 3's ARTIFACTS table and wording line verbatim, where L3/L4/L6 all use the lighter "Do Step N, with these changes" form. A future fix to one table will not reach the other.
C	skills/recce-dev-review/SKILL.md Step 6 point 4	The justification for clearing the tracked-change record is cloud-only prose ("the session under review is an upload of this working tree"). Nothing is uploaded on the local journey; the local reason is that L2 already verified target/ is fresh. The action is still correct, so this is wording only.
D	tests/	Nothing covers check-preflight.sh's new BASE= output or the BASE/RECCE_CLOUD routing table. The suite currently exercises findings records and reviewer tool declarations only.

Verification summary

Check	Result
Test suite (pytest tests/)	90/90 pass
bash -n on both changed shell scripts	clean
Claims verified against recce main	5/5 accurate (listed above)
Security scan	no findings — no new dependencies, no workflow changes, no credentials, no user input reaching a shell command

Caveat on finding 1 and 2

Both rest on recce main as it stands today. set_backend is not yet on PyPI (latest is 1.41.0), so if you are developing against a newer recce branch where single_env is no longer frozen at startup, finding 1 would not hold — please say so and I will withdraw it. Finding 3 does not depend on any recce version.

[iamcxa]

HIGH — this remedy can never succeed, so the user loops forever.

Consider when this branch is actually reachable. To arrive at L1 at all, Step 1's routing table had to see BASE=present, and check-preflight.sh only prints that when both target-base/manifest.json and target-base/catalog.json exist on disk. So by the time this line runs, the artifacts are already there.

So why would single_env still be true? Because recce freezes the flag at server startup:

# recce/cli.py, mcp_server()
if not target_base_path.is_dir():
    kwargs["single_env"] = True

After that, self.single_env only ever changes inside _tool_set_backend — which L1 explicitly forbids calling, for good reasons stated two paragraphs up.

That leaves exactly one reachable scenario: the developer built target-base/ after the Claude Code session started. Running dbt docs generate --target-path target-base again cannot move a flag that was frozen at startup, so /recce-dev-review hits the identical single_env: true on the next run, and the one after that. There is no exit.

The fix is already written three lines above, in the mode=cloud branch — a session restart is what actually reloads the flag. Something like:

Your target-base/ artifacts were built after this session started, so Recce has not loaded them. Start a new Claude Code session and run /recce-dev-review again.

Worth noting this is the only case that reaches here, so the message can state the cause with confidence rather than hedging.

[iamcxa]

MEDIUM — the stale-base hazard the PR body names is left open for exactly the users this PR serves.

The PR body reasons that "a stale local base gives a confident wrong review", and builds RECCE_CLOUD=missing into the routing to prevent it. But that guard only protects developers who have a Cloud account. The developer this PR exists for — own target-base/, no Cloud — gets no staleness check at all:

• L1 reads single_env and stops there. recce's local get_server_info returns more than that:

  # recce/mcp_server.py:2546-2552
  # "fresh" | "stale_time" | "stale_sha" | "missing" | "single_env" | "unknown"
  result["base_status"] = getattr(self, "_base_status", "unknown")

• L2 does not cover it either: check-artifacts.py defaults to --target-path target, so it compares target/ against the working tree and never looks at target-base/.

So a target-base/ built a month ago passes both gates, and the review comes back Data status: measured with nothing saying the baseline is old.

The signal is right there in the same get_server_info response, and this repo's sibling plugin already consumes it — plugins/recce/skills/recce-verify/SKILL.md:26-30 treats stale_time / stale_sha / unknown as a base that cannot be trusted. Reusing that vocabulary here would close the gap and keep the two plugins consistent.

If you would rather land this as a follow-up, that is a reasonable call — I would just ask that it not disappear silently, since the PR body is what raised the hazard in the first place.

[iamcxa]

MEDIUM — this is a regression that also reaches people who never use the new local path.

.mcp.json runs this script at every session start for anyone with recce-devloop installed, with cwd inherited from Claude Code, and before anything has established that cwd is a dbt project. Two consequences:

1. Directory pollution. With recce on PATH, opening Claude Code in any unrelated repository now leaves an empty target/ behind.

2. The one that actually hurts. This script runs under set -euo pipefail (line 8). If cwd is not writable, or a file named target already exists, mkdir fails, set -e aborts the script, and the Recce MCP server never starts — taking the whole plugin down. Before this PR it launched fine in those conditions, and the cloud journey does not need the state file at all.

The sibling launcher is still a clean exec recce mcp-server (plugins/recce/scripts/run-mcp-stdio.sh:27), which is the behaviour being diverged from here.

A guard plus a non-fatal mkdir keeps the new capability without the blast radius:

# Only a dbt project can have a local base worth persisting checks against,
# and a failure here must not take the MCP server down with it.
if [ -f dbt_project.yml ]; then
    mkdir -p target || true
fi

If target/ is genuinely absent, create_check fails on its own with a real error — which is a better outcome than no MCP server at all.