diff --git a/src/agent/index.ts b/src/agent/index.ts index 63d0568..de3be39 100644 --- a/src/agent/index.ts +++ b/src/agent/index.ts @@ -1292,8 +1292,11 @@ const registerHandlerTool = defineTool("register_handler", { "Register (or update) a named JavaScript handler in the sandbox.", "The code is compiled but NOT executed — call execute_javascript to run it.", "", - "REQUIRED: Code must define `function handler(event) { ... return result; }`", + "REQUIRED: Code must define `function handler(...) { ... return result; }`", + "Use `async function handler(...)` only when the handler contains await.", "The function MUST be named exactly 'handler' — not Handler, handle, main.", + "The parameter name is flexible; use 'event' by convention, or omit it if unused.", + "Do NOT use arrow handlers such as `const handler = (event) => { ... }`.", "", "⚠️ TO UPDATE EXISTING CODE: Use get_handler_source first!", "When fixing errors, call get_handler_source(name) to get current code,", @@ -1332,8 +1335,9 @@ const registerHandlerTool = defineTool("register_handler", { code: { type: "string", description: - "JavaScript source code. Simple mode: use `return` for output. " + - "Module mode: define `function handler(event) { ... }`.", + "JavaScript source code. Define `function handler(event) { ... return result; }`, " + + "`function handler() { ... return result; }` if no input is needed, " + + "or `async function handler(event) { ... return result; }` if using await.", }, }, required: ["name", "code"], @@ -2893,9 +2897,10 @@ const HELP_TOPICS: Record = { "- Handlers CANNOT call other handlers — they are isolated modules", "- YOU orchestrate: pass handler A's result as handler B's event", "", - "TWO CODE STYLES (auto-detected):", - "- Simple: no 'function handler' → wrapped as function body, locals reset each call", - "- Module: defines 'function handler(event)' → module-level state persists", + "HANDLER CODE STYLE:", + "- Every handler must define function handler(event) or async function handler(event)", + "- The function name is mandatory; parameter names are flexible, and zero parameters are allowed", + "- Module-level state persists across execute_javascript calls", "", "COMMON MISTAKES:", "- Function must be named exactly 'handler' (not Handler, handle, main)", @@ -2949,7 +2954,7 @@ const HELP_TOPICS: Record = { debugging: [ "DEBUGGING TIPS:", "- 'not a function' = you guessed a method name. Call module_info/plugin_info to verify.", - "- register_handler returns codeSize and mode (module/simple) — check these", + "- register_handler returns codeSize and mode — check these", "- If handler errors, try a minimal version first", "- Build up complexity gradually — add one feature at a time", "- Use try/catch inside handlers to catch runtime errors cleanly", diff --git a/src/agent/system-message.ts b/src/agent/system-message.ts index f912b74..2136ce2 100644 --- a/src/agent/system-message.ts +++ b/src/agent/system-message.ts @@ -42,9 +42,15 @@ EVERYTHING goes through sandbox tools — register_handler, execute_javascript, ║ MANDATORY HANDLER FORMAT — YOUR CODE WILL BE REJECTED WITHOUT THIS ║ ╚══════════════════════════════════════════════════════════════════════╝ -Every register_handler call MUST define: function handler(event) { ... return result; } +Every register_handler call MUST define a real handler function with one of these signatures: + function handler(event) { ... return result; } + function handler() { ... return result; } // when no input is needed + async function handler(event) { ... return result; } // only when using await + The function MUST be named exactly "handler". Not Handler, handle, main, run, process. -Code without "function handler" is ALWAYS rejected by the validator. +The parameter name is flexible; examples use "event" because execute_javascript passes JSON event data. +Handlers that do not need input may omit the parameter: function handler() { ... }. +Code without "function handler" or "async function handler" is ALWAYS rejected by the validator. TEMPLATE — copy this structure every time: import * as pptx from "ha:pptx"; // imports at top @@ -54,7 +60,10 @@ TEMPLATE — copy this structure every time: } RULES: - - function handler(event) — EXACTLY this signature, no exceptions + - Use function handler(event) for normal code + - Use async function handler(event) only if the handler contains await + - Do NOT use arrow handlers: const handler = (event) => { ... } is rejected + - Parameter names are flexible: event, input, data, args, or no parameter are all allowed - MUST return a value — handler without return = runtime error - event is JSON in, return value is JSON out - One-shot: runs once, returns, done diff --git a/src/code-validator/guest/runtime/src/validator.rs b/src/code-validator/guest/runtime/src/validator.rs index fc8c08f..a279e6b 100644 --- a/src/code-validator/guest/runtime/src/validator.rs +++ b/src/code-validator/guest/runtime/src/validator.rs @@ -469,6 +469,7 @@ pub fn validate_javascript(source: &str, context: &ValidationContext) -> Validat let imports = extract_imports(&clean); let has_handler_export = check_handler_export(&clean); let has_handler_function = check_handler_function(&clean); + let handler_signature_issue = check_handler_signature_issue(&clean); let is_module = check_is_module(&clean); // 1. Check handler name conflicts FIRST (cheap check) @@ -543,44 +544,49 @@ pub fn validate_javascript(source: &str, context: &ValidationContext) -> Validat if context.expect_handler && !has_handler_export && !has_handler_function { errors.push(ValidationError { error_type: "structure".to_string(), - message: concat!( - "Handler code MUST define: function handler(event) { ... return result; }\n", + message: handler_signature_issue.as_ref().map(|(message, _)| message.clone()).unwrap_or_else(|| concat!( + "Handler code MUST define: function handler(...) { ... return result; }\n", " The function MUST be named exactly 'handler' (not Handler, handle, main, run).\n", - " It MUST accept one argument (event) and MUST return a value.\n", + " The handler may accept one JSON argument, conventionally named 'event'.\n", + " Use async function handler(...) only if the handler contains await.\n", + " Do NOT use arrow handlers like const handler = (event) => { ... }.\n", " Example:\n", " import * as pptx from 'ha:pptx';\n", " function handler(event) {\n", " const result = pptx.createPresentation();\n", " return { success: true, data: result };\n", " }" - ).to_string(), - line: None, + ).to_string()), + line: handler_signature_issue.as_ref().and_then(|(_, line)| *line), column: None, }); } // 4b. If handler exists, check it has a return statement and correct signature if context.expect_handler && (has_handler_export || has_handler_function) { - // Check for return statement inside handler (uses comment-stripped source) - let has_return = check_handler_has_return(&clean); - if !has_return { + let has_signature_issue = handler_signature_issue.is_some(); + if let Some((message, line)) = handler_signature_issue { errors.push(ValidationError { error_type: "structure".to_string(), - 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(), - line: None, + message, + line, column: None, }); } - // Check handler takes at least one parameter - let has_param = check_handler_has_param(&clean); - if !has_param { - errors.push(ValidationError { - error_type: "structure".to_string(), - message: "Handler function MUST accept an event parameter: function handler(event) { ... return result; }. Without it you cannot receive input from execute_javascript.".to_string(), - line: None, - column: None, - }); + // Only check for return when handler is a real function declaration and + // has no signature issues — invalid shapes (arrow handlers, etc.) can + // cause misleading "MUST return" errors. + if !has_signature_issue && has_handler_function { + let has_return = check_handler_has_return(&clean); + if !has_return { + errors.push(ValidationError { + error_type: "structure".to_string(), + 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(), + line: None, + column: None, + }); + } } } @@ -993,17 +999,78 @@ fn check_handler_function(source: &str) -> bool { if source.contains("function handler") { return true; } - if source.contains("const handler") - || source.contains("let handler") - || source.contains("var handler") - { - return true; - } false } +/// Diagnose handler declarations that look close but are not the required shape. +fn check_handler_signature_issue(source: &str) -> Option<(String, Option)> { + for (line_index, line) in source.lines().enumerate() { + let trimmed = line.trim(); + let line_number = Some((line_index + 1) as u32); + + if trimmed.contains("=>") + && (trimmed.contains("handler") + || trimmed.contains("Handler") + || trimmed.contains("handle") + || trimmed.contains("main") + || trimmed.contains("run")) + { + return Some(( + concat!( + "Handler must be a function declaration, not an arrow function. ", + "Fix: replace it with function handler(event) { ... return result; } ", + "or async function handler(event) { ... return result; } if you use await." + ) + .to_string(), + line_number, + )); + } + + if trimmed.starts_with("export default function handler") { + return Some(( + concat!( + "Handler must be declared as function handler(...) or async function handler(...). ", + "Do not use export default function handler(...)." + ) + .to_string(), + line_number, + )); + } + + let mut declaration = trimmed; + if let Some(rest) = declaration.strip_prefix("export ") { + declaration = rest.trim_start(); + } + if let Some(rest) = declaration.strip_prefix("async ") { + declaration = rest.trim_start(); + } + + if let Some(rest) = declaration.strip_prefix("function ") { + let name_end = rest + .find(|c: char| !(c.is_ascii_alphanumeric() || c == '_' || c == '$')) + .unwrap_or(rest.len()); + let name = &rest[..name_end]; + if name == "handler" { + continue; + } else if matches!(name, "Handler" | "handle" | "main" | "run" | "process") { + return Some(( + alloc::format!( + "Handler function must be named exactly 'handler'. Found function '{}'. Fix: rename it to function handler(event) {{ ... return result; }}.", + name + ), + line_number, + )); + } + } + } + + None +} + /// Check if the handler function contains a return statement. /// Scans ONLY the handler function body (brace-matched) for 'return'. +/// Skips nested function declarations, generator functions, and arrow +/// function bodies so their returns are not counted as handler returns. fn check_handler_has_return(source: &str) -> bool { // Source is already comment-stripped — just brace-match the handler body if let Some(handler_pos) = source.find("function handler") { @@ -1033,6 +1100,33 @@ fn check_handler_has_return(source: &str) -> bool { found_return = true; } } + b'f' => { + // Skip nested function and function* declarations + if (rest[i..].starts_with("function ") + || rest[i..].starts_with("function*")) + && let Some(after_nested) = skip_nested_block(rest, i) + { + i = after_nested; + continue; + } + } + b'=' => { + // Skip arrow function bodies: => { ... } + if rest[i..].starts_with("=>") { + let after_arrow = rest[i + 2..].trim_start(); + if after_arrow.starts_with('{') { + // Find the position of the opening brace in the + // original rest slice and skip the braced body + let arrow_brace_offset = rest.len() - after_arrow.len(); + if let Some(after_nested) = + skip_nested_block(rest, arrow_brace_offset) + { + i = after_nested; + continue; + } + } + } + } _ => {} } i += 1; @@ -1059,56 +1153,43 @@ fn check_handler_has_return(source: &str) -> bool { false } -/// Check if the handler function has at least one parameter. -fn check_handler_has_param(source: &str) -> bool { - // Simple string search — more reliable than regex in no_std - for line in source.lines() { - let trimmed = line.trim(); - // Match: function handler(something or export function handler(something - if trimmed.contains("function handler(") { - // Check if there's a non-empty param: handler( followed by non-) non-whitespace - if let Some(paren_pos) = trimmed.find("handler(") { - let after_paren = &trimmed[paren_pos + 8..]; - if let Some(c) = after_paren.trim_start().chars().next() - && c != ')' - { - return true; - } - } - } - // Arrow: const handler = (event) => or handler = event => - // Must verify it actually has a parameter, not just () => - if (trimmed.contains("const handler") || trimmed.contains("let handler")) - && trimmed.contains("=>") - { - // Check for non-empty parens: handler = (something) => - if let Some(eq_pos) = trimmed.find('=') { - let after_eq = trimmed[eq_pos + 1..].trim_start(); - // Skip any second '=' (==) - if after_eq.starts_with('=') { - // Not an assignment — skip - } else if after_eq.starts_with('(') { - // Parenthesised params: check for non-empty (x) vs () - if let Some(close) = after_eq.find(')') { - let params = after_eq[1..close].trim(); - if !params.is_empty() { - return true; - } - } - } else if !after_eq.starts_with("=>") { - // Bare param without parens: handler = event => ... - return true; - } - } +fn skip_nested_block(source: &str, start: usize) -> Option { + let bytes = source.as_bytes(); + let mut open_brace = start; + while open_brace < bytes.len() && bytes[open_brace] != b'{' { + open_brace += 1; + } + if open_brace >= bytes.len() { + return None; + } + + let mut depth = 1; + let mut index = open_brace + 1; + while index < bytes.len() && depth > 0 { + match bytes[index] { + b'{' => depth += 1, + b'}' => depth -= 1, + _ => {} } + index += 1; } - false + + Some(index) } /// Check if source uses ES module syntax. fn check_is_module(source: &str) -> bool { let patterns = [ - "import ", "import\t", "import\n", "import{", "export ", "export\t", "export\n", "export{", + "import ", + "import\t", + "import\n", + "import{", + "export ", + "export\t", + "export\n", + "export{", + "function handler", + "async function handler", ]; for pattern in patterns { if source.contains(pattern) { @@ -2233,6 +2314,103 @@ mod tests { ); } + #[test] + fn test_handler_declaration_is_module_mode() { + let source = r#" + function handler(event) { + return { done: true }; + } + "#; + let result = validate_javascript(source, &default_context()); + assert!( + result.valid, + "Expected valid, got errors: {:?}", + result.errors + ); + assert!(result.is_module); + } + + #[test] + fn test_rejects_arrow_handler_with_guidance() { + let source = r#" + const handler = (event) => { + return { result: event.data }; + }; + export { handler }; + "#; + let result = validate_javascript(source, &default_context()); + assert!(!result.valid); + assert!(result.errors.iter().any(|e| { + e.error_type == "structure" + && e.message.contains("not an arrow function") + && e.message.contains("function handler(event)") + })); + } + + #[test] + fn test_accepts_custom_handler_parameter_name() { + let source = r#" + function handler(input) { + return { result: input.data }; + } + "#; + let result = validate_javascript(source, &default_context()); + assert!( + result.valid, + "Expected valid, got errors: {:?}", + result.errors + ); + } + + #[test] + fn test_accepts_handler_without_parameter() { + let source = r#" + function handler() { + return { result: "ok" }; + } + "#; + let result = validate_javascript(source, &default_context()); + assert!( + result.valid, + "Expected valid, got errors: {:?}", + result.errors + ); + } + + #[test] + fn test_rejects_nested_return_without_handler_return() { + let source = r#" + function handler(event) { + function buildResult() { + return { result: event.data }; + } + } + "#; + let result = validate_javascript(source, &default_context()); + assert!(!result.valid); + assert!( + result.errors.iter().any(|e| { + e.error_type == "structure" && e.message.contains("MUST return a value") + }) + ); + } + + #[test] + fn test_rejects_renamed_handler_function() { + let source = r#" + function handle(event) { + return { result: event.data }; + } + "#; + let result = validate_javascript(source, &default_context()); + assert!(!result.valid); + assert!(result.errors.iter().any(|e| { + e.error_type == "structure" + && e.message.contains("must be named exactly 'handler'") + && e.message.contains("Found function 'handle'") + })); + } + // ========================================================================= // Phase 4.5: Deep Method Validation Tests // ========================================================================= diff --git a/tests/analysis-guest.test.ts b/tests/analysis-guest.test.ts index f8c9553..cc5d194 100644 --- a/tests/analysis-guest.test.ts +++ b/tests/analysis-guest.test.ts @@ -170,9 +170,162 @@ describe("analysis-guest", () => { }); expect(result.valid).toBe(true); + expect(result.isModule).toBe(true); expect(result.errors).toHaveLength(0); }); + it("validateJavaScript should treat plain handler declarations as module mode", async () => { + if (!isAvailable) return; + + const source = ` + function handler(event) { + return { result: event.data }; + } + `; + + const result = await validateJavaScript(source, { + handlerName: "test-handler", + registeredHandlers: [], + availableModules: [], + expectHandler: true, + }); + + expect(result.valid).toBe(true); + expect(result.isModule).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("validateJavaScript should accept async handler declarations", async () => { + if (!isAvailable) return; + + const source = ` + async function handler(event) { + const value = await Promise.resolve(event.data); + return { result: value }; + } + `; + + const result = await validateJavaScript(source, { + handlerName: "test-handler", + registeredHandlers: [], + availableModules: [], + expectHandler: true, + }); + + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("validateJavaScript should reject arrow handlers with repair guidance", async () => { + if (!isAvailable) return; + + const source = ` + const handler = (event) => { + return { result: event.data }; + }; + export { handler }; + `; + + const result = await validateJavaScript(source, { + handlerName: "test-handler", + registeredHandlers: [], + availableModules: [], + expectHandler: true, + }); + + expect(result.valid).toBe(false); + expect(result.errors[0]?.type).toBe("structure"); + expect(result.errors[0]?.message).toContain("not an arrow function"); + expect(result.errors[0]?.message).toContain("function handler(event)"); + }); + + it("validateJavaScript should accept custom handler parameter names", async () => { + if (!isAvailable) return; + + const source = ` + function handler(input) { + return { result: input.data }; + } + `; + + const result = await validateJavaScript(source, { + handlerName: "test-handler", + registeredHandlers: [], + availableModules: [], + expectHandler: true, + }); + + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("validateJavaScript should accept handlers without parameters", async () => { + if (!isAvailable) return; + + const source = ` + function handler() { + return { result: "ok" }; + } + `; + + const result = await validateJavaScript(source, { + handlerName: "test-handler", + registeredHandlers: [], + availableModules: [], + expectHandler: true, + }); + + expect(result.valid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it("validateJavaScript should reject nested returns without handler return", async () => { + if (!isAvailable) return; + + const source = ` + function handler(event) { + function buildResult() { + return { result: event.data }; + } + } + `; + + const result = await validateJavaScript(source, { + handlerName: "test-handler", + registeredHandlers: [], + availableModules: [], + expectHandler: true, + }); + + expect(result.valid).toBe(false); + expect(result.errors[0]?.type).toBe("structure"); + expect(result.errors[0]?.message).toContain("MUST return a value"); + }); + + it("validateJavaScript should reject renamed handler functions", async () => { + if (!isAvailable) return; + + const source = ` + function handle(event) { + return { result: event.data }; + } + `; + + const result = await validateJavaScript(source, { + handlerName: "test-handler", + registeredHandlers: [], + availableModules: [], + expectHandler: true, + }); + + expect(result.valid).toBe(false); + expect(result.errors[0]?.type).toBe("structure"); + expect(result.errors[0]?.message).toContain( + "must be named exactly 'handler'", + ); + expect(result.errors[0]?.message).toContain("Found function 'handle'"); + }); + it("validateJavaScript should reject syntax errors", async () => { if (!isAvailable) return; diff --git a/tests/sandbox-tool.test.ts b/tests/sandbox-tool.test.ts index 2e31283..81cfd3d 100644 --- a/tests/sandbox-tool.test.ts +++ b/tests/sandbox-tool.test.ts @@ -256,6 +256,26 @@ describe("registerHandler", () => { expect(r.handlers).toContain("counter"); }); + it("should execute module handlers with custom parameter names", async () => { + const code = "function handler(input) { return { value: input.value }; }"; + const r = await tool.registerHandler("custom-param", code); + expect(r.success).toBe(true); + + const exec = await tool.executeJavaScript("custom-param", { value: 42 }); + expect(exec.success).toBe(true); + expect(exec.result).toEqual({ value: 42 }); + }); + + it("should execute module handlers without parameters", async () => { + const code = 'function handler() { return { value: "no-input" }; }'; + const r = await tool.registerHandler("no-param", code); + expect(r.success).toBe(true); + + const exec = await tool.executeJavaScript("no-param", { ignored: true }); + expect(exec.success).toBe(true); + expect(exec.result).toEqual({ value: "no-input" }); + }); + it("should report no-op when same name+code registered again", async () => { const r = await tool.registerHandler("calc", "return 42;"); expect(r.success).toBe(true);