fix: v2.5.0 bug fixes across listen, config, and validation by leggetter · Pull Request #342 · hookdeck/hookdeck-cli

leggetter · 2026-08-13T10:10:11Z

Fixes #331, #332, #333, #334, #335, #336, #337, #338, #339.

Six triaged issues plus three found while scoping them. They share one failure shape: the CLI silently does something other than what the caller asked, and the first symptom is missing traffic rather than an error. Four were found by running the CLI as an agent would, via hookdeck/evals and a Hermes agent plugin.

Design rule applied throughout

A prompt must never be the only path. Without a terminal, reads pick the safe default and writes fail loudly — never silently succeed differently. Rendering degrades rather than dies. Exit codes tell the truth.

Changes

Issue	Was	Now
#332	`ci --local` / `login --local` wrote the global config as well as the local one, silently repointing the machine's active project	`--local` redirects the write instead of adding one
#333	`listen` built the interactive renderer, which opens `/dev/tty`, and died at startup in CI/Docker/agents	Falls back to `--output compact` when stdout is not a terminal
#339	A renderer failure was logged and discarded — `listen` exited 0 with no tunnel	Renderer failures propagate; normal quit still exits 0
#334	`listen` ignored `HOOKDECK_API_KEY` and silently created a guest account	Exchanges it for CLI credentials and saves them, as `hookdeck ci` does
#335	`--webhook-secret ""` was accepted, creating a source with no verification at all	Explicitly empty secret and identity flags are rejected
#337	Any unauthenticated command dropped into browser sign-in and hung ~4 minutes	Gated on a terminal; reports how to authenticate without one
#338	Five delete/dismiss commands exited 0 having deleted nothing without a terminal	Fail non-zero and name `--force`
#331	`go-github` v28 linked `x/crypto/openpgp` (GO-2026-5932, no upstream fix)	Replaced with a `net/http` call; `govulncheck` job added
#336	Release skill's CI gate always reported `pending`, blocking every release	Uses GraphQL `statusCheckRollup`; skill deduplicated into a symlink

Two worth reading closely

#334 is not the one-line fix the issue proposed. HOOKDECK_API_KEY holds a Project API key. Verified against the live API: it gets 401 from GET /2025-07-01/cli-auth/validate but 200 from GET /2025-07-01/connections. Defaulting the persistent --api-key flag from the env var would have swapped a silent guest account for a silent 401. The fix performs the same POST /cli-auth/ci exchange hookdeck ci does, then persists — one round trip per machine, not per invocation. Precedence: --cli-key → stored login → HOOKDECK_API_KEY → guest, so nobody already signed in gets repointed.

#335 loses no capability. An empty secret never cleared anything: SourceConfigStripeAuth is nullable: true with webhook_secret_key required when present, so the API expresses "no verification" as a null auth object. Meanwhile sourceConfigFlags.hasAny() is a pure != "" test, so the value was dropped entirely — on create the source got no auth at all. Clearing remains available via --config '{"auth": null}', untouched here. Identity flags (--name, --type, --url) are included because Cobra's MarkFlagRequired tests Changed, not the value, so --name "" passed too.

Testing

Unit: new tests for every change. pkg/version getLatestVersion was previously untested — the old code hardcoded github.NewClient(nil) with no injectable transport; it now runs against httptest.
Acceptance: new tests under existing feature tags (source, connection_upsert, listen, project_use), so the CI matrix needs no changes. The listen: detect a non-interactive environment and fall back to compact output #333 and Destructive commands exit 0 without deleting anything when there is no terminal #338 tests fail on current main.
Verified live against the API: the Reject an empty --source-webhook-secret instead of creating a source that can never verify #335 evals commands now error; listen with HOOKDECK_API_KEY lands in the right project with no guest notice; source delete without --force exits 1 and leaves the source intact; the corrected CI gate returns SUCCESS where the old one returned pending (total_count=0).
govulncheck no longer reports GO-2026-5932.

Docs

README § Running in CI showed a full-screen TUI transcript as expected CI output while documenting HOOKDECK_API_KEY as the way to authenticate — the exact combination #333 and #334 disprove. Updated, along with listen help and REFERENCE.md.

Not included

Deliberately left for follow-ups, both filed with full findings: #340 (agent-readiness epic — output contract, remaining interactive surfaces, env var coverage) and #341 (pkg/cmd/sources is dead code: source auth validation never runs, because the parser targets schema paths the API no longer publishes and returns an empty map with a nil error).

🤖 Generated with Claude Code

https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

Six triaged issues plus three related bugs found while scoping them. They share one failure shape: the CLI silently does something other than what the caller asked, and the first symptom is missing traffic rather than an error. Design rule applied throughout: a prompt must never be the only path. Without a terminal, reads pick the safe default and writes fail loudly; rendering degrades rather than dies; exit codes tell the truth. - #332 `ci --local` / `login --local` no longer write the global config. The login flows saved the profile before --local was inspected, so the flag added a second write instead of redirecting the first, silently repointing the machine's active project. Gated at the single writeConfig chokepoint. - #333 `listen` falls back to compact output when stdout is not a terminal. The interactive renderer opens /dev/tty itself, so in CI, Docker, nohup or an AI agent it died at startup and the command exited 0 having forwarded nothing. Renderer failures now propagate as a non-zero exit rather than being logged and discarded. - #334 `listen` honours HOOKDECK_API_KEY. It previously fell through to a guest account: traffic arrived locally so it looked like it worked, but none of it was in the caller's project. HOOKDECK_API_KEY holds a Project API key, which /cli-auth/validate rejects with 401, so it is exchanged for a CLI client key via POST /cli-auth/ci exactly as `hookdeck ci` does, then saved. Precedence: --cli-key, stored login, HOOKDECK_API_KEY, guest. - #335 Explicitly empty secret and identity flags are rejected. `"$UNSET_VAR"` expands to "", and hasAny() is a pure != "" test, so the value was dropped and the source created with no verification at all while looking configured. An empty value never cleared anything: the API expresses "no verification" as a null auth object, and `--config '{"auth": null}'` still does that. - #336 The release skill's CI gate used the legacy commit-status API, which GitHub Actions never writes, so it returned pending off total_count 0 and blocked every release. Replaced with the GraphQL statusCheckRollup. The skill was also duplicated byte-for-byte under .agents/; that is now a symlink, as .claude/skills and .cursor/skills already were. - #331 Dropped go-github v28, whose only use was one unauthenticated call and whose only other effect was linking x/crypto/openpgp through package init (GO-2026-5932, no upstream fix). Replaced with a net/http call, now testable and tested. Added a govulncheck job, since nothing enforced the scan. Also fixed, previously unreported: - Unauthenticated commands no longer hang. Any command failing for want of credentials dropped into interactive login: blocking on Enter, opening a browser, then polling for ~4 minutes. Now gated on a terminal, and reports how to authenticate without one. - Destructive commands no longer exit 0 on a skipped confirmation. The five delete/dismiss commands called fmt.Scanln and discarded its error, so without a terminal they printed "cancelled" and returned nil: a CI job deleted nothing and reported success. They now fail and name --force. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

Copilot

Pull request overview

Fixes silent and non-interactive CLI failures across listening, authentication, configuration, validation, and destructive commands.

Changes:

Adds safe headless behavior, credential precedence, and truthful failure propagation.
Rejects empty critical flags and centralizes destructive confirmations.
Removes a vulnerable dependency and strengthens tests, documentation, CI scanning, and release guidance.

Reviewed changes

Copilot reviewed 47 out of 48 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`test/acceptance/source_test.go`	Tests empty secrets and headless deletion.
`test/acceptance/login_ci_local_test.go`	Tests local-only configuration writes.
`test/acceptance/listen_test.go`	Tests headless listen and environment authentication.
`test/acceptance/helpers.go`	Adds environment-aware CLI runner.
`test/acceptance/connection_upsert_test.go`	Tests empty inline source secrets.
`skills/hookdeck-cli-release/SKILL.md`	Corrects the release CI gate.
`REFERENCE.md`	Updates generated CLI reference.
`README.md`	Revises CI and headless-listen guidance.
`pkg/version/version.go`	Replaces `go-github` with direct HTTP.
`pkg/version/version_test.go`	Tests release-version fetching.
`pkg/listen/proxy/renderer.go`	Adds renderer error reporting.
`pkg/listen/proxy/renderer_simple.go`	Implements the renderer error contract.
`pkg/listen/proxy/renderer_interactive.go`	Captures Bubble Tea failures.
`pkg/listen/proxy/renderer_err_test.go`	Tests renderer error semantics.
`pkg/listen/proxy/proxy.go`	Propagates renderer failures.
`pkg/config/local_only_test.go`	Tests suppressed global writes.
`pkg/config/config.go`	Adds local-only write mode.
`pkg/cmd/transformation_delete.go`	Uses shared confirmation handling.
`pkg/cmd/source_upsert.go`	Rejects empty critical flags.
`pkg/cmd/source_update.go`	Rejects empty critical flags.
`pkg/cmd/source_delete.go`	Uses shared confirmation handling.
`pkg/cmd/source_create.go`	Rejects empty critical flags.
`pkg/cmd/root.go`	Prevents headless automatic-login hangs.
`pkg/cmd/root_auth_fallback_test.go`	Tests authentication fallback selection.
`pkg/cmd/login.go`	Enables local-only credential persistence.
`pkg/cmd/listen.go`	Adds headless output and environment authentication.
`pkg/cmd/listen_output_mode_test.go`	Tests output fallback behavior.
`pkg/cmd/listen_env_api_key_test.go`	Tests environment-key precedence.
`pkg/cmd/issue_dismiss.go`	Uses shared confirmation handling.
`pkg/cmd/empty_flags.go`	Implements empty-value validation.
`pkg/cmd/empty_flags_test.go`	Tests validation and command wiring.
`pkg/cmd/destination_upsert.go`	Rejects empty critical flags.
`pkg/cmd/destination_update.go`	Rejects empty critical flags.
`pkg/cmd/destination_delete.go`	Uses shared confirmation handling.
`pkg/cmd/destination_create.go`	Rejects empty critical flags.
`pkg/cmd/connection_upsert.go`	Rejects empty inline flags.
`pkg/cmd/connection_delete.go`	Uses shared confirmation handling.
`pkg/cmd/connection_create.go`	Rejects empty inline flags.
`pkg/cmd/confirm.go`	Adds terminal-aware confirmation.
`pkg/cmd/confirm_test.go`	Tests headless confirmation failures.
`pkg/cmd/ci.go`	Redirects local credential writes.
`go.sum`	Removes obsolete dependency checksums.
`go.mod`	Removes `go-github`.
`AGENTS.md`	Documents canonical skill symlinks.
`.github/workflows/test.yml`	Adds vulnerability scanning.
`.agents/skills/hookdeck-cli-release/SKILL.md`	Removes duplicated release skill.
`.agents/skills/hookdeck-cli-release/references/release-notes-template.md`	Removes duplicated template.

Suppressed comments (2)

test/acceptance/listen_test.go:191

Wait for the killed process before reading its buffers. With non-file stdout/stderr, os/exec copies output on background goroutines until Wait completes; reading immediately races those writes and may miss the authentication message being asserted.
pkg/cmd/empty_flags.go:122
As above, shell expansion does not require variables to be exported. The plural error should say the variables must be set in the invoking shell rather than prescribing export.

	return fmt.Errorf(
		"%s were empty. If you passed shell variables, check they are exported",
		strings.Join(flags, ", "),
	)

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

The previous acceptance tests asserted configuration and error text. These assert the outcomes users actually reported, and each one has been verified to fail against unmodified main and pass on this branch. - listen forwards a real event end to end with no terminal. Starts a local HTTP server, creates a source and CLI connection, runs `listen` with no --output, POSTs to the source URL and waits for the payload to arrive locally. This is the scenario #333 was found in: start a service, start a tunnel, send an event, check it arrives. Asserting only on the absence of a TTY error would have passed even if nothing was being forwarded. - Unauthenticated commands fail fast. Asserts elapsed time, not just the error: against main this test takes 261s, which is the ~4 minute browser-login poll #337 describes. A message-only assertion would pass while still hanging. - `ci --local` leaves the active project alone, checked through `whoami` rather than file contents. The reported symptom was not "a file changed", it was that every other hookdeck invocation on the machine silently moved project (#332). - delete without --force now covered on destination, connection and transformation as well as source. The five commands share one confirmation helper, but each has to call it; a command that quietly stops doing so keeps working and just stops catching the bug (#338). Issue dismiss is covered by unit tests only: it shares the same helper, and creating a real issue takes ~40s of setup for a one-line wiring check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

…rding Five review findings, all valid. One was a real bug. - HOOKDECK_API_KEY was ignored once a guest profile existed. GuestLogin persists its key, so Profile.APIKey is non-empty after a single guest run and the "no credentials stored" test treated the throwaway account as a real login. Every later run then stayed on the guest project despite an exported Project API key — #334 again, one run later. A guest profile is now identified by guest_url alongside the key, and the environment key takes precedence over it while still losing to a real stored login. Replacing a guest profile is announced rather than silent, since discarding a sandbox link is exactly the kind of side effect this release is about surfacing. - The acceptance helpers raced. os/exec writes subprocess output from its own goroutine while the tests read the buffer mid-run, so a bare bytes.Buffer could return partial output. Reads and writes now go through a mutex. - "Check it is exported" misdiagnosed the reported cause. In #335 the secret was in a workspace .env that application code loaded but the shell never did, so the variable was not set in the invoking shell at all; exporting only matters for a variable that is set locally but not visible to child processes. The error and the README now say "set in this shell" and note that .env values are not loaded automatically. - REFERENCE.md advertised --cli-key as a global flag. The root persistent flag was registered unhidden while --api-key beside it is hidden, and regenerating the docs surfaced it — contradicting README "CLI authentication keys" and AGENTS.md, which state authentication is command-specific. It is now hidden like --api-key: still functional for existing callers, no longer advertised. The command-specific `login --cli-key` and `listen --cli-key` remain documented. Adds the guest-profile acceptance case the review asked for, using a synthetic guest config so it stays deterministic and does not create a real guest account on every CI run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

leggetter · 2026-08-13T10:53:05Z

Thanks — five findings, all valid, all addressed in 7b66f39. One was a real bug.

1. Guest profile ignored HOOKDECK_API_KEY — correct, and this was a genuine miss.

GuestLogin persists its key, so Profile.APIKey is non-empty after a single guest run and my currentAPIKey == "" test treated the throwaway account as a real login. Every later run then stayed on the guest project despite an exported Project API key — #334 again, one run later.

The predicate now takes guestURL and gives the environment key precedence over a guest profile while still losing to a real stored login:

func shouldExchangeEnvAPIKey(currentAPIKey, guestURL, envKey string) bool {
	if envKey == "" {
		return false
	}
	return currentAPIKey == "" || guestURL != ""
}

Replacing a guest profile now prints a notice naming the sandbox URL, rather than silently discarding the link — silently swapping accounts is the exact class of bug this PR is about.

Added the guest-profile case you asked for, at both levels: unit cases in listen_env_api_key_test.go, and TestListenPrefersEnvAPIKeyOverGuestProfile in the acceptance suite. The acceptance test uses a synthetic guest config (api_key + guest_url, the shape GuestLogin persists) so it stays deterministic and doesn't create a real guest account on every CI run.

2. bytes.Buffer race — correct. os/exec writes from its own goroutine while the tests read mid-run. Replaced with a mutex-guarded syncBuffer for both reads and writes.

3 & 4. "exported" wording — correct, and it mattered. In #335 the secret was in a workspace .env that application code loaded but the shell never did, so the variable wasn't set in the invoking shell at all; exporting only matters for a variable set locally but not visible to child processes. So "check it is exported" would have misdiagnosed the actual reported case. Both the error and the README now say "set in this shell" and note that .env values aren't loaded automatically. Tests updated to assert the new wording.

5. --cli-key as a global flag — correct. The root persistent flag is registered unhidden at root.go:274 while --api-key immediately below it is hidden, so regenerating REFERENCE.md surfaced a pre-existing inconsistency: the generated docs hadn't been regenerated since #306 added it. It now gets MarkHidden like --api-key, which matches README § CLI authentication keys and AGENTS.md. The flag still works for anyone already using it — verified hookdeck --cli-key <key> whoami succeeds and root --help no longer lists it. The command-specific login --cli-key and listen --cli-key stay documented, which is the intended contract.

Worth flagging: #306 is unreleased, so hiding it isn't removing anything from a shipped version.

All unit tests and the affected acceptance tags pass.

.agents/skills is the cross-harness location — it is not tied to any one tool, so it is the right home for the real files. This inverts what landed earlier in this branch, which kept the files at a root skills/ and pointed .agents/skills at it. - .agents/skills/hookdeck-cli-release/ now holds the files (tracked as a rename, so history is preserved). - .claude/skills and .cursor/skills are symlinks to ../.agents/skills. - Root skills/ is gone; there is one canonical directory and no second copy to drift out of sync, which was the underlying problem in #336. The skill sits one directory deeper than before, so its relative links needed ../../ -> ../../../. All four (release workflow, .goreleaser/, .goreleaser/mac.yml, README) verified to resolve. AGENTS.md, README.md and CLAUDE.md updated to point at the new location. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

Copilot

Pull request overview

Copilot reviewed 53 out of 54 changed files in this pull request and generated no new comments.

Suppressed comments (4)

test/acceptance/listen_test.go:305

Wait for the killed process before reading its output and config. Process.Kill only sends the signal; without cmd.Wait(), os/exec may still be copying output or the process may still be finishing its config write, making the assertions below flaky and leaving the child unreaped.
pkg/cmd/listen.go:193
This documented precedence omits the guest-profile exception implemented below: when GuestURL is set, HOOKDECK_API_KEY overrides those stored credentials. As written, users are told every stored login wins, even though exporting the environment key replaces their guest sandbox. Document the exception here and regenerate the matching REFERENCE/README text.

Authentication order: "--cli-key", then stored credentials from "hookdeck login"
or "hookdeck ci", then HOOKDECK_API_KEY. Setting HOOKDECK_API_KEY to a Project
API key is enough to run in CI — the CLI exchanges it for CLI credentials and
saves them. With none of these, a temporary guest account is created, which has

pkg/cmd/issue_dismiss.go:55

This is the only one of the five migrated destructive commands without a non-interactive regression test. The existing issue-dismiss acceptance test covers only --force and is skipped, while the source, destination, connection, and transformation callers each verify that omitting --force fails and preserves the resource. Add coverage proving this caller actually invokes the helper and returns the --force error before dismissing.

		proceed, err := confirmDestructiveAction(
			fmt.Sprintf("Are you sure you want to dismiss issue %s?", issueID),
			"Dismiss cancelled.",
			"force",
		)

pkg/cmd/listen.go:397

The specific recovery text added here is discarded for the common invalid-key case. CILogin returns a 401 APIError, and Execute classifies wrapped 401s with hookdeck.IsUnauthorizedError and replaces this error with its generic login message. Consequently users never see that HOOKDECK_API_KEY must be a Project API key or that CLI keys belong in --cli-key. Preserve this contextual message in the root error handling and cover an invalid environment key.

		return fmt.Errorf(
			"could not authenticate with HOOKDECK_API_KEY: %w\n\n"+
				"HOOKDECK_API_KEY must be a Project API key from the Hookdeck dashboard "+
				"(Project Settings > API Keys). For a CLI key, use --cli-key instead.",
			err,

…verage Three findings, all valid. - The HOOKDECK_API_KEY guidance was being discarded. CILogin returns a 401 APIError, and Execute classifies it with IsUnauthorizedError — which matches both through errors.As and through a "status code: 401" substring check — so any wrapping lost the message and the user saw the generic "API key is invalid or expired" instead of the one thing that helps: that HOOKDECK_API_KEY takes a Project API key and a CLI client key belongs in --cli-key. Added an actionableError type that Execute checks before its 401 handling, so a command that has already explained the failure keeps its message. - listen's help documented a precedence it no longer follows. It said stored credentials always beat HOOKDECK_API_KEY, which is untrue for a guest profile since the review fix. Help text and README now state the exception and that replacing a guest profile discards the sandbox link. REFERENCE.md regenerated. - issue dismiss was the only one of the five destructive commands without a non-interactive test. The earlier justification for skipping it — that creating a real issue costs ~40s of setup — was wrong: runIssueDismissCmd confirms before it calls DismissIssue, so a non-existent ID exercises the guard in ~2s. All five callers are now covered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

leggetter · 2026-08-13T11:37:47Z

Second pass — three findings, all valid, all fixed in 570b71f. (These came through in the review body rather than as inline threads, so there's nothing to resolve; the five threads from the first pass are already resolved.)

1. listen.go:193 — help text omitted the guest exception. Correct.

The Long help still promised that stored credentials always beat HOOKDECK_API_KEY, which stopped being true when I added the guest-profile exception in the previous round. Fixed in the help text:

One exception: HOOKDECK_API_KEY does take precedence over a stored guest profile, so a machine that once ran "listen" without credentials still uses your project when the variable is set. Replacing a guest profile is announced, and discards the link to that sandbox — unset HOOKDECK_API_KEY to keep it.

Same wording added to README § Running in CI, and REFERENCE.md regenerated (generate-reference --check passes).

2. issue_dismiss.go:55 — missing non-interactive test. Correct, and my reasoning for skipping it was wrong.

I'd justified the gap in the previous commit message by saying a real issue costs ~40s of setup. That was the wrong analysis: runIssueDismissCmd confirms before it calls DismissIssue, so a non-existent ID exercises the guard without touching the API. TestIssueDismissWithoutForceFailsNonInteractively runs in 1.99s and asserts the failure is attributed to the missing terminal rather than a not-found error. All five destructive callers are now covered.

3. listen.go:397 — the recovery text was being discarded. This was a real bug, and worse than described.

You're right that Execute rewrites it, and it's unavoidable by wrapping alone: IsUnauthorizedError matches both through errors.As and through a strings.Contains(msg, "status code: 401") check on the rendered string, so neither %w nor %v escapes it.

Fixed with an actionableError type that Execute checks in a case before its 401 handling, so a command that has already explained the failure keeps its message. Unwrap() still exposes the API error for anything else that wants to inspect it.

Verified end to end with a deliberately invalid environment key:

$ HOOKDECK_API_KEY=definitely_not_a_valid_key_123456 hookdeck listen 3000 x
could not authenticate with HOOKDECK_API_KEY: error: unexpected http status code: 401, raw response body: Unauthorized

HOOKDECK_API_KEY must be a Project API key from the Hookdeck dashboard (Project Settings > API Keys). For a CLI client key, use --cli-key instead.

Before the fix that printed the generic "your API key is invalid or expired" and nothing about which key belongs where.

All unit tests and the affected acceptance tags pass.

The actionableError type added in 570b71f changes the root error path but had no test. The assertion that matters is the precondition: a wrapped 401 IS still recognised by IsUnauthorizedError, so it is purely Execute's case ordering that keeps the specific guidance. Writing that down means the day the precondition stops holding, the test says so and the extra case can be removed rather than lingering as cargo. Also covers the other direction: an unmarked error must still receive the generic recovery message, or every 401 would print raw API text instead of telling the user how to sign in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

Copilot

Pull request overview

Copilot reviewed 55 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pkg/version/version.go:185

GitHub’s REST API commonly requires a User-Agent header; without it, this request can get a 403 and permanently disable upgrade checks (it will be swallowed as a non-200). Set an explicit User-Agent (ideally including the CLI version) on the request.
.github/workflows/test.yml:50
This installs govulncheck with @latest, which makes CI non-deterministic (a new govulncheck release can change behavior or break the job without any repo change). Prefer pinning to a specific version and bumping it intentionally when needed.

      - name: Run govulncheck
        run: |
          go install golang.org/x/vuln/cmd/govulncheck@latest
          govulncheck ./...

@latest

Two suppressed comments from the third Copilot pass. Neither is breaking today, both are worth doing. - The release check sent Go's default "Go-http-client/1.1". GitHub accepts it (verified: 200), but asks callers to identify themselves and applies rate limits per User-Agent — and go-github used to set one, so dropping it was an unintended regression from that removal. Now sends "hookdeck-cli/<version>". Built inline rather than via pkg/useragent, which imports pkg/version and would make this an import cycle. - The govulncheck job installed @latest, so a new release could change behaviour or fail CI with no change to the repo. Pinned to v1.6.0. The vulnerability database is still fetched at run time, so newly disclosed issues are picked up without a version bump. Verified end to end: a binary stamped 2.0.0 still detects v2.4.0 against the real API, and govulncheck v1.6.0 reports no findings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRp3XjWGqiYSun29SSNVJt

leggetter · 2026-08-13T11:46:52Z

Third pass came back with no new comments. It surfaced two suppressed items; both were worth acting on, fixed in 741eac3.

1. Missing User-Agent on the release check. Not breaking today — I verified GitHub returns 200 for Go's default Go-http-client/1.1, and even for an empty agent — so the "can get a 403 and permanently disable upgrade checks" framing overstates it. But the underlying point is right and I'd missed a consequence of my own change: go-github used to set a User-Agent, so removing it dropped one silently. GitHub applies rate limits per agent, so a generic one shares a bucket with every other Go program on the same IP. Now sends hookdeck-cli/<version>.

Worth recording why it isn't built with pkg/useragent: that package imports pkg/version, so calling it from here would be an import cycle. The header is constructed inline instead.

Verified end to end rather than just in tests — a binary stamped 2.0.0 still detects the newer release against the real API:

$ hookdeck version
hookdeck version 2.0.0
A newer version of the Hookdeck CLI is available, please update to: v2.4.0

The httptest case now asserts the header, so a future refactor can't drop it again.

2. govulncheck@latest is non-deterministic. Agreed — a new release could change behaviour or fail the job with no change to the repo, which is a poor property for a gate that blocks merges. Pinned to v1.6.0, with a comment that it should be bumped deliberately. The vulnerability database is still fetched at run time, so newly disclosed issues are still caught without a version bump — pinning the tool doesn't freeze coverage. Confirmed v1.6.0 reports no findings, so GO-2026-5932 stays clear.

Tally across the three passes: 10 findings, all valid, all addressed. Three were real bugs rather than polish — the guest-profile precedence regression, a data race in the acceptance helpers, and an error path that discarded its own recovery guidance.

leggetter requested a balanced review from Copilot August 13, 2026 10:27

leggetter changed the title ~~fix: make the CLI safe to run without a terminal~~ fix: v2.5.0 bug fixes across listen, config, and validation Aug 13, 2026

Copilot started reviewing on behalf of leggetter August 13, 2026 10:27 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread test/acceptance/listen_test.go Outdated

Comment thread REFERENCE.md Outdated

Comment thread README.md Outdated

Comment thread pkg/cmd/empty_flags.go

Comment thread pkg/cmd/listen.go Outdated

leggetter and others added 2 commits August 13, 2026 11:43

leggetter requested a balanced review from Copilot August 13, 2026 11:19

Copilot started reviewing on behalf of leggetter August 13, 2026 11:19 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

leggetter requested a lite review from Copilot August 13, 2026 11:39

Copilot started reviewing on behalf of leggetter August 13, 2026 11:40 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

leggetter merged commit 1840423 into main Aug 13, 2026
13 checks passed

leggetter deleted the fix/agent-safety-v2.5.0 branch August 13, 2026 12:04

This was referenced Aug 13, 2026

listen exits 0 when the renderer fails to start #339

Closed

Track the CLI agent-safety work that came out of this benchmark hookdeck/evals#12

Open

Navigation Menu

Uh oh!

fix: v2.5.0 bug fixes across listen, config, and validation - #342

fix: v2.5.0 bug fixes across listen, config, and validation#342
leggetter merged 7 commits into
mainfrom
fix/agent-safety-v2.5.0

leggetter commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leggetter commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

leggetter commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

leggetter commented Aug 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

leggetter commented Aug 13, 2026

Design rule applied throughout

Changes

Two worth reading closely

Testing

Docs

Not included

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leggetter commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

leggetter commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

leggetter commented Aug 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants