feat: add turn_end observer hook#2578
Open
AresNing wants to merge 10 commits into
Open
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Thanks @AresNing for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
774217e to
74d69c3
Compare
9 tasks
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
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
Adds the Phase 2
turn_endlifecycle hook from #1364 as an observer-only post-turn event.Scope:
HookEvent::TurnEnd/event = "turn_end"discovery via/hooks events.turn_endfrom the TUITurnCompletebranch after post-turn state updates and before queued-message dispatch.continue_on_errorobserver semantics, and updates the Hooks need mutation rights on user submit and a turn-end event #1364 RFC baseline.web/package-lock.jsonso Web Frontend CI can runnpm ciafter the web docs change.Not in this slice:
turn_end.Builds on: #2434
Issues: Refs #1364 (partial)
Testing
cargo fmt --checkcargo checkcargo clippy --workspace --all-targets --all-featurescargo test -p codewhale-tui hooks::env HOME=/private/tmp/codewhale-test-home-turnend CARGO_HOME=/Users/aresning/.cargo RUSTUP_HOME=/Users/aresning/.rustup cargo test -p codewhale-tui mcp::tests::legacy_sse_closed_stream_reconnects_and_retries_tool_call --all-featuresenv HOME=/private/tmp/codewhale-test-home-turnend CARGO_HOME=/Users/aresning/.cargo RUSTUP_HOME=/Users/aresning/.rustup RUST_TEST_THREADS=1 cargo test --workspace --all-featuresnpm install --package-lock-only --ignore-scripts --registry=https://registry.npmjs.org/inweb/npm ci --ignore-scripts --registry=https://registry.npmjs.org/inweb/npm run lintinweb/npx tsc --noEmitinweb/git diff --checkNotes:
~/.deepseek/state.db; final full run used isolatedHOME.connection closed before message completed); the exact test passed, and the final single-thread full workspace run passed.npm ciwithout--ignore-scriptshung in a macOS postinstall path, so dependency/lockfile validation used--ignore-scripts; GitHub Actions runs fullnpm cion Ubuntu.continue_on_errordocumentation feedback was addressed and the review thread is resolved.Checklist
Greptile Summary
This PR lands Phase 2 of the #1364 hooks-lifecycle RFC: a
turn_endobserver hook that fires after every completed, interrupted, or failed model turn. The hook receives a structured JSON payload on stdin (status, usage counters, turn duration, tool count, queued message count) and is executed non-blocking viatokio::task::spawn_blocking, with failures logged as warnings and stdout ignored.turn_endhook: addsHookEvent::TurnEnd,execute_structured_observer, andturn_end_payloadbuilder; fires from theTurnCompletebranch inui.rsafter all post-turn state updates and before queued-message dispatch; documents observer semantics andcontinue_on_errorbehaviour.siliconflow-CNprovider: adds a dedicated provider ID for the regional.cnendpoint, sharing credentials and config table withsiliconflow; fixes duplicate match arms and indentation regressions from an earlierSiliconflowCnaddition.ApiProvider::SiliconflowCnmatch arms, reformats several long match arms forcargo fmt, and updates CHANGELOG, docs, RFC, web UI, andscripts/check-provider-registry.py.Confidence Score: 5/5
Safe to merge. The turn_end hook fires correctly for all terminal turn outcomes, is fully non-blocking, and cannot mutate app state. The siliconflow-CN provider and duplicate match-arm removals are straightforward.
The core hook infrastructure is well-implemented: execute_structured_observer iterates unconditionally (documented and tested), spawn_blocking correctly offloads blocking execution, tool_evidence is cleared at turn start so the count is accurate, and the payload ordering matches the RFC spec. The only gap is that unit tests cover status = 'completed' only; 'interrupted' and 'failed' paths are correct by exhaustive Rust match but have no dedicated test cases. No runtime defects were found.
No files require special attention. The hooks.rs test suite is the only area where additional coverage for non-completed turn statuses would strengthen the implementation per the RFC's own review checklist.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Engine participant EventLoop as run_event_loop (ui.rs) participant App participant SpawnBlocking as tokio::task::spawn_blocking participant HookExecutor Engine->>EventLoop: "EngineEvent::TurnComplete { usage, status, error, ... }" EventLoop->>App: Clear loading/streaming state EventLoop->>App: Record turn_elapsed EventLoop->>App: Update runtime_turn_status EventLoop->>App: Accumulate session token counters EventLoop->>App: Accrue session cost estimate EventLoop->>App: Emit desktop notification / receipt EventLoop->>App: Schedule session persistence Note over EventLoop,App: All post-turn state updates done alt hooks configured for TurnEnd EventLoop->>EventLoop: Build turn_end_payload(status, usage, totals, tool_count, ...) EventLoop->>SpawnBlocking: spawn_blocking (fire-and-forget) SpawnBlocking->>HookExecutor: execute_structured_observer(TurnEnd, context, payload) loop each matching hook HookExecutor->>HookExecutor: write payload to stdin HookExecutor->>HookExecutor: ignore stdout alt hook fails HookExecutor->>HookExecutor: tracing::warn!(...) — continue to next hook end end end EventLoop->>App: pop_queued_message() — dispatch next queued message if anyReviews (7): Last reviewed commit: "fix: document siliconflow cn registry" | Re-trigger Greptile