Hmbown / Hmbown/Codewhale

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

Open
#6,242 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.