Defer keyring probe to first credential use; belt the installers

[jeremy]

Stacked on #575 (retargets to main when it merges). Draft until #575 lands.

The hang is CLI-wide, not a CI quirk

Every command runs root PersistentPreRunE → appctx.NewApp → auth.NewManager → auth.NewStore → credstore.NewStore, which eagerly probes OS keyring availability with a write. On macOS that probe shells out to an uncancellable security child (go-keyring keyring_darwin.go); a locked keychain with no TTY or GUI blocks forever — on every command, including credential-free ones like setup agents and skill install. That's how the headless installer hung (#568; canary discovery #569).

The fix (two layers)

Go — lazy construction (internal/auth/keyring.go): NewStore records the fallback dir; the credstore is built behind sync.Once on first credential operation. Credential-free commands (setup agents, skill install, completion, etc.) never touch the keyring. Credential-touching commands probe on first use exactly as before, with the documented BASECAMP_NO_KEYRING=1 escape hatch. Reaches users at the next release.

Installer belt (deploys on merge, covers released binaries ≤ v0.7.2 which still probe eagerly): post_install_setup in install.sh prefixes each best-effort child with per-command BASECAMP_NO_KEYRING=1; Invoke-PostInstallSetup in install.ps1 sets it for the duration of setup and restores the caller's value on exit. These children never touch credentials — the var only skips old binaries' startup probe. The setup --help capability probes in install.sh stay bare (help short-circuits PersistentPreRunE).

Deliberately no probe timeout: an abandoned goroutine cannot kill the hung security child, so a timeout would hide the hang while preserving the resource leak. Bounded fallback for credential-touching commands is deferred pending cancellation support upstream in credstore/go-keyring (issue to follow on basecamp/cli).

Canary re-scope

The workflow-level BASECAMP_NO_KEYRING is removed with no step-scoped replacement: the macOS leg now genuinely exercises the deployed belt (a missing belt = hung security child = 10-minute timeout = notify), and the direct basecamp version asserts skip PersistentPreRunE entirely (root.go handles help/version before app construction). Once a release with the lazy fix ships, the canary stops distinguishing belt presence; the bats + pwsh tests remain the durable belt verification.

Tests

• keyring_test.go: counting-fake seam proves the constructor makes zero credstore calls, first operation makes one, subsequent operations reuse it. (The "credential-free commands never probe" conclusion additionally rests on the audited command paths — stated in the test comment, not overclaimed.)
• installer.bats: belt tests explicitly unset the suite-wide BASECAMP_NO_KEYRING and assert both directions — real setup agents/skill install calls log nk=1, the sh setup --help probe logs nk=unset (belt is scoped, not blanket).
• pwsh behavioral test (the Windows canary can never prove the ps1 belt — Credential Manager works headlessly either way): extracts Invoke-PostInstallSetup from install.ps1's AST via Parser.ParseFile and evaluates only that function — no test hooks or skip-main env vars in the production script — then asserts real calls see 1 and the env var is restored (removed) after return. Skips locally without pwsh; fails closed in CI.

Verification

• go test ./internal/auth/... and full bin/ci green; bats e2e/installer.bats 14 pass + pwsh skip locally.
• pwsh test validated end-to-end in a PowerShell container (arm64): restored-ok, nk=1 setup agents.
• Manual lazy proof on macOS: credential-free completion bash averages 105ms on the eager binary vs 43ms lazy — the ~60ms delta is the keyring probe, gone. auth status output is byte-identical eager vs lazy with the keyring active, and unchanged under BASECAMP_NO_KEYRING=1.
• After merge: the push canary fires on both scripts (post-ci: harden installer canary and add pre-merge Bash 3.2 gate #575 paths) — the macOS leg is genuine belt coverage; Linux and Windows are deployment smoke. Plus one manual cache-busted curl … install-cli?sha=<merge-sha> | bash in a scratch HOME without exporting BASECAMP_NO_KEYRING.

Refs #558 #568 #569

---

Summary by cubic

Defers the OS keyring probe to first credential use and adds a scoped installer “belt” that sets BASECAMP_NO_KEYRING=1 to prevent hangs on headless macOS and in older releases (≤ v0.7.2). Also updates CI to exercise the belt directly.

• Bug Fixes

  • Lazy keyring: internal/auth/keyring.go now builds credstore.Store on first use via sync.Once with a test seam newCredStore. Credential-free commands never touch the keyring; credentialed paths are unchanged and still honor BASECAMP_NO_KEYRING.
  • Installer belt: scripts/install.sh prefixes real setup/skill calls with BASECAMP_NO_KEYRING=1 while keeping setup --help bare; scripts/install.ps1 sets BASECAMP_NO_KEYRING=1 for the duration of Invoke-PostInstallSetup and restores any caller value on exit. .github/workflows/installer-smoke.yml removes the workflow-level var so macOS canaries validate the deployed belt.

• Tests

  • Added internal/auth/keyring_test.go to pin lazy construction. Expanded e2e/installer.bats to assert the sh belt and a PowerShell pass that verifies both restoring to unset and preserving a caller-set BASECAMP_NO_KEYRING value.

^{Written for commit 76e9b97. Summary will update on new commits.}

[github-actions]

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

• .github/workflows/installer-smoke.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

[cubic-dev-ai]

All reported issues were addressed across 6 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[Copilot]

Pull request overview

Defers OS keyring probing to first credential use (avoiding hangs on headless macOS for credential-free commands) and “belts” the installers so post-install setup of older binaries won’t trigger the eager startup keyring probe.

Changes:

• Lazily constructs the underlying credstore.Store behind sync.Once on first credential operation.
• Adds installer “belt” behavior: BASECAMP_NO_KEYRING=1 is applied to best-effort post-install setup/skill calls (scoped per-command in install.sh, scoped to setup duration with restoration in install.ps1).
• Updates installer smoke canary to remove workflow-level BASECAMP_NO_KEYRING so CI exercises the deployed belt.

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.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
scripts/install.sh	Prefixes post-install setup/skill invocations with BASECAMP_NO_KEYRING=1 to prevent hangs for older eager-probe binaries.
scripts/install.ps1	Sets BASECAMP_NO_KEYRING=1 for the duration of Invoke-PostInstallSetup and restores the prior value (or absence) on exit.
internal/auth/keyring.go	Makes credstore.NewStore lazy via sync.Once so credential-free paths don’t hit the keyring probe.
internal/auth/keyring_test.go	Adds a constructor seam + test to assert lazy construction and one-time initialization behavior.
e2e/installer.bats	Adds belt assertions for both installers; validates install.ps1 env-var restore behavior via AST-extracted function evaluation.
.github/workflows/installer-smoke.yml	Removes workflow-level BASECAMP_NO_KEYRING to ensure the canary exercises the deployed installer belt.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.