nodejs: Expose onAgentStop session hook#2054
Draft
belaltaher8 wants to merge 1 commit into
Draft
Conversation
The runtime already fires the top-level agent's `agentStop` hook and
registers it for SDK callback sessions (REGISTERED_CALLBACK_EVENT_NAMES),
with a working block/continue re-prompt loop, but the Node SDK never
exposed it: SessionHooks had no `onAgentStop` and `_handleHooksInvoke`
had no dispatch entry, so `agentStop` callbacks were silently dropped.
Add the AgentStopHookInput/Output/Handler types, the `onAgentStop` field
on SessionHooks, and the `agentStop` entry in the hook dispatcher.
Returning `{ decision: "block", reason }` keeps the agent running with
`reason` enqueued as a follow-up message (e.g. to remediate findings a
handler surfaced); returning nothing lets the agent stop.
This unblocks the Copilot cloud agent restoring its post-completion
security-tool hooks (dependabot / secret scanning) on the proper platform
hook rather than ad-hoc per-commit hooks.
Note: parallel changes for the Python/Go/.NET SDKs are follow-ups.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2031e02b-7fe5-4075-9d75-eb20eb29f407
Contributor
There was a problem hiding this comment.
Pull request overview
Exposes the Node.js SDK’s top-level agentStop lifecycle hook.
Changes:
- Adds typed agent-stop input, output, and handler APIs.
- Routes
agentStopcallbacks through the session dispatcher. - Adds dispatcher and JSON-RPC unit tests.
Show a summary per file
| File | Description |
|---|---|
nodejs/src/types.ts |
Defines the agent-stop hook API. |
nodejs/src/session.ts |
Dispatches agent-stop callbacks. |
nodejs/test/client.test.ts |
Tests direct and JSON-RPC dispatch. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Medium
| sessionStart: this.hooks.onSessionStart as GenericHandler | undefined, | ||
| sessionEnd: this.hooks.onSessionEnd as GenericHandler | undefined, | ||
| errorOccurred: this.hooks.onErrorOccurred as GenericHandler | undefined, | ||
| agentStop: this.hooks.onAgentStop as GenericHandler | undefined, |
Comment on lines
+1516
to
+1519
| export type AgentStopHandler = ( | ||
| input: AgentStopHookInput, | ||
| invocation: { sessionId: string } | ||
| ) => Promise<AgentStopHookOutput | void> | AgentStopHookOutput | void; |
Comment on lines
+1575
to
+1580
| * `{ decision: "block", reason }` to keep the agent running with `reason` | ||
| * enqueued as a follow-up message — for example, to have the agent | ||
| * remediate findings the handler surfaced. Returning nothing lets the | ||
| * agent stop. | ||
| */ | ||
| onAgentStop?: AgentStopHandler; |
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
Exposes the top-level agent's
agentStoplifecycle hook on the Node SDK.The Copilot CLI runtime already fires the main agent's
agentStophook when the agent reaches a natural terminal stop, and already registersagentStopfor SDK callback sessions (REGISTERED_CALLBACK_EVENT_NAMES) — including a working block/continue re-prompt loop (a{decision:"block", reason}result enqueuesreasonas a follow-up message; consecutive blocks are capped to prevent runaway loops).The only missing piece was on the SDK side:
SessionHookshad noonAgentStop, and_handleHooksInvokehad no dispatch entry — soagentStopinvocations from the runtime were silently dropped for SDK clients.Changes (
nodejs)src/types.ts: addAgentStopHookInput,AgentStopHookOutput({ decision?: "block"; reason?: string }),AgentStopHandler, and theonAgentStopfield onSessionHooks.src/session.ts: add theagentStopentry to the_handleHooksInvokehandler map.test/client.test.ts: dispatcher unit tests — direct dispatch returning ablockdecision, and the fullhooks.invokeJSON-RPC wire path.Motivation
This unblocks the Copilot cloud coding agent (CCA v3) restoring its post-completion security-tool hooks (Dependabot advisory checker, secret scanning) on the proper platform hook — instead of the ad-hoc per-commit
report_progresshooks that accumulated repeated "missed commit boundary" review feedback.onAgentStopfires once at the natural end of the run and supports remediation via block/continue.Notes / follow-ups
stop_hook_active(snake_case) in the agentStop input. This PR'sAgentStopHookInput.stopHookActivedocuments that field; normalizing snake→camel for this one hook (or emitting camelCase runtime-side) is a small follow-up. ThestopReason/transcriptPathfields and the{decision, reason}output — the load-bearing parts — are correct today.onAgentStopadditions for the Python / Go / .NET SDKs are follow-ups (this draft covers Node.js, which CCA v3 uses).Draft (fork PR) for review/discussion.
Related: github/copilot-agent-runtime#13230 (issue is being corrected — the runtime-side
agentStopgap it describes does not exist).