fix(harness): let hosts exclude replayed history from user-input screening - #223
Conversation
…ening `prepare_agent_turn` screened every user message in the request as `ContentOrigin::User` on every turn. A host whose text tool dialect replays tool results as user rows re-screens those results under the user policy each turn. The results were already screened one by one as `Tool` output when they ran, but coalesced into one row they can cross the threshold. That turns into a permanent block: the row lives in the durable transcript, so every later turn fails before the first model call (openhuman#6710). Add `AgentTurnRequest::with_replayed_prefix(n)`. The first `n` messages are history the host already screened when it admitted them, so only the messages after them are screened as this turn's input and used as the recall query. The default of 0 screens every user message, exactly as before, so hosts that do not opt in lose no screening.
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lanes and found 3 active actionable findings. The change adds `AgentInvocation::with_replayed_prefix` to let hosts exclude replayed history from user-input screening. Two earlier findings about API stability and authorization bypass remain unresolved. State: Changes requested Review snapshot
Completeness: Complete What changedThe PR moves the `replayed_prefix` field from `AgentTurnRequest` to `AgentInvocation`, adds a builder method `with_replayed_prefix`, and modifies the `prepare_agent_turn_bounded` and `screen_user_messages` functions to skip screening of the replayed prefix. Validation rejects a prefix larger than the message count and logs a warning when the prefix covers all messages ending with a user message. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Resolved this pass
Before merge
How this fits togetherflowchart LR
n0["start_progress_dispatcher<br/>changed<br/>3 findings"]:::blocking
n1["prepared_tools_are_dynamic_and_never_reused<br/>changed"]:::changed
n2["new"]:::impacted
n3["new"]:::impacted
n4["...corded_tools_into_a_compaction_generation"]:::impacted
n5["...xact_tool_turn_neither_merges_nor_records"]:::impacted
n6["...restores_tools_from_its_write_destination"]:::impacted
n7["clone"]:::impacted
n0 -->|calls| n3
n0 -->|calls| n7
n1 -->|calls| n2
n1 -->|tests| n2
n4 -->|calls| n2
n4 -->|tests| n2
n5 -->|calls| n2
n5 -->|tests| n2
n6 -->|calls| n2
n6 -->|tests| n2
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
|
|
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 (3)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesReplayed-prefix screening
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No new merge-blocking issue is established by the supplied evidence. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Opt-in replay can prevent legitimate conversations from being blocked by repeated screening, but its safety depends on hosts replaying only content that was previously admitted in its screened form. The runtime does not verify that condition. No production-host exploit is established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
A rabbit checks the history line, Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0241 · 186,990 in / 15,441 out · 6,098 cached (3%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 466 embedded
critique: $0.0146 · 110,729 in / 4,411 out · 4,228 cached (4%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0056 · 41,777 in / 1,177 out · 1,870 cached (4%) · gpt-5.6-luna
tests: $0.0015 · 17,275 in / 79 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0012 · 8,927 in / 2,421 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p1
|
Review at 1. Can the slice ever skip the turn's NEW input? Not with the default.
2. Recall query from the same slice: yes. 3. Other callers of 4. Struct-literal break note: accurate. There are no On For the openhuman consumer (not this PR), to verify there: skipping the replay screen is only safe if the persisted user row is the redacted form. The harness redacts user rows in place in |
…on contract - A `replayed_prefix` greater than the message count is always a host bug. It is now a validation error instead of being clamped, which would have silently skipped screening of the whole request. - A prefix equal to the count screens nothing. That is legitimate for a deferred resume, so it is accepted, with a warning when the last message is a user message. - Document the opt-in contract: replayed user rows must be the form the gate returned (redacted), or a secret redacted on its first turn reaches the model raw later. A test replays a run's own transcript and asserts the redacted form is what the model sees.
|
Re-checked at |
…river history Hosts that set `with_replayed_prefix(len - 1)` screen only the last message (openhuman#6710). That is sound only while the runtime appends exactly the turn's single input after the committed history; a second appended row would be replayed unscreened. Fault-checked by making the runtime push an extra row.
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0475 · 381,567 in / 20,368 out · 52,915 cached (14%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash, deepseek-v4-flash · 756 embedded
critique: $0.0260 · 181,227 in / 9,464 out · 6,857 cached (4%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0192 · 140,807 in / 4,383 out · 7,146 cached (5%) · gpt-5.6-luna
tests: $0.0013 · 39,751 in / 2,715 out · 38,912 cached (98%) · deepseek/deepseek-v4-flash
description: $0.0003 · 11,959 in / 3,249 out · 0 cached (0%) · deepseek-v4-flash
|
Re-checked |
`AgentTurnRequest` is a public struct with public fields, and adding `replayed_prefix` to it broke downstream struct literals. The boundary now lives on `AgentInvocation` as a private field with a `with_replayed_prefix` builder. `AgentInvocation` already has a private field, so no downstream code can build it with a literal, and this addition is source-compatible. `AgentTurnRequest` is unchanged from main. A recursive child's invocation (`from_shared_host`) always screens everything.
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0290 · 216,620 in / 24,423 out · 24,878 cached (11%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 811 embedded
critique: $0.0128 · 93,183 in / 4,628 out · 4,142 cached (4%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0092 · 62,012 in / 3,317 out · 0 cached (0%) · gpt-5.6-luna
tests: $0.0036 · 37,098 in / 9,020 out · 19,456 cached (52%) · deepseek/deepseek-v4-flash
description: $0.0015 · 9,970 in / 3,957 out · 1,280 cached (13%) · deepseek/deepseek-v4-flash
…humansai#6723) Brings in tinyhumansai/tinyagents#223 (replay boundary, merged as fefbd11b; tree identical to the previously pinned b8b08187) and tinyhumansai/tinyagents#224 (text-dialect undecodable-block nudge, tinyhumansai#6723). No nested submodule moves.
2 participants
Summary
prepare_agent_turnscreens every user message in anAgentTurnRequestasContentOrigin::User, on every turn. A host whose text tool dialect replays tool results as user rows therefore re-screens those results under the user-input policy on each turn.The results were already screened one at a time as
Tooloutput when they ran (agent_loop/tools.rs). Coalesced into a single row, though, they can cross the threshold. On a real OpenHuman thread, four GitHub-issue results scored 0.18, 0.48, 0.42 and 0.18 on their own; combined into one row they scored 0.72, which blocks. Because that row lives in the durable transcript, every later turn fails before its first model call and the thread never recovers. Details are in tinyhumansai/openhuman#6710.Refs tinyhumansai/openhuman#6710
This PR adds
AgentInvocation::with_replayed_prefix(n). The host declares that the firstnmessages are replayed history it already screened when it admitted them. Only the messages after them are screened as this turn's user input, and only those feed the memory/experience recall query.Why a boundary set by the host and not an inferred rule such as "last message only": another embedder may legitimately send several new user messages in one request, and an inferred rule would quietly stop screening them.
API Or Behavior Changes
AgentInvocation::with_replayed_prefix(count). It sets a private field;AgentTurnRequestis unchanged frommain.AgentInvocationalready has a private field (runtime), so no downstream code can build it with a struct literal. Adding another private field breaks nothing. (An earlier revision put a public field onAgentTurnRequest, which would have broken struct literals.)0= unchanged behaviour: every user message is screened. A recursive child invocation (from_shared_host) always uses 0.Validationerror; it is always a host bug. A prefix equal to the count screens nothing, which is legitimate for a deferred resume. It is accepted, with awarn!when the last message is aUsermessage.input_text) also feeds the memory/experience recall query andTurnSummary::with_text/Experience::newinfinish_host_turn. For a host that opts in, those inputs narrow to this turn's new messages.with_replayed_prefix): opt in only if your replayed history stores user rows exactly as the gate returned them (redacted), e.g. themessagesof theAgentRunthat admitted them.Security
ContentOrigin::User.Toolwhen the tool runs. That screen is untouched.Tests
New tests in
runtime/test.rs:hosted_turn_screens_only_the_new_user_input_not_replayed_history: blocked content sits in replayed[Tool results]-style rows, including an interrupted-turn shape where a results row is directly followed by the new input. The turn must succeed.hosted_turn_rejects_a_replayed_prefix_past_the_messages: with the old clamp restored, it fails at itsexpect_err.hosted_turn_accepts_a_replayed_prefix_covering_every_message: the deferred-resume shape is not rejected.hosted_turn_replays_the_redacted_form_of_an_admitted_user_row: replays the first run's transcript. If the host instead replays the raw input, it fails at!second.contains("secret").tinyagents-runtime:driver_history_is_the_committed_history_plus_exactly_the_turn_input. A host that setswith_replayed_prefix(len - 1)relies on each turn appending exactly one input after the committed history. This test pins that behaviour. Fault check: when the runtime pushes an extra row, the test fails.hosted_turn_still_blocks_new_user_input_and_screens_all_by_default: three cases.Revert-check: with only the slice in
prepare_agent_turnreverted, the first test fails at its.expect("replayed history must not be re-screened as new user input")and the second still passes.Commands run locally:
cargo fmt --all --checkcargo clippy -p tinyagents-harness --tests -- -D warningscargo test -p tinyagents-harness --lib -- runtime::test::hosted_(16 passed), pluscargo test -p tinyagents-runtime --lib -- test::driver_history_is test::trailing_input(2 passed) andcargo clippy -p tinyagents-runtime --tests -- -D warnings, at b8b0818cargo clippy --all-targets -- -D warnings: not run, left to CIcargo clippy --all-targets --all-features -- -D warnings: not run, left to CIcargo build --all-targets: not run, left to CIcargo build --all-targets --all-features: not run, left to CIcargo test: not run in full, left to CIcargo test --all-features: not run, left to CIDocumentation
Rustdoc on the new field and builder, plus an updated rustdoc for
screen_user_messages. No other docs describe screening scope.Summary by CodeRabbit