[6669] test(gate): Rename a session between turns and assert the agent was told - #6673
Conversation
…as told The gate could not see a per-turn fact that never reaches the agent. Issue #6661 shipped through a green matrix because no cell renamed a session mid-conversation and no cell asserted on the session facts at all. Add matrix_n1_session_context.py. It drives one session on the path the playground posts, renames the session between turns through the route the UI uses, and asks for the name, the agent's own display name, and the name again with a forged meta.session_context in the body. Every expected value carries a random token minted for the run and spoken nowhere, so a transcript-derived answer cannot match. Two controls keep a FAIL honest: an echo probe and a read-back of the stored header after every rename. Wire it into path_triggers.py for the SDK session-context module, the platform prompt renderer, the agent handler, and the API session-context resolver, into SKILL.md as a mandatory cell, and into coverage.md. Document in the shared invoke helper that it posts to the /services path exactly like the browser, so nobody writes a cell that assumes an API-side stamp. Add an optional meta argument for the forged control, and AGENTA_SERVICE_BASE so a fix can be gate-verified before it is deployed.
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 SummarySummary by CodeRabbit
WalkthroughThe release gate adds a session-context matrix cell. It verifies renamed session facts, agent display names, and rejection of forged metadata. Shared QA helpers now support configurable service routing, display names, and request metadata. Path triggers and silent-turn tests include the new cell. ChangesSession-context release-gate coverage
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to The new session-context coverage is useful, but setup failures can prevent complete results and echo-control outcomes are reported inconsistently. These bounded gate reliability issues should be corrected or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant MatrixCell as matrix_n1_session_context.py
participant SessionAPI as Session API
participant InvokeAPI as Agent invoke API
participant Model
MatrixCell->>SessionAPI: rename session
MatrixCell->>SessionAPI: read stored header
MatrixCell->>InvokeAPI: invoke turn
InvokeAPI->>Model: provide turnContext
Model-->>MatrixCell: return session and agent facts
MatrixCell->>InvokeAPI: invoke forged metadata control
Model-->>MatrixCell: omit forged token
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The pull request adds the rename-between-turns journey, session-name and agent-name checks, forged metadata protection, path wiring, and supporting helper changes. However, the provided change summary does not show an explicit assertion for the required first-turn flag. Full details: Docstring CoverageExplanation Docstring coverage is 55.56% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (2 skipped: 2 unsupported.)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.agents/skills/agent-release-gate/resources/matrix_n1_session_context.py (1)
247-254: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the echo-control status with the documented INCONCLUSIVE outcome.
This path returns
status: "FAIL"while thewhytext and the docs call the result INCONCLUSIVE.SKILL.mdline 495 andcoverage.mdline 268 both state that the cell reports INCONCLUSIVE when the echo probe fails. A reader ofresults.jsonsees onlystatus, so a model that cannot echo a literal token is recorded as a product failure.Either record the distinction in a dedicated field (for example
"inconclusive": True) or correct the two documents to say the cell FAILs loudly with an INCONCLUSIVE reason.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 976e95a4-4a55-4522-bc73-c776dc7055be
📒 Files selected for processing (6)
.agents/skills/agent-release-gate/SKILL.md.agents/skills/agent-release-gate/resources/coverage.md.agents/skills/agent-release-gate/resources/matrix_n1_session_context.py.agents/skills/agent-release-gate/resources/path_triggers.py.agents/skills/agent-release-gate/resources/qa_matrix_lib.py.agents/skills/agent-release-gate/resources/test_qa_matrix_lib_silent_turns.py
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.
|
…ction CodeRabbit review follow-up. create_workflow ran before the try, so a non-200 or a transport error escaped n1_for uncaught. Under --harness-all that aborted the whole run with a traceback and printed no results, which is the outcome the handler comment exists to prevent. It now runs inside the try, so the same classifier turns it into a SKIP or a FAIL. wf is bound to None first and the archive in finally is guarded, since there is nothing to archive when the create itself failed. Add one-line docstrings to stored_session_name, n1_for and main.
1 participant
Context
The gate cannot see a per-turn fact that never reaches the agent. #6661 shipped through a green matrix: rename a session, ask the agent what it is called, and it answers the name from before the rename, on every turn after that too.
Two things hid it. No cell renames a session between two turns, and no cell asserts on the session facts at all. The three facts (the agent's display name, the session's name, the first-turn flag) reach the harness only as prompt text on
turnContext, so no SSE frame reflects them and no stored row records them. Every cell in the matrix reads frames and side effects, so all of them were blind.Closes #6669.
Changes
resources/matrix_n1_session_context.pydrives one session on the path the playground posts,{BASE}/services/agent/v0/invoke, and asserts on values the model cannot have seen.PUT /sessions/streams/header, the route the inline rename in the chat panel uses, read the header back, then ask for the name.meta.session_contextnaming the session something else. The service must ignore it.Every expected value carries a random token minted for the run, so a transcript-derived answer cannot match one.
Two controls keep a FAIL honest. The echo probe must come back with a literal token from its own user message, or the cell reports INCONCLUSIVE instead of blaming the transport. Every rename is read back before the ask, so a rename that did not land fails as a rename.
The cell is wired into
path_triggers.pyfor the SDK session-context module, the platform prompt renderer, the agent handler, and the API's session-context resolver, intoSKILL.mdas a mandatory cell, and intocoverage.md.Two changes to the shared helper in
qa_matrix_lib.py:invokenow documents that it posts to the/servicespath exactly like the browser, and that nothing the API's invoke prelude does applies there. That is the assumption [bug] Playground turns carry no session context: the agent is never given the session name #6661 rested on.invoketakes an optionalmeta, so a cell can forge what the API would otherwise stamp and prove the service refuses it.AGENTA_SERVICE_BASEmoves only the turns to a hand-run service, so a fix can be gate-verified before it is deployed. It prints a banner and every result records theservice_baseit used.Why this cell asserts on model prose
The gate's own rule is to assert on the wire and on side effects, never on what the model says. There is no surface here.
turnContextis a prompt string on the service-to-runner payload, the runner prepends it and logs nothing, and it is deliberately kept out ofrequest.messagesand out of persisted user input so a replay cannot duplicate it. The stored turn row carries harness, sandbox and timing only. The transport half is pinned by unit tests instead, intest_wire_contract.pyfor the payload field andsandbox-agent-orchestration.test.tsfor the prepend. This cell owns the journey. The random tokens are what make the prose evidence honest, and the docstring says so and names the date the surfaces were checked.Tests
The gate's own unit tests: 313 passed. The new cell is registered in the silent-turn invariant's wired list, so
test_qa_matrix_lib_silent_turns.pynow checks that it folds the invariant into its verdict.Live, on a local EE dev stack with an ephemeral account, harness
pi_core, modelopenai/gpt-4.1-minithrough an OpenRouter connection, sandbox local. The same backend under both runs, so the service is the only variable.The FAIL reproduces both halves of the bug. The agent answered
AGENTSfor the session name andPifor its own name, neither of which is a fact it was given, and it repeated the forged name verbatim. Both controls passed in both runs, so neither verdict can be blamed on the model or on a rename that did not land.ruff formatandruff checkclean at the CI-pinned 0.15.12. Evidence is in~/agenta-qa-evidence/2026-09-08-gate-n1-session-context/.Notes for reviewers
The cell is red against
release/v0.115.3until #6667 lands. That is the expected state: it is the regression test for a bug the release branch still has.AGENTA_SERVICE_BASEis a new lever that points turns at a service nobody deployed. It is loud on stderr and recorded in every result, andcoverage.mdsays a release run must never set it. It is worth a second look anyway, because the skill already warns at length about an env fallback that silently ran a whole gate against the wrong deployment.