feat(secrets): make keyring open timeout configurable, default 30s

[malob]

Summary

The keyring open/operation timeout — added so an unresponsive backend can't hang
the CLI forever (#513, #221) — defaults to 10s. This raises that default to 30s
and makes it configurable via GOG_KEYRING_OPEN_TIMEOUT (a Go duration),
mirroring the existing GOG_KEYRING_LOCK_TIMEOUT.

• GOG_KEYRING_OPEN_TIMEOUT=45s (any Go duration) overrides the timeout;
  empty/invalid/non-positive values fall back to the default.
• Default raised 10s → 30s.
• The timeout error now points at GOG_KEYRING_OPEN_TIMEOUT.

Why

On macOS this timeout bounds every Keychain read, including the wait for an
interactive permission prompt. 10s is usually enough to approve one — but it
leaves little margin for the normal frictions: not noticing the prompt for a few
seconds, fumbling and re-entering the login password, or macOS showing two
prompts back to back (client secret + token). When that margin runs out, a
legitimate prompt times out before it's approved and auth fails on a perfectly
healthy machine. 30s gives comfortable headroom for those cases while still
bounding the failure the timeout was created for — an indefinite hang
(commands hung until SIGKILL, #513), not a matter of seconds.
GOG_KEYRING_OPEN_TIMEOUT then lets anyone tune it further, just like
GOG_KEYRING_LOCK_TIMEOUT already does for the lock wait.

If raising the default is contentious, the env var alone is still a worthwhile
addition (parity with the lock-timeout knob) — but I'd argue 30s should be the
default: 10s is too tight for the common interactive-Keychain path, and 30s still
firmly bounds the hang case.

Behavior proof

With the Keychain permission prompt left unanswered (so the read blocks until the
timeout fires), wall-clock time matches the configured timeout:

# default — read gives up after 30s
$ time ./bin/gog calendar calendars --account <redacted> --json
... keyring connection timed out after 30s while reading keyring item (...);
    set GOG_KEYRING_OPEN_TIMEOUT to allow more time, or set GOG_KEYRING_BACKEND=file ...
# ≈ 30s elapsed

# override — the same read gives up after the configured 5s
$ time GOG_KEYRING_OPEN_TIMEOUT=5s ./bin/gog calendar calendars --account <redacted> --json
... keyring connection timed out after 5s while reading keyring item (...);
    set GOG_KEYRING_OPEN_TIMEOUT to allow more time, or set GOG_KEYRING_BACKEND=file ...
# ≈ 5s elapsed

Testing

• make fmt-check, make lint, make docs-check clean.
• go test ./internal/secrets -run 'OpenTimeout|ParseKeyringOpenTimeout|TimeoutKeyring' —
  covers env parsing (valid/empty/invalid/non-positive), the OpenOptionsFromLookup
  wiring, and that the timeout error names GOG_KEYRING_OPEN_TIMEOUT.
• Documented the env var in docs/spec.md.

No CHANGELOG.md entry — left to the release/landing process per repo convention.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed June 19, 2026, 3:33 AM ET / 07:33 UTC.

Summary
The PR makes the keyring open timeout configurable with GOG_KEYRING_OPEN_TIMEOUT, raises the macOS default to 30s while keeping other platforms at 10s, and updates timeout guidance, docs, tests, and the changelog.

Reproducibility: not applicable. this is a feature/default-change PR rather than a current-main bug report. The PR body does provide redacted live-output proof for the proposed timeout behavior.

Review metrics: 2 noteworthy metrics.

• Touched surface: 6 files changed, 92 additions, 14 deletions. The PR changes runtime timeout behavior, docs, unit tests, and a release-note file, so maintainers should review both code and user-facing wording.
• Timeout default: macOS 10s -> 30s; other platforms stay 10s. This is the upgrade-facing behavior change that remains a maintainer decision.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Maintainers should explicitly choose whether the macOS default may move to 30s or whether the env var should land with the old 10s default.

Risk before merge

• [P2] The macOS default intentionally delays timeout guidance for genuinely stuck Keychain operations from 10s to 30s, so maintainers need to accept that availability tradeoff before merge.

Maintainer options:

1. Accept the macOS 30s default
   Land the scoped default if maintainers agree that avoiding premature Keychain prompt failures is worth slower failure on a genuinely stuck macOS Keychain operation.
2. Keep 10s as the default
   Ask for a narrow revision that adds GOG_KEYRING_OPEN_TIMEOUT while preserving the existing 10s default on every platform.
3. Pause the timeout surface
   Pause or close the PR if maintainers do not want a new environment variable for keyring operation timing.

Next step before merge

• [P2] The remaining blocker is maintainer acceptance of the macOS default-timeout tradeoff, not a narrow automated repair.

Security
Cleared: The diff adds a duration env var, docs, and tests without changing credential storage boundaries, dependencies, permissions, or secret exposure.

Review details

Best possible solution:

Land the env-var support with explicit maintainer acceptance of the macOS-only 30s default, or keep the 10s default everywhere and let users opt into longer waits.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a feature/default-change PR rather than a current-main bug report. The PR body does provide redacted live-output proof for the proposed timeout behavior.

Is this the best way to solve the issue?

Unclear: the env-var implementation is narrow and matches the existing lock-timeout pattern, but the macOS 30s default is an operational policy choice. The safer compatibility alternative is env-var support with the existing 10s default.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b35481eefd60.

Label changes

Label justifications:

• P2: This is a normal-priority keyring reliability/configuration improvement with limited blast radius but real timeout behavior impact.
• merge-risk: 🚨 availability: Merging changes macOS stuck-Keychain failure latency from 10s to 30s by default.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes redacted copied terminal output showing after-change timeout behavior for both the new default and an env override.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted copied terminal output showing after-change timeout behavior for both the new default and an env override.

Evidence reviewed

What I checked:

• Repository policy checked: AGENTS.md was read fully and its PR-review/read-only guidance was applied. (AGENTS.md:1, b35481eefd60)
• Current main lacks the new knob: Current main still sets OpenTimeout to the fixed keyringOpenTimeout value from OpenOptionsFromLookup; no GOG_KEYRING_OPEN_TIMEOUT symbol exists on main. (internal/secrets/backend.go:83, b35481eefd60)
• Latest release lacks the new knob: v0.28.0 still uses the 10s keyring open timeout and docs list no open-timeout env var, so the PR is not already shipped. (internal/secrets/backend.go:168, 6bc67d7dd770)
• PR implementation: The PR head adds keyringOpenTimeoutEnv, parses the env value, adds macOS/non-macOS defaults, and uses the platform default when option values are unset or non-positive. (internal/secrets/backend.go:19, e380be2ec407)
• Tests and docs: The PR adds env parsing/default tests and documents the macOS 30s versus other-platform 10s default in the spec. (internal/secrets/store_test.go:79, e380be2ec407)
• Proof in PR body: The PR body includes redacted copied terminal output showing a 30s default timeout and a 5s GOG_KEYRING_OPEN_TIMEOUT override on a blocking Keychain permission prompt. (e380be2ec407)

Likely related people:

• steipete: Authored the current macOS keyring operation-timeout work and the PR-head follow-up that scoped the longer timeout to macOS. (role: introduced behavior and recent area contributor; confidence: high; commits: 6430dd1a99f3, 36713a39afe4, e380be2ec407; files: internal/secrets/backend.go, internal/secrets/timeout_keyring.go, internal/secrets/store_test.go)
• salmonumbrella: Introduced the earlier headless Linux keyring hang avoidance and Homebrew Keychain trust behavior next to this timeout policy. (role: original adjacent behavior contributor; confidence: medium; commits: 64aa17f16772, 0a7dd586afea; files: internal/secrets/store.go, internal/secrets/store_test.go)
• lox: Merged recent auth/keyring prompt-storm hardening that touched timeout-keyring behavior and nearby auth read paths. (role: recent adjacent contributor; confidence: medium; files: internal/secrets/timeout_keyring.go, internal/cmd/auth_list_helpers.go, internal/cmd/auth_tokens.go)
• sardoru: Opened the earlier macOS keyring timeout PR that was closed after a maintainer rewrite landed the current timeout implementation. (role: prior fix proposer and reporter; confidence: medium; commits: dff209a89f84; files: internal/secrets/store.go, internal/secrets/store_test.go)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[steipete]

Landed with the maintainer policy adjustment: 30s by default on macOS, 10s elsewhere, and GOG_KEYRING_OPEN_TIMEOUT authoritative on every platform.

• Local: focused internal/secrets tests and full make ci passed.
• Review: autoreview clean; no accepted/actionable findings.
• E2E: reviewed the recorded macOS Keychain prompt proof for both the default and explicit override; this machine's encrypted file-keyring access was unavailable for a second live prompt run.
• CI: exact-head CI and Docker runs passed.