From f4469f70f1c9a82b41b763a92157132c04560e95 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 15:57:23 -0500 Subject: [PATCH 01/17] feat(e2e): Snowflake connector create smoke (4 auth modes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds e2e/connector_snowflake_test.go covering password, keypair, oauth-sso, and oauth-individual leaves. Each test skips when its required ANA_E2E_SF_* env vars are unset (no defaults — Snowflake auth shapes make unreachable defaults meaningless). Harness additions: - RunStdin(stdin, args...) — plumbs stdin into the existing Run path so tests can exercise --*-stdin secret flags without bypassing the CLI layer. - RegisterConnectorCleanup(id) — LIFO cleanup for connectors created via CLI dispatch (id parsed from stdout) rather than the raw-RPC helper. - client.go wires ANA_E2E_ENDPOINT into connector.Deps.Endpoint so the oauth-sso success note renders against the active endpoint. --- e2e/CLAUDE.md | 1 + e2e/README.md | 39 ++++++ e2e/connector_snowflake_test.go | 224 ++++++++++++++++++++++++++++++++ e2e/harness/client.go | 2 +- e2e/harness/harness.go | 10 +- e2e/harness/resources.go | 8 ++ 6 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 e2e/connector_snowflake_test.go diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index 9926fdf..f61bdb8 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -13,4 +13,5 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `auth_test.go` | Login/logout/whoami round-trip against a real org. | | `chat_test.go` | Chat CRUD + streaming `send`. | | `connector_test.go` | Connector CRUD (create/update/test/delete) with ledger-backed cleanup. | +| `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | | `org_test.go` | Org list/show + nested members/roles/permissions. | diff --git a/e2e/README.md b/e2e/README.md index 0dcc977..0bc7c85 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -38,6 +38,45 @@ Optional env: | `ANA_E2E_SWEEP_ONLY=1` | Run the leftover-sweep only, then skip tests | | `ANA_E2E_PG_HOST` etc. | Use a real postgres for connector tests (optional) | +### Snowflake connector env + +Snowflake tests (`e2e/connector_snowflake_test.go`) skip per-test when their +required vars are absent — unlike Postgres, there are no sensible defaults. +Two vars are shared across every mode; set both or every Snowflake test +skips: + +| Variable | Meaning | +|------------------------|-----------------------------------------------------------------| +| `ANA_E2E_SF_LOCATOR` | Snowflake account locator (e.g. `abc12345.us-east-1`) | +| `ANA_E2E_SF_DATABASE` | Database name (required) | +| `ANA_E2E_SF_WAREHOUSE` | Default warehouse (optional; unset to exercise omitempty) | +| `ANA_E2E_SF_SCHEMA` | Default schema (optional) | +| `ANA_E2E_SF_ROLE` | Default role (optional) | + +Password mode (`TestConnectorCreateSnowflakePassword`): + +| Variable | Meaning | +|------------------------|-----------------------------------------------------------------| +| `ANA_E2E_SF_USER` | Snowflake username | +| `ANA_E2E_SF_PASSWORD` | Password; piped via `--password-stdin` | + +Keypair mode (`TestConnectorCreateSnowflakeKeypair`): + +| Variable | Meaning | +|--------------------------------------|---------------------------------------------------| +| `ANA_E2E_SF_USER` | Snowflake username bound to the public key | +| `ANA_E2E_SF_PRIVATE_KEY_PATH` | Path to a PEM-encoded PKCS#8 private key file | +| `ANA_E2E_SF_PRIVATE_KEY_PASSPHRASE` | Optional; piped via `--private-key-passphrase-stdin` when set | + +OAuth SSO + OAuth individual (`TestConnectorCreateSnowflakeOAuthSSO`, +`TestConnectorCreateSnowflakeOAuthIndividual`) share the same vars and only +differ in wire `authStrategy`: + +| Variable | Meaning | +|-----------------------------------|---------------------------------------------------------| +| `ANA_E2E_SF_OAUTH_CLIENT_ID` | Snowflake OAuth client id | +| `ANA_E2E_SF_OAUTH_CLIENT_SECRET` | Client secret; piped via `--oauth-client-secret-stdin` | + Invocations: ```sh diff --git a/e2e/connector_snowflake_test.go b/e2e/connector_snowflake_test.go new file mode 100644 index 0000000..0450ae2 --- /dev/null +++ b/e2e/connector_snowflake_test.go @@ -0,0 +1,224 @@ +package e2e + +import ( + "fmt" + "os" + "regexp" + "strconv" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// connectorId: is the first line of non-JSON stdout from every +// snowflake create leaf; this regex extracts the id so the test can register +// cleanup and assert on the value. +var snowflakeConnectorIDRE = regexp.MustCompile(`(?m)^connectorId:\s+(\d+)\s*$`) + +// sfCommonEnv holds the Snowflake connection fields every auth mode shares. +// Empty optional fields are preserved so tests can exercise omitempty paths +// (warehouse/schema/role). +type sfCommonEnv struct { + locator string + database string + warehouse string + schema string + role string +} + +// snowflakeCommonEnvOrSkip reads the mode-agnostic ANA_E2E_SF_* env vars and +// skips the calling test if any required field (LOCATOR, DATABASE) is empty. +// Snowflake auth shapes make unreachable defaults meaningless — unlike +// Postgres, the server does more up-front validation — so each Snowflake test +// skips outright when its env is absent rather than submitting a made-up spec. +func snowflakeCommonEnvOrSkip(t *testing.T) sfCommonEnv { + t.Helper() + env := sfCommonEnv{ + locator: os.Getenv("ANA_E2E_SF_LOCATOR"), + database: os.Getenv("ANA_E2E_SF_DATABASE"), + warehouse: os.Getenv("ANA_E2E_SF_WAREHOUSE"), + schema: os.Getenv("ANA_E2E_SF_SCHEMA"), + role: os.Getenv("ANA_E2E_SF_ROLE"), + } + if env.locator == "" || env.database == "" { + t.Skip("e2e: ANA_E2E_SF_LOCATOR and ANA_E2E_SF_DATABASE must be set for Snowflake tests") + } + return env +} + +// snowflakeCommonArgs returns the --name/--locator/--database (+ optional +// warehouse/schema/role) flags shared by every Snowflake auth-mode leaf. +func snowflakeCommonArgs(h *harness.H, suffix string, env sfCommonEnv) []string { + args := []string{ + "--name", h.ResourceName(suffix), + "--locator", env.locator, + "--database", env.database, + } + if env.warehouse != "" { + args = append(args, "--warehouse", env.warehouse) + } + if env.schema != "" { + args = append(args, "--schema", env.schema) + } + if env.role != "" { + args = append(args, "--role", env.role) + } + return args +} + +// extractConnectorID pulls the integer id out of `connectorId: ` stdout. +// Fails the test if no match — the leaf's contract is to always emit this line +// on success, so a miss means the output shape drifted. +func extractConnectorID(t *testing.T, stdout string) int { + t.Helper() + m := snowflakeConnectorIDRE.FindStringSubmatch(stdout) + if len(m) != 2 { + t.Fatalf("could not find connectorId in stdout:\n%s", stdout) + } + id, err := strconv.Atoi(m[1]) + if err != nil { + t.Fatalf("connectorId %q is not an int: %v", m[1], err) + } + return id +} + +// TestConnectorCreateSnowflakePassword smokes `connector create snowflake +// password --password-stdin`. Requires ANA_E2E_SF_USER + ANA_E2E_SF_PASSWORD +// in addition to the common LOCATOR/DATABASE pair. +func TestConnectorCreateSnowflakePassword(t *testing.T) { + common := snowflakeCommonEnvOrSkip(t) + user := os.Getenv("ANA_E2E_SF_USER") + password := os.Getenv("ANA_E2E_SF_PASSWORD") + if user == "" || password == "" { + t.Skip("e2e: ANA_E2E_SF_USER and ANA_E2E_SF_PASSWORD required for Snowflake password mode") + } + + h := harness.Begin(t) + args := append([]string{"connector", "create", "snowflake", "password"}, + snowflakeCommonArgs(h, "sf-password", common)...) + args = append(args, "--user", user, "--password-stdin") + + stdout, stderr, err := h.RunStdin(password+"\n", args...) + if err != nil { + t.Fatalf("connector create snowflake password: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { + t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) + } + // Verify the server can read the new row back. + if _, estderr, gerr := h.Run("connector", "get", fmt.Sprint(id)); gerr != nil { + t.Fatalf("connector get %d: %v\nstderr: %s", id, gerr, estderr) + } +} + +// TestConnectorCreateSnowflakeKeypair smokes the keypair leaf. Requires +// ANA_E2E_SF_USER + ANA_E2E_SF_PRIVATE_KEY_PATH (optional passphrase env via +// --private-key-passphrase-stdin). +func TestConnectorCreateSnowflakeKeypair(t *testing.T) { + common := snowflakeCommonEnvOrSkip(t) + user := os.Getenv("ANA_E2E_SF_USER") + keyPath := os.Getenv("ANA_E2E_SF_PRIVATE_KEY_PATH") + if user == "" || keyPath == "" { + t.Skip("e2e: ANA_E2E_SF_USER and ANA_E2E_SF_PRIVATE_KEY_PATH required for Snowflake keypair mode") + } + passphrase := os.Getenv("ANA_E2E_SF_PRIVATE_KEY_PASSPHRASE") + + h := harness.Begin(t) + args := append([]string{"connector", "create", "snowflake", "keypair"}, + snowflakeCommonArgs(h, "sf-keypair", common)...) + args = append(args, "--user", user, "--private-key-file", keyPath) + stdin := "" + if passphrase != "" { + args = append(args, "--private-key-passphrase-stdin") + stdin = passphrase + "\n" + } + + stdout, stderr, err := h.RunStdin(stdin, args...) + if err != nil { + t.Fatalf("connector create snowflake keypair: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { + t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) + } +} + +// TestConnectorCreateSnowflakeOAuthSSO smokes the oauth-sso leaf. Asserts the +// success note references the configured endpoint — that's the fix that +// shipped alongside the leaf and is the only per-leaf-unique line in stdout. +func TestConnectorCreateSnowflakeOAuthSSO(t *testing.T) { + common := snowflakeCommonEnvOrSkip(t) + clientID := os.Getenv("ANA_E2E_SF_OAUTH_CLIENT_ID") + clientSecret := os.Getenv("ANA_E2E_SF_OAUTH_CLIENT_SECRET") + if clientID == "" || clientSecret == "" { + t.Skip("e2e: ANA_E2E_SF_OAUTH_CLIENT_ID and ANA_E2E_SF_OAUTH_CLIENT_SECRET required for Snowflake oauth-sso mode") + } + + h := harness.Begin(t) + args := append([]string{"connector", "create", "snowflake", "oauth-sso"}, + snowflakeCommonArgs(h, "sf-oauth-sso", common)...) + args = append(args, "--oauth-client-id", clientID, "--oauth-client-secret-stdin") + + stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) + if err != nil { + t.Fatalf("connector create snowflake oauth-sso: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { + t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) + } + endpoint := os.Getenv("ANA_E2E_ENDPOINT") + if endpoint == "" { + t.Fatalf("ANA_E2E_ENDPOINT should be set inside Begin — got empty") + } + if !strings.Contains(stdout, "complete OAuth at "+endpoint) { + t.Errorf("oauth-sso note should reference ANA_E2E_ENDPOINT %q:\n%s", endpoint, stdout) + } +} + +// TestConnectorCreateSnowflakeOAuthIndividual smokes the oauth-individual +// leaf. Asserts the per-member-lazy note since that's the only leaf-unique +// piece of stdout. +func TestConnectorCreateSnowflakeOAuthIndividual(t *testing.T) { + common := snowflakeCommonEnvOrSkip(t) + clientID := os.Getenv("ANA_E2E_SF_OAUTH_CLIENT_ID") + clientSecret := os.Getenv("ANA_E2E_SF_OAUTH_CLIENT_SECRET") + if clientID == "" || clientSecret == "" { + t.Skip("e2e: ANA_E2E_SF_OAUTH_CLIENT_ID and ANA_E2E_SF_OAUTH_CLIENT_SECRET required for Snowflake oauth-individual mode") + } + + h := harness.Begin(t) + args := append([]string{"connector", "create", "snowflake", "oauth-individual"}, + snowflakeCommonArgs(h, "sf-oauth-individual", common)...) + args = append(args, "--oauth-client-id", clientID, "--oauth-client-secret-stdin") + + stdout, stderr, err := h.RunStdin(clientSecret+"\n", args...) + if err != nil { + t.Fatalf("connector create snowflake oauth-individual: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: SNOWFLAKE") { + t.Errorf("stdout missing connectorType: SNOWFLAKE:\n%s", stdout) + } + if !strings.Contains(stdout, "lazily at first query") { + t.Errorf("oauth-individual note should mention lazy per-member auth:\n%s", stdout) + } +} diff --git a/e2e/harness/client.go b/e2e/harness/client.go index bb6dc72..dab9246 100644 --- a/e2e/harness/client.go +++ b/e2e/harness/client.go @@ -90,7 +90,7 @@ func buildVerbs(client *transport.Client, env func(string) string, cfgPath strin "auth": auth.New(authDeps(client, env, cfgPath)), "profile": profile.New(profileDeps(env, cfgPath)), "org": org.New(org.Deps{Unary: client.Unary}), - "connector": connector.New(connector.Deps{Unary: client.Unary}), + "connector": connector.New(connector.Deps{Unary: client.Unary, Endpoint: os.Getenv("ANA_E2E_ENDPOINT")}), "chat": chat.New(chatDeps(client)), "dashboard": dashboard.New(dashboard.Deps{Unary: client.Unary}), "playbook": playbook.New(playbook.Deps{Unary: client.Unary}), diff --git a/e2e/harness/harness.go b/e2e/harness/harness.go index 7455e6a..f38da47 100644 --- a/e2e/harness/harness.go +++ b/e2e/harness/harness.go @@ -173,6 +173,14 @@ func (h *H) forbiddenCheck(resourceType string, id any) { // may appear in args; Dispatch handles them. If DryRun is set, Run records // the intent and returns an empty result without calling the RPC. func (h *H) Run(args ...string) (string, string, error) { + h.t.Helper() + return h.RunStdin("", args...) +} + +// RunStdin is Run with a stdin payload, used by leaves that accept secrets via +// --*-stdin flags (e.g. connector create snowflake password +// --password-stdin). An empty stdin is equivalent to Run. +func (h *H) RunStdin(stdin string, args ...string) (string, string, error) { h.t.Helper() if h.env.dryRun { h.t.Logf("dryrun: ana %s", strings.Join(args, " ")) @@ -180,7 +188,7 @@ func (h *H) Run(args ...string) (string, string, error) { } var stdout, stderr bytes.Buffer stdio := cli.IO{ - Stdin: strings.NewReader(""), + Stdin: strings.NewReader(stdin), Stdout: &stdout, Stderr: &stderr, Env: h.envFn, diff --git a/e2e/harness/resources.go b/e2e/harness/resources.go index 724f83c..18f1229 100644 --- a/e2e/harness/resources.go +++ b/e2e/harness/resources.go @@ -59,6 +59,14 @@ func (h *H) CreateConnector(suffix string, spec ConnSpec) int { return id } +// RegisterConnectorCleanup registers a LIFO cleanup that DeleteConnectors id. +// Tests that create a connector via `h.Run("connector","create",…)` (rather +// than the raw-RPC CreateConnector helper) call this after parsing the id +// out of stdout so the ledger still owns the teardown. +func (h *H) RegisterConnectorCleanup(id int) { + h.Register(func() { h.deleteConnector(id) }) +} + func (h *H) deleteConnector(id int) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() From 3604d3d6ff0ec432051b70d3f77356b19519eb59 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:05:39 -0500 Subject: [PATCH 02/17] feat(e2e): dashboard list/get/folders/health/spawn coverage - list + list --json assert header and JSON shape - folders list + --json - get + --json discover an id from list (skip if org has no dashboards) - health + spawn env-gated on ANA_E2E_DASHBOARD_ID since spawn touches billed runtime quotas --- docs/CLAUDE.md | 4 + docs/prompts/CLAUDE.md | 12 ++ docs/prompts/e2e-comprehensive-coverage.md | 223 +++++++++++++++++++++ e2e/CLAUDE.md | 1 + e2e/dashboard_test.go | 186 +++++++++++++++++ 5 files changed, 426 insertions(+) create mode 100644 docs/prompts/CLAUDE.md create mode 100644 docs/prompts/e2e-comprehensive-coverage.md create mode 100644 e2e/dashboard_test.go diff --git a/docs/CLAUDE.md b/docs/CLAUDE.md index 37251cc..ecdb4b7 100644 --- a/docs/CLAUDE.md +++ b/docs/CLAUDE.md @@ -8,3 +8,7 @@ Human-readable planning docs. Read these before touching `api-catalog/`. - `cli-readiness.md` — CLI-implementer's view of `features.md`. TL;DR, per-surface confidence table (✅/🟡/❗), enum catalog, known quirks, first-cut command shape, and prioritized follow-up probes. - `ci-scope.md` — which file globs count as "code" and trigger the full CI matrix vs docs-only skip. - `windows-smartscreen.md` — why signed release binaries run clean but `go install` / `make build` builds get SmartScreen-blocked, and how to unblock. + +## Subdirectories + +- `prompts/` — self-contained prompt briefs for fresh Claude Code sessions driving larger multi-commit workstreams. diff --git a/docs/prompts/CLAUDE.md b/docs/prompts/CLAUDE.md new file mode 100644 index 0000000..24fbabd --- /dev/null +++ b/docs/prompts/CLAUDE.md @@ -0,0 +1,12 @@ +# docs/prompts + +Self-contained prompt briefs used to kick off fresh Claude Code sessions for +larger refactors or multi-commit workstreams. Each file is written so it can be +pasted into a cold session and drive the work end-to-end without prior +conversation context. + +## Files + +- `e2e-comprehensive-coverage.md` — brief for the "smoke every verb/leaf/flag + across the CLI" workstream; base branch `feature/e2e-snowflake-coverage`, + ledger-backed cleanup, `h.Run`/`h.RunStdin` only, one verb per commit. diff --git a/docs/prompts/e2e-comprehensive-coverage.md b/docs/prompts/e2e-comprehensive-coverage.md new file mode 100644 index 0000000..b3efd66 --- /dev/null +++ b/docs/prompts/e2e-comprehensive-coverage.md @@ -0,0 +1,223 @@ +# Prompt: comprehensive e2e coverage for ana-cli + +Paste this into a fresh Claude session inside the `ana-cli` repo +(`/Users/bfair/highperformance-tech/ana-cli`). The session will have no prior +conversation context, so read this carefully and investigate before acting. + +--- + +## Goal + +Bring `ana-cli`'s live smoke suite (`e2e/`) up to **comprehensive coverage of +every user-visible command and every meaningful argument variation** across +the entire CLI — not just the dialect-specific gaps we've been filling piecewise. +The existing suite was seeded opportunistically (postgres create, chat send, +auth round-trip, etc.) and we now need it to be a rigorous contract check. + +A Snowflake-only starter PR exists on branch +`feature/e2e-snowflake-coverage` (1 commit, not yet pushed) that covers only +the four Snowflake `connector create` leaves. **Use that branch as your base +and build on it** — don't discard its harness additions (`RunStdin`, +`RegisterConnectorCleanup`, endpoint wiring in `e2e/harness/client.go`). Extend +them as needed. + +## What "comprehensive" means here + +For every verb package under `internal/` that the CLI exposes, every leaf +command must be smoked at least once with live RPC, plus meaningful argument +permutations (not a combinatorial explosion — the permutations that would +exercise distinct wire shapes or error paths). Specifically: + +1. **Every leaf gets at least one happy-path test** that drives it through + the CLI (`h.Run`/`h.RunStdin`), parses real stdout, asserts the created + resource round-trips (get after create, get after update, etc.), and + registers cleanup with the harness ledger. + +2. **Per-leaf argument matrix** — read the leaf source to enumerate its flags. + For each flag that changes wire shape (not just cosmetic flags like + `--json`), add a test case that exercises that variation. Examples: + - `--ssl=true` vs `--ssl=false` on postgres create + - `--warehouse` present vs absent on snowflake create (omitempty path) + - `--json` vs default output on every read leaf + - partial updates on `connector update` (one field at a time) vs full + merges (multiple fields in one call) + - `--since ` variants on `audit tail` + - pagination / filter flags on every `list` verb that has them + +3. **Stdin-capable secret flags** must be tested via the `--*-stdin` + variant, not the inline `--password`/`--oauth-client-secret`/`--pat` + variant. The in-args variants are fine to smoke once per leaf, but + the primary path is stdin. Use `h.RunStdin(stdin, args...)`. + +4. **Error-path smokes** for the obvious contract violations — e.g., + `connector get ` should fail with a typed RPC error, + `connector create postgres password` missing `--user` should exit with + `cli.ErrUsage`. Don't go chasing every possible error; hit the ones + the server or dispatch enforces as a contract. + +5. **Read-leaf coverage** — `list`, `get`, `tables`, `examples`, audit + `tail`, feed `show`/`stats`, etc. — should cover both default output and + `--json` output, asserting that the JSON is parseable and contains the + fields the catalog (`api-catalog/`) says it should. + +6. **Mutations require ledger discipline** — every `create` or `update` must + register a cleanup with the harness so `sweepConnectors` (and the + analogous sweeps for chats, profiles, service accounts, API keys) + proves nothing leaked. + +## Packages to cover + +Work top-down through `internal/`, one verb package per commit (or small +logical group). Existing e2e files to reference: + +| Package | Verbs | Existing e2e file | +|---------|-------|-------------------| +| `internal/auth` | login, logout, whoami, keys (list/create/rotate/revoke), service-accounts (list/create/delete) | `e2e/auth_test.go` | +| `internal/profile` | add, use, remove, list, show | — (none yet; profile is mostly local config, may need minimal live coverage) | +| `internal/org` | list, show, members list/get, roles list/get, permissions list/get | `e2e/org_test.go` | +| `internal/connector` | list, get, create (postgres password + 4 snowflake modes), update, delete, test, tables, examples | `e2e/connector_test.go`, `e2e/connector_snowflake_test.go` (new) | +| `internal/chat` | list, get, create, delete, send (streaming), share | `e2e/chat_test.go` | +| `internal/dashboard` | list, get, folders, health, spawn | — (none yet) | +| `internal/playbook` | list, get, reports, lineage | — (none yet) | +| `internal/ontology` | list, get | — (none yet) | +| `internal/feed` | show, stats | — (none yet) | +| `internal/audit` | tail (with `--since`) | `e2e/audit_test.go` | + +The packages without an existing e2e file are your biggest gap. Create +`e2e/_test.go` files for each, mirroring the style of +`connector_snowflake_test.go` (per-test skip via `t.Skip` when env is missing, +CLI-path via `h.Run`, ledger-backed cleanup). + +## Harness gaps you'll likely hit + +1. **Read-only `list`/`get` leaves don't need cleanup.** Don't over-engineer — + `h.Run` is enough. + +2. **Streaming send (`chat send`)** already has a test; extend it to cover + `--json` mode and `--attach-connector `. + +3. **`dashboard spawn`** creates a dashboard from a template. Assert the spawned + dashboard shows up in `dashboard list` and register cleanup. + +4. **Profile commands are mostly local.** `profile add`/`profile use`/ + `profile show` operate on `$XDG_CONFIG_HOME/ana/config.json` (already + redirected to a temp dir in `e2e/harness/harness.go`'s `Begin`). These + need e2e tests that use the harness's temp config, not a live RPC, but + still through `h.Run` to exercise the dispatch path. + +5. **Harness doesn't expose a way to manipulate `$XDG_CONFIG_HOME` directly.** + Profile tests may need a helper; add it to `e2e/harness/` rather than + reaching into private state from the test file. + +## Environment variables + +The existing `ANA_E2E_ENDPOINT` / `ANA_E2E_TOKEN` / `ANA_E2E_EXPECT_ORG` are +required globally. Per-leaf env needed: + +- **Postgres connector** — `ANA_E2E_PG_*` (already defined; see + `e2e/connector_test.go` for the full list). +- **Snowflake connector** — `ANA_E2E_SF_*` (already defined in + `e2e/README.md` on the feature branch). +- **Chat** — requires a working connector. Tests create a throwaway connector + (postgres preferred since it's the cheapest to skip) for the chat to hang + off of, or accept `ANA_E2E_CONNECTOR_ID` to reuse an existing one. +- **Dashboard / playbook / ontology** — these leaves are mostly read-only + for the MVP. Spawn/share/report variants may need `ANA_E2E_DASHBOARD_ID`, + `ANA_E2E_PLAYBOOK_ID`, etc., as skip gates for the leaves that mutate. + +Document every new env var in `e2e/README.md` as you add it, grouped by verb. + +## Design rules + +1. **Drive through `h.Run` / `h.RunStdin`**, not raw RPC helpers. The existing + `CreateConnector(suffix, ConnSpec)` in `e2e/harness/resources.go` is a + legacy shortcut kept for chat tests; don't add more of those. The whole + point of e2e is to smoke the CLI itself. + +2. **Per-test skip on missing env**, not file-level skip. Match + `e2e/connector_snowflake_test.go` — each test prints exactly which vars + it needs so operators know what's gated. + +3. **Assertions must be resilient to server drift**: assert the presence of + contract fields (`connectorId`, `connectorType`, etc.), not exact values + for fields that could change (timestamps, display names set by the + server). + +4. **Cleanup runs in LIFO via `h.Register(func)`.** Every mutation registers + a cleanup before completing the test — even if the test fails after + creating the resource, cleanup still runs. + +5. **Never log secrets.** Env-var values passed via `RunStdin` are piped to + the leaf; don't `t.Logf` them. The harness already suppresses secret + logging in `dryRun` mode — preserve that. + +6. **`t.Parallel()` where safe.** Tests that create + delete their own + resources are usually safe. Tests that manipulate org-level state + (members, roles) may not be. + +7. **Don't modify code in `internal/...`** unless you find an actual bug + that blocks coverage — that belongs in a separate PR per the repo's + conventions. Coverage gaps in the CLI implementation are fair game to + flag but not to fix in this PR. + +## Investigation first + +Before writing any tests, read in this order: + +1. `CLAUDE.md` (root) and every `CLAUDE.md` under `internal/` — these + describe the verb-package conventions. +2. `e2e/CLAUDE.md` and `e2e/harness/CLAUDE.md` — the harness contract. +3. `e2e/harness/harness.go`, `e2e/harness/guard.go`, `e2e/harness/ledger.go`, + `e2e/harness/sweep.go` — the mutation-safety plumbing. +4. Existing e2e tests (`e2e/auth_test.go`, `e2e/chat_test.go`, + `e2e/connector_test.go`, `e2e/audit_test.go`, `e2e/org_test.go`, + `e2e/connector_snowflake_test.go`) — the established style. +5. `api-catalog/` — wire-shape contracts for every endpoint. +6. Every leaf's `.go` source under `internal//` — to enumerate flags. + +## Scope of this PR + +Ambitious but bounded: + +- **In:** every `internal//` package listed above, every leaf, + meaningful argument variations, new env vars documented, updated + `e2e/README.md` and `e2e/CLAUDE.md`. +- **Out:** changes to `internal/...` (CLI code), new `api-catalog/` entries, + `Databricks` connector create tests (Databricks leaves haven't shipped + yet — tracked as task #20), Phase 3g `update auth` subcommand (deferred). + +If you find a gap in the CLI itself that blocks a test, **don't fix it +here** — note it in the PR description and open a separate issue. + +## Workflow + +1. Start on `feature/e2e-snowflake-coverage` (already exists, one commit + ahead of main, not pushed). Don't rebase or force-push it. +2. Rename the branch to something broader — e.g. + `feature/e2e-comprehensive-coverage` — via `git branch -m`. +3. Work one verb package per commit. Conventional commit messages: + `feat(e2e): — `. +4. After each package commit, run `make lint` and `make test` (not + `make e2e` — that's live and requires env). Both must stay green. +5. `make cover` must keep showing 100% on `./internal/...` — this PR + shouldn't touch `internal/` code, so this should be automatic. +6. **Dry-run each new test** via `ANA_E2E_DRYRUN=1 go test ./e2e/...` + to confirm argv construction is correct without hitting the server. +7. When all packages are covered, **do not push or open a PR** — hand + the branch back for human review and live smoke first. + +## What to hand back when done + +A single message summarizing: + +- Which verb packages now have e2e coverage (one line each with the + commit hash). +- How many new tests landed in total. +- Which flags you explicitly chose NOT to cover and why (there will be + some — document the reasoning). +- Any CLI bugs or gaps you found while investigating (DO NOT fix them; + note them for a follow-up PR). +- Confirmation that `make lint`, `make test`, and + `ANA_E2E_DRYRUN=1 go test ./e2e/...` all pass. +- The exact `gh pr create` command you'd run (don't run it — the human + will, after a live-smoke pass). diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index f61bdb8..95014be 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -14,4 +14,5 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `chat_test.go` | Chat CRUD + streaming `send`. | | `connector_test.go` | Connector CRUD (create/update/test/delete) with ledger-backed cleanup. | | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | +| `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | | `org_test.go` | Org list/show + nested members/roles/permissions. | diff --git a/e2e/dashboard_test.go b/e2e/dashboard_test.go new file mode 100644 index 0000000..988580e --- /dev/null +++ b/e2e/dashboard_test.go @@ -0,0 +1,186 @@ +package e2e + +import ( + "encoding/json" + "os" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// firstDashboardID asks `dashboard list --json` for at least one id, returning +// empty when the org has no dashboards (in which case the caller should skip). +// Reused by every read-only dashboard leaf that needs a known-good id without +// forcing an env var onto operators who happen to have any dashboard already. +func firstDashboardID(t *testing.T, h *harness.H) string { + t.Helper() + raw, err := h.RunJSON("dashboard", "list") + if err != nil { + t.Fatalf("dashboard list --json: %v", err) + } + arr, _ := raw["dashboards"].([]any) + for _, item := range arr { + entry, _ := item.(map[string]any) + if id, _ := entry["id"].(string); id != "" { + return id + } + } + return "" +} + +// TestDashboardList exercises the default table render. No skip gate: an empty +// org still renders the header row. +func TestDashboardList(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("dashboard", "list") + if err != nil { + t.Fatalf("dashboard list: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "ID") || !strings.Contains(out, "NAME") { + t.Errorf("dashboard list missing table header: %s", out) + } +} + +// TestDashboardListJSON asserts the --json mode emits a parseable object +// containing a `dashboards` array. Shape contract — value drift is OK. +func TestDashboardListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("dashboard", "list") + if err != nil { + t.Fatalf("dashboard list --json: %v", err) + } + if _, ok := raw["dashboards"]; !ok { + t.Errorf("--json response missing `dashboards` key: %v", raw) + } +} + +// TestDashboardFoldersList renders the ID/NAME folders table. +func TestDashboardFoldersList(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("dashboard", "folders", "list") + if err != nil { + t.Fatalf("dashboard folders list: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "ID") || !strings.Contains(out, "NAME") { + t.Errorf("dashboard folders list missing table header: %s", out) + } +} + +// TestDashboardFoldersListJSON covers --json on the folders leaf. +func TestDashboardFoldersListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + stdout, stderr, err := h.Run("--json", "dashboard", "folders", "list") + if err != nil { + t.Fatalf("dashboard folders list --json: %v\nstderr: %s", err, stderr) + } + if stdout == "" { + return + } + var raw map[string]any + if err := json.Unmarshal([]byte(stdout), &raw); err != nil { + t.Fatalf("folders list --json: not valid JSON: %v\nstdout=%q", err, stdout) + } +} + +// TestDashboardGet uses the first id surfaced by list (skip if the org has no +// dashboards) and asserts the default summary renders id/name fields. +func TestDashboardGet(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstDashboardID(t, h) + if id == "" { + t.Skip("e2e: no dashboards in org; skipping dashboard get") + } + out, stderr, err := h.Run("dashboard", "get", id) + if err != nil { + t.Fatalf("dashboard get %s: %v\nstderr: %s", id, err, stderr) + } + if !strings.Contains(out, id) { + t.Errorf("dashboard get output missing id %q: %s", id, out) + } +} + +// TestDashboardGetJSON covers the --json render path. +func TestDashboardGetJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstDashboardID(t, h) + if id == "" { + t.Skip("e2e: no dashboards in org; skipping dashboard get --json") + } + raw, err := h.RunJSON("dashboard", "get", id) + if err != nil { + t.Fatalf("dashboard get %s --json: %v", id, err) + } + dash, ok := raw["dashboard"].(map[string]any) + if !ok { + t.Fatalf("--json response missing `dashboard` object: %v", raw) + } + if gotID, _ := dash["id"].(string); gotID != id { + t.Errorf("dashboard.id = %q, want %q", gotID, id) + } +} + +// TestDashboardHealth exercises the runtime health check for an existing +// dashboard. Requires ANA_E2E_DASHBOARD_ID because an arbitrary dashboard may +// never have been spawned — in which case the server returns a non-contract +// error rather than a health row — and we don't want a flaky assertion. +func TestDashboardHealth(t *testing.T) { + id := os.Getenv("ANA_E2E_DASHBOARD_ID") + if id == "" { + t.Skip("e2e: ANA_E2E_DASHBOARD_ID not set; skipping dashboard health") + } + h := harness.Begin(t) + out, stderr, err := h.Run("dashboard", "health", id) + if err != nil { + t.Fatalf("dashboard health %s: %v\nstderr: %s", id, err, stderr) + } + if h.DryRun() { + return + } + // Output is " HEALTHY" or " UNHEALTHY: " — both acceptable. + // Assert the id is echoed rather than the specific health label so this + // test doesn't flake when the runtime happens to be down. + if !strings.Contains(out, id) { + t.Errorf("dashboard health output missing id %q: %s", id, out) + } +} + +// TestDashboardSpawn asks the server to (re)spawn a dashboard runtime. Does +// not create a new dashboard row — spawn just refreshes the runtime for an +// existing dashboard — so no ledger cleanup is needed. Requires an explicit +// env-gated id since spawning touches billed runtime quotas. +func TestDashboardSpawn(t *testing.T) { + id := os.Getenv("ANA_E2E_DASHBOARD_ID") + if id == "" { + t.Skip("e2e: ANA_E2E_DASHBOARD_ID not set; skipping dashboard spawn") + } + h := harness.Begin(t) + out, stderr, err := h.Run("dashboard", "spawn", id) + if err != nil { + t.Fatalf("dashboard spawn %s: %v\nstderr: %s", id, err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "spawned "+id) && !strings.Contains(out, "refreshedAt") { + t.Errorf("dashboard spawn output should reference id or refreshedAt: %s", out) + } +} From 2aa441957f5e1acd964b99da1449ee0dcf727b28 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:07:22 -0500 Subject: [PATCH 03/17] feat(e2e): playbook list/get/reports/lineage coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - list + list --json header + JSON shape - get + --json discover an id from list --json (skip if org is empty) - reports default assertion is header-only — zero historical runs still emits the RUN_ID/SUBJECT/RAN_AT row - lineage accepts either the FROM/TO/TYPE table or the "(no lineage edges)" marker; catalog sample is literally `{}` so either is contract-valid --- e2e/CLAUDE.md | 1 + e2e/playbook_test.go | 202 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+) create mode 100644 e2e/playbook_test.go diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index 95014be..274ea2e 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -15,4 +15,5 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `connector_test.go` | Connector CRUD (create/update/test/delete) with ledger-backed cleanup. | | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | | `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | +| `playbook_test.go` | Playbook list/get/reports/lineage read leaves (default + `--json`); id discovered via `list --json`. | | `org_test.go` | Org list/show + nested members/roles/permissions. | diff --git a/e2e/playbook_test.go b/e2e/playbook_test.go new file mode 100644 index 0000000..9efd54b --- /dev/null +++ b/e2e/playbook_test.go @@ -0,0 +1,202 @@ +package e2e + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// firstPlaybookID pulls an id out of `playbook list --json`. Empty return +// signals the caller to t.Skip — the readonly leaves have nothing to assert +// against without at least one playbook in the org. +func firstPlaybookID(t *testing.T, h *harness.H) string { + t.Helper() + raw, err := h.RunJSON("playbook", "list") + if err != nil { + t.Fatalf("playbook list --json: %v", err) + } + arr, _ := raw["playbooks"].([]any) + for _, item := range arr { + entry, _ := item.(map[string]any) + if id, _ := entry["id"].(string); id != "" { + return id + } + } + return "" +} + +// TestPlaybookList renders the ID/NAME/SCHEDULE table. Empty orgs still emit +// the header row, so no skip gate. +func TestPlaybookList(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("playbook", "list") + if err != nil { + t.Fatalf("playbook list: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + for _, col := range []string{"ID", "NAME", "SCHEDULE"} { + if !strings.Contains(out, col) { + t.Errorf("playbook list missing column %q: %s", col, out) + } + } +} + +// TestPlaybookListJSON asserts --json emits a `playbooks` array (shape +// contract only — drift in individual row values is expected). +func TestPlaybookListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("playbook", "list") + if err != nil { + t.Fatalf("playbook list --json: %v", err) + } + if _, ok := raw["playbooks"]; !ok { + t.Errorf("--json response missing `playbooks` key: %v", raw) + } +} + +// TestPlaybookGet discovers a playbook from list and asserts the summary +// echoes its id. Skips when the org has no playbooks. +func TestPlaybookGet(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstPlaybookID(t, h) + if id == "" { + t.Skip("e2e: no playbooks in org; skipping playbook get") + } + out, stderr, err := h.Run("playbook", "get", id) + if err != nil { + t.Fatalf("playbook get %s: %v\nstderr: %s", id, err, stderr) + } + if !strings.Contains(out, id) { + t.Errorf("playbook get output missing id %q: %s", id, out) + } +} + +// TestPlaybookGetJSON asserts --json returns a `playbook` object with the +// requested id (contract check — server may add fields; we only assert id). +func TestPlaybookGetJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstPlaybookID(t, h) + if id == "" { + t.Skip("e2e: no playbooks in org; skipping playbook get --json") + } + raw, err := h.RunJSON("playbook", "get", id) + if err != nil { + t.Fatalf("playbook get %s --json: %v", id, err) + } + pb, ok := raw["playbook"].(map[string]any) + if !ok { + t.Fatalf("--json response missing `playbook` object: %v", raw) + } + if gotID, _ := pb["id"].(string); gotID != id { + t.Errorf("playbook.id = %q, want %q", gotID, id) + } +} + +// TestPlaybookReports exercises the RUN_ID/SUBJECT/RAN_AT render. A playbook +// with zero historical runs still emits the header row, so no per-row check +// — the smoke is "the endpoint is reachable and the table flushes". +func TestPlaybookReports(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstPlaybookID(t, h) + if id == "" { + t.Skip("e2e: no playbooks in org; skipping playbook reports") + } + out, stderr, err := h.Run("playbook", "reports", id) + if err != nil { + t.Fatalf("playbook reports %s: %v\nstderr: %s", id, err, stderr) + } + for _, col := range []string{"RUN_ID", "SUBJECT", "RAN_AT"} { + if !strings.Contains(out, col) { + t.Errorf("playbook reports missing column %q: %s", col, out) + } + } +} + +// TestPlaybookReportsJSON asserts --json is parseable. The catalog sample +// shows a `reports` key; we assert that loosely (accept empty object too, in +// case the server drops the key when the list is empty). +func TestPlaybookReportsJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstPlaybookID(t, h) + if id == "" { + t.Skip("e2e: no playbooks in org; skipping playbook reports --json") + } + stdout, stderr, err := h.Run("--json", "playbook", "reports", id) + if err != nil { + t.Fatalf("playbook reports %s --json: %v\nstderr: %s", id, err, stderr) + } + if stdout == "" { + return + } + var raw map[string]any + if err := json.Unmarshal([]byte(stdout), &raw); err != nil { + t.Fatalf("reports --json: not valid JSON: %v\nstdout=%q", err, stdout) + } +} + +// TestPlaybookLineage covers the FROM/TO/TYPE table — or the empty-edges +// fallback ("(no lineage edges)") that the command prints when the server +// returns an empty payload. The captured sample in api-catalog is `{}`, so +// accept either output. +func TestPlaybookLineage(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstPlaybookID(t, h) + if id == "" { + t.Skip("e2e: no playbooks in org; skipping playbook lineage") + } + out, stderr, err := h.Run("playbook", "lineage", id) + if err != nil { + t.Fatalf("playbook lineage %s: %v\nstderr: %s", id, err, stderr) + } + hasTable := strings.Contains(out, "FROM") && strings.Contains(out, "TO") + hasEmpty := strings.Contains(out, "(no lineage edges)") + if !hasTable && !hasEmpty { + t.Errorf("playbook lineage: neither FROM/TO header nor empty-edges marker: %s", out) + } +} + +// TestPlaybookLineageJSON asserts --json returns valid JSON. An empty body +// is acceptable (catalog sample is literally `{}`). +func TestPlaybookLineageJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstPlaybookID(t, h) + if id == "" { + t.Skip("e2e: no playbooks in org; skipping playbook lineage --json") + } + stdout, stderr, err := h.Run("--json", "playbook", "lineage", id) + if err != nil { + t.Fatalf("playbook lineage %s --json: %v\nstderr: %s", id, err, stderr) + } + if stdout == "" { + return + } + var raw map[string]any + if err := json.Unmarshal([]byte(stdout), &raw); err != nil { + t.Fatalf("lineage --json: not valid JSON: %v\nstdout=%q", err, stdout) + } +} From 6eca49554838b0b3d75261e87852d2bfc85cdbf2 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:08:15 -0500 Subject: [PATCH 04/17] feat(e2e): ontology list/get coverage - list + list --json header + JSON shape - get + --json discover an integer id from list --json (skip if org has none); command rejects non-integer ids at dispatch, so we always pass the captured int --- e2e/CLAUDE.md | 1 + e2e/ontology_test.go | 106 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 e2e/ontology_test.go diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index 274ea2e..0583a9f 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -16,4 +16,5 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | | `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | | `playbook_test.go` | Playbook list/get/reports/lineage read leaves (default + `--json`); id discovered via `list --json`. | +| `ontology_test.go` | Ontology list/get read leaves (default + `--json`); id is integer on the wire. | | `org_test.go` | Org list/show + nested members/roles/permissions. | diff --git a/e2e/ontology_test.go b/e2e/ontology_test.go new file mode 100644 index 0000000..c349b00 --- /dev/null +++ b/e2e/ontology_test.go @@ -0,0 +1,106 @@ +package e2e + +import ( + "strconv" + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// firstOntologyID pulls an id out of `ontology list --json`. Ontology ids are +// integers on the wire (see internal/ontology/get.go), so we marshal the +// number back to a string for the CLI call. Empty return = skip. +func firstOntologyID(t *testing.T, h *harness.H) string { + t.Helper() + raw, err := h.RunJSON("ontology", "list") + if err != nil { + t.Fatalf("ontology list --json: %v", err) + } + arr, _ := raw["ontologies"].([]any) + for _, item := range arr { + entry, _ := item.(map[string]any) + // JSON numbers decode as float64. + if n, ok := entry["id"].(float64); ok && n != 0 { + return strconv.FormatInt(int64(n), 10) + } + } + return "" +} + +// TestOntologyList renders ID/NAME. Empty orgs still emit the header. +func TestOntologyList(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("ontology", "list") + if err != nil { + t.Fatalf("ontology list: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "ID") || !strings.Contains(out, "NAME") { + t.Errorf("ontology list missing header: %s", out) + } +} + +// TestOntologyListJSON asserts the --json envelope has an `ontologies` key. +func TestOntologyListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("ontology", "list") + if err != nil { + t.Fatalf("ontology list --json: %v", err) + } + if _, ok := raw["ontologies"]; !ok { + t.Errorf("--json response missing `ontologies` key: %v", raw) + } +} + +// TestOntologyGet pulls an id from list and asserts the summary echoes it. +// Skips when the org has no ontologies. Note: the command rejects +// non-integer ids at dispatch time (UsageErrf), so we always pass the raw +// integer as captured from list. +func TestOntologyGet(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstOntologyID(t, h) + if id == "" { + t.Skip("e2e: no ontologies in org; skipping ontology get") + } + out, stderr, err := h.Run("ontology", "get", id) + if err != nil { + t.Fatalf("ontology get %s: %v\nstderr: %s", id, err, stderr) + } + if !strings.Contains(out, id) { + t.Errorf("ontology get output missing id %q: %s", id, out) + } +} + +// TestOntologyGetJSON asserts --json returns an `ontology` object whose `id` +// (integer-typed) matches the requested id. +func TestOntologyGetJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + id := firstOntologyID(t, h) + if id == "" { + t.Skip("e2e: no ontologies in org; skipping ontology get --json") + } + raw, err := h.RunJSON("ontology", "get", id) + if err != nil { + t.Fatalf("ontology get %s --json: %v", id, err) + } + onto, ok := raw["ontology"].(map[string]any) + if !ok { + t.Fatalf("--json response missing `ontology` object: %v", raw) + } + got, _ := onto["id"].(float64) + if strconv.FormatInt(int64(got), 10) != id { + t.Errorf("ontology.id = %v, want %q", onto["id"], id) + } +} From 85a28f0799708d612cdf9295424dff1355113a6a Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:09:06 -0500 Subject: [PATCH 05/17] feat(e2e): feed show/stats coverage - show + show --json header + JSON shape (`posts`) - stats renders multi-key summary; JSON mode asserts at least one of the catalog counters is present (values drift every tick) --- e2e/CLAUDE.md | 1 + e2e/feed_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 e2e/feed_test.go diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index 0583a9f..507dfe2 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -17,4 +17,5 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | | `playbook_test.go` | Playbook list/get/reports/lineage read leaves (default + `--json`); id discovered via `list --json`. | | `ontology_test.go` | Ontology list/get read leaves (default + `--json`); id is integer on the wire. | +| `feed_test.go` | Feed show + stats (default + `--json`). | | `org_test.go` | Org list/show + nested members/roles/permissions. | diff --git a/e2e/feed_test.go b/e2e/feed_test.go new file mode 100644 index 0000000..05544a2 --- /dev/null +++ b/e2e/feed_test.go @@ -0,0 +1,88 @@ +package e2e + +import ( + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// TestFeedShow renders ID/TITLE/AGENT/UPVOTES/CREATED. Empty feed still +// emits the header row, so no skip gate. +func TestFeedShow(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("feed", "show") + if err != nil { + t.Fatalf("feed show: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + for _, col := range []string{"ID", "TITLE", "AGENT", "UPVOTES", "CREATED"} { + if !strings.Contains(out, col) { + t.Errorf("feed show missing column %q: %s", col, out) + } + } +} + +// TestFeedShowJSON asserts --json emits a `posts` key. +func TestFeedShowJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("feed", "show") + if err != nil { + t.Fatalf("feed show --json: %v", err) + } + if _, ok := raw["posts"]; !ok { + t.Errorf("--json response missing `posts` key: %v", raw) + } +} + +// TestFeedStats renders the key/value counter block. Assert one of the +// counter keys is present so we know the typed render path fired (vs. the +// WriteJSON fallback). +func TestFeedStats(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("feed", "stats") + if err != nil { + t.Fatalf("feed stats: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + for _, key := range []string{"messagesToday", "activeAgents", "connectorsConfigured"} { + if !strings.Contains(out, key) { + t.Errorf("feed stats missing key %q: %s", key, out) + } + } +} + +// TestFeedStatsJSON asserts --json is parseable and carries at least one of +// the catalog keys (don't assert exact values — counters drift every tick). +func TestFeedStatsJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("feed", "stats") + if err != nil { + t.Fatalf("feed stats --json: %v", err) + } + want := []string{ + "messagesToday", "messagesAllTime", "activeAgents", + "dashboardsCreated", "threadsCreated", "playbooksCreated", + "connectorsConfigured", + } + found := false + for _, k := range want { + if _, ok := raw[k]; ok { + found = true + break + } + } + if !found { + t.Errorf("feed stats --json: none of %v present in %v", want, raw) + } +} From 9ff816c97d5d5d6f4ee5fca5281125f7eb071075 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:10:12 -0500 Subject: [PATCH 06/17] feat(e2e): audit tail --json + no-since coverage - --json asserts JSON Lines (each line round-trips through encoding/json) - no-since covers the omitempty branch where `since` drops off the wire - default render now asserts all four header columns --- e2e/audit_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/e2e/audit_test.go b/e2e/audit_test.go index de5e74f..3b5e3a9 100644 --- a/e2e/audit_test.go +++ b/e2e/audit_test.go @@ -1,14 +1,16 @@ package e2e import ( + "encoding/json" + "strings" "testing" "github.com/highperformance-tech/ana-cli/e2e/harness" ) // TestAuditTail exercises ListAuditLogs with a short --since so the response -// stays small. No assertion on specific entries — just "RPC succeeds + output -// is not empty of headers". +// stays small. Assert the table header renders — an empty org still emits +// `TIME ACTOR ACTION TARGET`. func TestAuditTail(t *testing.T) { h := harness.Begin(t) out, stderr, err := h.Run("audit", "tail", "--since", "24h", "--limit", "5") @@ -18,9 +20,53 @@ func TestAuditTail(t *testing.T) { if h.DryRun() { return } - // Table header must be present regardless of whether the org has - // activity — an empty org still renders `TIME ACTOR ACTION TARGET`. - if out == "" { - t.Errorf("audit tail produced no output") + for _, col := range []string{"TIME", "ACTOR", "ACTION", "TARGET"} { + if !strings.Contains(out, col) { + t.Errorf("audit tail missing header %q: %s", col, out) + } + } +} + +// TestAuditTailNoSince covers the "since omitted" branch where the wire +// payload drops the field entirely (omitempty). Keep --limit small so the +// response doesn't balloon. +func TestAuditTailNoSince(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("audit", "tail", "--limit", "5") + if err != nil { + t.Fatalf("audit tail --limit 5: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "TIME") { + t.Errorf("audit tail missing TIME header: %s", out) + } +} + +// TestAuditTailJSON asserts the --json path emits JSON Lines. Empty output +// is acceptable (no entries in the last 24h on an idle org). If lines do +// come back, each must round-trip through encoding/json. +func TestAuditTailJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + stdout, stderr, err := h.Run("--json", "audit", "tail", "--since", "24h", "--limit", "5") + if err != nil { + t.Fatalf("audit tail --json: %v\nstderr: %s", err, stderr) + } + stdout = strings.TrimSpace(stdout) + if stdout == "" { + return + } + for i, line := range strings.Split(stdout, "\n") { + if line == "" { + continue + } + var obj map[string]any + if err := json.Unmarshal([]byte(line), &obj); err != nil { + t.Fatalf("audit tail --json line %d not valid JSON: %v\nline=%q", i, err, line) + } } } From f1b0ee0ffc9459cd4801de7ac63df925df5f6399 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:11:20 -0500 Subject: [PATCH 07/17] feat(e2e): org roles/permissions + --json across list/show/members - org roles list + --json (RBACService/ListRoles) - org permissions list + --json (RBACService/ListPermissions) - org show/list/members list each gain a --json shape check (organization, organizations, members respectively) --- e2e/CLAUDE.md | 2 +- e2e/org_test.go | 111 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index 507dfe2..3ba5172 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -18,4 +18,4 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `playbook_test.go` | Playbook list/get/reports/lineage read leaves (default + `--json`); id discovered via `list --json`. | | `ontology_test.go` | Ontology list/get read leaves (default + `--json`); id is integer on the wire. | | `feed_test.go` | Feed show + stats (default + `--json`). | -| `org_test.go` | Org list/show + nested members/roles/permissions. | +| `org_test.go` | Org list/show + nested members/roles/permissions, each with `--json` shape assertions. | diff --git a/e2e/org_test.go b/e2e/org_test.go index c7c161d..e32226a 100644 --- a/e2e/org_test.go +++ b/e2e/org_test.go @@ -38,6 +38,40 @@ func TestOrgList(t *testing.T) { } } +// TestOrgShowJSON asserts --json returns an envelope keyed by `organization`. +func TestOrgShowJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("org", "show") + if err != nil { + t.Fatalf("org show --json: %v", err) + } + if _, ok := raw["organization"]; !ok { + // Some servers have wrapped this under `org` historically — accept + // either so the smoke doesn't flake on a minor rename. + if _, alt := raw["org"]; !alt { + t.Errorf("--json response missing organization envelope: %v", raw) + } + } +} + +// TestOrgListJSON asserts --json emits an `organizations` array. +func TestOrgListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("org", "list") + if err != nil { + t.Fatalf("org list --json: %v", err) + } + if _, ok := raw["organizations"]; !ok { + t.Errorf("--json response missing `organizations` key: %v", raw) + } +} + // TestOrgMembersList exercises the nested members verb. No assertion on // specific emails — just that the RPC round-trips and renders a header. func TestOrgMembersList(t *testing.T) { @@ -54,6 +88,21 @@ func TestOrgMembersList(t *testing.T) { } } +// TestOrgMembersListJSON asserts --json emits a `members` key. +func TestOrgMembersListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("org", "members", "list") + if err != nil { + t.Fatalf("org members list --json: %v", err) + } + if _, ok := raw["members"]; !ok { + t.Errorf("--json response missing `members` key: %v", raw) + } +} + // TestWhoami asserts the token resolves to an org that matches EXPECT_ORG. // The guard in Begin already enforces this; the test covers the whoami verb // render path separately so drift in GetMember shape fails loudly. @@ -73,3 +122,65 @@ func TestWhoami(t *testing.T) { t.Errorf("whoami missing email row: %s", out) } } + +// TestOrgRolesList renders ID/NAME for RBAC roles. +func TestOrgRolesList(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("org", "roles", "list") + if err != nil { + t.Fatalf("org roles list: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "ID") || !strings.Contains(out, "NAME") { + t.Errorf("org roles list missing header: %s", out) + } +} + +// TestOrgRolesListJSON asserts --json emits a `roles` key. +func TestOrgRolesListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("org", "roles", "list") + if err != nil { + t.Fatalf("org roles list --json: %v", err) + } + if _, ok := raw["roles"]; !ok { + t.Errorf("--json response missing `roles` key: %v", raw) + } +} + +// TestOrgPermissionsList renders the ID/NAME table for RBAC permissions. +// This surface is a static catalog for the org, so the header + at least +// one row are expected in any real org. +func TestOrgPermissionsList(t *testing.T) { + h := harness.Begin(t) + out, stderr, err := h.Run("org", "permissions", "list") + if err != nil { + t.Fatalf("org permissions list: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "ID") || !strings.Contains(out, "NAME") { + t.Errorf("org permissions list missing header: %s", out) + } +} + +// TestOrgPermissionsListJSON asserts --json emits a `permissions` key. +func TestOrgPermissionsListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("org", "permissions", "list") + if err != nil { + t.Fatalf("org permissions list --json: %v", err) + } + if _, ok := raw["permissions"]; !ok { + t.Errorf("--json response missing `permissions` key: %v", raw) + } +} From b1cfb6636ae7837cdd3015cecc53ceac059bbc91 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:12:49 -0500 Subject: [PATCH 08/17] feat(e2e): profile add/list/use/show/remove lifecycle - full round-trip through harness temp XDG using --token-stdin on add - list/show --json assert hasToken + active flags - error-path smokes: missing on add, unknown profile on use --- e2e/CLAUDE.md | 1 + e2e/profile_test.go | 149 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 e2e/profile_test.go diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index 3ba5172..f988523 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -18,4 +18,5 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `playbook_test.go` | Playbook list/get/reports/lineage read leaves (default + `--json`); id discovered via `list --json`. | | `ontology_test.go` | Ontology list/get read leaves (default + `--json`); id is integer on the wire. | | `feed_test.go` | Feed show + stats (default + `--json`). | +| `profile_test.go` | Profile add/list/use/show/remove round-trip through harness temp XDG; error-path smokes for missing name + unknown profile. | | `org_test.go` | Org list/show + nested members/roles/permissions, each with `--json` shape assertions. | diff --git a/e2e/profile_test.go b/e2e/profile_test.go new file mode 100644 index 0000000..1716fa7 --- /dev/null +++ b/e2e/profile_test.go @@ -0,0 +1,149 @@ +package e2e + +import ( + "strings" + "testing" + + "github.com/highperformance-tech/ana-cli/e2e/harness" +) + +// TestProfileLifecycle drives add -> list -> show -> use -> remove through +// `h.Run`/`h.RunStdin`. The harness already seeds a `default` profile into a +// t.TempDir() config (see e2e/harness/harness.go:Begin); this test adds a +// second profile alongside it so the list/use/remove branches get exercised. +// +// Every action goes through cli.Dispatch — no raw config writes — so the +// dispatch path is the one under test even though no RPCs are involved. +func TestProfileLifecycle(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + + const name = "e2e-temp" + // 1. add with --token-stdin (primary stdin path; inline --token would + // log the value and is deliberately not tested). + addOut, addErr, err := h.RunStdin("fake-token-value", "profile", "add", name, + "--endpoint", "https://example.invalid", "--org", "e2e-org", "--token-stdin") + if err != nil { + t.Fatalf("profile add: %v\nstderr: %s", err, addErr) + } + if !strings.Contains(addOut, "saved profile "+name) { + t.Errorf("profile add output missing 'saved profile' line: %s", addOut) + } + + // 2. list: new profile present, default still active. + listOut, _, err := h.Run("profile", "list") + if err != nil { + t.Fatalf("profile list: %v", err) + } + if !strings.Contains(listOut, name) { + t.Errorf("profile list missing new profile %q: %s", name, listOut) + } + if !strings.Contains(listOut, "default") { + t.Errorf("profile list missing seeded default: %s", listOut) + } + + // 3. list --json: `profiles` envelope + hasToken flag on the new entry. + raw, err := h.RunJSON("profile", "list") + if err != nil { + t.Fatalf("profile list --json: %v", err) + } + profiles, _ := raw["profiles"].([]any) + var found bool + for _, p := range profiles { + entry, _ := p.(map[string]any) + if n, _ := entry["name"].(string); n == name { + found = true + if has, _ := entry["hasToken"].(bool); !has { + t.Errorf("profile %q hasToken = false, want true", name) + } + } + } + if !found { + t.Errorf("profile list --json missing new profile %q: %v", name, raw) + } + + // 4. use: switch the active pointer. + useOut, _, err := h.Run("profile", "use", name) + if err != nil { + t.Fatalf("profile use: %v", err) + } + if !strings.Contains(useOut, "active profile: "+name) { + t.Errorf("profile use output: %s", useOut) + } + + // 5. show: defaults to the active profile, so should echo the new name. + showOut, _, err := h.Run("profile", "show") + if err != nil { + t.Fatalf("profile show: %v", err) + } + if !strings.Contains(showOut, name) { + t.Errorf("profile show missing %q: %s", name, showOut) + } + + // 6. show --json: active=true + hasToken=true on the renamed profile. + showRaw, err := h.RunJSON("profile", "show") + if err != nil { + t.Fatalf("profile show --json: %v", err) + } + if got, _ := showRaw["name"].(string); got != name { + t.Errorf("profile show --json name = %q, want %q", got, name) + } + if active, _ := showRaw["active"].(bool); !active { + t.Errorf("profile show --json active = false after use") + } + + // 7. remove: should also clear the active pointer since we just switched + // to the profile we're now deleting. + rmOut, _, err := h.Run("profile", "remove", name) + if err != nil { + t.Fatalf("profile remove: %v", err) + } + if !strings.Contains(rmOut, name) { + t.Errorf("profile remove output missing name: %s", rmOut) + } + + // 8. list after remove: the profile is gone. + gone, _, err := h.Run("profile", "list") + if err != nil { + t.Fatalf("profile list (post-remove): %v", err) + } + if strings.Contains(gone, name) { + t.Errorf("profile list still contains removed profile %q: %s", name, gone) + } +} + +// TestProfileAddMissingName asserts the usage-error path: dispatch should +// return a cli.ErrUsage when is absent. The harness's exit-code +// mapping surfaces this as a non-nil error with "name is required" in it. +func TestProfileAddMissingName(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + _, stderr, err := h.RunStdin("tok", "profile", "add", "--token-stdin") + if err == nil { + t.Fatalf("profile add with no name should fail; stderr=%s", stderr) + } + combined := err.Error() + stderr + if !strings.Contains(combined, "name is required") { + t.Errorf("expected 'name is required' in error; got err=%v stderr=%q", err, stderr) + } +} + +// TestProfileUseUnknown asserts `profile use ` fails with the +// ErrUnknownProfile sentinel surfaced by the dispatch layer. +func TestProfileUseUnknown(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + _, stderr, err := h.Run("profile", "use", "does-not-exist") + if err == nil { + t.Fatalf("profile use should fail; stderr=%s", stderr) + } + if !strings.Contains(err.Error(), "unknown profile") { + t.Errorf("expected unknown-profile error; got %v", err) + } +} From 9ec8e0889dab2846439b838d60770ebb6002463b Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:14:21 -0500 Subject: [PATCH 09/17] feat(e2e): CLI-driven auth keys/service-accounts + shape checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - keys create/rotate/revoke through h.Run — id recovered from `list --json` since the create verb only emits the plaintext token - service-accounts create/delete parses the ` ` line - list --json for keys + service-accounts asserts the envelope key - error-path smokes: missing --name on keys create, extra positional on keys rotate --- e2e/CLAUDE.md | 2 +- e2e/auth_test.go | 198 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 199 insertions(+), 1 deletion(-) diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index f988523..e4cd7fe 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -10,7 +10,7 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `harness/` | `H`, `Begin`/`End`, guarded mutations, resource factories, pre/post snapshot sweep. | | `testdata/` | Static fixtures — currently just the `manual-revert.md` template. | | `audit_test.go` | Audit-log `tail` smoke. | -| `auth_test.go` | Login/logout/whoami round-trip against a real org. | +| `auth_test.go` | Keys + service-accounts CLI-driven create/rotate/revoke/delete (helper-backed legacy tests still live here for coverage); `--json` shape checks + error-path smokes for usage guards. | | `chat_test.go` | Chat CRUD + streaming `send`. | | `connector_test.go` | Connector CRUD (create/update/test/delete) with ledger-backed cleanup. | | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | diff --git a/e2e/auth_test.go b/e2e/auth_test.go index 263e923..577ed78 100644 --- a/e2e/auth_test.go +++ b/e2e/auth_test.go @@ -7,6 +7,26 @@ import ( "github.com/highperformance-tech/ana-cli/e2e/harness" ) +// apiKeyIDByName walks `auth keys list --json` and returns the id of the +// row whose name matches. Empty return means "not found" — callers treat +// that as a fatal test error. +func apiKeyIDByName(t *testing.T, h *harness.H, name string) string { + t.Helper() + raw, err := h.RunJSON("auth", "keys", "list") + if err != nil { + t.Fatalf("auth keys list --json: %v", err) + } + arr, _ := raw["apiKeys"].([]any) + for _, item := range arr { + entry, _ := item.(map[string]any) + if n, _ := entry["name"].(string); n == name { + id, _ := entry["id"].(string) + return id + } + } + return "" +} + // TestAuthKeysListCreateRevoke is the Tier-1 golden chain for API keys. The // harness creates a test-prefixed key and defers the revoke; the body // verifies ListApiKeys sees it and that it survives JSON round-trip. @@ -66,3 +86,181 @@ func TestAuthServiceAccountsCreateDelete(t *testing.T) { t.Errorf("new service account not in list: %s", out) } } + +// TestAuthKeysListJSON asserts --json emits an `apiKeys` array. +func TestAuthKeysListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("auth", "keys", "list") + if err != nil { + t.Fatalf("auth keys list --json: %v", err) + } + if _, ok := raw["apiKeys"]; !ok { + t.Errorf("--json response missing `apiKeys` key: %v", raw) + } +} + +// TestAuthKeysCreateCLI drives `auth keys create` + `revoke` straight through +// the CLI. The id is recovered from `list --json` since the create verb +// intentionally only emits the plaintext token — see auth/keys.go's +// emitPlaintextToken. +func TestAuthKeysCreateCLI(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + _, _, _ = h.Run("auth", "keys", "create", "--name", h.ResourceName("cli-key")) + return + } + name := h.ResourceName("cli-key") + stdout, stderr, err := h.Run("auth", "keys", "create", "--name", name) + if err != nil { + t.Fatalf("auth keys create: %v\nstderr: %s", err, stderr) + } + plaintext := strings.TrimSpace(stdout) + if plaintext == "" { + t.Errorf("auth keys create: stdout empty (expected plaintext token)") + } + // Secret: do not echo plaintext back via t.Logf. The reminder we assert + // on goes to stderr. + if !strings.Contains(stderr, "will not be shown again") { + t.Errorf("auth keys create: missing 'will not be shown again' reminder on stderr: %s", stderr) + } + id := apiKeyIDByName(t, h, name) + if id == "" { + t.Fatalf("auth keys create: could not find %q in keys list", name) + } + h.Register(func() { + _, revErr, err := h.Run("auth", "keys", "revoke", id) + if err != nil { + h.RecordManualRevert("api_key:"+id, "revoke failed: "+err.Error()+" stderr="+revErr) + } + }) +} + +// TestAuthKeysRotateCLI drives the create -> rotate -> revoke chain through +// the CLI. The original id is rotated server-side (old revoked automatically) +// so cleanup targets the rotated id. The rotate call emits a fresh plaintext +// which must not be logged. +func TestAuthKeysRotateCLI(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + name := h.ResourceName("cli-rotate") + stdout, _, err := h.Run("auth", "keys", "create", "--name", name) + if err != nil { + t.Fatalf("auth keys create (for rotate): %v", err) + } + if strings.TrimSpace(stdout) == "" { + t.Errorf("create returned empty plaintext") + } + origID := apiKeyIDByName(t, h, name) + if origID == "" { + t.Fatalf("could not find created key %q in list", name) + } + rotateOut, rotateErr, err := h.Run("auth", "keys", "rotate", origID) + if err != nil { + t.Fatalf("auth keys rotate: %v\nstderr: %s", err, rotateErr) + } + if strings.TrimSpace(rotateOut) == "" { + t.Errorf("rotate returned empty plaintext") + } + if !strings.Contains(rotateErr, "will not be shown again") { + t.Errorf("rotate missing reminder on stderr: %s", rotateErr) + } + // The rotated key takes the same logical name, so look it up again. + newID := apiKeyIDByName(t, h, name) + if newID == "" { + t.Fatalf("could not find rotated key %q in list", name) + } + if newID == origID { + t.Errorf("rotate returned same id %q as original", origID) + } + h.Register(func() { + _, revErr, err := h.Run("auth", "keys", "revoke", newID) + if err != nil { + h.RecordManualRevert("api_key:"+newID, "post-rotate revoke failed: "+err.Error()+" stderr="+revErr) + } + }) +} + +// TestAuthServiceAccountsCreateCLI drives `service-accounts create` + +// `delete` through the CLI. The create command prints ` ` +// so we can parse the id directly without hitting --json. +func TestAuthServiceAccountsCreateCLI(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + name := h.ResourceName("cli-sa") + stdout, stderr, err := h.Run("auth", "service-accounts", "create", + "--name", name, "--description", "e2e temp") + if err != nil { + t.Fatalf("service-accounts create: %v\nstderr: %s", err, stderr) + } + fields := strings.Fields(strings.TrimSpace(stdout)) + if len(fields) < 2 { + t.Fatalf("service-accounts create: expected ' ', got %q", stdout) + } + id := fields[0] + h.Register(func() { + _, delErr, err := h.Run("auth", "service-accounts", "delete", id) + if err != nil { + h.RecordManualRevert("service_account:"+id, "delete failed: "+err.Error()+" stderr="+delErr) + } + }) + listOut, _, err := h.Run("auth", "service-accounts", "list") + if err != nil { + t.Fatalf("service-accounts list (post-create): %v", err) + } + if !strings.Contains(listOut, id) { + t.Errorf("service-accounts list missing new id %q: %s", id, listOut) + } +} + +// TestAuthServiceAccountsListJSON asserts --json emits `serviceAccounts`. +func TestAuthServiceAccountsListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("auth", "service-accounts", "list") + if err != nil { + t.Fatalf("service-accounts list --json: %v", err) + } + if _, ok := raw["serviceAccounts"]; !ok { + t.Errorf("--json response missing `serviceAccounts` key: %v", raw) + } +} + +// TestAuthKeysCreateMissingName asserts the usage-error path when --name +// is absent. +func TestAuthKeysCreateMissingName(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + _, stderr, err := h.Run("auth", "keys", "create") + if err == nil { + t.Fatalf("expected error for missing --name; stderr=%s", stderr) + } + if !strings.Contains(err.Error()+stderr, "name") { + t.Errorf("expected --name complaint; got err=%v stderr=%q", err, stderr) + } +} + +// TestAuthKeysRotateExtraArg asserts the "exactly one " guard fires. +func TestAuthKeysRotateExtraArg(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + _, stderr, err := h.Run("auth", "keys", "rotate", "one", "two") + if err == nil { + t.Fatalf("expected usage error for extra arg; stderr=%s", stderr) + } + if !strings.Contains(err.Error(), "exactly one") { + t.Errorf("expected 'exactly one' in error; got %v", err) + } +} From 17abfa6e84903d7c444ff42efec546773ba1d38c Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:18:05 -0500 Subject: [PATCH 10/17] feat(e2e): connector --json, CLI postgres create matrix, tables/examples/test smokes - list/get --json envelope assertions - postgres password leaf via --password-stdin with --ssl=false and --ssl=true - update --password-stdin + rename multi-flag merge - tables (env-gated on ANA_E2E_PG_HOST), examples, test (OK|FAIL:) leaves - get error-path smoke --- e2e/CLAUDE.md | 2 +- e2e/connector_test.go | 195 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+), 1 deletion(-) diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index e4cd7fe..d7706ee 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -12,7 +12,7 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `audit_test.go` | Audit-log `tail` smoke. | | `auth_test.go` | Keys + service-accounts CLI-driven create/rotate/revoke/delete (helper-backed legacy tests still live here for coverage); `--json` shape checks + error-path smokes for usage guards. | | `chat_test.go` | Chat CRUD + streaming `send`. | -| `connector_test.go` | Connector CRUD (create/update/test/delete) with ledger-backed cleanup. | +| `connector_test.go` | Connector CRUD + `--json` envelopes, CLI postgres create matrix (password-stdin × ssl on/off), `update --password-stdin`, `tables`/`examples`/`test` leaves, `get ` error-path. | | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | | `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | | `playbook_test.go` | Playbook list/get/reports/lineage read leaves (default + `--json`); id discovered via `list --json`. | diff --git a/e2e/connector_test.go b/e2e/connector_test.go index 650417d..2668576 100644 --- a/e2e/connector_test.go +++ b/e2e/connector_test.go @@ -93,3 +93,198 @@ func TestConnectorUpdate(t *testing.T) { t.Errorf("update did not take effect; get output: %s", out) } } + +// TestConnectorListJSON asserts the --json envelope is keyed by `connectors`. +func TestConnectorListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("connector", "list") + if err != nil { + t.Fatalf("connector list --json: %v", err) + } + if _, ok := raw["connectors"]; !ok { + t.Errorf("--json response missing `connectors` key: %v", raw) + } +} + +// TestConnectorGetJSON creates a throwaway connector via the harness helper +// and confirms --json returns a `connector` object whose id matches. +func TestConnectorGetJSON(t *testing.T) { + h := harness.Begin(t) + id := h.CreateConnector("get-json", connSpecFromEnv()) + if id == 0 { + if h.DryRun() { + return + } + t.Fatalf("CreateConnector returned id=0") + } + raw, err := h.RunJSON("connector", "get", fmt.Sprint(id)) + if err != nil { + t.Fatalf("connector get --json: %v", err) + } + conn, ok := raw["connector"].(map[string]any) + if !ok { + t.Fatalf("--json response missing `connector` object: %v", raw) + } + // id may decode as float64 (generic JSON) — compare numerically. + if n, _ := conn["id"].(float64); int(n) != id { + t.Errorf("connector.id = %v, want %d", conn["id"], id) + } +} + +// TestConnectorCreatePostgresCLI drives `connector create postgres password` +// with `--password-stdin`. Uses the shared connectorId regex (defined in +// connector_snowflake_test.go) — the output shape is identical across +// dialects. Explicit --ssl=false is sent so the boolean wire field is +// exercised without requiring TLS on the test target. +func TestConnectorCreatePostgresCLI(t *testing.T) { + h := harness.Begin(t) + spec := connSpecFromEnv() + args := []string{ + "connector", "create", "postgres", "password", + "--name", h.ResourceName("pg-cli"), + "--host", spec.Host, + "--port", fmt.Sprint(spec.Port), + "--user", spec.User, + "--database", spec.Database, + "--password-stdin", + "--ssl=false", + } + stdout, stderr, err := h.RunStdin(spec.Password+"\n", args...) + if err != nil { + t.Fatalf("connector create postgres password: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) + if !strings.Contains(stdout, "connectorType: POSTGRES") { + t.Errorf("stdout missing connectorType: POSTGRES:\n%s", stdout) + } +} + +// TestConnectorCreatePostgresCLISSL repeats the CLI create with --ssl=true so +// both booleans hit the wire at least once. Cleanup runs LIFO before the +// sibling test's. +func TestConnectorCreatePostgresCLISSL(t *testing.T) { + h := harness.Begin(t) + spec := connSpecFromEnv() + args := []string{ + "connector", "create", "postgres", "password", + "--name", h.ResourceName("pg-cli-ssl"), + "--host", spec.Host, + "--port", fmt.Sprint(spec.Port), + "--user", spec.User, + "--database", spec.Database, + "--password-stdin", + "--ssl", + } + stdout, stderr, err := h.RunStdin(spec.Password+"\n", args...) + if err != nil { + t.Fatalf("connector create postgres password --ssl: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + id := extractConnectorID(t, stdout) + h.RegisterConnectorCleanup(id) +} + +// TestConnectorUpdatePasswordStdin covers the update --password-stdin path +// — the only update flag combination that reads stdin. A rename happens +// alongside so the test confirms multi-flag updates still merge the +// pre-fetched baseline correctly. +func TestConnectorUpdatePasswordStdin(t *testing.T) { + h := harness.Begin(t) + id := h.CreateConnector("update-pwd", connSpecFromEnv()) + renamed := h.ResourceName("update-pwd-renamed") + _, stderr, err := h.RunStdin("new-password\n", "connector", "update", fmt.Sprint(id), + "--name", renamed, "--password-stdin") + if err != nil { + t.Fatalf("connector update --password-stdin: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + out, _, err := h.Run("connector", "get", fmt.Sprint(id)) + if err != nil { + t.Fatalf("connector get after update: %v", err) + } + if !strings.Contains(out, renamed) { + t.Errorf("rename not applied: %s", out) + } +} + +// TestConnectorTables runs `connector tables `. Requires +// ANA_E2E_PG_HOST because a connector pointing at `e2e.invalid` will time +// out the driver probe rather than surface a clean empty table. +func TestConnectorTables(t *testing.T) { + if os.Getenv("ANA_E2E_PG_HOST") == "" { + t.Skip("e2e: ANA_E2E_PG_HOST required for connector tables (driver must reach a real db)") + } + h := harness.Begin(t) + id := h.CreateConnector("tables", connSpecFromEnv()) + out, stderr, err := h.Run("connector", "tables", fmt.Sprint(id)) + if err != nil { + t.Fatalf("connector tables: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "SCHEMA") || !strings.Contains(out, "NAME") { + t.Errorf("connector tables missing header: %s", out) + } +} + +// TestConnectorExamples runs `connector examples ` against a throwaway +// connector. The endpoint works even if the db isn't reachable — example +// queries are server-side templates keyed off dialect — so no PG env gate. +func TestConnectorExamples(t *testing.T) { + h := harness.Begin(t) + id := h.CreateConnector("examples", connSpecFromEnv()) + _, stderr, err := h.Run("connector", "examples", fmt.Sprint(id)) + if err != nil { + t.Fatalf("connector examples: %v\nstderr: %s", err, stderr) + } + _ = stderr + // Output may be empty for a connector the server hasn't profiled yet; + // no content assertion — the smoke is "endpoint answers without error". +} + +// TestConnectorTest runs `connector test `. Accepts either `OK` or a +// `FAIL:` prefix because the connector points at `e2e.invalid` by default; +// on such a connector the driver probe fails with a real error message, +// which is still a valid contract response (see internal/connector/test.go +// — the command classifies either branch as non-error). +func TestConnectorTest(t *testing.T) { + h := harness.Begin(t) + id := h.CreateConnector("test", connSpecFromEnv()) + out, stderr, err := h.Run("connector", "test", fmt.Sprint(id)) + if err != nil { + t.Fatalf("connector test: %v\nstderr: %s", err, stderr) + } + if h.DryRun() { + return + } + if !strings.Contains(out, "OK") && !strings.Contains(out, "FAIL:") { + t.Errorf("connector test output should start with OK or FAIL: got %q", out) + } +} + +// TestConnectorGetNotFound covers the typed-error contract for a missing +// connector id. The server returns a Connect-RPC error that the dispatch +// layer surfaces as non-nil; exact message varies, so we only assert that +// the command exits non-zero. +func TestConnectorGetNotFound(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + _, _, err := h.Run("connector", "get", "999999999") + if err == nil { + t.Fatalf("connector get : expected error, got nil") + } +} From 15ef166f6eec0ede4f2cd35289d495a71fb78984 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:19:48 -0500 Subject: [PATCH 11/17] feat(e2e): chat list/show --json, CLI chat new, history, bookmark/unbookmark - list/get --json envelope assertions (chats array, chat object id round-trip) - chat new via h.Run with CLI-path delete cleanup and list round-trip - history smoke (endpoint answers without error for a fresh chat) - bookmark/unbookmark chain printing `ok` - chat show error-path smoke --- e2e/CLAUDE.md | 2 +- e2e/chat_test.go | 155 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index d7706ee..e18a5a0 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -11,7 +11,7 @@ Live-smoke tests that drive real `app.textql.com` RPCs through the same verb pac | `testdata/` | Static fixtures — currently just the `manual-revert.md` template. | | `audit_test.go` | Audit-log `tail` smoke. | | `auth_test.go` | Keys + service-accounts CLI-driven create/rotate/revoke/delete (helper-backed legacy tests still live here for coverage); `--json` shape checks + error-path smokes for usage guards. | -| `chat_test.go` | Chat CRUD + streaming `send`. | +| `chat_test.go` | Chat CRUD + streaming `send`, `--json` envelopes on list/show, CLI-path `chat new`, `history`/`bookmark`/`unbookmark`, `show ` error-path. | | `connector_test.go` | Connector CRUD + `--json` envelopes, CLI postgres create matrix (password-stdin × ssl on/off), `update --password-stdin`, `tables`/`examples`/`test` leaves, `get ` error-path. | | `connector_snowflake_test.go` | Snowflake create leaves (password/keypair/oauth-sso/oauth-individual), per-mode env-gated. | | `dashboard_test.go` | Dashboard list/get/folders read leaves (default + `--json`); `health`/`spawn` env-gated on `ANA_E2E_DASHBOARD_ID`. | diff --git a/e2e/chat_test.go b/e2e/chat_test.go index f80e68c..ef0848b 100644 --- a/e2e/chat_test.go +++ b/e2e/chat_test.go @@ -8,6 +8,25 @@ import ( "github.com/highperformance-tech/ana-cli/e2e/harness" ) +// chatIDByTitle walks `chat list --json` and returns the id of the first +// row whose summary equals the given title. Empty means not found. +func chatIDByTitle(t *testing.T, h *harness.H, title string) string { + t.Helper() + raw, err := h.RunJSON("chat", "list") + if err != nil { + t.Fatalf("chat list --json: %v", err) + } + arr, _ := raw["chats"].([]any) + for _, item := range arr { + entry, _ := item.(map[string]any) + if s, _ := entry["summary"].(string); s == title { + id, _ := entry["id"].(string) + return id + } + } + return "" +} + // TestChatNewShowDelete covers the Tier-1 create -> show -> delete chain. // Deferred DeleteChat is registered by CreateChat; test body only exercises // the read-side to confirm the chat actually lives server-side. @@ -121,3 +140,139 @@ func TestChatDuplicateDelete(t *testing.T) { } }) } + +// TestChatListJSON asserts --json emits a `chats` array. +func TestChatListJSON(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + raw, err := h.RunJSON("chat", "list") + if err != nil { + t.Fatalf("chat list --json: %v", err) + } + if _, ok := raw["chats"]; !ok { + t.Errorf("--json response missing `chats` key: %v", raw) + } +} + +// TestChatShowJSON confirms --json returns a `chat` object whose id matches. +func TestChatShowJSON(t *testing.T) { + h := harness.Begin(t) + conn := h.CreateConnector("show-json", connSpecFromEnv()) + chatID := h.CreateChat("show-json", []int{conn}) + if chatID == "" { + if h.DryRun() { + return + } + t.Fatalf("CreateChat returned empty id") + } + raw, err := h.RunJSON("chat", "show", chatID) + if err != nil { + t.Fatalf("chat show --json: %v", err) + } + chat, ok := raw["chat"].(map[string]any) + if !ok { + t.Fatalf("--json response missing `chat` object: %v", raw) + } + if got, _ := chat["id"].(string); got != chatID { + t.Errorf("chat.id = %q, want %q", got, chatID) + } +} + +// TestChatNewCLI drives `chat new --connector --title ` through +// the CLI. The command emits the new chat id on stdout; we register +// `chat delete` for cleanup since the harness's parent-nesting path was +// bypassed. +func TestChatNewCLI(t *testing.T) { + h := harness.Begin(t) + conn := h.CreateConnector("new-cli", connSpecFromEnv()) + if h.DryRun() { + _, _, _ = h.Run("chat", "new", "--connector", fmt.Sprint(conn), + "--title", h.ResourceName("new-cli")) + return + } + title := h.ResourceName("new-cli") + stdout, stderr, err := h.Run("chat", "new", + "--connector", fmt.Sprint(conn), + "--title", title) + if err != nil { + t.Fatalf("chat new: %v\nstderr: %s", err, stderr) + } + chatID := strings.TrimSpace(stdout) + if chatID == "" { + t.Fatalf("chat new: empty id on stdout") + } + h.Register(func() { + if _, delErr, err := h.Run("chat", "delete", chatID); err != nil { + h.RecordManualRevert( + fmt.Sprintf("chat id=%s", chatID), + fmt.Sprintf("CLI-path delete failed: %v stderr=%s", err, delErr), + ) + } + }) + // Round-trip: the new chat should show up in `chat list` under the title. + if id := chatIDByTitle(t, h, title); id != chatID { + t.Errorf("chat list lookup of %q = %q, want %q", title, id, chatID) + } +} + +// TestChatHistory runs `chat history ` against a freshly-created chat. +// A new chat with no messages yields an empty table; the smoke is that the +// endpoint returns without error. +func TestChatHistory(t *testing.T) { + h := harness.Begin(t) + conn := h.CreateConnector("hist", connSpecFromEnv()) + chatID := h.CreateChat("hist", []int{conn}) + if chatID == "" { + if h.DryRun() { + return + } + t.Fatalf("CreateChat returned empty id") + } + if _, stderr, err := h.Run("chat", "history", chatID); err != nil { + t.Fatalf("chat history: %v\nstderr: %s", err, stderr) + } +} + +// TestChatBookmarkUnbookmark exercises the bookmark + unbookmark chain. Both +// emit `ok` on success; deletion of the parent chat naturally clears any +// bookmark state, so no separate ledger entry is needed. +func TestChatBookmarkUnbookmark(t *testing.T) { + h := harness.Begin(t) + conn := h.CreateConnector("bm", connSpecFromEnv()) + chatID := h.CreateChat("bm", []int{conn}) + if chatID == "" { + if h.DryRun() { + return + } + t.Fatalf("CreateChat returned empty id") + } + bmOut, stderr, err := h.Run("chat", "bookmark", chatID) + if err != nil { + t.Fatalf("chat bookmark: %v\nstderr: %s", err, stderr) + } + if !strings.Contains(bmOut, "ok") { + t.Errorf("chat bookmark: expected ok on stdout, got %q", bmOut) + } + unbmOut, stderr, err := h.Run("chat", "unbookmark", chatID) + if err != nil { + t.Fatalf("chat unbookmark: %v\nstderr: %s", err, stderr) + } + if !strings.Contains(unbmOut, "ok") { + t.Errorf("chat unbookmark: expected ok on stdout, got %q", unbmOut) + } +} + +// TestChatShowNotFound covers the error-path: `chat show ` must +// exit non-zero. +func TestChatShowNotFound(t *testing.T) { + h := harness.Begin(t) + if h.DryRun() { + return + } + _, _, err := h.Run("chat", "show", "00000000-0000-0000-0000-000000000000") + if err == nil { + t.Fatalf("chat show : expected error, got nil") + } +} From b8b35ddb62fb133c10c5b9188dd5bc289e4acb47 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 17:20:44 -0500 Subject: [PATCH 12/17] docs(e2e): document postgres connector env vars and ANA_E2E_DASHBOARD_ID - Expand optional env table with ANA_E2E_DASHBOARD_ID (dashboard spawn/health gate) - Add a dedicated Postgres connector env section matching the Snowflake one - Spell out defaults (e2e.invalid / 5432 / e2e / postgres) and which vars gate `connector tables` (requires a reachable host) --- e2e/README.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/e2e/README.md b/e2e/README.md index 0bc7c85..e04329c 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -36,7 +36,22 @@ Optional env: |------------------------|----------------------------------------------------| | `ANA_E2E_DRYRUN=1` | Log planned mutations without issuing RPCs | | `ANA_E2E_SWEEP_ONLY=1` | Run the leftover-sweep only, then skip tests | -| `ANA_E2E_PG_HOST` etc. | Use a real postgres for connector tests (optional) | +| `ANA_E2E_DASHBOARD_ID` | Existing dashboard id to probe (`health`, `spawn`); unset skips those leaves | + +### Postgres connector env + +Connector tests default to a syntactically valid but unreachable spec +(`host=e2e.invalid`); `CreateConnector` accepts it because only `connector +test` actually dials the db. Set these to exercise the driver path against a +reachable Postgres: + +| Variable | Meaning | +|------------------------|---------------------------------------------------| +| `ANA_E2E_PG_HOST` | Hostname (default `e2e.invalid`; required for `connector tables`) | +| `ANA_E2E_PG_PORT` | TCP port (default `5432`) | +| `ANA_E2E_PG_USER` | Username (default `e2e`) | +| `ANA_E2E_PG_PASSWORD` | Password (default `e2e`); piped via `--password-stdin` | +| `ANA_E2E_PG_DATABASE` | Database name (default `postgres`) | ### Snowflake connector env From 0a98a91178bf92f5a3337fe7ab48fa43a7f7eaa0 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 20:02:19 -0500 Subject: [PATCH 13/17] docs(e2e): TODO ephemeral dashboard creation for health/spawn tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ephemeral is the correct pattern: the test should create its own dashboard and register a deferred delete. Blocked on two missing pieces: - `chat new --dashboard-mode` flag (CLI doesn't expose the dashboardMode wire field today, even though the struct carries it) - DashboardService delete endpoint (not in api-catalog yet — webapp must have it, but the path isn't known; can't cascade-clean without it) Until both land the tests stay env-gated on ANA_E2E_DASHBOARD_ID and skip when the var is unset. --- e2e/dashboard_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/e2e/dashboard_test.go b/e2e/dashboard_test.go index 988580e..b85e504 100644 --- a/e2e/dashboard_test.go +++ b/e2e/dashboard_test.go @@ -142,6 +142,15 @@ func TestDashboardGetJSON(t *testing.T) { // dashboard. Requires ANA_E2E_DASHBOARD_ID because an arbitrary dashboard may // never have been spawned — in which case the server returns a non-contract // error rather than a health row — and we don't want a flaky assertion. +// +// TODO(e2e): create the dashboard ephemerally inside the test (chat with +// dashboardMode=true + "publish a sin(x) dashboard" prompt + parse id from +// the stream output), register a deferred delete, and drop the +// ANA_E2E_DASHBOARD_ID env var entirely. Blocked on: CLI doesn't expose +// `chat new --dashboard-mode`, and DashboardService/Delete* isn't captured +// in api-catalog yet — need to identify the real delete endpoint so the +// harness can cascade-clean the dashboard the chat publishes. Until that +// lands, this test stays env-gated and skips when the var is unset. func TestDashboardHealth(t *testing.T) { id := os.Getenv("ANA_E2E_DASHBOARD_ID") if id == "" { @@ -167,6 +176,11 @@ func TestDashboardHealth(t *testing.T) { // not create a new dashboard row — spawn just refreshes the runtime for an // existing dashboard — so no ledger cleanup is needed. Requires an explicit // env-gated id since spawning touches billed runtime quotas. +// +// TODO(e2e): same as TestDashboardHealth — replace the env-gated id with an +// ephemeral chat-published dashboard + deferred delete once the delete +// endpoint is captured. Leaving dashboards in the org is the wrong pattern; +// every e2e resource should be fully self-contained. func TestDashboardSpawn(t *testing.T) { id := os.Getenv("ANA_E2E_DASHBOARD_ID") if id == "" { From 433fc1e50a8abcc96c0a6a8b1e4f1f9a567a0e80 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Tue, 21 Apr 2026 20:43:46 -0500 Subject: [PATCH 14/17] fix(e2e): atomic create-then-cleanup + endpoint threading Address PR #18 review: - harness/resources.go: add RegisterConnectorCleanupByName, RegisterAPIKeyCleanupByName, RegisterServiceAccountCleanupByName so callers can pre-register a name-based safety net before mutating. LIFO ordering makes the by-name cleanup a no-op when the id-based cleanup that runs after a successful parse already covered it. - auth/connector/snowflake CLI tests: pre-register name-based cleanup before invoking the CLI so a failed id-extraction can't orphan a resource server-side. - harness/client.go + harness.go: thread resolved endpoint into buildVerbs so the connector verb sees the same value the harness already resolved (was reading os.Getenv twice). - chat_test.go: rename stderr capture var to delStderr for accuracy. - dashboard_test.go: fail fast on empty --json stdout instead of silently returning. --- e2e/auth_test.go | 7 ++ e2e/chat_test.go | 4 +- e2e/connector_snowflake_test.go | 6 ++ e2e/connector_test.go | 12 +++- e2e/dashboard_test.go | 4 +- e2e/harness/CLAUDE.md | 2 +- e2e/harness/client.go | 6 +- e2e/harness/harness.go | 2 +- e2e/harness/resources.go | 118 ++++++++++++++++++++++++++++++++ 9 files changed, 151 insertions(+), 10 deletions(-) diff --git a/e2e/auth_test.go b/e2e/auth_test.go index 577ed78..80bdeb0 100644 --- a/e2e/auth_test.go +++ b/e2e/auth_test.go @@ -113,6 +113,9 @@ func TestAuthKeysCreateCLI(t *testing.T) { return } name := h.ResourceName("cli-key") + // Pre-register the name-based safety net so a successful create followed + // by a failing list-lookup or assertion can't leak a real API key. + h.RegisterAPIKeyCleanupByName(name) stdout, stderr, err := h.Run("auth", "keys", "create", "--name", name) if err != nil { t.Fatalf("auth keys create: %v\nstderr: %s", err, stderr) @@ -148,6 +151,9 @@ func TestAuthKeysRotateCLI(t *testing.T) { return } name := h.ResourceName("cli-rotate") + // Same-name safety net works across the rotate: the rotated key keeps + // the logical name, so list-by-name will find whichever id is current. + h.RegisterAPIKeyCleanupByName(name) stdout, _, err := h.Run("auth", "keys", "create", "--name", name) if err != nil { t.Fatalf("auth keys create (for rotate): %v", err) @@ -194,6 +200,7 @@ func TestAuthServiceAccountsCreateCLI(t *testing.T) { return } name := h.ResourceName("cli-sa") + h.RegisterServiceAccountCleanupByName(name) stdout, stderr, err := h.Run("auth", "service-accounts", "create", "--name", name, "--description", "e2e temp") if err != nil { diff --git a/e2e/chat_test.go b/e2e/chat_test.go index ef0848b..acb6085 100644 --- a/e2e/chat_test.go +++ b/e2e/chat_test.go @@ -204,10 +204,10 @@ func TestChatNewCLI(t *testing.T) { t.Fatalf("chat new: empty id on stdout") } h.Register(func() { - if _, delErr, err := h.Run("chat", "delete", chatID); err != nil { + if _, delStderr, err := h.Run("chat", "delete", chatID); err != nil { h.RecordManualRevert( fmt.Sprintf("chat id=%s", chatID), - fmt.Sprintf("CLI-path delete failed: %v stderr=%s", err, delErr), + fmt.Sprintf("CLI-path delete failed: %v stderr=%s", err, delStderr), ) } }) diff --git a/e2e/connector_snowflake_test.go b/e2e/connector_snowflake_test.go index 0450ae2..2fdb44a 100644 --- a/e2e/connector_snowflake_test.go +++ b/e2e/connector_snowflake_test.go @@ -95,6 +95,9 @@ func TestConnectorCreateSnowflakePassword(t *testing.T) { } h := harness.Begin(t) + // Pre-register a name-based safety-net cleanup so a successful create + // followed by a failing extractConnectorID can't orphan the connector. + h.RegisterConnectorCleanupByName(h.ResourceName("sf-password")) args := append([]string{"connector", "create", "snowflake", "password"}, snowflakeCommonArgs(h, "sf-password", common)...) args = append(args, "--user", user, "--password-stdin") @@ -130,6 +133,7 @@ func TestConnectorCreateSnowflakeKeypair(t *testing.T) { passphrase := os.Getenv("ANA_E2E_SF_PRIVATE_KEY_PASSPHRASE") h := harness.Begin(t) + h.RegisterConnectorCleanupByName(h.ResourceName("sf-keypair")) args := append([]string{"connector", "create", "snowflake", "keypair"}, snowflakeCommonArgs(h, "sf-keypair", common)...) args = append(args, "--user", user, "--private-key-file", keyPath) @@ -165,6 +169,7 @@ func TestConnectorCreateSnowflakeOAuthSSO(t *testing.T) { } h := harness.Begin(t) + h.RegisterConnectorCleanupByName(h.ResourceName("sf-oauth-sso")) args := append([]string{"connector", "create", "snowflake", "oauth-sso"}, snowflakeCommonArgs(h, "sf-oauth-sso", common)...) args = append(args, "--oauth-client-id", clientID, "--oauth-client-secret-stdin") @@ -202,6 +207,7 @@ func TestConnectorCreateSnowflakeOAuthIndividual(t *testing.T) { } h := harness.Begin(t) + h.RegisterConnectorCleanupByName(h.ResourceName("sf-oauth-individual")) args := append([]string{"connector", "create", "snowflake", "oauth-individual"}, snowflakeCommonArgs(h, "sf-oauth-individual", common)...) args = append(args, "--oauth-client-id", clientID, "--oauth-client-secret-stdin") diff --git a/e2e/connector_test.go b/e2e/connector_test.go index 2668576..7569a5a 100644 --- a/e2e/connector_test.go +++ b/e2e/connector_test.go @@ -142,9 +142,15 @@ func TestConnectorGetJSON(t *testing.T) { func TestConnectorCreatePostgresCLI(t *testing.T) { h := harness.Begin(t) spec := connSpecFromEnv() + name := h.ResourceName("pg-cli") + // Register the name-based safety net BEFORE running the CLI: if the create + // succeeds server-side but extractConnectorID later fails, the by-name + // cleanup catches the orphan. The id-based cleanup registered after a + // successful parse runs first (LIFO), making this a no-op on the happy path. + h.RegisterConnectorCleanupByName(name) args := []string{ "connector", "create", "postgres", "password", - "--name", h.ResourceName("pg-cli"), + "--name", name, "--host", spec.Host, "--port", fmt.Sprint(spec.Port), "--user", spec.User, @@ -172,9 +178,11 @@ func TestConnectorCreatePostgresCLI(t *testing.T) { func TestConnectorCreatePostgresCLISSL(t *testing.T) { h := harness.Begin(t) spec := connSpecFromEnv() + name := h.ResourceName("pg-cli-ssl") + h.RegisterConnectorCleanupByName(name) args := []string{ "connector", "create", "postgres", "password", - "--name", h.ResourceName("pg-cli-ssl"), + "--name", name, "--host", spec.Host, "--port", fmt.Sprint(spec.Port), "--user", spec.User, diff --git a/e2e/dashboard_test.go b/e2e/dashboard_test.go index b85e504..3e143f4 100644 --- a/e2e/dashboard_test.go +++ b/e2e/dashboard_test.go @@ -86,8 +86,8 @@ func TestDashboardFoldersListJSON(t *testing.T) { if err != nil { t.Fatalf("dashboard folders list --json: %v\nstderr: %s", err, stderr) } - if stdout == "" { - return + if strings.TrimSpace(stdout) == "" { + t.Fatalf("dashboard folders list --json: empty stdout (expected JSON envelope)") } var raw map[string]any if err := json.Unmarshal([]byte(stdout), &raw); err != nil { diff --git a/e2e/harness/CLAUDE.md b/e2e/harness/CLAUDE.md index daaaa55..bbedcee 100644 --- a/e2e/harness/CLAUDE.md +++ b/e2e/harness/CLAUDE.md @@ -8,6 +8,6 @@ Per-test scaffolding for live smoke tests against a real TextQL endpoint. Duplic - `client.go` — mirrors `cmd/ana/main.go`'s verb builder so harness and binary share the same wiring shape. - `guard.go` — wraps mutating RPCs: records them on the ledger before invoking, aborts if the pre-flight guard fails (wrong org, missing env, etc.). - `ledger.go` — `ManualRevertLog` + `Record`/`Close`. Writes any unreverted mutation using `e2e/testdata/manual-revert.template.md`. -- `resources.go` — factories for throwaway connectors, chats, profiles, etc. Every factory registers its own revert with the harness. +- `resources.go` — factories for throwaway connectors, chats, profiles, etc. Every factory registers its own revert with the harness; CLI-driven tests can also pre-register name-based safety-net cleanups (`RegisterConnectorCleanupByName`, `RegisterAPIKeyCleanupByName`, `RegisterServiceAccountCleanupByName`) for the gap between create and id-extraction. - `snapshot.go` — captures the pre-test state of the target org (connector list, etc.) so `sweep.go` can prove nothing leaked. - `sweep.go` — post-test diff of snapshot vs. current state; fails the test if anything new slipped past the ledger. diff --git a/e2e/harness/client.go b/e2e/harness/client.go index dab9246..32124ed 100644 --- a/e2e/harness/client.go +++ b/e2e/harness/client.go @@ -85,12 +85,14 @@ func makeEnv(configHome string) func(string) string { // buildVerbs duplicates cmd/ana/main.go:buildVerbs. The logic is kept in sync // by hand; TestBuildVerbs_Shape in cmd/ana/main_test.go guards the verb set. -func buildVerbs(client *transport.Client, env func(string) string, cfgPath string) map[string]cli.Command { +// `endpoint` is threaded through (rather than re-read from the environment) +// so the connector verb sees the same value the harness already resolved. +func buildVerbs(client *transport.Client, env func(string) string, cfgPath, endpoint string) map[string]cli.Command { return map[string]cli.Command{ "auth": auth.New(authDeps(client, env, cfgPath)), "profile": profile.New(profileDeps(env, cfgPath)), "org": org.New(org.Deps{Unary: client.Unary}), - "connector": connector.New(connector.Deps{Unary: client.Unary, Endpoint: os.Getenv("ANA_E2E_ENDPOINT")}), + "connector": connector.New(connector.Deps{Unary: client.Unary, Endpoint: endpoint}), "chat": chat.New(chatDeps(client)), "dashboard": dashboard.New(dashboard.Deps{Unary: client.Unary}), "playbook": playbook.New(playbook.Deps{Unary: client.Unary}), diff --git a/e2e/harness/harness.go b/e2e/harness/harness.go index f38da47..0732eb4 100644 --- a/e2e/harness/harness.go +++ b/e2e/harness/harness.go @@ -62,7 +62,7 @@ func Begin(t *testing.T) *H { client := buildTransport(env.endpoint, env.token) envFn := makeEnv(dir) - verbs := buildVerbs(client, envFn, cfgPath) + verbs := buildVerbs(client, envFn, cfgPath, env.endpoint) h := &H{ t: t, diff --git a/e2e/harness/resources.go b/e2e/harness/resources.go index 18f1229..f58994b 100644 --- a/e2e/harness/resources.go +++ b/e2e/harness/resources.go @@ -67,6 +67,49 @@ func (h *H) RegisterConnectorCleanup(id int) { h.Register(func() { h.deleteConnector(id) }) } +// RegisterConnectorCleanupByName registers a best-effort cleanup that, at End +// time, lists connectors and deletes any whose name equals `name`. Tests that +// drive `connector create` through the CLI register this BEFORE invoking the +// command so an orphan can't escape if id-extraction from stdout fails after +// the server has already created the row. The id-based cleanup registered +// after a successful parse runs first (LIFO); this helper then no-ops on the +// follow-up list because the row is already gone. +func (h *H) RegisterConnectorCleanupByName(name string) { + h.Register(func() { h.deleteConnectorByName(name) }) +} + +func (h *H) deleteConnectorByName(name string) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + var resp struct { + Connectors []struct { + ID int `json:"id"` + Name string `json:"name"` + } `json:"connectors"` + } + const listPath = "/rpc/public/textql.rpc.public.connector.ConnectorService/GetConnectors" + if err := h.client.Unary(ctx, listPath, struct{}{}, &resp); err != nil { + // Non-fatal: the by-name cleanup is a safety net. A list failure here + // most likely means the test was already in trouble; surface but don't + // fail cleanup itself. + h.t.Logf("cleanup-by-name list connectors (%s): %v", name, err) + return + } + for _, c := range resp.Connectors { + if c.Name != name { + continue + } + const delPath = "/rpc/public/textql.rpc.public.connector.ConnectorService/DeleteConnector" + if err := h.client.Unary(ctx, delPath, map[string]any{"connectorId": c.ID}, nil); err != nil { + h.t.Errorf("cleanup-by-name DeleteConnector(%s id=%d): %v", name, c.ID, err) + h.RecordManualRevert( + fmt.Sprintf("connector id=%d name=%s", c.ID, name), + fmt.Sprintf("by-name auto-delete failed: %v", err), + ) + } + } +} + func (h *H) deleteConnector(id int) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -83,6 +126,81 @@ func (h *H) deleteConnector(id int) { } } +// RegisterAPIKeyCleanupByName registers a best-effort cleanup that, at End +// time, lists api keys and revokes any whose name equals `name`. Tests that +// drive `auth keys create` through the CLI call this BEFORE the create so an +// orphan key can't survive if the post-create lookup or assertion errors out. +func (h *H) RegisterAPIKeyCleanupByName(name string) { + h.Register(func() { h.revokeAPIKeyByName(name) }) +} + +func (h *H) revokeAPIKeyByName(name string) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + var resp struct { + APIKeys []struct { + ID string `json:"id"` + Name string `json:"name"` + } `json:"apiKeys"` + } + const listPath = "/rpc/public/textql.rpc.public.rbac.RBACService/ListApiKeys" + if err := h.client.Unary(ctx, listPath, struct{}{}, &resp); err != nil { + h.t.Logf("cleanup-by-name list api keys (%s): %v", name, err) + return + } + for _, k := range resp.APIKeys { + if k.Name != name { + continue + } + const revPath = "/rpc/public/textql.rpc.public.rbac.RBACService/RevokeApiKey" + if err := h.client.Unary(ctx, revPath, map[string]any{"apiKeyId": k.ID}, nil); err != nil && !isNotFound(err) { + h.t.Errorf("cleanup-by-name RevokeApiKey(%s id=%s): %v", name, k.ID, err) + h.RecordManualRevert( + fmt.Sprintf("api key id=%s name=%s", k.ID, name), + fmt.Sprintf("by-name auto-revoke failed: %v", err), + ) + } + } +} + +// RegisterServiceAccountCleanupByName registers a best-effort cleanup that, +// at End time, lists service accounts and deletes any whose displayName +// equals `name`. CLI-driven create tests call this before the create so a +// successful create followed by a failed post-create lookup cannot orphan +// the SA. +func (h *H) RegisterServiceAccountCleanupByName(name string) { + h.Register(func() { h.deleteServiceAccountByName(name) }) +} + +func (h *H) deleteServiceAccountByName(name string) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + var resp struct { + ServiceAccounts []struct { + MemberID string `json:"memberId"` + DisplayName string `json:"displayName"` + } `json:"serviceAccounts"` + } + const listPath = "/rpc/public/textql.rpc.public.rbac.RBACService/ListServiceAccounts" + if err := h.client.Unary(ctx, listPath, struct{}{}, &resp); err != nil { + h.t.Logf("cleanup-by-name list service accounts (%s): %v", name, err) + return + } + for _, sa := range resp.ServiceAccounts { + if sa.DisplayName != name { + continue + } + const delPath = "/rpc/public/textql.rpc.public.rbac.RBACService/DeleteServiceAccount" + if err := h.client.Unary(ctx, delPath, map[string]any{"memberId": sa.MemberID}, nil); err != nil && !isNotFound(err) { + h.t.Errorf("cleanup-by-name DeleteServiceAccount(%s memberId=%s): %v", name, sa.MemberID, err) + h.RecordManualRevert( + fmt.Sprintf("service account memberId=%s name=%s", sa.MemberID, name), + fmt.Sprintf("by-name auto-delete failed: %v", err), + ) + } + } +} + // CreateChat posts CreateChat bound to the given connector ids, returning // the chat id. Cleanup cascades to any child messages, shares, etc. func (h *H) CreateChat(suffix string, connectorIDs []int) string { From 9a6ac93b3c6b8c07221777cade36638ea2931a33 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 10:29:55 -0500 Subject: [PATCH 15/17] fix(e2e): ledger by-name cleanup list failures When the by-name cleanup's list call fails, record a manual-revert entry so any orphan resource is visible to operators. Previously the failure was logged and swallowed, which violates the harness contract that anything unreverted lands in manual-revert.md. Addresses PR #18 review. --- e2e/harness/resources.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/e2e/harness/resources.go b/e2e/harness/resources.go index f58994b..e8f4d78 100644 --- a/e2e/harness/resources.go +++ b/e2e/harness/resources.go @@ -91,8 +91,12 @@ func (h *H) deleteConnectorByName(name string) { if err := h.client.Unary(ctx, listPath, struct{}{}, &resp); err != nil { // Non-fatal: the by-name cleanup is a safety net. A list failure here // most likely means the test was already in trouble; surface but don't - // fail cleanup itself. + // fail cleanup itself. Ledger it so any orphan is visible to operators. h.t.Logf("cleanup-by-name list connectors (%s): %v", name, err) + h.RecordManualRevert( + fmt.Sprintf("connector name=%s", name), + fmt.Sprintf("by-name cleanup list failed: %v", err), + ) return } for _, c := range resp.Connectors { @@ -146,6 +150,10 @@ func (h *H) revokeAPIKeyByName(name string) { const listPath = "/rpc/public/textql.rpc.public.rbac.RBACService/ListApiKeys" if err := h.client.Unary(ctx, listPath, struct{}{}, &resp); err != nil { h.t.Logf("cleanup-by-name list api keys (%s): %v", name, err) + h.RecordManualRevert( + fmt.Sprintf("api key name=%s", name), + fmt.Sprintf("by-name cleanup list failed: %v", err), + ) return } for _, k := range resp.APIKeys { @@ -184,6 +192,10 @@ func (h *H) deleteServiceAccountByName(name string) { const listPath = "/rpc/public/textql.rpc.public.rbac.RBACService/ListServiceAccounts" if err := h.client.Unary(ctx, listPath, struct{}{}, &resp); err != nil { h.t.Logf("cleanup-by-name list service accounts (%s): %v", name, err) + h.RecordManualRevert( + fmt.Sprintf("service account name=%s", name), + fmt.Sprintf("by-name cleanup list failed: %v", err), + ) return } for _, sa := range resp.ServiceAccounts { From 7aa4c8ee1b75fc1f88ef72e4f475d0ca397a1a23 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 10:38:37 -0500 Subject: [PATCH 16/17] fix(e2e): tolerate 404 in connector by-name cleanup Match the API-key and service-account by-name variants: a 404 on DeleteConnector means the id-based cleanup (which runs first LIFO) already got the row, which is the happy path. Was tripping false manual-revert entries for every successful CLI create. Addresses PR #18 review. --- e2e/harness/resources.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/e2e/harness/resources.go b/e2e/harness/resources.go index e8f4d78..065021e 100644 --- a/e2e/harness/resources.go +++ b/e2e/harness/resources.go @@ -104,7 +104,10 @@ func (h *H) deleteConnectorByName(name string) { continue } const delPath = "/rpc/public/textql.rpc.public.connector.ConnectorService/DeleteConnector" - if err := h.client.Unary(ctx, delPath, map[string]any{"connectorId": c.ID}, nil); err != nil { + // 404 is expected when the id-based cleanup (registered after a + // successful parse) ran first and already deleted the row — the + // by-name helper is just the safety net. + if err := h.client.Unary(ctx, delPath, map[string]any{"connectorId": c.ID}, nil); err != nil && !isNotFound(err) { h.t.Errorf("cleanup-by-name DeleteConnector(%s id=%d): %v", name, c.ID, err) h.RecordManualRevert( fmt.Sprintf("connector id=%d name=%s", c.ID, name), From 1be2340d83b65dfffe25f8ad1751a02dda8ae483 Mon Sep 17 00:00:00 2001 From: Brad Fair Date: Wed, 22 Apr 2026 10:56:08 -0500 Subject: [PATCH 17/17] fix(e2e): guard cleanup registration helpers against dryRun and invalid ids Register{Connector,APIKey,ServiceAccount}CleanupByName and RegisterConnectorCleanup now short-circuit when ANA_E2E_DRYRUN=1 (or when the argument is zero/empty), so the helpers can never schedule a real RPC from End() when no mutation actually happened. --- e2e/harness/resources.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/e2e/harness/resources.go b/e2e/harness/resources.go index 065021e..2190c9a 100644 --- a/e2e/harness/resources.go +++ b/e2e/harness/resources.go @@ -62,8 +62,13 @@ func (h *H) CreateConnector(suffix string, spec ConnSpec) int { // RegisterConnectorCleanup registers a LIFO cleanup that DeleteConnectors id. // Tests that create a connector via `h.Run("connector","create",…)` (rather // than the raw-RPC CreateConnector helper) call this after parsing the id -// out of stdout so the ledger still owns the teardown. +// out of stdout so the ledger still owns the teardown. No-op under +// ANA_E2E_DRYRUN and for invalid ids (<= 0) so the helper can't schedule a +// real RPC when no mutation actually happened. func (h *H) RegisterConnectorCleanup(id int) { + if h.env.dryRun || id <= 0 { + return + } h.Register(func() { h.deleteConnector(id) }) } @@ -73,8 +78,13 @@ func (h *H) RegisterConnectorCleanup(id int) { // command so an orphan can't escape if id-extraction from stdout fails after // the server has already created the row. The id-based cleanup registered // after a successful parse runs first (LIFO); this helper then no-ops on the -// follow-up list because the row is already gone. +// follow-up list because the row is already gone. No-op under ANA_E2E_DRYRUN +// and for empty names so the helper can't schedule a real RPC when no +// mutation actually happened. func (h *H) RegisterConnectorCleanupByName(name string) { + if h.env.dryRun || name == "" { + return + } h.Register(func() { h.deleteConnectorByName(name) }) } @@ -137,7 +147,11 @@ func (h *H) deleteConnector(id int) { // time, lists api keys and revokes any whose name equals `name`. Tests that // drive `auth keys create` through the CLI call this BEFORE the create so an // orphan key can't survive if the post-create lookup or assertion errors out. +// No-op under ANA_E2E_DRYRUN and for empty names. func (h *H) RegisterAPIKeyCleanupByName(name string) { + if h.env.dryRun || name == "" { + return + } h.Register(func() { h.revokeAPIKeyByName(name) }) } @@ -178,8 +192,12 @@ func (h *H) revokeAPIKeyByName(name string) { // at End time, lists service accounts and deletes any whose displayName // equals `name`. CLI-driven create tests call this before the create so a // successful create followed by a failed post-create lookup cannot orphan -// the SA. +// the SA. No-op under ANA_E2E_DRYRUN and for empty names so the helper +// can't schedule a real RPC when no mutation actually happened. func (h *H) RegisterServiceAccountCleanupByName(name string) { + if h.env.dryRun || name == "" { + return + } h.Register(func() { h.deleteServiceAccountByName(name) }) }