feat(events): emit GOAL_ACHIEVED on terminal-node success#6499
Open
gavksingh wants to merge 1 commit intoaden-hive:mainfrom
Open
feat(events): emit GOAL_ACHIEVED on terminal-node success#6499gavksingh wants to merge 1 commit intoaden-hive:mainfrom
gavksingh wants to merge 1 commit intoaden-hive:mainfrom
Conversation
bryanadenhq
requested changes
Mar 16, 2026
Collaborator
bryanadenhq
left a comment
There was a problem hiding this comment.
Thanks for wiring up the GOAL_ACHIEVED event, that part looks clean and straightforward. However, this PR bundles in significant fan-out executor changes (memory conflict detection, per-branch timeouts, refactored result processing) that aren't related to the goal achieved event or #5736. Could you split those into a separate PR? It'll make both easier to review and safer to merge/revert independently.
The GOAL_ACHIEVED EventType was defined in the enum and fully handled by the TUI (subscription, chat_repl handler, log_pane formatting), but no backend code ever emitted it. This wires up the missing emission: - Add emit_goal_achieved() convenience method to EventBus - Emit GOAL_ACHIEVED from GraphExecutor when execution reaches a terminal node successfully - Update EVENT_TYPES.md to document the payload and emission point - Add tests for both the EventBus method and executor integration Fixes aden-hive#5736
0aab93e to
8c0270a
Compare
Contributor
Author
|
@bryanadenhq thanks for pointing that out! The executor changes (branch timeout + memory conflict) were from #5706 which I also worked on and got merged separately via PR #6504. Forgot to rebase this branch after that. Rebased and force pushed now so this PR only has the GOAL_ACHIEVED event emission. |
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
emit_goal_achieved()convenience method toEventBus, mirroring the pattern of existing emitters likeemit_constraint_violation()GOAL_ACHIEVEDfromGraphExecutorwhen execution completes at a terminal node successfullyEVENT_TYPES.mdto document the payload shape and emission pointThe
GOAL_ACHIEVEDEventTypewas already defined in the enum and fully handled by the TUI (subscription,chat_replhandler,log_paneformatting), but no backend code ever emitted it. This completes the agent lifecycle observability story.Fixes #5736
Test plan
emit_goal_achieved()publishes anAgentEventwith correct type, stream_id, execution_id, and data{}when None is passedGraphExecutorcallsemit_goal_achievedon successful terminal-node completionGraphExecutorworks without crash whenevent_busis Nonetest_executor_skips_events_for_event_loop_nodesupdated to distinguish node-level vs graph-level eventsruff checkpasses on all modified files