Apply suggestions from code review

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: simongdavies

src/js-host-api/examples/mcp-server/demo-copilot-cli.ps1

@@ -457,29 +457,6 @@ INSTRUCTIONS: You have an MCP tool called 'execute_javascript' from the 'hyperli
 
     # Write the prompt to a temp file and pass via @file to avoid PS7's
     # native-command argument mangling with multi-line here-strings.
-    # The copilot CLI reads -p @file the same as -p "string".
-    # "Nobody puts Baby in a corner." — Dirty Dancing (1987)
-    $promptFile = Join-Path ([System.IO.Path]::GetTempPath()) "hyperlight-prompt-$PID-$(Get-Random).txt"
-    $fullPrompt | Set-Content -Path $promptFile -Encoding utf8NoBOM
-
-    # Build the command args list for copilot CLI.
-    # We assemble it as an array so we can display it with -ShowCommand
-    # before actually executing it.
-    $copilotArgs = @(
-        '-p', $fullPrompt,
-        '-s',
-        '--additional-mcp-config', "@$mcpTmp",
-        '--allow-all-tools',
-        '--deny-tool', 'shell',
-        '--deny-tool', 'write',
-        '--deny-tool', 'read',
-        '--deny-tool', 'fetch',
-        '--no-custom-instructions',
-        '--no-ask-user',
-        '--disable-builtin-mcps',
-        '--model', $Model
-    )
-
     # Emit the command if -ShowCommand was requested.
     # "Show me the money!" — Jerry Maguire (1996... close enough to the 80s)
     if ($ShowCommand) {

src/js-host-api/examples/mcp-server/demo-copilot-cli.sh

@@ -82,9 +82,24 @@ separator() {
 }
 
 # Timing helpers — because "Time... is not on my side" — The Rolling Stones (1964, close enough)
-# Uses millisecond precision via date +%s%3N (GNU coreutils)
+# Uses millisecond precision when available; falls back to portable options on macOS/BSD.
 now_ms() {
-    date +%s%3N
+    # Prefer GNU date if available (system date or gdate), and verify output is numeric.
+    if date +%s%3N 2>/dev/null | grep -Eq '^[0-9]+$'; then
+        date +%s%3N
+    elif command -v gdate >/dev/null 2>&1; then
+        gdate +%s%3N
+    elif command -v python3 >/dev/null 2>&1; then
+        python3 - <<'EOF'
+import time, sys
+sys.stdout.write(str(int(time.time() * 1000)))
+EOF
+    elif command -v node >/dev/null 2>&1; then
+        node -e 'console.log(Date.now())'
+    else
+        # Portable fallback: seconds since epoch with millisecond field set to 000.
+        date +%s000
+    fi
 }
 
 # Log elapsed time since a given start timestamp (ms)
@@ -144,8 +159,7 @@ check_prerequisites() {
     info "Verifying native addon loads correctly..."
     local lib_js="${SCRIPT_DIR}/../../lib.js"
     local smoke_err
-    smoke_err="$(node --input-type=module -e "import('${lib_js}').then(() => console.log('OK')).catch(e => { console.error(e.message); process.exit(1); })" 2>&1)"
-    if [[ $? -ne 0 ]]; then
+    if ! smoke_err="$(node --input-type=module -e "import('${lib_js}').then(() => console.log('OK')).catch(e => { console.error(e.message); process.exit(1); })" 2>&1)"; then
         fail "Native addon failed to load — rebuild with 'just build'.\n${smoke_err}"
     fi
     ok "Native addon loads successfully"

src/js-host-api/examples/mcp-server/server.js

@@ -353,19 +353,59 @@ mcpServer.registerTool(
             }
         }
 
+        const safeStringifyResult = (value) => {
+            const seen = new WeakSet();
+            return JSON.stringify(
+                value,
+                (key, val) => {
+                    if (typeof val === 'bigint') {
+                        // Represent BigInt values as strings to avoid JSON.stringify throwing.
+                        return val.toString();
+                    }
+                    if (typeof val === 'object' && val !== null) {
+                        if (seen.has(val)) {
+                            // Replace circular references with a placeholder.
+                            return '[Circular]';
+                        }
+                        seen.add(val);
+                    }
+                    return val;
+                },
+                2
+            );
+        };
+
         const startTime = Date.now();
         const { success, result, error } = await executeJavaScript(code);
         const elapsed = Date.now() - startTime;
 
         if (success) {
-            return {
-                content: [
-                    {
-                        type: 'text',
-                        text: JSON.stringify(result, null, 2),
-                    },
-                ],
-            };
+            try {
+                const serialized = safeStringifyResult(result);
+                return {
+                    content: [
+                        {
+                            type: 'text',
+                            text: serialized,
+                        },
+                    ],
+                };
+            } catch (serializeError) {
+                return {
+                    content: [
+                        {
+                            type: 'text',
+                            text:
+                                `❌ Failed to serialize sandbox result: ` +
+                                (serializeError instanceof Error
+                                    ? serializeError.message
+                                    : String(serializeError)) +
+                                `\n\n(elapsed: ${elapsed}ms)`,
+                        },
+                    ],
+                    isError: true,
+                };
+            }
         } else {
             return {
                 content: [

src/js-host-api/examples/mcp-server/tests/config.test.js

@@ -28,24 +28,84 @@ function send(proc, message) {
     proc.stdin.write(JSON.stringify(message) + '\n');
 }
 
+/**
+ * Shared per-process stdout state to correctly handle NDJSON streams.
+ * Ensures that multiple messages in a single chunk are not dropped and
+ * that leftover buffered data is preserved across waitForResponse calls.
+ */
+const stdoutState = new WeakMap();
+
+function getStdoutState(proc) {
+    let state = stdoutState.get(proc);
+    if (state) return state;
+
+    state = {
+        buffer: '',
+        pendingLines: [],
+        waiters: [],
+        listening: false,
+        listener: null,
+    };
+
+    const listener = (chunk) => {
+        state.buffer += chunk.toString();
+
+        while (true) {
+            const idx = state.buffer.indexOf('\n');
+            if (idx === -1) break;
+
+            let line = state.buffer.slice(0, idx);
+            state.buffer = state.buffer.slice(idx + 1);
+
+            // Normalize Windows line endings and skip empty lines.
+            line = line.replace(/\r$/, '');
+            if (line.length === 0) continue;
+
+            if (state.waiters.length > 0) {
+                const { resolve, reject } = state.waiters.shift();
+                try {
+                    resolve(JSON.parse(line));
+                } catch (_err) {
+                    reject(new Error(`Invalid JSON from server: ${line}`));
+                }
+            } else {
+                state.pendingLines.push(line);
+            }
+        }
+    };
+
+    state.listener = listener;
+    stdoutState.set(proc, state);
+    return state;
+}
+
 function waitForResponse(proc) {
+    const state = getStdoutState(proc);
+
     return new Promise((resolve, reject) => {
-        let buffer = '';
-        const onData = (chunk) => {
-            buffer += chunk.toString();
-            const idx = buffer.indexOf('\n');
-            if (idx === -1) return;
-            const line = buffer.slice(0, idx).replace(/\r$/, '');
-            buffer = buffer.slice(idx + 1);
-            proc.stdout.off('data', onData);
-            if (line.length === 0) return;
+        const deliverFromLine = (line) => {
             try {
                 resolve(JSON.parse(line));
             } catch (_err) {
                 reject(new Error(`Invalid JSON from server: ${line}`));
             }
         };
-        proc.stdout.on('data', onData);
+
+        // If we already have a complete line buffered, use it immediately.
+        if (state.pendingLines.length > 0) {
+            const line = state.pendingLines.shift();
+            deliverFromLine(line);
+            return;
+        }
+
+        // Otherwise, enqueue this waiter and let the shared listener fulfill it.
+        state.waiters.push({ resolve, reject });
+
+        // Attach the shared listener once per process.
+        if (!state.listening) {
+            proc.stdout.on('data', state.listener);
+            state.listening = true;
+        }
     });
 }
 

src/js-host-api/examples/mcp-server/tests/prompt-examples.test.js

@@ -24,24 +24,62 @@ function send(proc, message) {
     proc.stdin.write(JSON.stringify(message) + '\n');
 }
 
+// Shared per-process line reader state: buffer, queued lines, and waiters.
+const procLineState = new WeakMap();
+
+function ensureLineReader(proc) {
+    let state = procLineState.get(proc);
+    if (state) return state;
+
+    state = {
+        buffer: '',
+        lines: [],
+        waiters: [],
+    };
+
+    const onData = (chunk) => {
+        state.buffer += chunk.toString();
+        let idx;
+        while ((idx = state.buffer.indexOf('\n')) !== -1) {
+            let line = state.buffer.slice(0, idx).replace(/\r$/, '');
+            state.buffer = state.buffer.slice(idx + 1);
+            if (line.length === 0) {
+                continue;
+            }
+
+            if (state.waiters.length > 0) {
+                const { resolve, reject } = state.waiters.shift();
+                try {
+                    resolve(JSON.parse(line));
+                } catch (_err) {
+                    reject(new Error(`Invalid JSON from server: ${line}`));
+                }
+            } else {
+                state.lines.push(line);
+            }
+        }
+    };
+
+    proc.stdout.on('data', onData);
+    procLineState.set(proc, state);
+    return state;
+}
+
 function waitForResponse(proc) {
     return new Promise((resolve, reject) => {
-        let buffer = '';
-        const onData = (chunk) => {
-            buffer += chunk.toString();
-            const idx = buffer.indexOf('\n');
-            if (idx === -1) return;
-            const line = buffer.slice(0, idx).replace(/\r$/, '');
-            buffer = buffer.slice(idx + 1);
-            proc.stdout.off('data', onData);
-            if (line.length === 0) return;
+        const state = ensureLineReader(proc);
+
+        if (state.lines.length > 0) {
+            const line = state.lines.shift();
             try {
                 resolve(JSON.parse(line));
             } catch (_err) {
                 reject(new Error(`Invalid JSON from server: ${line}`));
             }
-        };
-        proc.stdout.on('data', onData);
+            return;
+        }
+
+        state.waiters.push({ resolve, reject });
     });
 }
 

src/js-host-api/examples/mcp-server/tests/timing.test.js

@@ -28,24 +28,56 @@ function send(proc, message) {
     proc.stdin.write(JSON.stringify(message) + '\n');
 }
 
+// Shared per-process NDJSON line reader state.
+const ndjsonState = new WeakMap();
+
+function getNdjsonState(proc) {
+    let state = ndjsonState.get(proc);
+    if (state) return state;
+
+    state = {
+        buffer: '',
+        queue: [],
+        waiting: [],
+    };
+
+    const onData = (chunk) => {
+        state.buffer += chunk.toString();
+        let idx;
+        // Extract all complete lines currently in the buffer.
+        while ((idx = state.buffer.indexOf('\n')) !== -1) {
+            let line = state.buffer.slice(0, idx).replace(/\r$/, '');
+            state.buffer = state.buffer.slice(idx + 1);
+            if (line.length === 0) continue;
+            state.queue.push(line);
+        }
+        dispatchNdjson(state);
+    };
+
+    proc.stdout.on('data', onData);
+    ndjsonState.set(proc, state);
+    return state;
+}
+
+function dispatchNdjson(state) {
+    // Pair up queued lines with waiting promises in FIFO order.
+    while (state.queue.length > 0 && state.waiting.length > 0) {
+        const line = state.queue.shift();
+        const { resolve, reject } = state.waiting.shift();
+        try {
+            resolve(JSON.parse(line));
+        } catch (_err) {
+            reject(new Error(`Invalid JSON from server: ${line}`));
+        }
+    }
+}
+
 function waitForResponse(proc) {
+    const state = getNdjsonState(proc);
     return new Promise((resolve, reject) => {
-        let buffer = '';
-        const onData = (chunk) => {
-            buffer += chunk.toString();
-            const idx = buffer.indexOf('\n');
-            if (idx === -1) return;
-            const line = buffer.slice(0, idx).replace(/\r$/, '');
-            buffer = buffer.slice(idx + 1);
-            proc.stdout.off('data', onData);
-            if (line.length === 0) return;
-            try {
-                resolve(JSON.parse(line));
-            } catch (_err) {
-                reject(new Error(`Invalid JSON from server: ${line}`));
-            }
-        };
-        proc.stdout.on('data', onData);
+        state.waiting.push({ resolve, reject });
+        // In case lines were already queued before this call.
+        dispatchNdjson(state);
     });
 }