test: cover macOS keychain metadata

[rianjs]

Summary

• add a gated Darwin+cgo test that writes through credstore.Store.Set and inspects the real ByteNess Keychain metadata via GetMetadata
• assert Keychain label and description match the concrete service account format used to avoid prompts naming ""
• log the synthetic service/account and clean up the synthetic item after the run

Existing blank-label Keychain items are not mutated by reads or failed no-overwrite writes. Metadata is present for new items and successful overwrites.

No semver tag/release is being cut here; downstream consumers still need to be verified against this fix before the coordinated release train is considered complete.

Closes #62

Verification

• go test ./credstore
• go test -tags keyring_no1password,keyring_nopassage ./credstore
• CREDSTORE_OS_KEYRING_TEST=1 go test ./credstore -run 'TestOSKeyringBackendsGated|TestKeychainMetadataGated' -count=1 -v
• make check
• go test -race -tags keyring_nopassage ./...
• go test -race -tags keyring_no1password,keyring_nopassage ./...

[rianjs]

Findings

None.

Code Quality Lens

The diff is tightly scoped to #62: one Darwin+cgo gated empirical test, no public API changes, no formatting changes, and no downstream or release work. The test exercises the intended product path by writing through credstore.Store.Set, then independently reads real Keychain metadata through ByteNess GetMetadata and asserts the exact non-empty label/description format. Cleanup is registered before assertions and the synthetic service/account are logged for manual recovery.

I reran go test ./credstore locally; it passed.

[rianjs]

• Major: credstore/osbackend_keychain_metadata_darwin_test.go:35 only proves one fresh write path. It never exercises a true overwrite/update of an existing Keychain item, so a regression where first writes get the right Label/Description but overwrites keep blank metadata would still pass. Since the PR body claims metadata is present for successful overwrites and existing blank items are not mutated on reads or failed no-overwrite writes, this leaves the main behavioral claim under-tested. I would add a second phase that seeds an existing item and rewrites it, or a separate assertion around the rejected no-overwrite case.

• Minor: credstore/osbackend_keychain_metadata_darwin_test.go:15 makes the test fully opt-in, so it will not run in ordinary go test or most CI lanes. That is understandable for real Keychain coverage, but it does mean the regression guard is manual unless there is a dedicated macOS job.

[rianjs]

Minor

• credstore/osbackend_keychain_metadata_darwin_test.go: The legacy-seed phase should assert the seeded item is actually nonconforming before using it to prove no-overwrite preservation. Right now it captures seededLabel / seededDescription; if raw ByteNess/macOS ever defaults these to the target metadata, that portion of the test becomes vacuous. Prefer asserting both are empty if stable, or at least asserting they differ from wantLabel / wantDescription.

• credstore/osbackend_keychain_metadata_darwin_test.go: Consider opening the raw seed keyring with KeychainTrustApplication: true. The later no-overwrite path calls Get for the existence pre-check, which asks Keychain for secret data on the seeded item. Leaving the seed untrusted can introduce an extra authorization prompt during the gated test; the test should be deterministic once the login Keychain is unlocked.

Code Quality Lens

No blockers or product drift. The added coverage is still scoped to #62, exercises real Keychain metadata, and avoids public API changes. I reran go test ./credstore; it passed.

[monit-reviewer]

PR review failed and the daemon has stopped retrying for this commit. I am removing my pending review request to stop the automated review cycle. Re-request @monit-reviewer to retry, or push a new commit and re-request.

[monit-reviewer]

Automated PR Review

Reviewed commit: 6e07882

Manually approved via CLI.

[rianjs]

Findings

None.

The review-driven edits stayed within the agreed architecture: the test still writes through credstore.Store.Set, reads real Keychain metadata through ByteNess, avoids public API changes, keeps release/tag work out of this ticket, and now covers the legacy/no-overwrite/overwrite behavior without adding avoidable Keychain trust prompt noise.

I reran go test ./credstore; it passed.