Skip to content

[test] Gate blocks when a model switch loses the warm sandbox - #6371

Merged
mmabrouk merged 3 commits into
release/v0.114.4from
gate/warm-model-switch
Aug 31, 2026
Merged

[test] Gate blocks when a model switch loses the warm sandbox#6371
mmabrouk merged 3 commits into
release/v0.114.4from
gate/warm-model-switch

Conversation

@mmabrouk

Copy link
Copy Markdown
Member

Context

This week's lifecycle bug (#6364) was visible to the release gate and the gate stayed green. L1's model case actually observed the wrong route, but it was evidence-only (not blocking) and it ran only on the claude harness, while the bug hit only pi_core: the router keyed its capability table on the bare pi literal, the wire carries pi_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:

  • The model case becomes blocking. An alias-to-alias switch on the same self_managed anthropic connection moves only the model facet, so a rebuild there is a broken live route, not a deployment quirk.
  • A second blocking variant, model_pi_core, runs the whole session on pi_core with a fully qualified same-provider id switch (claude-haiku-4-5 to claude-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 model moved, 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

…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
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Aug 29, 2026
@vercel

vercel Bot commented Aug 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Ready Ready Preview Aug 31, 2026 4:13pm

Request Review

@dosubot dosubot Bot added the tests label Aug 29, 2026
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Tests

    • Strengthened release-gate checks for model switching and route warm-up behavior.
    • Added coverage for same-connection model switching and pi_core model routing.
    • Added validation for fully qualified model identifiers.
    • These scenarios now block release when expected warm routes are not maintained.
  • Documentation

    • Updated release-gate guidance with expanded blocking checks.
    • Documented connection configuration requirements and clarified how credential preflight affects warm-route verification.

Walkthrough

The 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.

Changes

Lifecycle route coverage

Layer / File(s) Summary
Blocking model cases
.agents/skills/agent-release-gate/resources/matrix_l1_lifecycle_routes.py
The existing model case is now blocking. A blocking model_pi_core case validates the model-only switch with one warm sandbox.
pi_core harness wiring
.agents/skills/agent-release-gate/resources/matrix_l1_lifecycle_routes.py, .agents/skills/agent-release-gate/SKILL.md, .agents/skills/agent-release-gate/resources/LESSONS.md
The new case uses pi_core harness and model constants. Its mutation, connection parameters, documentation, and lifecycle guidance cover the pi_core wire spelling and Daytona-era fixture behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to a0ee7

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: the release gate now blocks when a model switch loses the warm sandbox.
Description check ✅ Passed The description directly explains the lifecycle regression, the blocking model checks, the pi_core variant, fixture changes, and planned verification.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

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.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gate/warm-model-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mmabrouk

Copy link
Copy Markdown
Member Author

Please change the pi_core fixture to use vault-backed authentication before merging.

pi_base_params copies the default Claude configuration and changes only the harness and model. This leaves llm.connection as {"mode": "self_managed", "slug": null}. A local self-managed Pi run requires PI_CODING_AGENT_DIR to be mounted. On a release-gate deployment that has the documented provider key but no Pi subscription mount, turn 1 will error and this new blocking case will fail before it tests lifecycle routing.

Please set the Pi connection explicitly:

pi_base_params["agent"]["llm"]["connection"] = {
    "mode": "agenta",
    "slug": None,
}

This matches the existing Pi fixtures in matrix_l5_live_route_observed.py and bench_lib.py. After the change, run L1 against a deployment containing #6364 and confirm model_pi_core records one sandbox ID.

@mmabrouk
mmabrouk changed the base branch from main to release/v0.114.4 August 31, 2026 16:03
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
@mmabrouk

Copy link
Copy Markdown
Member Author

Done in 9bf7d23. The pi_core fixture now sets llm.connection = {"mode": "agenta", "slug": null}, with a comment that records why: a self_managed Pi run needs the PI_CODING_AGENT_DIR mount, which a gate deployment does not have. The L1 run against a deployment that contains #6364 is on the QA list for the v0.114.4 gate.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 9bf7d23 and a0ee7dc.

📒 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

Suggested change
#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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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.

Suggested change
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

@mmabrouk
mmabrouk merged commit 8620964 into release/v0.114.4 Aug 31, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant