|
| 1 | +# QA lessons: the traps, and how to not re-learn them |
| 2 | + |
| 3 | +Written during the 2026-07-14 pre-release QA. Every item here cost real time to discover. Read |
| 4 | +this BEFORE writing another agent QA test — most of these produce a **green test that proves |
| 5 | +nothing**, which is worse than a red one. |
| 6 | + |
| 7 | +The one-line summary: **assert on the wire and on side effects, never on model prose — and make |
| 8 | +your test client behave EXACTLY like the real frontend, or you are testing your own bug.** |
| 9 | + |
| 10 | +--- |
| 11 | + |
| 12 | +## 1. The test client must replay history byte-faithfully, or every turn silently goes COLD |
| 13 | + |
| 14 | +**The trap.** Our driver replayed each assistant turn as a text-only message |
| 15 | +(`{role:"assistant", parts:[{type:"text",...}]}`), dropping the assistant's **tool parts**. |
| 16 | + |
| 17 | +The runner fingerprints the conversation over **(ordered user texts, ordered deduped tool-call |
| 18 | +ids, user-turn count)** — `session-pool.ts:226` `historyFingerprint`, and `:252` |
| 19 | +`expectedNextHistoryFingerprint`, which folds in the tool-call ids the runner emitted last turn. |
| 20 | +A replay with no tool-call ids therefore **cannot match** after any tool-using turn: |
| 21 | + |
| 22 | +``` |
| 23 | +[keepalive] mismatch (history) key=…; evict + cold |
| 24 | +``` |
| 25 | + |
| 26 | +**Why it poisons everything.** Every turn goes cold → a fresh harness process → the runner replays |
| 27 | +a hand-rendered transcript instead of the harness's real context. So: |
| 28 | +- warm/cold numbers are meaningless (nothing was ever warm), |
| 29 | +- **compaction never triggers** (the harness context never accumulates), so a long-context / |
| 30 | + "loses information" test can pass while testing nothing at all. |
| 31 | + |
| 32 | +**The rule.** Echo back the **full** assistant `UIMessage.parts` — text parts *and* `tool-<name>` |
| 33 | +parts with `toolCallId`, `input`, `state`, `output` — exactly as the AI SDK does |
| 34 | +(`web/packages/agenta-playground/src/state/execution/agentRequest.ts:401`). If your driver |
| 35 | +synthesizes assistant turns, it is not testing the product. |
| 36 | + |
| 37 | +**The tell.** `grep 'mismatch (history)'` in the runner log. If it fires on turns your client |
| 38 | +believes are warm, your client is the bug. |
| 39 | + |
| 40 | +## 2. Never assert on the model's prose. It will lie to you. |
| 41 | + |
| 42 | +First version of the tool test asked the agent to run `echo "QA-BASH-$((6*7+1))"` and asserted the |
| 43 | +reply contained `QA-BASH-43`. The model **computed 43 itself and reported it without running |
| 44 | +bash** — so a *denied* tool call still produced a "passing" reply. The wire said denied; the text |
| 45 | +said success. |
| 46 | + |
| 47 | +**The rule.** Ground truth is the frame stream: |
| 48 | +- tool executed → `tool-output-available` |
| 49 | +- tool refused → `tool-output-error` / `tool-output-denied` |
| 50 | +- approval raised → `tool-approval-request` |
| 51 | +A token in the text is only ever *corroborating* evidence, and only if the model **cannot compute |
| 52 | +it** (use a container hostname, a random file's contents — never arithmetic stated in the prompt). |
| 53 | + |
| 54 | +## 3. Scope tool assertions to ONE tool call, keyed by its INPUT |
| 55 | + |
| 56 | +A turn routinely contains several tool calls — an auto-approved read-only one alongside the gated |
| 57 | +one. A turn-wide "did any tool run?" gives false failures. |
| 58 | + |
| 59 | +And you cannot key on `toolCallId`: **on approval-resume the harness RE-ISSUES the gated call under |
| 60 | +a brand-new `toolCallId`**. You cannot key on the tool name either: Claude calls the shell |
| 61 | +`Terminal`, Pi calls it `Bash`. **Key on the tool's `input`** (the command itself). |
| 62 | + |
| 63 | +## 4. `tool-input-available` carries INCOMPLETE input, and the tool name changes case mid-stream |
| 64 | + |
| 65 | +The frame fires repeatedly for one call, streaming a progressively-built partial input: |
| 66 | + |
| 67 | +``` |
| 68 | +toolName "bash" input {"command":"echo \"QA-BASH-"} <- partial! |
| 69 | +toolName "bash" input {"command":"echo \"QA-BASH-$(hostname"} |
| 70 | +toolName "Bash" input {"command":"echo \"QA-BASH-$(hostname)\""} <- complete; name case flips |
| 71 | +``` |
| 72 | + |
| 73 | +Take the **last** frame per `toolCallId`. Taking the first — a reasonable reading of "available" — |
| 74 | +approves a **truncated command under the wrong name**; the runner keys approval decisions by |
| 75 | +name+args, so the decision misses the parked gate and **the approval re-parks forever**. |
| 76 | +(Reported as F-5: the frame name is a genuine wire-hygiene bug.) |
| 77 | + |
| 78 | +## 5. Approvals are IN-BAND. The REST route is a different product. |
| 79 | + |
| 80 | +The browser approves by re-POSTing the whole message history to `/invoke` with the tool part set to |
| 81 | +`state:"approval-responded"`, `approval:{id, approved}`. There IS a REST endpoint |
| 82 | +(`/api/sessions/interactions/{id}/respond`) but it is the **out-of-band Slack/trigger** path. |
| 83 | +Testing it tests code the UI never runs. |
| 84 | + |
| 85 | +## 6. A paused turn "finishes" |
| 86 | + |
| 87 | +An approval-paused turn ends with `finish.finishReason: "other"`, not a distinct status. "The turn |
| 88 | +ended" does NOT mean "the turn completed". Assert the reason. |
| 89 | + |
| 90 | +## 7. `code` tools do not exist on the product path |
| 91 | + |
| 92 | +The sidecar rejects them: *"Code tools are not supported by the sidecar."* They only work against |
| 93 | +the in-process service — which is what the OLD driver (`run_matrix.py`) targets, and why copying |
| 94 | +its scenarios into a product-path test fails instantly. The product's real tool surface is |
| 95 | +`builtin` / `gateway` / `mcp`. |
| 96 | + |
| 97 | +## 8. Gateway tools: discovery output is NOT config input, and the action has no prefix |
| 98 | + |
| 99 | +- The action is **`FETCH_EMAILS`**, not `GMAIL_FETCH_EMAILS`. The prefixed name appears inside the |
| 100 | + tool's own description text, which is how you get seduced into using it. Wrong name → run fails |
| 101 | + with `Action not found: composio/gmail/GMAIL_FETCH_EMAILS (HTTP 404)`. |
| 102 | +- `/api/tools/discover` returns the tool WITH `input_schema` + `description`; `GatewayToolConfig` |
| 103 | + **forbids** those keys. Feeding discovery's own output back into the agent config 500s with |
| 104 | + `extra_forbidden`. Strip to `{type, provider, integration, action, connection, name, permission}`. |
| 105 | + (Reported as F-8 — the round trip should just work.) |
| 106 | + |
| 107 | +## 9. NEVER diagnose from a run that overlapped a container restart |
| 108 | + |
| 109 | +A full matrix run showed every cell failing with 500s (`Could not verify credentials … 404`), plus |
| 110 | +a UI-visible `404 on /api/workflows/revisions/resolve`, plus `[sessions/persist] DROPPED … fetch |
| 111 | +failed` and `getaddrinfo ENOTFOUND api`. **All phantoms** — another agent was recreating the |
| 112 | +api/worker containers at that moment. Everything went green on re-run. |
| 113 | + |
| 114 | +Check `docker ps` uptimes before believing a failure. Re-run before reporting. |
| 115 | + |
| 116 | +## 10. Read-only by construction when real accounts are connected |
| 117 | + |
| 118 | +The project has live Gmail and GitHub connections. QA must never send mail, reply to a thread, or |
| 119 | +write to GitHub as a side effect. Derive tools from read-only use-cases, then **allowlist** the |
| 120 | +result: keep only actions whose name begins with a known read verb (FETCH/LIST/GET/READ/SEARCH/ |
| 121 | +VIEW/DESCRIBE/COUNT/FIND) and drop everything else. A write-verb *denylist* fails open — an action |
| 122 | +whose verb you never anticipated slips through and could mutate the real account. The allowlist |
| 123 | +fails closed: an unfamiliar action is dropped rather than run against the connected account. |
| 124 | + |
| 125 | +## 11. The product fails OPEN, so absence of an error means nothing |
| 126 | + |
| 127 | +The recurring shape of every serious bug this pass: **a component fails, the runner logs it and |
| 128 | +carries on.** The turn succeeds. The UI looks normal. |
| 129 | + |
| 130 | +- mounts 503 → run in a throwaway `/tmp` cwd, every file lost (F-1) |
| 131 | +- Pi permission extension can't install → **run with no enforcement**; `ask` never asks, `deny` |
| 132 | + never denies (F-3) |
| 133 | +- Daytona's tunnel to the store fails → skip the mount, "not fatal"; files never persist (F-7) |
| 134 | +- session records fail to POST → dropped after 3 retries, turn proceeds |
| 135 | + |
| 136 | +**Therefore: a passing turn is not evidence.** For every capability, verify the side effect |
| 137 | +(the file is really there next turn; the commit really exists) and grep the runner log for |
| 138 | +`degraded|skipped|without this mount|tunnel discovery failed|DROPPED|cold`. |
| 139 | + |
| 140 | +## 12. Environment-shaped bugs hide behind image differences |
| 141 | + |
| 142 | +Pi's permission enforcement failed only on the deployment's image, because |
| 143 | +`PI_CODING_AGENT_DIR=/pi-agent` doesn't exist there and the runner runs as uid 1000; our EE dev |
| 144 | +image runs as root and ships the dir. Same code, opposite behavior. **Always check the container's |
| 145 | +user and the actual paths** (`docker exec … id; ls -ld <dir>`) before concluding the code is fine. |
| 146 | +And note a workaround applied with `docker exec` is **lost on container recreate** — re-verify it |
| 147 | +before every batch. |
| 148 | + |
| 149 | +--- |
| 150 | + |
| 151 | +## 13. Findings expire on redeploy — re-run blocker-level findings after the stack is rebuilt |
| 152 | + |
| 153 | +F-9 ("Claude harness never resumes its native session") was CONFIRMED across 72h of real traffic |
| 154 | +and triaged as a release blocker. A deployment repair landed later the same day, pulling in recent |
| 155 | +upstream fixes. Nobody re-ran F-9 against the rebuilt stack before trusting it — until a decisive |
| 156 | +cold-context experiment on 2026-07-14 showed native session resume now working 4/4 runs, downgrading |
| 157 | +F-9 to a residual resilience concern (see STATUS.md). |
| 158 | + |
| 159 | +**The trap.** A deployment under active repair invalidates earlier observations made against it. |
| 160 | +Once the repair lands, the finding is stale, not necessarily wrong — but you don't know which |
| 161 | +until you re-check. Treating "CONFIRMED" as permanent past a redeploy is how a fixed bug survives |
| 162 | +in a triage doc as a blocker. |
| 163 | + |
| 164 | +**The rule.** For any blocker-level finding, record WHICH build/commit/deploy window it was |
| 165 | +observed on. After any redeploy that touches the relevant code path, re-run the decisive experiment |
| 166 | +before shipping a release decision on that finding — do not just re-read the old evidence. |
| 167 | + |
| 168 | +## 14. The v0 revision is a SEED — a committed config only persists on the SECOND commit |
| 169 | + |
| 170 | +Committing an agent config as a workflow revision (`POST /api/workflows/revisions/commit`) looks |
| 171 | +like it stores your `data.parameters` immediately. It does not on the first commit. The DAO |
| 172 | +force-nulls `data`/`flags`/`meta` for **version 0** (`api/oss/src/dbs/postgres/git/dao.py` |
| 173 | +`_null_revision_fields`, `if revision.version == "0"`). So a fresh variant's first commit is an |
| 174 | +empty seed; your config lands on the **second** commit (v1). A test that commits once and asserts |
| 175 | +`data.parameters == X` fails with `KeyError: 'data'` and looks like a broken endpoint — it is not. |
| 176 | +Commit twice (seed, then the real change) and assert v0→v1 plus the changed field surviving a |
| 177 | +`GET /api/workflows/revisions/{id}`. Also: `data` is `extra="forbid"` — only |
| 178 | +`{uri,url,headers,runtime,script,schemas,parameters}` are accepted. |
| 179 | + |
| 180 | +Second trap in the same area: this is a WORKFLOW-revision commit, NOT the in-stream |
| 181 | +`data-committed-revision` SSE frame (which is a different mechanism — the agent committing during a |
| 182 | +turn) and NOT a git commit. The playground's Save/Commit button hits the REST route above. |
| 183 | + |
| 184 | +## 15. User MCP servers are Claude-only, public-HTTPS-only, and the harness dials them |
| 185 | + |
| 186 | +Three things will each silently break an MCP smoke test: |
| 187 | + |
| 188 | +- **Pi rejects any run that declares `mcps`** (`run-plan.ts` `PI_USER_MCP_UNSUPPORTED_MESSAGE`). |
| 189 | + User MCP needs a harness with `capabilities.mcpTools` — i.e. **Claude**. Do not smoke-test MCP on |
| 190 | + a Pi cell; SKIP it there. |
| 191 | +- **A local MCP server is unreachable.** The SDK resolver AND the runner both run an SSRF guard |
| 192 | + (`assert_endpoint_url_allowed` / `validateUserMcpUrl`) that rejects `http://` and |
| 193 | + private/loopback/metadata hosts unless `AGENTA_INSECURE_EGRESS_ALLOWED` / |
| 194 | + `AGENTA_AGENT_MCPS_HOST_ALLOWLIST` is set (neither is, on bighetzner). Use a **public HTTPS** |
| 195 | + server. DeepWiki (`https://mcp.deepwiki.com/mcp`, no auth) works. |
| 196 | +- **The harness — not the runner process — opens the connection**, from the runner host on `local` |
| 197 | + (from the sandbox on Daytona). The endpoint must be reachable from wherever the harness runs. |
| 198 | + |
| 199 | +The config entry is a full object, not a URL string: |
| 200 | +`{"name","connection":{"type":"http","url":...},"policy":{"tools":{"mode":"all"}}}`. Assert on the |
| 201 | +wire: a `tool-output-available` frame for a tool named `mcp__<server>__<tool>`. |
| 202 | + |
| 203 | +## The checklist for the next QA run |
| 204 | + |
| 205 | +1. `docker ps` — is anything restarting? If yes, wait. |
| 206 | +2. Does the runner have its harness dirs (`/pi-agent`)? Is it root or not? |
| 207 | +3. Drive the **product path** (`/services/agent/v0/invoke`), not the service `/invoke`. |
| 208 | +4. Echo history **faithfully** (tool parts included), then confirm `hit-continue` in the log. |
| 209 | +5. Assert on frames + side effects. Never on prose. |
| 210 | +6. After every capability passes, grep the log for silent degradation. |
| 211 | +7. Re-run anything that failed once before reporting it. |
| 212 | +8. Before trusting an existing blocker-level finding, check whether the stack has been redeployed |
| 213 | + since it was observed — if so, re-run the decisive experiment. |
0 commit comments