workspace_declares_exactly_one_turn_loop has been green the whole time main has had two turn loops — it matches a name, not a property
- Dominant language
- Rust
- Stars
- 41k
- Forks
- 3.6k
- Avg merge
- 13h 59m
- Merged PRs (30d)
- 299
Description
`crates/core/tests/single_turn_loop.rs` is the guard for the product's central
architectural rule: **one turn loop.** It has been passing continuously while
`main` has had two.
## What it actually asserts
```rust
if trimmed.starts_with("async fn run_turn")
|| trimmed.starts_with("pub async fn run_turn")
|| trimmed.starts_with("pub(crate) async fn run_turn")
|| trimmed.starts_with("pub(super) async fn run_turn")
```
It scans for a function **named `run_turn`** and asserts there is exactly one.
Today that finds `crates/tui/src/core/engine/turn_loop.rs:669`, and passes.
## What it cannot see
`crates/tui/src/acp_server.rs`:
```
:69 const MAX_ACP_TOOL_ROUNDS: usize = 50;
:1286 async fn run_agentic_prompt_turn(
:1305 for _round in 0..MAX_ACP_TOOL_ROUNDS {
```
That is a turn loop — its own bounded tool-call round loop, its own prompt
assembly, its own conversation store — and the guard is blind to it because it
is not spelled `run_turn`. This is the subject of #6088, filed independently by
reading the code. The guard contributed nothing to finding it and would not
have.
## Why this is worth fixing rather than shrugging at
A guard that can only be satisfied by naming is a guard an author passes by
choosing a different name — and nobody chose a different name maliciously here,
which is the point. `run_agentic_prompt_turn` is a perfectly natural name. The
rule it violates is one of the few this product describes as existential:
> A second turn loop, a second event authority, or a second prompt authority
> anywhere in the stack is the thing this product exists to not be.
A test that reports compliance while the violation is 5,000 lines away in the
same crate is worse than no test, because it is cited as evidence.
## Ask
Assert the property, not the identifier. Some candidates, none of them free:
- Detect the *shape*: a bounded loop over tool-call rounds that calls a
provider-streaming entry point. More work, catches renames.
- Make the allow-list explicit: enumerate the files permitted to drive a turn
and fail on any other file that calls the streaming client directly.
`acp_server.rs` calls `client.create_message_stream` directly today, which is
the cheap signal.
- At minimum, extend the name match to the known second loop and make the test's
own doc comment state plainly what it does and does not cover, so nobody reads
a pass as proof of the rule.
I'd take the third immediately and one of the first two properly, rather than
leave the current version cited as the receipt.
## Evidence
`origin/main` at `bce761083`. Found while adversarially verifying a claim that
#6088 was already resolved on a branch; the claim failed, and this fell out of
checking why the guard had never fired.
Contributor guide
Research direction
Start with crates/core/tests/single_turn_loop.rs and compare its run_turn name matching with crates/tui/src/core/engine/turn_loop.rs and crates/tui/src/acp_server.rs, especially run_agentic_prompt_turn and the direct client.create_message_stream call. Decide whether the guard should detect the loop shape, enforce an explicit allow-list, or at minimum cover the known second loop and document its limits; done means the test no longer reports compliance while this second loop exists.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- cli, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100