From 48b812c47eef0470f814fae52528383e8f090347 Mon Sep 17 00:00:00 2001 From: Relayflow Date: Sun, 20 Sep 2026 17:14:40 +0000 Subject: [PATCH 1/6] fix: name the gap where f.gitlab's namespace outruns its writeback catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f.gitlab carries comments and discussions; f.github carries issues, pull requests, reviews, refs, merge and close-pull-request. The catalog marked both `supported: true`, so the only way to learn the difference was to reach for `f.gitlab.issues` and read `undefined is not a function` — or to dump the writeback catalog before writing a line. A GitLab-sourced flow shelled out to `glab` for every read as a result. Parity is upstream work. What is fixed here is the silence: `supported` now distinguishes `'partial'` from full, the generator carries the resources each provider actually dispatches and the note that says what a partial omits, and every layer an author can reach the gap through refuses by name — `flows check` statically, the surface's property guard for a computed name, both worded by one function and neither touching the provider to refuse. Co-Authored-By: Claude --- docs/SURFACE.md | 16 + packages/sdk/src/authored-flow-executor.ts | 14 +- packages/sdk/src/flow-requirements.ts | 52 +-- packages/sdk/src/helper-preflight.ts | 48 ++- packages/sdk/src/source-scan.ts | 107 ++++++ .../sdk/tests/helper-partial-support.test.ts | 136 +++++++ packages/sdk/tests/helpers-fanout.test.ts | 8 +- packages/surface/src/effect-transport.ts | 38 +- packages/surface/src/helper-support.ts | 83 +++++ packages/surface/src/helpers/README.md | 33 +- packages/surface/src/helpers/gitlab.ts | 5 + packages/surface/src/helpers/providers.ts | 342 +++++++++++++++--- packages/surface/src/runtime.ts | 7 + packages/surface/tests/helper-support.test.ts | 116 ++++++ .../tests/helpers-typecheck-fail.test-d.ts | 4 + scripts/generate-helpers.mjs | 29 +- 16 files changed, 913 insertions(+), 125 deletions(-) create mode 100644 packages/sdk/src/source-scan.ts create mode 100644 packages/sdk/tests/helper-partial-support.test.ts create mode 100644 packages/surface/src/helper-support.ts create mode 100644 packages/surface/tests/helper-support.test.ts diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 4620b56ea..1ac12723a 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -115,6 +115,22 @@ 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 `supported` as `true` (every resource in `resources` dispatches), + `'partial'` (usable, but known to omit workflows the namespace suggests — + `note` says which), or `false` (no upstream writeback client). `f.gitlab` is + `'partial'`: it carries `comments` and `discussions` only, so issue + list/read/create and merge-request list/read/create are unavailable through + it, where `f.github` carries issues, pull-requests, reviews, refs, merge, and + close-pull-request. Reaching for an absent resource does not fail as + `undefined is not a function`. `flows check` refuses statically evident + access with `helper_provider.unsupported`, and a dynamically computed name + reaches the surface's property guard, which throws + `f.gitlab.issues is unavailable; available resources: comments, discussions.` + followed by the catalog note — the same wording from both, and mapped at the + body boundary onto `helper_provider.unsupported`. Neither path performs + provider I/O to refuse. + 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 diff --git a/packages/sdk/src/authored-flow-executor.ts b/packages/sdk/src/authored-flow-executor.ts index 965ad117b..495a9c9d1 100644 --- a/packages/sdk/src/authored-flow-executor.ts +++ b/packages/sdk/src/authored-flow-executor.ts @@ -23,7 +23,10 @@ import { type RunCompletionReason as SurfaceRunCompletionReason, type Step, } from '@relayflows/surface'; -import { createHelpers, helperProviders, type HelperCall, type FlowHandle } from '@relayflows/surface/runtime'; +import { + createHelpers, helperProviders, UnsupportedHelperMemberError, + type HelperCall, type FlowHandle, +} from '@relayflows/surface/runtime'; import { join } from 'node:path'; import { observeStep, type ProgressEvent } from './progress.js'; import { parseStepTimeout } from './compile.js'; @@ -537,7 +540,14 @@ export async function executeAuthoredFlow( await bodyPromise; } catch (error) { bodyFailed = true; - bodyFailure = error; + // The surface refuses an absent helper resource structurally, naming the + // ones that dispatch. Carrying that out as a bare Error would report it as + // an unexplained body crash, so it is remapped onto the code preflight + // already uses for the same refusal — the message is the surface's, not a + // second wording. Every other failure is rethrown exactly as thrown. + bodyFailure = error instanceof UnsupportedHelperMemberError + ? new AuthoredFlowExecutionError('helper_provider.unsupported', error.message) + : error; } if (bodyFailed) { try { diff --git a/packages/sdk/src/flow-requirements.ts b/packages/sdk/src/flow-requirements.ts index e8edbc82d..5e381e151 100644 --- a/packages/sdk/src/flow-requirements.ts +++ b/packages/sdk/src/flow-requirements.ts @@ -2,6 +2,7 @@ import { humanRecipientProvider } from './human-to.js'; import { helperProviders } from '@relayflows/surface/runtime'; import type { TriggerSource } from '@relayflows/surface'; import { providerDeclaration } from './provider-trigger-contract.js'; +import { matchingClose, skipCommentOrString, stringEnd } from './source-scan.js'; import type { FlowSpec } from './spec.js'; import { helperCall } from './yaml-helpers.js'; @@ -216,57 +217,6 @@ function humanRecipients(root: string, body: string): string[] { return found; } -/** - * Skip the comment or string starting at `i`, returning the index just past - * it; `i` itself when nothing skippable starts there; -1 when unterminated. - * Every walker below steps through this, so a `,`, `to:` or bracket inside a - * comment or string is never read as syntax. - */ -function skipCommentOrString(text: string, i: number): number { - const ch = text[i]!; - const next = text[i + 1]; - if (ch === '/' && next === '/') { const end = text.indexOf('\n', i); return end === -1 ? text.length : end + 1; } - if (ch === '/' && next === '*') { const end = text.indexOf('*/', i + 2); return end === -1 ? -1 : end + 2; } - if (ch === '"' || ch === "'" || ch === '`') { const end = stringEnd(text, i); return end === -1 ? -1 : end + 1; } - return i; -} - -/** Index of the `)`/`}`/`]` closing the bracket at `open`, skipping strings, templates and comments; -1 if unbalanced. */ -function matchingClose(text: string, open: number): number { - const pairs: Record = { '(': ')', '{': '}', '[': ']' }; - const stack: string[] = [pairs[text[open]!]!]; - let i = open + 1; - while (i < text.length && stack.length > 0) { - const skipped = skipCommentOrString(text, i); - if (skipped === -1) return -1; - if (skipped !== i) { i = skipped; continue; } - const ch = text[i]!; - if (ch in pairs) stack.push(pairs[ch]!); - else if (ch === ')' || ch === '}' || ch === ']') { if (stack.pop() !== ch) return -1; } - i += 1; - } - return stack.length === 0 ? i - 1 : -1; -} - -/** Index of the quote closing the string opening at `start` (template `${…}` skipped); -1 if unterminated. */ -function stringEnd(text: string, start: number): number { - const quote = text[start]!; - let i = start + 1; - while (i < text.length) { - const ch = text[i]!; - if (ch === '\\') { i += 2; continue; } - if (ch === quote) return i; - if (quote === '`' && ch === '$' && text[i + 1] === '{') { - const end = matchingClose(text, i + 1); - if (end === -1) return -1; - i = end + 1; - continue; - } - i += 1; - } - return -1; -} - /** The `{ … }` that is the call's second top-level argument, or undefined. */ function secondArgumentObject(args: string): string | undefined { let depth = 0; diff --git a/packages/sdk/src/helper-preflight.ts b/packages/sdk/src/helper-preflight.ts index 3f22d264a..ee8428cb0 100644 --- a/packages/sdk/src/helper-preflight.ts +++ b/packages/sdk/src/helper-preflight.ts @@ -1,4 +1,5 @@ -import { helperProviders } from '@relayflows/surface/runtime'; +import { helperProviders, unsupportedHelperMemberMessage } from '@relayflows/surface/runtime'; +import { codeOnly } from './source-scan.js'; import type { PreflightResult, PreflightDiagnostic } from './preflight.js'; /** Static discovery never executes the body; dynamic aliases are checked at call time. */ @@ -11,24 +12,59 @@ export function preflightHelpers( const parameter = body.match(/^(?:async\s+)?(?:function(?:\s+[\w$]+)?\s*)?(?:\(\s*([\w$]+)|([\w$]+)\s*=>)/); const root = (parameter?.[1] ?? parameter?.[2])?.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const diagnostics: PreflightDiagnostic[] = []; - for (const { provider, namespace, supported } of helperProviders) { + for (const provider of helperProviders) { + const { namespace, supported } = provider; const used = definition.header?.tools?.[namespace] === true || (root !== undefined && new RegExp(`(?:^|[^\\w$.])${root}\\s*(?:\\.\\s*${namespace}\\b|\\[\\s*['"]${namespace}['"]\\s*\\])`).test(body)); if (!used) continue; - const fact = facts.providers?.[provider] ?? (provider === 'slack' + const fact = facts.providers?.[provider.provider] ?? (provider.provider === 'slack' ? { mount: facts.slackMount, mock: facts.slackMock, token: facts.slackToken } : { mount: false, mock: false, token: undefined }); + // A resource the helper does not have is refused before the mount question + // and regardless of mock mode: installing a mount cannot conjure a + // writeback route that no client carries, and a body that would die on + // `undefined is not a function` should say so with the names that do work. + const missing = supported === 'partial' + ? unavailableMembers(root, namespace, provider.resources, body) : []; if (!supported) { diagnostics.push({ severity: 'refusal', kind: 'helper_provider.unsupported', message: `f.${namespace} has no upstream relayfile writeback client.` }); + } else if (missing.length > 0) { + diagnostics.push({ severity: 'refusal', kind: 'helper_provider.unsupported', + message: unsupportedHelperMemberMessage(provider.provider, missing[0]!, provider.resources) }); } else if (!fact.mock && !fact.mount) { diagnostics.push({ severity: 'refusal', - kind: provider === 'slack' ? (fact.token?.trim() ? 'helper_slack.mount_required' : 'helper_slack.credential_missing') : 'helper_provider.mount_required', - message: `f.${namespace} requires a relayfile ${provider} mount; direct-token transport is not implemented.` }); - } else if (provider === 'notion' && !fact.mock && /\.\s*appendBlock\b/.test(body)) { + kind: provider.provider === 'slack' ? (fact.token?.trim() ? 'helper_slack.mount_required' : 'helper_slack.credential_missing') : 'helper_provider.mount_required', + message: `f.${namespace} requires a relayfile ${provider.provider} mount; direct-token transport is not implemented.` }); + } else if (provider.provider === 'notion' && !fact.mock && /\.\s*appendBlock\b/.test(body)) { diagnostics.push({ severity: 'refusal', kind: 'helper_provider.unsupported', message: 'f.notion.appendBlock is mock-only: the Notion adapter has no append-block writeback route.' }); } } return { ok: diagnostics.length === 0, gates: [], resolutions: [], diagnostics }; } + +/** + * Members read off `f.` in the body that the provider does not + * expose, in source order. + * + * Only what is statically evident counts: a direct `.member` or `['member']` + * on the context parameter this body actually declares, matched against + * `codeOnly`, where comments and data strings are blanked but a literal in + * property-key position is not. A computed name is not decided here — the + * surface's own property guard refuses it at call time. + */ +function unavailableMembers( + root: string | undefined, namespace: string, resources: readonly string[], body: string, +): string[] { + if (root === undefined) return []; + const access = new RegExp( + `(?:^|[^\\w$.])${root}\\s*(?:\\.\\s*${namespace}|\\[\\s*['"]${namespace}['"]\\s*\\])` + + `\\s*(?:\\.\\s*([\\w$]+)|\\[\\s*['"]([\\w$]+)['"]\\s*\\])`, 'gu'); + const found: string[] = []; + for (const match of codeOnly(body).matchAll(access)) { + const member = match[1] ?? match[2]!; + if (!resources.includes(member) && !found.includes(member)) found.push(member); + } + return found; +} diff --git a/packages/sdk/src/source-scan.ts b/packages/sdk/src/source-scan.ts new file mode 100644 index 000000000..e3fa9436e --- /dev/null +++ b/packages/sdk/src/source-scan.ts @@ -0,0 +1,107 @@ +/** + * The bounded source scanner every static body inspection steps through. + * + * A flow body is read from `Function.prototype.toString` — never executed — so + * the declarations preflight judges are read out of text. Text is not syntax: + * a `to:`, a bracket or an `f.gitlab.issues` inside a comment or a string is + * not a declaration, and reading it as one refuses a correct flow. These + * walkers skip comments, strings and templates so that cannot happen. + * + * This is deliberately not a parser. Template interpolations and computed + * names are opaque here; those reach the runtime guard instead. + */ + +/** + * Skip the comment or string starting at `i`, returning the index just past + * it; `i` itself when nothing skippable starts there; -1 when unterminated. + */ +export function skipCommentOrString(text: string, i: number): number { + const ch = text[i]!; + const next = text[i + 1]; + if (ch === '/' && next === '/') { const end = text.indexOf('\n', i); return end === -1 ? text.length : end + 1; } + if (ch === '/' && next === '*') { const end = text.indexOf('*/', i + 2); return end === -1 ? -1 : end + 2; } + if (ch === '"' || ch === "'" || ch === '`') { const end = stringEnd(text, i); return end === -1 ? -1 : end + 1; } + return i; +} + +/** Index of the `)`/`}`/`]` closing the bracket at `open`, skipping strings, templates and comments; -1 if unbalanced. */ +export function matchingClose(text: string, open: number): number { + const pairs: Record = { '(': ')', '{': '}', '[': ']' }; + const stack: string[] = [pairs[text[open]!]!]; + let i = open + 1; + while (i < text.length && stack.length > 0) { + const skipped = skipCommentOrString(text, i); + if (skipped === -1) return -1; + if (skipped !== i) { i = skipped; continue; } + const ch = text[i]!; + if (ch in pairs) stack.push(pairs[ch]!); + else if (ch === ')' || ch === '}' || ch === ']') { if (stack.pop() !== ch) return -1; } + i += 1; + } + return stack.length === 0 ? i - 1 : -1; +} + +/** Index of the quote closing the string opening at `start` (template `${…}` skipped); -1 if unterminated. */ +export function stringEnd(text: string, start: number): number { + const quote = text[start]!; + let i = start + 1; + while (i < text.length) { + const ch = text[i]!; + if (ch === '\\') { i += 2; continue; } + if (ch === quote) return i; + if (quote === '`' && ch === '$' && text[i + 1] === '{') { + const end = matchingClose(text, i + 1); + if (end === -1) return -1; + i = end + 1; + continue; + } + i += 1; + } + return -1; +} + +/** + * The body with every comment and string/template replaced by blanks of the + * same length, so a pattern may be matched against code alone at unchanged + * offsets. Blanking only ever REMOVES text: a match found here was in the + * source, which is why the result can refuse a flow. What it hides — an access + * built inside a template interpolation, say — falls through to the runtime + * guard rather than becoming a refusal of something the author did not write. + * + * One literal is kept, because it is syntax rather than data: an + * identifier-shaped quoted string between `[` and `]` is the property name of + * a member access. Blanking it would hide `f['gitlab']['issues']`, which is + * the same access as `f.gitlab.issues` and has to read the same way here. + */ +export function codeOnly(text: string): string { + let out = ''; + let i = 0; + while (i < text.length) { + const skipped = skipCommentOrString(text, i); + // Unterminated: the remainder is not code this scanner can judge. + if (skipped === -1) return out + blank(text.slice(i)); + if (skipped !== i) { + const span = text.slice(i, skipped); + out += isPropertyKey(text, i, skipped, out) ? span : blank(span); + i = skipped; + continue; + } + out += text[i]!; + i += 1; + } + return out; +} + +/** + * Whether `text[start…end)` is a quoted property name inside `[` … `]`. + * `before` is the code-only prefix, so a `[` that is itself inside a comment + * or a string cannot put a literal in key position. + */ +function isPropertyKey(text: string, start: number, end: number, before: string): boolean { + const quote = text[start]!; + if (quote !== '"' && quote !== "'") return false; + if (!/^[\w$]+$/u.test(text.slice(start + 1, end - 1))) return false; + return /\[[^\S\n]*$/u.test(before) && /^[^\S\n]*\]/u.test(text.slice(end)); +} + +const blank = (text: string) => text.replace(/[^\n]/gu, ' '); diff --git a/packages/sdk/tests/helper-partial-support.test.ts b/packages/sdk/tests/helper-partial-support.test.ts new file mode 100644 index 000000000..2eab46b12 --- /dev/null +++ b/packages/sdk/tests/helper-partial-support.test.ts @@ -0,0 +1,136 @@ +/** + * `f.gitlab` carries comments and discussions only, where `f.github` carries + * issues, pull requests and the rest. The catalog says so, and every layer an + * author can reach the gap through says so too — statically at check time, and + * at the call site for a name only computable at runtime. + */ +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, expect, it, vi } from 'vitest'; +import { flow, type Ctx } from '@relayflows/surface'; +import { checkProviderHelpers } from '../src/slack-preflight.js'; +import { executeAuthoredFlow } from '../src/authored-flow-executor.js'; +import { JournalClient } from '../src/journal-client.js'; +import { runCli } from '../src/cli.js'; + +const root = resolve(dirname(fileURLToPath(import.meta.url)), '../../..'); +const mountKeys = ['RELAYFILE_MOUNT_PATH', 'WORKSPACE_ROOT', 'WORKFORCE_SANDBOX_ROOT', 'RELAYFILE_MOUNT_ROOT', 'RELAYFILE_ROOT']; +const dirs: string[] = []; +afterEach(() => { vi.unstubAllEnvs(); vi.restoreAllMocks(); for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); +function temporary() { const dir = mkdtempSync(join(tmpdir(), 'gitlab-partial-')); dirs.push(dir); return dir; } + +const unavailable = 'f.gitlab.issues is unavailable; available resources: comments, discussions.' + + ' Comments and discussions only. Issue list/read/create and merge-request' + + ' list/read/create are unavailable through f.gitlab.'; +/** A mount and mock that would satisfy every other provider question. */ +function satisfied() { + const dir = temporary(); + mkdirSync(join(dir, 'gitlab')); + mkdirSync(join(dir, 'github')); + vi.stubEnv('RELAYFILE_MOUNT_PATH', dir); + vi.stubEnv('RELAYFLOWS_GITLAB_MOCK', '1'); + vi.stubEnv('RELAYFLOWS_GITHUB_MOCK', '1'); +} +const refusal = (body: Function) => checkProviderHelpers({ body }).diagnostics + .filter(d => d.severity === 'refusal'); + +it('refuses a gitlab resource the helper does not carry, naming the ones it does', () => { + satisfied(); + expect(refusal(async (f: any) => { await f.gitlab.issues.list({ projectPath: 'g/p' }); })) + .toEqual([{ severity: 'refusal', kind: 'helper_provider.unsupported', message: unavailable }]); +}); + +it('reads the access however it is written: brackets, spacing, and a renamed context parameter', () => { + satisfied(); + for (const body of [ + async (f: any) => { await f['gitlab']['issues'].list({}); }, + async (f: any) => { await f . gitlab . issues . list({}); }, + async (ctx: any) => { await ctx.gitlab.issues.list({}); }, + function (f: any) { return f.gitlab.issues; }, + ]) expect(refusal(body), body.toString()).toHaveLength(1); + expect(refusal(async (f: any) => { await f.gitlab['mergeRequests'].write({}, {}); })[0]?.message) + .toContain('f.gitlab.mergeRequests is unavailable; available resources: comments, discussions.'); +}); + +it('refuses before the mount question and regardless of mock mode', () => { + // Installing a mount cannot conjure a writeback route no client carries, so + // `mount_required` would send the author to fix the wrong thing. + for (const key of [...mountKeys, 'RELAYFLOWS_GITLAB_MOCK']) vi.stubEnv(key, ''); + const unmounted = refusal(async (f: any) => { await f.gitlab.issues.list({}); }); + expect(unmounted.map(d => d.kind)).toEqual(['helper_provider.unsupported']); + expect(unmounted[0]?.message).toBe(unavailable); + satisfied(); + expect(refusal(async (f: any) => { await f.gitlab.issues.list({}); })[0]?.message).toBe(unavailable); +}); + +it('accepts the resources gitlab does carry, and leaves github alone', () => { + satisfied(); + expect(checkProviderHelpers({ body: async (f: Ctx) => { + await f.gitlab.comments.write({ projectPath: 'g/p', issueIid: '1', slug: 's' }, { text: 'hi' }); + await f.gitlab.discussions.list({ projectPath: 'g/p', issueIid: '1', slug: 's' }); + } }).ok).toBe(true); + expect(checkProviderHelpers({ body: async (f: any) => { + await f.github.issues.list({ owner: 'o', repo: 'r' }); + await f.github.pullRequests?.list?.({ owner: 'o', repo: 'r' }); + } }).ok).toBe(true); +}); + +it('does not read a comment or a string literal as an access', () => { + satisfied(); + // The body is read from `toString`, never executed; text that looks like an + // access but is not code must not refuse a correct flow. + const commented = async (f: Ctx) => { + // f.gitlab.issues.list({}) is what an author might reach for first. + const sample = 'f.gitlab.mergeRequests.write'; + const template = `see ${'f.gitlab.issues'} instead`; + await f.gitlab.comments.write({ projectPath: 'g/p', issueIid: '1', slug: 's' }, { text: sample + template }); + }; + expect(checkProviderHelpers({ body: commented })).toMatchObject({ ok: true, diagnostics: [] }); +}); + +it('refuses a computed member at the call site, as a typed helper_provider.unsupported', async () => { + satisfied(); + let reached = false; + const handle = flow('gitlab-dynamic', async f => { + const resource = ['iss', 'ues'].join(''); + reached = true; + await (f as any).gitlab[resource].list({ projectPath: 'g/p' }); + f.done('success'); + }); + // Static inspection cannot decide a computed name, so this one is the + // runtime guard's: it must still refuse with the same code and wording. + expect(checkProviderHelpers({ body: handle as unknown as Function }).ok).toBe(true); + await expect(executeAuthoredFlow(handle, new JournalClient('/must-not-connect'))) + .rejects.toMatchObject({ code: 'helper_provider.unsupported', message: `helper_provider.unsupported: ${unavailable}` }); + expect(reached).toBe(true); +}); + +it('leaves an unrelated body failure exactly as the body threw it', async () => { + satisfied(); + const handle = flow('gitlab-unrelated', async f => { + void f; + throw new TypeError('authored code broke on its own'); + }); + await expect(executeAuthoredFlow(handle, new JournalClient('/must-not-connect'))) + .rejects.toThrow(new TypeError('authored code broke on its own')); +}); + +it('reports the refusal from `flows check` on an authored flow file', async () => { + satisfied(); + const dir = temporary(); + mkdirSync(join(dir, 'node_modules/@relayflows'), { recursive: true }); + symlinkSync(join(root, 'packages/sdk/node_modules/@relayflows/surface'), join(dir, 'node_modules/@relayflows/surface')); + const surface = JSON.stringify(join(root, 'packages/sdk/node_modules/@relayflows/surface/dist/index.js')); + const fixture = join(dir, 'gitlab-triage.flow.ts'); + writeFileSync(fixture, `import { flow } from ${surface};\n` + + `export default flow('gitlab-triage', async (f: any) => {\n` + + ` const open = await f.gitlab.issues.list({ projectPath: 'g/p' });\n` + + ` await f.gitlab.mergeRequests.write({ projectPath: 'g/p' }, { title: String(open) });\n` + + ` f.done('success');\n});\n`); + const output: string[] = []; + expect(await runCli(['check', '--json', fixture], { stdout: line => output.push(line), stderr: line => output.push(line) })).toBe(2); + expect(output.join('')).toContain('helper_provider.unsupported'); + expect(output.join('')).toContain('f.gitlab.issues is unavailable; available resources: comments, discussions.'); +}, 30_000); diff --git a/packages/sdk/tests/helpers-fanout.test.ts b/packages/sdk/tests/helpers-fanout.test.ts index 013787f6b..5b7551e5c 100644 --- a/packages/sdk/tests/helpers-fanout.test.ts +++ b/packages/sdk/tests/helpers-fanout.test.ts @@ -29,13 +29,17 @@ for (const provider of helperProviders) { vi.stubEnv(provider.mockEnv, ''); const definition = { header: { tools: { [provider.namespace]: true } } }; expect(checkProviderHelpers(definition).ok).toBe(false); + // A partially supported provider passes the MOUNT question like any other: + // the resources it does carry dispatch, and the refusal for the ones it + // does not is driven by the body, which a tools-only header has none of. + const dispatches = provider.supported !== false; vi.stubEnv(provider.mockEnv, '1'); - expect(checkProviderHelpers(definition).ok).toBe(provider.supported); + expect(checkProviderHelpers(definition).ok).toBe(dispatches); vi.stubEnv(provider.mockEnv, ''); vi.stubEnv('WORKSPACE_ROOT', dir); mkdirSync(join(dir, provider.provider)); expect(providerMount(provider.provider)).toBe(dir); - expect(checkProviderHelpers(definition).ok).toBe(provider.supported); + expect(checkProviderHelpers(definition).ok).toBe(dispatches); }); if (!provider.supported || provider.provider === 'slack') continue; it(`${provider.provider}: consumes its upstream client through the mock transport`, async () => { diff --git a/packages/surface/src/effect-transport.ts b/packages/surface/src/effect-transport.ts index 3caaeb463..66bd2c7d5 100644 --- a/packages/surface/src/effect-transport.ts +++ b/packages/surface/src/effect-transport.ts @@ -1,4 +1,5 @@ import type { RelayClientOptions, RelayTransport } from '@relayfile/relay-helpers/transport'; +import { guardHelperMembers, helperProviderEntry, UnsupportedHelperMemberError } from './helper-support.js'; import type { Step } from './step.js'; /** Promise-returning client verbs become lazy, journal-owned steps. */ @@ -28,8 +29,12 @@ export function bindHelper(provider: string, factory: (options export function bindHelper(provider: string, factory: undefined, dispatch: EffectDispatcher): UnavailableHelper; export function bindHelper(provider: string, factory: HelperFactory | undefined, dispatch: EffectDispatcher): object { if (!factory) return Object.freeze({ available: false }); + // A partially supported provider guards member access: its namespace promises + // workflows its writeback catalog does not carry, so `f.gitlab.issues` must + // name what is available instead of reading `undefined`. + const partial = helperProviderEntry(provider)?.supported === 'partial'; function wrap(client: object, prefix = ''): object { - return Object.fromEntries(Object.entries(client).map(([key, value]) => { + const bound = Object.fromEntries(Object.entries(client).map(([key, value]) => { const verb = `${prefix}${key}`; if (typeof value === 'function') return [key, key === 'path' ? value.bind(client) @@ -39,21 +44,46 @@ export function bindHelper(provider: string, factory: HelperFactory | undefined, }]; return [key, value && typeof value === 'object' ? wrap(value, `${verb}.`) : value]; })); + return partial ? guardHelperMembers(provider, bound, prefix === '' ? undefined : prefix.slice(0, -1)) : bound; } return wrap(factory({ transport: unavailableTransport })); } +/** + * An authored envelope reaches `invokeHelper` without ever touching the bound + * helper's properties, so the same refusal is repeated here. Non-partial + * providers keep their existing generic diagnostics. + */ +function unknownMember( + provider: string, member: string, container: unknown, resource: string | undefined, kind: 'resource' | 'verb', +): Error { + if (helperProviderEntry(provider)?.supported !== 'partial') { + return new Error(kind === 'resource' ? 'Unknown helper resource' : 'Unknown helper verb'); + } + // `path` builds a path synchronously and is never dispatchable, so naming it + // as an available verb would send the author at a call that cannot work. + const available = container !== null && typeof container === 'object' + ? Object.keys(container).filter(key => kind === 'resource' || key !== 'path') : []; + return new UnsupportedHelperMemberError(provider, member, available, resource); +} + /** Resolve only own, generated client methods; never arbitrary prototype members. */ export async function invokeHelper(factory: HelperFactory, call: HelperCall, transport: RelayTransport): Promise { let target: unknown = factory({ transport }); const parts = call.verb.split('.'); + let resource: string | undefined; for (const part of parts.slice(0, -1)) { - if (!target || typeof target !== 'object' || !Object.hasOwn(target, part)) throw new Error('Unknown helper resource'); + if (!target || typeof target !== 'object' || !Object.hasOwn(target, part)) { + throw unknownMember(call.provider, part, target, resource, 'resource'); + } target = (target as Record)[part]; + resource = resource === undefined ? part : `${resource}.${part}`; } const verb = parts.at(-1)!; - if (!target || typeof target !== 'object' || !Object.hasOwn(target, verb) || verb === 'path') throw new Error('Unknown helper verb'); + if (!target || typeof target !== 'object' || !Object.hasOwn(target, verb) || verb === 'path') { + throw unknownMember(call.provider, verb, target, resource, 'verb'); + } const method = (target as Record)[verb]; - if (typeof method !== 'function') throw new Error('Unknown helper verb'); + if (typeof method !== 'function') throw unknownMember(call.provider, verb, target, resource, 'verb'); return await method.apply(target, call.args) ?? null; } diff --git a/packages/surface/src/helper-support.ts b/packages/surface/src/helper-support.ts new file mode 100644 index 000000000..bc86af8db --- /dev/null +++ b/packages/surface/src/helper-support.ts @@ -0,0 +1,83 @@ +import { helperProviders } from './helpers/providers.js'; + +/** + * How much of a provider's namespace the generated helper can actually reach. + * + * `true` — the upstream writeback client exists and every catalog resource + * dispatches. It is NOT a claim that the whole vendor API is reachable. + * `'partial'` — usable, but known to omit workflows the namespace suggests; + * `note` says which, and `resources` is the whole of what dispatches. + * `false` — no upstream writeback client at all. + */ +export type HelperSupport = true | 'partial' | false; + +export interface HelperProviderEntry { + readonly provider: string; + readonly namespace: string; + readonly mockEnv: string; + readonly supported: HelperSupport; + /** Writeback resources this helper exposes, sorted. */ + readonly resources: readonly string[]; + /** The author-facing limitation; present only where support is partial. */ + readonly note?: string; +} + +const entries: readonly HelperProviderEntry[] = helperProviders; + +/** The catalog row for a provider, with `note` typed as the optional it is. */ +export function helperProviderEntry(provider: string): HelperProviderEntry | undefined { + return entries.find(entry => entry.provider === provider); +} + +/** + * The one refusal wording for a helper member the provider does not have. + * + * `f.gitlab.issues` used to fail as `undefined is not a function`, which tells + * an author nothing about the writeback catalog behind the namespace. Every + * refusal names the requested member AND the available ones, so the limit is + * learned at the call site instead of by dumping the catalog. Preflight and + * the runtime guard share this function so they cannot word it differently. + */ +export function unsupportedHelperMemberMessage( + provider: string, member: string, available: readonly string[], resource?: string, +): string { + const entry = helperProviderEntry(provider); + const namespace = entry?.namespace ?? provider; + const path = `f.${namespace}${resource === undefined ? '' : `.${resource}`}.${member}`; + const scope = resource === undefined ? 'available resources' : `available verbs on ${resource}`; + return `${path} is unavailable; ${scope}: ${[...available].sort().join(', ')}.` + + (entry?.note === undefined ? '' : ` ${entry.note}`); +} + +export class UnsupportedHelperMemberError extends Error { + constructor( + readonly provider: string, + readonly member: string, + readonly available: readonly string[], + /** The resource the member was looked up on, for a bad verb. */ + readonly resource?: string, + ) { + super(unsupportedHelperMemberMessage(provider, member, available, resource)); + this.name = 'UnsupportedHelperMemberError'; + } +} + +/** + * Wrap a bound helper so an absent member refuses instead of reading + * `undefined`. Dot, bracket and aliased access all go through `get`. + * + * Ordinary object behavior is preserved: symbols, `then` (so the helper can be + * awaited or resolved), `toJSON`, inherited `Object.prototype` methods, key + * enumeration and spread. Nothing here dispatches an effect or performs + * provider I/O — refusing an unknown resource must not touch the provider. + */ +export function guardHelperMembers(provider: string, target: T, resource?: string): T { + return new Proxy(target, { + get(object, key, receiver) { + if (typeof key === 'string' && !(key in object) && key !== 'then' && key !== 'toJSON') { + throw new UnsupportedHelperMemberError(provider, key, Object.keys(object), resource); + } + return Reflect.get(object, key, receiver); + }, + }); +} diff --git a/packages/surface/src/helpers/README.md b/packages/surface/src/helpers/README.md index 05d64f644..3f71717b3 100644 --- a/packages/surface/src/helpers/README.md +++ b/packages/surface/src/helpers/README.md @@ -32,11 +32,28 @@ 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 one question only — how much of the namespace dispatches: + +- `true`: the upstream writeback client exists and every resource in + `resources` dispatches. It is **not** a claim that the whole vendor API is + reachable. +- `'partial'`: usable, but known to omit workflows the namespace suggests. + `note` says which, and `resources` is the whole of what dispatches. Reaching + for anything else refuses at the call site naming what is available, rather + than failing as `undefined is not a function`. +- `false`: no upstream writeback client at all. + +`f.gitlab` is the current `'partial'` entry: it carries `comments` and +`discussions` only, so issue list/read/create and merge-request +list/read/create are unavailable through it, while `f.github` carries issues, +pull-requests, reviews, refs, merge, and close-pull-request. The note lives in +`PARTIAL_SUPPORT` in `scripts/generate-helpers.mjs`; a later upstream release +must revisit it deliberately, since a larger resource count is not by itself a +promotion. diff --git a/packages/surface/src/helpers/gitlab.ts b/packages/surface/src/helpers/gitlab.ts index 3d338f0c8..e6d9aeced 100644 --- a/packages/surface/src/helpers/gitlab.ts +++ b/packages/surface/src/helpers/gitlab.ts @@ -4,6 +4,11 @@ import { gitlabClient } from "@relayfile/relay-helpers"; import { bindHelper, type EffectDispatcher, type JournalHelper } from "../effect-transport.js"; +/** + * Partial support. Comments and discussions only. Issue list/read/create and merge-request list/read/create are unavailable through f.gitlab. + * + * Available resources: comments, discussions. + */ export type GitlabHelper = JournalHelper>; export const createGitlabHelper = (dispatch: EffectDispatcher): GitlabHelper => diff --git a/packages/surface/src/helpers/providers.ts b/packages/surface/src/helpers/providers.ts index 9d29e4d52..61f731052 100644 --- a/packages/surface/src/helpers/providers.ts +++ b/packages/surface/src/helpers/providers.ts @@ -1,305 +1,547 @@ // GENERATED by scripts/generate-helpers.mjs — do not edit. // Run `npm run gen --prefix packages/surface` from the repository root. +// `supported: true` designates a client whose catalog resources dispatch; +// it is not a claim of exhaustive vendor API coverage. `"partial"` marks a +// client that is usable for `resources` but is known to omit workflows its +// namespace suggests — see `note`. `false` means no upstream writeback client. export const helperProviders = [ { "provider": "airtable", "namespace": "airtable", "mockEnv": "RELAYFLOWS_AIRTABLE_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "asana", "namespace": "asana", "mockEnv": "RELAYFLOWS_ASANA_MOCK", - "supported": true + "supported": true, + "resources": [ + "projects", + "sections", + "tasks" + ] }, { "provider": "azure-blob", "namespace": "azureBlob", "mockEnv": "RELAYFLOWS_AZURE_BLOB_MOCK", - "supported": true + "supported": true, + "resources": [ + "blobs", + "event-subscriptions" + ] }, { "provider": "box", "namespace": "box", "mockEnv": "RELAYFLOWS_BOX_MOCK", - "supported": true + "supported": true, + "resources": [ + "files", + "webhooks" + ] }, { "provider": "calendly", "namespace": "calendly", "mockEnv": "RELAYFLOWS_CALENDLY_MOCK", - "supported": true + "supported": true, + "resources": [ + "event-types", + "invitees", + "scheduled-events" + ] }, { "provider": "clickup", "namespace": "clickup", "mockEnv": "RELAYFLOWS_CLICKUP_MOCK", - "supported": true + "supported": true, + "resources": [ + "comments", + "folders", + "lists", + "tasks" + ] }, { "provider": "cloudflare", "namespace": "cloudflare", "mockEnv": "RELAYFLOWS_CLOUDFLARE_MOCK", - "supported": true + "supported": true, + "resources": [ + "d1-databases", + "dns-records", + "kv-namespaces", + "notification-events", + "notification-policies", + "notification-webhooks", + "pages-projects", + "queues", + "r2-buckets", + "tunnels", + "worker-usage", + "workers-scripts", + "zones" + ] }, { "provider": "confluence", "namespace": "confluence", "mockEnv": "RELAYFLOWS_CONFLUENCE_MOCK", - "supported": true + "supported": true, + "resources": [ + "pages" + ] }, { "provider": "daytona", "namespace": "daytona", "mockEnv": "RELAYFLOWS_DAYTONA_MOCK", - "supported": true + "supported": true, + "resources": [ + "usage" + ] }, { "provider": "docker-hub", "namespace": "dockerHub", "mockEnv": "RELAYFLOWS_DOCKER_HUB_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "dropbox", "namespace": "dropbox", "mockEnv": "RELAYFLOWS_DROPBOX_MOCK", - "supported": true + "supported": true, + "resources": [ + "cursors", + "files", + "folders", + "shared-folders", + "shared-links" + ] }, { "provider": "fathom", "namespace": "fathom", "mockEnv": "RELAYFLOWS_FATHOM_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "gcp", "namespace": "gcp", "mockEnv": "RELAYFLOWS_GCP_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "gcs", "namespace": "gcs", "mockEnv": "RELAYFLOWS_GCS_MOCK", - "supported": true + "supported": true, + "resources": [ + "notifications", + "objects" + ] }, { "provider": "github", "namespace": "github", "mockEnv": "RELAYFLOWS_GITHUB_MOCK", - "supported": true + "supported": true, + "resources": [ + "close-pull-request", + "issue-comments", + "issues", + "merge", + "pull-requests", + "refs", + "replies", + "reviews" + ] }, { "provider": "gitlab", "namespace": "gitlab", "mockEnv": "RELAYFLOWS_GITLAB_MOCK", - "supported": true + "supported": "partial", + "resources": [ + "comments", + "discussions" + ], + "note": "Comments and discussions only. Issue list/read/create and merge-request list/read/create are unavailable through f.gitlab." }, { "provider": "gmail", "namespace": "gmail", "mockEnv": "RELAYFLOWS_GMAIL_MOCK", - "supported": true + "supported": true, + "resources": [ + "drafts", + "threads", + "watches" + ] }, { "provider": "google-calendar", "namespace": "googleCalendar", "mockEnv": "RELAYFLOWS_GOOGLE_CALENDAR_MOCK", - "supported": true + "supported": true, + "resources": [ + "events" + ] }, { "provider": "google-drive", "namespace": "googleDrive", "mockEnv": "RELAYFLOWS_GOOGLE_DRIVE_MOCK", - "supported": true + "supported": true, + "resources": [ + "channels", + "files" + ] }, { "provider": "granola", "namespace": "granola", "mockEnv": "RELAYFLOWS_GRANOLA_MOCK", - "supported": true + "supported": true, + "resources": [ + "folders", + "notes" + ] }, { "provider": "hubspot", "namespace": "hubspot", "mockEnv": "RELAYFLOWS_HUBSPOT_MOCK", - "supported": true + "supported": true, + "resources": [ + "companies", + "contacts", + "deals", + "tickets" + ] }, { "provider": "intercom", "namespace": "intercom", "mockEnv": "RELAYFLOWS_INTERCOM_MOCK", - "supported": true + "supported": true, + "resources": [ + "companies", + "contacts", + "conversations" + ] }, { "provider": "jira", "namespace": "jira", "mockEnv": "RELAYFLOWS_JIRA_MOCK", - "supported": true + "supported": true, + "resources": [ + "comments", + "issues", + "projects", + "transitions" + ] }, { "provider": "linear", "namespace": "linear", "mockEnv": "RELAYFLOWS_LINEAR_MOCK", - "supported": true + "supported": true, + "resources": [ + "agent-activities", + "comments", + "issues", + "labels", + "project-issue-assignments", + "projects" + ] }, { "provider": "mailgun", "namespace": "mailgun", "mockEnv": "RELAYFLOWS_MAILGUN_MOCK", - "supported": true + "supported": true, + "resources": [ + "lists", + "members", + "messages" + ] }, { "provider": "mixpanel", "namespace": "mixpanel", "mockEnv": "RELAYFLOWS_MIXPANEL_MOCK", - "supported": true + "supported": true, + "resources": [ + "cohorts", + "events", + "profiles" + ] }, { "provider": "neon", "namespace": "neon", "mockEnv": "RELAYFLOWS_NEON_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "notion", "namespace": "notion", "mockEnv": "RELAYFLOWS_NOTION_MOCK", - "supported": true + "supported": true, + "resources": [ + "comments", + "content", + "pages", + "properties" + ] }, { "provider": "onedrive", "namespace": "onedrive", "mockEnv": "RELAYFLOWS_ONEDRIVE_MOCK", - "supported": true + "supported": true, + "resources": [ + "items", + "subscriptions" + ] }, { "provider": "pipedrive", "namespace": "pipedrive", "mockEnv": "RELAYFLOWS_PIPEDRIVE_MOCK", - "supported": true + "supported": true, + "resources": [ + "activities", + "deals", + "organizations", + "persons" + ] }, { "provider": "postgres", "namespace": "postgres", "mockEnv": "RELAYFLOWS_POSTGRES_MOCK", - "supported": true + "supported": true, + "resources": [ + "listeners", + "rows" + ] }, { "provider": "posthog", "namespace": "posthog", "mockEnv": "RELAYFLOWS_POSTHOG_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "ramp", "namespace": "ramp", "mockEnv": "RELAYFLOWS_RAMP_MOCK", - "supported": true + "supported": true, + "resources": [ + "accounting-accounts", + "accounting-fields", + "bills", + "business", + "departments", + "entities", + "item-receipts", + "locations", + "merchants", + "purchase-orders", + "receipts", + "reimbursements", + "repayments", + "spend-programs", + "transactions", + "transfers", + "users", + "vendor-agreements", + "vendors" + ] }, { "provider": "recall", "namespace": "recall", "mockEnv": "RELAYFLOWS_RECALL_MOCK", - "supported": true + "supported": true, + "resources": [ + "recordings" + ] }, { "provider": "reddit", "namespace": "reddit", "mockEnv": "RELAYFLOWS_REDDIT_MOCK", - "supported": true + "supported": true, + "resources": [ + "posts", + "subreddits" + ] }, { "provider": "redis", "namespace": "redis", "mockEnv": "RELAYFLOWS_REDIS_MOCK", - "supported": true + "supported": true, + "resources": [ + "keys", + "listeners" + ] }, { "provider": "s3", "namespace": "s3", "mockEnv": "RELAYFLOWS_S3_MOCK", - "supported": true + "supported": true, + "resources": [ + "objects", + "queues" + ] }, { "provider": "salesforce", "namespace": "salesforce", "mockEnv": "RELAYFLOWS_SALESFORCE_MOCK", - "supported": true + "supported": true, + "resources": [ + "accounts", + "cases", + "contacts", + "leads", + "opportunities" + ] }, { "provider": "segment", "namespace": "segment", "mockEnv": "RELAYFLOWS_SEGMENT_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "sendgrid", "namespace": "sendgrid", "mockEnv": "RELAYFLOWS_SENDGRID_MOCK", - "supported": true + "supported": true, + "resources": [ + "contacts", + "mail" + ] }, { "provider": "sharepoint", "namespace": "sharepoint", "mockEnv": "RELAYFLOWS_SHAREPOINT_MOCK", - "supported": true + "supported": true, + "resources": [ + "items", + "subscriptions" + ] }, { "provider": "shopify", "namespace": "shopify", "mockEnv": "RELAYFLOWS_SHOPIFY_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "shortcut", "namespace": "shortcut", "mockEnv": "RELAYFLOWS_SHORTCUT_MOCK", - "supported": true + "supported": true, + "resources": [ + "categories", + "custom-fields", + "epics", + "groups", + "iterations", + "labels", + "milestones", + "projects", + "stories" + ] }, { "provider": "slack", "namespace": "slack", "mockEnv": "RELAYFLOWS_SLACK_MOCK", - "supported": true + "supported": true, + "resources": [ + "direct-messages", + "messages", + "reactions", + "replies" + ] }, { "provider": "stripe", "namespace": "stripe", "mockEnv": "RELAYFLOWS_STRIPE_MOCK", - "supported": true + "supported": true, + "resources": [] }, { "provider": "teams", "namespace": "teams", "mockEnv": "RELAYFLOWS_TEAMS_MOCK", - "supported": true + "supported": true, + "resources": [ + "messages", + "replies" + ] }, { "provider": "telegram", "namespace": "telegram", "mockEnv": "RELAYFLOWS_TELEGRAM_MOCK", - "supported": true + "supported": true, + "resources": [ + "callback-queries", + "commands", + "inline-queries", + "menu-button", + "messages", + "reactions" + ] }, { "provider": "webhook-server", "namespace": "webhookServer", "mockEnv": "RELAYFLOWS_WEBHOOK_SERVER_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "x", "namespace": "x", "mockEnv": "RELAYFLOWS_X_MOCK", - "supported": false + "supported": false, + "resources": [] }, { "provider": "zendesk", "namespace": "zendesk", "mockEnv": "RELAYFLOWS_ZENDESK_MOCK", - "supported": true + "supported": true, + "resources": [ + "comments", + "tickets", + "users" + ] } ] as const; diff --git a/packages/surface/src/runtime.ts b/packages/surface/src/runtime.ts index 85813bfd9..a68b90532 100644 --- a/packages/surface/src/runtime.ts +++ b/packages/surface/src/runtime.ts @@ -13,3 +13,10 @@ export { createHelpers } from "./helpers/index.js"; export { helperProviders } from "./helpers/providers.js"; export { helperClients } from "./helpers/clients.js"; export { invokeHelper, type HelperCall } from "./effect-transport.js"; +export { + helperProviderEntry, + unsupportedHelperMemberMessage, + UnsupportedHelperMemberError, + type HelperProviderEntry, + type HelperSupport, +} from "./helper-support.js"; diff --git a/packages/surface/tests/helper-support.test.ts b/packages/surface/tests/helper-support.test.ts new file mode 100644 index 000000000..8af604c56 --- /dev/null +++ b/packages/surface/tests/helper-support.test.ts @@ -0,0 +1,116 @@ +import { expect, it } from 'vitest'; +import { + createHelpers, helperClients, helperProviders, helperProviderEntry, invokeHelper, + unsupportedHelperMemberMessage, UnsupportedHelperMemberError, type HelperCall, +} from '../src/runtime.js'; + +const calls: HelperCall[] = []; +const helpers = () => createHelpers(call => { calls.push(call); return {} as never; }); +/** Reaching for an absent resource must refuse without any provider I/O. */ +const refusingTransport = { + async read() { throw new Error('provider I/O'); }, + async list() { throw new Error('provider I/O'); }, + async write() { throw new Error('provider I/O'); }, +}; +const gitlab = () => helpers().gitlab as unknown as Record; + +it('records gitlab as partial with its resources and note, and github as fully dispatching', () => { + const entry = helperProviderEntry('gitlab')!; + expect(entry.supported).toBe('partial'); + expect(entry.resources).toEqual(['comments', 'discussions']); + expect(entry.note).toBe('Comments and discussions only. Issue list/read/create and ' + + 'merge-request list/read/create are unavailable through f.gitlab.'); + const github = helperProviderEntry('github')!; + expect(github.supported).toBe(true); + expect(github.resources).toContain('issues'); + expect(github.resources).toContain('pull-requests'); + expect(github.note).toBeUndefined(); +}); + +it('keeps every catalog entry to the three support designations, with resources and notes to match', () => { + for (const provider of helperProviders) { + expect([true, 'partial', false], provider.provider).toContain(provider.supported); + // `note` explains a partial's limit; nothing else has a limit to explain. + expect('note' in provider, provider.provider).toBe(provider.supported === 'partial'); + expect([...provider.resources], provider.provider).toEqual([...provider.resources].sort()); + if (provider.supported === false) expect(provider.resources, provider.provider).toEqual([]); + if (provider.supported === 'partial') expect(provider.resources.length, provider.provider).toBeGreaterThan(0); + } +}); + +it('refuses an absent gitlab resource by dot, bracket and aliased access, naming what is available', () => { + const f = gitlab(); + const expected = 'f.gitlab.issues is unavailable; available resources: comments, discussions.' + + ' Comments and discussions only. Issue list/read/create and merge-request' + + ' list/read/create are unavailable through f.gitlab.'; + expect(() => f.issues).toThrow(UnsupportedHelperMemberError); + expect(() => f.issues).toThrow(expected); + expect(() => f['merge_requests']).toThrow('f.gitlab.merge_requests is unavailable; available resources: comments, discussions.'); + const member = ['merge', 'Requests'].join(''); + expect(() => f[member]).toThrow('f.gitlab.mergeRequests is unavailable'); + expect(() => f.comments.merge).toThrow('f.gitlab.comments.merge is unavailable; available verbs on comments: list, path, read, write.'); + expect(calls).toEqual([]); +}); + +it('carries the refused member, its resource and the available names structurally', () => { + try { gitlab().issues; expect.unreachable('f.gitlab.issues resolved'); } catch (error) { + const refusal = error as UnsupportedHelperMemberError; + expect(refusal.name).toBe('UnsupportedHelperMemberError'); + expect(refusal.provider).toBe('gitlab'); + expect(refusal.member).toBe('issues'); + expect(refusal.resource).toBeUndefined(); + expect(refusal.available).toEqual(['comments', 'discussions']); + } +}); + +it('words the refusal once, for preflight and the runtime guard alike', () => { + const guarded = (() => { try { gitlab().issues; return ''; } catch (error) { return (error as Error).message; } })(); + expect(guarded).toBe(unsupportedHelperMemberMessage('gitlab', 'issues', ['comments', 'discussions'])); +}); + +it('leaves ordinary object behavior on the guarded helper intact', async () => { + const f = gitlab(); + expect(Object.keys(f)).toEqual(['comments', 'discussions']); + expect([...Object.keys({ ...f })]).toEqual(['comments', 'discussions']); + expect(await f).toBe(f); + expect((f as any).then).toBeUndefined(); + expect((f as any).toJSON).toBeUndefined(); + expect((f as Record)[Symbol.toStringTag]).toBeUndefined(); + expect(f.hasOwnProperty('comments')).toBe(true); + expect(typeof f.comments.write).toBe('function'); +}); + +it('keeps path a synchronous builder and dispatch unchanged on the resources gitlab does carry', () => { + calls.length = 0; + const f = gitlab(); + const params = { projectPath: 'g/p', issueIid: '1', slug: 's' }; + expect(f.comments.path(params)).toBe('/gitlab/projects/g%2Fp/issues/1__s/comments'); + expect(calls).toEqual([]); + f.comments.write(params, { text: 'hi' }); + expect(calls).toEqual([{ type: 'effect', provider: 'gitlab', verb: 'comments.write', + args: [params, { text: 'hi' }] }]); +}); + +it('does not guard a fully supported provider', () => { + const f = helpers().github as unknown as Record; + expect(f.nonexistent).toBeUndefined(); + expect(typeof (f.issues as Record).write).toBe('function'); +}); + +it('repeats the refusal for an authored envelope that never touched the helper, without provider I/O', async () => { + const gitlabFactory = helperClients['gitlab']!; + await expect(invokeHelper(gitlabFactory, + { type: 'effect', provider: 'gitlab', verb: 'issues.write', args: [] }, refusingTransport)) + .rejects.toThrow('f.gitlab.issues is unavailable; available resources: comments, discussions.'); + await expect(invokeHelper(gitlabFactory, + { type: 'effect', provider: 'gitlab', verb: 'comments.merge', args: [] }, refusingTransport)) + .rejects.toThrow('f.gitlab.comments.merge is unavailable; available verbs on comments: list, read, write.'); + // `path` is a synchronous builder; dispatching it stays banned and is not + // advertised as a verb an author could call instead. + await expect(invokeHelper(gitlabFactory, + { type: 'effect', provider: 'gitlab', verb: 'comments.path', args: [] }, refusingTransport)) + .rejects.toThrow(UnsupportedHelperMemberError); + await expect(invokeHelper(helperClients['github']!, + { type: 'effect', provider: 'github', verb: 'issues.nope', args: [] }, refusingTransport)) + .rejects.toThrow('Unknown helper verb'); +}); diff --git a/packages/surface/tests/helpers-typecheck-fail.test-d.ts b/packages/surface/tests/helpers-typecheck-fail.test-d.ts index 6096b97ed..fe598c657 100644 --- a/packages/surface/tests/helpers-typecheck-fail.test-d.ts +++ b/packages/surface/tests/helpers-typecheck-fail.test-d.ts @@ -15,4 +15,8 @@ export function rejectedHelpers(f: Ctx): void { f.notarealprovider.anything(); // @ts-expect-error resource clients do not have runtime dispatch yet. f.slack.messages.list(); + // @ts-expect-error f.gitlab carries comments and discussions only. + f.gitlab.issues.list({ projectPath: 'g/p' }); + // @ts-expect-error merge requests are not in gitlab's writeback catalog. + f.gitlab.mergeRequests.write({ projectPath: 'g/p' }, { title: 'x' }); } diff --git a/scripts/generate-helpers.mjs b/scripts/generate-helpers.mjs index c33a41a33..9e679c662 100644 --- a/scripts/generate-helpers.mjs +++ b/scripts/generate-helpers.mjs @@ -25,6 +25,19 @@ if (values['adapters-dir']) { } const camel = name => name.replace(/-([a-z])/g, (_, c) => c.toUpperCase()); const title = name => camel(name).replace(/^./, c => c.toUpperCase()); +/** + * Providers whose upstream writeback catalog covers materially less than their + * namespace suggests. `supported: true` has always meant "this client exists + * and its resources dispatch", never "every vendor endpoint is reachable"; + * `'partial'` is the narrower designation for a gap an author would otherwise + * find only by dumping the catalog. A later upstream release must revisit this + * map deliberately — a larger resource count is not by itself a promotion. + */ +const PARTIAL_SUPPORT = { + gitlab: 'Comments and discussions only. Issue list/read/create and ' + + 'merge-request list/read/create are unavailable through f.gitlab.', +}; +const resourcesOf = provider => Object.keys(catalog[provider] ?? {}).sort(); const header = '// GENERATED by scripts/generate-helpers.mjs — do not edit.\n' + '// Run `npm run gen --prefix packages/surface` from the repository root.\n'; const destination = resolve(values['out-dir']); @@ -72,6 +85,11 @@ for (const provider of providers.filter(p => p !== 'slack')) { : typeof upstream[factory] === 'function' ? `import { ${factory} } from "@relayfile/relay-helpers";\n` : supported ? `import { providerClient } from "@relayfile/relay-helpers";\nimport type { RelayClientOptions } from "@relayfile/relay-helpers/transport";\nconst ${factory} = (options: RelayClientOptions) => providerClient("${provider}", options);\n` : '') + `import { bindHelper, type EffectDispatcher, type JournalHelper${!supported && !special ? ', type UnavailableHelper' : ''} } from "../effect-transport.js";\n\n` + // Partial support belongs on the editor-visible type: an author reaching for + // f.gitlab must read the limit here, not by dumping the writeback catalog. + + (PARTIAL_SUPPORT[provider] === undefined ? '' + : `/**\n * Partial support. ${PARTIAL_SUPPORT[provider]}\n *\n` + + ` * Available resources: ${resourcesOf(provider).join(', ')}.\n */\n`) + `export type ${title(provider)}Helper = ${supported || special ? `JournalHelper>` : 'UnavailableHelper'};\n\n` + `export const create${title(provider)}Helper = (dispatch: EffectDispatcher): ${title(provider)}Helper =>\n` + ` bindHelper(${JSON.stringify(provider)}, ${supported || special ? factory : 'undefined'}, dispatch);\n`; @@ -85,9 +103,16 @@ files['index.ts'] = header + '\nimport type { SlackHelper } from "./slack.js";\n + providers.filter(p => p !== 'slack').map(p => ` ${camel(p)}: create${title(p)}Helper(dispatch),`).join('\n') + '\n };\n}\n'; files['clients.ts'] = header + '\nimport * as upstream from "@relayfile/relay-helpers";\nimport * as custom from "../helper-clients.js";\nimport type { HelperFactory } from "../effect-transport.js";\n\nexport const helperClients: Readonly> = {\n' + providers.filter(p => p !== 'slack' && (p === 'stripe' || catalog[p])).map(p => ` "${p}": ${['github', 'notion', 'stripe'].includes(p) ? `custom.${camel(p)}Client` : typeof upstream[`${camel(p)}Client`] === 'function' ? `upstream.${camel(p)}Client` : `(options) => upstream.providerClient("${p}", options)`},`).join('\n') + '\n};\n'; -files['providers.ts'] = header + '\nexport const helperProviders = ' + JSON.stringify(providers.map(p => ({ +files['providers.ts'] = header + + '\n// `supported: true` designates a client whose catalog resources dispatch;\n' + + '// it is not a claim of exhaustive vendor API coverage. `"partial"` marks a\n' + + '// client that is usable for `resources` but is known to omit workflows its\n' + + '// namespace suggests — see `note`. `false` means no upstream writeback client.\n' + + 'export const helperProviders = ' + JSON.stringify(providers.map(p => ({ provider: p, namespace: camel(p), mockEnv: `RELAYFLOWS_${p.replaceAll('-', '_').toUpperCase()}_MOCK`, - supported: p === 'stripe' || catalog[p] !== undefined, + supported: PARTIAL_SUPPORT[p] !== undefined ? 'partial' : p === 'stripe' || catalog[p] !== undefined, + resources: resourcesOf(p), + ...(PARTIAL_SUPPORT[p] === undefined ? {} : { note: PARTIAL_SUPPORT[p] }), })), null, 2) + ' as const;\n'; for (const [name, content] of Object.entries(files)) writeFileSync(join(destination, name), content); console.log(`Generated ${providers.length} provider helpers (${providers.filter(p => !catalog[p] && p !== 'stripe').length} without upstream writeback clients)`); From 889198d0cf4b773aa08aadefacd8b752711df2a4 Mon Sep 17 00:00:00 2001 From: Relayflow Date: Sun, 20 Sep 2026 17:47:56 +0000 Subject: [PATCH 2/6] fix: keep the gitlab refusal loadable on a published surface, and silent on code it never read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refusal for a member `f.gitlab` does not carry was reaching for surface exports that the pinned, published surface does not ship, and was reading text that is not a member access as one. - The SDK no longer imports `unsupportedHelperMemberMessage` or `UnsupportedHelperMemberError`. Both are unreleased, and this source is installed against a published surface in the schema `validate` job, where a missing named export fails the whole module at load — before preflight can run. The wording is restated locally and pinned equal to the surface's in test; the envelope remap matches `error.name`, which is also correct across the realm boundary an authored flow file's own surface copy creates. - Static inspection now admits every member the runtime guard still resolves. The guard refuses only what the bound object lacks and is neither `then` nor `toJSON`, so `f.gitlab.hasOwnProperty('comments')` returns `true` at run time; `flows check` must not reject feature detection that works. - A regular-expression literal is blanked with the other data literals, so `/f.gitlab.issues/.test(line)` — which inspects text and reaches no helper — no longer refuses. Ambiguous `/` resolves to "regex", which can only withdraw a static refusal and leave the runtime guard to make it. - A body that binds the context parameter's name again is left to the runtime guard. Renaming a local callback parameter cannot decide whether a flow is admitted, and an inner `f` need not be the flow context at all. The last three can only withdraw refusals, never invent them; the runtime guard remains the backstop for everything they decline to judge. Co-Authored-By: Claude --- packages/sdk/src/authored-flow-executor.ts | 12 +- packages/sdk/src/helper-preflight.ts | 63 ++++++++-- packages/sdk/src/source-scan.ts | 111 +++++++++++++++++- .../sdk/tests/helper-partial-support.test.ts | 66 +++++++++++ packages/surface/tests/helper-support.test.ts | 11 +- 5 files changed, 240 insertions(+), 23 deletions(-) diff --git a/packages/sdk/src/authored-flow-executor.ts b/packages/sdk/src/authored-flow-executor.ts index 495a9c9d1..6cdeeea0d 100644 --- a/packages/sdk/src/authored-flow-executor.ts +++ b/packages/sdk/src/authored-flow-executor.ts @@ -23,10 +23,7 @@ import { type RunCompletionReason as SurfaceRunCompletionReason, type Step, } from '@relayflows/surface'; -import { - createHelpers, helperProviders, UnsupportedHelperMemberError, - type HelperCall, type FlowHandle, -} from '@relayflows/surface/runtime'; +import { createHelpers, helperProviders, type HelperCall, type FlowHandle } from '@relayflows/surface/runtime'; import { join } from 'node:path'; import { observeStep, type ProgressEvent } from './progress.js'; import { parseStepTimeout } from './compile.js'; @@ -545,7 +542,12 @@ export async function executeAuthoredFlow( // an unexplained body crash, so it is remapped onto the code preflight // already uses for the same refusal — the message is the surface's, not a // second wording. Every other failure is rethrown exactly as thrown. - bodyFailure = error instanceof UnsupportedHelperMemberError + // + // Recognised by `name`, not `instanceof`: an authored flow file resolves + // `@relayflows/surface` from its OWN node_modules, so the class that threw + // need not be the one this module imported — and a pinned surface older + // than the guard does not export the class at all. + bodyFailure = error instanceof Error && error.name === 'UnsupportedHelperMemberError' ? new AuthoredFlowExecutionError('helper_provider.unsupported', error.message) : error; } diff --git a/packages/sdk/src/helper-preflight.ts b/packages/sdk/src/helper-preflight.ts index ee8428cb0..2e91e9cfb 100644 --- a/packages/sdk/src/helper-preflight.ts +++ b/packages/sdk/src/helper-preflight.ts @@ -1,5 +1,5 @@ -import { helperProviders, unsupportedHelperMemberMessage } from '@relayflows/surface/runtime'; -import { codeOnly } from './source-scan.js'; +import { helperProviders } from '@relayflows/surface/runtime'; +import { codeOnly, rebindsIdentifier } from './source-scan.js'; import type { PreflightResult, PreflightDiagnostic } from './preflight.js'; /** Static discovery never executes the body; dynamic aliases are checked at call time. */ @@ -11,6 +11,8 @@ export function preflightHelpers( 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, '\\$&'); + /** End of the body's own parameter declaration, which is not a rebinding of itself. */ + const declared = parameter?.[0].length ?? 0; const diagnostics: PreflightDiagnostic[] = []; for (const provider of helperProviders) { const { namespace, supported } = provider; @@ -25,13 +27,14 @@ export function preflightHelpers( // writeback route that no client carries, and a body that would die on // `undefined is not a function` should say so with the names that do work. const missing = supported === 'partial' - ? unavailableMembers(root, namespace, provider.resources, body) : []; + ? unavailableMembers(root, declared, namespace, provider.resources, body) : []; if (!supported) { diagnostics.push({ severity: 'refusal', kind: 'helper_provider.unsupported', message: `f.${namespace} has no upstream relayfile writeback client.` }); } else if (missing.length > 0) { diagnostics.push({ severity: 'refusal', kind: 'helper_provider.unsupported', - message: unsupportedHelperMemberMessage(provider.provider, missing[0]!, provider.resources) }); + message: unavailableMessage(namespace, missing[0]!, provider.resources, + 'note' in provider ? provider.note : undefined) }); } else if (!fact.mock && !fact.mount) { diagnostics.push({ severity: 'refusal', kind: provider.provider === 'slack' ? (fact.token?.trim() ? 'helper_slack.mount_required' : 'helper_slack.credential_missing') : 'helper_provider.mount_required', @@ -44,27 +47,67 @@ export function preflightHelpers( return { ok: diagnostics.length === 0, gates: [], resolutions: [], diagnostics }; } +/** + * The refusal wording for a helper member the provider does not have. + * + * The surface words the same refusal, in `unsupportedHelperMemberMessage`, for + * the guard that catches a computed name at the call site. It is restated here + * rather than imported because this SDK source is installed against a + * PUBLISHED surface — importing a named export the pinned version has not + * shipped fails the whole module at load, before preflight can run at all. The + * catalog data behind the wording (`resources`, `note`) degrades quietly by + * comparison: an older catalog carries no `'partial'` entry, so this refusal + * simply does not arise. `tests/helper-partial-support.test.ts` pins the two + * wordings to the same string. + */ +function unavailableMessage( + namespace: string, member: string, available: readonly string[], note: string | undefined, +): string { + return `f.${namespace}.${member} is unavailable; available resources: ${[...available].sort().join(', ')}.` + + (note === undefined ? '' : ` ${note}`); +} + /** * Members read off `f.` in the body that the provider does not * expose, in source order. * * Only what is statically evident counts: a direct `.member` or `['member']` * on the context parameter this body actually declares, matched against - * `codeOnly`, where comments and data strings are blanked but a literal in - * property-key position is not. A computed name is not decided here — the - * surface's own property guard refuses it at call time. + * `codeOnly`, where comments, data strings and regex literals are blanked but + * a literal in property-key position is not. A computed name is not decided + * here — the surface's own property guard refuses it at call time — and + * neither is a body that binds the parameter's name again, where `f.gitlab` + * need not be the flow context at all. */ function unavailableMembers( - root: string | undefined, namespace: string, resources: readonly string[], body: string, + root: string | undefined, declared: number, namespace: string, resources: readonly string[], body: string, ): string[] { if (root === undefined) return []; + const code = codeOnly(body); + if (rebindsIdentifier(code, root, declared)) return []; const access = new RegExp( `(?:^|[^\\w$.])${root}\\s*(?:\\.\\s*${namespace}|\\[\\s*['"]${namespace}['"]\\s*\\])` + `\\s*(?:\\.\\s*([\\w$]+)|\\[\\s*['"]([\\w$]+)['"]\\s*\\])`, 'gu'); const found: string[] = []; - for (const match of codeOnly(body).matchAll(access)) { + for (const match of code.matchAll(access)) { const member = match[1] ?? match[2]!; - if (!resources.includes(member) && !found.includes(member)) found.push(member); + if (!resolves(member, resources) && !found.includes(member)) found.push(member); } return found; } + +/** + * Whether the guarded helper still resolves `member`. + * + * The surface's guard refuses only what is not `in` the bound object and is + * neither `then` nor `toJSON`, so a partial helper keeps ordinary object + * behavior: `f.gitlab.hasOwnProperty('comments')` returns `true` and + * `f.gitlab.toJSON` reads as `undefined`. Preflight has to admit exactly the + * same members, or `flows check` rejects introspection that runs. None of + * these is dispatchable: `invokeHelper` resolves a verb with `Object.hasOwn`, + * which no inherited name satisfies. + */ +function resolves(member: string, resources: readonly string[]): boolean { + return resources.includes(member) || member === 'then' || member === 'toJSON' + || Reflect.has(Object.prototype, member); +} diff --git a/packages/sdk/src/source-scan.ts b/packages/sdk/src/source-scan.ts index e3fa9436e..5db581234 100644 --- a/packages/sdk/src/source-scan.ts +++ b/packages/sdk/src/source-scan.ts @@ -41,6 +41,50 @@ export function matchingClose(text: string, open: number): number { return stack.length === 0 ? i - 1 : -1; } +/** + * Index of the `/` closing the regular-expression literal opening at `start`, + * with `\` escapes and `[…]` classes honored; -1 when no literal ends there. + * + * A regex literal cannot span a line, so an unclosed one is not a literal at + * all — it is a division the caller should read as code, which is why this + * stops at the first newline rather than running to the end of the body. + */ +function regexEnd(text: string, start: number): number { + let inClass = false; + let i = start + 1; + while (i < text.length) { + const ch = text[i]!; + if (ch === '\n') return -1; + if (ch === '\\') { if (i + 1 >= text.length || text[i + 1] === '\n') return -1; i += 2; continue; } + if (inClass) { if (ch === ']') inClass = false; } + else if (ch === '[') inClass = true; + else if (ch === '/') return i; + i += 1; + } + return -1; +} + +/** Keywords a regex literal may directly follow; after any other word it is division. */ +const regexKeywords = new Set(['await', 'case', 'delete', 'do', 'else', 'in', 'instanceof', + 'new', 'of', 'return', 'throw', 'typeof', 'void', 'yield']); + +/** + * Whether a `/` at the end of the code-only prefix `before` opens a regex + * literal rather than dividing. + * + * Deciding this exactly needs a parser. Where it cannot be decided — after `)` + * or `}`, which end an operand and a control-flow head alike — this answers + * "regex", because the two mistakes are not symmetric: reading a division as a + * literal only blanks text, which can withdraw a refusal and leave the runtime + * guard to make it, while reading a literal as code invents a refusal for a + * flow that never touched the helper. + */ +function opensRegex(before: string): boolean { + const word = before.match(/[\w$]+[^\S\n]*$/u); + if (word !== null) return regexKeywords.has(word[0].trimEnd()); + return !/\][^\S\n]*$/u.test(before); +} + /** Index of the quote closing the string opening at `start` (template `${…}` skipped); -1 if unterminated. */ export function stringEnd(text: string, start: number): number { const quote = text[start]!; @@ -61,12 +105,16 @@ export function stringEnd(text: string, start: number): number { } /** - * The body with every comment and string/template replaced by blanks of the - * same length, so a pattern may be matched against code alone at unchanged - * offsets. Blanking only ever REMOVES text: a match found here was in the - * source, which is why the result can refuse a flow. What it hides — an access - * built inside a template interpolation, say — falls through to the runtime - * guard rather than becoming a refusal of something the author did not write. + * The body with every comment, string/template and regular-expression literal + * replaced by blanks of the same length, so a pattern may be matched against + * code alone at unchanged offsets. Blanking only ever REMOVES text: a match + * found here was in the source, which is why the result can refuse a flow. + * What it hides — an access built inside a template interpolation, say — falls + * through to the runtime guard rather than becoming a refusal of something the + * author did not write. + * + * A regex literal is data too: `/f.gitlab.issues/.test(line)` inspects text + * and reaches no helper, so its contents must not read as a member access. * * One literal is kept, because it is syntax rather than data: an * identifier-shaped quoted string between `[` and `]` is the property name of @@ -86,12 +134,63 @@ export function codeOnly(text: string): string { i = skipped; continue; } + if (text[i] === '/' && opensRegex(out)) { + const end = regexEnd(text, i); + if (end !== -1) { out += blank(text.slice(i, end + 1)); i = end + 1; continue; } + } out += text[i]!; i += 1; } return out; } +/** + * Whether `name` is bound again in `code` after `from`, so an `name.x` later + * in the body need not be the flow context at all. + * + * `code` is a `codeOnly` result and `from` is the end of the body's own + * parameter declaration, which must not count as a rebinding of itself. + * Renaming a local callback parameter cannot decide whether a flow is + * admitted, so a caller that finds a rebinding declines to judge the body + * statically and leaves it to the runtime guard. This over-declines — an + * inner binding that never shadows the access is still a rebinding here — + * which loses a static refusal rather than inventing one. + */ +export function rebindsIdentifier(code: string, name: string, from: number): boolean { + const after = code.slice(from); + const word = `(?:^|[^\\w$.])${name}`; + // A single-parameter arrow, a declaration, or an assignment over the parameter. + if (new RegExp(`${word}\\s*=>`, 'u').test(after)) return true; + if (new RegExp(`(?:^|[^\\w$.])(?:const|let|var|function|class)\\s+${name}(?:[^\\w$]|$)`, 'u').test(code)) return true; + if (new RegExp(`${word}\\s*=(?![=>])`, 'u').test(after)) return true; + const bound = new RegExp(`${word}(?:[^\\w$]|$)`, 'u'); + for (const parameters of parameterLists(code, from)) if (bound.test(parameters)) return true; + return false; +} + +/** The text inside each `(…)` that binds names after `from`: arrow, `function` and `catch` parameters. */ +function* parameterLists(code: string, from: number): Generator { + for (const arrow of code.matchAll(/\)\s*=>/gu)) { + const open = matchingOpen(code, arrow.index); + if (open >= from) yield code.slice(open + 1, arrow.index); + } + for (const head of code.matchAll(/(?:^|[^\w$.])(?:function(?:\s+[\w$]+)?|catch)\s*\(/gu)) { + const open = head.index + head[0].length - 1; + const close = open >= from ? matchingClose(code, open) : -1; + if (close !== -1) yield code.slice(open + 1, close); + } +} + +/** Index of the `(` opening the group closed at `close`; -1 if unbalanced. `code` is comment- and literal-free. */ +function matchingOpen(code: string, close: number): number { + let depth = 0; + for (let i = close; i >= 0; i -= 1) { + if (code[i] === ')') depth += 1; + else if (code[i] === '(' && (depth -= 1) === 0) return i; + } + return -1; +} + /** * Whether `text[start…end)` is a quoted property name inside `[` … `]`. * `before` is the code-only prefix, so a `[` that is itself inside a comment diff --git a/packages/sdk/tests/helper-partial-support.test.ts b/packages/sdk/tests/helper-partial-support.test.ts index 2eab46b12..b3d372847 100644 --- a/packages/sdk/tests/helper-partial-support.test.ts +++ b/packages/sdk/tests/helper-partial-support.test.ts @@ -10,6 +10,7 @@ import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { afterEach, expect, it, vi } from 'vitest'; import { flow, type Ctx } from '@relayflows/surface'; +import { createHelpers } from '@relayflows/surface/runtime'; import { checkProviderHelpers } from '../src/slack-preflight.js'; import { executeAuthoredFlow } from '../src/authored-flow-executor.js'; import { JournalClient } from '../src/journal-client.js'; @@ -35,6 +36,8 @@ function satisfied() { } const refusal = (body: Function) => checkProviderHelpers({ body }).diagnostics .filter(d => d.severity === 'refusal'); +/** A body whose `toString` is the given source verbatim, untouched by this file's transform. */ +const fromSource = (source: string): Function => new Function(`return ${source};`)() as Function; it('refuses a gitlab resource the helper does not carry, naming the ones it does', () => { satisfied(); @@ -77,6 +80,54 @@ it('accepts the resources gitlab does carry, and leaves github alone', () => { } }).ok).toBe(true); }); +it('admits every member the guarded helper still resolves', () => { + satisfied(); + // The runtime guard refuses only what the bound object does not have, so + // inherited object behaviour, `then` and `toJSON` all still resolve. Static + // inspection has to admit exactly those, or `flows check` rejects feature + // detection that runs. + const gitlab = createHelpers(() => { throw new Error('a refusal must not dispatch'); }) + .gitlab as unknown as Record; + expect(gitlab.hasOwnProperty('comments')).toBe(true); + expect(gitlab.toJSON).toBeUndefined(); + expect(gitlab.then).toBeUndefined(); + expect(String(gitlab)).toBe('[object Object]'); + for (const body of [ + (f: any) => f.gitlab.hasOwnProperty('comments'), + (f: any) => f.gitlab.toJSON, + (f: any) => f.gitlab.then, + (f: any) => f.gitlab.toString(), + (f: any) => f.gitlab['propertyIsEnumerable']('discussions'), + ]) expect(refusal(body), body.toString()).toEqual([]); +}); + +it('does not read a regex literal as an access, and still reads a division as code', () => { + satisfied(); + // `/f.gitlab.issues/` inspects text; it reaches no helper at all. + expect(refusal((f: any) => /f.gitlab.issues/.test(String(f.gitlab.comments)))).toEqual([]); + expect(refusal((f: any) => { if (f) /f.gitlab.mergeRequests/.test('x'); return f.gitlab.discussions; })).toEqual([]); + // A `/` that opens no literal is arithmetic, and the access after it stands. + const divided = (f: any) => { const half = 10 / 2; return [f.gitlab.issues, half]; }; + expect(refusal(divided)[0]?.message).toBe(unavailable); +}); + +it('declines to judge a body that binds the context parameter name again', () => { + satisfied(); + // Renaming a local callback parameter cannot decide whether a flow is + // admitted; an inner `f` is a record here, not the flow context. Built from + // source text because this file's own transform renames a shadowed binding, + // which would leave nothing shadowed for preflight to read. + for (const source of [ + '(f) => [{ gitlab: { issues: [] } }].map(f => f.gitlab.issues)', + '(f) => [{ gitlab: { issues: [] } }].map(function (f) { return f.gitlab.issues; })', + '(f) => { const rows = [{ gitlab: { issues: 1 } }]; for (const f of rows) void f.gitlab.issues; }', + '(f) => { try { void f; } catch (f) { void f.gitlab.issues; } }', + '(f) => { f = { gitlab: { issues: 1 } }; return f.gitlab.issues; }', + ]) expect(refusal(fromSource(source)), source).toEqual([]); + // The unshadowed body each is distinguished from still refuses. + expect(refusal(fromSource('(f) => f.gitlab.issues'))[0]?.message).toBe(unavailable); +}); + it('does not read a comment or a string literal as an access', () => { satisfied(); // The body is read from `toString`, never executed; text that looks like an @@ -107,6 +158,21 @@ it('refuses a computed member at the call site, as a typed helper_provider.unsup expect(reached).toBe(true); }); +it('words the static refusal exactly as the surface words the runtime guard', async () => { + satisfied(); + // The SDK cannot import the surface's wording function: this source is + // installed against a PUBLISHED surface, and a named import the pinned + // version has not shipped fails the module at load, before preflight can + // run. The two wordings are pinned equal here instead, against whichever + // surface this SDK actually resolves. + const runtime = await import('@relayflows/surface/runtime') as { + unsupportedHelperMemberMessage?: (provider: string, member: string, available: readonly string[]) => string; + }; + const wording = runtime.unsupportedHelperMemberMessage; + expect(wording?.('gitlab', 'issues', ['comments', 'discussions']) ?? unavailable).toBe(unavailable); + expect(refusal(async (f: any) => { await f.gitlab.issues.list({}); })[0]?.message).toBe(unavailable); +}); + it('leaves an unrelated body failure exactly as the body threw it', async () => { satisfied(); const handle = flow('gitlab-unrelated', async f => { diff --git a/packages/surface/tests/helper-support.test.ts b/packages/surface/tests/helper-support.test.ts index 8af604c56..90e62fb22 100644 --- a/packages/surface/tests/helper-support.test.ts +++ b/packages/surface/tests/helper-support.test.ts @@ -63,9 +63,16 @@ it('carries the refused member, its resource and the available names structurall } }); -it('words the refusal once, for preflight and the runtime guard alike', () => { +it('words the refusal once, for the property guard and the authored envelope alike', async () => { + const expected = unsupportedHelperMemberMessage('gitlab', 'issues', ['comments', 'discussions']); const guarded = (() => { try { gitlab().issues; return ''; } catch (error) { return (error as Error).message; } })(); - expect(guarded).toBe(unsupportedHelperMemberMessage('gitlab', 'issues', ['comments', 'discussions'])); + expect(guarded).toBe(expected); + // The SDK's `flows check` restates this wording rather than importing it — + // it is installed against a published surface, which need not export the + // function yet — and pins the two equal in its own helper-partial-support test. + await expect(invokeHelper(helperClients['gitlab']!, + { type: 'effect', provider: 'gitlab', verb: 'issues.list', args: [] }, refusingTransport)) + .rejects.toThrow(expected); }); it('leaves ordinary object behavior on the guarded helper intact', async () => { From 2426a90f3254ddb4f7196c62201611f1f8470639 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Tue, 22 Sep 2026 22:48:23 -0700 Subject: [PATCH 3/6] fix(sdk): compile against the published surface catalog; see method and destructured rebindings --- packages/sdk/src/helper-preflight.ts | 17 ++++++++--- packages/sdk/src/source-scan.ts | 29 +++++++++++++++++++ .../sdk/tests/helper-partial-support.test.ts | 20 +++++++++++++ 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/packages/sdk/src/helper-preflight.ts b/packages/sdk/src/helper-preflight.ts index 2e91e9cfb..dba4599f3 100644 --- a/packages/sdk/src/helper-preflight.ts +++ b/packages/sdk/src/helper-preflight.ts @@ -14,8 +14,18 @@ export function preflightHelpers( /** End of the body's own parameter declaration, which is not a rebinding of itself. */ const declared = parameter?.[0].length ?? 0; const diagnostics: PreflightDiagnostic[] = []; - for (const provider of helperProviders) { + for (const catalogEntry of helperProviders) { + // This SDK compiles against a PUBLISHED surface whose catalog predates + // 'partial': its `supported` is boolean and it carries no `resources` or + // `note`. Read the row structurally so the same source typechecks against + // both catalog shapes; on the published one `resources` is simply absent. + const provider = catalogEntry as { + provider: string; namespace: string; + supported: boolean | 'partial'; + resources?: readonly string[]; note?: string; + }; const { namespace, supported } = provider; + const resources = provider.resources ?? []; const used = definition.header?.tools?.[namespace] === true || (root !== undefined && new RegExp(`(?:^|[^\\w$.])${root}\\s*(?:\\.\\s*${namespace}\\b|\\[\\s*['"]${namespace}['"]\\s*\\])`).test(body)); if (!used) continue; @@ -27,14 +37,13 @@ export function preflightHelpers( // writeback route that no client carries, and a body that would die on // `undefined is not a function` should say so with the names that do work. const missing = supported === 'partial' - ? unavailableMembers(root, declared, namespace, provider.resources, body) : []; + ? unavailableMembers(root, declared, namespace, resources, body) : []; if (!supported) { diagnostics.push({ severity: 'refusal', kind: 'helper_provider.unsupported', message: `f.${namespace} has no upstream relayfile writeback client.` }); } else if (missing.length > 0) { diagnostics.push({ severity: 'refusal', kind: 'helper_provider.unsupported', - message: unavailableMessage(namespace, missing[0]!, provider.resources, - 'note' in provider ? provider.note : undefined) }); + message: unavailableMessage(namespace, missing[0]!, resources, provider.note) }); } else if (!fact.mock && !fact.mount) { diagnostics.push({ severity: 'refusal', kind: provider.provider === 'slack' ? (fact.token?.trim() ? 'helper_slack.mount_required' : 'helper_slack.credential_missing') : 'helper_provider.mount_required', diff --git a/packages/sdk/src/source-scan.ts b/packages/sdk/src/source-scan.ts index 5db581234..cb399bdca 100644 --- a/packages/sdk/src/source-scan.ts +++ b/packages/sdk/src/source-scan.ts @@ -165,9 +165,38 @@ export function rebindsIdentifier(code: string, name: string, from: number): boo if (new RegExp(`${word}\\s*=(?![=>])`, 'u').test(after)) return true; const bound = new RegExp(`${word}(?:[^\\w$]|$)`, 'u'); for (const parameters of parameterLists(code, from)) if (bound.test(parameters)) return true; + // A destructured declaration binds the name without writing it as the + // declaration's own identifier: `const { f } = …` shadows `f` exactly as + // `const f = …` does, so the simple-declaration rule above cannot see it. + for (const declaration of code.matchAll(/(?:^|[^\w$.])(?:const|let|var)\s*[[{]/gu)) { + const open = declaration.index + declaration[0].length - 1; + const close = matchingClose(code, open); + if (close !== -1 && bound.test(code.slice(open + 1, close))) return true; + } + // An object or class method binds its parameters the way `function` does, + // but carries no keyword for the parameterLists walker to key on: + // `read(f) { … }`. The `(` must open a parameter list — i.e. close directly + // ahead of a `{` — and the head must not be a control keyword (`if (f) {` + // binds nothing). Anything else `ident(…){` can only be a method. + for (const head of code.matchAll(/(?:^|[^\w$.])([A-Za-z_$][\w$]*)\s*\(/gu)) { + if (CONTROL_HEADS.has(head[1]!)) continue; + const open = head.index + head[0].length - 1; + if (open < from) continue; + const close = matchingClose(code, open); + if (close === -1 || !/^\s*\{/.test(code.slice(close + 1))) continue; + if (bound.test(code.slice(open + 1, close))) return true; + } return false; } +/** Keywords whose `name (…) {` is a control clause, not a parameter list. */ +const CONTROL_HEADS: ReadonlySet = new Set([ + 'if', 'else', 'for', 'while', 'do', 'switch', 'catch', 'with', 'try', 'finally', + 'function', 'return', 'throw', 'typeof', 'new', 'delete', 'void', 'in', 'of', + 'instanceof', 'await', 'yield', 'case', 'const', 'let', 'var', 'class', + 'extends', 'import', 'export', 'default', +]); + /** The text inside each `(…)` that binds names after `from`: arrow, `function` and `catch` parameters. */ function* parameterLists(code: string, from: number): Generator { for (const arrow of code.matchAll(/\)\s*=>/gu)) { diff --git a/packages/sdk/tests/helper-partial-support.test.ts b/packages/sdk/tests/helper-partial-support.test.ts index b3d372847..0b5fc8ed2 100644 --- a/packages/sdk/tests/helper-partial-support.test.ts +++ b/packages/sdk/tests/helper-partial-support.test.ts @@ -128,6 +128,26 @@ it('declines to judge a body that binds the context parameter name again', () => expect(refusal(fromSource('(f) => f.gitlab.issues'))[0]?.message).toBe(unavailable); }); +it('declines to judge destructured declarations and method parameters too', () => { + satisfied(); + // `const { f } = …` and `read(f) { … }` bind the name without a form the + // simple-declaration or keyword-parameter rules can see. Both bodies here + // return 42 without touching a GitLab helper, so refusing either would be + // the scanner deciding admission on a renamed local — the finding this + // test pins closed. + 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; } }', + '(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(refusal(fromSource(source)), source).toEqual([]); + // A control clause is not a parameter list: `if (f) {` binds nothing, so a + // plain access under it still refuses. + expect(refusal(fromSource('(f) => { if (f) { return f.gitlab.issues; } }'))[0]?.message) + .toBe(unavailable); +}); + it('does not read a comment or a string literal as an access', () => { satisfied(); // The body is read from `toString`, never executed; text that looks like an From 4632825a2e605608f7b0cd10d7ba24c03097a864 Mon Sep 17 00:00:00 2001 From: Proactive Runtime Bot Date: Wed, 23 Sep 2026 12:50:48 -0700 Subject: [PATCH 4/6] fix(surface): drop a type assertion this design never makes `typecheck:regressions` failed with TS2578: the `@ts-expect-error` on `f.gitlab.issues.list` was unused, because the helper namespace is structural and the call does typecheck. That is the design, not a gap. gitlab is `supported: "partial"` with resources comments and discussions, and the catalog refuses the rest at runtime with a named member, its resource and the available names -- `f.gitlab.issues is unavailable; available resources: comments, discussions.` -- covered across dot, bracket and aliased access in helper-support.test.ts and the SDK's helper-partial-support.test.ts. The directive claimed a type-level narrowing that was never implemented, so the file asserted something untrue and the compiler said so. The `mergeRequests` case stays: that member genuinely does not exist on the type, and removing its directive still raises TS2551. Co-Authored-By: Claude Opus 5 --- packages/surface/tests/helpers-typecheck-fail.test-d.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/surface/tests/helpers-typecheck-fail.test-d.ts b/packages/surface/tests/helpers-typecheck-fail.test-d.ts index fe598c657..1db7236b1 100644 --- a/packages/surface/tests/helpers-typecheck-fail.test-d.ts +++ b/packages/surface/tests/helpers-typecheck-fail.test-d.ts @@ -15,8 +15,12 @@ export function rejectedHelpers(f: Ctx): void { f.notarealprovider.anything(); // @ts-expect-error resource clients do not have runtime dispatch yet. f.slack.messages.list(); - // @ts-expect-error f.gitlab carries comments and discussions only. - f.gitlab.issues.list({ projectPath: 'g/p' }); + // `f.gitlab.issues` is deliberately NOT here: the helper namespace is + // structural, so it typechecks, and the catalog refuses it at runtime with + // `f.gitlab.issues is unavailable; available resources: comments, + // discussions.` — covered by helper-support.test.ts and the SDK's + // helper-partial-support.test.ts across dot, bracket and aliased access. + // Asserting a type error here claimed a narrowing this design does not do. // @ts-expect-error merge requests are not in gitlab's writeback catalog. f.gitlab.mergeRequests.write({ projectPath: 'g/p' }, { title: 'x' }); } From 53a10fd7df74b1fd5cdea7d0283de561c0011104 Mon Sep 17 00:00:00 2001 From: Miya Date: Fri, 25 Sep 2026 06:05:27 +0200 Subject: [PATCH 5/6] fix(sdk): scope helper shadowing to real bindings Session-Id: 01a0d6ad-a1bd-78b0-9284-d13e4db53e96 --- packages/sdk/src/helper-reference.ts | 88 ++++++++++++++++++--- packages/sdk/tests/helper-reference.test.ts | 34 ++++++++ 2 files changed, 109 insertions(+), 13 deletions(-) diff --git a/packages/sdk/src/helper-reference.ts b/packages/sdk/src/helper-reference.ts index a9811e290..38e6ed479 100644 --- a/packages/sdk/src/helper-reference.ts +++ b/packages/sdk/src/helper-reference.ts @@ -127,13 +127,26 @@ function walkReferences( state: { rootFunctionFound: boolean }, visit: (node: AstNode) => void, ): void { - let hidden = shadowed; if (isFunction(node)) { const parameters = Array.isArray(node.params) ? node.params.filter(isNode) : []; const bindsParameter = parameters.some(parameter => patternBinds(parameter, root)); + let bodyHidden = shadowed; if (bindsParameter && !state.rootFunctionFound) state.rootFunctionFound = true; - else if (bindsParameter || (isNode(node.id) && patternBinds(node.id, root))) hidden = true; - } else if (node.type === 'BlockStatement' && blockBinds(node, root)) { + 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; + 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; @@ -173,18 +186,67 @@ function blockBinds(node: AstNode, root: string): boolean { /** Identifier occurrences that introduce bindings, including nested patterns. */ function patternBinds(pattern: AstNode, root: string): boolean { - if (pattern.type === 'Identifier') return pattern.name === root; - if (pattern.type === 'MemberExpression') return false; - for (const key of Object.keys(pattern)) { - if (key === 'type' || key === 'start' || key === 'end' || key === 'loc') continue; - const child = pattern[key]; - if (Array.isArray(child)) { - if (child.some(entry => isNode(entry) && patternBinds(entry, root))) return true; - } else if (isNode(child) && patternBinds(child, root)) { - return true; + 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; } - 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 { diff --git a/packages/sdk/tests/helper-reference.test.ts b/packages/sdk/tests/helper-reference.test.ts index d59c7df06..42d37c8c3 100644 --- a/packages/sdk/tests/helper-reference.test.ts +++ b/packages/sdk/tests/helper-reference.test.ts @@ -148,6 +148,40 @@ describe('helper references are read as syntax, not text', () => { ]) 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('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'); From 28c46e45b64d658a59e6cec0d9fabb1bd81c4d2f Mon Sep 17 00:00:00 2001 From: Miya Date: Fri, 25 Sep 2026 06:10:18 +0200 Subject: [PATCH 6/6] fix(sdk): keep root parameter defaults visible Session-Id: 01a0d6ad-a1bd-78b0-9284-d13e4db53e96 --- packages/sdk/src/helper-reference.ts | 5 +++-- packages/sdk/tests/helper-reference.test.ts | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/sdk/src/helper-reference.ts b/packages/sdk/src/helper-reference.ts index 38e6ed479..37749e614 100644 --- a/packages/sdk/src/helper-reference.ts +++ b/packages/sdk/src/helper-reference.ts @@ -131,7 +131,8 @@ function walkReferences( const parameters = Array.isArray(node.params) ? node.params.filter(isNode) : []; const bindsParameter = parameters.some(parameter => patternBinds(parameter, root)); let bodyHidden = shadowed; - if (bindsParameter && !state.rootFunctionFound) state.rootFunctionFound = true; + 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; } @@ -139,7 +140,7 @@ function walkReferences( // 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; + 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); diff --git a/packages/sdk/tests/helper-reference.test.ts b/packages/sdk/tests/helper-reference.test.ts index 42d37c8c3..d21df5198 100644 --- a/packages/sdk/tests/helper-reference.test.ts +++ b/packages/sdk/tests/helper-reference.test.ts @@ -176,6 +176,11 @@ describe('helper references are read as syntax, not text', () => { ))).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(); }',