Handler shape validation guidance#105
Merged
simongdavies merged 4 commits intomainfrom May 6, 2026
Merged
Conversation
Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
…sistency
- Gate return-check on has_handler_function && no signature issue to
avoid misleading 'MUST return' errors on invalid handler shapes
- Expand check_handler_has_return skip logic to also skip arrow function
bodies (=> { ... }) and generator declarations (function*)
- Fix conflicting guidance in register_handler tool description: use
function handler(...) instead of function handler(event) in REQUIRED line
- Add zero-parameter form function handler() to system-message signature list
Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens and clarifies the required shape of sandbox handler code (function handler(...) { ... } / async function handler(...) { ... }), updates validator behavior and error guidance accordingly, and expands test coverage to lock in the new expectations.
Changes:
- Updated user-facing guidance (system prompt + CLI tool docs/help) to explicitly require
function handlerdeclarations (including optional params / async guidance) and to disallow arrow-based handlers. - Refined the Rust validator to emit more actionable structure errors (including line attribution) and to improve handler-return detection by ignoring returns in nested scopes.
- Added/expanded tests for handler declaration variants (custom param names, zero-parameter handlers, arrow handler rejection, and module-mode detection).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sandbox-tool.test.ts | Adds execution tests for module-style handlers with custom parameter names and no parameters. |
| tests/analysis-guest.test.ts | Expands guest validator tests for handler declaration modes and improved error messaging. |
| src/code-validator/guest/runtime/src/validator.rs | Implements stricter handler-shape diagnostics and more precise handler-return detection logic. |
| src/agent/system-message.ts | Updates system prompt guidance to reflect stricter handler declaration requirements. |
| src/agent/index.ts | Updates tool/help text for handler registration and handler code style guidance. |
- Skip misnamed-handler check when valid function handler already exists, preventing false positives on nested helpers (e.g. function process()) - Fix inconsistent help text: handler(...) instead of handler(event) to match the zero-parameter guidance - Also skip function( and function*( expression forms in return check so anonymous function expression returns are not counted as handler returns Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
Keep our PR review fixes (handler validation improvements): - Nested helper false-positive prevention - Consistent handler(...) help text - Anonymous function expression return skip Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
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.
This pull request updates the handler registration system for JavaScript code in the sandbox, clarifying and enforcing stricter requirements for handler function declarations. It improves both the user-facing documentation and the internal validator logic, ensuring that only correctly structured handler functions are accepted. The changes also enhance error messages and add comprehensive tests for various handler declaration scenarios.
Handler declaration requirements and documentation:
system-message.ts,index.ts) to clarify that handlers must be declared asfunction handler(...) { ... }orasync function handler(...) { ... }(if usingawait), with flexible parameter names and optional parameters. Arrow functions and incorrect names are explicitly disallowed, and clear guidance is given for correct usage.Validator logic improvements:
Handler return detection:
returnstatement inside the handler function, ensuring that returns in nested functions or arrow functions are not incorrectly counted as valid handler returns.Testing:
These changes make handler requirements clearer for users and more robustly enforced by the system, reducing common errors and improving the developer experience.