diff --git a/.agents/skills/agent-release-gate/SKILL.md b/.agents/skills/agent-release-gate/SKILL.md index 7e2db7db34..e1132f44b8 100644 --- a/.agents/skills/agent-release-gate/SKILL.md +++ b/.agents/skills/agent-release-gate/SKILL.md @@ -34,6 +34,8 @@ export AGENTA_API_KEY=... # project API key uv run resources/qa_product.py --all --custom-slug # every cell, every journey uv run resources/qa_product.py --cell P1 # one cell uv run resources/qa_product.py --cell C1 --only chat # one journey +uv run resources/qa_product.py --cell C3 --only records --only sessions # the sessions-storage journeys +uv run resources/qa_product.py --cell C3 --last-message-only # minimal-history differential run ``` Paths are relative to this skill's directory. The deployment's vault must hold the provider keys @@ -55,13 +57,14 @@ cell — user MCP is Claude-only). Any `FAIL` blocks the release until triaged. The runtime **fails open**: a component can break, get logged, and the turn still succeeds with a normal-looking answer. A green turn is therefore not proof on its own. Before trusting a pass, read `resources/LESSONS.md` — every trap there produced a green test that proved nothing. The two -that bite hardest: replay conversation history byte-faithfully (tool parts included) or every turn -silently goes cold, and re-run any prior blocker-level finding after a redeploy before believing it. +that bite hardest: a full-history client must replay conversation history byte-faithfully (tool +parts included) or every turn silently goes cold — a last-message-only client is exempt by design, +see LESSONS #1 — and re-run any prior blocker-level finding after a redeploy before believing it. ## Resources (read on demand) - `resources/coverage.md` — the cells (harness × sandbox × auth) and journeys (chat, mount, tool, - approve, deny, commit, warm, mcp) with a one-line meaning for each. + approve, deny, commit, warm, mcp, records, sessions, followup) with a one-line meaning for each. - `resources/LESSONS.md` — the traps. Read before writing or trusting any agent QA test. - `resources/qa_product.py` — the gate driver (cells × journeys). - `resources/qa_probe.py` — a one-turn wire probe: `uv run resources/qa_probe.py` confirms the @@ -71,4 +74,42 @@ silently goes cold, and re-run any prior blocker-level finding after a redeploy - `resources/seeds/` — representative green `results.json` files kept as regression-seed references. Release-night findings and the full evidence history are archived in -`docs/design/agent-workflows/projects/qa/` (STATUS.md, findings.md, matrix.md). +`docs/design/agent-workflows/projects/qa/` (findings.md, matrix.md, README.md). + +## Sessions rework (v0.106) addendum + +The gate above predates the sessions-storage rework (`feat/sessions-storage-rework`, v0.106.x) and +does not yet exercise its flag-gated paths — see `resources/coverage.md` for the exact list of +what is not covered. + +**The four flags.** Parsing is NOT uniform across them — check the literal value, not just +whether the variable is set: + +- `AGENTA_SESSIONS_RECONSTRUCT` (runner) — rebuilds prior turns from the durable record log for a + minimal-history request. Accepts ONLY the literal string `"true"` (case-insensitive); `1`, + `yes`, `on` silently do nothing (`reconstruct-history.ts` `reconstructEnabled`). +- `AGENTA_RECORDS_DURABLE` (runner) — stronger retry + drop-counting on record persistence. Same + literal-`"true"`-only parsing (`sessions/persist.ts` `durableRecordsEnabled`). +- `NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY` (web) — sends only the trailing user message on a fresh + turn instead of full history; a HITL resume still sends full history. Broader truthy parsing + (`true`/`1`/`t`/`y`/`yes`/`on`/`enable`/`enabled`), unlike its runner counterpart. **Must be + flipped together with `AGENTA_SESSIONS_RECONSTRUCT`** — nothing enforces the pairing at runtime, + so web-on/runner-off silently loses all context on every cold turn (the client sends one + message; the runner has no reconstruction to fall back on). +- `AGENTA_RECORDS_SMART_TRUNCATION` (API) — preserves record structure instead of dropping an + oversized record body wholesale. Same broader truthy parsing as the web flag + (`api/oss/src/utils/env.py` `SessionsRecordsConfig`). + +**Differential QA, not verdict QA.** A flags-on run and a flags-off run against the same stack each +report their own PASS/FAIL and tell you nothing about each other. The method that actually finds +defects here: run the gate **twice against ONE flags-on stack**, changing only the client's history +mode (full history vs. last-message-only), and diff the message arrays the runner hands the model, +turn by turn. The defects live in the diff — a dropped tool part, a duplicated turn, turns +reconstructed in the wrong order — not in either run's verdict. + +**Release QA plan.** The concrete plan for this release (target stack, flag matrix, division of +labor) is `docs/design/agent-workflows/projects/qa/release-2026-07-sessions-storage-rework.md`. + +**Caution.** A green flags-off run says nothing about the flags-on path, and vice versa — they +exercise different code (server-side history reconstruction, client-side history truncation). Run +both before shipping a release decision; never extrapolate one to the other. diff --git a/.agents/skills/agent-release-gate/resources/LESSONS.md b/.agents/skills/agent-release-gate/resources/LESSONS.md index e04616b140..37d3dec755 100644 --- a/.agents/skills/agent-release-gate/resources/LESSONS.md +++ b/.agents/skills/agent-release-gate/resources/LESSONS.md @@ -9,33 +9,49 @@ your test client behave EXACTLY like the real frontend, or you are testing your --- -## 1. The test client must replay history byte-faithfully, or every turn silently goes COLD +## 1. The test client must replay history byte-faithfully — unless it deliberately sends minimal history instead **The trap.** Our driver replayed each assistant turn as a text-only message (`{role:"assistant", parts:[{type:"text",...}]}`), dropping the assistant's **tool parts**. The runner fingerprints the conversation over **(ordered user texts, ordered deduped tool-call -ids, user-turn count)** — `session-pool.ts:226` `historyFingerprint`, and `:252` -`expectedNextHistoryFingerprint`, which folds in the tool-call ids the runner emitted last turn. -A replay with no tool-call ids therefore **cannot match** after any tool-using turn: +ids, user-turn count)** — `historyFingerprint` and `expectedNextHistoryFingerprint` in +`services/runner/src/engines/sandbox_agent/session-identity.ts:212` and `:238`, the second folding +in the tool-call ids the runner emitted last turn. A replay with no tool-call ids therefore +**cannot match** after any tool-using turn: ``` [keepalive] mismatch (history) key=…; evict + cold ``` -**Why it poisons everything.** Every turn goes cold → a fresh harness process → the runner replays -a hand-rendered transcript instead of the harness's real context. So: +**Since the sessions rework, this only binds a full-history client.** When the request carries +minimal history — exactly one message, a fresh user turn, no approval envelope +(`carriesMinimalHistory`, same file, `:302`) — the keepalive check **skips the history-fingerprint +comparison entirely** (`server.ts:603` `clientAssertsHistory`) and, with +`AGENTA_SESSIONS_RECONSTRUCT=true`, the runner rebuilds prior turns from the durable record log +instead (`reconstruct-history.ts`). The client is no longer asserting the conversation at all, so +there is nothing to fingerprint-match against. + +**Why a full-history driver still poisons everything if it gets this wrong.** Every turn goes cold +→ a fresh harness process → the runner replays a hand-rendered transcript instead of the harness's +real context. So: - warm/cold numbers are meaningless (nothing was ever warm), - **compaction never triggers** (the harness context never accumulates), so a long-context / "loses information" test can pass while testing nothing at all. -**The rule.** Echo back the **full** assistant `UIMessage.parts` — text parts *and* `tool-` -parts with `toolCallId`, `input`, `state`, `output` — exactly as the AI SDK does -(`web/packages/agenta-playground/src/state/execution/agentRequest.ts:401`). If your driver -synthesizes assistant turns, it is not testing the product. +**The rule.** In full-history mode (the default, and the only mode until +`NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY` is on): echo back the **full** assistant `UIMessage.parts` +— text parts *and* `tool-` parts with `toolCallId`, `input`, `state`, `output` — exactly as +the AI SDK does (`web/packages/agenta-playground/src/state/execution/agentRequest.ts:402-413`). In +last-message-only mode, send exactly the trailing user message and nothing else — a driver that +sends "most of" the history (say, the last two turns) satisfies neither mode and mismatches either +way. If your driver synthesizes assistant turns without knowing which mode it is testing, it is not +testing the product. **The tell.** `grep 'mismatch (history)'` in the runner log. If it fires on turns your client -believes are warm, your client is the bug. +believes are warm, your client is the bug — unless the client is deliberately minimal-history, in +which case the fingerprint check never runs and this grep is the wrong tell; grep `[reconstruct]` +instead. ## 2. Never assert on the model's prose. It will lie to you. @@ -154,7 +170,9 @@ F-9 ("Claude harness never resumes its native session") was CONFIRMED across 72h and triaged as a release blocker. A deployment repair landed later the same day, pulling in recent upstream fixes. Nobody re-ran F-9 against the rebuilt stack before trusting it — until a decisive cold-context experiment on 2026-07-14 showed native session resume now working 4/4 runs, downgrading -F-9 to a residual resilience concern (see STATUS.md). +F-9 to a residual resilience concern. (The original STATUS.md write-up of this downgrade did not +survive the later docs consolidation into `findings.md`/`matrix.md`; this paragraph is the +surviving record.) **The trap.** A deployment under active repair invalidates earlier observations made against it. Once the repair lands, the finding is stale, not necessarily wrong — but you don't know which @@ -205,7 +223,9 @@ wire: a `tool-output-available` frame for a tool named `mcp____`. 1. `docker ps` — is anything restarting? If yes, wait. 2. Does the runner have its harness dirs (`/pi-agent`)? Is it root or not? 3. Drive the **product path** (`/services/agent/v0/invoke`), not the service `/invoke`. -4. Echo history **faithfully** (tool parts included), then confirm `hit-continue` in the log. +4. In full-history mode, echo history **faithfully** (tool parts included), then confirm + `hit-continue` in the log; in last-message-only mode, send just the trailing user message and + confirm `[reconstruct]` instead. 5. Assert on frames + side effects. Never on prose. 6. After every capability passes, grep the log for silent degradation. 7. Re-run anything that failed once before reporting it. diff --git a/.agents/skills/agent-release-gate/resources/coverage.md b/.agents/skills/agent-release-gate/resources/coverage.md index dc20b798d6..603b241eb8 100644 --- a/.agents/skills/agent-release-gate/resources/coverage.md +++ b/.agents/skills/agent-release-gate/resources/coverage.md @@ -36,9 +36,37 @@ cell — keep them in sync if a cell changes. | `commit` | Save an agent config as a new workflow revision, then fetch it back. | The changed parameter survives the round trip and the version bumps (v0 seed → v1; see LESSONS #14). Harness-agnostic — it drives the config REST API, not a turn. | | `warm` | Run three turns, watch latency and the runner log. | Turns 2-3 are faster and the log confirms the session was genuinely **loaded**, not silently cold. | | `mcp` | Deliver an MCP server in the agent config and call one of its tools. | A `tool-output-available` frame fires for an `mcp__*` tool. **Claude only** — Pi rejects user MCP, so this `SKIP`s on every Pi cell. Uses the public DeepWiki server by default; override with `--mcp-url`. | +| `records` | Force a tool call, then poll `POST /sessions/records/query` (ingestion is async, worker-drained off Redis). | Record types cover a user message, an assistant message, a `tool_call`, and a `tool_result`; `timestamp` is non-decreasing in returned order; the unguessable bash token appears inside a `tool_result` body; no record is the bare `{"_truncated": true}` legacy drop-in. Harness-agnostic, like `commit`. | +| `sessions` | REST lifecycle over `/api/sessions/*`: create (one cheap turn), list, archive, unarchive, rename (`PUT /sessions/streams/header`), delete. | Each step's effect on `POST /sessions/query` is exactly right: archived hides by default and shows with `include_archived`; unarchive restores it; rename shows in the next query; delete is a real hard delete — gone even with every include flag on. Harness-agnostic, like `commit`; cleans up on every path. | +| `followup` | After an approved resume settles, send ONE more normal user turn on the same session forcing a second tool call. | The followup gets a fresh wire `toolCallId` (never the gated call's) and its own durable `tool_call` record — no `record_id` is shared between the two calls. Probes an open defect prediction (2026-07-24 review: a fresh post-approval turn could silently collide/overwrite the approved call's record) that was never exercised before; live-verified clean in both full-history and `--last-message-only` modes on 2026-07-28. | + +**`--last-message-only`** (a global flag, not a journey): mirrors the frontend's minimal-send switch +(`NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY` / `agentRequest.ts:401-415`) — a fresh user turn sends +only its trailing message instead of full history, while an approval resume still sends full +history, exactly like the browser. Every turn's exact `sent_messages` lands in `results.json` so a +full-history run and a `--last-message-only` run against the same stack can be diffed offline. Triggers are deliberately **out of scope** for this gate. +## Not covered (sessions rework, as of 2026-07-28) + +These feature areas shipped in `feat/sessions-storage-rework` and have no journey in +`qa_product.py` yet. Listed here so the gap is explicit rather than assumed away — flip a row to +covered once a journey lands, do not delete it silently. + +| Feature | Status | Note | +|---|---|---| +| Durable-records readback | covered | `records` journey (J8): polls `POST /sessions/records/query`, asserts type coverage, timestamp order, real tool-result content, and no bare-truncated bodies. | +| Last-message-only client mode | covered | `--last-message-only` global flag mirrors `NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY`'s minimal-send condition and dumps each turn's exact `sent_messages` for an offline diff against a full-history run. | +| Sessions REST surface (query/archive/rename/delete/revive) | covered | `sessions` journey (J9): create → query → archive → unarchive → rename → delete, asserting each state transition's effect on `POST /sessions/query`. | +| Cold-replay approval resume | not covered | The paused-turn + resume transcript fold is UI-side; no wire-level journey exercises it. | +| Batch approvals | not covered | Approve-all/Deny-all with context peek is UI-side; the `approve`/`deny` journeys are single-gate only. | +| Warm Stop | not covered | Cooperative cancel that leaves the session resumable (sandbox destroyed) — no journey. | +| Steer | not covered | Deny + redirect, behind `NEXT_PUBLIC_AGENT_CHAT_STEER` — no journey. | + +See `docs/design/agent-workflows/projects/qa/release-2026-07-sessions-storage-rework.md` for the +full flag-gated risk list this table is a slice of. + ## Optional probes (`qa_longctx.py`) Separate from the gate, these need live **Gmail and GitHub Composio connections** in the target diff --git a/.agents/skills/agent-release-gate/resources/qa_product.py b/.agents/skills/agent-release-gate/resources/qa_product.py index f3b1eb5a69..640844b01e 100644 --- a/.agents/skills/agent-release-gate/resources/qa_product.py +++ b/.agents/skills/agent-release-gate/resources/qa_product.py @@ -12,6 +12,7 @@ uv run qa_product.py --cell C3 # one cell uv run qa_product.py --all # every cell uv run qa_product.py --cell C3 --only approve # one journey + uv run qa_product.py --cell C3 --last-message-only # minimal-history differential run Credentials come from the environment (AGENTA_BASE, AGENTA_PROJECT_ID, AGENTA_API_KEY), falling back to --env-file. Results land in ./qa-gate-runs// (override with AGENTA_QA_RUNS_DIR) @@ -27,6 +28,7 @@ import re import time import uuid +from datetime import datetime import httpx @@ -101,15 +103,39 @@ def resolve_credentials(env_file: str | pathlib.Path | None = None) -> None: DEFAULT_MCP_URL = "https://mcp.deepwiki.com/mcp" MCP_URL = DEFAULT_MCP_URL - -def api_call(method: str, path: str, timeout: float = 60.0, **kwargs) -> httpx.Response: - """One REST call to the /api surface (the routes the playground UI drives for config/commits), - NOT the SSE /services/agent/v0/invoke turn endpoint. Auth is the same ApiKey header, and - project_id rides the query string (never the body), exactly like the browser.""" +# --last-message-only: mirrors the frontend's minimal-send switch EXACTLY +# (web/packages/agenta-playground/src/state/execution/agentRequest.ts:401-415 +# `isSessionsLastMessageOnlyEnabled() && opts.sessionId && lastMessage?.role === "user"`). When on, +# a turn whose trailing message is a fresh USER message sends ONLY that message, letting the +# runner rebuild prior turns from the durable record log (requires the backend's +# AGENTA_SESSIONS_RECONSTRUCT=true to actually have context — this driver does not set backend +# flags, it only shapes the request). An approval RESUME's trailing message is the assistant's +# paused turn with the decision inlined (role "assistant", see `approval_reply`), never "user", so +# it is never shortened by this flag — full history rides every resume, exactly like the browser. +# Applied inside `invoke()` so every journey gets identical behavior with no per-journey plumbing. +LAST_MESSAGE_ONLY = False + + +def api_call( + method: str, path: str, timeout: float = 60.0, params: dict | None = None, **kwargs +) -> httpx.Response: + """One REST call to the /api surface (the routes the playground UI drives for config/commits, + and the sessions lifecycle routes), NOT the SSE /services/agent/v0/invoke turn endpoint. Auth + is the same ApiKey header, and project_id rides the query string (never the body), exactly + like the browser. + + `params` merges EXTRA query params (e.g. `session_id` for the sessions routes) alongside + `project_id` — httpx does NOT merge a `params=` kwarg with any query string already in `path`, + it silently REPLACES it, so a caller-supplied params dict is merged here rather than appended + to the path string. + """ + merged_params = {"project_id": PROJECT} + if params: + merged_params.update(params) return httpx.request( method, f"{BASE}/api{path}", - params={"project_id": PROJECT}, + params=merged_params, headers={"Authorization": f"ApiKey {KEY}", "Content-Type": "application/json"}, timeout=timeout, **kwargs, @@ -309,6 +335,10 @@ def __init__(self) -> None: self.committed_revision: dict | None = None self.http_status: int = 0 self.ms: int = 0 + # The EXACT `messages` array this turn sent on the wire (post --last-message-only + # shortening, if any) — set by invoke(). Dumped into summary() so two runs (full-history + # vs. last-message-only) can be diffed offline, turn by turn. + self.sent_messages: list = [] @property def reply(self) -> str: @@ -371,6 +401,9 @@ def summary(self) -> dict: "approval": bool(self.approval), "errors": self.errors, "reply": self.reply[:400], + # Full, untruncated: this is the byte-accurate artifact the --last-message-only + # differential is FOR (diff two runs' sent_messages, not just their verdicts). + "sent_messages": self.sent_messages, } @@ -378,9 +411,16 @@ def invoke( session_id: str, messages: list, params: dict, timeout: float = 300.0 ) -> Turn: t = Turn() + # --last-message-only: send just the trailing message when it's a fresh USER turn — the exact + # condition the frontend gates on (see LAST_MESSAGE_ONLY comment above). An approval resume's + # trailing message has role "assistant" (approval_reply), so this never fires for a resume. + outbound = messages + if LAST_MESSAGE_ONLY and messages and messages[-1].get("role") == "user": + outbound = [messages[-1]] + t.sent_messages = outbound body = { "session_id": session_id, - "data": {"inputs": {"messages": messages}, "parameters": {"agent": params}}, + "data": {"inputs": {"messages": outbound}, "parameters": {"agent": params}}, } headers = { "Authorization": f"ApiKey {KEY}", @@ -533,9 +573,50 @@ def j3_tool(cell: dict) -> dict: } -def _approval_flow(cell: dict, approved: bool) -> dict: +# Records are written by the runner to a Redis stream (`publish_record`) and drained into Postgres +# by a worker asynchronously — `POST /sessions/records/query` can lag the turn that produced them +# by a few seconds. Poll instead of asserting immediately (up to ~20s), short backoff. Shared by +# `_approval_flow`'s followup check and `j8_records`. +RECORDS_POLL_TIMEOUT_S = 20.0 +RECORDS_POLL_INTERVAL_S = 1.5 + + +def _poll_records(session_id: str, until) -> tuple[list, str | None]: + """Poll `POST /sessions/records/query` until `until(records)` is truthy or ~20s elapse. + Returns whatever the last successful query returned (possibly not satisfying `until`, e.g. on + timeout) plus the last HTTP error string, if any query failed outright.""" + records: list = [] + last_err: str | None = None + deadline = time.time() + RECORDS_POLL_TIMEOUT_S + while time.time() < deadline: + r = api_call("POST", "/sessions/records/query", json={"session_id": session_id}) + if r.status_code == 200: + records = r.json().get("records", []) + if until(records): + break + else: + last_err = f"HTTP {r.status_code}: {r.text[:200]}" + time.sleep(RECORDS_POLL_INTERVAL_S) + return records, last_err + + +def _approval_flow(cell: dict, approved: bool, check_followup: bool = False) -> dict: """J4: with permission default `ask`, a tool call must PAUSE with a tool-approval-request, - then resume on the user's decision — the same in-band protocol the browser uses.""" + then resume on the user's decision — the same in-band protocol the browser uses. + + `check_followup` (the `followup` journey): after an APPROVED resume settles, send ONE more + normal user turn on the SAME session that forces a second, independent tool call, then query + the session's durable records and assert no `tool_call` record shares its `record_id` with + another — the exact shape of an open defect prediction (2026-07-24 review, never exercised + until now): a fresh post-approval turn colliding on the prior turn's record id would silently + UPSERT over it instead of getting its own row, hiding one of the two executions. Only + meaningful when `approved` — a denied gate produced no executed call to disambiguate from. + + Works in both full-history and --last-message-only modes with no extra branching: the + followup is a PLAIN user turn, so `invoke()`'s --last-message-only shortening (see the + LAST_MESSAGE_ONLY global) applies to it exactly like any other normal turn; the approval + RESUME itself is unaffected either way, because its trailing message is the assistant's + approval-responded turn, never role "user" (see LESSONS.md #1).""" s = str(uuid.uuid4()) params = template( cell, @@ -578,12 +659,78 @@ def _approval_flow(cell: dict, approved: bool) -> dict: # resume can't be misread as a successful deny. ok = paused_ok and outcome == "denied" and not t2.errors and not t2.approval why = f"denied: the gated command never executed (outcome={outcome})" + + if not check_followup: + return { + "pass": ok, + "why": why, + "paused_finish_other": paused_ok, + "turn_paused": t1.summary(), + "turn_resumed": t2.summary(), + } + + if not (approved and ok): + return { + "pass": False, + "why": f"{why} (followup check skipped: the approval resume itself did not succeed)", + "paused_finish_other": paused_ok, + "turn_paused": t1.summary(), + "turn_resumed": t2.summary(), + } + + # Followup: ONE more normal user turn, same session, a SECOND independent tool call. Give it + # its own `allow` params — the approval gate itself is already proven above; this turn's job + # is only to probe for a duplicated/reused record id, not to re-walk the approval protocol. + followup_params = template( + cell, + tools=[BASH_TOOL], + instructions="Use the bash tool when asked to run a command. Report only its stdout.", + permission_default="allow", + ) + followup_msgs = msgs + [t2.assistant_message(), user_msg(BASH_PROMPT)] + t3 = invoke(s, followup_msgs, followup_params) + followup_ran = ( + tool_ran(t3) and bool(BASH_TOKEN_RE.search(t3.reply)) and not t3.errors + ) + + # Wire-level sanity: the followup must mint a FRESH toolCallId, never the gated call's. NOT a + # check that t1's and t2's ids differ from each other — the resume LEGITIMATELY re-reports the + # SAME toolCallId while it settles (confirmed live: Claude keeps one id across pause->resume), + # so comparing every id turn-to-turn indiscriminately would flag that normal reuse as a false + # positive. Only a genuinely NEW call (the followup) reusing an EXISTING id would be the bug. + gated_wire_id = t1.approval["toolCallId"] + followup_wire_ids = {c["toolCallId"] for c in t3.tool_calls} + followup_wire_id_fresh = gated_wire_id not in followup_wire_ids + + # The authoritative check: the DURABLE record, not the wire. Poll until at least the gated + # call's and the followup's tool_call records have both landed. + records, poll_err = _poll_records( + s, + lambda recs: sum(1 for r in recs if r.get("record_type") == "tool_call") >= 2, + ) + tool_call_record_ids = [ + rec.get("record_id") for rec in records if rec.get("record_type") == "tool_call" + ] + no_dup_record_ids = len(tool_call_record_ids) == len(set(tool_call_record_ids)) + enough_records = len(tool_call_record_ids) >= 2 + + ok2 = ( + followup_ran and followup_wire_id_fresh and no_dup_record_ids and enough_records + ) return { - "pass": ok, - "why": why, - "paused_finish_other": paused_ok, + "pass": ok2, + "why": ( + f"approval resume ok ({why}); followup tool ran cleanly={followup_ran}; " + f"followup wire toolCallId is fresh (not the gated call's)={followup_wire_id_fresh}; " + f"tool_call record ids={tool_call_record_ids} " + f"(no duplicates={no_dup_record_ids}, both landed={enough_records}" + f"{f', last poll error={poll_err}' if poll_err and not enough_records else ''})" + ), + "session_id": s, "turn_paused": t1.summary(), "turn_resumed": t2.summary(), + "turn_followup": t3.summary(), + "tool_call_record_ids": tool_call_record_ids, } @@ -595,13 +742,23 @@ def j4_deny(cell: dict) -> dict: return _approval_flow(cell, approved=False) +def j10_followup(cell: dict) -> dict: + """J10: the turn immediately after an approval resume gets its own durable record — see + `_approval_flow`'s `check_followup` docstring for the defect this proves absent.""" + return _approval_flow(cell, approved=True, check_followup=True) + + def j6_warm(cell: dict) -> dict: """J6 (latency half): three turns in one session; turns 2/3 should be faster than turn 1. - The cold/warm TRUTH lives in the runner log — this only measures. See STATUS.md F-2.""" + Latency alone is a proxy: it is NOT observable from here whether a turn was genuinely warm + (harness process reused) or a fast cold start — that truth lives only in the runner log + (grep for the session's harness-start line), never in anything the browser or this driver + can see on the wire (finding F-2).""" s = str(uuid.uuid4()) params = template(cell) msgs: list = [] times = [] + turns = [] # every turn's summary (incl. sent_messages), win or lose — the diffable artifact. for i, q in enumerate( [ "Reply with exactly: ONE", @@ -612,15 +769,22 @@ def j6_warm(cell: dict) -> dict: msgs = msgs + [user_msg(q)] t = invoke(s, msgs, params) times.append(t.ms) + turns.append(t.summary()) msgs = msgs + [t.assistant_message()] if t.errors: - return {"pass": False, "why": f"turn {i + 1} errored", "turn": t.summary()} + return { + "pass": False, + "why": f"turn {i + 1} errored", + "turn": t.summary(), + "turns": turns, + } warm_gain = times[0] - min(times[1], times[2]) return { "pass": warm_gain > 0, "why": f"turn1={times[0]}ms, turn2={times[1]}ms, turn3={times[2]}ms (warm gain {warm_gain}ms)", "session_id": s, "times_ms": times, + "turns": turns, } @@ -908,6 +1072,271 @@ def j7_mcp(cell: dict) -> dict: } +# --------------------------------------------------------------------------- +# Records / sessions REST journeys (sessions-storage rework) +# --------------------------------------------------------------------------- + +# The bare legacy drop-in: the WHOLE record body replaced by this exact dict when +# AGENTA_RECORDS_SMART_TRUNCATION is off and a record body exceeds the byte cap +# (`api/oss/src/core/sessions/records/streaming.py` `publish_record`). Smart truncation keeps the +# event shape (`{"type":..., "id":..., "_truncated": {...}}` or `{"..., "_truncated": True, ...}` +# with extra keys) so this bare two-key dict is specifically the "content fully dropped" case. +BARE_TRUNCATION_PLACEHOLDER = {"_truncated": True} + + +def _parse_ts(value) -> datetime | None: + """Parse a record's ISO-8601 `timestamp` into a comparable datetime, or None if absent/bad.""" + if not value: + return None + try: + return datetime.fromisoformat(str(value).replace("Z", "+00:00")) + except ValueError: + return None + + +def j8_records(cell: dict) -> dict: + """J8: the durable session record log has real content, in order — not just envelopes. + + Forces a tool call (the same bash-token proof as J3), then polls + `POST /sessions/records/query` (`api/oss/src/apis/fastapi/sessions/router.py` + `RecordsRouter.query_records`) until records for the session land. Asserts: + (a) record TYPES cover a user message, an assistant message, a tool_call, and a tool_result + — the coalesced shapes `services/runner/src/sessions/persist.ts` `buildPersistingEmitter` + actually writes (`record_source` "user"/"agent" x `record_type` "message"/"tool_call"/ + "tool_result"), not raw per-delta events. + (b) `timestamp` is monotonically non-decreasing in the order the endpoint returns rows — + the DAO's own sort key (`dbs/postgres/sessions/records/dao.py` `get_records`). + (c) the unguessable bash token shows up INSIDE a tool_result record's body — proof real tool + output persisted, not just a shell of an event with the payload dropped. + (d) no record body is the bare legacy truncation placeholder (an oversized body silently + replaced wholesale rather than smart-truncated). + + Harness-agnostic (drives session-scoped REST, not a turn's wire shape) — like `commit`, it + runs once per selected cell rather than varying its assertions by harness. + """ + s = str(uuid.uuid4()) + token = f"QA-RECORDS-{uuid.uuid4().hex[:10]}" + prompt = ( + f'Use the bash tool to run exactly: echo "{token}-$(hostname)" ' + "and reply with only its stdout." + ) + t = invoke( + s, + [user_msg(prompt)], + template( + cell, + tools=[BASH_TOOL], + instructions="Use the bash tool when asked to run a command. Report only its stdout.", + permission_default="allow", + ), + ) + if not tool_ran(t) or t.errors: + return { + "pass": False, + "why": "setup turn never ran the tool — nothing to verify records against", + "turn": t.summary(), + } + + def _have_all_types(records: list) -> bool: + types = {(rec.get("record_source"), rec.get("record_type")) for rec in records} + return ( + ("user", "message") in types + and ("agent", "message") in types + and any(rec.get("record_type") == "tool_call" for rec in records) + and any(rec.get("record_type") == "tool_result" for rec in records) + ) + + records, last_err = _poll_records(s, _have_all_types) + + if not records: + return { + "pass": False, + "why": f"no records landed for the session within {RECORDS_POLL_TIMEOUT_S}s (last error: {last_err})", + "session_id": s, + } + + coverage = { + "user_message": any( + rec.get("record_source") == "user" and rec.get("record_type") == "message" + for rec in records + ), + "assistant_message": any( + rec.get("record_source") == "agent" and rec.get("record_type") == "message" + for rec in records + ), + "tool_call": any(rec.get("record_type") == "tool_call" for rec in records), + "tool_result": any(rec.get("record_type") == "tool_result" for rec in records), + } + coverage_ok = all(coverage.values()) + + timestamps = [_parse_ts(rec.get("timestamp")) for rec in records] + monotonic_ok = all(ts is not None for ts in timestamps) and all( + a <= b for a, b in zip(timestamps, timestamps[1:]) + ) + + token_in_tool_result = any( + rec.get("record_type") == "tool_result" + and token in json.dumps(rec.get("attributes") or {}) + for rec in records + ) + + bare_truncated_ids = [ + str(rec.get("record_id")) + for rec in records + if rec.get("attributes") == BARE_TRUNCATION_PLACEHOLDER + ] + + ok = ( + coverage_ok and monotonic_ok and token_in_tool_result and not bare_truncated_ids + ) + return { + "pass": ok, + "why": ( + f"type coverage={coverage} (ok={coverage_ok}), timestamps monotonic={monotonic_ok}, " + f"token in a tool_result body={token_in_tool_result}, " + f"bare-truncated records={bare_truncated_ids or 'none'}" + ), + "session_id": s, + "record_count": len(records), + "turn": t.summary(), + } + + +def _sessions_query( + *, include_ended: bool = False, include_archived: bool = False, limit: int = 200 +) -> list: + r = api_call( + "POST", + "/sessions/query", + json={ + "include_ended": include_ended, + "include_archived": include_archived, + "windowing": {"limit": limit}, + }, + ) + if r.status_code != 200: + raise RuntimeError(f"sessions query HTTP {r.status_code}: {r.text[:200]}") + return r.json().get("sessions", []) + + +def _find_session(sessions: list, session_id: str) -> dict | None: + return next( + (sess for sess in sessions if sess.get("session_id") == session_id), None + ) + + +def j9_sessions(cell: dict) -> dict: + """J9: the REST lifecycle over `/api/sessions/*` + (`api/oss/src/apis/fastapi/sessions/router.py` `SessionsRootRouter` + `set_session_stream_header`) + — create, list, archive, unarchive, rename, delete. One cheap chat turn creates a live session; + everything else is pure REST against session_id. + + Harness-agnostic (drives session-level REST, not a turn's wire shape) — like `commit`, it runs + once per selected cell rather than varying its assertions by harness. Cleans up (deletes the QA + session) in a `finally`, including on a failure path, so a broken run doesn't leave a + renamed/archived session behind in the target project. + """ + s = str(uuid.uuid4()) + steps: dict = {} + try: + t = invoke(s, [user_msg("Reply with exactly: PONG")], template(cell)) + if t.errors or t.finish_reason != "stop": + return { + "pass": False, + "why": "setup turn failed to create the session", + "turn": t.summary(), + } + + found = _find_session(_sessions_query(), s) + steps["created_visible_in_query"] = found is not None + if not found: + return { + "pass": False, + "why": "session did not appear in POST /sessions/query after creation", + "steps": steps, + } + + # Archive: vanishes from the default query, appears only with include_archived. + r = api_call("POST", "/sessions/archive", params={"session_id": s}) + if r.status_code != 200: + return { + "pass": False, + "why": f"archive HTTP {r.status_code}: {r.text[:200]}", + "steps": steps, + } + archived = r.json().get("session") or {} + steps["archive_response_sets_archived_at"] = ( + archived.get("archived_at") is not None + ) + steps["archived_hidden_by_default"] = ( + _find_session(_sessions_query(), s) is None + ) + steps["archived_visible_with_include_flag"] = ( + _find_session(_sessions_query(include_archived=True), s) is not None + ) + + # Unarchive: returns to the default query. + r = api_call("POST", "/sessions/unarchive", params={"session_id": s}) + if r.status_code != 200: + return { + "pass": False, + "why": f"unarchive HTTP {r.status_code}: {r.text[:200]}", + "steps": steps, + } + unarchived = r.json().get("session") or {} + steps["unarchive_response_clears_archived_at"] = ( + unarchived.get("archived_at") is None + ) + steps["unarchived_visible_again"] = ( + _find_session(_sessions_query(), s) is not None + ) + + # Rename via the durable stream header endpoint; the query must reflect the new name. + new_name = f"qa-session-{uuid.uuid4().hex[:8]}" + r = api_call( + "PUT", + "/sessions/streams/header", + params={"session_id": s}, + json={"name": new_name}, + ) + if r.status_code != 200: + return { + "pass": False, + "why": f"rename HTTP {r.status_code}: {r.text[:200]}", + "steps": steps, + } + renamed = r.json().get("stream") or {} + steps["rename_response_reflects_name"] = renamed.get("name") == new_name + after_rename = _find_session(_sessions_query(), s) + steps["rename_query_reflects_name"] = ( + bool(after_rename) and after_rename.get("name") == new_name + ) + + # Delete: gone even with every include flag on — a real hard delete, not another soft flag. + r = api_call("DELETE", "/sessions/", params={"session_id": s}) + if r.status_code != 200: + return { + "pass": False, + "why": f"delete HTTP {r.status_code}: {r.text[:200]}", + "steps": steps, + } + steps["deleted_gone_by_default"] = _find_session(_sessions_query(), s) is None + steps["deleted_gone_with_include_flags"] = ( + _find_session(_sessions_query(include_ended=True, include_archived=True), s) + is None + ) + + ok = all(steps.values()) + return {"pass": ok, "why": f"lifecycle steps: {steps}", "session_id": s} + finally: + # Best-effort cleanup on every path (including a failure above) so a broken run never + # leaves a QA session — possibly renamed or archived — behind in the target project. + try: + api_call("DELETE", "/sessions/", params={"session_id": s}) + except Exception: + pass + + JOURNEYS = { "chat": j1_chat, "mount": j2_mount, @@ -917,6 +1346,9 @@ def j7_mcp(cell: dict) -> dict: "commit": j5_commit, "warm": j6_warm, "mcp": j7_mcp, + "records": j8_records, + "sessions": j9_sessions, + "followup": j10_followup, } @@ -950,6 +1382,17 @@ def main() -> int: "--env-file", help=f"credentials file (fallback when the env vars are unset; default {DEFAULT_ENV_FILE})", ) + p.add_argument( + "--last-message-only", + action="store_true", + help=( + "send only the trailing user message on a fresh turn (mirrors " + "NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY); an approval resume still sends full " + "history. Requires the target stack to run with AGENTA_SESSIONS_RECONSTRUCT=true or " + "the runner has no context to rebuild from. Every turn's exact `sent_messages` lands " + "in results.json for a byte-accurate diff against a full-history run." + ), + ) args = p.parse_args() resolve_credentials(args.env_file) @@ -972,6 +1415,9 @@ def main() -> int: if args.model: for cid in cells: CELLS[cid]["model"] = args.model + if args.last_message_only: + global LAST_MESSAGE_ONLY + LAST_MESSAGE_ONLY = True stamp = time.strftime("%Y%m%d-%H%M%S") outdir = RUNS / stamp diff --git a/.agents/skills/release-qa/SKILL.md b/.agents/skills/release-qa/SKILL.md new file mode 100644 index 0000000000..f0904b6f2c --- /dev/null +++ b/.agents/skills/release-qa/SKILL.md @@ -0,0 +1,129 @@ +--- +name: release-qa +description: >- + Orchestrate pre-release QA for a release branch or any large integration branch: + map what the branch actually ships, write a QA plan with an execution log, then run + layered QA (build + migration gate, wire-level release gate across flag states, + REST-surface probes, acceptance suites, recorded browser pass) with parallel + subagents. Use when the ask is "QA this branch pre-release", "review what this + branch does and QA it", or before merging a long-lived feature train. Complements + the agent-release-gate skill (the wire harness this skill drives). +allowed-tools: Read, Edit, Write, Grep, Glob, Bash, Agent +user-invocable: true +--- + +# Release QA + +QA for a release branch is not one test run. It is: understand what ships, split it by +risk, test each risk class with the cheapest tool that produces hard evidence, and leave +a written trail someone else can audit. + +Everything this skill needs ships with it. Load the extras only when you reach the step +that uses them (progressive disclosure): + +- `resources/plan-template.md` — the plan-doc skeleton. Read when you start Phase B. +- `resources/example-sessions-rework.md` — a sanitized worked example of a full run, + with the lessons it generated. Read before your first-ever run, or when unsure how + much evidence a log row needs. + +## Phase A — map the branch before planning anything + +Delegate two investigations in parallel (subagents; you only need the conclusions): + +1. **Branch map.** `git log --first-parent origin/main..HEAD` for the merged PRs, diff + stats by top-level dir for weight, then read the design docs the branch itself added + and the key changed files. Deliverable: one section per feature cluster with (a) + user-visible behavior, (b) gating flags and their defaults AND parsing rules per + layer, (c) layers touched, (d) riskiest interactions grounded in the code read, not + generic checklists, (e) migrations. The flags table and migration list are + load-bearing: get exact env var names, defaults, and truthiness parsing per layer + (layers disagree; a flag set to `1` can be a silent no-op in one layer and on in + another). +2. **Harness audit.** Read the QA harness you plan to use (for agent-runtime work: + the `agent-release-gate` skill) end to end and diff its assumptions against the + branch map. Two outputs: stale content to fix before trusting it, and coverage gaps + to fill. A green harness that only tests the flags-off legacy path proves nothing + about the branch's headline feature. + +Also check who else is QA-ing. If a teammate has a scope, get it and weight your plan +toward what their scope does not cover; write the division of labor into the plan. + +## Phase B — write the plan as a repo doc, not a chat message + +Create the plan doc from `resources/plan-template.md`, in the repo location your team +uses for QA records (in this repo: `docs/design/agent-workflows/projects/qa/`). The two +structural rules that matter: split what ships into **always-on** versus **flag-gated** +(always-on is where release risk lives, because every customer gets it on upgrade day +regardless of flags), and keep an **execution log** table you fill in as runs complete, +with dates, verdicts, and one-paragraph evidence summaries. Commit it with the QA +changes and open a draft PR against the branch under test — the PR is where recordings +and findings land. + +## Phase C — execute in layers, parallel where independent + +Run these as parallel subagents; only stack-state-mutating steps need to be serial. + +1. **Release mechanics (cheap, first).** Production build of the frontend (build gates + change; a type-error gate that fails builds is a release blocker found in minutes). + New required env vars / deployment couplings (grep compose for new `:?` requirements). +2. **Migration upgrade-in-place.** Scratch database container, run main's migration + chain, seed realistic rows for every table the branch drops or backfills, run the + branch's chain over it. Assert: clean run, data fate matches intent, destructive + migrations round-trip on downgrade. Before declaring a data-loss finding, verify the + seeded shape matches what main's code actually wrote (a synthetic seed can invent a + loss that cannot occur). Note untested load behavior of table-wide backfills. +3. **Wire-level gate across flag states.** Flags-off run (the default customer path), + flags-on run, and — when the branch changes client/server protocol behavior — a + **differential** pair: two runs against one stack changing only the client behavior, + dumping the exact messages sent per turn, and diffing. Defects live in the diff even + when every journey verdict is PASS. Ground warm/cold and drop assertions in the + runner log, not latency or API responses (the runtime fails open; `ok: true` does + not mean persisted). +4. **Flag-mismatch cells.** Test the broken pairings, not just all-on and all-off. + Characterize what actually happens; code analysis over-predicts. Distinguish "fails + loudly" (acceptable hazard) from "silently wrong" (release blocker). +5. **REST surface + acceptance suites.** New/changed endpoints get a scripted lifecycle + journey (create, query, mutate, delete, revive) and the area's existing pytest + acceptance suite pointed at the live stack. +6. **Depth probes** when the branch touches history/session/context handling: a + long-conversation flood (plant a token, ~12 filler turns, recall at depth, latency + per turn to catch unbounded growth), a concurrent-sessions leak check, and a + two-writer race on one session. +7. **Recorded browser pass, last.** One scenario list, ordered, each scenario short and + focused, recorded (GIF/MP4). Cover what only a browser shows: dock/batch flows, + drawer behavior, refresh/cold-replay fidelity, cross-tab sync. Post the recording on + the PR, listed first (house rule). Do this after all stack mutations are restored. + **Always use an isolated browser profile** (a DevTools-launched instance, a dedicated + profile, or incognito) — never the developer's real browser. Cookies are scoped by + hostname without the port, so a QA login on one deployment silently logs the + developer out of every other deployment they have open on the same host. + +**Stack-state discipline:** flag toggles and container recreates run in ONE serial +chain, never parallel with other stack users. Back up the env file first, diff it back +to byte-identical at the end, and prove restoration with a smoke run. If a teammate +shares the stack, check it is idle (request logs) before mutating. + +## Evidence rules + +- Assert on frames, records read back, DB rows, and runner logs. Never on model prose. +- Every "nothing was dropped" claim needs a readback query plus a log grep, because + ingest paths return success unconditionally. +- A finding is not a finding until reproduced against the live stack with the exact + request captured. File real product bugs as GitHub issues immediately (tooling bugs + too); note known/already-filed bugs in the plan so browser agents do not re-file them. +- Record environment facts the next run needs (stack name, creds file location, minted + account ids, teardown list) in a session HANDOFF file, not in the committed plan doc. + Machine-specific details (hosts, ports, key locations) belong in agent memory or + wherever your team records its environments, never in this skill or the plan doc. + +## Orchestration notes + +- Fan out: branch map, harness audit, build check, migration test, and creds-prep can + all run as parallel subagents on day one. Serialize only stack mutations. +- Prep agents mint throwaway accounts via the admin endpoint (see + `api/oss/tests/pytest/utils/accounts.py`) and write a creds env file other agents + source. Vault keys copied from local env files are teardown debt: track and delete. +- Give every agent explicit "report what IS" instructions for hazard cells; expected + failures are findings, not QA failures. +- Update the harness skill (stale refs, new journeys, new lessons) as part of the QA + PR, so the next release starts from a truthful harness. diff --git a/.agents/skills/release-qa/resources/example-sessions-rework.md b/.agents/skills/release-qa/resources/example-sessions-rework.md new file mode 100644 index 0000000000..355f1028c4 --- /dev/null +++ b/.agents/skills/release-qa/resources/example-sessions-rework.md @@ -0,0 +1,59 @@ +# Worked example: sessions-storage-rework release QA (v0.106, 2026-07-28) + +A sanitized snapshot of a real run of this skill, kept as the reference example. +Machine-specific values are replaced with placeholders. The live execution log for that +release stayed in the repo's QA docs; this copy is frozen for teaching. + +## What the branch shipped + +**Always on (no flag):** +1. A turns ledger replaced the mutable `session_states` blob; the old table is dropped + by a lossy migration, so pre-upgrade sessions lose resume pointers and go cold once. +2. Server-backed session list: query, archive/unarchive, propagated delete, revive on + resume, rename synced to a durable header, auto-naming from the first message. +3. Concurrent-approvals hardening plus batch UI (Approve all / Deny all) and + always-allow grants written into the draft config. +4. Warm Stop (cooperative cancel) and Steer (deny an approval with a redirect). +5. Cold-replay transcript fixes (paused turn + resume fold into one message). +6. Config drawer rework (changed-path highlighting, inline what-changed). + +**Flag-gated, default off:** durable records, server-side history reconstruction, +last-message-only sends, smart truncation. Four flags across three layers with +different truthiness parsing (two runner flags accept only the literal `"true"`). + +Release mechanics: web production builds newly fail on any type error; the API gained a +hard dependency on a runner token env var. + +## Division of labor + +A teammate re-ran each sessions-train PR's fixed scenario plus a wire pass and a +regression sweep, flags on. This plan therefore weighted toward: the always-on +surfaces, the upgrade migration, flag-mismatch cells, wire-level record assertions, +build mechanics, and one open defect prediction from an earlier review. + +## How the phases landed (execution log summary) + +| Phase | Verdict | What it proved | +|---|---|---| +| Migration upgrade | PASS | Main's schema + seeded data migrated cleanly; destructive drop round-trips on downgrade. A suspected "name loss" finding was retracted after checking what main actually wrote to the dropped table — synthetic seeds can invent losses that cannot occur. | +| Wire gate, flags off | PASS | 11/11 journeys on the default customer path; the only log `mismatch` was the expected config eviction after the commit journey. | +| Wire gate, flags on, full history | PASS | 10/10 including records readback and sessions REST lifecycle; warm reuse proven from the runner log, not latency. | +| Differential leg (last-message-only) | PASS | Per-turn sent-message dumps proved plain turns shrink to one message while approval resumes keep full history. The open defect prediction (duplicated tool-call record after approval resume) did not reproduce; learned that the harness legitimately reuses a wire toolCallId when a paused call settles. | +| Truncation shapes | PASS | Direct ingest of an oversized record (harnesses pre-clip tool output, so the natural route can never reach the cap): flag off gives a surviving placeholder record; flag on preserves structure with truncation metadata. | +| Flag-mismatch cell | NARROWED | The predicted "silent total context loss" did not occur: native harness continuity (independent of the reconstruct flag) restored context across warm eviction; the only forced break failed loudly. Residual risk confined to multi-replica routing, explicitly recorded as untested. | +| Acceptance suite | PASS | 177/177 against the live stack. | +| Two-writer race | FINDINGS | Concurrent turns are not gated at start (parallel sandboxes per session); the loser is reaped only at its next 30s heartbeat; the turn right after a takeover can land on a pool entry mid-teardown and fail user-visibly with no retry. Two issues filed (one bug, one design characterization). Record log stayed consistent throughout — worth stating explicitly. | +| Depth probes + recorded browser pass | run per the skill's phase list | + +## Transferable lessons this run generated + +1. The differential method finds what verdicts cannot: both legs passed everything; + the evidence value was in the diffs and logs. +2. Characterization beats prediction: two "certain" hazards from code analysis (context + loss, name loss) both dissolved under live probing, while an unpredicted race became + the sharpest bug of the day. +3. Check who else is testing before planning; half the branch was already covered. +4. Serial stack mutations with byte-identical env restoration made five flag states + testable on one shared stack without breaking anyone. +5. QA logins in the developer's real browser kill their other sessions (cookies are + host-scoped, not port-scoped): isolated browser profile, always. diff --git a/.agents/skills/release-qa/resources/plan-template.md b/.agents/skills/release-qa/resources/plan-template.md new file mode 100644 index 0000000000..a11525032b --- /dev/null +++ b/.agents/skills/release-qa/resources/plan-template.md @@ -0,0 +1,67 @@ +# Release QA plan: `` () + +Date: . Target stack: , deployed from , env file +. Flag state on that stack: . + +## What the branch ships + +Two risk classes. Fill this from the branch map (Phase A of the skill), not from PR +titles. + +**Always on (no flag) — what every customer gets on upgrade:** + +1. +2. ... + +**Flag-gated, default off:** + +Release mechanics: . + +## Division of labor + + + +## Phases + +**Phase 0 — release mechanics.** + +**Phase 1 — wire gate, flags off.** + +**Phase 2 — flags on + differential.** + +**Phase 3 — REST surface + acceptance suites.** + +**Phase 3b — depth probes.** + +**Phase 4 — recorded browser pass, last, in an isolated browser profile.** + +## Targeted edge cases + +Derived from reading the changed code, not from a generic checklist. Numbered, each one +sentence of what to do plus the failure it would expose. + +1. ... + +## Flags + +| Flag | Layer | Default | Parsing | +|---|---|---|---| + +Parsing matters: layers disagree on truthiness, and a flag set to `1` can be a silent +no-op in one layer while another accepts it. + +## Execution log + +Fill in as runs complete. Every row carries date, verdict, and a one-paragraph evidence +summary someone can audit without the transcripts. FINDINGS rows link the issues filed. + +| Phase | Status | Notes | +|---|---|---| diff --git a/.claude/skills/release-qa b/.claude/skills/release-qa new file mode 120000 index 0000000000..6a6d6ea844 --- /dev/null +++ b/.claude/skills/release-qa @@ -0,0 +1 @@ +../../.agents/skills/release-qa \ No newline at end of file diff --git a/.gitignore b/.gitignore index 53b7bd01af..4c2a66a9de 100644 --- a/.gitignore +++ b/.gitignore @@ -85,12 +85,14 @@ services/runner/tests/results/ !.agents/skills/ .agents/skills/* !.agents/skills/agent-release-gate/ +!.agents/skills/release-qa/ !.agents/skills/write-template-playbooks/ !.claude/ .claude/* !.claude/skills/ .claude/skills/* !.claude/skills/agent-release-gate +!.claude/skills/release-qa !.claude/skills/write-template-playbooks # Temporary SDK copies created by run.sh --local diff --git a/docs/design/agent-workflows/projects/qa/release-2026-07-sessions-storage-rework.md b/docs/design/agent-workflows/projects/qa/release-2026-07-sessions-storage-rework.md new file mode 100644 index 0000000000..32d7101c01 --- /dev/null +++ b/docs/design/agent-workflows/projects/qa/release-2026-07-sessions-storage-rework.md @@ -0,0 +1,205 @@ +# Release QA plan: `feat/sessions-storage-rework` (v0.106.x) + +Date: 2026-07-28. Target stack: `agenta-ee-dev-sessions` on :8480, deployed from +`/home/mahmoud/code/agenta-106-2` in dev mode (source-mounted), env file +`hosting/docker-compose/ee/.env.ee.dev.sessions`. Session flags ON on that stack +(`AGENTA_SESSIONS_RECONSTRUCT`, `AGENTA_RECORDS_DURABLE`, +`NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY`); `AGENTA_RECORDS_SMART_TRUNCATION` is NOT set +on the API and must be enabled for the truncation tests. + +## What the branch ships + +Two risk classes: + +**Always on (no flag) — this is what every customer gets on upgrade:** + +1. Turns ledger replaces `session_states` (migration `oss000000017` drops the old table + with no data migration; pre-upgrade sessions lose their resume pointer and go cold). +2. Server-backed session list: `POST /sessions/query`, archive/unarchive via + `archived_at`, server-propagated delete, revive of killed sessions on resume, rename + synced to the durable stream header, auto-naming from the first user message. +3. Concurrent-approvals hardening (parked gate map replaces the single latch) plus the + batch UI (Approve all / Deny all with context peek) and "always allow this tool" + grants written into the draft config. +4. Warm Stop (cooperative cancel; session resumable, sandbox destroyed) and Steer + (deny + redirect, behind `NEXT_PUBLIC_AGENT_CHAT_STEER`). +5. Cold-replay transcript fixes: paused turn + resume fold into one message, pause + sentinels render as nudges, behind-server snapshots cannot clobber a paused local tail. +6. Config drawer rework: changed-path highlighting with restore, dirty sections expand + inline showing only changed controls, inline provider-key connect. + +**Flag-gated, default off:** durable records, server-side history reconstruction, +last-message-only sends, smart truncation. The six fix PRs from the July 24 differential +QA (#5488–#5495) are merged. + +Release mechanics: web production builds now fail on any TypeScript error +(`ignoreBuildErrors: false`), and the platform API calls the runner directly for kill, +so `AGENTA_RUNNER_TOKEN` must be set or compose fails. + +## Division of labor + +A colleague is re-running each sessions-train PR's fixed scenario against a live stack, +plus a wire-level pass (chat/approve/deny/warm/mount) and a regression sweep, with the +four flags on, and separately verifying they default off in source. This plan therefore +weights toward what that scope does not cover: + +- Upgrade-in-place across the lossy migration. +- Flag-mismatch cells (web on / runner off loses all context on cold turns; runner on / + web off still reconstructs on turn 1). +- The unflagged surfaces: session list REST + UI, batch approvals, always-allow grants, + Stop, Steer, config drawer, cold-replay approval fidelity. +- Wire-level record assertions (ordering, truncation shape, silent drops) rather than + scenario re-runs. +- Build mechanics (gh image build under the tsc gate; `AGENTA_RUNNER_TOKEN` coupling). +- The open case from July 24: duplicated tool_call id on a fresh turn after an approval + resume. + +## Phases + +**Phase 0 — release mechanics.** Build gh images (tsc gate). DB-level upgrade test: +main's migrations + seeded `session_states`/streams rows on a scratch Postgres, then the +branch's migrations; assert clean run, then confirm a pre-existing session lists and +continues (cold) on the stack. + +**Phase 1 — release gate, flags off.** Run `agent-release-gate` (cells C1 + C3 minimum) +with the three runner/web flags off, to regression-check the legacy path on branch code. + +**Phase 2 — flags-on differential.** Same stack, flags on. Run the gate twice: once with +the stock full-history client, once with the driver's last-message-only mode, and diff +the message arrays the runner hands the model. Re-verify the four fixed defects (no +duplicated turn-1 prompt; warm session survives minimal history, log-grounded; oversized +records truncated, not dropped, with and without smart truncation; producer-time +ordering). Close the duplicated-tool_call-id case after an approval resume. + +**Phase 3 — sessions REST surface.** Wire journeys against `/api/sessions/*`: query with +`include_ended`, archive → unarchive, rename via the header endpoint, hard delete +fan-out, revive on resume. Plus records queries asserting order and truncation shape. +Run the API acceptance suite (`api/oss/tests/pytest/acceptance/sessions/`). + +**Phase 4 — UI QA, recorded.** MP4 for the PR: cross-device list sync, archive/rename/ +delete from the rail, a multi-gate turn answered with Approve all, an always-allow grant +honored on the next call, Stop mid-turn then continue, Steer, paused-turn refresh (one +merged message, no duplicate cards), config drawer changed-path highlight + restore + +inline what-changed. Known-broken and excluded: mid-turn refresh sticks on a stale +transcript (issue #5530, assigned). + +## Targeted edge cases + +1. Delete/archive a session under ~60s old (not yet server-known): next poll must not + resurrect it. (Known risk: local-only action.) +2. Reconcile treats absence as deletion; a session whose turn-reference join misses must + not wipe the local transcript. +3. Two tabs on one session: a lock steal must not abort a healthy turn or tear down the + sandbox (`is_current_turn: false` path). +4. Runner restart while parked on an approval: next turn must not 409/overwrite the turn + row into an unresumable state. +5. Flag mismatch cells (both directions), and `AGENTA_RECORDS_DURABLE=1` being a silent + no-op (runner accepts only the literal string `true`). +6. Always-allow: non-matching grant fails silently; rule must match the tool name + verbatim; Undo pins `permission: "ask"` leaving a phantom draft diff. +7. Reused toolCallId across turns folds into the old bubble and the approval dock (which + scans only the last message) shows nothing: run parks with no visible gate. +8. A real tool output beginning with a pause-sentinel string renders as a nudge. +9. Rename on a killed session: API returns 200 and silently does nothing; reachable from + the UI since ended sessions are listed. +10. Records query 403 (credential without VIEW_SESSIONS) silently degrades a cold turn to + one-message context. + +## Flags + +| Flag | Layer | Default | Parsing | +|---|---|---|---| +| `AGENTA_RECORDS_DURABLE` | runner | off | strict `"true"` only | +| `AGENTA_SESSIONS_RECONSTRUCT` | runner | off | strict `"true"` only | +| `NEXT_PUBLIC_SESSIONS_LAST_MESSAGE_ONLY` | web | off | must pair with runner reconstruct | +| `AGENTA_RECORDS_SMART_TRUNCATION` | API | off | broader truthy set | +| `NEXT_PUBLIC_AGENT_CHAT_STEER` | web | off | gates Redirect button | +| `NEXT_PUBLIC_AGENT_CHAT_STOP_KILLS_SESSION` | web | off | restores hard-kill Stop | + +## Execution log + +Filled in as runs complete; per-run results land under the gate's runs directory and are +summarized here. + +| Phase | Status | Notes | +|---|---|---| +| 0 build gate | PASS | 2026-07-28: OSS and EE typecheck 0 errors; OSS and EE + production builds pass under `ignoreBuildErrors: false` (~2m each). Only benign + webpack warnings for the AI SDK's optional peer deps (`effect`, + `@valibot/to-json-schema`), not type errors. Matches tsc-error-inventory (0/0). | +| 0 migration | PASS | 2026-07-28, scratch Postgres 16. Main's chain, seeded session + data, then the branch chain: clean run, no constraint failures; `016` backfill + extracts `agent_id` correctly; `017` downgrade/re-upgrade round-trips. Confirmed + loss is limited to resume pointers (`session_states` held only continuity fields on + main; names were never stored there), so pre-upgrade sessions resume cold but keep + their history. Caveat: `016`'s table-wide mounts UPDATE is untested at production + scale. Reusable migration command recorded in the job log. | +| 1 flags-off gate | PASS | 2026-07-28, C1/haiku, three session flags off (legacy + path): 11/11 journeys PASS; records still ingest via the fire-and-forget path; the + only runner-log `mismatch` was the expected config-fingerprint eviction after the + commit journey bumps the revision. | +| 1b truncation shapes | PASS | Direct ingest of a 100KB tool_result (harnesses + pre-clip bash output at 30-50KB, so the bash route can never reach the 64KB cap): + flag off → record persisted with the bare `{"_truncated": true}` placeholder (never + absent, the #5491 check); flag on → structure preserved, trimmed field marked, with + `_truncated: {fields, original_bytes}`. | +| 1c mismatch hazard | NARROWED | Client minimal-history with reconstruction off did + NOT silently lose context on this stack: the harness's always-on native session + continuity (session/load from the durable mount) restored context across warm-pool + eviction on both Claude and Pi. The only forced break (runner replica change, local + sandbox) fails loudly: "Refusing to cold-start on the wrong host." Residual risk is + multi-replica / Daytona routing, untested here. | +| 2 differential, leg 1 | PASS | 2026-07-28, cell C1 (claude/local/subscription, haiku), + flags on, full-history client: 10/10 journeys PASS including the new `records` and + `sessions` journeys. Runner log: warm reuse confirmed on multi-turn journeys + (`hit-continue`/`resume`), no `DROPPED`/`degraded`/`skipped` hits; the only `cold` + lines are legitimate first turns. | +| 2 differential, leg 2 | PASS | 2026-07-28, C1/haiku, flags on, `--last-message-only`: + chat, warm, and the new `followup` journey all PASS. `sent_messages` dumps prove + plain turns send exactly one message while approval resumes keep full history, + matching `agentRequest.ts`. The open July 24 case (duplicated tool_call record after + an approval resume) did NOT reproduce, in either client mode. Note for future runs: + Claude reuses the same wire toolCallId when a paused call settles post-resume; that + is expected, not a defect. | +| 3 acceptance suite | PASS | 2026-07-28, 177/177 against the live stack (:8480): + archive/unarchive, records ingest contract, stream headers, DAO unit+integration. | +| 3b two-writer race | FINDINGS | Concurrent turns on one session are not gated at + start (parallel sandboxes per session); the loser is reaped only at its next 30s + heartbeat; a disconnected client's turn runs headless until that same beat; and the + turn right after a takeover can land on a pool entry mid-teardown and fail with a + user-visible internal error, no retry (1 of 2 attempts; self-heals next turn). + Records stayed consistent throughout. Filed #5538 (teardown race, bug) and #5539 + (characterization). | +| 3c depth probes | PASS | Memory flood: token planted turn 1 recalled exactly at turn + 14, in BOTH client modes; with last-message-only the runner reconstructed from a + linearly growing record log (159 records by turn 14) with no measurable latency + growth at this depth (caveat: does not rule out degradation at much larger depths). + Concurrent probe: 4 parallel sessions, each recalled only its own token, no leaks. + Records ~12/turn, no truncation drop-ins, order correct. Gmail probe skipped (no + Composio connections on the QA project). | +| 3 REST surface | pending | | +| 4 UI pass | FINDINGS | 2026-07-28, isolated browser profile. PASS: rail basics + (auto-name, rename survives refresh, archive/unarchive, mature delete), always-allow + (grant honored next turn, visible under Advanced permissions, draft state shown), + warm Stop (clean stop, context-aware follow-up), Steer (deny + redirect executed), + cross-tab list sync. FAIL: refreshing while paused on an approval gate loses the + gate; turn renders "No response", command never runs (#5542, distinct from #5530). + Delete of a ~69s-old session resurrects on reconcile (#5543). Config drawer shows no + changed-path/Restore treatment for provider-level changes; dirty section expands to + the full form (#5544). Batch approvals not exercisable on the Claude harness (gates + arrive one at a time). | +| 4b batch approvals (Pi) | FINDINGS | Pi genuinely raised 3 simultaneous bash gates + (twice), but the dock renders 1 live gate + N "waiting on another approval" cards, + so the Approve-all split button (`count > 1`) never appears on any harness: the + batch UI is unreachable, not broken (#5545). Sequential stepping works end to end + for both approve-all-one-by-one and deny-all-one-by-one. | + +## Verdict summary + +The always-on surface is release-ready with three UI defects to fix or accept +(#5542 paused-gate lost on refresh — the sharpest one; #5543 young-delete +resurrection; #5544 config-drawer what-changed gaps) and one unreachable feature +(#5545 batch-approval UI). The flag-gated sessions train passed everything thrown at +it, including depth and differential probes. Runner concurrency has one real bug +(#5538) and one design decision to make (#5539). Build and migration gates are green. +Tooling: #5534 (run.sh --env-file).