fix(account): surface malformed CSHIP_ACCOUNT in explain hint, warn on empty - #204
Merged
Conversation
…n empty Follow-up to PR #201's CSHIP_ACCOUNT account source: - explain.rs: the "account" hint arm now checks CSHIP_ACCOUNT first, so a set-but-unparseable value is reported as the cause instead of the misleading "no credential found" / re-authenticate hint - modules/account.rs: parse_account_env now warns on set-but-empty CSHIP_ACCOUNT (previously silent), matching the "absent data -> warn" convention already used for the malformed-JSON case - docs/configuration.md: qualify the two now-inaccurate unconditional statements about the OAuth fetch/cache and credential-store prerequisite with "unless CSHIP_ACCOUNT is set" Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Follow-up to code review of #201 (CSHIP_ACCOUNT account source). Three minimal fixes:
src/explain.rs— the"account"arm oferror_hint_fornow checksCSHIP_ACCOUNTfirst. When it's set and non-empty but fails to parse as the expected JSON, the hint now names the malformedCSHIP_ACCOUNTpayload as the cause instead of the misleading "no Claude Code credential found" / re-authenticate message. Reusesmodules::account::parse_account_envrather than duplicating the parsing logic.docs/configuration.md— qualified two pre-existing unconditional statements that now contradict theCSHIP_ACCOUNTparagraph: the "fetched once from the OAuth endpoint and cached for 24 hours" line and the "requires an OAuth token in the OS credential store" prerequisite, both now read "unlessCSHIP_ACCOUNTis set".src/modules/account.rs—parse_account_envnow emitstracing::warn!on set-but-emptyCSHIP_ACCOUNT(previously silentNone), matching CLAUDE.md's "absent data → explicit match + tracing::warn! + None" convention already applied to the malformed-JSON case. Updated the existing test's comment to reflect both branches now warn.Test plan
cargo testcargo clippy --all-targets -- -D warningscargo fmt --check🤖 Generated with Claude Code