feat: add turn_end observer hook#2578
Open
AresNing wants to merge 11 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 implements the Phase 2
turn_endobserver hook from the #1364 lifecycle RFC. The hook fires after each terminal turn outcome (completed,interrupted,failed), runs asynchronously viatokio::task::spawn_blockingto keep the event loop non-blocking, and delivers a structured JSON payload to hook scripts on stdin while ignoring stdout.HookEvent::TurnEnd,execute_structured_observer, andturn_end_payloadinhooks.rs; wires the hook into theEngineEvent::TurnCompletebranch ofui.rsafter all post-turn state updates and before queued-message dispatch.continue_on_errordoes not gate later hooks, stdout is ignored, failures are warn-only) in bothdocs/CONFIGURATION.mdand the RFC; surface-level web doc update addsturn_endto the quick-reference comment.web/package-lock.jsonto resolve a vitest esbuild version split so Web CI can runnpm cicleanly.Confidence Score: 5/5
Safe to merge. The observer hook is strictly additive, fire-and-forget, and cannot mutate or block any caller state.
The implementation is architecturally sound: spawn_blocking keeps the Tokio event loop unblocked, has_hooks_for_event correctly gates on the enabled flag, the payload snapshot is taken before queued-message pop so counts are accurate, and the unconditional-iteration / warn-only failure model is both intentional and clearly documented. Tests cover stdin delivery, stdout-ignored contract, and the continue_on_error=false non-propagation case explicitly. No behavioral regressions to existing hook events are possible because the new code path only activates when TurnEnd hooks are explicitly configured.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Engine participant EventLoop as UI Event Loop participant App participant BlockingPool as Tokio Blocking Pool participant HookScript as Hook Script Engine->>EventLoop: EngineEvent::TurnComplete EventLoop->>App: Clear loading/streaming state EventLoop->>App: Set runtime_turn_status EventLoop->>App: Update token counters and cost EventLoop->>App: Schedule session snapshot EventLoop->>App: has_hooks_for_event(TurnEnd)? alt hooks configured and enabled EventLoop->>App: Snapshot post-update state into payload EventLoop->>App: Clone HookExecutor EventLoop->>BlockingPool: spawn_blocking fire-and-forget EventLoop->>App: pop_queued_message and dispatch next turn BlockingPool->>HookScript: exec with JSON on stdin HookScript-->>BlockingPool: exit code and stderr Note over BlockingPool: stdout ignored, failures warn-only else no hooks configured EventLoop->>App: pop_queued_message and dispatch next turn endReviews (8): Last reviewed commit: "merge origin/main into phase2 branch" | Re-trigger Greptile