fix(secrets): stop account sessions from popping Keychain

[Hmbown]

Summary

codewhale web and the TUI were hitting macOS Keychain on every start because account sessions always called Secrets::system_keyring(). Unsigned or rebuilt binaries are a new Keychain ACL principal, so the leftover deepseek / codewhale-cloud-auth-v1-* item popped a password dialog every time.

Account sessions now use ~/.codewhale/secrets/secrets.json (0600), the same store as provider keys. codewhale doctor already reports backend: file.

This is the Keychain-retirement half of #5632, without the worker-model changes. Do not merge #5632 while its tests are red.

No-Issue: Keychain ACL prompt on unsigned rebuilds; extracted from #5632 without closing that PR

Testing

• cargo test -p codewhale-secrets --locked session_secrets_use_the_file_store_not_keychain
• ./scripts/release/check-versions.sh

Checklist

• Updated docs or comments as needed
• Added or updated tests where relevant
• Verified TUI behavior manually if UI changes
• Harvested/co-authored credit uses a GitHub numeric noreply address

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[claude]

Claude finished @Hmbown's task in 2m 3s —— View job

---

Code Review

• Gather context (diff, CLAUDE.md/AGENTS.md rules)
• Review crates/secrets/src/account.rs changes
• Review CHANGELOG.md changes
• Check for provider/model route-safety implications
• Check test coverage
• Post final review

Summary: Small, well-scoped fix. secure_account_session_secrets() now calls Secrets::file_backed() unconditionally instead of Secrets::system_keyring(), which correctly stops the Keychain probe (and its ACL-mismatch password prompt) for account sessions specifically, without touching provider-key resolution (Secrets::auto_detect() / system_keyring() elsewhere are untouched). The updated test session_secrets_use_the_file_store_not_keychain (crates/secrets/src/account.rs:420) is a reasonable, cheap regression guard for this behavior. No provider/model route-safety concerns — this touches only the secrets backend, not any model/provider selection logic.

Finding — stale comment now contradicts the new behavior:
crates/cli/src/cloud.rs:527-531 (cloud_session_secrets(), not part of this diff but directly describes the function this PR changed):

fn cloud_session_secrets() -> Result<Secrets> {
    // Codex-style storage contract: the OS credential manager is preferred
    // but never required; without one, sessions live in the private 0600
    // Codewhale secrets file. Only an unresolvable store path fails here.
    secure_account_session_secrets().map_err(|err| anyhow!(err.to_string()))
}

This comment still describes the old "prefer the OS keyring, fall back to file" contract that secure_account_session_secrets() (crates/secrets/src/account.rs:282) no longer implements — it's now file-only, always. Leaving this as-is risks a future contributor reintroducing the keyring path here based on the stale comment. Worth a one-line update to match the new doc comment on secure_account_session_secrets.
Fix this →

Minor, non-blocking note: ACCOUNT_KEYRING_SERVICE (crates/secrets/src/account.rs:28) is now unreferenced anywhere in the workspace since account sessions no longer probe the keyring under that service name. It's pre-existing (not touched by this diff) and still a plausible constant for a future keyring-based migration/cleanup path, so not asking for removal here — just flagging it's currently dead.

Good choices:

• Removing the "prefer keyring, fall back to file" doc comment and test in favor of an explicit "never touch Keychain" contract accurately reflects the new, simpler behavior (crates/secrets/src/account.rs:274-282).
• The test now explicitly asserts the backend name does not contain "keychain"/"keyring", which is a stronger and more direct regression guard than the old "file or keyring, either is fine" assertion.
• Changelog entry is clear, explains the root cause (unsigned/rebuilt binaries = new Keychain ACL principal), and correctly notes this is a subset of One worker system; retire Keychain product path #5632 extracted to unblock independently.

Not run: cargo fmt --all -- --check and cargo clippy could not be executed in this environment (sandboxed, no command-execution approval available) — please confirm these pass in CI as noted in the PR's own testing checklist.

Branch: grok/v0912-retire-keychain-session-20260827

[claude]

Claude finished @Hmbown's task in 1m 6s —— View job

---

Code Review (re-run after new commit)

• Gather context (diff since previous review, new commit)
• Check if prior finding (stale comment in crates/cli/src/cloud.rs) was addressed
• Review crates/secrets/src/account.rs changes
• Review CHANGELOG changes
• Post final review

Summary: New commit c2cca1a is docs-only — it mirrors the CHANGELOG.md "Keychain retirement" entry into crates/tui/CHANGELOG.md verbatim (same wording, same ### Fixed section). No code changes since the previous review; the secrets/src/account.rs fix (file-backed store, no Keychain probe) and its updated test (session_secrets_use_the_file_store_not_keychain) are unchanged and still correct.

Still open — stale comment not updated:
The finding from the previous review was not addressed in this commit. crates/cli/src/cloud.rs still has two comments describing the old "prefer OS keyring, file is a fallback" contract that secure_account_session_secrets() no longer implements:

• crates/cli/src/cloud.rs:500-501 (above the cloud_session_secrets() call site): "Account refresh tokens require an OS credential manager."
• crates/cli/src/cloud.rs:528-530 (cloud_session_secrets() itself): "the OS credential manager is preferred but never required."

Both should be updated to reflect that account sessions always use the private 0600 file store and never touch the OS keyring, per the new doc comment on secure_account_session_secrets() in crates/secrets/src/account.rs:274-279. Not a correctness bug (behavior is right), but leaves misleading guidance for the next person touching this code path.
Fix this →

No new issues found in this update. Provider/model route-safety: unaffected (docs-only change). ACCOUNT_KEYRING_SERVICE remains unreferenced workspace-wide, as noted previously — still pre-existing and non-blocking.

Not run: cargo fmt --all -- --check and cargo clippy could not be executed in this environment — please confirm CI is green.

Branch: grok/v0912-retire-keychain-session-20260827