From 5b83b40fee1147373c15ccaf1c784b65ab58fec6 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 03:43:24 -0300 Subject: [PATCH 1/4] =?UTF-8?q?feat(compiler):=20SYN010=20=E2=80=94=20warn?= =?UTF-8?q?=20on=20setTimeout/setInterval/queueMicrotask=20calls=20in=20fn?= =?UTF-8?q?=20bodies=20(=3Fbs=200.7+)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Adds **SYN010**: a new warning that fires when a fn body calls `setTimeout(...)`, `setInterval(...)`, or `queueMicrotask(...)` at `?bs 0.7+` - These globals schedule callbacks that run after the fn returns — any effects inside those callbacks are invisible to callers: no capability declaration, no `writes {}` label, and no `throws {}` entry can cover them - Same bypass class as `fetch` (SYN007) and `WebSocket` (SYN008): real side effects that sidestep the declared capability surface, but deferred rather than immediate - Suppressed inside `unsafe "reason" { }` blocks and `unsafe "reason" fn` bodies - Member calls (`obj.setTimeout(...)`) and bare references (without `(`) are excluded - Note: should be merged after PR #150 (SYN009) lands; SYN010 is the next available code Changes: - error-codes.ts: add SYN010 entry with rule/idiom/rewrite/example - syn-check.ts: add SYN010 detection; add SYN006 + SYN010 to module header comment; fix outdated unsafe-fn skip comment - tests/syn010-check.test.ts: 12 tests covering all three globals, optional-call form, severity, version gating, unsafe suppression, member-call exclusion, bare reference - tests/error-codes.test.ts: add SYN010 to exhaustive allowlist - mcp/src/explanations.ts: add SYN010 long-form explanation with fails/passes examples - mcp/tests/server.test.ts: add SYN010 to KNOWN_CODES list - AGENTS.md: add SYN010 row to diagnostic table - README.md: add SYN010 to explain tool code list --- AGENTS.md | 1 + README.md | 2 +- packages/compiler/src/error-codes.ts | 34 +++++ packages/compiler/src/passes/syn-check.ts | 74 +++++++++- packages/compiler/tests/error-codes.test.ts | 2 +- packages/compiler/tests/syn010-check.test.ts | 138 +++++++++++++++++++ packages/mcp/src/explanations.ts | 38 +++++ packages/mcp/tests/server.test.ts | 1 + 8 files changed, 286 insertions(+), 4 deletions(-) create mode 100644 packages/compiler/tests/syn010-check.test.ts diff --git a/AGENTS.md b/AGENTS.md index c3cf4214..17e7d2e4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -201,6 +201,7 @@ parse the resulting `{ ok: false, diagnostics: [...] }` envelope. | SYN004 | (0.7+, warning) A fn body calls `eval(...)` / `eval?.(...)` (global eval not preceded by `.`/`?.`) or calls `Function(...)` / `Function?.(...)` / `new Function(...)` (Function constructor not preceded by `.`/`?.`). All forms execute strings as code at runtime — every static capability check (CAP001/CAP002), resource declaration (reads/writes), and safety check (SYN002/SYN003) can be bypassed by routing any unsafe pattern through eval or the Function constructor. Suppressed inside `unsafe {}` blocks and `unsafe fn` bodies. `.eval(...)` (method call on a local) and `Function.*` member accesses are excluded. | Refactor the eval-based pattern to use explicit code paths. If eval is genuinely required (e.g. sandboxed interpreter), wrap in `unsafe "" { eval(...) }`. | | SYN005 | (0.7+, warning) A fn body accesses `process.env`. `process.env` is a global deployment-environment namespace — access is invisible to callers and to static analysis; no capability or resource declaration covers it, so the fn has an undeclared dependency on deployment configuration. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `env`. `obj.process.env` (member access on a local), `unsafe {}` blocks, and `unsafe "reason" fn` bodies are excluded. | Pass config and secrets as explicit fn parameters so the dependency is visible in the call signature; if env access is required at the load site, wrap in `unsafe "reads deployment env" { }`. | | SYN006 | (0.7+, warning) A fn body calls `process.exit()`, `process?.exit()`, or `process.exit?.()`. All forms terminate the entire host process — not just the fn, not just the bot. They produce no return value, bypass `Result` propagation, `throws {}`, `match`, and any caller recovery path. No capability declaration covers them. Detection: `process` not preceded by `.`/`?.`, followed by `.`/`?.` then `exit` then `(` or `?.(`. `obj.process.exit(...)`, `process.exit` without `(`, and `process.exitCode` are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Return `err(...)` and let the caller decide whether to terminate. If `process.exit` is genuinely required at a bootstrap entry point, wrap in `unsafe "exits on invalid config" { process.exit(1) }`. | +| SYN010 | (0.7+, warning) A fn body calls `setTimeout(...)`, `setInterval(...)`, or `queueMicrotask(...)`. These globals schedule callbacks that run after the fn returns — any effects inside those callbacks are invisible to callers: no capability declaration, no `writes {}` label, and no `throws {}` entry can cover them. Detection: identifier not preceded by `.`/`?.`, followed by `(` or `?.(`. Member calls (`obj.setTimeout(...)`) and bare references (without `(`) are excluded. `unsafe {}` blocks and `unsafe "reason" fn` bodies are suppressed. | Make the timing explicit: return a `Promise` the caller awaits, or return a teardown function so the caller controls the lifecycle. If a timer is genuinely required, wrap in `unsafe "schedules deferred effect" { setTimeout(...) }`. | | INT002 | (0.7+) A fn declares `intent: "pure"` but its body directly references a stdlib capability (e.g. `http.get`, `fs.read`). Pure intent is enforced at the body level as well as the header. | Remove the stdlib call from the body, or change the intent. | | INT003 | (0.7+) A fn declares `intent: "idempotent"` but also has `uses { random }` or `uses { time }`. Both capabilities produce different values on each call, making the function non-idempotent. Only `random` and `time` are flagged; other capabilities are not structurally flagged by this check (INT003 is a narrow heuristic, not a proof of idempotence). | Remove `random`/`time` from `uses {}`, or change the intent. | | INT004 | (0.7+) A fn declares `intent: "idempotent"` but its body directly references `random` or `time` without declaring them. Under-declaration variant of INT003 — fires when INT003 does not. | Remove the non-idempotent call from the body, or declare the capability and remove the idempotent intent. | diff --git a/README.md b/README.md index 2ce0d9b1..2901cca9 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ claude mcp add botscript -- npx -y @mbfarias/botscript-mcp | ----------- | -------------------------------------- | --------------------------------------------------------------------------------------------------- | | `primer` | (no args) | The canonical language primer (same text the `?primer` directive emits). | | `transform` | `{ source: string, filename?: string }` | `{ ok: true, code, forms, version, warnings: [...] }` on success, or `{ ok: false, diagnostics: [...] }` on failure. `warnings` is an array of non-blocking diagnostics (e.g. CAP003). | -| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN004`, `SYN005`, `SYN006`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | +| `explain` | `{ code: string }` | Long-form explanation for any stable diagnostic code (`ALI001`, `ALI002`, `ALI003`, `BS001`, `BS002`, `CAP001`–`CAP003`, `DEP001`–`DEP004`, `EFF002`–`EFF004`, `FMT001`, `INT001`–`INT005`, `MAT001`–`MAT004`, `RES001`, `RES002`, `SYN001`, `SYN002`, `SYN003`, `SYN004`, `SYN005`, `SYN006`, `SYN010`, `THR001`–`THR004`, `UNS001`–`UNS005`, `VER001`–`VER003`) plus a fails/passes example pair. | A bot's loop becomes deterministic: `transform` → if `ok=false`, read `diagnostics[0].code` → `explain(code)` → apply `rewrite` → `transform` again. diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index a1c7c21a..dfa9dcb0 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -642,6 +642,40 @@ const E: Record = { " return ok(undefined)\n" + "}", }, + SYN010: { + code: "SYN010", + title: "setTimeout / setInterval / queueMicrotask defers side effects outside the fn's capability surface", + rule: + "`setTimeout(fn, ms)`, `setInterval(fn, ms)`, and `queueMicrotask(fn)` schedule callbacks that run " + + "after the current fn returns — any effects inside those callbacks are invisible to the caller: " + + "no capability declaration, no `writes {}` label, no `throws {}` entry can reflect them. " + + "Callers see a fn that returns normally; the real work happens later, in a different call frame, " + + "with no signal in the fn header.", + idiom: + "pass the delay and callback to the caller as a return value so the timing is visible (e.g. return a Promise " + + "the caller awaits); if a timer is genuinely required here, wrap in " + + "`unsafe \"schedules deferred effect\" { setTimeout(...) }`", + rewrite: + "// before — deferred effect invisible to callers\n" + + "fn scheduleRetry(fn: () -> void, ms: number) -> void {\n" + + " setTimeout(fn, ms) // SYN010\n" + + "}\n\n" + + "// after — caller controls the timing\n" + + "async fn scheduleRetry(fn: () -> void, ms: number) -> Promise {\n" + + " await new Promise(resolve => setTimeout(resolve, ms))\n" + + " fn()\n" + + "}", + example: + "// SYN010: deferred callback hides a network effect from callers\n" + + "fn pollStatus(url: string) uses { net } -> void {\n" + + " setInterval(() => http.get(url), 5000) // SYN010\n" + + "}\n\n" + + "// fix: return a teardown fn so the polling is visible at the call site\n" + + "fn pollStatus(url: string) uses { net } -> () -> void {\n" + + " const id = setInterval(() => http.get(url), 5000)\n" + + " return () => clearInterval(id)\n" + + "}", + }, DEP001: { code: "DEP001", title: "fn transitively reads a resource category not declared in its header", diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index 6e0320bd..e3ba666a 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -23,6 +23,19 @@ * depends on runtime deployment values that callers cannot see, * audit, or mock in tests. The idiomatic fix is to pass config * and secrets as explicit fn parameters. + * + * SYN006 A `process.exit()` call was detected in a fn body (?bs 0.7+). + * `process.exit()` terminates the entire host process — not just the + * fn, not just the bot. It produces no return value and bypasses + * Result propagation, throws {}, match, and any caller recovery + * path. The idiomatic fix is `return err(...)` so the caller can + * decide whether to terminate. + * + * SYN010 A `setTimeout(...)`, `setInterval(...)`, or `queueMicrotask(...)` + * call was detected in a fn body (?bs 0.7+). These globals schedule + * callbacks to run after the current fn returns — any effects inside + * those callbacks are invisible to callers: no capability declaration, + * no `writes {}` label, and no `throws {}` entry covers them. */ import type { Diagnostic } from "../diagnostics.js"; @@ -56,8 +69,9 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const syn004 = getErrorCode("SYN004")!; const syn005 = getErrorCode("SYN005")!; const syn006 = getErrorCode("SYN006")!; + const syn010 = getErrorCode("SYN010")!; - // Collect char-offset ranges where SYN002/SYN003/SYN004/SYN005/SYN006 are suppressed: + // Collect char-offset ranges where all SYN checks are suppressed: // 1. `unsafe "reason" { ... }` expression blocks — explicit acknowledgment. // 2. `unsafe "reason" fn` bodies — the entire body is exempt, including any // non-unsafe nested fns declared inside it (matching uns-check's pattern). @@ -71,7 +85,7 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult const nesting = computeNesting(program.fns.map((f) => f.decl)); for (const { decl } of program.fns) { - // An `unsafe "reason" fn` body is an explicit acknowledgment — skip SYN002/SYN003/SYN004/SYN005. + // An `unsafe "reason" fn` body is an explicit acknowledgment — all SYN checks are skipped. // The range-based suppression above also covers nested non-unsafe fns within it, // so this early-continue is kept purely as an optimisation. if (decl.unsafeReason !== undefined) continue; @@ -575,6 +589,62 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult rewrite: syn006.rewrite, }); } + + // SYN010: setTimeout / setInterval / queueMicrotask detection. + // Fires when a fn body calls any of these timer/microtask globals, which schedule + // callbacks to run after the fn returns. Any side effects inside those callbacks are + // invisible to callers: no capability, writes {}, or throws {} can reflect them. + // Suppressed inside `unsafe "reason" { }` blocks and `unsafe "reason" fn` bodies. + const TIMER_GLOBALS = new Set(["setTimeout", "setInterval", "queueMicrotask"]); + nextInner = 0; + const open010: typeof inner = []; + for (let i = bodyStart; i < decl.tokenEnd; i++) { + while (open010.length > 0 && open010[open010.length - 1]!.tokenEnd <= i) open010.pop(); + while (nextInner < inner.length && inner[nextInner]!.tokenStart <= i) { + open010.push(inner[nextInner]!); + nextInner++; + } + if (open010.length > 0) continue; + + const tok10 = tokens[i]; + if (!tok10 || tok10.kind !== "ident" || !TIMER_GLOBALS.has(tok10.text)) continue; + + // Exclude property accesses: obj.setTimeout(...) + const prevIdx10 = prevSignificant(tokens, i - 1); + const prev10 = tokens[prevIdx10]; + if (prev10 && ((prev10.kind === "punct" && prev10.text === ".") || prev10.kind === "questionDot")) + continue; + + // Must be followed by `(` or `?.(` — confirming this is a call, not a reference. + let afterIdx10 = nextSignificant(tokens, i + 1); + let afterTok10 = tokens[afterIdx10]; + if (afterTok10 && afterTok10.kind === "questionDot") { + afterIdx10 = nextSignificant(tokens, afterIdx10 + 1); + afterTok10 = tokens[afterIdx10]; + } + if (!afterTok10 || !(afterTok10.kind === "open" && afterTok10.text === "(")) continue; + + if (isInsideRange(tok10.start, unsafeRanges)) continue; + + const loc10 = locationOf(src, tok10.start); + warnings.push({ + code: "SYN010", + severity: "warning", + file: null, + line: loc10.line, + column: loc10.column, + start: tok10.start, + end: tok10.end, + message: + `fn '${decl.name}' calls ${tok10.text}() — ` + + `${tok10.text} schedules a callback that runs after the fn returns; ` + + `any effects inside that callback are invisible to callers and cannot be declared in the fn header; ` + + `wrap in unsafe "schedules deferred effect" { ${tok10.text}(...) }`, + rule: syn010.rule, + idiom: syn010.idiom, + rewrite: syn010.rewrite, + }); + } } return { code: src, warnings }; diff --git a/packages/compiler/tests/error-codes.test.ts b/packages/compiler/tests/error-codes.test.ts index 5b57aae2..b2f61184 100644 --- a/packages/compiler/tests/error-codes.test.ts +++ b/packages/compiler/tests/error-codes.test.ts @@ -15,7 +15,7 @@ describe("error-code registry", () => { "INT001", "INT002", "INT003", "INT004", "INT005", "MAT001", "MAT002", "MAT003", "MAT004", "RES001", "RES002", - "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", + "SYN001", "SYN002", "SYN003", "SYN004", "SYN005", "SYN006", "SYN010", "THR001", "THR002", "THR003", "THR004", "UNS001", "UNS002", "UNS003", "UNS004", "UNS005", "VER001", "VER002", "VER003", diff --git a/packages/compiler/tests/syn010-check.test.ts b/packages/compiler/tests/syn010-check.test.ts new file mode 100644 index 00000000..738a76d3 --- /dev/null +++ b/packages/compiler/tests/syn010-check.test.ts @@ -0,0 +1,138 @@ +/** + * Tests for SYN010: setTimeout / setInterval / queueMicrotask call detection (?bs 0.7+). + * + * SYN010 is a non-blocking warning — transform must not throw. + */ + +import { describe, it, expect } from "vitest"; +import { transform } from "../src/index.js"; + +function compile(src: string) { + return transform(src, {}); +} + +describe("SYN010: timer / microtask global call detection", () => { + it("fires on setTimeout() inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("fires on setInterval() inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn poll(fn: () -> void) -> void {\n" + + " setInterval(fn, 5000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("fires on queueMicrotask() inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn defer(fn: () -> void) -> void {\n" + + " queueMicrotask(fn)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("fires on optional-call form setTimeout?.()", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout?.(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(true); + }); + + it("produces a warning-severity diagnostic", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN010"); + expect(w?.severity).toBe("warning"); + }); + + it("diagnostic message names the timer global", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setInterval(fn, 5000)\n" + + "}\n"; + const result = compile(src); + const w = result.warnings.find((w) => w.code === "SYN010"); + expect(w?.message).toContain("setInterval"); + }); + + it("does NOT fire below ?bs 0.7", () => { + const src = + "?bs 0.6\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + " setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire inside an unsafe block", () => { + const src = + "?bs 0.7\n" + + "fn scheduleRetry(fn: () -> void) -> void {\n" + + ' unsafe "schedules deferred effect" { setTimeout(fn, 1000) }\n' + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire inside an unsafe fn body", () => { + const src = + "?bs 0.7\n" + + 'unsafe "uses timers" fn scheduleRetry(fn: () -> void) -> void {\n' + + " setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire on obj.setTimeout() (member call on a local)", () => { + const src = + "?bs 0.7\n" + + "fn test(ctx: any) -> void {\n" + + " ctx.setTimeout(fn, 1000)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire on bare setTimeout reference (not called)", () => { + const src = + "?bs 0.7\n" + + "fn test() -> any {\n" + + " return setTimeout\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("fires once per distinct timer call", () => { + const src = + "?bs 0.7\n" + + "fn scheduleAll(fn: () -> void) -> void {\n" + + " setTimeout(fn, 100)\n" + + " setInterval(fn, 1000)\n" + + " queueMicrotask(fn)\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.filter((w) => w.code === "SYN010").length).toBe(3); + }); +}); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 7191efac..69ebba91 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -849,6 +849,44 @@ export const EXPLANATIONS: Readonly> = { "}\n", }, }, + SYN010: { + code: "SYN010", + title: "setTimeout / setInterval / queueMicrotask defers side effects outside the fn's capability surface", + body: + "Botscript's capability model is static: the compiler reads the fn header and infers what that fn " + + "may do — network access, resource reads/writes, error types. Timer and microtask globals break this " + + "contract by scheduling work to run *after* the fn returns.\n\n" + + "When a fn calls `setTimeout(() => http.get(url), 5000)`, it has a live network dependency — but " + + "that dependency runs in a callback that fires in a future event-loop tick. The fn returns `void` " + + "(or a timer ID) immediately. No `uses { net }` declaration in the header can cover it, because the " + + "capability lives in the deferred callback, not in the fn's direct call graph.\n\n" + + "The impact in bot code is concrete:\n" + + "- `setTimeout(() => db.write(state), 0)` — a write that runs after the fn returns; callers cannot observe it\n" + + "- `setInterval(() => pollApi(), 60_000)` — a recurring effect started by the fn; the caller has no teardown handle\n" + + "- `queueMicrotask(() => emitEvent())` — a microtask-queued side effect, invisible to the static analysis\n\n" + + "This is the same bypass class as `fetch` (SYN007) and `WebSocket` (SYN008): real effects that " + + "sidestep the declared capability surface, but deferred rather than immediate.\n\n" + + "**Fix:** make the timing explicit. If the fn needs to delay work, return a `Promise` the caller " + + "awaits, or return a teardown function the caller can control. If a timer is genuinely required, " + + "wrap in `unsafe \"schedules deferred effect\" { setTimeout(...) }` to make the escape hatch visible.\n\n" + + "SYN010 fires at `?bs 0.7+` as a non-blocking warning. Detection is token-based: `setTimeout`, " + + "`setInterval`, or `queueMicrotask` not preceded by `.`/`?.`, followed by `(` or `?.(`. " + + "Member calls (`obj.setTimeout(...)`) are excluded. " + + "Calls inside `unsafe { }` blocks or `unsafe \"reason\" fn` bodies are suppressed.", + example: { + fails: + "?bs 0.7\n" + + "fn pollStatus(url: string) uses { net } -> void {\n" + + " setInterval(() => http.get(url), 5000)\n" + + "}\n", + passes: + "?bs 0.7\n" + + "fn pollStatus(url: string) uses { net } -> () -> void {\n" + + " const id = unsafe \"schedules polling\" { setInterval(() => http.get(url), 5000) }\n" + + " return () => clearInterval(id)\n" + + "}\n", + }, + }, DEP001: { code: "DEP001", title: "fn transitively reads a resource category not declared in its header", diff --git a/packages/mcp/tests/server.test.ts b/packages/mcp/tests/server.test.ts index 4f4d3622..41991681 100644 --- a/packages/mcp/tests/server.test.ts +++ b/packages/mcp/tests/server.test.ts @@ -78,6 +78,7 @@ describe("botscript-mcp explanations", () => { "SYN004", "SYN005", "SYN006", + "SYN010", "THR001", "THR002", "THR003", From f10d9021c0ea05391306e92dfbe5acc9bad5a47d Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 11:27:57 -0300 Subject: [PATCH 2/4] =?UTF-8?q?fix(syn010):=20address=20Copilot=20review?= =?UTF-8?q?=20=E2=80=94=20hoist=20TIMER=5FGLOBALS,=20fix=20param=20name=20?= =?UTF-8?q?and=20reason=20string?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Hoist TIMER_GLOBALS to module scope (was re-created per fn, wasteful in hot pass) - Rename `fn` parameter to `callback` in unsafe-fn test (fn is a reserved keyword) - Fix unsafe reason string: "suspends execution" → "schedules deferred effect" (consistent with idiom) Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/error-codes.ts | 2 +- packages/compiler/src/passes/syn-check.ts | 3 ++- packages/compiler/tests/syn010-check.test.ts | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/error-codes.ts b/packages/compiler/src/error-codes.ts index dfa9dcb0..c155911b 100644 --- a/packages/compiler/src/error-codes.ts +++ b/packages/compiler/src/error-codes.ts @@ -662,7 +662,7 @@ const E: Record = { "}\n\n" + "// after — caller controls the timing\n" + "async fn scheduleRetry(fn: () -> void, ms: number) -> Promise {\n" + - " await new Promise(resolve => setTimeout(resolve, ms))\n" + + " await new Promise(resolve => unsafe \"schedules deferred effect\" { setTimeout(resolve, ms) })\n" + " fn()\n" + "}", example: diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index e3ba666a..dc5e0691 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -57,6 +57,8 @@ const CONSOLE_OUTPUT_METHODS = new Set([ "table", "trace", "group", "groupCollapsed", "groupEnd", ]); +const TIMER_GLOBALS = new Set(["setTimeout", "setInterval", "queueMicrotask"]); + export function passSynCheck(src: string, version: VersionInfo): SynCheckResult { if (!atLeast(version.resolved, "0.7")) return { code: src, warnings: [] }; @@ -595,7 +597,6 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult // callbacks to run after the fn returns. Any side effects inside those callbacks are // invisible to callers: no capability, writes {}, or throws {} can reflect them. // Suppressed inside `unsafe "reason" { }` blocks and `unsafe "reason" fn` bodies. - const TIMER_GLOBALS = new Set(["setTimeout", "setInterval", "queueMicrotask"]); nextInner = 0; const open010: typeof inner = []; for (let i = bodyStart; i < decl.tokenEnd; i++) { diff --git a/packages/compiler/tests/syn010-check.test.ts b/packages/compiler/tests/syn010-check.test.ts index 738a76d3..0660c2e3 100644 --- a/packages/compiler/tests/syn010-check.test.ts +++ b/packages/compiler/tests/syn010-check.test.ts @@ -97,8 +97,8 @@ describe("SYN010: timer / microtask global call detection", () => { it("does NOT fire inside an unsafe fn body", () => { const src = "?bs 0.7\n" + - 'unsafe "uses timers" fn scheduleRetry(fn: () -> void) -> void {\n' + - " setTimeout(fn, 1000)\n" + + 'unsafe "uses timers" fn scheduleRetry(callback: () -> void) -> void {\n' + + " setTimeout(callback, 1000)\n" + "}\n"; const result = compile(src); expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); From e0a2cd9c8e084c4c995803654956eb9147affbb6 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 15:31:29 -0300 Subject: [PATCH 3/4] =?UTF-8?q?fix(syn010):=20address=20remaining=20Copilo?= =?UTF-8?q?t=20feedback=20=E2=80=94=20false=20positive=20on=20declarations?= =?UTF-8?q?=20and=20explanation=20codes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - syn-check: exclude function declarations named setTimeout/setInterval/queueMicrotask (prev significant token is `function`) — these are definitions, not calls - syn-check: exclude object/class method shorthands where after closing `)` is `{` or `:` - tests: add two tests covering function-declaration and object-method-shorthand exclusions - explanations: remove SYN007/SYN008 code references in SYN010 body; reference by name instead to avoid dangling code cross-references until those PRs land Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/syn-check.ts | 18 ++++++++++++++++++ packages/compiler/tests/syn010-check.test.ts | 20 ++++++++++++++++++++ packages/mcp/src/explanations.ts | 2 +- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index dc5e0691..a5ab174f 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -36,6 +36,8 @@ * callbacks to run after the current fn returns — any effects inside * those callbacks are invisible to callers: no capability declaration, * no `writes {}` label, and no `throws {}` entry covers them. + * Excluded: member calls (`obj.setTimeout`), function declarations + * named `setTimeout`, and object/class method shorthands. */ import type { Diagnostic } from "../diagnostics.js"; @@ -616,6 +618,9 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult if (prev10 && ((prev10.kind === "punct" && prev10.text === ".") || prev10.kind === "questionDot")) continue; + // Exclude function declarations: function setTimeout(fn, ms) {} + if (prev10 && prev10.kind === "ident" && prev10.text === "function") continue; + // Must be followed by `(` or `?.(` — confirming this is a call, not a reference. let afterIdx10 = nextSignificant(tokens, i + 1); let afterTok10 = tokens[afterIdx10]; @@ -625,6 +630,19 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult } if (!afterTok10 || !(afterTok10.kind === "open" && afterTok10.text === "(")) continue; + // Exclude method shorthands and class methods: { setTimeout(fn) { ... } } + // When after the closing `)` is `{` (method body) or `:` (return type), it's a definition. + const closeParenIdx10 = afterTok10.matchedAt; + if (closeParenIdx10 !== undefined) { + const afterParenIdx10 = nextSignificant(tokens, closeParenIdx10 + 1); + const afterParen10 = tokens[afterParenIdx10]; + if ( + afterParen10 && + ((afterParen10.kind === "open" && afterParen10.text === "{") || + (afterParen10.kind === "punct" && afterParen10.text === ":")) + ) continue; + } + if (isInsideRange(tok10.start, unsafeRanges)) continue; const loc10 = locationOf(src, tok10.start); diff --git a/packages/compiler/tests/syn010-check.test.ts b/packages/compiler/tests/syn010-check.test.ts index 0660c2e3..739a28b7 100644 --- a/packages/compiler/tests/syn010-check.test.ts +++ b/packages/compiler/tests/syn010-check.test.ts @@ -135,4 +135,24 @@ describe("SYN010: timer / microtask global call detection", () => { const result = compile(src); expect(result.warnings.filter((w) => w.code === "SYN010").length).toBe(3); }); + + it("does NOT fire on function declarations named setTimeout inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn test(callback: () -> void, ms: number) -> void {\n" + + " function setTimeout(cb: () -> void, delay: number) -> void {}\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + + it("does NOT fire on object method shorthands named setTimeout", () => { + const src = + "?bs 0.7\n" + + "fn test(callback: () -> void) -> any {\n" + + " return { setTimeout(cb: () -> void, ms: number) { callback() } }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); }); diff --git a/packages/mcp/src/explanations.ts b/packages/mcp/src/explanations.ts index 69ebba91..1aa2c9d7 100644 --- a/packages/mcp/src/explanations.ts +++ b/packages/mcp/src/explanations.ts @@ -864,7 +864,7 @@ export const EXPLANATIONS: Readonly> = { "- `setTimeout(() => db.write(state), 0)` — a write that runs after the fn returns; callers cannot observe it\n" + "- `setInterval(() => pollApi(), 60_000)` — a recurring effect started by the fn; the caller has no teardown handle\n" + "- `queueMicrotask(() => emitEvent())` — a microtask-queued side effect, invisible to the static analysis\n\n" + - "This is the same bypass class as `fetch` (SYN007) and `WebSocket` (SYN008): real effects that " + + "This is the same bypass class as `fetch()` and `WebSocket` globals: real effects that " + "sidestep the declared capability surface, but deferred rather than immediate.\n\n" + "**Fix:** make the timing explicit. If the fn needs to delay work, return a `Promise` the caller " + "awaits, or return a teardown function the caller can control. If a timer is genuinely required, " + From 6847365c37f3c2439135427da636dc219830ee08 Mon Sep 17 00:00:00 2001 From: Marcelo Farias Date: Thu, 11 Jun 2026 19:41:15 -0300 Subject: [PATCH 4/4] =?UTF-8?q?fix(compiler):=20address=20SYN010=20Copilot?= =?UTF-8?q?=20review=20feedback=20=E2=80=94=20exclude=20fn=20keyword=20dec?= =?UTF-8?q?larations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - syn-check.ts: also skip `fn setTimeout(...) -> void {}` botscript-style nested function declarations (prev token is `keyword`/`fn`), not just JS `function` declarations — prevents false positives on nested fn declarations inside fn bodies - syn010-check.test.ts: add test asserting SYN010 does NOT fire on `fn setTimeout(delay: number) -> void { }` inside a fn body Co-Authored-By: Claude Sonnet 4.6 --- packages/compiler/src/passes/syn-check.ts | 3 ++- packages/compiler/tests/syn010-check.test.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/passes/syn-check.ts b/packages/compiler/src/passes/syn-check.ts index a5ab174f..f50da198 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -618,8 +618,9 @@ export function passSynCheck(src: string, version: VersionInfo): SynCheckResult if (prev10 && ((prev10.kind === "punct" && prev10.text === ".") || prev10.kind === "questionDot")) continue; - // Exclude function declarations: function setTimeout(fn, ms) {} + // Exclude function declarations: function setTimeout(fn, ms) {} or fn setTimeout(...) -> void {} if (prev10 && prev10.kind === "ident" && prev10.text === "function") continue; + if (prev10 && prev10.kind === "keyword" && prev10.text === "fn") continue; // Must be followed by `(` or `?.(` — confirming this is a call, not a reference. let afterIdx10 = nextSignificant(tokens, i + 1); diff --git a/packages/compiler/tests/syn010-check.test.ts b/packages/compiler/tests/syn010-check.test.ts index 739a28b7..0e771edf 100644 --- a/packages/compiler/tests/syn010-check.test.ts +++ b/packages/compiler/tests/syn010-check.test.ts @@ -146,6 +146,16 @@ describe("SYN010: timer / microtask global call detection", () => { expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); }); + it("does NOT fire on botscript fn declarations named setTimeout inside a fn body", () => { + const src = + "?bs 0.7\n" + + "fn test(callback: () -> void, ms: number) -> void {\n" + + " fn setTimeout(delay: number) -> void { }\n" + + "}\n"; + const result = compile(src); + expect(result.warnings.some((w) => w.code === "SYN010")).toBe(false); + }); + it("does NOT fire on object method shorthands named setTimeout", () => { const src = "?bs 0.7\n" +