Repository navigation
test(agent): give the turn-overrides fixture a hosted root authority - #6628
Merged
senamakel merged 1 commit intoSep 25, 2026
Merged
Conversation
Every session turn is a hosted root invocation: it resolves its agent id
against the host catalogue before composing a message and refuses the turn
when the id is not there. This fixture only ever set
`agent_definition_name("turn-overrides/orchestrator")`, which stamps an id
without putting a definition behind it — and no built-in uses that id, by
design. Post-cutover every turn in the file was therefore unresolvable, and
all four tests were quarantined under one marker.
Bring the definition, via `SessionHostBuilder::agent_definition`, so the
fixture is its own catalogue. Scope is `Wildcard` rather than a named list:
these tests assert what `suppress_tools` does to the session's own belt, so
the authority must not narrow it, and `Named` silently drops any name missing
from the parent registry — a renamed tool would become an empty toolbelt and
a confusing failure two asserts later.
Two of the four now pass and are un-quarantined:
suppress_active_goal_keeps_the_thread_goal_out_of_the_prompt
thread_goal_complete_and_clear_stop_the_goal_reaching_later_turns
The other two still fail, so they stay quarantined — but their markers no
longer blame the authority, which is no longer what blocks them:
suppress_transcript_autoload_... fails in its own CONTROL: a fresh agent
does not auto-load the prior transcript by agent name on the hosted path,
so the test cannot prove the override prevents anything. Not isolated:
either `auto_save` no longer writes where `latest_for_agent` looks, or the
lookup key moved with the stamped session id.
turn_overrides_apply_to_exactly_one_turn_and_then_reset: turn 2 sets no
overrides and must carry the belt, but the upstream request's tool schema
is empty. Ruled out: `ToolScope` — `Named` and `Wildcard` behave the same.
Turn 1's assertion cannot tell a suppressed belt from one that never
arrives, so the open question is whether `suppress_tools` leaks past its
one turn or the belt never reaches the model here at all.
Verified as CI runs it (`cargo test -p openhuman-cli --test
agent_turn_overrides_e2e`): 2 passed, 0 failed, 2 ignored.
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe E2E fixture now registers and supplies a hosted orchestrator definition. The active-goal suppression and terminal-goal API tests are enabled. The transcript-autoload and one-turn-reset tests remain ignored with updated TODO notes. ChangesHosted agent turn override tests
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Merge Risk: ⚪ Minimal · up to The hosted fixture enables two goal tests while leaving the two tests with known blockers ignored. No merge-blocking issue is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
A rabbit checks the hosted gate, Comment ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. How this fits togetherflowchart LR
n0["agent_with<br/>changed"]:::changed
n1["...s_the_thread_goal_out_of_the_prompt_inner<br/>changed"]:::changed
n2["..._to_exactly_one_turn_and_then_reset_inner<br/>changed"]:::changed
n3["vec"]:::impacted
n4["..._stop_the_goal_reaching_later_turns_inner"]:::impacted
n5["...t_replay_a_prior_threads_transcript_inner"]:::impacted
n6["...vious_handle_and_joins_successfully_inner"]:::impacted
n7["context_config"]:::impacted
n8["event_context"]:::impacted
n0 -->|calls| n7
n0 -->|tests| n7
n0 -->|calls| n8
n0 -->|tests| n8
n1 -->|calls| n0
n1 -->|calls| n3
n1 -->|tests| n3
n2 -->|calls| n0
n2 -->|calls| n3
n2 -->|tests| n3
n4 -->|calls| n0
n4 -->|calls| n3
n4 -->|tests| n3
n5 -->|calls| n0
n5 -->|calls| n3
n5 -->|tests| n3
n6 -->|calls| n3
n6 -->|tests| n3
n6 -->|calls| n7
n6 -->|tests| n7
n6 -->|calls| n8
n6 -->|tests| n8
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0020 · 72,012 in / 6,908 out · 8,422 cached (12%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 549 embedded
critique: $0.0008 · 31,334 in / 1,091 out · 2,029 cached (6%) · gpt-5.6-luna
security: $0.0008 · 30,846 in / 530 out · 1,785 cached (6%) · gpt-5.6-luna
description: $0.0001 · 6,291 in / 1,353 out · 1,536 cached (24%) · deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p3
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
2 participants
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.
Summary
tests/agent_turn_overrides_e2e.rsa hosted root authority of its own, viaSessionHostBuilder::agent_definition, so its sessions can resolve the agent id they are stamped with.Problem
Every session turn is a hosted root invocation: it resolves its agent id against the host catalogue before composing a message and refuses the turn when the id is not there.
This fixture only ever set
agent_definition_name("turn-overrides/orchestrator"). That stamps an id but does not put a definition behind it, and no built-in definition uses that id — deliberately, so a hit can only come from the fixture's own definition. After the hosted-TinyAgents cutover every turn in the file was unresolvable, and all four tests were quarantined under one marker: "fixture must use the hosted root authority".The marker was right about the cause for half the file and, it turns out, incomplete for the other half.
Solution
agent_withnow builds with.agent_definition(turn_overrides_definition()), a definition filed under the same id the session is stamped with (setters.rs:336rejects a contradicting name, so the two must agree).The tool scope is
Wildcard, not a named list. The resolved definition's tools are the turn's allow-list, intersected with the belt the session was built with, and these tests assert whatsuppress_toolsdoes to that belt — so the authority must not narrow it.Namedwould also work for the single tool in use, but it silently drops names missing from the parent registry, which would turn a future rename into an empty toolbelt and a failure two asserts away from its cause.Un-quarantined (verified passing):
suppress_active_goal_keeps_the_thread_goal_out_of_the_promptthread_goal_complete_and_clear_stop_the_goal_reaching_later_turnsStill quarantined, markers re-aimed:
suppress_transcript_autoload_does_not_replay_a_prior_threads_transcript— fails in its own control, not its assertion: a fresh agent does not auto-load the prior transcript by agent name on the hosted path, so the test cannot prove the override prevents anything. Not isolated — eitherauto_saveno longer writes wherelatest_for_agentlooks, or the lookup key moved with the stamped session id. Deliberately not lifted by relaxing the control; the control is what makes the test non-vacuous.turn_overrides_apply_to_exactly_one_turn_and_then_reset— turn 2 sets no overrides and must therefore carry the session's belt, but the upstream request's tool schema comes back empty.ToolScopeis ruled out:Named(["turn_overrides_echo"])andWildcardproduce the same empty schema. Turn 1's assertion cannot distinguish a suppressed belt from one that never arrives, so the open question is whethersuppress_toolsleaks past its one turn or the belt never reaches the model on this path at all.Verified the way CI runs it —
cargo test -p openhuman-cli --test agent_turn_overrides_e2e— 2 passed, 0 failed, 2 ignored.Submission Checklist
N/A: the diff is entirely test code and test-support code intests/, which the coverage gate does not measure as changed product lines.N/A: behaviour-only change— no feature row is added, removed or renamed; two existing tests move from disabled to running.## RelatedScriptedModeland a localEchoTool; nothing leaves the process.N/A: no product surface changes.Closes #NNNin the## Relatedsection —N/A: deliberately NOT closing. Two of the four quarantined tests still fail, so this usesRefs— see below.Impact
None at runtime. Test-only: no product code, no dependencies, no migrations. Two tests that were skipped now execute in the
agent_turn_overrides_e2etarget.Related
#[ignore]markers and in## Solutionabove. They may warrant splitting into their own issues, since neither is about the fixture's authority any more.docs/TEST-COVERAGE-MATRIX.mdcoversagent_turn_overrides_e2e.Summary by CodeRabbit