|
| 1 | +--- |
| 2 | +name: hookdeck-cli-manual-qa |
| 3 | +description: >- |
| 4 | + Runs exploratory manual QA of the Hookdeck CLI and its MCP servers against |
| 5 | + real acceptance-test projects, safely. Provides a credential guard that keeps |
| 6 | + destructive commands off any project not named up front and never touches the |
| 7 | + operator's own login, an MCP stdio driver, per-surface checklists, and how to |
| 8 | + choose what to test when nobody hands you a list. Use when manually testing |
| 9 | + CLI commands or MCP tools, exercising write paths, validating a release |
| 10 | + candidate, or reproducing a defect against the live API. |
| 11 | +--- |
| 12 | + |
| 13 | +# Hookdeck CLI — manual QA |
| 14 | + |
| 15 | +Exploratory testing against the live API, in acceptance-test projects, without |
| 16 | +putting anything else at risk. |
| 17 | + |
| 18 | +This exists because the automated suites cannot cover everything. They run in |
| 19 | +fixed shapes; a QA pass goes looking for the things nobody thought to assert — |
| 20 | +wrong success messages, defaults that never get sent, filters that silently do |
| 21 | +nothing. Every defect this process has found so far was of one kind: **a wrong |
| 22 | +answer that reads like a right one.** Look for those specifically. |
| 23 | + |
| 24 | +## Safety model — read before running anything |
| 25 | + |
| 26 | +Manual QA runs destructive commands against real projects. Two rules make that |
| 27 | +survivable, and both are enforced by [scripts/hd-testenv.sh](scripts/hd-testenv.sh): |
| 28 | + |
| 29 | +1. **Never use the default config.** Every command takes |
| 30 | + `--hookdeck-config "$HD_CONFIG"`. The operator's own `hookdeck login` is |
| 31 | + never read or overwritten. This is not hypothetical — a suite run without an |
| 32 | + isolated config has previously written CI credentials over a working login. |
| 33 | +2. **The project is confirmed from the credential, not from the argument.** You |
| 34 | + name the project you expect up front; the guard authenticates, resolves the |
| 35 | + project, and refuses to hand back a config if it is anything else or is |
| 36 | + outside the acceptance-test organisation. |
| 37 | + |
| 38 | +Credentials come only from `test/acceptance/.env` (gitignored, CI test keys), so |
| 39 | +the reachable blast radius is the test projects those keys can see. |
| 40 | + |
| 41 | +**Before any delete, call `hd_assert "$HD_PROJECT"` and abort on failure.** It is |
| 42 | +one cheap line and it catches a config swapped or a project switched mid-run. |
| 43 | + |
| 44 | +If the guard refuses, **stop and read the message**. It fails closed on purpose; |
| 45 | +working around it defeats the point. |
| 46 | + |
| 47 | +### Record every config file before you start, and check them at the end |
| 48 | + |
| 49 | +The two rules above protect the config the guard hands you. They cannot protect |
| 50 | +a config the guard does not know about, and in pass 1 that gap cost the operator |
| 51 | +a working config: `hookdeck project use` honoured `--hookdeck-config` for reads |
| 52 | +but re-derived its own write target, so it overwrote the `.hookdeck/config.toml` |
| 53 | +that happened to be in the working directory and lost its contents (#424). |
| 54 | + |
| 55 | +Take the before state, and compare it after: |
| 56 | + |
| 57 | +```bash |
| 58 | +BEFORE="${TMPDIR:-/tmp}/hd-qa-configs.before" |
| 59 | +for c in .hookdeck/config.toml "$HOME/.config/hookdeck/config.toml"; do |
| 60 | + [ -f "$c" ] && printf '%s %s\n' "$(md5 -q "$c")" "$c" |
| 61 | +done | tee "$BEFORE" |
| 62 | +ls -l .hookdeck/config.toml "$HOME/.config/hookdeck/config.toml" 2>/dev/null |
| 63 | +``` |
| 64 | + |
| 65 | +Re-run the same block at the end and diff it against `$BEFORE`; that is what let |
| 66 | +pass 2 state "nothing outside the guarded config was touched" as evidence rather |
| 67 | +than as a hope. **Compare hashes; never `cat` a config** — the CLI writes |
| 68 | +single-quoted TOML, so a redaction pattern matching only double quotes prints |
| 69 | +the key verbatim. |
| 70 | + |
| 71 | +The underlying bug is fixed on this branch (#424, commit `3fc8124`), so |
| 72 | +`--hookdeck-config` can now be trusted for writes as well as reads and |
| 73 | +`pkg/cmd/project_use_config_target_test.go` keeps it that way. Do the check |
| 74 | +anyway: two lines, and the next such bug will not announce itself either. |
| 75 | + |
| 76 | +### Setup |
| 77 | + |
| 78 | +```bash |
| 79 | +source .agents/skills/hookdeck-cli-manual-qa/scripts/hd-testenv.sh |
| 80 | +hd_testenv HOOKDECK_CLI_TESTING_API_KEY tm_xxxxxxxxxxxx # gateway project |
| 81 | +hd_testenv HOOKDECK_CLI_OUTPOST_TESTING_API_KEY tm_yyyyyyyyyyyy # outpost project |
| 82 | + |
| 83 | +go build -o hookdeck . # test the build, not `go run` each time |
| 84 | +./hookdeck --hookdeck-config "$HD_CONFIG" whoami |
| 85 | +``` |
| 86 | + |
| 87 | +**Rebuild before every pass, and after every fix.** A stale binary reports the |
| 88 | +old behaviour convincingly: missing tools, missing flags, old names. If |
| 89 | +something looks broken in a way that seems too fundamental to be true, check |
| 90 | +that first — it has already produced one false finding. |
| 91 | + |
| 92 | +Ask the maintainer which project IDs to use if you do not already know them; |
| 93 | +they are deliberately not hardcoded here. |
| 94 | + |
| 95 | +## Running a pass |
| 96 | + |
| 97 | +Two independent surfaces. Run them as separate agents when doing a full sweep — |
| 98 | +they do not share state beyond the projects, and one must not wait on the other. |
| 99 | + |
| 100 | +- **CLI surface** — [references/cli-checklist.md](references/cli-checklist.md) |
| 101 | +- **MCP surface** — [references/mcp-checklist.md](references/mcp-checklist.md) |
| 102 | + |
| 103 | +For MCP, drive the server with [scripts/mcp-call.py](scripts/mcp-call.py) rather |
| 104 | +than wiring up an editor: |
| 105 | + |
| 106 | +```bash |
| 107 | +python3 .agents/skills/hookdeck-cli-manual-qa/scripts/mcp-call.py \ |
| 108 | + --config "$HD_CONFIG" --server gateway --list |
| 109 | +``` |
| 110 | + |
| 111 | +## What to look for, and how to choose it |
| 112 | + |
| 113 | +Ordinary "does it work" testing finds little at this point, and a pass steered |
| 114 | +by a list of recently-changed areas only looks where somebody already suspected |
| 115 | +a problem. [references/choosing-what-to-test.md](references/choosing-what-to-test.md) |
| 116 | +is how to pick targets from the product itself. It carries, with the evidence |
| 117 | +from every pass so far: |
| 118 | + |
| 119 | +- the eight defect **shapes** that have actually yielded findings — a capability |
| 120 | + diverging across the CLI and MCP; prose promising what the code does not do; |
| 121 | + the failure and empty paths; the argument you did not pass; an input accepted |
| 122 | + and then dropped; machine-readable output polluted by human text; an exit code |
| 123 | + that does not match what happened; state verbs with unrecorded side effects — |
| 124 | + and how to go looking for each; |
| 125 | +- a **routine** that produces targets with nobody handing you a list; |
| 126 | +- **what manual QA catches that the acceptance suite structurally cannot**, and |
| 127 | + where that boundary honestly sits; |
| 128 | +- **what is now guarded by a test**, so a pass does not spend itself |
| 129 | + rediscovering findings that have already become CI failures. |
| 130 | + |
| 131 | +Read it before planning a pass, not after. |
| 132 | + |
| 133 | +## Smoke-testing a published release |
| 134 | + |
| 135 | +Checking a published artifact is a different job from the QA above — it tests |
| 136 | +packaging, not behaviour — and it needs its own guard, because the documented |
| 137 | +install command is destructive to the operator's machine: |
| 138 | + |
| 139 | +```bash |
| 140 | +npm install -g hookdeck-cli@beta # DO NOT run this to test a release |
| 141 | +``` |
| 142 | + |
| 143 | +`-g` overwrites whatever `hookdeck` the operator already has. On a machine where |
| 144 | +the CLI came from Homebrew it writes into the same `bin` directory and shadows or |
| 145 | +replaces the working install, and the operator is left on a pre-release build |
| 146 | +without having asked to be. |
| 147 | + |
| 148 | +Install into an isolated prefix instead, and run the binary by path: |
| 149 | + |
| 150 | +```bash |
| 151 | +SMOKE="${TMPDIR:-/tmp}/hookdeck-smoke" |
| 152 | +rm -rf "$SMOKE" && mkdir -p "$SMOKE" |
| 153 | +npm install --prefix "$SMOKE" hookdeck-cli@beta |
| 154 | +HD="$SMOKE/node_modules/.bin/hookdeck" |
| 155 | +``` |
| 156 | + |
| 157 | +Then use `$HD` for every command, still with `--hookdeck-config "$HD_CONFIG"` |
| 158 | +from the guard above, so neither the operator's binary nor their login is touched. |
| 159 | + |
| 160 | +**Record the before state and check it afterwards.** Claiming "nothing was |
| 161 | +changed" is worth nothing without evidence: |
| 162 | + |
| 163 | +```bash |
| 164 | +which hookdeck; hookdeck version | head -1 # before and after |
| 165 | +npm ls -g hookdeck-cli --depth=0 # must stay unchanged |
| 166 | +md5 -q ~/.config/hookdeck/config.toml # must be identical after |
| 167 | +``` |
| 168 | + |
| 169 | +What is worth checking on a published artifact, none of which a local build can tell you: |
| 170 | + |
| 171 | +- the version reported matches the tag |
| 172 | +- a native binary is shipped for the host architecture, and is the one selected |
| 173 | + (`file` the binary; an `arm64` host silently running an `amd64` build is a |
| 174 | + packaging defect that still "works") |
| 175 | +- the command tree is present, and a real read reaches the API |
| 176 | +- for a release that renames or adds MCP tools, that `tools/list` shows the new |
| 177 | + names from the *installed* package rather than from your working tree |
| 178 | + |
| 179 | +Remove the prefix when finished — it is around 90 MB, because the package ships |
| 180 | +binaries for every platform. |
| 181 | + |
| 182 | +## Clean up |
| 183 | + |
| 184 | +Acceptance-test projects accumulate resources fast, and a QA pass adds to it. |
| 185 | + |
| 186 | +- Name everything you create with a run-specific prefix (`qa-<date>-<n>`) so it |
| 187 | + can be found and removed later. |
| 188 | +- Delete what you created before finishing, in reverse order of creation. |
| 189 | + Deleting a tenant cascades to its destinations. |
| 190 | +- If you leave anything behind, say so explicitly in the report with the names |
| 191 | + and the reason — silent residue is indistinguishable from a leak. |
| 192 | + |
| 193 | +## Reporting |
| 194 | + |
| 195 | +Return findings ranked by severity. Separate **confirmed defects** from **things |
| 196 | +worth a look** — mixing them makes the confirmed ones cheaper to ignore. |
| 197 | + |
| 198 | +Every finding carries five things, and is not worth filing without them: |
| 199 | + |
| 200 | +1. **The exact command or tool call**, arguments included, as run — not a |
| 201 | + paraphrase of it. |
| 202 | +2. **The exact output**, including the **exit code**. `echo $?`. |
| 203 | +3. **What you expected instead, and why**: the help text, the schema, the |
| 204 | + sibling surface, the documented default. A finding with no stated expectation |
| 205 | + is an opinion. |
| 206 | +4. **The minimal reproduction** — the shortest call that still shows it, with |
| 207 | + the incidental arguments removed one at a time. |
| 208 | +5. **Whether you ruled out the environment**, and how. Say which explanation you |
| 209 | + eliminated, not just that you are confident. |
| 210 | + |
| 211 | +### Rule out the environment before you call it a defect |
| 212 | + |
| 213 | +Two things were nearly reported as product defects in one day: a CI slice |
| 214 | +failing with 429s, where the rate limiting was noise and the real cause was |
| 215 | +eleven acceptance tests still calling pre-split tool names (an earlier pass read |
| 216 | +the 429s as the cause and moved on — `plans/mcp_read_write_tool_split.md` §7); |
| 217 | +and a project-list test that fails intermittently on list ordering (#409), which |
| 218 | +is a test defect, not a product one. What separates the two, cheapest first: |
| 219 | + |
| 220 | +- **Rebuild.** A stale binary reports the old behaviour convincingly. |
| 221 | +- **Run it again, and run it alone.** Anything that passes on a second run, or |
| 222 | + passes outside its suite, is a flake or an ordering dependency until proven |
| 223 | + otherwise. |
| 224 | +- **Read the status code.** A 429 or a 502 is the environment unless a single |
| 225 | + call reproduces it deterministically. |
| 226 | +- **Find where the behaviour originates.** #431 was confirmed as server-side by |
| 227 | + noticing the CLI calls the dedicated pause endpoint rather than sending the |
| 228 | + field — which changed the report from "our bug" into "confirm the intent with |
| 229 | + the API team", a different and more useful ask. |
| 230 | +- **Construct the minimal call that distinguishes the two explanations.** A |
| 231 | + report that says "verified: `log_level` is severity, not a completion flag, |
| 232 | + here are the five cases" is actionable; "this looks suspicious" costs someone |
| 233 | + else the same investigation. |
| 234 | + |
| 235 | +If you cannot decide, file it as **worth a look**, and say in one line exactly |
| 236 | +what you could not rule out. |
| 237 | + |
| 238 | +State plainly what you did **not** cover. A pass that reports only successes is |
| 239 | +usually a pass that did not go looking. |
0 commit comments