test(agent): package the agent release gate as a skill with resources#5348
Closed
mmabrouk wants to merge 8 commits into
Closed
test(agent): package the agent release gate as a skill with resources#5348mmabrouk wants to merge 8 commits into
mmabrouk wants to merge 8 commits into
Conversation
…time Adds a product-path QA driver that drives /services/agent/v0/invoke the same way the playground does and asserts on SSE frames rather than model prose, plus a long-context/compaction/concurrency probe. Also adds the Playwright testability backlog and the STATUS.md findings log from today's pre-release QA pass, with the run artifacts (results.json + summary.md) as evidence.
… swallowed provider errors - F-7 (release blocker): on a gh self-host deployment + Daytona, the durable mount is silently skipped — the bundled SeaweedFS (added by #5315) is loopback-only with traefik disabled, and gh ships no tunnel (only dev has ngrok). Agent files never persist; the UI looks normal. - F-9: the Claude harness never resumes its native session — 96/96 mode=create, zero loads over 72h on the dev stack. Every cold turn is rebuilt from a hand-rendered transcript that hard-truncates tool results at 4000 chars. - F-11 (regression from 42075a5): Pi transcripts moved to <cwd>/agents/sessions/pi/ but findSwallowedPiError's callsite still reads the old agent dir, so every Pi provider error (dead key, quota, rate limit) surfaces as "The agent produced no output." - F-3: Pi's permission layer fails open when its enforcement extension can't install — ask/deny become no-ops. - F-5 / F-8: wire and DX traps — tool-input-available streams partial input and flips the tool name's case; /tools/discover output is rejected by the agent config (extra_forbidden). - Adds a product-path QA driver asserting on SSE frames and side effects (never model prose), plus LESSONS.md. Also allowlists the synthetic QA canary token shape (QA-MEM-..., QA-CONC...) in .gitleaks.toml so the runs/** evidence files pass the secret scan. Claude-Session: https://claude.ai/code/session_01Hyn9365BLPXDmNZShrQkmH
…ession resume works after redeploy Ran a decisive experiment: plant an EARLY token near the start of a large bash output and a LATE token past the 4000-char transcript truncation cap, wait past the 60s keepalive pool TTL so the session goes cold, then ask for both back via a faithful history replay. Against the redeployed bighetzner stack, both tokens came back in 4/4 runs (2x claude/local, 2x pi_core/local), and the runner log shows native session load succeeding for both harnesses (mode=load, loaded=true). Downgrades F-9 from a CONFIRMED release blocker to a residual resilience risk: the lossy 4000-char rebuild path still exists as a fallback when native load genuinely fails (sandbox recreate, Daytona teardown, runner restart), but is no longer the routine outcome of a cold turn. Likely cause: the same redeploy that pulled recent upstream fixes tonight; the original 96/96 mode=create observation predates it. Also extends the gitleaks QA-canary allowlist to cover the EARLY/LATE token shape, and un-ignores runs/**/run.log so raw runner-log evidence can be committed alongside results.json.
J5 commit drives the playground's Save/Commit REST route (POST /api/workflows/revisions/commit): create workflow+variant, seed v0, commit the real change as v1, then fetch the revision back and assert the changed parameter survived and the version bumped. Handles the v0-seed trap (the DAO force-nulls version-0 data) and archives the QA workflow to clean up. J7 mcp declares a public HTTPS MCP server (DeepWiki) in the agent config and asserts an mcp__* tool executes (tool-output-available on the wire). Claude-only (Pi rejects any run with mcps) so it SKIPs on Pi cells; public-HTTPS-only (SSRF guard) so a local server is unreachable. --mcp-url overrides the default. Both PASS on the settled bighetzner stack (commit on P1, mcp on C1). STATUS and LESSONS updated with the endpoint/seed and Claude-only/public-URL facts.
… on the redeployed stack C2 (claude + daytona) passes all six journeys with the funded vault key, including the durable mount — the store exposure fix landed on the deployment, so F-7 is verified fixed end to end. Adds the S1 cell (pi_core + openai-codex subscription), which passes chat, tool use, and approval. The OpenAI vault key remains out of quota (external issue, not a runtime defect). Claude-Session: https://claude.ai/code/session_01Hyn9365BLPXDmNZShrQkmH
…ce bug, Daytona file persistence verified A three-config UI walk passed the golden path end to end, including Daytona file persistence through the store exposure. It found a HIGH Claude Terminal approval-persistence bug (one approve becomes a permanent grant across sessions), an Advanced-drawer flake, and a missing Daytona+subscription warning. The testability selector list is appended to the findings. Claude-Session: https://claude.ai/code/session_01Hyn9365BLPXDmNZShrQkmH
…oves read-only bash under Ask, no persisted grant A follow-up investigation (live probes + code trace) found no persisted grant: approvals are answered once-only, no settings file is written, and mutating commands re-gate every time including in new sessions. The "Terminal never asked again" repro was Claude Code's own command classifier auto-approving READ-ONLY commands under the "Ask" policy, which never raise a gate at all. Reclassifies the bug from HIGH (security persistence) to MEDIUM (policy-projection gap), same family as F-6 (the Policy control not governing Claude's builtins). Keeps the original observation for the record in both ui-exploration-20260714.md and STATUS.md.
Graduate the pre-release agent-QA harness out of its design-docs folder into a portable, tracked skill. The three drivers move into .agents/skills/agent-release-gate/resources/ and are cleaned to read credentials from AGENTA_BASE / AGENTA_PROJECT_ID / AGENTA_API_KEY (env-first, --env-file fallback) with a clear missing-credentials error, and to write runs to ./qa-gate-runs/ in the CWD (never into the skill). Adds SKILL.md (progressive disclosure), coverage.md (cells x journeys), and seeds/ with representative green results. Deletes superseded/broken/graduated files from the design folder (run_matrix.py + warm_daytona_probe.py, mcp_qa_server.mjs, regression-skill-DRAFT.md, cleanup-plan.md); that folder stays as the release-night findings archive. Claude-Session: https://claude.ai/code/session_019TK4ow9a3dXUNY657djbQr
Member
Author
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds the ChangesAgent release gate
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant QA_Driver
participant InvokeEndpoint
participant AgentRunner
participant Sandbox
QA_Driver->>InvokeEndpoint: POST invocation with SSE headers
InvokeEndpoint->>AgentRunner: execute configured agent turn
AgentRunner->>Sandbox: run tool or persist working-directory state
Sandbox-->>AgentRunner: tool output and side-effect result
AgentRunner-->>InvokeEndpoint: SSE wire frames
InvokeEndpoint-->>QA_Driver: parsed Turn and journey verdict
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: be20917a-1d32-4f9f-bd62-2a03f7ff70e1
⛔ Files ignored due to path filters (1)
docs/design/agent-workflows/projects/qa/runs/coldctx-20260714-201002/run.logis excluded by!**/*.log
📒 Files selected for processing (124)
.agents/skills/agent-release-gate/SKILL.md.agents/skills/agent-release-gate/resources/LESSONS.md.agents/skills/agent-release-gate/resources/coverage.md.agents/skills/agent-release-gate/resources/qa_longctx.py.agents/skills/agent-release-gate/resources/qa_probe.py.agents/skills/agent-release-gate/resources/qa_product.py.agents/skills/agent-release-gate/resources/seeds/README.md.agents/skills/agent-release-gate/resources/seeds/longctx-daytona-all-green.results.json.agents/skills/agent-release-gate/resources/seeds/product-C2-full-green.results.json.claude/skills/agent-release-gate.gitignore.gitleaks.tomldocs/design/agent-workflows/projects/qa/README.mddocs/design/agent-workflows/projects/qa/STATUS.mddocs/design/agent-workflows/projects/qa/cleanup-plan.mddocs/design/agent-workflows/projects/qa/playwright-testability.mddocs/design/agent-workflows/projects/qa/regression-skill-DRAFT.mddocs/design/agent-workflows/projects/qa/runs/20260714-173326/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-173326/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-173439/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-173439/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-173718/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-173718/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-173905/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-173905/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-173938/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-173938/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-174547/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-174547/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-174733/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-174733/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-174932/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-174932/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-175105/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-175105/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-175715/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-175715/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-183158/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-183158/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185051/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185051/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185108/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185108/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185123/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185123/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185149/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185149/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185153/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185153/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185157/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185157/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185209/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185209/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185425/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185425/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185430/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185430/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185506/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185506/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185510/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185510/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185514/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185514/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185525/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185525/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185547/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185547/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185609/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185609/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-185639/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-185639/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-190210/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-190210/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-190316/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-190316/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-212356/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-212356/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-212400/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-212400/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-212423/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-212423/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-214209/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-214209/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-214246/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-214246/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-214326/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-214326/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-214412/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-214412/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-214453/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-214453/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-214528/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-214528/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215005/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215005/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215055/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215055/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215144/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215144/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215238/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215238/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215252/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215252/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215309/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215309/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215322/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215322/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-215335/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-215335/summary.mddocs/design/agent-workflows/projects/qa/runs/20260714-222731/results.jsondocs/design/agent-workflows/projects/qa/runs/20260714-222731/summary.mddocs/design/agent-workflows/projects/qa/runs/coldctx-20260714-201002/results.jsondocs/design/agent-workflows/projects/qa/runs/longctx-daytona-20260714-184404/results.jsondocs/design/agent-workflows/projects/qa/runs/longctx-local-20260714-183451/results.jsondocs/design/agent-workflows/projects/qa/runs/longctx-local-20260714-183553/results.jsondocs/design/agent-workflows/projects/qa/runs/longctx-local-20260714-183656/results.jsondocs/design/agent-workflows/projects/qa/runs/longctx-local-20260714-184046/results.jsondocs/design/agent-workflows/projects/qa/runs/longctx-local-20260714-185716/results.jsondocs/design/agent-workflows/projects/qa/runs/longctx-local-20260714-191137/results.jsondocs/design/agent-workflows/projects/qa/scripts/coldctx_experiment.pydocs/design/agent-workflows/projects/qa/scripts/mcp_qa_server.mjsdocs/design/agent-workflows/projects/qa/scripts/warm_daytona_probe.pydocs/design/agent-workflows/projects/qa/ui-exploration-20260714.mddocs/designs/testing/README.md
💤 Files with no reviewable changes (4)
- docs/design/agent-workflows/projects/qa/scripts/mcp_qa_server.mjs
- docs/design/agent-workflows/projects/qa/scripts/warm_daytona_probe.py
- docs/design/agent-workflows/projects/qa/cleanup-plan.md
- docs/design/agent-workflows/projects/qa/regression-skill-DRAFT.md
Comment on lines
+142
to
+162
| def probe_gmail(sandbox: str) -> dict: | ||
| """Do the Gmail tools resolve AND execute?""" | ||
| s = str(uuid.uuid4()) | ||
| p = params(sandbox, GATEWAY_TOOLS + [BASH]) | ||
| t = qa.invoke( | ||
| s, | ||
| [ | ||
| qa.user_msg( | ||
| "List the subjects of the 3 most recent emails in my inbox, then list my GitHub repositories. Use the tools." | ||
| ) | ||
| ], | ||
| p, | ||
| timeout=420.0, | ||
| ) | ||
| ok = qa.tool_ran(t) and not t.errors | ||
| return { | ||
| "pass": ok, | ||
| "why": "a Gmail gateway tool executed (tool-output-available) with no error", | ||
| "tools_called": [c.get("toolName") for c in t.tool_calls], | ||
| "turn": t.summary(), | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require successful gateway calls, not any tool output.
Because Bash is also configured, tool_ran(t) can pass without Gmail or GitHub executing. Match successful outcomes to the discovered gateway tools and require the integrations claimed by this probe.
Comment on lines
+191
to
+218
| for i in range(turns): | ||
| q = filler[i % len(filler)] | ||
| msgs.append(qa.user_msg(q)) | ||
| t = qa.invoke(s, msgs, p, timeout=420.0) | ||
| msgs.append(t.assistant_message()) | ||
| trace.append( | ||
| {"turn": i + 2, "ms": t.ms, "tools": len(t.tool_calls), "err": t.errors[:1]} | ||
| ) | ||
| print( | ||
| f" flood turn {i + 2}/{turns + 1}: {t.ms}ms tools={len(t.tool_calls)}", | ||
| flush=True, | ||
| ) | ||
|
|
||
| msgs.append( | ||
| qa.user_msg( | ||
| "What was the exact token I asked you to remember at the very start? Reply with only the token." | ||
| ) | ||
| ) | ||
| final = qa.invoke(s, msgs, p, timeout=420.0) | ||
| ok = token in final.reply | ||
| return { | ||
| "pass": ok, | ||
| "why": f"the token planted in turn 1 survived {turns} tool-heavy turns (token={token})", | ||
| "token": token, | ||
| "recalled": final.reply[:120], | ||
| "flood": trace, | ||
| "session_id": s, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail when the flood never produced the context being tested.
Flood-turn errors and missing tool executions are ignored, while the final verdict only checks prose. The token can therefore be recalled from replayed history without exercising bulky tool output or compaction. Require successful expected tool output on each flood turn and reject all turn errors.
Comment on lines
+225
to
+269
| def one(i: int) -> dict: | ||
| s = str(uuid.uuid4()) | ||
| p = params(sandbox, GATEWAY_TOOLS + [BASH]) | ||
| tok = tokens[i] | ||
| msgs = [qa.user_msg(f"Remember this token: {tok}. Reply only: OK")] | ||
| t = qa.invoke(s, msgs, p, timeout=420.0) | ||
| msgs.append(t.assistant_message()) | ||
| msgs.append( | ||
| qa.user_msg( | ||
| "Fetch my 3 most recent emails with the Gmail tool and summarize them." | ||
| ) | ||
| ) | ||
| t = qa.invoke(s, msgs, p, timeout=420.0) | ||
| msgs.append(t.assistant_message()) | ||
| msgs.append( | ||
| qa.user_msg( | ||
| "What token did I ask you to remember? Reply with only the token." | ||
| ) | ||
| ) | ||
| t = qa.invoke(s, msgs, p, timeout=420.0) | ||
| return { | ||
| "i": i, | ||
| "expected": tok, | ||
| "reply": t.reply[:80], | ||
| "session": s, | ||
| "errors": t.errors[:1], | ||
| } | ||
|
|
||
| with ThreadPoolExecutor(max_workers=n) as ex: | ||
| results = list(ex.map(one, range(n))) | ||
|
|
||
| own = all(r["expected"] in r["reply"] for r in results) | ||
| # The leak check: did any session echo ANOTHER session's token? | ||
| leaks = [ | ||
| {"session": r["i"], "leaked_token_of": j} | ||
| for r in results | ||
| for j, tok in tokens.items() | ||
| if j != r["i"] and tok in r["reply"] | ||
| ] | ||
| return { | ||
| "pass": own and not leaks, | ||
| "why": f"each of {n} concurrent sessions recalled ITS OWN token and none leaked another's", | ||
| "leaks": leaks, | ||
| "results": results, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include turn health in the concurrency verdict.
The probe passes solely from token replies; the Gmail load step and captured errors do not affect pass. Accumulate errors and successful tool outcomes across all three turns before evaluating ownership and leaks.
Comment on lines
+556
to
+575
| # A paused turn finishes with reason "other", not "stop". | ||
| paused_ok = t1.finish_reason == "other" | ||
|
|
||
| gated_call = next( | ||
| (c for c in t1.tool_calls if c["toolCallId"] == t1.approval["toolCallId"]), | ||
| t1.tool_calls[-1] if t1.tool_calls else {}, | ||
| ) | ||
| gated_input = gated_call.get("input") or {} | ||
| msgs = msgs + [approval_reply(t1, approved)] | ||
| t2 = invoke(s, msgs, params) | ||
| outcome = outcome_for_input(t2, gated_input) | ||
|
|
||
| if approved: | ||
| ok = outcome == "available" and not t2.errors | ||
| why = f"approved: the gated command executed after approval (outcome={outcome}, paused finish=other: {paused_ok})" | ||
| else: | ||
| # Denied: the gated COMMAND must never have executed. Assert the WIRE, never the reply — | ||
| # a denied model will happily hallucinate the output it never received. | ||
| ok = outcome != "available" | ||
| why = f"denied: the gated command never executed (outcome={outcome})" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not treat a missing denial outcome as a pass.
outcome != "available" accepts None, including failed resume or indefinite re-parking. Neither branch enforces paused_ok, and denial ignores turn errors. Require a clean paused turn and the exact terminal outcome.
Proposed fix
+ terminal_ok = paused_ok and not t1.errors and not t2.errors
if approved:
- ok = outcome == "available" and not t2.errors
+ ok = terminal_ok and outcome == "available"
else:
- ok = outcome != "available"
+ ok = terminal_ok and outcome == "denied"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # A paused turn finishes with reason "other", not "stop". | |
| paused_ok = t1.finish_reason == "other" | |
| gated_call = next( | |
| (c for c in t1.tool_calls if c["toolCallId"] == t1.approval["toolCallId"]), | |
| t1.tool_calls[-1] if t1.tool_calls else {}, | |
| ) | |
| gated_input = gated_call.get("input") or {} | |
| msgs = msgs + [approval_reply(t1, approved)] | |
| t2 = invoke(s, msgs, params) | |
| outcome = outcome_for_input(t2, gated_input) | |
| if approved: | |
| ok = outcome == "available" and not t2.errors | |
| why = f"approved: the gated command executed after approval (outcome={outcome}, paused finish=other: {paused_ok})" | |
| else: | |
| # Denied: the gated COMMAND must never have executed. Assert the WIRE, never the reply — | |
| # a denied model will happily hallucinate the output it never received. | |
| ok = outcome != "available" | |
| why = f"denied: the gated command never executed (outcome={outcome})" | |
| # A paused turn finishes with reason "other", not "stop". | |
| paused_ok = t1.finish_reason == "other" | |
| gated_call = next( | |
| (c for c in t1.tool_calls if c["toolCallId"] == t1.approval["toolCallId"]), | |
| t1.tool_calls[-1] if t1.tool_calls else {}, | |
| ) | |
| gated_input = gated_call.get("input") or {} | |
| msgs = msgs + [approval_reply(t1, approved)] | |
| t2 = invoke(s, msgs, params) | |
| outcome = outcome_for_input(t2, gated_input) | |
| terminal_ok = paused_ok and not t1.errors and not t2.errors | |
| if approved: | |
| ok = terminal_ok and outcome == "available" | |
| why = f"approved: the gated command executed after approval (outcome={outcome}, paused finish=other: {paused_ok})" | |
| else: | |
| # Denied: the gated COMMAND must never have executed. Assert the WIRE, never the reply — | |
| # a denied model will happily hallucinate the output it never received. | |
| ok = terminal_ok and outcome == "denied" | |
| why = f"denied: the gated command never executed (outcome={outcome})" |
Comment on lines
+593
to
+619
| 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.""" | ||
| s = str(uuid.uuid4()) | ||
| params = template(cell) | ||
| msgs: list = [] | ||
| times = [] | ||
| for i, q in enumerate( | ||
| [ | ||
| "Reply with exactly: ONE", | ||
| "Reply with exactly: TWO", | ||
| "Reply with exactly: THREE", | ||
| ] | ||
| ): | ||
| msgs = msgs + [user_msg(q)] | ||
| t = invoke(s, msgs, params) | ||
| times.append(t.ms) | ||
| msgs = msgs + [t.assistant_message()] | ||
| if t.errors: | ||
| return {"pass": False, "why": f"turn {i + 1} errored", "turn": t.summary()} | ||
| 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, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Latency alone cannot prove a warm session.
Any positive jitter makes this pass, although the documented contract requires explicit session-load evidence. Verify a runner signal such as hit-continue keyed to this session, or report latency as observational rather than PASS.
Comment on lines
+1
to
+38
| { | ||
| "C3": { | ||
| "config": { | ||
| "harness": "pi_core", | ||
| "sandbox": "local", | ||
| "model": "gpt-5.6-luna", | ||
| "provider": "openai" | ||
| }, | ||
| "journeys": { | ||
| "chat": { | ||
| "pass": false, | ||
| "why": "finish=stop and reply contains PONG", | ||
| "turn": { | ||
| "http": 200, | ||
| "ms": 7453, | ||
| "finish": null, | ||
| "frames": [ | ||
| "start", | ||
| "start-step", | ||
| "error", | ||
| "error", | ||
| "finish-step", | ||
| "error", | ||
| "finish" | ||
| ], | ||
| "tools": [], | ||
| "approval": false, | ||
| "errors": [ | ||
| "{\"type\": \"error\", \"errorText\": \"pi_core: the model provider account has insufficient credit (check the project's OpenAI key).\"}", | ||
| "{\"type\": \"error\", \"errorText\": \"Agent run failed: pi_core: the model provider account has insufficient credit (check the project's OpenAI key).\"}", | ||
| "{\"type\": \"error\", \"errorText\": \"The agent produced no output.\"}" | ||
| ], | ||
| "reply": "" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Schema mismatch with downstream replay tests.
The newly generated results.json files introduce a nested schema (e.g., { "C3": { "journeys": { ... } } }) that breaks the downstream replay harness. As evidenced by sdks/python/oss/tests/pytest/integration/agents/_qa_transcripts.py, the load_transcript and transcript_messages methods expect "request" and "reply" keys at the root of the JSON file to successfully reconstruct the runtime conversation.
docs/design/agent-workflows/projects/qa/runs/20260714-214209/results.json#L1-L38: update the producer QA harness or downstream parsing logic so this file structure doesn't cause aKeyErroron replay.docs/design/agent-workflows/projects/qa/runs/20260714-173326/results.json#L1-L107: apply the same schema compatibility fix here.docs/design/agent-workflows/projects/qa/runs/20260714-173439/results.json#L1-L185: apply the same schema compatibility fix here.docs/design/agent-workflows/projects/qa/runs/20260714-212423/results.json#L1-L23: apply the same schema compatibility fix here.
📍 Affects 4 files
docs/design/agent-workflows/projects/qa/runs/20260714-214209/results.json#L1-L38(this comment)docs/design/agent-workflows/projects/qa/runs/20260714-173326/results.json#L1-L107docs/design/agent-workflows/projects/qa/runs/20260714-173439/results.json#L1-L185docs/design/agent-workflows/projects/qa/runs/20260714-212423/results.json#L1-L23
| ], | ||
| "approval": false, | ||
| "errors": [], | ||
| "reply": "### 3 most recent inbox emails\n\n1. **Re: [Agenta-AI/agenta] [integration] big-agents (PR #4791)**\n2. **The Next AI Goldrush: Tokens, Loops, and Neofirms**\n3. **There\u2019s been a login from a new device**\n\n### GitHub repositories\n\n- Agenta-AI/agenta-skills\n- Agenta-AI/agenta\n- mmabrouk/sandbox-agent\n- Agenta-AI/job_extractor_template\n- mmabrouk/daytona\n- Agenta-AI/agenta_cloud *(private)*\n- mmabrouk/r" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact live mailbox and private-repository data before committing this evidence.
The archived reply exposes real email subjects, a security-login notification, and private repository names. Preserve the pass/tool/frame metadata, but replace the reply with sanitized synthetic output.
Comment on lines
+42
to
+49
| REPO = pathlib.Path("/home/mahmoud/code/agenta") | ||
| QA_PRODUCT = REPO / "docs/design/agent-workflows/projects/qa/scripts/qa_product.py" | ||
| CONTAINER = "agenta-oss-team-runner-1" | ||
| COLD_WAIT_S = 75 # keepalive pool TTL is 60_000ms; wait past it | ||
|
|
||
| _spec = importlib.util.spec_from_file_location("qa", QA_PRODUCT) | ||
| qa = importlib.util.module_from_spec(_spec) | ||
| _spec.loader.exec_module(qa) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the packaged QA driver instead of an author-specific path.
This import fails outside /home/mahmoud/code/agenta and still targets the superseded design-doc location rather than the packaged skill driver.
Proposed fix
-REPO = pathlib.Path("/home/mahmoud/code/agenta")
-QA_PRODUCT = REPO / "docs/design/agent-workflows/projects/qa/scripts/qa_product.py"
+REPO = pathlib.Path(__file__).resolve().parents[6]
+QA_PRODUCT = (
+ REPO / ".agents/skills/agent-release-gate/resources/qa_product.py"
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| REPO = pathlib.Path("/home/mahmoud/code/agenta") | |
| QA_PRODUCT = REPO / "docs/design/agent-workflows/projects/qa/scripts/qa_product.py" | |
| CONTAINER = "agenta-oss-team-runner-1" | |
| COLD_WAIT_S = 75 # keepalive pool TTL is 60_000ms; wait past it | |
| _spec = importlib.util.spec_from_file_location("qa", QA_PRODUCT) | |
| qa = importlib.util.module_from_spec(_spec) | |
| _spec.loader.exec_module(qa) | |
| REPO = pathlib.Path(__file__).resolve().parents[6] | |
| QA_PRODUCT = ( | |
| REPO / ".agents/skills/agent-release-gate/resources/qa_product.py" | |
| ) | |
| CONTAINER = "agenta-oss-team-runner-1" | |
| COLD_WAIT_S = 75 # keepalive pool TTL is 60_000ms; wait past it | |
| _spec = importlib.util.spec_from_file_location("qa", QA_PRODUCT) | |
| qa = importlib.util.module_from_spec(_spec) | |
| _spec.loader.exec_module(qa) |
Comment on lines
+238
to
+275
| went_cold = any("miss" in ln and "cold" in ln for ln in session_lines) or any( | ||
| "expire" in ln or "evict" in ln for ln in session_lines | ||
| ) | ||
| mode_lines = [ln for ln in session_lines if "create_session" in ln] | ||
|
|
||
| result.update( | ||
| { | ||
| "turn2": { | ||
| "window": [t2_start, t2_end], | ||
| "reply": t2.reply, | ||
| "summary": t2.summary(), | ||
| "early_recalled": early_recalled, | ||
| "late_recalled": late_recalled, | ||
| }, | ||
| "container_started_after": started_after, | ||
| "container_status_after": status_after, | ||
| "container_restarted_during_run": restarted, | ||
| "runner_log_session_lines": session_lines, | ||
| "runner_log_cold_replay_lines": cold_replay_lines, | ||
| "runner_log_unattributed_cold_replay_lines": unattributed_cold_replay, | ||
| "runner_log_mode_lines": mode_lines, | ||
| "runner_log_full_slice": log_slice, | ||
| "went_cold": went_cold, | ||
| } | ||
| ) | ||
| if restarted: | ||
| result["verdict"] = "DISCARD: container restarted during this run" | ||
| elif not went_cold: | ||
| result["verdict"] = ( | ||
| "INVALID: no cold/miss/expire evidence for this session in turn-2 log window -- " | ||
| "session likely stayed warm (hit-continue); rerun with a longer wait" | ||
| ) | ||
| else: | ||
| result["verdict"] = ( | ||
| f"early_recalled={early_recalled} late_recalled={late_recalled} " | ||
| f"(hypothesis predicts early=True, late=False on claude; " | ||
| f"pi may preserve both if it does a native mode=load)" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the captured replay-mode evidence before issuing a decisive verdict.
mode_lines and cold_replay_lines are recorded but ignored. A generic miss/expire/evict line can therefore produce a valid-looking verdict without proving Claude used mode=create plus transcript replay, or Pi used native mode=load. Gate each cell on its expected mechanism; otherwise mark the run invalid.
Comment on lines
+82
to
+84
| Complements `playwright-testability.md`. Everything below was located by coordinates + screenshots because the accessibility tree gives unnamed nodes: | ||
|
|
||
| - **Unnamed buttons everywhere**: the drawer's Create/Commit/Deploy, section rows (Model & harness, Advanced...), approval Approve/Deny, chat "New session" are `button` with no accessible name (`read_page` shows `button [ref_x]` with empty labels). Suggest `data-testid` per playwright-testability conventions: `agent-draft-create`, `playground-commit`, `approval-dock-approve`, `approval-dock-deny`, `config-section-advanced`, `chat-new-session`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Specify accessible names, not only test IDs.
Unnamed Create, Commit, Approve, Deny, and session buttons are accessibility blockers. Require visible labels or aria-label/aria-labelledby; keep data-testid only as supplemental test instrumentation.
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The pre-release QA harness for the agent runtime lived inside a design-docs folder (
docs/design/agent-workflows/projects/qa/), wired to one machine: it read credentials from a hardcoded~/.agenta-bighetzner.envand wrote run output back into the tracked tree. It could not be run anywhere else without editing the scripts, and its home advertised it as scratch, not a tool. The methodology it encodes is worth keeping: it drives the same product endpoint the playground drives and asserts on the SSE frame stream and real side effects, never on model prose, so it is genuinely deployment-agnostic once the machine-specific bits are removed.This graduates that harness into a tracked, portable
agent-release-gateskill built on progressive disclosure, and leaves the design folder as the release-night findings archive.Changes
A skill, not a scratch folder.
.agents/skills/agent-release-gate/with a smallSKILL.md(what the gate is, when to run it, the minimal procedure) and aresources/directory holding the three drivers, the traps doc, a coverage matrix, and regression seeds. A tracked.claude/skills/agent-release-gatesymlink and two.gitignoreallowlist entries make it discoverable by both Claude and Codex, matching the existingwrite-template-playbookspattern.Run-anywhere. The drivers now read
AGENTA_BASE,AGENTA_PROJECT_ID, andAGENTA_API_KEYfrom the environment first, falling back to--env-file(default~/.agenta-bighetzner.envfor backward compatibility). Missing credentials produce a clear startup error naming exactly what is unset. Run output defaults to./qa-gate-runs/in the current working directory (override withAGENTA_QA_RUNS_DIR), so runs never accumulate inside the skill or the repo.Before (hardcoded, module import time):
After (env-first, resolved in
main()):Deletions from the design folder.
run_matrix.pyandwarm_daytona_probe.py(superseded by the product-path drivers),mcp_qa_server.mjs(broken, and carried an invisible U+200E in its filename),regression-skill-DRAFT.md(graduated), andcleanup-plan.md(stale).coldctx_experiment.pystays as a host-only experiment (one docstring line added). The folder's README gains a one-line pointer to the skill, anddocs/designs/testing/README.mdgains a short subsection linking the gate.The branch's earlier commits carry the release-night findings and evidence history (STATUS.md, findings.md, matrix.md, the recorded runs); this graduation is the final commit on top.
Note:
run_matrix.pyhad an unrelated uncommitted modification in the working tree from another session. Deleting it would have discarded that change, so the file and its modification were left untouched and it was not deleted in this PR.Tests / notes
Verified the skill the way a stranger would use it.
Missing-credentials failure (env unset, no env file):
--helpworks with no credentials present for all three drivers (qa_longctx.pyno longer hits the network at import).Live run (real credentials exported as env vars, from a temp dir outside the repo):
Results landed in
./qa-gate-runs/in the temp dir, not in the skill.ruff formatandruff checkpass on all three drivers.https://claude.ai/code/session_019TK4ow9a3dXUNY657djbQr