Port basecamp-cli's onboarding flow: setup wizard, agent setup, login/logout - #228
Merged
Conversation
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Ports the onboarding flow, agent integration setup, authentication shortcuts, and installer handoff into hey-cli.
Changes:
- Adds first-run setup and interactive authentication flows.
- Adds Claude Code/Codex setup, diagnostics, and skill refresh.
- Updates installers, documentation, and test coverage.
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 44 out of 44 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.surface |
Adds new command surfaces. |
AGENTS.md |
Documents onboarding architecture. |
README.md |
Documents setup and agent integration. |
skills/hey/SKILL.md |
Adds setup command guidance. |
scripts/install.sh |
Adds setup handoff on Unix. |
scripts/install.ps1 |
Adds setup handoff on Windows. |
tests/e2e/installer.bats |
Tests installer setup behavior. |
tests/smoke/auth_test.go |
Tests login/logout shortcuts. |
internal/auth/auth.go |
Adds configurable OAuth progress logging. |
internal/auth/auth_test.go |
Tests OAuth logging. |
internal/config/config.go |
Adds onboarding and noninteractive state. |
internal/config/config_test.go |
Tests new configuration behavior. |
internal/harness/harness.go |
Defines integration health results. |
internal/harness/agent.go |
Adds the agent registry. |
internal/harness/agent_test.go |
Tests registry behavior. |
internal/harness/claude.go |
Adds Claude detection and checks. |
internal/harness/claude_test.go |
Tests Claude integration checks. |
internal/harness/codex.go |
Adds Codex detection and checks. |
internal/harness/codex_test.go |
Tests Codex integration checks. |
internal/tui/brand.go |
Extracts the HEY wordmark. |
internal/tui/prompt.go |
Adds confirmation prompts. |
internal/tui/prompt_test.go |
Tests prompt behavior. |
internal/cmd/auth.go |
Adds shortcuts, greetings, and agent nudges. |
internal/cmd/auth_commands_test.go |
Tests authentication changes. |
internal/cmd/config.go |
Exposes onboarding configuration. |
internal/cmd/doctor.go |
Adds agent diagnostics. |
internal/cmd/formatting.go |
Adds prompt-aware terminal detection. |
internal/cmd/help_test.go |
Updates setup help expectations. |
internal/cmd/local_config_trust.go |
Exempts local-only setup commands. |
internal/cmd/local_config_trust_test.go |
Tests trust-gate exemptions. |
internal/cmd/login.go |
Adds the login shortcut. |
internal/cmd/logout.go |
Adds the logout shortcut. |
internal/cmd/root.go |
Routes onboarding and auth prompts. |
internal/cmd/root_test.go |
Tests root routing and authentication. |
internal/cmd/setup.go |
Implements the setup wizard. |
internal/cmd/setup_test.go |
Tests wizard behavior. |
internal/cmd/setup_agent.go |
Implements per-agent setup. |
internal/cmd/setup_agents.go |
Implements noninteractive agent setup. |
internal/cmd/setup_agents_test.go |
Tests agent selection and setup. |
internal/cmd/skill_install.go |
Extends managed skill installation. |
internal/cmd/skill_install_test.go |
Tests skill replacement safety. |
internal/cmd/skill_refresh.go |
Adds release-based skill refresh. |
internal/cmd/skill_refresh_test.go |
Tests safe skill refresh. |
internal/cmd/tui.go |
Routes TUI through a test seam. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e89a59b472
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
Review of #228 surfaced that ownership was enforced at removal and refresh but not at write: installSkillFiles and installSkillToCodex overwrote an unmarked user-authored skill and then claimed it — reachable automatically through the installer's setup agents handoff — and removeExistingSkillLink's bare os.Remove deleted any user file or symlink before the directory check ever ran. Rather than patch each site, ownership now lives at the single layer that writes: - claimSkillDir gates every skill write (baseline, Codex, copy fallback): create-and-mark a missing or empty directory, accept a marked one, refuse anything else — including symlinks, which would land the write somewhere never inspected. - removeExistingSkillLink starts from Lstat: only our canonical symlink or a marked, allowlisted copy directory is removed; a foreign link, regular file, or unmarked directory errors. - refresh writes only marked, non-symlinked regular files in non-symlinked directories, and skips entirely when ConfigDir() is empty rather than dropping its sentinel in the cwd. - setup codex mirrors the Claude guard: it never fabricates ~/.codex and then counts its own creation as detection. - a styled hey setup <agent> verdicts from a fresh health snapshot and exits nonzero (setup_incomplete) instead of telling the user to start a session against an unconnected integration. - commands that never touch the server (setup agents|claude|codex, skill, …) skip legacy-credential migration, so the installer's HEY_NO_KEYRING=1 handoff can no longer move old config.json tokens into plaintext credentials.json. - doctor warns on an unmanaged baseline skill with the adoption path; pre-marker hey-cli installs adopt via hey skill install. Regression tests pin each refusal: unmarked baseline/Codex skills survive explicit install and setup agents byte-for-byte and unclaimed, foreign links and regular files are refused, refresh never writes through symlinks, the sentinel never lands in the cwd, and legacy credentials migrate for auth status but not for setup agents.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfcf241e36
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
- Narrow the credential-migration exemption to commands that neither use credentials nor rewrite the global config (setup subcommands, skill). The previous predicate exempted all of config, whose Save* helpers decode config.json into a struct with no credential fields — so `hey config set` on a legacy install deleted embedded tokens instead of migrating them. Regression test pins that config set migrates before it rewrites. - The wizard no longer reports a complete, signed-in setup over stored credentials HEY rejects: an auth-coded identity failure becomes a "Stored sign-in rejected" issue with a hey auth login hint (other failures keep the best-effort greeting, so offline setup still works). - setup agents counts an agent connected only when its handler succeeded AND health checks pass. Presence checks alone flipped a refused install (unmanaged skill at the canonical path) back to plugin_installed: true under a "connected" summary. - The refresh sentinel records the active Codex home alongside the version, so switching CODEX_HOME mid-release triggers one rescan instead of leaving the other home's marked skill stale until the next release.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a184800c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8b72235d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
The credential finding was the third variation of config rewrites interacting with legacy embedded secrets, so the fix is at the instrument: saveGlobalConfig now rewrites only the keys the schema owns and carries every unknown key through verbatim — unmigrated legacy credentials and any future version's settings survive every rewrite, including a wizard that persists onboarded after a failed migration. Successful migration scrubs the secret keys explicitly (config.ScrubLegacyCredentials), so the cleanup that used to happen by accident still happens, now on purpose. The rest: - wizardBreadcrumbs joins the checklist on a shared hasAuthIssue predicate, so rejected stored credentials route machine clients to hey auth login instead of hey tui. - The bundled SKILL.md catches up with the hey tui entry point. - The installer's TTY branch treats a wizard the user cancels as a warning plus next steps, not an install failure under set -e. - The skill-refresh notice is suppressed for automatic JSON output (piped stdout), not just explicit machine flags.
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
The credential finding was the third variation of config rewrites interacting with legacy embedded secrets, so the fix is at the instrument: saveGlobalConfig now rewrites only the keys the schema owns and carries every unknown key through verbatim — unmigrated legacy credentials and any future version's settings survive every rewrite, including a wizard that persists onboarded after a failed migration. Successful migration scrubs the secret keys explicitly (config.ScrubLegacyCredentials), so the cleanup that used to happen by accident still happens, now on purpose. The rest: - wizardBreadcrumbs joins the checklist on a shared hasAuthIssue predicate, so rejected stored credentials route machine clients to hey auth login instead of hey tui. - The bundled SKILL.md catches up with the hey tui entry point. - The installer's TTY branch treats a wizard the user cancels as a warning plus next steps, not an install failure under set -e. - The skill-refresh notice is suppressed for automatic JSON output (piped stdout), not just explicit machine flags.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b922a0f6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
Review of #228 surfaced that ownership was enforced at removal and refresh but not at write: installSkillFiles and installSkillToCodex overwrote an unmarked user-authored skill and then claimed it — reachable automatically through the installer's setup agents handoff — and removeExistingSkillLink's bare os.Remove deleted any user file or symlink before the directory check ever ran. Rather than patch each site, ownership now lives at the single layer that writes: - claimSkillDir gates every skill write (baseline, Codex, copy fallback): create-and-mark a missing or empty directory, accept a marked one, refuse anything else — including symlinks, which would land the write somewhere never inspected. - removeExistingSkillLink starts from Lstat: only our canonical symlink or a marked, allowlisted copy directory is removed; a foreign link, regular file, or unmarked directory errors. - refresh writes only marked, non-symlinked regular files in non-symlinked directories, and skips entirely when ConfigDir() is empty rather than dropping its sentinel in the cwd. - setup codex mirrors the Claude guard: it never fabricates ~/.codex and then counts its own creation as detection. - a styled hey setup <agent> verdicts from a fresh health snapshot and exits nonzero (setup_incomplete) instead of telling the user to start a session against an unconnected integration. - commands that never touch the server (setup agents|claude|codex, skill, …) skip legacy-credential migration, so the installer's HEY_NO_KEYRING=1 handoff can no longer move old config.json tokens into plaintext credentials.json. - doctor warns on an unmanaged baseline skill with the adoption path; pre-marker hey-cli installs adopt via hey skill install. Regression tests pin each refusal: unmarked baseline/Codex skills survive explicit install and setup agents byte-for-byte and unclaimed, foreign links and regular files are refused, refresh never writes through symlinks, the sentinel never lands in the cwd, and legacy credentials migrate for auth status but not for setup agents.
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
- Narrow the credential-migration exemption to commands that neither use credentials nor rewrite the global config (setup subcommands, skill). The previous predicate exempted all of config, whose Save* helpers decode config.json into a struct with no credential fields — so `hey config set` on a legacy install deleted embedded tokens instead of migrating them. Regression test pins that config set migrates before it rewrites. - The wizard no longer reports a complete, signed-in setup over stored credentials HEY rejects: an auth-coded identity failure becomes a "Stored sign-in rejected" issue with a hey auth login hint (other failures keep the best-effort greeting, so offline setup still works). - setup agents counts an agent connected only when its handler succeeded AND health checks pass. Presence checks alone flipped a refused install (unmanaged skill at the canonical path) back to plugin_installed: true under a "connected" summary. - The refresh sentinel records the active Codex home alongside the version, so switching CODEX_HOME mid-release triggers one rescan instead of leaving the other home's marked skill stale until the next release.
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
The credential finding was the third variation of config rewrites interacting with legacy embedded secrets, so the fix is at the instrument: saveGlobalConfig now rewrites only the keys the schema owns and carries every unknown key through verbatim — unmigrated legacy credentials and any future version's settings survive every rewrite, including a wizard that persists onboarded after a failed migration. Successful migration scrubs the secret keys explicitly (config.ScrubLegacyCredentials), so the cleanup that used to happen by accident still happens, now on purpose. The rest: - wizardBreadcrumbs joins the checklist on a shared hasAuthIssue predicate, so rejected stored credentials route machine clients to hey auth login instead of hey tui. - The bundled SKILL.md catches up with the hey tui entry point. - The installer's TTY branch treats a wizard the user cancels as a warning plus next steps, not an install failure under set -e. - The skill-refresh notice is suppressed for automatic JSON output (piped stdout), not just explicit machine flags.
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
- Read-side health now shares the write-side file-shape rule: a new harness.RegularSkillFile (Lstat, regular file) backs both agent skill checks and baselineSkillInstalled, so a symlinked SKILL.md — the state every write path refuses and refresh skips — is reported as a failing check instead of a healthy install the wizard shortcuts past. - Machine-mode hey setup claude|codex returns an error envelope (setup_incomplete, manual steps in the hint) with a nonzero exit when the integration did not connect. ok:true with plugin_installed:false was in-band honesty that automation keying off the exit status never saw. The aggregate setup agents keeps its in-band contract: its core outcome is the skill, and per-agent detail rides in the envelope. - setup agents synthesizes missing-binary remediation only when the absence actually prevented the connection — Codex is routinely detected by its home directory alone, and its skill-only setup was drawing a warning that told the user to rerun what had just worked. - The two new gosec findings get the house #nosec annotations with justifications: fixed skill locations under the user's own home, gated by claimSkillDir and Lstat.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c72b07bc9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
added a commit
that referenced
this pull request
Aug 21, 2026
- baselineSkillInstalled requires the ownership marker as well as a regular file — the final presence predicate aligned with the rules every write path enforces, so an unmarked hand-authored baseline is refused by install AND never reported as a healthy installation. - The agent-local setup subcommands (and skill) enroll in #225's commandIgnoresLocalConfig path: they load the global config only, degrading to defaults, so a malformed repository .hey/config.json cannot stop the installer's setup agents handoff before the trust exemption is even consulted. Runtime-config commands still surface the parse error. - install.ps1 gets the same optional-setup guard install.sh got: a wizard that exits nonzero (declined OAuth, timeout, busy callback port) prints next steps instead of failing or silently swallowing a completed install, with $LASTEXITCODE checked because Windows PowerShell 5.1 does not throw on native nonzero exits.
After a verified install, install.sh runs `hey setup` when stdin and stdout are a terminal; otherwise (CI, piped input, coding-agent shells, HEY_SKIP_SETUP=1) it runs the non-interactive `hey setup agents` and prints next steps. post_install_setup capability-probes `setup --help` so the hosted script stays correct against older release binaries: without `setup agents` it connects only an explicitly selected agent (HEY_SETUP_AGENT), capability-checked per agent so an unadvertised id is never handed to an old parent as a stray arg, and otherwise installs the shared skill. Every real call carries HEY_NO_KEYRING=1 per command so a locked headless keychain cannot wedge the installer; the help probe stays bare. install.ps1 mirrors this with Invoke-PostInstallSetup and a Test-InteractiveSession gate, restoring HEY_NO_KEYRING on the way out. installer.bats replaces the old "no interactive setup" pin with the ported contract: stub binaries logging argv, new/old binary dispatch, selector fallbacks, the keyring belt, and the ps1 helper evaluated from its AST.
interactiveStdio() honored the escape hatch, but the wizard branched on styled output alone: on a real PTY with HEY_NONINTERACTIVE=1 it still ran tui.Confirm (eating a keystroke and emitting Bubble Tea control traffic) and, logged out, parked in the six-minute OAuth wait — exactly what the README promised it would not do. Sign-in now runs only when it can be seen through: stdin is a terminal and HEY_NONINTERACTIVE is off (deliberately looser than interactiveStdio() so `hey setup --json` from a terminal still signs in with progress on stderr). The agent-setup prompt runs only under interactiveStdio(); otherwise the wizard proceeds with the prompt's default answer, matching the machine-mode contract. The prompt moves behind a confirmAgentSetup seam so the boundary is testable, and AGENTS.md's routing paragraph catches up with the hey tui contract.
The installer's non-TTY handoff runs `hey setup agents` from whatever directory curl was piped in — possibly a repository with an untrusted .hey/config.json — and the trust gate was failing it before the skill ever installed, a failure install.sh's best-effort `|| true` then swallowed without a trace. setup agents|claude|codex only touch local agent files, never the effective server or account, so they now skip the gate the same way skill/upgrade/version do. The wizard itself signs in against the effective server and stays gated.
After a symlink failure the copy fallback leaves a real, populated directory at ~/.claude/skills/hey, and the next install died trying to os.Remove it before ever reaching the fallback again. Removal now recognizes exactly the shape our fallback writes — a plain directory holding only SKILL.md and its version stamp — and replaces it; any other populated directory is user content and still refused, as the existing regression pins. The symlink call moves behind a seam so two consecutive fallback installs run as a real test.
Shape was standing in for ownership: any directory holding only a SKILL.md matched the copy-fallback profile and could be RemoveAll'd, so a user's hand-authored ~/.claude/skills/hey was deleted and replaced by a symlink. The release refresh was broader still — it rewrote every existing Claude/Codex SKILL.md at the known paths without proving hey-cli wrote them, so merely running any command on a new release clobbered a custom skill. Every hey-cli install now stamps its directory with a .managed-by-hey-cli marker (the copy fallback claims its freshly created directory up front, so a partial copy stays recognizable and replaceable). Replacement requires the marker AND the known-files allowlist; automatic refresh skips any location whose directory lacks the marker, treating foreign skills as finished state — the sentinel still advances, no retry loop — and stamps the baseline version only when that directory is ours. Pre-marker installs are adopted the next time hey skill install or the setup wizard runs. Boundary tests pin both adversarial cases: an unmarked directory holding only SKILL.md is refused and preserved by install, and a synthetic release refresh leaves unmarked baseline/Claude/Codex skills byte-for-byte alone.
The post-upgrade refresh treated every dangling ~/.claude/skills/hey symlink as ours and relinked it to the baseline, so a user's link to a temporarily unmounted volume was silently destroyed by the first command run on a new release — even with a genuinely marked baseline beside it. Repair now requires provenance on both ends: os.Readlink must equal the exact canonical relative target hey-cli writes, and the baseline it points at must carry the ownership marker. Every other broken link is preserved. The old test blessed an arbitrary dangling target; it is replaced by the provenance table (canonical over marked, canonical over unmarked, foreign target, absolute spelling of the same place) plus end-to-end pins that a foreign broken link survives the refresh and our canonical link passes through it intact.
With provenance required on both ends the repair could never fire: a canonical link over a marked baseline is by definition healthy, and a missing baseline has no marker. Safety was correct, but a function named repair that cannot repair misstates what the refresh does. Refresh now touches files it owns and nothing else; the end-to-end pins stay — a foreign dangling link survives, and our canonical link passes through intact.
Review of #228 surfaced that ownership was enforced at removal and refresh but not at write: installSkillFiles and installSkillToCodex overwrote an unmarked user-authored skill and then claimed it — reachable automatically through the installer's setup agents handoff — and removeExistingSkillLink's bare os.Remove deleted any user file or symlink before the directory check ever ran. Rather than patch each site, ownership now lives at the single layer that writes: - claimSkillDir gates every skill write (baseline, Codex, copy fallback): create-and-mark a missing or empty directory, accept a marked one, refuse anything else — including symlinks, which would land the write somewhere never inspected. - removeExistingSkillLink starts from Lstat: only our canonical symlink or a marked, allowlisted copy directory is removed; a foreign link, regular file, or unmarked directory errors. - refresh writes only marked, non-symlinked regular files in non-symlinked directories, and skips entirely when ConfigDir() is empty rather than dropping its sentinel in the cwd. - setup codex mirrors the Claude guard: it never fabricates ~/.codex and then counts its own creation as detection. - a styled hey setup <agent> verdicts from a fresh health snapshot and exits nonzero (setup_incomplete) instead of telling the user to start a session against an unconnected integration. - commands that never touch the server (setup agents|claude|codex, skill, …) skip legacy-credential migration, so the installer's HEY_NO_KEYRING=1 handoff can no longer move old config.json tokens into plaintext credentials.json. - doctor warns on an unmanaged baseline skill with the adoption path; pre-marker hey-cli installs adopt via hey skill install. Regression tests pin each refusal: unmarked baseline/Codex skills survive explicit install and setup agents byte-for-byte and unclaimed, foreign links and regular files are refused, refresh never writes through symlinks, the sentinel never lands in the cwd, and legacy credentials migrate for auth status but not for setup agents.
- Narrow the credential-migration exemption to commands that neither use credentials nor rewrite the global config (setup subcommands, skill). The previous predicate exempted all of config, whose Save* helpers decode config.json into a struct with no credential fields — so `hey config set` on a legacy install deleted embedded tokens instead of migrating them. Regression test pins that config set migrates before it rewrites. - The wizard no longer reports a complete, signed-in setup over stored credentials HEY rejects: an auth-coded identity failure becomes a "Stored sign-in rejected" issue with a hey auth login hint (other failures keep the best-effort greeting, so offline setup still works). - setup agents counts an agent connected only when its handler succeeded AND health checks pass. Presence checks alone flipped a refused install (unmanaged skill at the canonical path) back to plugin_installed: true under a "connected" summary. - The refresh sentinel records the active Codex home alongside the version, so switching CODEX_HOME mid-release triggers one rescan instead of leaving the other home's marked skill stale until the next release.
Codex's third review round showed two classes still leaking at individual sites, so the fixes move to the layer each class lives at rather than patching the sites: - Skill health checks now require the ownership marker. Presence was standing in for health, which is why every consumer — the wizard, the setup agents record, the per-agent verdict — kept needing its own handler-success patch: an unmanaged skill at the canonical path passed the check that all of them share. With the check itself reporting "occupied by a skill hey-cli did not write", every current and future consumer verdicts correctly. The wizard additionally records a handler refusal as its own issue for the rare failure that leaves no failing check. - Every skill file write goes through writeSkillFile, which Lstat-refuses non-regular files: a symlinked SKILL.md planted inside a marked directory can no longer be followed and truncated by install, wizard, handoff, or the version stamp — matching the guard refresh already had. - interactiveStdio requires stderr to be a terminal too: prompts render on stderr, and hey boxes 2>log at a TTY was waiting on a prompt the user could not see. Redirected stderr now takes the documented non-interactive auth-error path. - The styled summary derives "Signed in" from the auth issues, so it can no longer show a green check beside "Stored sign-in rejected".
The credential finding was the third variation of config rewrites interacting with legacy embedded secrets, so the fix is at the instrument: saveGlobalConfig now rewrites only the keys the schema owns and carries every unknown key through verbatim — unmigrated legacy credentials and any future version's settings survive every rewrite, including a wizard that persists onboarded after a failed migration. Successful migration scrubs the secret keys explicitly (config.ScrubLegacyCredentials), so the cleanup that used to happen by accident still happens, now on purpose. The rest: - wizardBreadcrumbs joins the checklist on a shared hasAuthIssue predicate, so rejected stored credentials route machine clients to hey auth login instead of hey tui. - The bundled SKILL.md catches up with the hey tui entry point. - The installer's TTY branch treats a wizard the user cancels as a warning plus next steps, not an install failure under set -e. - The skill-refresh notice is suppressed for automatic JSON output (piped stdout), not just explicit machine flags.
- Read-side health now shares the write-side file-shape rule: a new harness.RegularSkillFile (Lstat, regular file) backs both agent skill checks and baselineSkillInstalled, so a symlinked SKILL.md — the state every write path refuses and refresh skips — is reported as a failing check instead of a healthy install the wizard shortcuts past. - Machine-mode hey setup claude|codex returns an error envelope (setup_incomplete, manual steps in the hint) with a nonzero exit when the integration did not connect. ok:true with plugin_installed:false was in-band honesty that automation keying off the exit status never saw. The aggregate setup agents keeps its in-band contract: its core outcome is the skill, and per-agent detail rides in the envelope. - setup agents synthesizes missing-binary remediation only when the absence actually prevented the connection — Codex is routinely detected by its home directory alone, and its skill-only setup was drawing a warning that told the user to rerun what had just worked. - The two new gosec findings get the house #nosec annotations with justifications: fixed skill locations under the user's own home, gated by claimSkillDir and Lstat.
- baselineSkillInstalled requires the ownership marker as well as a regular file — the final presence predicate aligned with the rules every write path enforces, so an unmarked hand-authored baseline is refused by install AND never reported as a healthy installation. - The agent-local setup subcommands (and skill) enroll in #225's commandIgnoresLocalConfig path: they load the global config only, degrading to defaults, so a malformed repository .hey/config.json cannot stop the installer's setup agents handoff before the trust exemption is even consulted. Runtime-config commands still surface the parse error. - install.ps1 gets the same optional-setup guard install.sh got: a wizard that exits nonzero (declined OAuth, timeout, busy callback port) prints next steps instead of failing or silently swallowing a completed install, with $LASTEXITCODE checked because Windows PowerShell 5.1 does not throw on native nonzero exits.
- The installer never invokes an old binary's legacy setup commands. hey's history differs from basecamp-cli's here: every release that predates setup agents also predates the ownership gate on skill install, so the ported cross-version fallback always dispatched into an implementation that overwrites whatever occupies the skill paths. The scaffolding guarded releases that never existed — removed rather than armored. Old binaries get the printed next steps; only the ownership-aware setup agents is ever run automatically. install.ps1 matches, and the bats contract now pins never-invoke-legacy across every selector. - The ownership marker itself must be a regular file: SkillDirOwned goes through RegularSkillFile (and cmd's duplicate predicate collapses onto it), so a symlink or directory planted in the marker's name confers no ownership and the unmanaged-content refusal still protects the SKILL.md next to it. - doctor reports presence and health separately again: an occupied but unmanaged path gets the move-aside remediation instead of "Not installed" with a hint that would refuse. - setup, setup agents and setup <agent> reject --ids-only/--count up front: a wizard result is not list data, and the writer's late refusal used to land after OAuth, agent installs and the onboarded flag.
…back Two remaining name-based reads in the copy-fallback path, both closed with the rules the class already established: - isManagedSkillCopy requires every entry to be a regular file, so a symlink planted in the marker's name (or any other allowlisted name) authorizes no deletion and the user's SKILL.md beside it survives Claude setup. - copySkillFiles copies only the files hey-cli owns (SKILL.md, version stamp, marker). A user file added to the managed baseline no longer rides into the Claude copy, where it would fail the copy's own allowlist on the next run and strand the fallback.
…rite
- A rejected HEY_TOKEN now points at the environment ("Update or unset
HEY_TOKEN") instead of hey auth login, which the env token outranks —
following the old hint repeated the same failure forever.
- The refresh sentinel is written through writeSkillFile, the same
no-follow rule as every other file this feature writes, so a symlink
planted at ~/.config/hey-cli/.last-run-version is never truncated.
- The wizard's structured breadcrumbs point a rejected HEY_TOKEN at the environment (unset HEY_TOKEN), completing the round-nine hint fix — the env token outranks anything hey auth login saves, so the old breadcrumb looped agents forever. - The bundled skill stops promising that --json never prompts: with a terminal on stdin it deliberately starts browser sign-in, so agents on allocated PTYs are told to use HEY_NONINTERACTIVE=1 for status-only setup. - pluginInstalled no longer falls back to raw text search when installed_plugins.json is unparseable: Claude itself cannot read a malformed registry, so its raw bytes prove nothing and the check reports not installed instead of shortcutting the wizard past a broken integration.
Same correction as the bundled skill: --json changes only the output format, and a terminal on stdin (allocated PTYs included) still starts browser OAuth. Agents are told to set HEY_NONINTERACTIVE=1 for a status-only run.
Review arbitration on the three open judgment calls: the bare-hey credential-validation and symlinked-directory declines stand, the scrub-retry decline was overruled. - migrateOldCredentials no longer returns blind when the store already loads: with a usable stored credential (access token or session cookie present) it retries the scrub, so a past save-success/ scrub-failure cannot strand plaintext secrets in config.json forever. An empty stored record authorizes nothing — the legacy fields stay recoverable. Regression covers scrubbed secrets, a surviving unrelated future key, and byte-identical stored credentials. - The RegularSkillFile and baselineSkillInstalled comments now state the deliberate bound instead of overclaiming symmetry: intermediate directory symlinks resolve, so a user-symlinked skill directory can pass reads that every write, removal and refresh still refuses.
Follow-ups to the last three changes, each a first-order consequence: - The origin guard the scrub-retry needed applies to the whole migration: legacy config.json credentials carry their own server, and with HEY_BASE_URL pointed elsewhere the current store key proves nothing about them — migrating would misfile them under the wrong origin and scrubbing would delete the only copy for their real one. Migration now engages only when the legacy origin matches the effective credential key; foreign-origin fields ride along untouched (rewrites already preserve them). - A refresh whose sentinel cannot be written is not complete: it now reports no completed refresh, so the update notice cannot repeat on every subsequent command from an unwritable config dir. - The wizard docs stop calling HEY_NONINTERACTIVE=1 status-only: it suppresses OAuth and prompts but still connects detected agents and persists onboarded; hey doctor is the inspection path.
- A config.json containing literal JSON null decodes to a nil map without error, and the preserve-unknown-keys merge then panicked on assignment, crashing every config-writing path. The map is reinitialized after decoding. - linkSkillToClaude requires a healthy managed baseline before creating the link: setup paths that recorded installSkillFiles' refusal used to continue into the link step and point ~/.claude/skills/hey at the very content the install declined to claim.
monorkin
pushed a commit
that referenced
this pull request
Aug 21, 2026
Review of #228 surfaced that ownership was enforced at removal and refresh but not at write: installSkillFiles and installSkillToCodex overwrote an unmarked user-authored skill and then claimed it — reachable automatically through the installer's setup agents handoff — and removeExistingSkillLink's bare os.Remove deleted any user file or symlink before the directory check ever ran. Rather than patch each site, ownership now lives at the single layer that writes: - claimSkillDir gates every skill write (baseline, Codex, copy fallback): create-and-mark a missing or empty directory, accept a marked one, refuse anything else — including symlinks, which would land the write somewhere never inspected. - removeExistingSkillLink starts from Lstat: only our canonical symlink or a marked, allowlisted copy directory is removed; a foreign link, regular file, or unmarked directory errors. - refresh writes only marked, non-symlinked regular files in non-symlinked directories, and skips entirely when ConfigDir() is empty rather than dropping its sentinel in the cwd. - setup codex mirrors the Claude guard: it never fabricates ~/.codex and then counts its own creation as detection. - a styled hey setup <agent> verdicts from a fresh health snapshot and exits nonzero (setup_incomplete) instead of telling the user to start a session against an unconnected integration. - commands that never touch the server (setup agents|claude|codex, skill, …) skip legacy-credential migration, so the installer's HEY_NO_KEYRING=1 handoff can no longer move old config.json tokens into plaintext credentials.json. - doctor warns on an unmanaged baseline skill with the adoption path; pre-marker hey-cli installs adopt via hey skill install. Regression tests pin each refusal: unmarked baseline/Codex skills survive explicit install and setup agents byte-for-byte and unclaimed, foreign links and regular files are refused, refresh never writes through symlinks, the sentinel never lands in the cwd, and legacy credentials migrate for auth status but not for setup agents.
monorkin
pushed a commit
that referenced
this pull request
Aug 21, 2026
- Narrow the credential-migration exemption to commands that neither use credentials nor rewrite the global config (setup subcommands, skill). The previous predicate exempted all of config, whose Save* helpers decode config.json into a struct with no credential fields — so `hey config set` on a legacy install deleted embedded tokens instead of migrating them. Regression test pins that config set migrates before it rewrites. - The wizard no longer reports a complete, signed-in setup over stored credentials HEY rejects: an auth-coded identity failure becomes a "Stored sign-in rejected" issue with a hey auth login hint (other failures keep the best-effort greeting, so offline setup still works). - setup agents counts an agent connected only when its handler succeeded AND health checks pass. Presence checks alone flipped a refused install (unmanaged skill at the canonical path) back to plugin_installed: true under a "connected" summary. - The refresh sentinel records the active Codex home alongside the version, so switching CODEX_HOME mid-release triggers one rescan instead of leaving the other home's marked skill stale until the next release.
monorkin
pushed a commit
that referenced
this pull request
Aug 21, 2026
The credential finding was the third variation of config rewrites interacting with legacy embedded secrets, so the fix is at the instrument: saveGlobalConfig now rewrites only the keys the schema owns and carries every unknown key through verbatim — unmigrated legacy credentials and any future version's settings survive every rewrite, including a wizard that persists onboarded after a failed migration. Successful migration scrubs the secret keys explicitly (config.ScrubLegacyCredentials), so the cleanup that used to happen by accident still happens, now on purpose. The rest: - wizardBreadcrumbs joins the checklist on a shared hasAuthIssue predicate, so rejected stored credentials route machine clients to hey auth login instead of hey tui. - The bundled SKILL.md catches up with the hey tui entry point. - The installer's TTY branch treats a wizard the user cancels as a warning plus next steps, not an install failure under set -e. - The skill-refresh notice is suppressed for automatic JSON output (piped stdout), not just explicit machine flags.
monorkin
pushed a commit
that referenced
this pull request
Aug 21, 2026
- Read-side health now shares the write-side file-shape rule: a new harness.RegularSkillFile (Lstat, regular file) backs both agent skill checks and baselineSkillInstalled, so a symlinked SKILL.md — the state every write path refuses and refresh skips — is reported as a failing check instead of a healthy install the wizard shortcuts past. - Machine-mode hey setup claude|codex returns an error envelope (setup_incomplete, manual steps in the hint) with a nonzero exit when the integration did not connect. ok:true with plugin_installed:false was in-band honesty that automation keying off the exit status never saw. The aggregate setup agents keeps its in-band contract: its core outcome is the skill, and per-agent detail rides in the envelope. - setup agents synthesizes missing-binary remediation only when the absence actually prevented the connection — Codex is routinely detected by its home directory alone, and its skill-only setup was drawing a warning that told the user to rerun what had just worked. - The two new gosec findings get the house #nosec annotations with justifications: fixed skill locations under the user's own home, gated by claimSkillDir and Lstat.
monorkin
pushed a commit
that referenced
this pull request
Aug 21, 2026
- baselineSkillInstalled requires the ownership marker as well as a regular file — the final presence predicate aligned with the rules every write path enforces, so an unmarked hand-authored baseline is refused by install AND never reported as a healthy installation. - The agent-local setup subcommands (and skill) enroll in #225's commandIgnoresLocalConfig path: they load the global config only, degrading to defaults, so a malformed repository .hey/config.json cannot stop the installer's setup agents handoff before the trust exemption is even consulted. Runtime-config commands still surface the parse error. - install.ps1 gets the same optional-setup guard install.sh got: a wizard that exits nonzero (declined OAuth, timeout, busy callback port) prints next steps instead of failing or silently swallowing a completed install, with $LASTEXITCODE checked because Windows PowerShell 5.1 does not throw on native nonzero exits.
monorkin
pushed a commit
that referenced
this pull request
Aug 21, 2026
- The installer never invokes an old binary's legacy setup commands. hey's history differs from basecamp-cli's here: every release that predates setup agents also predates the ownership gate on skill install, so the ported cross-version fallback always dispatched into an implementation that overwrites whatever occupies the skill paths. The scaffolding guarded releases that never existed — removed rather than armored. Old binaries get the printed next steps; only the ownership-aware setup agents is ever run automatically. install.ps1 matches, and the bats contract now pins never-invoke-legacy across every selector. - The ownership marker itself must be a regular file: SkillDirOwned goes through RegularSkillFile (and cmd's duplicate predicate collapses onto it), so a symlink or directory planted in the marker's name confers no ownership and the unmanaged-content refusal still protects the SKILL.md next to it. - doctor reports presence and health separately again: an occupied but unmanaged path gets the move-aside remediation instead of "Not installed" with a hint that would refuse. - setup, setup agents and setup <agent> reject --ids-only/--count up front: a wizard result is not list data, and the writer's late refusal used to land after OAuth, agent installs and the onboarded flag.
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.
Ports basecamp-cli's onboarding flow so a first run of
heyis seamless instead ofError: not logged in.Behavior
heyat an interactive terminal, logged out → the setup wizard (welcome → OAuth sign-in → linked-account greeting → coding-agent setup → summary). Once onboarded, a later logged-out run gets the lite wizard (sign-in only). Every other bareheyprints help, preserving main'shey tuicontract (#8d3e675).hey setupalways runs the full wizard.--jsonnever prompts; logged out without a terminal reportsstatus: incompletewith ahey auth loginbreadcrumb, so the piped installer can never hang on a browser.hey setup claude|codex|agentsbacked by a newinternal/harnessagent registry. Claude gets thehey@37signalsplugin frombasecamp/claude-pluginsplus a skill link; Codex gets the skill only until a.codex-pluginships.setup agentsis the installer's non-interactive path:HEY_SETUP_AGENT=claude|codex|all|none, ambiguity connects nobody.hey login/hey logoutshortcuts; OAuth login greets by identity and nudges towardhey setup <agent>when a detected agent is unhealthy.requireAuth()at an interactive terminal asks "Not logged in. Sign in now?" and continues after OAuth; piped, declined or machine-output runs getError: Not logged in/Run: hey auth login, exit 3.hey doctorgains baseline-skill and per-agent diagnostics; aPersistentPostRunEhook re-syncs installed skill copies once per release version.hey setupon a TTY and tohey setup agentsotherwise (HEY_SKIP_SETUP,HEY_SETUP_AGENT, per-commandHEY_NO_KEYRING=1);install.ps1mirrors it.HEY_NONINTERACTIVE=1disables every prompt, including on a PTY.Safety properties worth reviewing
~/.claude(hey setup claudeon a machine without Claude refuses)..managed-by-hey-climarker. Replacement requires the marker plus a known-files allowlist; the automatic refresh skips anything unmarked and never touches symlinks. A user-authored~/.claude/skills/hey— even one that is a singleSKILL.md— is preserved byte-for-byte.setup agents|claude|codexskip the local-config trust gate (they never touch the server); the wizard itself stays gated.Verification
make check, fullgo test ./..., all 40 installer bats (incl. the pwsh AST-extracted ones),.surfaceadditions only. Manual matrix and PTY probes against the built binary are in the commit messages. Not yet verified: live OAuth andmake test-smoke— the dev server atapp.hey.localhost:3003was unreachable throughout; that is the final merge gate.Follow-ups deliberately out of scope: wordmark animation,
.codex-plugin+ native Codex plugin install, a quick-start JSON envelope for non-TTY barehey.Summary by cubic
Ports basecamp-cli’s onboarding so first run guides setup instead of “Not logged in,” adds a coding‑agent harness and
hey login/hey logout, and hardens agent skill install/refresh and credential migration for safety and idempotence.hey: interactive and logged out → setup wizard; logged in → TUI. Non‑interactive or machine output never prompts.HEY_NONINTERACTIVE=1suppresses prompts and OAuth but still connects detected agents and setsonboarded. Prompts require stdin, stdout, and stderr TTYs.hey setup: always runs the full wizard.--jsonkeeps structured output and may start browser sign‑in when stdin is a TTY; setHEY_NONINTERACTIVE=1to avoid OAuth.hey setup agents|claude|codex: non‑interactive viainternal/harness; an agent connects only if its handler succeeded and health checks pass. They never fabricate~/.claudeor~/.codex, load only global config (skip the local‑config trust gate), and reject--ids-only/--count. In machine modesetup claude|codexexits non‑zero when the integration did not connect;setup agentsstays envelope‑only and synthesizes a missing‑binary hint only when absence blocked connection. Linking Claude now requires a healthy, managed baseline.hey login/hey logoutare shortcuts forhey auth login/logout. At an interactive terminal,requireAuth()offers to sign in and continues after OAuth; piped, declined, or machine‑output runs fail with “Not logged in” (exit 3) and a targeted hint (“hey auth login” or “Update or unset HEY_TOKEN”).internal/auth.LoginOptions.Loggercan route OAuth progress output.internal/harness. Unmanaged skills at canonical paths and symlinkedSKILL.mdfiles fail health with clear hints; baseline health requires the ownership marker. Claude plugin detection readsinstalled_plugins.jsononly — a malformed registry reports “not installed.”.managed-by-hey-climarker; the CLI refuses to write into unmarked or symlinked directories, never writes through symlinked files, and only writes regular files. Replacement requires provenance (only our canonical Claude symlink or a marked, allow‑listed copy directory is removed). Automatic refresh updates only marked, non‑symlinked regular files, tracks the active Codex home, skips entirely when no config dir is available, prints its notice only when the refresh completed, and suppresses that notice for automatic JSON output. Reads may traverse user‑symlinked directories; writes never follow symlinks.hey setup; otherwise it runshey setup agents. It respectsHEY_SKIP_SETUPandHEY_SETUP_AGENT, and setsHEY_NO_KEYRING=1on invoked commands. A canceled or non‑zero wizard prints next steps and does not fail. It never invokes legacy setup commands on old binaries; those get printed next steps. Windowsscripts/install.ps1mirrors this behavior.onboardedflag andhey config set onboarded=true|false. Only setup subcommands andhey skillskip legacy‑credential migration;hey config setmigrates before rewriting and preserves unknown keys (includingcover). Migration runs only when the legacy credential’s origin matches the effective credential key; successful migration scrubs legacy secrets (retried when a usable stored credential already loads). A literal nullconfig.jsonno longer crashes writes.tui.Watchersthrough therunTUIseam and fixes tests; config rewrites now round‑tripcover.Written for commit 13343ff. Summary will update on new commits.