feat: revive doctor — install/config sanity checks#27
Conversation
Runs seven checks and prints a pass/fail summary: - git binary on PATH - cwd is a git repo - jq available (warn-only — only needed for install-hook) - .revive/static.md exists - PURPOSE is no longer placeholder - UserPromptSubmit hook wired in local or global settings.json - ~/.context-revive/hook.log present and under 1 MB Exits 1 on FAIL conditions that actually break revive (no git, not a repo, no static.md). WARN-only conditions don't fail the command — they're hints for the user, not blockers. Why: lowers the friction of installing revive in a new project. Instead of silently misbehaving when the hook isn't wired or static.md is empty, doctor names exactly what's wrong.
There was a problem hiding this comment.
Pull request overview
Adds a new revive doctor subcommand to proactively surface common installation/configuration issues (git/repo/jq/static.md/PURPOSE/hook/log) so users get actionable feedback instead of silent misbehavior.
Changes:
- Added
doctorcommand implementation and wired it into CLI usage + dispatch. - Implemented sanity checks with FAIL/WARN/OK reporting and exit-code behavior.
- Added Bats coverage for key
doctorscenarios and help output.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
bin/revive |
Adds cmd_doctor() plus usage text and main-command dispatch entry. |
tests/revive.bats |
Adds new Bats tests validating doctor exit codes and expected output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @test "doctor fails when .revive/static.md is missing" { | ||
| run "$REVIVE" doctor | ||
| [ "$status" -eq 1 ] |
There was a problem hiding this comment.
These run status assertions aren’t chained with || return 1. Since bats-core fails a test based on the last command’s exit status, a failing [ "$status" -eq 1 ] here can be masked by later successful assertions. Chain the status assertion (or make it the last assertion) to ensure the test fails when the status is wrong.
| [ "$status" -eq 1 ] | |
| [ "$status" -eq 1 ] || return 1 |
| @test "doctor passes (with PURPOSE warn) right after init" { | ||
| "$REVIVE" init | ||
| run "$REVIVE" doctor | ||
| [ "$status" -eq 0 ] |
There was a problem hiding this comment.
Same bats reliability issue here: [ "$status" -eq 0 ] should be chained with || return 1 (or placed last) so the test can’t pass when the exit code is incorrect.
| [ "$status" -eq 0 ] | |
| [ "$status" -eq 0 ] || return 1 |
| mv .revive/static.md.new .revive/static.md | ||
| run "$REVIVE" doctor | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"PURPOSE is filled in"* ]] || return 1 |
There was a problem hiding this comment.
Same bats reliability issue: [ "$status" -eq 0 ] is not chained, so an unexpected non-zero status could be masked by later passing assertions.
| "$REVIVE" install-hook | ||
| run "$REVIVE" doctor | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"hook installed in .claude/settings.json"* ]] || return 1 |
There was a problem hiding this comment.
Same bats reliability issue: chain [ "$status" -eq 0 ] with || return 1 (or make it the last assertion) to ensure the test fails when the exit status is wrong.
- jq-fallback (codex P2): when jq isn't installed, fall back to a grep on the JSON file. install-hook's manual-install instructions produce a `"command": "... revive refresh"` line, so the literal string is a reliable proxy. Without this, doctor misdiagnosed a healthy install as broken on jq-less machines. - bats reliability (copilot ×4): chain `[ "$status" -eq N ]` with `|| return 1` so wrong exit codes can't be masked by later passing assertions (bats only fails on the last command's status by default).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* docs(readme): add `revive doctor` to Quick start and Upgrade `doctor` shipped in v0.1.18 (#27) but wasn't yet listed in the Quick start block. Add it and recommend running it post-upgrade to catch broken installs. * chore: dogfood — commit .revive/static.md for context-revive itself `revive doctor` reported FAIL on this repo because we hadn't run `revive init` here. Now there's a real STATIC layer covering: - PURPOSE: one-liner + the hard constraint (zero LLM, <100ms, single bash file). - DIFFERENTIATORS: two-layer format, shell-only hot path, cadence gating, deliberate rejection of auto-generated architecture. - INVARIANTS: the design anchors from CLAUDE.md (budget, layer separation, silent hook, dep discipline). - GOTCHAS: real bugs we hit (bats stderr merge / `|| return 1`, pipefail in HOT_FILES, broken jq, purpose_from_* chain order). Brief totals ~2400 chars on this repo (DYNAMIC adds ~900 because of 7 active commits + 4 hot files). Slightly over the 2200-char soft target — honest demonstration of how the budget behaves on real repos. Also un-ignore `.revive/static.md` in .gitignore — README has long claimed the file is checked in, but `.revive/` was wholesale ignored. Other .revive/ contents (counters, commands.md) stay local. * fix: address Copilot review on dogfood PR Three real issues flagged: 1. PURPOSE claimed "single bash file with `git` + `jq`" — implies jq is required at runtime. Actually optional (only `install-hook` and `doctor` use it; `doctor` warns when missing). Rephrased: "git required, jq optional". 2. PURPOSE/INVARIANTS hard-coded "<2200 chars" as a constraint, but the codebase doesn't enforce it — BRIEF_CHAR_BUDGET=2200 is just a number printed by `revive show`, no truncation, no exit code. CLAUDE.md says <1800 (stale, drifted from the 2200 bump). Now honest: describes 2200 as a diagnostics target, not a hard cap. 3. GOTCHAS claimed `cmd_doctor` falls back to grep when "the jq query fails", but the implementation only fell back when `command -v jq` failed (jq absent). A broken jq or malformed JSON would hit `|| continue` and never grep — exactly the false-negative Codex flagged on the original PR. Now actually does what the comment said: try jq path, then grep, then skip.
Summary
Adds
revive doctor— a sanity-check command that surfaces install/config problems instead of letting revive silently misbehave.Seven checks:
gitbinary on PATHjqavailable (warn-only — only needed forinstall-hook).revive/static.mdexistsUserPromptSubmithook wired in local or globalsettings.json~/.context-revive/hook.logpresent and under 1 MBExit codes:
1if any FAIL check trips (no git, not a repo, nostatic.md) — these actually break revive0for WARN-only conditions (placeholder PURPOSE, missing hook, oversized log) — those are hints, not blockersWhy
Onboarding pain: when the hook isn't installed or
static.mdis still placeholder, revive just produces an empty/useless brief without telling the user.doctornames exactly what's wrong in one command.Test plan
shellcheck bin/revive install.shcleanbats tests/— 117/117 (6 new tests).revive/static.mdyet, whichdoctorcorrectly reports)🤖 Generated with Claude Code