Skip to content
Merged
9 changes: 9 additions & 0 deletions docs/SURFACE.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ No process runs between events: the handler wakes, executes to its next await, p
and the other provider namespaces remain follow-up work; see
[the generator notes](../packages/surface/src/helpers/README.md).

A namespace is not a promise of a whole vendor API. The generated catalog
records the exact sorted adapter `resources` alongside each provider's
`supported` flag, so authors and preflight tooling can inspect the generated
resource surface without loading an adapter's private catalog. Bespoke
aliases such as `stripe.createInvoice` remain outside that catalog. In particular,
GitLab now lists `issues`, `merge-requests`, `refs`, `merge`, and
`close-merge-request` in addition to comments and discussions; the earlier
comment-only gap was closed by the pinned relay-helpers release.

The initial local memory slice supports `recall` and `why` in authored flows,
with no journal step for either read. Script scope is stable across runs of
the same flow file and name; reads cannot widen it to another flow. The
Expand Down
137 changes: 131 additions & 6 deletions packages/sdk/src/helper-reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export function helperNamespacesUsed(body: string, root: string): ReadonlySet<st
const program = parseFlowBody(body);
if (program === null) return textFallback(body, root);
const used = new Set<string>();
walk(program, (node) => {
const scope = { rootFunctionFound: false };
walkReferences(program, root, false, scope, (node) => {
if (node.type !== 'MemberExpression') return;
const object = node.object as AstNode | undefined;
if (object?.type !== 'Identifier' || object.name !== root) return;
Expand Down Expand Up @@ -111,20 +112,144 @@ type AstNode = {
[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);
/**
* Depth-first over references that still resolve to the outer flow context.
*
* A nested callback/method parameter or lexical destructuring can reuse the
* context's spelling while binding an unrelated value. Counting its members
* would refuse the flow for a mount it never touches. Acorn gives us binding
* shapes, so scopes are tracked as syntax instead of guessed from text.
*/
function walkReferences(
node: AstNode,
root: string,
shadowed: boolean,
state: { rootFunctionFound: boolean },
visit: (node: AstNode) => void,
): void {
if (isFunction(node)) {
const parameters = Array.isArray(node.params) ? node.params.filter(isNode) : [];
const bindsParameter = parameters.some(parameter => patternBinds(parameter, root));
let bodyHidden = shadowed;
const isRootFunction = bindsParameter && !state.rootFunctionFound;
if (isRootFunction) state.rootFunctionFound = true;
else if (bindsParameter || (isNode(node.id) && patternBinds(node.id, root)) || functionVarBinds(node, root)) {
bodyHidden = true;
}
// Parameter initializers run before the function body and are outside a
// body-level `var` scope. Keep them visible unless the parameter list
// itself binds the root name (in which case all parameter references are
// local to that parameter environment).
const parameterHidden = shadowed || (bindsParameter && !isRootFunction);
for (const parameter of parameters) walkReferences(parameter, root, parameterHidden, state, visit);
const body = isNode(node.body) ? node.body : undefined;
if (body !== undefined) walkReferences(body, root, bodyHidden, state, visit);
return;
}
let hidden = shadowed;
if (node.type === 'BlockStatement' && blockBinds(node, root)) {
hidden = true;
} else if (node.type === 'CatchClause' && isNode(node.param) && patternBinds(node.param, root)) {
hidden = true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For-loop bindings skip shadowing

Low Severity

The new scope walk hides block, catch, and function bindings of the context name, but a for / for-in / for-of head that lexically binds that name still counts member access in the loop as an outer helper reference. Preflight can then demand a mount the flow never uses.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 28c46e4. Configure here.


if (!hidden) 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);
for (const entry of child) if (isNode(entry)) walkReferences(entry, root, hidden, state, visit);
} else if (isNode(child)) {
walk(child, visit);
walkReferences(child, root, hidden, state, visit);
}
}
}

function isFunction(node: AstNode): boolean {
return node.type === 'ArrowFunctionExpression'
|| node.type === 'FunctionExpression'
|| node.type === 'FunctionDeclaration';
}

/** Lexical declarations bind across their complete block, including TDZ. */
function blockBinds(node: AstNode, root: string): boolean {
const statements = Array.isArray(node.body) ? node.body.filter(isNode) : [];
return statements.some((statement) => {
if (statement.type === 'VariableDeclaration' && statement.kind !== 'var') {
const declarations = Array.isArray(statement.declarations)
? statement.declarations.filter(isNode) : [];
return declarations.some(declaration => isNode(declaration.id) && patternBinds(declaration.id, root));
}
return (statement.type === 'FunctionDeclaration' || statement.type === 'ClassDeclaration')
&& isNode(statement.id) && patternBinds(statement.id, root);
});
}

/** Identifier occurrences that introduce bindings, including nested patterns. */
function patternBinds(pattern: AstNode, root: string): boolean {
switch (pattern.type) {
case 'Identifier':
return pattern.name === root;
case 'AssignmentPattern':
return isNode(pattern.left) && patternBinds(pattern.left, root);
case 'RestElement':
return isNode(pattern.argument) && patternBinds(pattern.argument, root);
case 'ArrayPattern': {
const elements = Array.isArray(pattern.elements) ? pattern.elements : [];
return elements.some(element => isNode(element) && patternBinds(element, root));
}
case 'ObjectPattern': {
const properties = Array.isArray(pattern.properties) ? pattern.properties : [];
return properties.some(property => {
if (!isNode(property)) return false;
if (property.type === 'RestElement') {
return isNode(property.argument) && patternBinds(property.argument, root);
}
// Object keys are labels, not bindings. Only the value side introduces
// the local (including a default-value AssignmentPattern's left side).
return property.type === 'Property'
&& isNode(property.value)
&& patternBinds(property.value, root);
});
}
default:
return false;
}
}

/** `var` is function-scoped, so a nested function's declaration hides the
* outer flow context for its entire body, including code before the
* declaration. Nested functions have their own var scopes and are skipped. */
function functionVarBinds(node: AstNode, root: string): boolean {
const body = isNode(node.body) ? node.body : undefined;
if (body === undefined) return false;
let found = false;
const scan = (current: AstNode): void => {
if (found) return;
if (current !== body && isFunction(current)) return;
if (current.type === 'VariableDeclaration' && current.kind === 'var') {
const declarations = Array.isArray(current.declarations)
? current.declarations.filter(isNode) : [];
if (declarations.some(declaration => isNode(declaration.id) && patternBinds(declaration.id, root))) {
found = true;
return;
}
}
for (const key of Object.keys(current)) {
if (key === 'type' || key === 'start' || key === 'end' || key === 'loc') continue;
const child = current[key];
if (Array.isArray(child)) {
for (const entry of child) if (isNode(entry)) scan(entry);
} else if (isNode(child)) {
scan(child);
}
if (found) return;
}
};
scan(body);
return found;
}

function isNode(value: unknown): value is AstNode {
return typeof value === 'object' && value !== null && typeof (value as { type?: unknown }).type === 'string';
}
2 changes: 1 addition & 1 deletion packages/sdk/src/hosted-extension-sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const ADDRESS_SPACE_BYTES = 16 * 1024 * 1024 * 1024;
const DATA_BYTES = 3 * 1024 * 1024 * 1024;
const SURFACE_RUNTIME_SHA256 = OBJECT_FREEZE({
'flow.js': '4aaeacc55de3074f4d121ce7253c3be50a93757e6540ba8159889a9450d1c05c',
'helpers/providers.js': '7bc62eccaa3a9e786ae0a689bf74160585149e91feef8208e17ef8eca51eed7f',
'helpers/providers.js': '4eb06d0d85ca0a3434bb2dbba7407e3d95eef2dfbedaeb0e2de0c0c0ea812457',
'provider-trigger.js': 'e2664c65397f93fb486eb6f1e756c7cec3f88b3851d79c23567cad986f80f1ff',
'schedule.js': '8fe72f176a75ec0b5f26e12db7a597575c259a2e2cbb59690f9dc20a5e63940b',
'triggers.js': '4a3515b571a318f6c7a5661f9310bc9af43e3faf20ea39903a9b363c51258e4c',
Expand Down
62 changes: 62 additions & 0 deletions packages/sdk/tests/helper-reference.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const refusals = (body: Function): string[] =>
preflightHelpers({ header: {}, body }, mountFacts).diagnostics
.filter((d) => d.severity === 'refusal')
.map((d) => d.kind);
const fromSource = (source: string): Function => new Function(`return ${source};`)() as Function;

describe('helper references are read as syntax, not text', () => {
it('does not demand a mount for a helper named inside a string', () => {
Expand Down Expand Up @@ -132,6 +133,67 @@ describe('helper references are read as syntax, not text', () => {
expect(refusals(holder.gen as never)).toContain('helper_provider.mount_required');
});

it('does not treat destructured locals as the outer flow context', () => {
for (const source of [
'(f) => { { const { f } = { f: { gitlab: { issues: 42 } } }; return f.gitlab.issues; } }',
'(f) => { { const [f] = [{ gitlab: { issues: 42 } }]; return f.gitlab.issues; } }',
]) expect(refusals(fromSource(source)), source).toEqual([]);
});

it('does not treat object or class method parameters as the outer flow context', () => {
for (const source of [
'(f) => ({ read(f) { return f.gitlab.issues; } }).read({ gitlab: { issues: 42 } })',
'(f) => new class { read(f) { return f.gitlab.issues; } }().read({ gitlab: { issues: 42 } })',
'(f) => ({ read({ f }) { return f.gitlab.issues; } }).read({ f: { gitlab: { issues: 42 } } })',
]) expect(refusals(fromSource(source)), source).toEqual([]);
});

it('only treats binding targets as shadowing in object/default patterns', () => {
for (const source of [
'(f) => { const callback = ({ f: x }) => f.gitlab.issues; return callback; }',
'(f) => { const callback = (x = f) => f.gitlab.issues; return callback; }',
]) expect(refusals(fromSource(source)), source).toContain('helper_provider.mount_required');
});

it('treats nested function-scoped var declarations as shadowing', () => {
expect(refusals(fromSource(
'(f) => { const callback = () => { var f; return f.gitlab.issues; }; return callback(); }',
))).toEqual([]);
expect(refusals(fromSource(
'(f) => { function callback() { var f; return f.gitlab.issues; } return callback(); }',
))).toEqual([]);
});

it('still reads the outer context outside a nested var scope', () => {
expect(refusals(fromSource(
'(f) => { const callback = () => { var f; return f.gitlab.issues; }; callback(); return f.gitlab.issues; }',
))).toContain('helper_provider.mount_required');
});

it('keeps outer helper references in parameter defaults outside var scope', () => {
expect(refusals(fromSource(
'(f) => { function read(x = f.gitlab.issues) { var f = local; return f.gitlab; } }',
))).toContain('helper_provider.mount_required');
});

it('keeps root-function parameter defaults visible', () => {
expect(refusals(fromSource('(f, x = f.gitlab.issues) => x')))
.toContain('helper_provider.mount_required');
});

it('does not let a deeper nested var scope hide an outer helper', () => {
expect(refusals(fromSource(
'(f) => { const outer = () => { const inner = () => { var f; return f.gitlab; }; return f.gitlab.issues; }; return outer(); }',
))).toContain('helper_provider.mount_required');
});

it('keeps reading the outer context outside a shadowing scope', () => {
const source = '(f) => { { const { f } = { f: { gitlab: {} } }; void f.gitlab; } return f.gitlab.issues; }';
expect(refusals(fromSource(source))).toContain('helper_provider.mount_required');
expect(refusals(fromSource('(f) => { if (f) return f.gitlab.issues; }')))
.toContain('helper_provider.mount_required');
});

// `flowRequirements` has its OWN parameter extraction, mirroring
// `preflightHelpers`. The shapes above are asserted through preflight, so
// without these the mirrored regex could be reverted with the suite green.
Expand Down
22 changes: 14 additions & 8 deletions packages/surface/src/helpers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,17 @@ Use `--out-dir /tmp/generated-helpers` to inspect output without changing source
CI regenerates from the installed pinned package and compares every generated
TypeScript file byte-for-byte, including the namespace index.

Follow-up for the full slice N: add GitHub, Notion, Linear, and Stripe once their
methods have runtime dispatch support; consume mapping/discovery resources and
generate the remaining providers. The current runtime implements only Slack.
The uniform upstream clients expose resource `read`/`list`/`write` methods,
not `stripe.createInvoice` or `notion.appendBlock`; those aliases need an agreed
runtime contract before this types-only generator can expose them. GitHub's
bespoke `createIssue` also requires `owner` in addition to `repo`, `title`, and
`body`. No new provider methods or resource methods are advertised in this proof.
Runtime dispatch is no longer Slack-only: every provider in `providers.ts` whose
`supported` is not `false` binds its upstream client's resource
`read`/`list`/`write` methods plus the bespoke aliases the generator knows
(`stripe.createInvoice`, `notion.appendBlock`, GitHub's `createIssue`, which
requires `owner` in addition to `repo`, `title`, and `body`). `path` stays a
synchronous path builder and is never dispatched.

`supported` answers whether the pinned upstream writeback client exists. The
adjacent sorted `resources` list is the exact adapter catalog; bespoke aliases
such as `stripe.createInvoice` remain outside it. The list is not a claim that
the whole vendor API is reachable. Keeping both fields in the generated file
makes catalog growth and regression reviewable as ordinary source diffs.
GitLab currently exposes `issues`, `merge-requests`, `refs`, `merge`, and
`close-merge-request` in addition to comments and discussions.
Loading
Loading