feat: allow trigger tests to terminate early on skill invocation#207
Merged
Conversation
Add CancelOnSkillInvocation flag to ExecutionRequest that cancels the execution context as soon as a SkillInvoked event is received. This allows trigger tests to return immediately once the target skill fires, instead of waiting for the agent to complete its full turn. Implementation: - Add onSkillInvoked callback to SessionEventsCollector - Wire up context cancellation in CopilotEngine.Execute when flag is set - Trigger runner sets CancelOnSkillInvocation=true on all test prompts - Context cancellation from skill invocation is treated as success Tests: - SessionEventsCollector callback fires on skill invocation - CopilotEngine cancels SendAndWait early when skill invoked - CopilotEngine completes normally when no skill fires (flag is safe) - Trigger runner sets the CancelOnSkillInvocation flag Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6badcbf to
822fb92
Compare
- Rename cancelledForSkill to canceledForSkill (American spelling) - Fix 'cancelled' misspelling in comments - Add mutex to capturingEngine to prevent data race in trigger tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
95f131e to
c44dcdd
Compare
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.
Closes #188
Summary
Trigger tests now terminate early when the target skill is invoked, instead of waiting for the agent to finish its full turn. This reduces execution time for trigger test suites, particularly in CI environments with hard time limits.
Approach
Added a
CancelOnSkillInvocationflag toExecutionRequestthat hooks into the event stream at the execution layer:SessionEventsCollector— Added an optionalonSkillInvokedcallback that fires when aSkillInvokedevent arrives with a valid Name or PathCopilotEngine.Execute()— When the flag is set, derives a cancellable context and registers the callback to cancel it on skill invocation. Context cancellation from this path is treated as success (not an error)trigger/runner.go— SetsCancelOnSkillInvocation=trueon all trigger test execution requestsWhy the execution layer?
The key insight from the rubber-duck review:
SendAndWait()blocks until the agent finishes its full turn. By the time the trigger runner checksSkillInvocations, the wait is already over. The fix has to happen mid-stream — cancelling the context whileSendAndWaitis still blocking.Tests
TestSessionEventsCollector_OnSkillInvokedCallback— callback fires on invocation, skips invalid events, nil callback is safeTestCopilotExecute_CancelOnSkillInvocation— mocked engine provesSendAndWaitreturns early when skill fires (completes in <5s vs 30s timeout)TestCopilotExecute_CancelOnSkillInvocation_NoSkillFired— flag is safe when no skill fires (completes normally)TestRunnerSetsCancelOnSkillInvocation— trigger runner sets the flag on execution requestsAll existing tests pass:
go test -mod=readonly ./...✅ andgo vet -mod=readonly ./...✅