Skip to content

Commit 5d67530

Browse files
committed
Address PR review: improve handler validation guards and guidance consistency
- 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>
1 parent ea24aef commit 5d67530

3 files changed

Lines changed: 39 additions & 12 deletions

File tree

src/agent/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,8 +1292,8 @@ const registerHandlerTool = defineTool("register_handler", {
12921292
"Register (or update) a named JavaScript handler in the sandbox.",
12931293
"The code is compiled but NOT executed — call execute_javascript to run it.",
12941294
"",
1295-
"REQUIRED: Code must define `function handler(event) { ... return result; }`",
1296-
"Use `async function handler(event)` only when the handler contains await.",
1295+
"REQUIRED: Code must define `function handler(...) { ... return result; }`",
1296+
"Use `async function handler(...)` only when the handler contains await.",
12971297
"The function MUST be named exactly 'handler' — not Handler, handle, main.",
12981298
"The parameter name is flexible; use 'event' by convention, or omit it if unused.",
12991299
"Do NOT use arrow handlers such as `const handler = (event) => { ... }`.",

src/agent/system-message.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ EVERYTHING goes through sandbox tools — register_handler, execute_javascript,
4444
4545
Every register_handler call MUST define a real handler function with one of these signatures:
4646
function handler(event) { ... return result; }
47+
function handler() { ... return result; } // when no input is needed
4748
async function handler(event) { ... return result; } // only when using await
4849
4950
The function MUST be named exactly "handler". Not Handler, handle, main, run, process.

src/code-validator/guest/runtime/src/validator.rs

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,7 @@ pub fn validate_javascript(source: &str, context: &ValidationContext) -> Validat
564564

565565
// 4b. If handler exists, check it has a return statement and correct signature
566566
if context.expect_handler && (has_handler_export || has_handler_function) {
567+
let has_signature_issue = handler_signature_issue.is_some();
567568
if let Some((message, line)) = handler_signature_issue {
568569
errors.push(ValidationError {
569570
error_type: "structure".to_string(),
@@ -573,15 +574,19 @@ pub fn validate_javascript(source: &str, context: &ValidationContext) -> Validat
573574
});
574575
}
575576

576-
// Check for return statement inside handler (uses comment-stripped source)
577-
let has_return = check_handler_has_return(&clean);
578-
if !has_return {
579-
errors.push(ValidationError {
580-
error_type: "structure".to_string(),
581-
message: "Handler function MUST return a value. Without 'return' you will get: 'The handler function did not return a value'. Fix: add 'return { ... };' at the end of your handler function.".to_string(),
582-
line: None,
583-
column: None,
584-
});
577+
// Only check for return when handler is a real function declaration and
578+
// has no signature issues — invalid shapes (arrow handlers, etc.) can
579+
// cause misleading "MUST return" errors.
580+
if !has_signature_issue && has_handler_function {
581+
let has_return = check_handler_has_return(&clean);
582+
if !has_return {
583+
errors.push(ValidationError {
584+
error_type: "structure".to_string(),
585+
message: "Handler function MUST return a value. Without 'return' you will get: 'The handler function did not return a value'. Fix: add 'return { ... };' at the end of your handler function.".to_string(),
586+
line: None,
587+
column: None,
588+
});
589+
}
585590
}
586591
}
587592

@@ -1064,6 +1069,8 @@ fn check_handler_signature_issue(source: &str) -> Option<(String, Option<u32>)>
10641069

10651070
/// Check if the handler function contains a return statement.
10661071
/// Scans ONLY the handler function body (brace-matched) for 'return'.
1072+
/// Skips nested function declarations, generator functions, and arrow
1073+
/// function bodies so their returns are not counted as handler returns.
10671074
fn check_handler_has_return(source: &str) -> bool {
10681075
// Source is already comment-stripped — just brace-match the handler body
10691076
if let Some(handler_pos) = source.find("function handler") {
@@ -1094,13 +1101,32 @@ fn check_handler_has_return(source: &str) -> bool {
10941101
}
10951102
}
10961103
b'f' => {
1097-
if rest[i..].starts_with("function ")
1104+
// Skip nested function and function* declarations
1105+
if (rest[i..].starts_with("function ")
1106+
|| rest[i..].starts_with("function*"))
10981107
&& let Some(after_nested) = skip_nested_block(rest, i)
10991108
{
11001109
i = after_nested;
11011110
continue;
11021111
}
11031112
}
1113+
b'=' => {
1114+
// Skip arrow function bodies: => { ... }
1115+
if rest[i..].starts_with("=>") {
1116+
let after_arrow = rest[i + 2..].trim_start();
1117+
if after_arrow.starts_with('{') {
1118+
// Find the position of the opening brace in the
1119+
// original rest slice and skip the braced body
1120+
let arrow_brace_offset = rest.len() - after_arrow.len();
1121+
if let Some(after_nested) =
1122+
skip_nested_block(rest, arrow_brace_offset)
1123+
{
1124+
i = after_nested;
1125+
continue;
1126+
}
1127+
}
1128+
}
1129+
}
11041130
_ => {}
11051131
}
11061132
i += 1;

0 commit comments

Comments
 (0)