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..c155911b 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 => unsafe \"schedules deferred effect\" { 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..f50da198 100644 --- a/packages/compiler/src/passes/syn-check.ts +++ b/packages/compiler/src/passes/syn-check.ts @@ -23,6 +23,21 @@ * 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. + * Excluded: member calls (`obj.setTimeout`), function declarations + * named `setTimeout`, and object/class method shorthands. */ import type { Diagnostic } from "../diagnostics.js"; @@ -44,6 +59,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: [] }; @@ -56,8 +73,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 +89,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 +593,78 @@ 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. + 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; + + // 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); + 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; + + // 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); + 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..0e771edf --- /dev/null +++ b/packages/compiler/tests/syn010-check.test.ts @@ -0,0 +1,168 @@ +/** + * 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(callback: () -> void) -> void {\n' + + " setTimeout(callback, 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); + }); + + 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 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" + + "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 7191efac..1aa2c9d7 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()` 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, " + + "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",