fix: delivery-group data loss and metrics defects, plus the missing acceptance coverage

REFERENCE.md

@@ -262,7 +262,7 @@ hookdeck gateway connection list
 hookdeck gateway source create --name my-source --type WEBHOOK
 
 # Query event metrics
-hookdeck gateway metrics events --start 2026-01-01T00:00:00Z --end 2026-02-01T00:00:00Z
+hookdeck gateway metrics events --start 2026-01-01T00:00:00Z --end 2026-02-01T00:00:00Z --measures count
 
 # Start the MCP server for AI agent access
 hookdeck gateway mcp
@@ -1746,7 +1746,7 @@ hookdeck gateway request list [flags]
 | `--prev` | `string` | Pagination cursor for previous page |
 | `--rejection-cause` | `string` | Filter by rejection cause |
 | `--source-id` | `string` | Filter by source ID |
-| `--status` | `string` | Filter by status |
+| `--status` | `string` | Filter by status (accepted, rejected) |
 | `--verified` | `string` | Filter by verified (true/false) |
 
 **Examples:**
@@ -1802,6 +1802,9 @@ hookdeck gateway request retry req_abc123 --connection-ids web_1,web_2
 
 List events (deliveries) created from a request.
 
+Filters match `hookdeck gateway event list`: this command queries the same event
+collection, narrowed to one request.
+
 **Usage:**
 
 ```bash
@@ -1812,16 +1815,39 @@ hookdeck gateway request events <request-id> [flags]
 
 | Flag | Type | Description |
 |------|------|-------------|
+| `--attempts` | `string` | Filter by number of attempts (integer or operators) |
+| `--body` | `string` | Filter by body (JSON string) |
+| `--cli-id` | `string` | Filter by CLI ID |
+| `--connection-id` | `string` | Filter by connection ID |
+| `--created-after` | `string` | Filter events created after (ISO date-time) |
+| `--created-before` | `string` | Filter events created before (ISO date-time) |
 | `--delivery-group` | `string` | Filter by delivery group |
+| `--destination-id` | `string` | Filter by destination ID |
+| `--dir` | `string` | Sort direction (asc, desc) |
+| `--error-code` | `string` | Filter by error code |
+| `--headers` | `string` | Filter by headers (JSON string) |
+| `--issue-id` | `string` | Filter by issue ID |
+| `--last-attempt-at-after` | `string` | Filter by last_attempt_at after (ISO date-time) |
+| `--last-attempt-at-before` | `string` | Filter by last_attempt_at before (ISO date-time) |
 | `--limit` | `int` | Limit number of results (default "100") |
 | `--next` | `string` | Pagination cursor for next page |
+| `--order-by` | `string` | Sort key (e.g. created_at) |
 | `--output` | `string` | Output format (json) |
+| `--parsed-query` | `string` | Filter by parsed query (JSON string) |
+| `--path` | `string` | Filter by path |
 | `--prev` | `string` | Pagination cursor for previous page |
+| `--response-status` | `string` | Filter by HTTP response status (e.g. 200, 500) |
+| `--source-id` | `string` | Filter by source ID |
+| `--status` | `string` | Filter by status (SCHEDULED, QUEUED, HOLD, SUCCESSFUL, FAILED, CANCELLED) |
+| `--successful-at-after` | `string` | Filter by successful_at after (ISO date-time) |
+| `--successful-at-before` | `string` | Filter by successful_at before (ISO date-time) |
 
 **Examples:**
 
 ```bash
 hookdeck gateway request events req_abc123
+hookdeck gateway request events req_abc123 --status FAILED
+hookdeck gateway request events req_abc123 --destination-id des_abc123
 ```
 ### hookdeck gateway request ignored-events
 
@@ -1920,7 +1946,9 @@ hookdeck gateway attempt get atm_abc123
 <!-- GENERATE_END -->
 ## Metrics
 
-Query Event Gateway metrics (events, requests, attempts, queue depth, pending events, events by issue, transformations). All metrics commands require `--start` and `--end` (ISO 8601 date-time).
+Query Event Gateway metrics. There are four subcommands — `events`, `requests`, `attempts` and `transformations` — and all of them require `--start` and `--end` (ISO 8601 date-time).
+
+Queue depth, pending events and per-issue breakdowns have no subcommand of their own: `metrics events` answers all three, choosing the endpoint from `--measures` and `--dimensions`.
 
 **Use cases and examples:**
 
@@ -1929,9 +1957,9 @@ Query Event Gateway metrics (events, requests, attempts, queue depth, pending ev
 | Event volume and failure rate over time | `hookdeck gateway metrics events --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --granularity 1d --measures count,failed_count,error_rate` |
 | Request acceptance vs rejection | `hookdeck gateway metrics requests --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --measures count,accepted_count,rejected_count` |
 | Delivery latency (attempts) | `hookdeck gateway metrics attempts --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --measures response_latency_avg,response_latency_p95` |
-| Queue backlog per destination | `hookdeck gateway metrics queue-depth --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --measures max_depth,max_age --destination-id dest_xxx` |
-| Pending events over time | `hookdeck gateway metrics pending --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --granularity 1h --measures count` |
-| Events grouped by issue (debugging) | `hookdeck gateway metrics events-by-issue iss_xxx --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --measures count` |
+| Queue backlog per destination | `hookdeck gateway metrics events --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --measures max_depth,max_age --destination-id dest_xxx` |
+| Pending events over time | `hookdeck gateway metrics events --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --granularity 1h --measures pending` |
+| Events grouped by issue (debugging) | `hookdeck gateway metrics events --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --measures count --dimensions issue_id --issue-id iss_xxx` |
 | Transformation errors | `hookdeck gateway metrics transformations --start 2026-02-01T00:00:00Z --end 2026-02-25T00:00:00Z --measures count,failed_count,error_rate` |
 
 **Common flags (all metrics subcommands):** `--start`, `--end` (required), `--granularity` (e.g. 1h, 5m, 1d), `--measures`, `--dimensions`, `--output` (json).
@@ -1951,6 +1979,19 @@ Passing one where it does not apply is an `unknown flag` error rather than a sil
 
 `metrics events` routes to a different endpoint depending on `--measures` and `--dimensions`, so some of its filters are rejected for a given query — `--delivery-group` and `--status` cannot be combined with `--measures pending`, for example. The error names the flag and the route.
 
+**`--dimensions` is gated the same way.** Each endpoint defines its own set, and `metrics events` advertises the union, so a dimension the chosen route does not group by is refused by name rather than sent to the API as a 422:
+
+| Route | Selected by | Groups by |
+| --- | --- | --- |
+| event metrics (default) | anything else | `source_id`, `destination_id`, `connection_id`, `delivery_group`, `status`, `error_code`, `event_data_id`, `cli_id`, `cli_user_id`, `attempts`, `response_status` |
+| queue depth metrics | `--measures queue_depth`, `max_depth` or `max_age` | `destination_id`, `delivery_group` |
+| pending event metrics | `--measures pending` | `destination_id` |
+| per-issue event metrics | `--dimensions issue_id` or `--issue-id` | `issue_id`, `source_id`, `destination_id`, `connection_id` |
+
+`metrics requests`, `metrics attempts` and `metrics transformations` each have a single set, listed in their own `--help`. One rule is the API's and applies wherever the dimension is offered, `metrics attempts` included: grouping by `delivery_group` requires a `--destination-id` filter.
+
+Only one endpoint answers a query, so a request cannot ask for two of them at once. `queue_depth`, `max_depth` and `max_age` select queue-depth metrics and `pending` selects pending metrics; neither can be combined with per-issue metrics (`--dimensions issue_id` or `--issue-id`), and measures belonging to two routes cannot be mixed in one `--measures`. Each combination is refused by name rather than answered from whichever route happened to match first.
+
 ## Utilities
 
 <!-- GENERATE:completion|ci:START -->

go.mod

@@ -10,11 +10,13 @@ require (
 	github.com/charmbracelet/bubbles v1.0.0
 	github.com/charmbracelet/bubbletea v1.3.10
 	github.com/charmbracelet/lipgloss v1.1.0
+	github.com/creack/pty v1.1.17
 	github.com/gorilla/websocket v1.5.3
 	github.com/gosimple/slug v1.15.0
 	github.com/logrusorgru/aurora v2.0.3+incompatible
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/modelcontextprotocol/go-sdk v1.7.0
+	github.com/muesli/termenv v0.16.0
 	github.com/sirupsen/logrus v1.9.4
 	github.com/spf13/cobra v1.10.2
 	github.com/spf13/pflag v1.0.10
@@ -52,7 +54,6 @@ require (
 	github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect
 	github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect
 	github.com/muesli/cancelreader v0.2.2 // indirect
-	github.com/muesli/termenv v0.16.0 // indirect
 	github.com/onsi/ginkgo v1.14.1 // indirect
 	github.com/onsi/gomega v1.10.1 // indirect
 	github.com/pelletier/go-toml/v2 v2.2.4 // indirect

pkg/ansi/ansi.go

@@ -101,10 +101,30 @@ func Italic(text string) string {
 	return color.Sprintf(color.Italic(text))
 }
 
+// ShouldUseColors reports whether ANSI decoration may be written to w, taking
+// --color, CLICOLOR/CLICOLOR_FORCE, NO_COLOR and whether w is a terminal into
+// account. Renderers that draw with something other than this package (the
+// interactive TUI draws with lipgloss) need the same answer, or --color off
+// silently applies to one output mode and not the other (#404).
+func ShouldUseColors(w io.Writer) bool {
+	return shouldUseColors(w)
+}
+
+// CanHyperlink reports whether OSC 8 hyperlinks can be written to w. It is the
+// same test colour uses, deliberately: a hyperlink is terminal decoration, so it
+// belongs wherever colour belongs and nowhere else.
+//
+// Before #403 the listen printer emitted OSC 8 unconditionally, which inverted
+// the rule — a piped run (a log file, a CI job) got the escape bytes while a
+// real terminal, the only thing that can render them, got a plain URL.
+func CanHyperlink(w io.Writer) bool {
+	return ShouldUseColors(w)
+}
+
 // Linkify returns an ANSI escape sequence with an hyperlink, if the writer
 // supports colors.
 func Linkify(text, url string, w io.Writer) string {
-	if !shouldUseColors(w) {
+	if !CanHyperlink(w) {
 		return text
 	}
 
@@ -220,5 +240,14 @@ func shouldUseColors(w io.Writer) bool {
 		}
 	}
 
+	// https://no-color.org: any non-empty NO_COLOR turns decoration off. It is
+	// checked after CLICOLOR_FORCE and before DisableColors so an explicit
+	// --color on still wins, matching how the other overrides are layered.
+	if !ForceColors {
+		if noColor, ok := os.LookupEnv("NO_COLOR"); ok && noColor != "" {
+			useColors = false
+		}
+	}
+
 	return useColors && !DisableColors
 }

pkg/ansi/ansi_test.go

@@ -0,0 +1,136 @@
+package ansi
+
+import (
+	"bytes"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// resetColorState restores the package globals a test flipped, so the next test
+// sees the default configuration.
+func resetColorState(t *testing.T) {
+	t.Helper()
+	force, disable := ForceColors, DisableColors
+	t.Cleanup(func() {
+		ForceColors, DisableColors = force, disable
+	})
+}
+
+// TestCanHyperlinkFollowsColor pins #403. OSC 8 hyperlinks were written
+// unconditionally by the listen printer, which meant a redirected run (a log
+// file, a CI job) received the escape bytes while a real terminal — the only
+// thing that can render them — received a plain URL. A hyperlink is terminal
+// decoration and must be gated exactly like colour.
+func TestCanHyperlinkFollowsColor(t *testing.T) {
+	t.Run("a plain writer is not a terminal, so no hyperlink", func(t *testing.T) {
+		resetColorState(t)
+
+		assert.False(t, CanHyperlink(&bytes.Buffer{}))
+	})
+
+	t.Run("a colour-capable writer can be hyperlinked", func(t *testing.T) {
+		resetColorState(t)
+		ForceColors = true
+
+		assert.True(t, CanHyperlink(&bytes.Buffer{}))
+	})
+
+	t.Run("--color off suppresses hyperlinks too", func(t *testing.T) {
+		resetColorState(t)
+		ForceColors = true
+		DisableColors = true
+
+		assert.False(t, CanHyperlink(&bytes.Buffer{}),
+			"--color off must strip the OSC 8 bytes, not just the SGR ones")
+	})
+
+	t.Run("CanHyperlink and ShouldUseColors agree", func(t *testing.T) {
+		resetColorState(t)
+		w := &bytes.Buffer{}
+
+		for _, force := range []bool{false, true} {
+			for _, disable := range []bool{false, true} {
+				ForceColors, DisableColors = force, disable
+				assert.Equal(t, ShouldUseColors(w), CanHyperlink(w),
+					"hyperlinks and colour must be gated by one decision")
+			}
+		}
+	})
+}
+
+// TestLinkifyEmitsOSC8OnlyWhenItCanBeRendered checks the bytes themselves.
+func TestLinkifyEmitsOSC8OnlyWhenItCanBeRendered(t *testing.T) {
+	const url = "https://dashboard.hookdeck.com/events/cli?team_id=tm_1"
+
+	t.Run("no terminal, no escape", func(t *testing.T) {
+		resetColorState(t)
+
+		out := Linkify("label", url, &bytes.Buffer{})
+
+		assert.Equal(t, "label", out)
+		assert.NotContains(t, out, "\x1b]8;;")
+	})
+
+	t.Run("terminal gets the escape", func(t *testing.T) {
+		resetColorState(t)
+		ForceColors = true
+
+		out := Linkify("label", url, &bytes.Buffer{})
+
+		assert.Equal(t, 2, strings.Count(out, "\x1b]8;;"),
+			"an OSC 8 hyperlink opens and closes")
+		assert.Contains(t, out, url)
+	})
+}
+
+// TestNoColorDisablesDecoration covers the NO_COLOR half of #403. The CLI
+// honoured --color off and CLICOLOR but had never implemented https://no-color.org,
+// so NO_COLOR only appeared to work in the places where output was not a
+// terminal anyway.
+func TestNoColorDisablesDecoration(t *testing.T) {
+	t.Run("NO_COLOR turns decoration off", func(t *testing.T) {
+		resetColorState(t)
+		t.Setenv("NO_COLOR", "1")
+		t.Setenv("CLICOLOR_FORCE", "1") // would otherwise force colour on
+
+		assert.False(t, ShouldUseColors(&bytes.Buffer{}))
+		assert.False(t, CanHyperlink(&bytes.Buffer{}))
+	})
+
+	t.Run("an empty NO_COLOR is not set", func(t *testing.T) {
+		resetColorState(t)
+		t.Setenv("NO_COLOR", "")
+		ForceColors = true
+
+		assert.True(t, ShouldUseColors(&bytes.Buffer{}),
+			"the spec treats only a non-empty value as set")
+	})
+
+	t.Run("--color on still wins", func(t *testing.T) {
+		resetColorState(t)
+		t.Setenv("NO_COLOR", "1")
+		ForceColors = true
+
+		assert.True(t, ShouldUseColors(&bytes.Buffer{}),
+			"an explicit flag beats an environment default")
+	})
+}
+
+// TestCanSpinStillRequiresATerminal guards the #376 fix: readiness reporting
+// branches on CanSpin, and it must stay false for a non-terminal so the plain
+// status line is printed instead of a spinner nobody can see.
+func TestCanSpinStillRequiresATerminal(t *testing.T) {
+	resetColorState(t)
+	ForceColors = true
+
+	assert.False(t, CanSpin(&bytes.Buffer{}), "a buffer is not a terminal")
+
+	devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = devNull.Close() })
+	assert.False(t, CanSpin(devNull), "/dev/null is a file, not a terminal")
+}

pkg/cmd/connection_create.go

@@ -577,7 +577,13 @@ func (cc *connectionCreateCmd) buildDestinationInput() (*hookdeck.DestinationCre
 			destinationConfig["http_method"] = method
 		}
 	case "CLI":
-		destinationConfig["path"] = cc.destinationCliPath
+		// An empty path means "leave it alone". connection create never reaches
+		// that (its --destination-cli-path defaults to "/"), but connection
+		// upsert deliberately clears it against an existing CLI destination, and
+		// sending "" would reset the stored path just as "/" did.
+		if cc.destinationCliPath != "" {
+			destinationConfig["path"] = cc.destinationCliPath
+		}
 	case "MOCK_API":
 		// No extra fields needed for MOCK_API
 	default:
@@ -620,6 +626,9 @@ func (cc *connectionCreateCmd) buildDestinationConfig() (map[string]interface{},
 	if err != nil {
 		return nil, err
 	}
+	if err := rejectDeliveryPolicyForCLI(cc.destinationType, policy, "destination-"); err != nil {
+		return nil, err
+	}
 	mergeDeliveryPolicy(config, policy)
 
 	if len(config) == 0 {