Disable keyring in installer canary - #571
Merged
Merged
Conversation
The canary's first macOS run hung for its full 10-minute timeout after "Skipping setup wizard": the post-install skill step reaches for the OS keyring, and on a headless runner the `security` tool blocks forever on the locked keychain (the cancelled job's orphan processes were `basecamp` and `security`). Set BASECAMP_NO_KEYRING=1 workflow-wide, the same escape hatch the Test workflow uses. Refs #569
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the installer smoke canary workflow to disable keyring usage so macOS runs don’t hang on headless keychain prompts during the post-install skill step.
Changes:
- Sets
BASECAMP_NO_KEYRING=1at the workflow level ininstaller-smoke.ymlso it applies to all jobs/steps (including childbasecampprocesses). - Documents the rationale inline, matching the existing escape-hatch usage in
test.yml.
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.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
jeremy
added a commit
that referenced
this pull request
Jul 27, 2026
Every CLI command runs root PersistentPreRunE -> appctx.NewApp -> auth.NewManager -> auth.NewStore, and credstore.NewStore eagerly probes OS keyring availability with a write. On macOS that probe shells out to an uncancellable `security` child; a locked keychain with no TTY or GUI blocks it forever — on *every* command, including credential-free ones like `setup agents` and `skill install`. That is how the installer hung headless (#568, canary discovery #569/#571 -> #574 drill lineage). Go fix (reaches users at the next release): Store now records the fallback dir at construction and builds the credstore lazily behind sync.Once on first credential operation. Credential-free commands never touch the keyring; credential-touching commands probe on first use exactly as before, with the documented BASECAMP_NO_KEYRING=1 escape hatch. No timeout was added deliberately: an abandoned goroutine cannot kill the hung `security` child, so a timeout would hide the hang while keeping the leak. Bounded fallback needs cancellation support upstream in credstore/go-keyring first. 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 the old binaries' startup probe. Tests: keyring_test.go pins constructor laziness through an injectable credstore seam. installer.bats gains belt tests that explicitly unset the suite-wide BASECAMP_NO_KEYRING and assert both directions (real calls carry it, the sh capability probe does not), plus a pwsh test that extracts Invoke-PostInstallSetup from install.ps1's AST (no test hooks in the production script), runs it against a logging stub, and asserts set + restore; it fails closed when pwsh is missing in CI. The canary's 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 = timeout = notify), and the direct `basecamp version` asserts skip PersistentPreRunE entirely. Refs #558 #568 #569
jeremy
added a commit
that referenced
this pull request
Jul 27, 2026
* Defer keyring probe to first credential use; belt the installers Every CLI command runs root PersistentPreRunE -> appctx.NewApp -> auth.NewManager -> auth.NewStore, and credstore.NewStore eagerly probes OS keyring availability with a write. On macOS that probe shells out to an uncancellable `security` child; a locked keychain with no TTY or GUI blocks it forever — on *every* command, including credential-free ones like `setup agents` and `skill install`. That is how the installer hung headless (#568, canary discovery #569/#571 -> #574 drill lineage). Go fix (reaches users at the next release): Store now records the fallback dir at construction and builds the credstore lazily behind sync.Once on first credential operation. Credential-free commands never touch the keyring; credential-touching commands probe on first use exactly as before, with the documented BASECAMP_NO_KEYRING=1 escape hatch. No timeout was added deliberately: an abandoned goroutine cannot kill the hung `security` child, so a timeout would hide the hang while keeping the leak. Bounded fallback needs cancellation support upstream in credstore/go-keyring first. 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 the old binaries' startup probe. Tests: keyring_test.go pins constructor laziness through an injectable credstore seam. installer.bats gains belt tests that explicitly unset the suite-wide BASECAMP_NO_KEYRING and assert both directions (real calls carry it, the sh capability probe does not), plus a pwsh test that extracts Invoke-PostInstallSetup from install.ps1's AST (no test hooks in the production script), runs it against a logging stub, and asserts set + restore; it fails closed when pwsh is missing in CI. The canary's 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 = timeout = notify), and the direct `basecamp version` asserts skip PersistentPreRunE entirely. Refs #558 #568 #569 * test(installer): pin restore of a caller-set BASECAMP_NO_KEYRING The pwsh belt test proved restore-to-unset only; a regression that overwrites an existing caller value would have passed. Second driver pass sets the var to 0 before Invoke-PostInstallSetup and asserts it survives. Review feedback on #578.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The canary added in #569 hung on its first two macOS runs (push-triggered and dispatched) for the full 10-minute
timeout-minutes, right after "Skipping setup wizard". The cancelled job's orphan processes werebasecampandsecurity: the post-install skill step reaches for the OS keyring, and on a headless macOS runner thesecuritytool blocks forever on the locked keychain. The Linux leg passed on both runs.Set
BASECAMP_NO_KEYRING=1at the workflow level — the same escape hatchtest.ymluses. The variable reaches the installer's childbasecampprocesses through the step environment.Verification: will dispatch the canary after merge and confirm both legs pass.
Refs #569
Summary by cubic
Disable keyring usage in the installer canary workflow to stop macOS runs from hanging on headless keychain prompts. Sets
BASECAMP_NO_KEYRING=1at the workflow level so childbasecampprocesses skip thesecuritykeychain call; Linux runs are unchanged.Written for commit 8d8b5ad. Summary will update on new commits.