fix(sdk): read helper references as syntax, and drop the surface peer override

packages/sdk/package.json

@@ -51,6 +51,7 @@
     "@relayfile/relay-helpers": "0.4.12",
     "@relayflows/surface": "2.0.30",
     "@types/js-yaml": "^4.0.9",
+    "acorn": "^8.18.0",
     "ai-hist": "0.4.1",
     "ajv": "^8.17.1",
     "ajv-draft-04": "^1.0.0",
@@ -80,10 +81,5 @@
     "@agent-relay/sdk": {
       "optional": true
     }
-  },
-  "overrides": {
-    "@relayflows/surface": {
-      "@relayfile/relay-helpers": "$@relayfile/relay-helpers"
-    }
   }
 }

packages/sdk/src/flow-requirements.ts

@@ -4,6 +4,7 @@ import type { TriggerSource } from '@relayflows/surface';
 import { providerDeclaration } from './provider-trigger-contract.js';
 import type { FlowSpec } from './spec.js';
 import { helperCall } from './yaml-helpers.js';
+import { helperNamespacesUsed } from './helper-reference.js';
 
 /**
  * What a flow needs from the workspace it deploys into, read from inert
@@ -155,10 +156,14 @@ export function flowRequirements(
     // payload as input, and handler bodies are not dispatched yet (flows #301).
     // A handler's *trigger* is still a requirement — it is what wakes the flow.
     const text = typeof flow.body === 'function' ? Function.prototype.toString.call(flow.body) : '';
-    const root = contextParameter(text);
-    if (root !== undefined) {
+    const rootName = contextParameter(text);
+    const root = rootName === undefined ? undefined : escapeRegExp(rootName);
+    if (root !== undefined && rootName !== undefined) {
+      // The AST walk compares against an Identifier name, so it needs the raw
+      // parameter; the regex scanners below need the escaped one.
+      const referenced = helperNamespacesUsed(text, rootName);
       for (const { provider, namespace } of helperProviders) {
-        if (helperReference(root, namespace).test(text)) declare({ provider, from: 'helper', detail: `f.${namespace}` });
+        if (referenced.has(namespace)) declare({ provider, from: 'helper', detail: `f.${namespace}` });
       }
       for (const use of workerCalls(root, text)) need(use.cli === undefined ? fallback : harnessFromCli(use.cli), use.detail);
       // `f.human(q, { to: "slack:#eng" })` is delivered by Cloud through that
@@ -219,15 +224,25 @@ function stringList(value: unknown): string[] {
   return Array.isArray(value) ? value.filter((entry): entry is string => typeof entry === 'string') : [];
 }
 
-/** The body's first parameter (`f` in `async (f, input) => …`), escaped for a pattern. */
+/**
+ * The body's first parameter (`f` in `async (f, input) => …`), RAW.
+ *
+ * Returned unescaped because the AST walk compares it to an `Identifier`
+ * name: escaping turned a legal parameter like `f$` into `f\$`, which matches
+ * no identifier and hid every helper call in that flow. Callers that build a
+ * pattern escape it themselves with `escapeRegExp`.
+ */
 function contextParameter(body: string): string | undefined {
-  const parameter = body.match(/^(?:async\s+)?(?:function(?:\s+[\w$]+)?\s*)?(?:\(\s*([\w$]+)|([\w$]+)\s*=>)/u);
-  return (parameter?.[1] ?? parameter?.[2])?.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&');
+  // The third alternative is an object-literal method (`async post(f) { … }`),
+  // a shape `Function.prototype.toString()` can return and which the first two
+  // do not match — leaving `root` undefined and the body unscanned entirely.
+  const parameter = body.match(/^(?:async\s+)?(?:function\s*\*?\s*(?:[\w$]+)?\s*)?(?:\(\s*([\w$]+)|([\w$]+)\s*=>|\*?\s*[\w$]+\s*\(\s*([\w$]+))/u);
+  return parameter?.[1] ?? parameter?.[2] ?? parameter?.[3];
 }
 
-/** Same recognition as `preflightHelpers`: `f.slack`, `f .slack`, `f["slack"]`. */
-function helperReference(root: string, namespace: string): RegExp {
-  return new RegExp(`(?:^|[^\\w$.])${root}\\s*(?:\\.\\s*${namespace}\\b|\\[\\s*['"]${namespace}['"]\\s*\\])`, 'u');
+/** Escape a literal for embedding in a RegExp source. */
+function escapeRegExp(value: string): string {
+  return value.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&');
 }
 
 /**

packages/sdk/src/helper-preflight.ts

@@ -1,5 +1,6 @@
 import { helperProviders } from '@relayflows/surface/runtime';
 import type { PreflightResult, PreflightDiagnostic } from './preflight.js';
+import { helperNamespacesUsed } from './helper-reference.js';
 
 /** Static discovery never executes the body; dynamic aliases are checked at call time. */
 export function preflightHelpers(
@@ -8,12 +9,18 @@ export function preflightHelpers(
     providers?: Readonly<Record<string, { mount: boolean; mock: boolean; token?: string }>> },
 ): PreflightResult {
   const body = typeof definition.body === 'function' ? Function.prototype.toString.call(definition.body) : '';
-  const parameter = body.match(/^(?:async\s+)?(?:function(?:\s+[\w$]+)?\s*)?(?:\(\s*([\w$]+)|([\w$]+)\s*=>)/);
-  const root = (parameter?.[1] ?? parameter?.[2])?.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  const parameter = body.match(/^(?:async\s+)?(?:function\s*\*?\s*(?:[\w$]+)?\s*)?(?:\(\s*([\w$]+)|([\w$]+)\s*=>|\*?\s*[\w$]+\s*\(\s*([\w$]+))/u);
+  // NOT regex-escaped: this is compared to an AST Identifier name, so a legal
+  // parameter like `f$` must stay `f$`. Escaping it hid every helper call.
+  const root = parameter?.[1] ?? parameter?.[2] ?? parameter?.[3];
   const diagnostics: PreflightDiagnostic[] = [];
+  // Read from a parse, not from the text: a helper named inside a string,
+  // comment, template quasi or regex is not used, and refusing on one demands
+  // a mount the flow never touches.
+  const referenced = root === undefined ? new Set<string>() : helperNamespacesUsed(body, root);
   for (const { provider, namespace, supported } of helperProviders) {
     const used = definition.header?.tools?.[namespace] === true
-      || (root !== undefined && new RegExp(`(?:^|[^\\w$.])${root}\\s*(?:\\.\\s*${namespace}\\b|\\[\\s*['"]${namespace}['"]\\s*\\])`).test(body));
+      || referenced.has(namespace);
     if (!used) continue;
     const fact = facts.providers?.[provider] ?? (provider === 'slack'
       ? { mount: facts.slackMount, mock: facts.slackMock, token: facts.slackToken }

packages/sdk/src/helper-reference.ts

@@ -0,0 +1,130 @@
+import { parse, parseExpressionAt } from 'acorn';
+
+/**
+ * Which `f.<namespace>` helpers a flow body actually USES.
+ *
+ * Read from a parse, not from the text. A helper merely NAMED in a string, a
+ * comment, a template quasi or a regex is not used, and declaring a
+ * requirement from one refuses a flow for a mount it never touches:
+ * `f.run("echo f.gitlab")`, an agent prompt naming `f.slack`, or a PR title in
+ * a commit message were each enough to demand a GitLab mount.
+ *
+ * This was first written as a lexer that blanked comments, strings and
+ * regexes. That is the wrong tool: `/` is a regex or a division depending on
+ * whether the preceding token ends an expression, which is not decidable
+ * without parse context, and the two shapes that ambiguity broke —
+ * interpolations (`${await f.gitlab…}`) and a quote inside a regex (`/'/`) —
+ * both HID real helper use, deploying a flow without its mount to fail at
+ * runtime. A parser settles every one of those by construction.
+ *
+ * What a parser still cannot see is indirection: `const p = 'gitlab';
+ * f[p].issues…` is invisible to any static reading, because it needs value
+ * tracking. This scan is therefore a convenience for the literal case, never
+ * the authority — `header.tools[namespace] === true` is the declaration that
+ * is, and it short-circuits this entirely.
+ */
+export function helperNamespacesUsed(body: string, root: string): ReadonlySet<string> {
+  const program = parseFlowBody(body);
+  if (program === null) return textFallback(body, root);
+  const used = new Set<string>();
+  walk(program, (node) => {
+    if (node.type !== 'MemberExpression') return;
+    const object = node.object as AstNode | undefined;
+    if (object?.type !== 'Identifier' || object.name !== root) return;
+    const namespace = memberName(node);
+    if (namespace !== undefined) used.add(namespace);
+  });
+  return used;
+}
+
+/** `f.slack`, `f["slack"]`, `f?.slack` — but not `f[variable]`, which is unknowable. */
+function memberName(node: AstNode): string | undefined {
+  const property = node.property as AstNode | undefined;
+  if (property === undefined) return undefined;
+  if (node.computed !== true) return property.type === 'Identifier' ? property.name : undefined;
+  return property.type === 'Literal' && typeof property.value === 'string' ? property.value : undefined;
+}
+
+/**
+ * Parse whatever `Function.prototype.toString()` produced.
+ *
+ * It can be an arrow, a function expression, an async method shorthand from an
+ * object literal, or a declaration — only some of which are expressions, so
+ * each shape gets a try. Types are already stripped by the time a body reaches
+ * here (Node strips before evaluating, and bundlers transpile), so this is
+ * plain JavaScript.
+ */
+function parseFlowBody(body: string): AstNode | null {
+  const options = { ecmaVersion: 'latest' as const, allowAwaitOutsideFunction: true, allowReturnOutsideFunction: true };
+  // `parseExpressionAt` stops at the end of the first expression and does not
+  // object to what follows, so `async post(f) { … }` parses as the identifier
+  // `async` and reports success having read three characters. Every attempt
+  // must therefore consume the whole source, or a method body would be walked
+  // as its own name and declare no helpers at all.
+  const whole = (source: string, node: AstNode): AstNode => {
+    const end = typeof node.end === 'number' ? node.end : -1;
+    if (end < 0 || source.slice(end).trim() !== '') throw new SyntaxError('unconsumed input');
+    return node;
+  };
+  for (const attempt of [
+    () => whole(body, parseExpressionAt(body, 0, options) as unknown as AstNode),
+    () => whole(`(${body})`, parseExpressionAt(`(${body})`, 0, options) as unknown as AstNode),
+    () => parse(body, options) as unknown as AstNode,
+    // An object-literal method (`async post(f) { … }`) is neither expression
+    // nor statement on its own; it only parses inside an object.
+    () => whole(`({${body}})`, parseExpressionAt(`({${body}})`, 0, options) as unknown as AstNode),
+  ]) {
+    try {
+      return attempt();
+    } catch {
+      continue;
+    }
+  }
+  return null;
+}
+
+/**
+ * Last resort when nothing parses: match the text.
+ *
+ * Deliberately the permissive direction. An unparseable body is a shape this
+ * code does not understand, and under-reporting would deploy a flow without a
+ * mount it needs and fail at the call; over-reporting only asks for a mount
+ * that may go unused. Reaching here at all is a bug worth hearing about.
+ */
+function textFallback(body: string, root: string): ReadonlySet<string> {
+  const used = new Set<string>();
+  const escaped = root.replace(/[.*+?^${}()|[\]\\]/gu, '\\$&');
+  const pattern = new RegExp(`(?:^|[^\\w$.])${escaped}\\s*(?:\\.\\s*([\\w$]+)|\\[\\s*['"]([^'"]+)['"]\\s*\\])`, 'gu');
+  for (const match of body.matchAll(pattern)) {
+    const namespace = match[1] ?? match[2];
+    if (namespace !== undefined) used.add(namespace);
+  }
+  return used;
+}
+
+type AstNode = {
+  type: string;
+  end?: number;
+  name?: string;
+  value?: unknown;
+  computed?: boolean;
+  [key: string]: unknown;
+};
+
+/** Depth-first over every child node, without pulling in a second package. */
+function walk(node: AstNode, visit: (node: AstNode) => void): void {
+  visit(node);
+  for (const key of Object.keys(node)) {
+    if (key === 'type' || key === 'start' || key === 'end' || key === 'loc') continue;
+    const child = node[key];
+    if (Array.isArray(child)) {
+      for (const entry of child) if (isNode(entry)) walk(entry, visit);
+    } else if (isNode(child)) {
+      walk(child, visit);
+    }
+  }
+}
+
+function isNode(value: unknown): value is AstNode {
+  return typeof value === 'object' && value !== null && typeof (value as { type?: unknown }).type === 'string';
+}