fix(cli): emit one JSON document from vera uninstall - #128

Part of #99

Fixes the first half only. The vera doctor exit code is the other half and is left alone deliberately: it is a behaviour change for anyone already running vera doctor in a pipeline, and the issue asks for a maintainer call on the direction. Happy to send it once you pick one.

The bug

vera uninstall --json printed two JSON documents on stdout:

$ vera uninstall --json | python3 -c "import json,sys; json.load(sys.stdin)"
json.decoder.JSONDecodeError: Extra data

commands/uninstall.rs:49-54 delegated skill removal to agent::run(Remove, All, All, json_output), which reaches do_remove and prints serde_json::to_string_pretty(&reports), an array. commands/uninstall.rs:90-94 then printed a second object, {"uninstalled": true, "removed": [...]}.

The array was never empty. resolve_locations(All, All) yields every supported client crossed with both scopes regardless of what is installed, and do_remove pushed a report per location unconditionally while only deleting the directory when SKILL.md was present.

That same unconditional push made the human output wrong. On a machine with zero skills installed, vera uninstall printed Removed Vera skill from: followed by a row for each of the 62 client/scope pairs, none of which were removed.

The fix

Split the filesystem work out of the printing. remove_skill_locations (crates/vera-cli/src/commands/agent.rs:595) deletes each location and returns a report per location; do_remove (crates/vera-cli/src/commands/agent.rs:556) is now that call plus its output, unchanged in behaviour for vera agent remove. remove_all_skills (crates/vera-cli/src/commands/agent.rs:648) is the silent entry point uninstall uses, so nothing prints on that path and uninstall folds the result into its own single document.

Record whether a location was actually removed. SkillLocationReport gains removed: Option<bool> (crates/vera-cli/src/commands/agent.rs:177), serialized only on the removal path, next to the existing up_to_date: Option<bool> and following the same convention. installed was already post-state and stays false; it could not answer "did anything happen here", which is what both renderers needed.

I went with the flag rather than dropping non-removed locations from the reports, because the two consumers want different things. vera agent remove --json is often called with an explicit --client/--scope, and reporting [] there loses the fact that the location was checked and found empty; the flag keeps that. vera uninstall --json and both human renderers filter on it, so they only ever name real removals. Dropping the entries would have been simpler but would have made vera agent remove --json silently indistinguishable between "not installed" and "not looked at".

Human output. write_removed_skill_locations (crates/vera-cli/src/commands/agent.rs:653) prints only removed == Some(true) rows, and falls back to No Vera skill installations found. when there are none, matching the message remove_interactive already used for the same situation. Both vera uninstall and vera agent remove render through it, so the JSON and the human output cannot disagree.

Truthful categories. uninstall's removed array used to list ["agent skills", "vera data dir", "PATH shim"] unconditionally, including when the data directory did not exist and no shim was found. Each entry is now pushed only when that step removed something (crates/vera-cli/src/commands/uninstall.rs:90, :99, :128). The document also carries skills, the paths actually deleted, which is the information the second document used to carry and which no parser could reach.

Testability. uninstall::run_at (crates/vera-cli/src/commands/uninstall.rs:65) takes the home, data directory, cwd, and shim directory as arguments and writes to injected sinks; run resolves them and passes real stdout/stderr. VERA_USER_BIN_DIR is now read once in run rather than deep inside shim_candidates, so a test can point every root at a temp tree and no test can reach a real skill install or a real shim.

Streams are unchanged: the skill listing goes to stdout as before, uninstall's own progress lines and closing notes to stderr.

After

$ vera uninstall --json
{"removed":["agent skills","vera data dir"],"skills":["/Users/me/.claude/skills/vera"],"uninstalled":true}

$ vera uninstall            # nothing installed
No Vera skill installations found.

Vera has been uninstalled.
Per-project indexes (.vera/ in each project) were not removed.

Tests

Eight tests in crates/vera-cli/src/commands/uninstall.rs, all driven against a tempdir() tree with no environment variables involved:

• uninstall_json_emits_exactly_one_document parses stdout with serde_json::from_str, the strict parse that json.load performs, and asserts the removed skill path is in the document.
• uninstall_json_claims_only_categories_that_were_removed asserts removed and skills are both empty when nothing was installed.
• uninstall_human_output_lists_only_removed_locations asserts the one installed path is listed, a path for a client that was not installed is not, and stdout is exactly three lines.
• uninstall_human_output_reports_nothing_when_no_skills_are_installed asserts the fallback message.

Three more cover the partial-failure path added in cbe424a, each with a fixture where an earlier location is deleted and a later one cannot be:

• uninstall_json_reports_skills_removed_before_a_later_removal_failed asserts the deleted path is in skills and agent skills is in removed.
• uninstall_human_output_names_skills_removed_before_a_later_removal_failed asserts the deleted path is named in the human output.
• uninstall_human_output_does_not_claim_nothing_was_installed_when_removal_failed asserts the fallback message is absent when the only location found could not be deleted.

All three also assert the earlier directory really was deleted, so the fixture cannot pass vacuously.

One more covers the inspection side, added in a18b7b6:

• uninstall_does_not_report_an_uninspectable_skill_as_absent installs a skill whose own directory is 0o000, so SKILL.md cannot be stat'd, and asserts the run neither claims nothing was installed nor stays silent about the reason. It also asserts the skill really did survive, so the fixture cannot pass vacuously.

Verified by reinjection

Restoring the two original behaviours (report every checked location, and print do_remove's array before the final object) fails all four of the first group with the production errors:

uninstall_json_emits_exactly_one_document
  stdout is not a single JSON document (trailing characters at line 437 column 1)

uninstall_json_claims_only_categories_that_were_removed
  Error("trailing characters", line: 437, column: 1)

uninstall_human_output_lists_only_removed_locations
  Removed Vera skill from:

    agents         global  /var/folders/.../home/.config/agents/skills/vera
    agents         project /var/folders/.../project/.agents/skills/vera
    amp            global  /var/folders/.../home/.config/agents/skills/vera
    ...  (62 rows, one installed)

uninstall_human_output_reports_nothing_when_no_skills_are_installed
   left: "Removed Vera skill from:\n\n  agents  global  ...(62 rows)"
  right: "No Vera skill installations found."

trailing characters is serde_json's wording for the same condition Python reports as Extra data.

The partial-failure half was reinjected separately, one half of the fix at a time:

• Restoring the ? on fs::remove_dir_all, so a failure propagates and run_at substitutes the default, fails all three of the second group. The JSON test reports the exact production document, {"removed":[],"skills":[],"uninstalled":true}, against a right-hand side naming the Claude skill path that really had been deleted.
• Restoring the unguarded if removed.is_empty() fallback fails exactly one, uninstall_human_output_does_not_claim_nothing_was_installed_when_removal_failed, which is the test whose fixture has an empty removal set and a non-empty failure set.
• Restoring Path::exists in place of try_exists fails exactly one, uninstall_does_not_report_an_uninspectable_skill_as_absent, with the production message: claimed nothing was installed while .../.claude/skills/vera was still on disk: No Vera skill installations found.

Verification

• cargo fmt --check clean
• cargo test -p vera-cli --bin vera 105 passed, 0 failed
• cargo test -p vera-core --lib 793 passed, 0 failed
• cargo clippy -p vera-core --lib 5 warnings, the pre-existing ones, none added

Summary by CodeRabbit

• New Features

  • Uninstall results now identify which skills and shims were actually removed.
  • JSON output includes removed skill locations.
  • Human-readable output clearly reports removed skills and shims.
  • Bulk skill removal supports reusable reporting and output handling.

• Bug Fixes

  • Missing skill locations are distinguished from successfully removed installations.
  • Skill removal failures are reported without stopping the rest of the uninstall process.

Summary by cubic

Emits a single JSON document from vera uninstall --json and reports only actual removals. It preserves earlier successes when a later location fails, and records inspection/deletion errors instead of claiming skills were absent.

• Outputs one JSON object with uninstalled, removed (only categories actually deleted), and skills (paths actually deleted); failures go to stderr so stdout stays a single document.
• Adds removed: Option<bool> to SkillLocationReport; vera agent remove --json includes it and prints reports before returning the first failure, so successful deletions remain visible.
• Uses try_exists to distinguish unreadable skills from absent ones; does not print “No Vera skill installations found.” when failures occurred.
• Lists categories only when work was done (data dir and PATH shim are reported only if deleted). Streams unchanged: listings to stdout; progress/notes to stderr.

Migration:

• Update scripts that parsed multiple JSON documents from vera uninstall --json to read a single object and derive removed categories from removed and skills.

^{Written for commit a18b7b6. Summary will update on new commits.}