[test] Gate blocks when a model switch loses the warm sandbox - #6371
Conversation
…core L1's model case observed this week's bug and could not stop it: the lifecycle router keyed its capability table on the bare 'pi' literal while the wire carries 'pi_core', so every playground Pi model switch silently rebuilt the warm sandbox (#6364) — and the case was (a) evidence-only and (b) claude-only, so the gate stayed green. Both closed: the model case is now BLOCKING (an alias switch on the same self_managed anthropic connection moves only the model facet), and a second blocking variant runs the whole session on pi_core with a fully qualified same-provider id switch. If the pi_core case goes red because a facet OTHER than model moved, that red is the discovery mechanism working — it names the next over-eviction to remove. Claude-Session: https://claude.ai/code/session_014s6jqKCsVKsNMmnrJVJkZt
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe release-gate QA matrix now blocks on same-connection model switches for claude and pi_core. It adds a fully qualified pi_core model case, configures the pi_core harness, and documents related lifecycle and wire-spelling traps. ChangesLifecycle route coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR’s lifecycle gate changes introduce no actionable merge-blocking risk; only minor documentation cleanup remains in LESSONS.md to keep examples valid and clear. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 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 |
|
Please change the
Please set the Pi connection explicitly: pi_base_params["agent"]["llm"]["connection"] = {
"mode": "agenta",
"slug": None,
}This matches the existing Pi fixtures in |
Review catch on #6371: the fixture copied the Claude defaults, which leave llm.connection as self_managed. A self_managed Pi run needs a PI_CODING_AGENT_DIR mount, which a gate deployment does not have, so turn 1 would error before the case tests any lifecycle routing. The Pi fixtures in matrix_l5_live_route_observed.py and bench_lib.py already pin mode=agenta. Claude-Session: https://claude.ai/code/session_0165tsjmvf3qvTPFcb9EV44g
|
Done in 9bf7d23. The pi_core fixture now sets |
Vault-backed fixture connections (the #6371 review catch), and the stuck-substitution rebuild that can fake an eviction in a sandbox-counting warm case after #6370. Claude-Session: https://claude.ai/code/session_0165tsjmvf3qvTPFcb9EV44g
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d58ea7e6-c96e-4cc5-9607-36559e96edab
📒 Files selected for processing (1)
.agents/skills/agent-release-gate/resources/LESSONS.md
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.
| ## Two Daytona-era traps for the lifecycle (L*) cases — 2026-08-31 | ||
|
|
||
| **Fixture connections must be vault-backed.** A fixture built by copying the Claude defaults and | ||
| changing only the harness keeps `llm.connection = {"mode":"self_managed","slug":null}`. A |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use Python None in the fixture examples.
These assignments are presented as Python, but null is JSON syntax. Copying either example into .agents/skills/agent-release-gate/resources/matrix_l1_lifecycle_routes.py raises NameError. The corresponding pi_base_params["agent"]["llm"]["connection"] assignment uses None. Replace both null values with None, or label the examples as JSON.
Proposed fix
-`llm.connection = {"mode":"self_managed","slug":null}`
+`llm.connection = {"mode":"self_managed","slug":None}`
-`llm.connection = {"mode":"agenta","slug":null}`
+`llm.connection = {"mode":"agenta","slug":None}`Also applies to: 237-237
| self_managed Pi run needs the `PI_CODING_AGENT_DIR` mount, which a gate deployment does not have, | ||
| so turn 1 errors and the case fails before it tests anything. Set | ||
| `llm.connection = {"mode":"agenta","slug":null}` on every non-default-harness fixture (caught on | ||
| #6371; the Pi fixtures in `matrix_l5_live_route_observed.py` and `bench_lib.py` already do this). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid starting the wrapped line with #6371.
The line starts with #6371, so markdownlint reports MD018. Prefix the reference with issue or keep it on the preceding line.
Proposed fix
-#6371; the Pi fixtures in `matrix_l5_live_route_observed.py` and `bench_lib.py` already do this).
+issue `#6371`; the Pi fixtures in `matrix_l5_live_route_observed.py` and `bench_lib.py` already do this).📝 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.
| #6371; the Pi fixtures in `matrix_l5_live_route_observed.py` and `bench_lib.py` already do this). | |
| issue #6371; the Pi fixtures in `matrix_l5_live_route_observed.py` and `bench_lib.py` already do this). |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 238-238: No space after hash on atx style heading
(MD018, no-missing-space-atx)
Source: Linters/SAST tools
|
|
||
| **A stuck-substitution rebuild is not an eviction.** Since the credential preflight (#6370), a | ||
| fresh Daytona sandbox whose Secret wiring failed (a vendor-side per-sandbox fault, a few percent | ||
| of creates) is convicted at ~10s and rebuilt ONCE. A warm-reuse case that counts sandbox ids can |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace convicted with the runner state.
A sandbox is not “convicted”. Use marked STUCK or detected as stuck to match the [credential-preflight] STUCK log entry and make the troubleshooting step clear.
Proposed fix
-~10s and rebuilt ONCE.
+~10s, marked STUCK, and rebuilt ONCE.📝 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.
| of creates) is convicted at ~10s and rebuilt ONCE. A warm-reuse case that counts sandbox ids can | |
| of creates) is marked STUCK at ~10s, and rebuilt ONCE. A warm-reuse case that counts sandbox ids can |
Context
This week's lifecycle bug (#6364) was visible to the release gate and the gate stayed green. L1's
modelcase actually observed the wrong route, but it was evidence-only (not blocking) and it ran only on the claude harness, while the bug hit onlypi_core: the router keyed its capability table on the barepiliteral, the wire carriespi_core, and every playground model switch silently rebuilt the warm sandbox.Per our standing rule, a missed bug births a standing check.
Changes
Two changes to
matrix_l1_lifecycle_routes.py:modelcase becomes blocking. An alias-to-alias switch on the same self_managed anthropic connection moves only themodelfacet, so a rebuild there is a broken live route, not a deployment quirk.model_pi_core, runs the whole session onpi_corewith a fully qualified same-provider id switch (claude-haiku-4-5toclaude-sonnet-5). This is the exact shape the bug class hides in: per-harness routing keyed off wire spellings.If the pi_core case ever goes red because a facet other than
modelmoved, the shadow log line names the facet, and that red is the discovery mechanism working: it names the next over-eviction to remove. The docstring says so, so nobody demotes the case to make the gate green.Tests
This IS the test; it runs against a live deployment as part of the gate matrix (
uv run matrix_l1_lifecycle_routes.py). It has not been run against a deployment carrying #6364 yet; the first gate run after that lands is the verification, and the case is expected red against any deployment without #6364.https://claude.ai/code/session_014s6jqKCsVKsNMmnrJVJkZt