fix: harden prep/auto/implement against missing companion extensions

[iddocohen]

Refs #16.

Problem

A missing companion extension degraded silently. prep went straight to "invoke the speckit-critique-run skill" and implement Phase 6 straight to "invoke the speckit-review-run skill", with no check that either was installed.

When spec-kit 1.0.x started rejecting the review extension's manifest (#16), the effect was not a loud failure. auto kept completing and reporting success with the review pass simply absent. An autonomous run that skipped a quality gate looked exactly like one that passed it.

Approach

Both phases resolve a provider before using it, by checking the filesystem rather than asking the agent whether it "has" a skill. Agents cannot reliably enumerate their own skills, and improvising around a skill that was never installed is precisely the failure being fixed.

Mode	Condition
extension	companion extension installed (.specify/extensions/<id>/, a speckit.<id>.run entry in .specify/extensions.yml, or the harness skill dir)
fallback	not installed, but clean-context subagents are available
none	not installed and no subagent dispatch either

In fallback mode the phase dispatches one clean-context subagent with an embedded brief (the six review aspects, or the dual-lens critique) and a fixed return shape, so the downstream fix-and-report logic never branches on provider. It is a safety net with less depth than the extension, and the report is required to say so rather than present it as an equivalent pass.

In none mode the gate is explicitly skipped and REVIEW: none marks the run INCOMPLETE, mirroring the existing local-pass-evidence blocking rule.

A missing reviewer is deliberately not a Phase 0 stop-condition. The existing aborts are there because a dirty tree or missing prep artifacts contaminate the HEAD@start..HEAD@now diff. A missing reviewer corrupts nothing, it only lowers assurance, so aborting would trade a silent gap for a hard outage.

Status-line contract

One field each, which auto parses and must surface in its summary:

prep:      STATUS: <READY|BLOCKED>          | SPEC_DIR: <path> | CRITIQUE: <extension|fallback|none|n/a> | REASON: <...>
implement: STATUS: <DONE|INCOMPLETE|BLOCKED> | SPEC_DIR: <path> | REVIEW: <extension|fallback|none|n/a>   | REASON: <...>

Existing fields keep their order and meaning. Anything outside this repo reading these lines positionally would need updating, which is why the CHANGELOG flags it. Both producer and consumer live in this repo and are updated together.

auto's Completion now has to state both values, leading with them when either gate was degraded, so a degraded run is visible at top level and not only in a report file the user may never open.

Changes

• commands/implement.md — Phase 0 provider resolution, Phase 6 split into 6a/6b/6c, Phase 7 §5 Review mode: line, §6 must record a non-extension mode, new review blocking rule, status line
• commands/prep.md — Phase 3 provider resolution and 3a/3b/3c, Completion summary and status line
• commands/auto.md — parses both new fields, required coverage statement in Completion
• README.md — companions documented as soft dependencies, plus a Troubleshooting section for the upstream review install failure on spec-kit 1.0.x and the REVIEW: none state
• CHANGELOG.md — entries under [Unreleased], including the contract change

extension.version is left at the unreleased 1.2.0. Happy to bump to 1.3.0 if you would rather the status-line change carry its own version.

Verification

No test framework in this repo, so these are command-file changes verified against a real CLI. On specify-cli 1.0.5:

• The extension installs cleanly with the modified command files
• All three modified command frontmatters parse and carry description
• Producer and consumer status-line contracts match field for field (CRITIQUE in prep vs auto, REVIEW in implement vs auto) and the mode vocabulary is identical across all three files
• The detection predicates discriminate correctly: in a project with the review extension installed, all 3 review signals present and all 3 critique signals absent; in a project with neither companion, 0 of 3 for both, which is the fallback path

Note on scope

This hardens our side. It does not fix the upstream manifest bug that caused #16, which is tracked at ismaelJimenez/spec-kit-review#4 with a fix in PR #5. Once upstream tags v1.0.2 the README pin should move to it, and this fallback goes back to being what it should be: a net that rarely catches anything.

[saltas888]

The framing makes sense to me - the real bug is that a skipped gate looked identical to a passed one, not the upstream manifest thing. Resolving the provider up front and not letting none report DONE both look right.

Two things I'd want sorted before merge: the detection check in implement.md isn't actually a check, and the README still leads with the install command that fails. Rest is minor.

No CI here and the verification is all self-reported, so the detection logic is the one part I can't re-run myself.

[saltas888]

There's no actual check here, just a description of one, so the agent has to invent the filesystem test itself. Can we put the literal command in the file?

Also .claude/skills/ only exists on Claude Code, and I thought .specify/extensions.yml was our own hook wiring rather than something the installer writes. If that's right then 2 of the 3 signals never fire and you always get the fallback.

[saltas888]

What happens if we resolve to extension and then the skill isn't actually there? No way back to 6b from here. Doesn't happen with #16 since the install bails before writing anything, but a half-installed extension would land here and go quiet, which is the thing we're trying to stop.

[saltas888]

This looks left over from the split, 3a and 3b both already say to run the critique. Move it above 3a or drop it.

[saltas888]

Fine with REVIEW: none blocking and CRITIQUE: none not blocking, but can the reason go in the file? It's only in the PR description right now and that's the bit nobody reads later.

[saltas888]

n/a doesn't really mean the extension was unavailable, it means we bailed before phase 3. Maybe "anything other than extension or n/a".

[saltas888]

The install command above this still fails though. Can we just put the working one there until upstream tags v1.0.2? Or at least move this note above it so people read it before running it.

[saltas888]

The new field goes in before REASON, so anything reading these positionally breaks. That's a major bump really, not 1.2.0 or 1.3.0. Probably nothing outside this repo reads them, but can you check infrahub-speckit doesn't before we settle it?

[iddocohen]

All seven fixed in 6bce0a1: the detection check is now a literal shell block keying off .specify/extensions/<id>/ and .specify/extensions/.registry (both installer-written, neither harness-specific), and 6a/3a downgrade to the fallback branch when the skill invocation itself fails, so a half-installed extension can no longer report extension for a pass that never ran. You were right about the other two signals: .claude/skills/ is Claude Code only and extensions.yml carries bare ids rather than speckit.<id>.run entries, so both are gone.

On versioning, agreed it is major, so 1.2.0 is now 2.0.0. The new field goes in ahead of REASON, which moves an existing field rather than appending one, and that breaks any positional reader regardless of how few there are. I checked infrahub-speckit before settling it: it ships three route commands and no status-line parsing at all. An org-wide code search for SPEC_DIR returns only vendored copies of this extension, so auto here is the only parser and it is updated in the same commit. That audit is written into the CHANGELOG entry rather than left in this thread.

[saltas888]

I'd agree with @BeArchiTek, lets do 1.3