docs: fix two broken README snippets and nine stale claims before 2.0.0 - #80
Merged
Conversation
Two examples did not work. Both were verified by running them, and both replacements verified the same way rather than by inspection. The DPAPI section registered the manager by type, AddSingleton<ICredentialManager, FileCredentialManager>(). FileCredentialManager takes a string credentialsDirectory, so DI throws "Unable to resolve service for type 'System.String'". Replaced with an explicit factory registration that also passes GetServices<ICredentialSummaryProvider>() -- without which the provider columns silently vanish from accounts list even once it resolves. The custom-encryption-backend section had the order backwards: it said register before AddCredentialStore. AddCredentialStore registers its own ICredentialEncryption and the last registration wins, so a custom backend registered first was silently ignored and credentials stayed on the default. No error, on a change made specifically to strengthen encryption -- the worst failure mode available. Now says after, says why order matters, and gives a one-liner to verify. Stale claims corrected: the libsecret intro still advertised KWallet and "any Secret Service implementation" twenty lines above the block declaring KWallet unsupported; that block still attributed the hang to the missing cancellable, which #74 disproved by observing the cancellation fire and the call still not return; "operations will throw" omitted that they can instead block; and Contributing pointed at now-closed #74. Missing documentation added: CancellationToken support, which is 2.0.0's headline; export/import in the features list; the (unreadable) marking in accounts list; and that accounts export skips credentials it cannot read and warns -- an archive can be legitimately incomplete, which matters because the alternative would restore a blank over a real secret. Providers.GitHub ships at 1.0.1 and was missing from the table, while the worked example walked readers through writing a GitHub provider from scratch. Listed, and the example now says it exists. Co-Authored-By: Claude Opus 5 <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
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.
Full README review ahead of 2.0.0. Twelve findings; the two code ones were verified by running them, and so were the replacements.
Two snippets that did not work
1. The DPAPI example could not resolve.
FileCredentialManagertakes astring credentialsDirectory:Replaced with an explicit factory registration — which also passes
GetServices<ICredentialSummaryProvider>(). That detail matters beyond compiling: without it the provider columns silently disappear fromaccounts list. The replacement is verified end to end (add+list,provider columns=1).2. The custom-encryption advice was backwards, and failed silently.
The README said register your
ICredentialEncryptionbeforeAddCredentialStore. ButAddCredentialStoreregisters its own, and last registration wins:So a custom backend registered first was ignored, and credentials kept using the default — with no error, on a change made specifically to strengthen encryption. That is the worst available failure mode for this particular piece of advice. Now documents after, explains why order matters, and gives a one-liner to verify. Confirmed:
resolved=MarkerEncryption.Stale after this cycle
ksecretdsendsaowhere libsecret expectsoon the prompt'sCompletedsignal. Now states the actual upstream cause.Missing
export/importabsent from the features list.accounts listmarking rows(unreadable).accounts exportskips credentials it cannot read and warns. Called out beside the "Fidelity" bullet, which previously read as though everything is always preserved. An archive can be legitimately incomplete — and the reason is worth stating, since exporting a blank payload instead would restore over a real secret at the far end.Providers.GitHubships at 1.0.1 and was missing from the table — while the worked example teaches you to build a GitHub provider from scratch. Now listed, and the example says so.Verification
Build clean; no code changed. Both new snippets executed successfully, and all external links resolve.
🤖 Generated with Claude Code