stash doctor: an absent optional package is a skipped check, not a pass - #884
Open
tobyhede wants to merge 1 commit into
Open
stash doctor: an absent optional package is a skipped check, not a pass#884tobyhede wants to merge 1 commit into
tobyhede wants to merge 1 commit into
Conversation
Two pre-existing issues, both older than the probe rework in #883 and untouched by it. `stash doctor` ended with "All checks passed." when `@cipherstash/stack` was absent. It is an optional peer, so this is the plain `npx stash doctor` path in a project that has not run `stash init` — the row said "not installed (run `stash init`)" and the outro then claimed a pass for a check that never executed. It now ends with `checksIncomplete`, the line the too-old-to-probe arm already used for exactly this distinction, and still exits 0: absence there is recoverable, not a failure. The row stays green for the same reason. The probe labels move to `messages.ts`. Both E2E suites match a row as `<label> — <detail>`; the detail halves were already constants while the labels were copied into each suite, one of them under a comment conceding the drift. `packages/cli/AGENTS.md` asks for exactly this: strings tests assert on live in `messages.ts`, and its "only when a test asserts on it" caveat is now satisfied. Verified non-vacuous: reverting the one-line outro change fails the absent-package E2E and nothing else. - CLI unit 1243 passed, e2e 108 passed - typecheck 3/3, biome error-free
🦋 Changeset detectedLatest commit: ac46a5a The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Base automatically changed from
toby/cip-3720-stash-doctor-probe-the-native-binding-for-real
to
main
August 13, 2026 09:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #883 — base is that branch, so this diff is one commit. Both fixes are for issues that predate it and that it leaves untouched; a review of #883 surfaced them, but neither is a regression from it.
An absent
@cipherstash/stackended the run with "All checks passed."It is an optional peer, so this is the plain
npx stash doctorpath in a project that has not runstash init. The row already saidnot installed (run \stash init`)` — and then the outro claimed a pass for a check that never executed. That is the one line of doctor's output a user has no way to second-guess.It now ends with
checksIncomplete("stash doctor could not run every check."), which is the line the too-old-to-probe arm already uses for exactly this distinction — #883 introduced that vocabulary for the structurally identical case and did not apply it here. Exit code stays 0 and the row stays green: absence beforeinitis recoverable, not a failure, and dressing it up as one would send users to a reinstall for a package they simply have not installed yet.The probe labels were hard-coded in three places
packages/cli/AGENTS.md: "Strings that tests assert on live insrc/messages.ts… Don't hard-code the new wording in a test." Both E2E suites match a row as<label> — <detail>. The detail halves were already constants; the labels were copied into each suite, one of them under a/** Mirrors the probe labels in \src/commands/doctor/index.ts`. */` comment conceding the drift.Moved to
messages.doctor.encryptionProbeLabel/authProbeLabel. The same file's caveat — "Add tomessages.tsonly when a test actually asserts on the string" — is what makes this the right call rather than premature extraction.Tests
doctor-probe-classification.e2e.test.tsasserted the old outro, so it changes with the behaviour. Verified non-vacuous: reverting the one-line prod change and rebuilding fails that test and nothing else.doctor.e2e.test.tsstill assertsAll checks passed.on a healthy install, which is the other half — this change must not touch the case where both probes ran.turbo typecheck3/3,biome check --diagnostic-level=errorcleanNotes for the reviewer
stashpatch — the outro wording is user-visible in a real scenario.skills/stash-clidescribesdoctorgenerically (one table row, one troubleshooting line) and names no output strings. Command and flag surface unchanged.main— the label move rebases ontomaincleanly, but the outro fix does not, sincechecksIncompletedoes not exist there.