[robzolkos]

Summary

Fixes #800: Linux commands can hang indefinitely when the system password vault stops responding.

• Bound Linux keyring probes and operations to 10 seconds. An initial failed probe retains the existing warned file fallback; a failure after the keyring is in use returns an error without silently switching storage.
• Preserve storage failure details in doctor, including later authentication reads and refresh reads/writes. Offer login only when the failure actually calls for it.
• Bound legacy migration calls, honor BASECAMP_NO_KEYRING, and withhold the completion marker when cleanup times out.
• Document the bypass and timeout behavior in help and SECURITY.md.

Uses the merged shared-library change #79, pinned to c9bce820. The Nix vendor hash is updated and verified.

Verification

Current head: 70829810c5cf6b83f84aeecb0b45ff5f7598fb33.

• Full local bin/ci passes.
• Credential-package race tests and focused doctor/migration race tests pass. The new failure-path tests pass across 20 repeated race-enabled runs. A full command-package race run exceeded the 300-second local budget and is not counted as a pass; hosted race checks rerun on this head.
• Before/after real-binary checks use an isolated simulated stalled vault and synthetic credentials only. Before the fix, auth status, doctor, and migrate each remain hung until stopped after 14 seconds. Afterward they return in approximately 10, 20, and 10 seconds respectively; explicit bypass returns in 0.02–0.04 seconds.
• New regression tests before the follow-up fix reproduce the later-read/refresh diagnostic failures and both migration deletion-timeout branches. The fix makes them pass. Controls retain genuine login advice and ordinary best-effort deletion behavior.
• All five release targets compile. The Nix build passes.
• Shared-library review covered all 7 test packages under the race detector and repeated deadline/concurrency probes. Fizzy passed all 10 test packages and command race tests against the identical library implementation; HEY uses a separate store.

Limits

This prevents the reported hang; it does not repair the underlying desktop service. The originally affected desktop and live macOS/Windows vault behavior have not been retested. Interactive macOS/Windows timeout policy is unchanged.

Already-started keyring writes/deletes cannot be canceled and may complete after timeout. The affected store refuses further operations rather than allowing an unsafe retry. A Linux unlock prompt taking longer than the deadline can trigger the documented timeout behavior.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Authentication storage behavior depends on the still-unmerged basecamp/cli#79 concurrency and timeout implementation.

Review effort: Balanced
Findings: None

What changed in this PR

Bounds Linux keyring operations to prevent stalled D-Bus handshakes from hanging the CLI, with improved diagnostics, migration handling, documentation, and regression coverage.

Changes:

• Applies 10-second Linux keyring probe/operation limits through basecamp/cli#79.
• Preserves explicit fallback/error semantics across authentication, doctor, and migration.
• Documents and tests BASECAMP_NO_KEYRING.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File	Description
SECURITY.md	Documents keyring deadlines and plaintext fallback risks.
nix/​package.nix	Updates the dependency vendor hash.
go.mod	Pins the reviewed credential-store implementation.
go.sum	Updates dependency checksums.
internal/​auth/​keyring.go	Configures Linux timeouts and improves timeout remedies.
internal/​auth/​keyring_test.go	Tests timeout configuration and errors.
internal/​auth/​issue800_keyring_linux_test.go	Reproduces the stalled D-Bus handshake.
internal/​commands/​auth.go	Adds credential-storage help.
internal/​commands/​auth_status_agent_test.go	Adapts agent diagnostic coverage.
internal/​commands/​doctor.go	Distinguishes unreadable credentials and bounds legacy lookup.
internal/​commands/​doctor_test.go	Covers doctor timeout and diagnostic behavior.
internal/​commands/​migrate.go	Bounds legacy keyring migration and honors bypass.
internal/​commands/​migrate_test.go	Tests migration timeout and bypass behavior.
internal/​cli/​help.go	Adds root credential-storage guidance.
internal/​cli/​help_test.go	Verifies override discoverability.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Doctor can still convert a timeout during its later authentication read into an incorrect login recommendation.

Review effort: Balanced
Findings: 1

Open (1)

• Preserve credential-store timeout errors during authentication · New

[Copilot]

Copilot review overview

🔵 Needs a closer look

A timeout during doctor’s later authentication read still incorrectly recommends logging in.

Review effort: Balanced
Findings: 1

Open (1)

• Preserve credential-store timeout errors during authentication

[robzolkos]

Rebased onto main at fe7738a and resolved the conflicts, preserving all eight original commits, including the regression-before-fix history. New head: 590dacd.

Fresh verification:

• Full local bin/ci passes.
• The isolated stalled-vault test still fails on the rebased pre-fix commit and passes with the fix: desktop and headless probes return in about 10 seconds; explicit bypass returns immediately.
• All five release targets compile.
• Refreshed the Nix vendor hash for the current dependency set. The standalone local Nix build is still downloading its toolchain; it has not yet been verified to completion. Hosted checks are running on the new head.

The shared-library dependency remains pinned to 86242a69567b; basecamp/cli#79 is still open and must land first. The test/check claims in the original description refer to the old head unless superseded above. Nothing is merged and auto-merge is not armed.

[Copilot]

🟡 Changes recommended

Migration can conceal deletion timeouts behind completion markers, and doctor can still substitute login advice for a refresh-time keyring failure.

3 open issues

• Preserve refresh storage errors and require genuine login remedies · New
• Record legacy deletion timeouts before writing migration marker · New
• Preserve credential-store timeout errors during authentication

_{Review effort: Balanced}

[robzolkos]

Fresh combined verification on e820556, now pinned to the final shared-library head afacf59fa6b6bb636ccdbe19900d9ae01934d3dd from basecamp/cli#79.

The real binaries were run against an isolated simulated stalled password vault, using synthetic credentials only:

• Before the fix: auth status, doctor, and migrate each remained hung until stopped after 14 seconds.
• With the fix: auth status returned in 10.03s, doctor in 20.08s, and migrate in 10.03s.
• Explicit bypass: all three returned in 0.03–0.04s without touching the simulated vault.

Full local bin/ci, credential race tests, all five release builds, and the Nix build pass. The Nix-built binary also runs. Shared-library and Fizzy compatibility checks pass; the fresh review found no new blockers in that scope. These results supersede the earlier-head verification notes.

Still unverified: the originally affected desktop and live macOS/Windows vault behavior. The underlying desktop-service problem is not fixed; its ability to hang commands is bounded. Hosted checks are rerunning. Nothing merged; #79 still needs to land first.

[robzolkos]

Holding this PR rather than merging: the final convergence check found 3 unresolved review threads. The initial stalled-vault reproduction and local checks pass, but later authentication/refresh failures can still give misleading login advice, and legacy deletion timeouts can be hidden by a migration completion marker. My earlier fresh-review summary did not account for these outstanding findings. They need explicit fixes or reasoned dispositions before merge. The shared-library dependency has now been merged in basecamp/cli#79; this PR and issue #800 remain open.

[Copilot]

🔵 Needs a closer look

Doctor still recommends login when its subsequent Authentication credential read times out after the initial checks succeed.

3 open issues

• Record legacy deletion timeouts before writing migration marker
• Preserve refresh storage errors and require genuine login remedies
• Preserve credential-store timeout errors during authentication

_{Review effort: Balanced}

[Copilot]

🟢 Approved

No unresolved findings remain, and regression coverage addresses timeout handling, backend selection, migration completion, and authentication remedies.

1 open issue

• Preserve credential-store timeout errors during authentication

2 resolved since last review

• Record legacy deletion timeouts before writing migration marker
• Preserve refresh storage errors and require genuine login remedies

_{Review effort: Balanced}

[robzolkos]

Ready for merge review on 7082981: all three findings are fixed and all three review threads are resolved, with no unanswered findings. Copilot approved this exact head. Hosted CI accounts for all 41 checks: 36 successful, 4 conditionally skipped, 1 neutral, none failed or pending. The neutral check is Cubic, which could not review because its monthly quota is exhausted.

Full local CI, focused doctor/migration race checks, 20 repeated runs of the new failure-path tests, all five release builds, the Nix build, and the original before/after stalled-vault reproduction pass. The full local command-package race attempt timed out, but all four hosted race shards and their aggregate pass on this head. The PR now uses the merged shared-library implementation.

No regression was found in the checked paths. The originally affected desktop and live macOS/Windows credential-vault behavior remain unverified. Nothing merged in this follow-up.