Skip to content

fix(engine): keep turn and submission attribution on TurnComplete - #7018

Merged
Hmbown merged 3 commits into
codewhale-hq:mainfrom
asto18089:up/turn-complete-attribution
Oct 18, 2026
Merged

Hmbown merged 3 commits into
codewhale-hq:mainfrom
asto18089:up/turn-complete-attribution

Conversation

@asto18089

@asto18089 asto18089 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

What

Terminal events lost their attribution. The internal TurnComplete carried no turn id, the wire event's turn_id was documented as filled "from the envelope when it knows it" and the in-process projection hardcoded None, preflight rejections (route-install failure, edit_last_turn rejections) emitted bare unattributed terminals, and the turn_end hooks lifecycle reported runtime_turn_id — a stale-surviving id that is not an authority for a later TurnComplete. Hosts correlating a submission with its outcome had to compensate heuristically.

TurnComplete now carries its attribution end to end:

  • internal Event::TurnComplete gains turn_id, submission_id, compaction_id;
  • wire EventMsg::TurnComplete gains submission_id and compaction_id (serde-default, absent in older producers — strictly additive; no existing field changes);
  • turn_id is the actually started turn, absent when a submitted operation fails before starting;
  • submission_id is the host submission correlation, including preflight rejection; runtime self-starts do not carry a host token;
  • compaction_id is the manual compaction operation identity, never the surrounding host submission;
  • the parity projection forwards all three, the UI's turn_end lifecycle receipt uses the terminal event's attribution when present (the U02-07 ownership heuristic remains only for absent attribution), and remote-control/ACP runtimes pass it through.

Tests

  • Protocol: additive round-trip (turn_complete_ownership_is_additive_and_round_trips) — old JSON without the fields decodes, new fields serialize only when present; full protocol suite green.
  • Engine: started turns echo their attribution, preflight rejections carry the submission id with no turn id, queued/manual compaction terminals keep their operation id, self-starts stay None.
  • UI/runtime-threads: the lifecycle receipt uses the terminal event's owner; stale runtime_turn_id no longer leaks into receipts.
  • cargo test -p codewhale-tui --lib targeted filters green (turn_complete, submission_id, terminal_correlation, runtime_threads 290 passed); clippy -D warnings clean (tui lib + protocol).

Ported from the Pinvou/CodeWhale fork (#97 by @zhuowp), re-expressed on the current engine (pre-admitted turns, parent_route_usage fields).

Refs #6557

Terminal `TurnComplete` events used to carry no operation identity of
their own: preflight rejections (route install failure, edit_last_turn
rejection), manual compaction terminals and model-turn terminals all
reached embedded hosts bare, so hosts closed whichever request happened
to be busy. The parity projection hardcoded `turn_id: None`, discarding
the attribution the engine already knew, and the TUI hooks lifecycle
emitted `turn_end` with the stale-surviving `runtime_turn_id`, letting a
later rejection or compaction close the preceding model turn.

The internal `Event::TurnComplete`, the wire `EventMsg::TurnComplete`
and the engine's terminal emissions now preserve each operation's
original identity:

- `turn_id` is the actually started turn (absent when a submitted
  operation fails before starting, e.g. reject_edit_last_turn or
  `handle_purge` failure).
- `submission_id` is the host submission correlation, echoed including
  preflight rejection; runtime self-starts carry `None`.
- `compaction_id` is the manual compaction operation identity, never the
  surrounding host submission; auto compaction keeps closing its owning
  model turn, and purge carries none of the identities.

`protocol_parity` projects all three instead of hardcoding `turn_id:
None`, and `tui/ui/event_loop.rs` captures a `TurnEndLifecycleReceipt`
from the terminal event itself, using the carried attribution for the
loading-ownership check and the `turn_end` lifecycle receipt; the
U02-07 heuristic stays only for events with absent attribution. Wire
changes are strictly additive and serde-default, so older producers
round-trip unchanged. Engine, remote_control, runtime_threads and
runtime_api tests pin the contract, and docs/ARCHITECTURE.md documents
the wire addition.

Co-authored-by: zhuowp <9426526+zhuowp@users.noreply.github.com>
Signed-off-by: asto <asto18089@126.com>
@asto18089
asto18089 requested a review from Hmbown as a code owner October 18, 2026 02:06

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added the contribution-gate Author not yet in .github/APPROVED_CONTRIBUTORS; a maintainer grants access with /lgtm label Oct 18, 2026
Hmbown pushed a commit that referenced this pull request Oct 18, 2026
#7016, #7018 and #7019 from @asto18089 carry a Co-authored-by trailer for
@zhuowp (the work was ported from a fork they contributed to).
scripts/check-contributor-credit.py, run as the per-PR "Version drift"
job, fails those PRs because @zhuowp is absent from
web/lib/release-credits.ts. docs/CONTRIBUTORS.md and CHANGELOG.md already
credit them. This adds the handle to UNRELEASED_CONTRIBUTORS, which the
release cut moves into the release block.

Refs #7016
Refs #7018
Refs #7019

Verified: python3 scripts/check-contributor-credit.py ends "Every
contributor in the window is credited on all three surfaces";
web check-facts OK; npx tsc --noEmit OK.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBKbhx19BRbZtvWK8rVKjz
@Hmbown
Hmbown merged commit ebc86e6 into codewhale-hq:main Oct 18, 2026
30 of 31 checks passed
Hmbown pushed a commit that referenced this pull request Oct 18, 2026
#7018 (TurnComplete attribution, @asto18089) changed docs/ARCHITECTURE.md,
which web/data/agent-docs.generated.json embeds, so the Web Frontend
check-facts step would fail on main. This regenerates it.

Two review follow-ups:
- ARCHITECTURE.md said every send/edit rejection before TurnStarted emits
  a failed terminal carrying the submission_id. Only a route-install
  failure and an edit_last_turn rejection do. The image and constitution
  rejections in handle_send_message still send only Error. A host built
  from the old text would wait for a terminal that never comes; the doc
  now names the cases.
- event_loop.rs kept the comment "TurnComplete carries no turn id", which
  #7018 made false. The U02-07 heuristic stays because TUI dispatches carry
  no submission id; the comment now says that.

Refs #7018
Refs #6557

Verified: web check-facts OK after npm run prebuild; cargo fmt --check
clean. Comment and doc changes only on the Rust side.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBKbhx19BRbZtvWK8rVKjz
@Hmbown

Hmbown commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

🤖 This reply is from Claude Opus 5.5, an AI agent working on behalf of the Codewhale maintainers. It is posted through @Hmbown's account, but Hunter did not write it.

Merged in ebc86e6. Thank you @asto18089. Terminal events now carry their own turn_id, submission_id and compaction_id. The two exits that sent no terminal at all now do: a route that fails to install, and a manual compaction whose route is rejected. Before, callers waiting on TurnComplete could hang on those, and the TUI's turn_end receipt borrowed the previous turn's id.

Follow-up on main (7c3828d):

  • docs/ARCHITECTURE.md now names which pre-start rejections emit a failed terminal: route install and edit_last_turn. The image and constitution rejections in handle_send_message still emit only Error, and the old wording would have had a host wait for a terminal that never comes.
  • The event-loop comment that said TurnComplete carries no turn id now says the U02-07 heuristic stays because TUI dispatches carry no submission id.
  • The generated web agent docs are regenerated, because they embed ARCHITECTURE.md.

Not in a release yet. The new ids are not used for loading ownership yet; that remains #6557.

— Written by Claude Opus 5.5 (AI agent) on behalf of the maintainers.

Hmbown pushed a commit that referenced this pull request Oct 18, 2026
Adds [Unreleased] entries for #7014, #7015, #7017, #7018, #7019 and #7020,
crediting @asto18089 (and @zhuowp for the fork ports). Each merged without
a changelog line. The entry for #7020 names its behavior change: the
app-server's legacy [providers.custom] table now refuses a non-Chat wire
with provider_wire_format_unsupported instead of forwarding a Chat body.

Refs #7014
Refs #7015
Refs #7017
Refs #7018
Refs #7019
Refs #7020
Refs #6997

Verified: sync-changelog --check up to date; web check-facts OK;
check-contributor-credit "Every contributor in the window is credited on
all three surfaces".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBKbhx19BRbZtvWK8rVKjz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution-gate Author not yet in .github/APPROVED_CONTRIBUTORS; a maintainer grants access with /lgtm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants