From 7d4c913467271fbfd8066ae2674206402ab5d4a9 Mon Sep 17 00:00:00 2001 From: Henry Stelle Date: Fri, 10 Jul 2026 11:40:51 -0700 Subject: [PATCH 1/6] SPIKE: derive POS intercept events from source via AST + wire into deploy Prototype AST detector that walks the extension import graph from its entry and detects every shopify.intercept('') callsite, ignoring control flow and tracking the intercept function value through aliasing (const alias, destructuring, object-alias member access, reassignment, cross-file re-export). Dynamic/computed event args are surfaced as unresolved, never dropped. deployConfig now unions derived events with any TOML-declared capabilities.intercepts, so derived events reach the backend through the existing wire field with no backend change. Includes demo fixture, tests, and a design writeup. Assisted-By: devx/76608ac5-410d-425f-93b9-9eea3cc49f3f --- .../pos-intercept-demo/src/aliased.ts | 5 + .../pos-intercept-demo/src/checkout.ts | 7 + .../fixtures/pos-intercept-demo/src/index.ts | 47 ++ .../pos_intercept_detection.DESIGN.md | 159 +++++++ .../pos_intercept_detection.test.ts | 56 +++ .../specifications/pos_intercept_detection.ts | 404 ++++++++++++++++++ .../specifications/pos_ui_extension.test.ts | 33 ++ .../specifications/pos_ui_extension.ts | 62 ++- 8 files changed, 772 insertions(+), 1 deletion(-) create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/aliased.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/checkout.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/index.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md create mode 100644 packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/aliased.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/aliased.ts new file mode 100644 index 00000000000..6b5bb012552 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/aliased.ts @@ -0,0 +1,5 @@ +// The intercept reference is aliased AND re-exported from here. The detector +// must resolve `block` (imported elsewhere) back to shopify.intercept. +declare const shopify: {intercept: (event: string, handler: () => void) => void} + +export const block = shopify.intercept diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/checkout.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/checkout.ts new file mode 100644 index 00000000000..be29ddc4b76 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/checkout.ts @@ -0,0 +1,7 @@ +// Imported file — its callsites are part of the extension's capability surface. +export function registerCheckoutGuards() { + const {intercept} = shopify + intercept('beforecapture', () => {}) +} + +declare const shopify: {intercept: (event: string, handler: () => void) => void} diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/index.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/index.ts new file mode 100644 index 00000000000..d0da03d237b --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/index.ts @@ -0,0 +1,47 @@ +// Demo POS UI extension entry. Exercises every alias path the detector must +// handle. The `shopify` global is injected by the host at runtime. +declare const shopify: {intercept: (event: string, handler: () => void) => void} + +import {registerCheckoutGuards} from './checkout.js' +import {block} from './aliased.js' + +// 1. Direct call. +shopify.intercept('beforecheckout', () => {}) + +// 2. Aliased via `const fn = shopify.intercept` (declared before use). +const guard = shopify.intercept +guard('beforepayment', () => {}) + +// 3. Destructured: `const {intercept} = shopify`. +const {intercept} = shopify +intercept('beforediscount', () => {}) + +// 4. Destructured + renamed: `const {intercept: rename} = shopify`. +const {intercept: renamed} = shopify +renamed('beforerefund', () => {}) + +// 5. shopify object aliased, then member call: `const s = shopify; s.intercept(...)`. +const s = shopify +s.intercept('beforeexchange', () => {}) + +// 6. Reassignment of a let binding. +let later +later = shopify.intercept +later('beforevoid', () => {}) + +// 7. Cross-file re-exported intercept reference. +block('beforecancel', () => {}) + +// 8. Inside control flow — MUST still be detected (reachability ignored). +if (Math.random() > 0.5) { + shopify.intercept('beforetax', () => {}) +} else { + shopify.intercept('beforeshipping', () => {}) +} + +// 9. UNRESOLVED: dynamic/computed event args — surfaced, never dropped. +const dynamicEvent = 'beforesomething' +shopify.intercept(dynamicEvent, () => {}) +shopify.intercept(`before${'checkout'}`, () => {}) + +registerCheckoutGuards() diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md new file mode 100644 index 00000000000..ac65f236f8b --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md @@ -0,0 +1,159 @@ +# SPIKE: Deriving POS intercept events from source code + +Branch: `henry/pos-intercept-ast-derive` (exploration only) + +## What this prototype does + +Instead of trusting the hand-authored `capabilities.intercepts` TOML array, it +**derives** the intercept events from the extension's **source code** via a +control-flow-agnostic AST walk of the import graph. + +Files: + +- `pos_intercept_detection.ts` — the detector (`detectPosIntercepts`, + `deriveInterceptsFromDirectory`, `findPosExtensionEntry`). +- `pos_intercept_detection.test.ts` — unit tests against the demo fixture. +- `fixtures/pos-intercept-demo/` — a POS extension exercising every alias form, + control-flow branches, a cross-file re-exported alias, and dynamic args. +- `pos_ui_extension.ts` — `deployConfig` now calls `deriveAndMergeIntercepts`. +- `pos_ui_extension.test.ts` — proves the derived events land in the deployed + `capabilities.intercepts`. + +### Detection behaviour (matches the brief) + +1. **Reuses the CLI's existing AST tooling.** It calls `findAllImportedFiles()` + from `type-generation.ts` to walk the full import graph from the extension's + `index.*` entry, and uses the same `typescript` compiler API + (`ts.createSourceFile` / `forEachChild` / `isCallExpression`). +2. **Ignores control flow.** Every `shopify.intercept(...)` callsite is counted + regardless of `if`/`else`/ternary/loop/dead-code. Capabilities are a static + **superset** of what the extension might do; reachability is a runtime + concern we deliberately do not evaluate. +3. **Tracks the intercept function value through aliasing.** A whole-graph + fixpoint resolves the following back to `shopify.intercept`: + - `const guard = shopify.intercept` + - `const {intercept} = shopify` + - `const {intercept: renamed} = shopify` + - `const s = shopify; s.intercept('...')` (object-alias then member access) + - `let fn; fn = shopify.intercept` (reassignment) + - `export const block = shopify.intercept` imported/called in another file + (cross-file re-export) +4. **Only string-literal first args are resolvable.** Variables, member + expressions and template strings with substitutions are surfaced as + `unresolved` with file/line and the raw arg text — never silently dropped. + +Run the tests: + +``` +cd packages/app +../../node_modules/.bin/vitest run \ + src/cli/models/extensions/specifications/pos_intercept_detection.test.ts \ + src/cli/models/extensions/specifications/pos_ui_extension.test.ts +``` + +--- + +## THE KEY DESIGN QUESTION: derived events → backend + +> If events are derived and therefore **not** present in the TOML, how does that +> information get stored in / transmitted to the backend? + +### Answer: reuse the existing `capabilities.intercepts` wire field. No new mechanism. + +There is **no dedicated transport for "derived" events** and there does not need +to be one. The transmission mechanism is the deployed **version config**, which +the CLI already builds per module in `deployConfig`. + +Concrete trace of how config reaches the backend on deploy: + +``` +pos_ui_extension.deployConfig(config, directory) + → returns { name, description, renderer_version, capabilities } + capabilities.intercepts ← derived ∪ TOML-declared events +ExtensionInstance.deployConfig() (extension-instance.ts:198) +ExtensionInstance.bundleConfig() (extension-instance.ts:402) + → { config: JSON.stringify(configValue), context, handle, uid, uuid, ... } +services/deploy.ts:254 app.allExtensions.map(ext.bundleConfig(...)) + → uploadExtensionsBundle({ appModules, ... }) + → backend persists each module's `config` blob as the version config +``` + +So the derived events ride inside the module's serialized `config` string, +**byte-for-byte identical** to how a TOML-declared `capabilities.intercepts` +array would be serialized. The backend consumer that reads +`capabilities.intercepts` today keeps working unchanged — it cannot tell (and +does not care) whether the array came from the TOML or from AST derivation. + +This is exactly what the prototype does: + +```ts +// pos_ui_extension.ts deployConfig +const capabilities = await deriveAndMergeIntercepts(config.capabilities, directory) +return { name, description, renderer_version, capabilities } +``` + +`deriveAndMergeIntercepts` **unions** derived events with any TOML-declared ones, +de-dupes, and sorts. That gives us three coexisting modes with zero backend +change: + +| Mode | TOML `intercepts` | Source | Emitted to backend | +|------|-------------------|--------|--------------------| +| Derivation-first (goal) | absent | `intercept('beforecheckout')` | `['beforecheckout']` | +| Declaration-only (today) | `['beforecheckout']` | none/unparseable | `['beforecheckout']` | +| Hybrid | `['manuallydeclared']` | `intercept('beforecheckout')` | `['beforecheckout','manuallydeclared']` | + +### Why this is the right shape (per the 2022 "source of truth" ADR) + +The ADR's principle — *"we derive the information if it can be derived"* — is +satisfied without changing the contract the backend already depends on. The TOML +field becomes an **optional override / escape hatch** rather than the required +source of truth. The deployed version config remains the single artifact of +record; derivation just changes *who fills in* `capabilities.intercepts` +(the compiler vs. the author), not *where it is stored*. + +### Alternatives considered (and rejected for this spike) + +- **New top-level `derived_intercepts` wire field.** Rejected: forces a backend + schema change and a dual-read code path for no benefit — the semantics are + identical to `capabilities.intercepts`. +- **Emit from the esbuild metafile instead of a standalone AST pass.** + `bundle.ts` already runs esbuild with `metafile: true` on production deploy. + The metafile gives us the exact module graph esbuild bundles (more accurate + than our hand-rolled resolver), but it does **not** contain callsite/AST + detail, so we'd still need an AST pass over those files. A follow-up could feed + the metafile's input list into `detectPosIntercepts` instead of + `findAllImportedFiles` for graph fidelity. Out of scope for the spike. + +--- + +## Reliability gaps (must be surfaced, not hidden) + +1. **Dynamic / computed event args are unresolvable.** `intercept(evt)`, + `intercept(obj.event)`, `` intercept(`before${x}`) `` cannot be statically + resolved. The detector reports them in `result.unresolved` (with file:line + + raw text) and `deriveAndMergeIntercepts` logs an `outputWarn`. **Product + decision needed:** warn-and-continue (current), hard-fail the deploy, or + require these to be TOML-declared. The TOML override path exists precisely for + this case. +2. **Import-graph resolution is best-effort.** The cross-file resolver handles + relative imports and the TS `./x.js`→`x.ts` convention, and skips + `node_modules`. `tsconfig` path aliases (`@/...`) are handled by the reused + `findAllImportedFiles` for graph walking but **not** by the cross-file + *alias-propagation* resolver — an intercept reference re-exported through a + path-aliased module could be missed. Unifying both on `ts.resolveModuleName` + would close this. +3. **Aliasing is intentionally shallow (basic data-flow).** We resolve direct + assignments, destructuring, object-aliasing and cross-file re-exports. We do + **not** track the reference through: function parameters / higher-order + passing (`wrap(shopify.intercept)`), array/object property storage + (`const m = {i: shopify.intercept}; m.i(...)`), or `.bind()/.call()`. These + would silently under-report. A full type-checker-backed symbol analysis + (`ts.createProgram` + `TypeChecker`) would be more robust than the + `createSourceFile`-per-file approach but is heavier. +4. **Dead code is included by design.** Because control flow is ignored, an + intercept in unreachable code still becomes a capability. This is the safe + direction (over-declaration) but means derived capabilities can be broader + than runtime behaviour. +5. **No JS-runtime evaluation.** Constant folding (`const E = 'beforecheckout'; + intercept(E)`) is not performed; `E` is reported unresolved even though it is + a compile-time constant. Simple const-propagation is a cheap future win. diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts new file mode 100644 index 00000000000..2261e2849b0 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts @@ -0,0 +1,56 @@ +import {detectPosIntercepts, deriveInterceptsFromDirectory, findPosExtensionEntry} from './pos_intercept_detection.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {fileURLToPath} from 'node:url' +import {dirname} from 'node:path' +import {describe, expect, test} from 'vitest' + +const currentDir = dirname(fileURLToPath(import.meta.url)) +const fixtureDir = joinPath(currentDir, 'fixtures', 'pos-intercept-demo') + +describe('detectPosIntercepts', () => { + test('derives events across the import graph, ignoring control flow and tracking aliases', async () => { + // Given + const entry = await findPosExtensionEntry(fixtureDir) + expect(entry).toBeDefined() + + // When + const result = await detectPosIntercepts(entry!) + + // Then — every resolvable event, regardless of alias form or control flow. + expect(result.events).toEqual([ + 'beforecancel', // cross-file re-exported alias + 'beforecapture', // imported file destructured alias + 'beforecheckout', // direct call + 'beforediscount', // const {intercept} = shopify + 'beforeexchange', // const s = shopify; s.intercept(...) + 'beforepayment', // const guard = shopify.intercept + 'beforerefund', // const {intercept: renamed} = shopify + 'beforeshipping', // else branch — reachability ignored + 'beforetax', // if branch — reachability ignored + 'beforevoid', // let reassignment + ]) + }) + + test('surfaces dynamic/computed event args as unresolved instead of dropping them', async () => { + const entry = await findPosExtensionEntry(fixtureDir) + const result = await detectPosIntercepts(entry!) + + // Two dynamic callsites: a variable and a template string with substitution. + expect(result.unresolved).toHaveLength(2) + expect(result.unresolved.every((callsite) => callsite.event === null)).toBe(true) + expect(result.unresolved.map((callsite) => callsite.argText).sort()).toEqual([ + '`before${\'checkout\'}`', + 'dynamicEvent', + ]) + result.unresolved.forEach((callsite) => { + expect(callsite.unresolvedReason).toBeTruthy() + expect(callsite.line).toBeGreaterThan(0) + }) + }) + + test('deriveInterceptsFromDirectory drives detection from an extension directory', async () => { + const result = await deriveInterceptsFromDirectory(fixtureDir) + expect(result?.events).toContain('beforecheckout') + expect(result?.analyzedFiles.length).toBeGreaterThanOrEqual(3) + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts new file mode 100644 index 00000000000..864837e4ed7 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts @@ -0,0 +1,404 @@ +import {findAllImportedFiles} from './type-generation.js' +import {fileExists, readFileSync} from '@shopify/cli-kit/node/fs' +import {dirname, joinPath, resolvePath} from '@shopify/cli-kit/node/path' +import {uniq} from '@shopify/cli-kit/common/array' +import type ts from 'typescript' + +// --------------------------------------------------------------------------- +// SPIKE / PROTOTYPE: derive POS intercept events from extension SOURCE CODE. +// +// Instead of trusting the hand-authored `capabilities.intercepts` TOML array, +// this walks the extension's import graph (starting from its `index.*` entry) +// and statically detects every `shopify.intercept('', ...)` callsite. +// +// Design notes / deliberate choices (per the spike brief): +// * We IGNORE control flow. Every callsite counts, regardless of whether it +// sits inside an `if`, a ternary, a loop, or dead code. Reachability is a +// runtime concern; capabilities are a static superset of "what this +// extension might do". +// * We TRACK THE INTERCEPT FUNCTION VALUE through aliasing. `shopify` is a +// global binding injected by the host, so any of the following resolve back +// to `shopify.intercept`: +// const intercept = shopify.intercept +// const {intercept} = shopify +// const {intercept: block} = shopify +// const s = shopify; s.intercept('...') +// let fn; fn = shopify.intercept (reassignment) +// export const intercept = shopify.intercept (re-export, cross-file) +// * Only STRING-LITERAL first args are statically resolvable. Anything else +// (variables, member expressions, template strings with substitutions) is +// surfaced as UNRESOLVED and never silently dropped. +// --------------------------------------------------------------------------- + +async function loadTypeScript(): Promise { + // typescript is CJS; dynamic import wraps it as { default: ... } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mod: any = await import('typescript') + return mod.default ?? mod +} + +/** A single detected `shopify.intercept(...)` callsite. */ +export interface InterceptCallsite { + /** Resolved event name, or null when the first arg is not a string literal. */ + event: string | null + file: string + line: number + column: number + /** Source text of the first argument, for diagnostics. */ + argText: string + /** Populated only when `event` is null: why we couldn't resolve it. */ + unresolvedReason?: string +} + +export interface InterceptDetectionResult { + /** Unique, sorted set of statically-resolved event names. */ + events: string[] + /** Every callsite we found (resolved and unresolved). */ + callsites: InterceptCallsite[] + /** Callsites whose event arg could not be statically resolved. */ + unresolved: InterceptCallsite[] + /** Files walked (entry + transitive local imports). */ + analyzedFiles: string[] +} + +const INTERCEPT_PROPERTY = 'intercept' +const SHOPIFY_GLOBAL = 'shopify' + +interface FileAnalysis { + path: string + sourceFile: ts.SourceFile + /** Identifiers in this file that point at the `shopify` global object. */ + shopifyAliases: Set + /** Identifiers in this file that point at the `shopify.intercept` function. */ + interceptAliases: Set + /** exportedName -> localName for `export {x}` / `export const x = ...`. */ + exports: Map + /** localName -> {resolvedPath, importedName} for imports we could resolve. */ + imports: Map +} + +/** + * Resolve the entry `index.{js,jsx,ts,tsx}` file for a POS UI extension given + * its directory. Mirrors AppLoader.findEntryPath for `single_js_entry_path` + * extensions so the detector can be driven straight from a deploy directory. + */ +export async function findPosExtensionEntry(directory: string): Promise { + const candidates = ['index'] + .flatMap((name) => [`${name}.js`, `${name}.jsx`, `${name}.ts`, `${name}.tsx`]) + .flatMap((fileName) => [`src/${fileName}`, fileName]) + .map((relativePath) => joinPath(directory, relativePath)) + + const found = await Promise.all( + candidates.map(async (candidate) => ((await fileExists(candidate)) ? candidate : undefined)), + ) + return found.find((candidate) => candidate !== undefined) +} + +function scriptKindFor(ts: typeof import('typescript'), filePath: string): ts.ScriptKind { + if (filePath.endsWith('.ts')) return ts.ScriptKind.TS + if (filePath.endsWith('.tsx')) return ts.ScriptKind.TSX + return ts.ScriptKind.JSX +} + +/** Best-effort relative-module resolver (node_modules are intentionally skipped). */ +async function resolveLocalModule(importPath: string, fromFile: string): Promise { + if (!importPath.startsWith('./') && !importPath.startsWith('../')) return undefined + // TS allows importing `./x.js` to refer to `x.ts`; strip a trailing JS-ish + // extension so the candidate list below can re-add the real source extension. + const normalized = importPath.replace(/\.(js|jsx|mjs|cjs)$/, '') + const base = resolvePath(dirname(fromFile), normalized) + const exts = ['', '.ts', '.tsx', '.js', '.jsx'] + for (const ext of exts) { + const withExt = base + ext + // eslint-disable-next-line no-await-in-loop + if (await fileExists(withExt)) return withExt + } + for (const ext of ['.ts', '.tsx', '.js', '.jsx']) { + const indexPath = joinPath(base, `index${ext}`) + // eslint-disable-next-line no-await-in-loop + if (await fileExists(indexPath)) return indexPath + } + return undefined +} + +/** Is `expr` a member access of the intercept property off a shopify alias? */ +function isShopifyInterceptAccess( + ts: typeof import('typescript'), + expr: ts.Expression, + shopifyAliases: Set, +): boolean { + if (ts.isPropertyAccessExpression(expr)) { + return ( + ts.isIdentifier(expr.expression) && + shopifyAliases.has(expr.expression.text) && + expr.name.text === INTERCEPT_PROPERTY + ) + } + if (ts.isElementAccessExpression(expr)) { + return ( + ts.isIdentifier(expr.expression) && + shopifyAliases.has(expr.expression.text) && + ts.isStringLiteralLike(expr.argumentExpression) && + expr.argumentExpression.text === INTERCEPT_PROPERTY + ) + } + return false +} + +/** + * Scan a file's declarations/assignments once, growing the shopify- and + * intercept-alias sets based on their current contents. Returns true if either + * set grew (so callers can run to a fixpoint — aliases may be defined out of + * order or depend on each other). + */ +function growAliases(ts: typeof import('typescript'), analysis: FileAnalysis): boolean { + const before = analysis.shopifyAliases.size + analysis.interceptAliases.size + const {shopifyAliases, interceptAliases} = analysis + + const considerBinding = (name: ts.BindingName, init: ts.Expression | undefined): void => { + if (!init) return + + // Identifier target: const x = + if (ts.isIdentifier(name)) { + if (ts.isIdentifier(init)) { + if (shopifyAliases.has(init.text)) shopifyAliases.add(name.text) + if (interceptAliases.has(init.text)) interceptAliases.add(name.text) + } else if (isShopifyInterceptAccess(ts, init, shopifyAliases)) { + interceptAliases.add(name.text) + } + return + } + + // Object destructuring target: const {intercept} = shopify / shopifyAlias + if (ts.isObjectBindingPattern(name) && ts.isIdentifier(init) && shopifyAliases.has(init.text)) { + for (const element of name.elements) { + const propName = + element.propertyName && ts.isIdentifier(element.propertyName) + ? element.propertyName.text + : ts.isIdentifier(element.name) + ? element.name.text + : undefined + if (propName === INTERCEPT_PROPERTY && ts.isIdentifier(element.name)) { + interceptAliases.add(element.name.text) + } + } + } + } + + const visit = (node: ts.Node): void => { + // const/let/var declarations + if (ts.isVariableDeclaration(node)) { + considerBinding(node.name, node.initializer) + } + + // Reassignment: fn = shopify.intercept ; s = shopify + if (ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.EqualsToken) { + if (ts.isIdentifier(node.left)) { + if (ts.isIdentifier(node.right)) { + if (shopifyAliases.has(node.right.text)) shopifyAliases.add(node.left.text) + if (interceptAliases.has(node.right.text)) interceptAliases.add(node.left.text) + } else if (isShopifyInterceptAccess(ts, node.right, shopifyAliases)) { + interceptAliases.add(node.left.text) + } + } + } + + ts.forEachChild(node, visit) + } + visit(analysis.sourceFile) + + return analysis.shopifyAliases.size + analysis.interceptAliases.size > before +} + +/** Collect import bindings and named exports (structural, computed once). */ +function collectImportsAndExports(ts: typeof import('typescript'), analysis: FileAnalysis, resolved: Map) { + const visit = (node: ts.Node): void => { + // import {intercept} from './x' ; import {intercept as foo} from './x' + if (ts.isImportDeclaration(node) && node.importClause && ts.isStringLiteral(node.moduleSpecifier)) { + const resolvedPath = resolved.get(node.moduleSpecifier.text) + if (resolvedPath) { + const named = node.importClause.namedBindings + if (named && ts.isNamedImports(named)) { + for (const spec of named.elements) { + const importedName = spec.propertyName?.text ?? spec.name.text + analysis.imports.set(spec.name.text, {resolvedPath, importedName}) + } + } + } + } + + // export {foo} ; export {foo as bar} + if (ts.isExportDeclaration(node) && node.exportClause && ts.isNamedExports(node.exportClause)) { + for (const spec of node.exportClause.elements) { + const localName = spec.propertyName?.text ?? spec.name.text + analysis.exports.set(spec.name.text, localName) + } + } + + // export const foo = ... + if (ts.isVariableStatement(node) && node.modifiers?.some((mod) => mod.kind === ts.SyntaxKind.ExportKeyword)) { + for (const decl of node.declarationList.declarations) { + if (ts.isIdentifier(decl.name)) analysis.exports.set(decl.name.text, decl.name.text) + } + } + + ts.forEachChild(node, visit) + } + visit(analysis.sourceFile) +} + +/** Final pass: collect every intercept callsite using the resolved alias sets. */ +function collectCallsites(ts: typeof import('typescript'), analysis: FileAnalysis): InterceptCallsite[] { + const callsites: InterceptCallsite[] = [] + const {sourceFile, shopifyAliases, interceptAliases} = analysis + + const visit = (node: ts.Node): void => { + if (ts.isCallExpression(node)) { + const callee = node.expression + const isInterceptCall = + isShopifyInterceptAccess(ts, callee, shopifyAliases) || + (ts.isIdentifier(callee) && interceptAliases.has(callee.text)) + + if (isInterceptCall) { + const firstArg = node.arguments[0] + const {line, character} = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) + const base = {file: analysis.path, line: line + 1, column: character + 1} + + if (!firstArg) { + callsites.push({ + ...base, + event: null, + argText: '', + unresolvedReason: 'intercept() called with no event argument', + }) + } else if (ts.isStringLiteralLike(firstArg)) { + // string literal or no-substitution template literal + callsites.push({...base, event: firstArg.text, argText: firstArg.getText(sourceFile)}) + } else { + callsites.push({ + ...base, + event: null, + argText: firstArg.getText(sourceFile), + unresolvedReason: `first argument is a ${ts.SyntaxKind[firstArg.kind]}, not a string literal`, + }) + } + } + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + return callsites +} + +/** Resolve every import module specifier in a file to a local path. */ +async function resolveFileModuleSpecifiers( + ts: typeof import('typescript'), + sourceFile: ts.SourceFile, + filePath: string, +): Promise> { + const specifiers = new Set() + const visit = (node: ts.Node): void => { + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { + specifiers.add(node.moduleSpecifier.text) + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + + const resolved = new Map() + await Promise.all( + [...specifiers].map(async (spec) => { + const path = await resolveLocalModule(spec, filePath) + if (path) resolved.set(spec, path) + }), + ) + return resolved +} + +/** + * Detect all POS intercept events reachable (statically, control-flow-agnostic) + * from the given entry file, following the full local import graph. + */ +export async function detectPosIntercepts(entryFilePath: string): Promise { + const ts = await loadTypeScript() + + const imported = await findAllImportedFiles(entryFilePath) + const allFiles = uniq([entryFilePath, ...imported]) + + // Parse + seed every file. + const analyses = new Map() + await Promise.all( + allFiles.map(async (filePath) => { + let content: string + try { + content = readFileSync(filePath).toString() + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return + } + const sourceFile = ts.createSourceFile(filePath, content, ts.ScriptTarget.Latest, true, scriptKindFor(ts, filePath)) + const analysis: FileAnalysis = { + path: filePath, + sourceFile, + shopifyAliases: new Set([SHOPIFY_GLOBAL]), + interceptAliases: new Set(), + exports: new Map(), + imports: new Map(), + } + const resolved = await resolveFileModuleSpecifiers(ts, sourceFile, filePath) + collectImportsAndExports(ts, analysis, resolved) + analyses.set(filePath, analysis) + }), + ) + + // Whole-graph fixpoint: grow intra-file aliases, then propagate intercept + // aliases across re-exports/imports, until nothing changes. + let changed = true + while (changed) { + changed = false + for (const analysis of analyses.values()) { + // Intra-file fixpoint. + let grew = true + while (grew) { + grew = growAliases(ts, analysis) + if (grew) changed = true + } + // Cross-file: import of an exported intercept alias becomes an alias here. + for (const [localName, {resolvedPath, importedName}] of analysis.imports) { + if (analysis.interceptAliases.has(localName)) continue + const target = analyses.get(resolvedPath) + if (!target) continue + const targetLocal = target.exports.get(importedName) + if (targetLocal && target.interceptAliases.has(targetLocal)) { + analysis.interceptAliases.add(localName) + changed = true + } + } + } + } + + // Collect callsites across the graph. + const callsites: InterceptCallsite[] = [] + for (const analysis of analyses.values()) { + callsites.push(...collectCallsites(ts, analysis)) + } + + const events = uniq( + callsites.filter((callsite) => callsite.event !== null).map((callsite) => callsite.event as string), + ).sort() + const unresolved = callsites.filter((callsite) => callsite.event === null) + + return {events, callsites, unresolved, analyzedFiles: allFiles} +} + +/** + * Convenience wrapper for the deploy path: given an extension directory, find + * its entry and return the derived events (empty array if no entry found). + */ +export async function deriveInterceptsFromDirectory( + directory: string, +): Promise { + const entry = await findPosExtensionEntry(directory) + if (!entry) return undefined + return detectPosIntercepts(entry) +} diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts new file mode 100644 index 00000000000..1a978b0ad55 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts @@ -0,0 +1,33 @@ +import {deriveAndMergeIntercepts} from './pos_ui_extension.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {fileURLToPath} from 'node:url' +import {dirname} from 'node:path' +import {describe, expect, test} from 'vitest' + +const currentDir = dirname(fileURLToPath(import.meta.url)) +const fixtureDir = joinPath(currentDir, 'fixtures', 'pos-intercept-demo') + +describe('deriveAndMergeIntercepts (deploy path)', () => { + test('emits derived events into capabilities.intercepts when TOML declares none', async () => { + // Given no declared intercepts, derivation alone populates the field the + // backend reads. + const result = await deriveAndMergeIntercepts(undefined, fixtureDir) + + expect(result?.intercepts).toContain('beforecheckout') + expect(result?.intercepts).toContain('beforecapture') + }) + + test('unions declared and derived events (declaration keeps working)', async () => { + const result = await deriveAndMergeIntercepts({intercepts: ['manuallydeclared']}, fixtureDir) + + expect(result?.intercepts).toContain('manuallydeclared') + expect(result?.intercepts).toContain('beforecheckout') + // De-duplicated + sorted. + expect(result?.intercepts).toEqual([...new Set(result?.intercepts)].sort()) + }) + + test('falls back to declared capabilities when no entry file exists', async () => { + const result = await deriveAndMergeIntercepts({intercepts: ['onlydeclared']}, '/tmp/does-not-exist-xyz') + expect(result?.intercepts).toEqual(['onlydeclared']) + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts index 75da3471ab6..f487a743cf5 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts @@ -2,8 +2,10 @@ import {getDependencyVersion} from '../../app/app.js' import {createExtensionSpecification} from '../specification.js' import {BaseSchema, CapabilitiesSchema} from '../schemas.js' import {ExtensionInstance} from '../extension-instance.js' +import {deriveInterceptsFromDirectory} from './pos_intercept_detection.js' import {BugError} from '@shopify/cli-kit/node/error' import {zod} from '@shopify/cli-kit/node/schema' +import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output' const dependency = '@shopify/retail-ui-extensions' @@ -47,6 +49,52 @@ const PosUISchema = BaseSchema.extend({ capabilities: PosCapabilitiesSchema.optional(), }) +type PosCapabilities = zod.infer + +/** + * SPIKE: derive intercept events from the extension source and merge them into + * the capabilities object emitted at deploy time. + * + * The derived events are unioned with any hand-authored `capabilities.intercepts` + * so both the derivation-first and the TOML-declaration workflows produce the + * same shape. Unresolved (dynamic) event args are logged as a warning — they are + * a known reliability gap and are never silently dropped. + * + * Detection failures are non-fatal: we fall back to whatever the TOML declared + * so a source-parsing edge case can never block a deploy. + */ +export async function deriveAndMergeIntercepts( + capabilities: PosCapabilities | undefined, + directory: string, +): Promise { + let derivedEvents: string[] = [] + try { + const detection = await deriveInterceptsFromDirectory(directory) + if (detection) { + derivedEvents = detection.events + if (detection.unresolved.length > 0) { + const locations = detection.unresolved + .map((callsite) => `${callsite.file}:${callsite.line} (${callsite.argText || ''})`) + .join(', ') + outputWarn( + `POS intercept detection could not statically resolve ${detection.unresolved.length} intercept event(s): ${locations}. ` + + `Declare these explicitly under capabilities.intercepts if the host must know about them.`, + ) + } + outputDebug(`Derived POS intercept events from source: ${derivedEvents.join(', ') || '(none)'}`) + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + outputWarn(`POS intercept source detection failed; falling back to declared capabilities.intercepts. ${error}`) + } + + const declaredEvents = capabilities?.intercepts ?? [] + const mergedEvents = [...new Set([...declaredEvents, ...derivedEvents])].sort() + + if (mergedEvents.length === 0) return capabilities + return {...capabilities, intercepts: mergedEvents} +} + const posUISpec = createExtensionSpecification({ identifier: 'pos_ui_extension', dependency, @@ -62,11 +110,23 @@ const posUISpec = createExtensionSpecification({ deployConfig: async (config, directory) => { const result = await getDependencyVersion(dependency, directory) if (result === 'not_found') throw new BugError(`Dependency ${dependency} not found`) + + // Derive intercept events from the extension's SOURCE CODE (control-flow + // agnostic AST walk of the import graph) and fold them into + // `capabilities.intercepts`. This is the transmission mechanism for derived + // events: there is no dedicated wire field. deployConfig emits the SAME + // `capabilities.intercepts` array the backend already reads — whether an + // event came from the TOML or from source, it lands in the deployed version + // config identically, so the backend persists it as a capability with no + // backend change required. Derived + TOML-declared events are unioned so a + // hand-authored declaration keeps working alongside derivation. + const capabilities = await deriveAndMergeIntercepts(config.capabilities, directory) + return { name: config.name, description: config.description, renderer_version: result?.version, - capabilities: config.capabilities, + capabilities, } }, }) From ccc34b11ffd404f721c229ddcb98d13318feedd9 Mon Sep 17 00:00:00 2001 From: Henry Stelle Date: Fri, 10 Jul 2026 11:54:06 -0700 Subject: [PATCH 2/6] SPIKE: target-driven entry discovery + simple detector comparison MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Entry points now come from declared targets (targeting/extension_points module paths), scoped to the pos.app.ready.data background target (the only POS target that supports intercepts — it exposes BackgroundShopifyGlobal). Dropped the index.* filename guessing. Render targets are ignored (proven by a home-tile fixture whose event must not leak). deployConfig derives from the parsed config. Adds a deliberately simple baseline detector (direct + same-file destructure only) and a comparison matrix test showing the complex detector's extra coverage is exactly: object-aliasing, reassignment, and cross-file re-exported references; parity on direct/destructure/branches/dynamic. Assisted-By: devx/76608ac5-410d-425f-93b9-9eea3cc49f3f --- .../pos-intercept-compare/case-branches.ts | 6 + .../case-crossfile-dep.ts | 2 + .../pos-intercept-compare/case-crossfile.ts | 2 + .../pos-intercept-compare/case-destructure.ts | 3 + .../pos-intercept-compare/case-direct.ts | 2 + .../pos-intercept-compare/case-dynamic.ts | 3 + .../case-object-alias.ts | 3 + .../pos-intercept-compare/case-reassign.ts | 4 + .../pos-intercept-demo/src/home-tile.ts | 8 + .../pos_intercept_detection.DESIGN.md | 62 ++++++- .../pos_intercept_detection.test.ts | 60 ++++++- .../specifications/pos_intercept_detection.ts | 89 +++++++--- ...pos_intercept_detection_comparison.test.ts | 129 +++++++++++++++ .../pos_intercept_detection_simple.ts | 155 ++++++++++++++++++ .../specifications/pos_ui_extension.test.ts | 24 ++- .../specifications/pos_ui_extension.ts | 31 ++-- 16 files changed, 527 insertions(+), 56 deletions(-) create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-branches.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile-dep.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-destructure.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-direct.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-dynamic.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-object-alias.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-reassign.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/home-tile.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-branches.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-branches.ts new file mode 100644 index 00000000000..4ccbebadbc6 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-branches.ts @@ -0,0 +1,6 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +if (Math.random() > 0.5) { + shopify.intercept('branchif', () => {}) +} else { + shopify.intercept('branchelse', () => {}) +} diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile-dep.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile-dep.ts new file mode 100644 index 00000000000..4bd8aa908e0 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile-dep.ts @@ -0,0 +1,2 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +export const block = shopify.intercept diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile.ts new file mode 100644 index 00000000000..87ece2760b8 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-crossfile.ts @@ -0,0 +1,2 @@ +import {block} from './case-crossfile-dep.js' +block('crossfile', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-destructure.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-destructure.ts new file mode 100644 index 00000000000..72a40ddd307 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-destructure.ts @@ -0,0 +1,3 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const {intercept} = shopify +intercept('destructured', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-direct.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-direct.ts new file mode 100644 index 00000000000..67e35b7d4d2 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-direct.ts @@ -0,0 +1,2 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +shopify.intercept('beforecheckout', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-dynamic.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-dynamic.ts new file mode 100644 index 00000000000..9409f849dff --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-dynamic.ts @@ -0,0 +1,3 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const evt = 'dynamic' +shopify.intercept(evt, () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-object-alias.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-object-alias.ts new file mode 100644 index 00000000000..d539e2207b0 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-object-alias.ts @@ -0,0 +1,3 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const s = shopify +s.intercept('objectalias', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-reassign.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-reassign.ts new file mode 100644 index 00000000000..7af18fcd7d5 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-reassign.ts @@ -0,0 +1,4 @@ +declare const shopify: {intercept: (event: string, handler: () => void) => void} +let fn: typeof shopify.intercept +fn = shopify.intercept +fn('reassigned', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/home-tile.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/home-tile.ts new file mode 100644 index 00000000000..2b726a460ca --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-demo/src/home-tile.ts @@ -0,0 +1,8 @@ +// Module for the `pos.home.tile.render` target. This target does NOT support +// intercepts, so the detector must NOT scan it. If scoping is broken, the event +// below would leak into the derived capabilities. +declare const shopify: {intercept: (event: string, handler: () => void) => void} + +export function renderTile() { + shopify.intercept('shouldnotappear', () => {}) +} diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md index ac65f236f8b..efdfa561312 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md @@ -10,21 +10,44 @@ control-flow-agnostic AST walk of the import graph. Files: -- `pos_intercept_detection.ts` — the detector (`detectPosIntercepts`, - `deriveInterceptsFromDirectory`, `findPosExtensionEntry`). +- `pos_intercept_detection.ts` — the full detector (`detectPosIntercepts`, + `deriveInterceptsFromConfig`, `findInterceptEntryModules`, `POS_INTERCEPT_TARGET`). +- `pos_intercept_detection_simple.ts` — a deliberately simple baseline detector + (`detectPosInterceptsSimple`) for evaluating whether the complexity is worth it. - `pos_intercept_detection.test.ts` — unit tests against the demo fixture. +- `pos_intercept_detection_comparison.test.ts` — complex-vs-simple matrix. - `fixtures/pos-intercept-demo/` — a POS extension exercising every alias form, - control-flow branches, a cross-file re-exported alias, and dynamic args. + control-flow branches, a cross-file re-exported alias, and dynamic args, plus a + render-target module (`home-tile.ts`) that must be IGNORED. +- `fixtures/pos-intercept-compare/` — one isolated file per pattern for the matrix. - `pos_ui_extension.ts` — `deployConfig` now calls `deriveAndMergeIntercepts`. - `pos_ui_extension.test.ts` — proves the derived events land in the deployed `capabilities.intercepts`. +## Entry points come from declared TARGETS, scoped to `pos.app.ready.data` + +Entry points are NOT guessed filenames (`index.*`). They are the `module` paths +of the extension's declared targets (`targeting` / legacy `extension_points`, +per `NewExtensionPointSchema`). Detection is scoped to a single target: + +> **`pos.app.ready.data`** — the session-lifetime BACKGROUND target. + +Confirmed against the published `@shopify/ui-extensions` point-of-sale surface: +`.../surfaces/point-of-sale/targets/pos.app.ready.data.d.ts` re-exports +`BackgroundShopifyGlobal as ShopifyGlobal`, and `globals.d.ts` documents that the +background-only global (valid only from `pos.app.ready.data`) is what carries +the host-mediated event/intercept APIs. Render targets +(`pos.home.tile.render`, `pos.home.modal.render`, …) see the narrower +`ShopifyGlobal` and do not support intercepts, so we don't scan them. +`findInterceptEntryModules(config, directory)` filters targets to +`pos.app.ready.data` and returns their resolved `module` paths as the entry set. + ### Detection behaviour (matches the brief) 1. **Reuses the CLI's existing AST tooling.** It calls `findAllImportedFiles()` - from `type-generation.ts` to walk the full import graph from the extension's - `index.*` entry, and uses the same `typescript` compiler API - (`ts.createSourceFile` / `forEachChild` / `isCallExpression`). + from `type-generation.ts` to walk the full import graph from the + `pos.app.ready.data` target's `module`(s), and uses the same `typescript` + compiler API (`ts.createSourceFile` / `forEachChild` / `isCallExpression`). 2. **Ignores control flow.** Every `shopify.intercept(...)` callsite is counted regardless of `if`/`else`/ternary/loop/dead-code. Capabilities are a static **superset** of what the extension might do; reachability is a runtime @@ -48,9 +71,34 @@ Run the tests: cd packages/app ../../node_modules/.bin/vitest run \ src/cli/models/extensions/specifications/pos_intercept_detection.test.ts \ - src/cli/models/extensions/specifications/pos_ui_extension.test.ts + src/cli/models/extensions/specifications/pos_ui_extension.test.ts \ + src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts ``` +## Complex vs simple detector — is the complexity worth it? + +A second, deliberately simple detector (`pos_intercept_detection_simple.ts`) +matches ONLY direct `shopify.intercept('x')` calls plus same-file +`const {intercept} = shopify` destructuring. The comparison matrix +(`pos_intercept_detection_comparison.test.ts`) runs both over per-pattern +fixtures: + +| pattern | complex | simple | +|---------|:-------:|:------:| +| `shopify.intercept('beforecheckout')` | catch | catch | +| `const {intercept} = shopify; intercept('x')` | catch | catch | +| `const s = shopify; s.intercept('x')` | catch | **MISS** | +| `let fn; fn = shopify.intercept; fn('x')` | catch | **MISS** | +| cross-file `export const block = shopify.intercept` imported+called | catch | **MISS** | +| `if/else` both branches | catch | catch | +| dynamic variable arg | unresolved | unresolved | + +The extra complexity buys coverage for exactly three real-world patterns: +object-aliasing, reassignment, and cross-file re-exported references. For direct +calls, same-file destructuring, control-flow branches, and dynamic args the two +detectors are identical. This is the data point for deciding how much alias +tracking to ship. + --- ## THE KEY DESIGN QUESTION: derived events → backend diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts index 2261e2849b0..49ee45d839f 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.test.ts @@ -1,4 +1,9 @@ -import {detectPosIntercepts, deriveInterceptsFromDirectory, findPosExtensionEntry} from './pos_intercept_detection.js' +import { + detectPosIntercepts, + deriveInterceptsFromConfig, + findInterceptEntryModules, + POS_INTERCEPT_TARGET, +} from './pos_intercept_detection.js' import {joinPath} from '@shopify/cli-kit/node/path' import {fileURLToPath} from 'node:url' import {dirname} from 'node:path' @@ -7,16 +12,45 @@ import {describe, expect, test} from 'vitest' const currentDir = dirname(fileURLToPath(import.meta.url)) const fixtureDir = joinPath(currentDir, 'fixtures', 'pos-intercept-demo') +// A parsed config for the demo extension: the intercept-supporting background +// target points at src/index.ts; a render target (which must be IGNORED) points +// at src/home-tile.ts. +const demoConfig = { + targeting: [ + {target: POS_INTERCEPT_TARGET, module: 'src/index.ts'}, + {target: 'pos.home.tile.render', module: 'src/home-tile.ts'}, + ], +} + +describe('findInterceptEntryModules', () => { + test('returns only the pos.app.ready.data target module, ignoring render targets', () => { + const modules = findInterceptEntryModules(demoConfig, fixtureDir) + expect(modules).toEqual([joinPath(fixtureDir, 'src/index.ts')]) + }) + + test('reads the legacy extension_points field too', () => { + const modules = findInterceptEntryModules( + {extension_points: [{target: POS_INTERCEPT_TARGET, module: 'src/index.ts'}]}, + fixtureDir, + ) + expect(modules).toEqual([joinPath(fixtureDir, 'src/index.ts')]) + }) + + test('returns nothing when no intercept-supporting target is declared', () => { + expect(findInterceptEntryModules({targeting: [{target: 'pos.home.tile.render', module: 'src/home-tile.ts'}]}, fixtureDir)).toEqual([]) + }) +}) + describe('detectPosIntercepts', () => { test('derives events across the import graph, ignoring control flow and tracking aliases', async () => { - // Given - const entry = await findPosExtensionEntry(fixtureDir) - expect(entry).toBeDefined() + // Given the intercept target module as the entry. + const [entry] = findInterceptEntryModules(demoConfig, fixtureDir) // When const result = await detectPosIntercepts(entry!) // Then — every resolvable event, regardless of alias form or control flow. + // 'shouldnotappear' lives in the render-target module and must NOT appear. expect(result.events).toEqual([ 'beforecancel', // cross-file re-exported alias 'beforecapture', // imported file destructured alias @@ -29,10 +63,11 @@ describe('detectPosIntercepts', () => { 'beforetax', // if branch — reachability ignored 'beforevoid', // let reassignment ]) + expect(result.events).not.toContain('shouldnotappear') }) test('surfaces dynamic/computed event args as unresolved instead of dropping them', async () => { - const entry = await findPosExtensionEntry(fixtureDir) + const [entry] = findInterceptEntryModules(demoConfig, fixtureDir) const result = await detectPosIntercepts(entry!) // Two dynamic callsites: a variable and a template string with substitution. @@ -47,10 +82,21 @@ describe('detectPosIntercepts', () => { expect(callsite.line).toBeGreaterThan(0) }) }) +}) - test('deriveInterceptsFromDirectory drives detection from an extension directory', async () => { - const result = await deriveInterceptsFromDirectory(fixtureDir) +describe('deriveInterceptsFromConfig', () => { + test('drives detection from the parsed config + directory (target-scoped)', async () => { + const result = await deriveInterceptsFromConfig(demoConfig, fixtureDir) expect(result?.events).toContain('beforecheckout') + expect(result?.events).not.toContain('shouldnotappear') expect(result?.analyzedFiles.length).toBeGreaterThanOrEqual(3) }) + + test('returns undefined when no intercept target is declared', async () => { + const result = await deriveInterceptsFromConfig( + {targeting: [{target: 'pos.home.tile.render', module: 'src/home-tile.ts'}]}, + fixtureDir, + ) + expect(result).toBeUndefined() + }) }) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts index 864837e4ed7..39af743abe0 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts @@ -8,8 +8,16 @@ import type ts from 'typescript' // SPIKE / PROTOTYPE: derive POS intercept events from extension SOURCE CODE. // // Instead of trusting the hand-authored `capabilities.intercepts` TOML array, -// this walks the extension's import graph (starting from its `index.*` entry) -// and statically detects every `shopify.intercept('', ...)` callsite. +// this walks the extension's import graph and statically detects every +// `shopify.intercept('', ...)` callsite. +// +// ENTRY POINTS COME FROM THE DECLARED TARGETS, not from guessed filenames. Each +// target/extension-point in the parsed config carries a `module` path (see +// NewExtensionPointSchema). We SCOPE detection to a single target: +// `pos.app.ready.data` — the session-lifetime BACKGROUND target (it exposes the +// `BackgroundShopifyGlobal`, under which `intercept` lives). The render targets +// (pos.home.tile.render, pos.home.modal.render, etc.) do not support intercepts, +// so we don't scan them. We walk the import graph from that target's module(s). // // Design notes / deliberate choices (per the spike brief): // * We IGNORE control flow. Every callsite counts, regardless of whether it @@ -78,20 +86,39 @@ interface FileAnalysis { } /** - * Resolve the entry `index.{js,jsx,ts,tsx}` file for a POS UI extension given - * its directory. Mirrors AppLoader.findEntryPath for `single_js_entry_path` - * extensions so the detector can be driven straight from a deploy directory. + * The ONLY POS target that supports intercepts: the session-lifetime background + * target. Its module is the entry point for intercept derivation. Confirmed + * against the @shopify/ui-extensions point-of-sale surface, where + * `pos.app.ready.data` re-exports `BackgroundShopifyGlobal` (the only global + * that carries the host-mediated event/intercept APIs). */ -export async function findPosExtensionEntry(directory: string): Promise { - const candidates = ['index'] - .flatMap((name) => [`${name}.js`, `${name}.jsx`, `${name}.ts`, `${name}.tsx`]) - .flatMap((fileName) => [`src/${fileName}`, fileName]) - .map((relativePath) => joinPath(directory, relativePath)) - - const found = await Promise.all( - candidates.map(async (candidate) => ((await fileExists(candidate)) ? candidate : undefined)), - ) - return found.find((candidate) => candidate !== undefined) +export const POS_INTERCEPT_TARGET = 'pos.app.ready.data' + +/** Minimal shape of a declared target/extension-point (see NewExtensionPointSchema). */ +export interface ExtensionTargetLike { + target?: string + module?: string +} + +/** + * Given an extension's parsed configuration and directory, return the absolute + * module path(s) for the intercept-supporting target (`pos.app.ready.data`). + * + * Targets are read from `targeting` (preferred) or the legacy `extension_points` + * array — exactly the fields ui_extension uses. Entry points are the declared + * `module` paths, NOT a guessed `index.*` filename. + */ +export function findInterceptEntryModules( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config: {targeting?: any; extension_points?: any} | undefined, + directory: string, +): string[] { + const rawTargets = config?.targeting ?? config?.extension_points ?? [] + if (!Array.isArray(rawTargets)) return [] + + return (rawTargets as ExtensionTargetLike[]) + .filter((entry) => entry?.target === POS_INTERCEPT_TARGET && typeof entry?.module === 'string') + .map((entry) => resolvePath(directory, entry.module as string)) } function scriptKindFor(ts: typeof import('typescript'), filePath: string): ts.ScriptKind { @@ -317,13 +344,22 @@ async function resolveFileModuleSpecifiers( /** * Detect all POS intercept events reachable (statically, control-flow-agnostic) - * from the given entry file, following the full local import graph. + * from the given entry point(s), following the full local import graph. Accepts + * one or more entry modules (a target may declare more than one, and an + * extension may declare the intercept target more than once). */ -export async function detectPosIntercepts(entryFilePath: string): Promise { +export async function detectPosIntercepts( + entryFilePaths: string | string[], +): Promise { const ts = await loadTypeScript() - const imported = await findAllImportedFiles(entryFilePath) - const allFiles = uniq([entryFilePath, ...imported]) + const entries = uniq((Array.isArray(entryFilePaths) ? entryFilePaths : [entryFilePaths]).filter(Boolean)) + + // Walk every entry's import graph, sharing a `visited` set so shared modules + // are analyzed once. + const visited = new Set() + const importedByEntry = await Promise.all(entries.map((entry) => findAllImportedFiles(entry, visited))) + const allFiles = uniq([...entries, ...importedByEntry.flat()]) // Parse + seed every file. const analyses = new Map() @@ -392,13 +428,16 @@ export async function detectPosIntercepts(entryFilePath: string): Promise { - const entry = await findPosExtensionEntry(directory) - if (!entry) return undefined - return detectPosIntercepts(entry) + const entryModules = findInterceptEntryModules(config, directory) + if (entryModules.length === 0) return undefined + return detectPosIntercepts(entryModules) } diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts new file mode 100644 index 00000000000..b7bb9ae204f --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts @@ -0,0 +1,129 @@ +import {detectPosIntercepts} from './pos_intercept_detection.js' +import {detectPosInterceptsSimple} from './pos_intercept_detection_simple.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import {fileURLToPath} from 'node:url' +import {dirname} from 'node:path' +import {describe, expect, test} from 'vitest' + +const currentDir = dirname(fileURLToPath(import.meta.url)) +const fixtures = joinPath(currentDir, 'fixtures', 'pos-intercept-compare') +const fixture = (name: string) => joinPath(fixtures, name) + +// --------------------------------------------------------------------------- +// COMPARISON MATRIX: complex (full) vs simple detector, per real-world pattern. +// +// pattern | complex | simple +// ------------------------------------------------|---------|-------- +// shopify.intercept('beforecheckout') | catch | catch +// const {intercept} = shopify; intercept('x') | catch | catch +// const s = shopify; s.intercept('x') | catch | MISS +// let fn; fn = shopify.intercept; fn('x') | catch | MISS +// export const block = shopify.intercept (x-file) | catch | MISS +// if/else both branches | catch | catch +// dynamic arg (variable) | unresolved (both) +// +// Each `expected` below encodes exactly this matrix and the test asserts both +// detectors against it, so the delta is explicit and self-documenting. +// --------------------------------------------------------------------------- + +interface Case { + name: string + file: string + /** Event names the COMPLEX detector should resolve. */ + complexEvents: string[] + /** Event names the SIMPLE detector should resolve. */ + simpleEvents: string[] + /** Unresolved-arg count expected from BOTH detectors. */ + unresolvedBoth?: number +} + +const cases: Case[] = [ + { + name: 'direct shopify.intercept — BOTH catch', + file: 'case-direct.ts', + complexEvents: ['beforecheckout'], + simpleEvents: ['beforecheckout'], + }, + { + name: 'same-file destructure const {intercept} = shopify — BOTH catch', + file: 'case-destructure.ts', + complexEvents: ['destructured'], + simpleEvents: ['destructured'], + }, + { + name: 'object-alias const s = shopify; s.intercept(...) — COMPLEX only', + file: 'case-object-alias.ts', + complexEvents: ['objectalias'], + simpleEvents: [], + }, + { + name: 'reassignment let fn; fn = shopify.intercept — COMPLEX only', + file: 'case-reassign.ts', + complexEvents: ['reassigned'], + simpleEvents: [], + }, + { + name: 'cross-file re-exported reference — COMPLEX only', + file: 'case-crossfile.ts', + complexEvents: ['crossfile'], + simpleEvents: [], + }, + { + name: 'if/else both branches — BOTH catch', + file: 'case-branches.ts', + complexEvents: ['branchelse', 'branchif'], + simpleEvents: ['branchelse', 'branchif'], + }, + { + name: 'dynamic variable arg — NEITHER resolves (both report unresolved)', + file: 'case-dynamic.ts', + complexEvents: [], + simpleEvents: [], + unresolvedBoth: 1, + }, +] + +describe('complex vs simple POS intercept detector — comparison matrix', () => { + cases.forEach((testCase) => { + test(testCase.name, async () => { + const [complex, simple] = await Promise.all([ + detectPosIntercepts(fixture(testCase.file)), + detectPosInterceptsSimple(fixture(testCase.file)), + ]) + + expect(complex.events).toEqual(testCase.complexEvents) + expect(simple.events).toEqual(testCase.simpleEvents) + + if (testCase.unresolvedBoth !== undefined) { + expect(complex.unresolved).toHaveLength(testCase.unresolvedBoth) + expect(simple.unresolved).toHaveLength(testCase.unresolvedBoth) + } + }) + }) + + test('summary: which patterns the extra complexity buys coverage for', async () => { + const results = await Promise.all( + cases.map(async (testCase) => { + const [complex, simple] = await Promise.all([ + detectPosIntercepts(fixture(testCase.file)), + detectPosInterceptsSimple(fixture(testCase.file)), + ]) + return {name: testCase.name, complex: complex.events, simple: simple.events} + }), + ) + + // Patterns where complex strictly beats simple (the value of the complexity). + const complexOnly = results.filter( + (row) => row.complex.length > 0 && row.simple.length < row.complex.length, + ) + expect(complexOnly.map((row) => row.name).sort()).toEqual([ + 'cross-file re-exported reference — COMPLEX only', + 'object-alias const s = shopify; s.intercept(...) — COMPLEX only', + 'reassignment let fn; fn = shopify.intercept — COMPLEX only', + ]) + + // Patterns where both agree (complexity adds nothing). + const parity = results.filter((row) => row.complex.join() === row.simple.join()) + expect(parity).toHaveLength(4) + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts new file mode 100644 index 00000000000..f2367c66395 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts @@ -0,0 +1,155 @@ +import {findInterceptEntryModules, InterceptCallsite, InterceptDetectionResult} from './pos_intercept_detection.js' +import {findAllImportedFiles} from './type-generation.js' +import {readFileSync} from '@shopify/cli-kit/node/fs' +import {uniq} from '@shopify/cli-kit/common/array' +import type ts from 'typescript' + +// --------------------------------------------------------------------------- +// SPIKE / PROTOTYPE: the DELIBERATELY SIMPLE POS intercept detector. +// +// This is a comparison baseline for the full detector in +// pos_intercept_detection.ts. It does ONLY the straightforward thing: +// +// * Match direct `shopify.intercept('')` callsites (string-literal +// first arg) across the same target-scoped import graph. +// * At most, handle SIMPLE same-file destructuring: +// const {intercept} = shopify → intercept('x') +// const {intercept: rename} = shopify → rename('x') +// ...only when that binding is used in the SAME file it was declared in. +// +// It DELIBERATELY DOES NOT DO (so we can measure what the complexity buys): +// * cross-file alias propagation / re-exported references +// * reassignment tracking (`let fn; fn = shopify.intercept`) +// * object-aliasing (`const s = shopify; s.intercept(...)`) +// * function-parameter / higher-order passing +// +// When a call isn't a direct or simple-destructured match, this detector simply +// MISSES it. That is the whole point of the comparison. +// +// Control-flow handling and unresolved-arg reporting match the full detector: +// every matched callsite counts regardless of branch, and non-string-literal +// first args are surfaced as unresolved (never dropped). +// --------------------------------------------------------------------------- + +async function loadTypeScript(): Promise { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mod: any = await import('typescript') + return mod.default ?? mod +} + +function scriptKindFor(ts: typeof import('typescript'), filePath: string): ts.ScriptKind { + if (filePath.endsWith('.ts')) return ts.ScriptKind.TS + if (filePath.endsWith('.tsx')) return ts.ScriptKind.TSX + return ts.ScriptKind.JSX +} + +/** Collect same-file names bound to `shopify.intercept` via simple destructuring. */ +function collectSimpleDestructureAliases(ts: typeof import('typescript'), sourceFile: ts.SourceFile): Set { + const aliases = new Set() + const visit = (node: ts.Node): void => { + // const {intercept} = shopify / const {intercept: rename} = shopify + if ( + ts.isVariableDeclaration(node) && + node.initializer && + ts.isIdentifier(node.initializer) && + node.initializer.text === 'shopify' && + ts.isObjectBindingPattern(node.name) + ) { + for (const element of node.name.elements) { + const propName = + element.propertyName && ts.isIdentifier(element.propertyName) ? element.propertyName.text : undefined + const localName = ts.isIdentifier(element.name) ? element.name.text : undefined + // `{intercept}` (no propertyName) or `{intercept: rename}` (propertyName === 'intercept') + if (localName && (propName === 'intercept' || (!propName && element.name.getText(sourceFile) === 'intercept'))) { + aliases.add(localName) + } + } + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + return aliases +} + +function analyzeFile(ts: typeof import('typescript'), filePath: string): InterceptCallsite[] { + let content: string + try { + content = readFileSync(filePath).toString() + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return [] + } + const sourceFile = ts.createSourceFile(filePath, content, ts.ScriptTarget.Latest, true, scriptKindFor(ts, filePath)) + const destructureAliases = collectSimpleDestructureAliases(ts, sourceFile) + const callsites: InterceptCallsite[] = [] + + const visit = (node: ts.Node): void => { + if (ts.isCallExpression(node)) { + const callee = node.expression + // Direct: shopify.intercept(...) + const isDirect = + ts.isPropertyAccessExpression(callee) && + ts.isIdentifier(callee.expression) && + callee.expression.text === 'shopify' && + callee.name.text === 'intercept' + // Simple same-file destructured alias: intercept(...) / rename(...) + const isSimpleAlias = ts.isIdentifier(callee) && destructureAliases.has(callee.text) + + if (isDirect || isSimpleAlias) { + const firstArg = node.arguments[0] + const {line, character} = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) + const base = {file: filePath, line: line + 1, column: character + 1} + if (!firstArg) { + callsites.push({...base, event: null, argText: '', unresolvedReason: 'intercept() called with no event argument'}) + } else if (ts.isStringLiteralLike(firstArg)) { + callsites.push({...base, event: firstArg.text, argText: firstArg.getText(sourceFile)}) + } else { + callsites.push({ + ...base, + event: null, + argText: firstArg.getText(sourceFile), + unresolvedReason: `first argument is a ${ts.SyntaxKind[firstArg.kind]}, not a string literal`, + }) + } + } + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + return callsites +} + +/** + * Simple detector: direct + same-file-destructured intercept calls only, over + * the target-scoped import graph. + */ +export async function detectPosInterceptsSimple( + entryFilePaths: string | string[], +): Promise { + const ts = await loadTypeScript() + const entries = uniq((Array.isArray(entryFilePaths) ? entryFilePaths : [entryFilePaths]).filter(Boolean)) + + const visited = new Set() + const importedByEntry = await Promise.all(entries.map((entry) => findAllImportedFiles(entry, visited))) + const allFiles = uniq([...entries, ...importedByEntry.flat()]) + + const callsites: InterceptCallsite[] = [] + for (const filePath of allFiles) { + callsites.push(...analyzeFile(ts, filePath)) + } + + const events = uniq(callsites.filter((cs) => cs.event !== null).map((cs) => cs.event as string)).sort() + const unresolved = callsites.filter((cs) => cs.event === null) + return {events, callsites, unresolved, analyzedFiles: allFiles} +} + +/** Deploy-path parity with the full detector: config-driven, target-scoped. */ +export async function deriveInterceptsFromConfigSimple( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + config: {targeting?: any; extension_points?: any} | undefined, + directory: string, +): Promise { + const entryModules = findInterceptEntryModules(config, directory) + if (entryModules.length === 0) return undefined + return detectPosInterceptsSimple(entryModules) +} diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts index 1a978b0ad55..362f4db5555 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.test.ts @@ -1,4 +1,5 @@ import {deriveAndMergeIntercepts} from './pos_ui_extension.js' +import {POS_INTERCEPT_TARGET} from './pos_intercept_detection.js' import {joinPath} from '@shopify/cli-kit/node/path' import {fileURLToPath} from 'node:url' import {dirname} from 'node:path' @@ -7,18 +8,30 @@ import {describe, expect, test} from 'vitest' const currentDir = dirname(fileURLToPath(import.meta.url)) const fixtureDir = joinPath(currentDir, 'fixtures', 'pos-intercept-demo') +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const configWith = (capabilities?: any): any => ({ + name: 'demo', + capabilities, + targeting: [ + {target: POS_INTERCEPT_TARGET, module: 'src/index.ts'}, + {target: 'pos.home.tile.render', module: 'src/home-tile.ts'}, + ], +}) + describe('deriveAndMergeIntercepts (deploy path)', () => { test('emits derived events into capabilities.intercepts when TOML declares none', async () => { // Given no declared intercepts, derivation alone populates the field the // backend reads. - const result = await deriveAndMergeIntercepts(undefined, fixtureDir) + const result = await deriveAndMergeIntercepts(configWith(undefined), fixtureDir) expect(result?.intercepts).toContain('beforecheckout') expect(result?.intercepts).toContain('beforecapture') + // Render-target event must not leak in. + expect(result?.intercepts).not.toContain('shouldnotappear') }) test('unions declared and derived events (declaration keeps working)', async () => { - const result = await deriveAndMergeIntercepts({intercepts: ['manuallydeclared']}, fixtureDir) + const result = await deriveAndMergeIntercepts(configWith({intercepts: ['manuallydeclared']}), fixtureDir) expect(result?.intercepts).toContain('manuallydeclared') expect(result?.intercepts).toContain('beforecheckout') @@ -26,8 +39,11 @@ describe('deriveAndMergeIntercepts (deploy path)', () => { expect(result?.intercepts).toEqual([...new Set(result?.intercepts)].sort()) }) - test('falls back to declared capabilities when no entry file exists', async () => { - const result = await deriveAndMergeIntercepts({intercepts: ['onlydeclared']}, '/tmp/does-not-exist-xyz') + test('falls back to declared capabilities when no intercept target is declared', async () => { + const result = await deriveAndMergeIntercepts( + {name: 'demo', capabilities: {intercepts: ['onlydeclared']}, targeting: []}, + fixtureDir, + ) expect(result?.intercepts).toEqual(['onlydeclared']) }) }) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts index f487a743cf5..e42a0ba5ffe 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_ui_extension.ts @@ -1,8 +1,8 @@ import {getDependencyVersion} from '../../app/app.js' import {createExtensionSpecification} from '../specification.js' -import {BaseSchema, CapabilitiesSchema} from '../schemas.js' +import {BaseSchema, CapabilitiesSchema, NewExtensionPointsSchema} from '../schemas.js' import {ExtensionInstance} from '../extension-instance.js' -import {deriveInterceptsFromDirectory} from './pos_intercept_detection.js' +import {deriveInterceptsFromConfig} from './pos_intercept_detection.js' import {BugError} from '@shopify/cli-kit/node/error' import {zod} from '@shopify/cli-kit/node/schema' import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output' @@ -47,6 +47,9 @@ type PosUIConfigType = zod.infer const PosUISchema = BaseSchema.extend({ name: zod.string(), capabilities: PosCapabilitiesSchema.optional(), + // Declared mount points. Each target carries a `module` path; intercept + // derivation reads the `pos.app.ready.data` target's module as its entry. + targeting: NewExtensionPointsSchema.optional(), }) type PosCapabilities = zod.infer @@ -64,12 +67,13 @@ type PosCapabilities = zod.infer * so a source-parsing edge case can never block a deploy. */ export async function deriveAndMergeIntercepts( - capabilities: PosCapabilities | undefined, + config: PosUIConfigType | undefined, directory: string, ): Promise { + const capabilities = config?.capabilities let derivedEvents: string[] = [] try { - const detection = await deriveInterceptsFromDirectory(directory) + const detection = await deriveInterceptsFromConfig(config, directory) if (detection) { derivedEvents = detection.events if (detection.unresolved.length > 0) { @@ -112,15 +116,16 @@ const posUISpec = createExtensionSpecification({ if (result === 'not_found') throw new BugError(`Dependency ${dependency} not found`) // Derive intercept events from the extension's SOURCE CODE (control-flow - // agnostic AST walk of the import graph) and fold them into - // `capabilities.intercepts`. This is the transmission mechanism for derived - // events: there is no dedicated wire field. deployConfig emits the SAME - // `capabilities.intercepts` array the backend already reads — whether an - // event came from the TOML or from source, it lands in the deployed version - // config identically, so the backend persists it as a capability with no - // backend change required. Derived + TOML-declared events are unioned so a - // hand-authored declaration keeps working alongside derivation. - const capabilities = await deriveAndMergeIntercepts(config.capabilities, directory) + // agnostic AST walk of the pos.app.ready.data target's import graph) and + // fold them into `capabilities.intercepts`. This is the transmission + // mechanism for derived events: there is no dedicated wire field. + // deployConfig emits the SAME `capabilities.intercepts` array the backend + // already reads — whether an event came from the TOML or from source, it + // lands in the deployed version config identically, so the backend persists + // it as a capability with no backend change required. Derived + TOML-declared + // events are unioned so a hand-authored declaration keeps working alongside + // derivation. + const capabilities = await deriveAndMergeIntercepts(config, directory) return { name: config.name, From acba79c9df5f3c416b6531ddd1bea88881783ddd Mon Sep 17 00:00:00 2001 From: Henry Stelle Date: Fri, 10 Jul 2026 12:01:31 -0700 Subject: [PATCH 3/6] SPIKE: reshape simple detector into safe-simplest (resolve-or-warn) Simple detector now resolves ONLY direct shopify.intercept('') calls and, instead of silently missing indirect usage, detects alias-creation patterns syntactically and warns (file:line + raw source) telling the developer to declare the intercept in TOML. Warn kinds: destructure, function-reference, object-alias-access, dynamic-arg. Object-alias warning is scoped to an actual .intercept access so a bare const s = shopify used for other reasons never warns (case-alias-noise fixture proves no false positive). Comparison test reworked into a three-way matrix: complex-resolves / simple-resolves / simple-warns, with a KEY PROPERTY assertion that the simple detector has zero silent misses on alias patterns. Assisted-By: devx/76608ac5-410d-425f-93b9-9eea3cc49f3f --- .../pos-intercept-compare/case-alias-noise.ts | 5 + .../pos_intercept_detection.DESIGN.md | 84 +++++-- ...pos_intercept_detection_comparison.test.ts | 120 ++++----- .../pos_intercept_detection_simple.ts | 228 ++++++++++++------ 4 files changed, 289 insertions(+), 148 deletions(-) create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-noise.ts diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-noise.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-noise.ts new file mode 100644 index 00000000000..d070c98f2de --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-noise.ts @@ -0,0 +1,5 @@ +// A shopify-object alias used for NON-intercept reasons. The simple detector +// must NOT warn here — it only flags aliases when `.intercept` is accessed. +declare const shopify: {intercept: (e: string, h: () => void) => void; toast: {show: (m: string) => void}} +const s = shopify +s.toast.show('hi') diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md index efdfa561312..6c7c4086c12 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md @@ -75,29 +75,67 @@ cd packages/app src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts ``` -## Complex vs simple detector — is the complexity worth it? - -A second, deliberately simple detector (`pos_intercept_detection_simple.ts`) -matches ONLY direct `shopify.intercept('x')` calls plus same-file -`const {intercept} = shopify` destructuring. The comparison matrix -(`pos_intercept_detection_comparison.test.ts`) runs both over per-pattern -fixtures: - -| pattern | complex | simple | -|---------|:-------:|:------:| -| `shopify.intercept('beforecheckout')` | catch | catch | -| `const {intercept} = shopify; intercept('x')` | catch | catch | -| `const s = shopify; s.intercept('x')` | catch | **MISS** | -| `let fn; fn = shopify.intercept; fn('x')` | catch | **MISS** | -| cross-file `export const block = shopify.intercept` imported+called | catch | **MISS** | -| `if/else` both branches | catch | catch | -| dynamic variable arg | unresolved | unresolved | - -The extra complexity buys coverage for exactly three real-world patterns: -object-aliasing, reassignment, and cross-file re-exported references. For direct -calls, same-file destructuring, control-flow branches, and dynamic args the two -detectors are identical. This is the data point for deciding how much alias -tracking to ship. +## Complex (resolve) vs "safe simplest" (resolve-or-warn) + +The second detector (`pos_intercept_detection_simple.ts`) is now the **safe +simplest** design. Its rule: **never silently under-report.** A call is either +DERIVED SILENTLY (direct `shopify.intercept('')`) or FLAGGED +LOUDLY (a warning with file:line + raw source telling the developer to declare +that intercept in the TOML). It resolves nothing indirect and follows nothing +cross-file — it recognizes intercept-shaped patterns *syntactically* and warns. + +Three-way matrix (`pos_intercept_detection_comparison.test.ts`): + +| pattern | complex resolves | simple resolves | simple warns | +|---------|:-------:|:-------:|:-----| +| `shopify.intercept('beforecheckout')` | yes | yes | — | +| `const {intercept} = shopify; intercept('x')` | yes | no | `destructure` | +| `const s = shopify; s.intercept('x')` | yes | no | `object-alias-access` | +| `let fn; fn = shopify.intercept; fn('x')` | yes | no | `function-reference` | +| cross-file `export const block = shopify.intercept` | yes | no | `function-reference` (at creation site) | +| `if/else` both branches | yes | yes | — | +| dynamic variable arg | unresolved | no | `dynamic-arg` | +| `const s = shopify; s.toast.show()` (noise) | n/a | no | **— (no false positive)** | + +**Key property:** the simple detector has ZERO silent misses on the alias +patterns — where it can't resolve, it warns. + +### Is warn-on-alias simpler or more complex than full resolution? + +**Honest assessment: simpler, and meaningfully so.** Code-ish (non-comment) +lines: complex ≈ 295, simple ≈ 172 (~42% smaller). More importantly, the simple +detector DROPS the expensive machinery: + +- No cross-file alias propagation and no whole-graph fixpoint loop. +- No per-file import/export symbol maps. +- No multi-pass alias-set growth (`shopifyAliases`/`interceptAliases`). +- No `let`-reassignment data-flow. + +What it ADDS is cheap and local: a single-pass syntactic check per file that (a) +resolves direct literal calls and (b) pattern-matches four warn shapes. The only +state it keeps is a one-level set of `const s = shopify` alias names, used *only* +to scope the object-alias warning. + +### False-positive / noise risk and how it's scoped + +- **`const s = shopify` used for non-intercept reasons.** We do NOT warn on the + alias declaration. We warn only at an actual `.intercept` access on that alias + (`s.intercept`). A `const s = shopify; s.toast.show()` produces no warning + (covered by the `case-alias-noise` fixture). +- **`function-reference` breadth.** Any non-call use of `shopify.intercept` + warns. This is intentional and low-noise — there is essentially no legitimate + reason to reference `shopify.intercept` without eventually calling it. +- **Residual noise vector:** a developer who aliases the shopify object *and* + legitimately calls `.intercept` with a literal (`const s = shopify; + s.intercept('beforecheckout')`) gets a warning even though the event is + knowable. That's the deliberate trade: the simple detector refuses to resolve + through the alias, so it asks for an explicit declaration instead. The complex + detector resolves it silently. This is exactly the cost/benefit Henry is + weighing. +- **Single-level aliasing.** `const a = shopify; const b = a; b.intercept()` is + not recognized as an alias chain, so `b.intercept` would warn as a + `function-reference`/miss depending on shape — still a warning, never a silent + miss, so the safety property holds. --- diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts index b7bb9ae204f..541fac0bcf1 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts @@ -1,5 +1,5 @@ import {detectPosIntercepts} from './pos_intercept_detection.js' -import {detectPosInterceptsSimple} from './pos_intercept_detection_simple.js' +import {detectPosInterceptsSimple, InterceptWarningKind} from './pos_intercept_detection_simple.js' import {joinPath} from '@shopify/cli-kit/node/path' import {fileURLToPath} from 'node:url' import {dirname} from 'node:path' @@ -10,80 +10,99 @@ const fixtures = joinPath(currentDir, 'fixtures', 'pos-intercept-compare') const fixture = (name: string) => joinPath(fixtures, name) // --------------------------------------------------------------------------- -// COMPARISON MATRIX: complex (full) vs simple detector, per real-world pattern. +// THREE-WAY COMPARISON MATRIX: complex (full) detector vs "safe simplest". // -// pattern | complex | simple -// ------------------------------------------------|---------|-------- -// shopify.intercept('beforecheckout') | catch | catch -// const {intercept} = shopify; intercept('x') | catch | catch -// const s = shopify; s.intercept('x') | catch | MISS -// let fn; fn = shopify.intercept; fn('x') | catch | MISS -// export const block = shopify.intercept (x-file) | catch | MISS -// if/else both branches | catch | catch -// dynamic arg (variable) | unresolved (both) +// For each pattern we record three things: +// [C] complex RESOLVES the event +// [S] simple RESOLVES the event (direct string-literal calls only) +// [W] simple WARNS instead (kind of warning emitted) // -// Each `expected` below encodes exactly this matrix and the test asserts both -// detectors against it, so the delta is explicit and self-documenting. +// pattern | C resolves | S resolves | S warns +// -------------------------------------------------|------------|------------|-------------------- +// shopify.intercept('beforecheckout') | yes | yes | — +// const {intercept}=shopify; intercept('x') | yes | no | destructure +// const s=shopify; s.intercept('x') | yes | no | object-alias-access +// let fn; fn=shopify.intercept; fn('x') | yes | no | function-reference +// cross-file export const block=shopify.intercept | yes | no | function-reference +// if/else both branches | yes | yes | — +// dynamic variable arg | unresolved | no | dynamic-arg +// const s=shopify; s.toast.show() (noise) | n/a | no | — (no false positive) +// +// KEY PROPERTY under test: the simple detector has ZERO SILENT MISSES on the +// alias patterns. When it fails to resolve, it WARNS — it never drops. // --------------------------------------------------------------------------- interface Case { name: string file: string - /** Event names the COMPLEX detector should resolve. */ + /** Events the COMPLEX detector should resolve. */ complexEvents: string[] - /** Event names the SIMPLE detector should resolve. */ + /** Events the SIMPLE detector should resolve (direct literals only). */ simpleEvents: string[] - /** Unresolved-arg count expected from BOTH detectors. */ - unresolvedBoth?: number + /** Warning kinds the SIMPLE detector should emit. */ + simpleWarnKinds: InterceptWarningKind[] } const cases: Case[] = [ { - name: 'direct shopify.intercept — BOTH catch', + name: 'direct call — C:resolve S:resolve (no warn)', file: 'case-direct.ts', complexEvents: ['beforecheckout'], simpleEvents: ['beforecheckout'], + simpleWarnKinds: [], }, { - name: 'same-file destructure const {intercept} = shopify — BOTH catch', + name: 'same-file destructure — C:resolve S:WARN(destructure)', file: 'case-destructure.ts', complexEvents: ['destructured'], - simpleEvents: ['destructured'], + simpleEvents: [], + simpleWarnKinds: ['destructure'], }, { - name: 'object-alias const s = shopify; s.intercept(...) — COMPLEX only', + name: 'object-alias s.intercept() — C:resolve S:WARN(object-alias-access)', file: 'case-object-alias.ts', complexEvents: ['objectalias'], simpleEvents: [], + simpleWarnKinds: ['object-alias-access'], }, { - name: 'reassignment let fn; fn = shopify.intercept — COMPLEX only', + name: 'reassignment fn=shopify.intercept — C:resolve S:WARN(function-reference)', file: 'case-reassign.ts', complexEvents: ['reassigned'], simpleEvents: [], + simpleWarnKinds: ['function-reference'], }, { - name: 'cross-file re-exported reference — COMPLEX only', + name: 'cross-file re-exported ref — C:resolve S:WARN(function-reference at creation site)', file: 'case-crossfile.ts', complexEvents: ['crossfile'], simpleEvents: [], + simpleWarnKinds: ['function-reference'], }, { - name: 'if/else both branches — BOTH catch', + name: 'if/else both branches — C:resolve S:resolve (no warn)', file: 'case-branches.ts', complexEvents: ['branchelse', 'branchif'], simpleEvents: ['branchelse', 'branchif'], + simpleWarnKinds: [], }, { - name: 'dynamic variable arg — NEITHER resolves (both report unresolved)', + name: 'dynamic variable arg — C:unresolved S:WARN(dynamic-arg)', file: 'case-dynamic.ts', complexEvents: [], simpleEvents: [], - unresolvedBoth: 1, + simpleWarnKinds: ['dynamic-arg'], + }, + { + name: 'noise: const s=shopify used for non-intercept — S: no resolve, NO false-positive warn', + file: 'case-alias-noise.ts', + complexEvents: [], + simpleEvents: [], + simpleWarnKinds: [], }, ] -describe('complex vs simple POS intercept detector — comparison matrix', () => { +describe('complex vs safe-simplest POS intercept detector — three-way matrix', () => { cases.forEach((testCase) => { test(testCase.name, async () => { const [complex, simple] = await Promise.all([ @@ -93,37 +112,28 @@ describe('complex vs simple POS intercept detector — comparison matrix', () => expect(complex.events).toEqual(testCase.complexEvents) expect(simple.events).toEqual(testCase.simpleEvents) + expect(simple.warnings.map((warning) => warning.kind).sort()).toEqual([...testCase.simpleWarnKinds].sort()) - if (testCase.unresolvedBoth !== undefined) { - expect(complex.unresolved).toHaveLength(testCase.unresolvedBoth) - expect(simple.unresolved).toHaveLength(testCase.unresolvedBoth) - } + // Every warning carries actionable location + raw source. + simple.warnings.forEach((warning) => { + expect(warning.line).toBeGreaterThan(0) + expect(warning.raw.length).toBeGreaterThan(0) + expect(warning.message).toContain('capabilities.intercepts') + }) }) }) - test('summary: which patterns the extra complexity buys coverage for', async () => { - const results = await Promise.all( - cases.map(async (testCase) => { - const [complex, simple] = await Promise.all([ - detectPosIntercepts(fixture(testCase.file)), - detectPosInterceptsSimple(fixture(testCase.file)), - ]) - return {name: testCase.name, complex: complex.events, simple: simple.events} - }), - ) - - // Patterns where complex strictly beats simple (the value of the complexity). - const complexOnly = results.filter( - (row) => row.complex.length > 0 && row.simple.length < row.complex.length, - ) - expect(complexOnly.map((row) => row.name).sort()).toEqual([ - 'cross-file re-exported reference — COMPLEX only', - 'object-alias const s = shopify; s.intercept(...) — COMPLEX only', - 'reassignment let fn; fn = shopify.intercept — COMPLEX only', - ]) - - // Patterns where both agree (complexity adds nothing). - const parity = results.filter((row) => row.complex.join() === row.simple.join()) - expect(parity).toHaveLength(4) + test('KEY PROPERTY: zero silent misses — simple either resolves or warns for every alias pattern', async () => { + const aliasPatterns = cases.filter((testCase) => testCase.simpleWarnKinds.length > 0 || testCase.complexEvents.length > 0) + for (const testCase of aliasPatterns) { + // eslint-disable-next-line no-await-in-loop + const simple = await detectPosInterceptsSimple(fixture(testCase.file)) + const covered = simple.events.length > 0 || simple.warnings.length > 0 + // Skip the parity-only "noise" cases; here we assert intercept-bearing + // patterns are never silently dropped. + if (testCase.complexEvents.length > 0 || testCase.simpleWarnKinds.length > 0) { + expect(covered, `"${testCase.name}" should resolve or warn, never silently miss`).toBe(true) + } + } }) }) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts index f2367c66395..1b8317e5ba4 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts @@ -1,34 +1,34 @@ -import {findInterceptEntryModules, InterceptCallsite, InterceptDetectionResult} from './pos_intercept_detection.js' +import {findInterceptEntryModules, InterceptCallsite} from './pos_intercept_detection.js' import {findAllImportedFiles} from './type-generation.js' import {readFileSync} from '@shopify/cli-kit/node/fs' import {uniq} from '@shopify/cli-kit/common/array' import type ts from 'typescript' // --------------------------------------------------------------------------- -// SPIKE / PROTOTYPE: the DELIBERATELY SIMPLE POS intercept detector. +// SPIKE / PROTOTYPE: the "SAFE SIMPLEST" POS intercept detector. // -// This is a comparison baseline for the full detector in -// pos_intercept_detection.ts. It does ONLY the straightforward thing: +// Comparison baseline for the full detector in pos_intercept_detection.ts. // -// * Match direct `shopify.intercept('')` callsites (string-literal -// first arg) across the same target-scoped import graph. -// * At most, handle SIMPLE same-file destructuring: -// const {intercept} = shopify → intercept('x') -// const {intercept: rename} = shopify → rename('x') -// ...only when that binding is used in the SAME file it was declared in. +// The guiding rule: NEVER silently under-report. A call is either +// (a) DERIVED SILENTLY — a direct `shopify.intercept('')`, or +// (b) FLAGGED LOUDLY — anything that *could* be an intercept but that this +// simple scan can't statically read. // -// It DELIBERATELY DOES NOT DO (so we can measure what the complexity buys): -// * cross-file alias propagation / re-exported references -// * reassignment tracking (`let fn; fn = shopify.intercept`) -// * object-aliasing (`const s = shopify; s.intercept(...)`) -// * function-parameter / higher-order passing +// It does NOT resolve or follow anything indirect. It does NOT track aliases, +// reassignments, cross-file references, or HOF passing. Instead it recognizes +// those patterns SYNTACTICALLY and emits a warning (file:line + raw source) +// telling the developer to declare that intercept explicitly in the TOML. // -// When a call isn't a direct or simple-destructured match, this detector simply -// MISSES it. That is the whole point of the comparison. +// Resolved events come ONLY from direct string-literal `shopify.intercept(...)`. // -// Control-flow handling and unresolved-arg reporting match the full detector: -// every matched callsite counts regardless of branch, and non-string-literal -// first args are surfaced as unresolved (never dropped). +// Warned patterns (detected, never resolved): +// * destructure: const {intercept} = shopify / {intercept: x} +// * function-reference: const f = shopify.intercept ; x = shopify.intercept ; +// wrap(shopify.intercept) (any non-call use of the fn) +// * object-alias-access: const s = shopify; ... s.intercept(...) (call or ref) +// * dynamic-arg: shopify.intercept() +// +// Control flow is ignored (every direct literal call counts, any branch). // --------------------------------------------------------------------------- async function loadTypeScript(): Promise { @@ -43,27 +43,58 @@ function scriptKindFor(ts: typeof import('typescript'), filePath: string): ts.Sc return ts.ScriptKind.JSX } -/** Collect same-file names bound to `shopify.intercept` via simple destructuring. */ -function collectSimpleDestructureAliases(ts: typeof import('typescript'), sourceFile: ts.SourceFile): Set { +export type InterceptWarningKind = 'destructure' | 'function-reference' | 'object-alias-access' | 'dynamic-arg' + +/** An intercept-shaped pattern the simple scan can't statically resolve. */ +export interface InterceptWarning { + kind: InterceptWarningKind + file: string + line: number + column: number + /** Raw source text of the flagged node, for the developer message. */ + raw: string + message: string +} + +export interface SimpleDetectionResult { + /** Events resolved from direct string-literal calls ONLY. */ + events: string[] + /** The direct string-literal callsites that produced `events`. */ + callsites: InterceptCallsite[] + /** Indirect/dynamic patterns flagged for explicit TOML declaration. */ + warnings: InterceptWarning[] + analyzedFiles: string[] +} + +const DECLARE_HINT = 'Declare this intercept explicitly under capabilities.intercepts in the extension TOML.' + +/** + * Collect identifiers directly aliased to the `shopify` global in this file: + * `const s = shopify` and `s = shopify`. Intentionally single-level (no alias + * chains) — keeping the simple detector simple. Used ONLY to scope the + * object-alias-access warning so a bare `const s = shopify` used for unrelated + * reasons never warns; we only flag when `.intercept` is actually accessed on it. + */ +function collectShopifyAliases(ts: typeof import('typescript'), sourceFile: ts.SourceFile): Set { const aliases = new Set() const visit = (node: ts.Node): void => { - // const {intercept} = shopify / const {intercept: rename} = shopify if ( ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && node.initializer && ts.isIdentifier(node.initializer) && - node.initializer.text === 'shopify' && - ts.isObjectBindingPattern(node.name) + node.initializer.text === 'shopify' ) { - for (const element of node.name.elements) { - const propName = - element.propertyName && ts.isIdentifier(element.propertyName) ? element.propertyName.text : undefined - const localName = ts.isIdentifier(element.name) ? element.name.text : undefined - // `{intercept}` (no propertyName) or `{intercept: rename}` (propertyName === 'intercept') - if (localName && (propName === 'intercept' || (!propName && element.name.getText(sourceFile) === 'intercept'))) { - aliases.add(localName) - } - } + aliases.add(node.name.text) + } + if ( + ts.isBinaryExpression(node) && + node.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isIdentifier(node.left) && + ts.isIdentifier(node.right) && + node.right.text === 'shopify' + ) { + aliases.add(node.left.text) } ts.forEachChild(node, visit) } @@ -71,61 +102,116 @@ function collectSimpleDestructureAliases(ts: typeof import('typescript'), source return aliases } -function analyzeFile(ts: typeof import('typescript'), filePath: string): InterceptCallsite[] { +function analyzeFileSimple( + ts: typeof import('typescript'), + filePath: string, +): {callsites: InterceptCallsite[]; warnings: InterceptWarning[]} { let content: string try { content = readFileSync(filePath).toString() // eslint-disable-next-line no-catch-all/no-catch-all } catch { - return [] + return {callsites: [], warnings: []} } const sourceFile = ts.createSourceFile(filePath, content, ts.ScriptTarget.Latest, true, scriptKindFor(ts, filePath)) - const destructureAliases = collectSimpleDestructureAliases(ts, sourceFile) + const aliases = collectShopifyAliases(ts, sourceFile) const callsites: InterceptCallsite[] = [] + const warnings: InterceptWarning[] = [] + + const posOf = (node: ts.Node) => { + const {line, character} = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) + return {file: filePath, line: line + 1, column: character + 1} + } + const truncate = (text: string) => (text.length > 80 ? `${text.slice(0, 77)}...` : text).replace(/\s+/g, ' ') const visit = (node: ts.Node): void => { - if (ts.isCallExpression(node)) { - const callee = node.expression - // Direct: shopify.intercept(...) - const isDirect = - ts.isPropertyAccessExpression(callee) && - ts.isIdentifier(callee.expression) && - callee.expression.text === 'shopify' && - callee.name.text === 'intercept' - // Simple same-file destructured alias: intercept(...) / rename(...) - const isSimpleAlias = ts.isIdentifier(callee) && destructureAliases.has(callee.text) - - if (isDirect || isSimpleAlias) { - const firstArg = node.arguments[0] - const {line, character} = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) - const base = {file: filePath, line: line + 1, column: character + 1} - if (!firstArg) { - callsites.push({...base, event: null, argText: '', unresolvedReason: 'intercept() called with no event argument'}) - } else if (ts.isStringLiteralLike(firstArg)) { - callsites.push({...base, event: firstArg.text, argText: firstArg.getText(sourceFile)}) + // 1. Destructure off shopify (or a shopify alias): const {intercept} = shopify + if ( + ts.isVariableDeclaration(node) && + ts.isObjectBindingPattern(node.name) && + node.initializer && + ts.isIdentifier(node.initializer) && + (node.initializer.text === 'shopify' || aliases.has(node.initializer.text)) + ) { + for (const element of node.name.elements) { + const propName = + element.propertyName && ts.isIdentifier(element.propertyName) + ? element.propertyName.text + : ts.isIdentifier(element.name) + ? element.name.text + : undefined + if (propName === 'intercept') { + warnings.push({ + kind: 'destructure', + ...posOf(node), + raw: truncate(node.getText(sourceFile)), + message: `intercept is destructured off shopify; the simple detector can't read the event name(s). ${DECLARE_HINT}`, + }) + } + } + } + + // 2/3/4. Any `.intercept` where obj is `shopify` or a shopify alias. + if ( + ts.isPropertyAccessExpression(node) && + node.name.text === 'intercept' && + ts.isIdentifier(node.expression) + ) { + const objName = node.expression.text + const isShopify = objName === 'shopify' + const isAlias = aliases.has(objName) + if (isShopify || isAlias) { + const parent = node.parent + const isDirectCallee = parent && ts.isCallExpression(parent) && parent.expression === node + + if (isShopify && isDirectCallee) { + // Direct `shopify.intercept(...)` — resolve literal, else dynamic-arg warn. + const call = parent as ts.CallExpression + const firstArg = call.arguments[0] + if (firstArg && ts.isStringLiteralLike(firstArg)) { + callsites.push({...posOf(call), event: firstArg.text, argText: firstArg.getText(sourceFile)}) + } else { + warnings.push({ + kind: 'dynamic-arg', + ...posOf(call), + raw: truncate(call.getText(sourceFile)), + message: `shopify.intercept called with a non-string-literal event argument. ${DECLARE_HINT}`, + }) + } + } else if (isShopify) { + // Function reference: const f = shopify.intercept / x = ... / HOF arg. + warnings.push({ + kind: 'function-reference', + ...posOf(node), + raw: truncate((node.parent ?? node).getText(sourceFile)), + message: `A reference to shopify.intercept is taken; the simple detector can't follow it. ${DECLARE_HINT}`, + }) } else { - callsites.push({ - ...base, - event: null, - argText: firstArg.getText(sourceFile), - unresolvedReason: `first argument is a ${ts.SyntaxKind[firstArg.kind]}, not a string literal`, + // Access on a shopify-object alias: s.intercept(...) or ref to s.intercept. + warnings.push({ + kind: 'object-alias-access', + ...posOf(node), + raw: truncate((node.parent ?? node).getText(sourceFile)), + message: `intercept is accessed via an alias of the shopify object; the simple detector can't read the event. ${DECLARE_HINT}`, }) } } } + ts.forEachChild(node, visit) } visit(sourceFile) - return callsites + return {callsites, warnings} } /** - * Simple detector: direct + same-file-destructured intercept calls only, over - * the target-scoped import graph. + * Safe-simplest detector: resolves ONLY direct string-literal + * `shopify.intercept('x')` calls; flags every other intercept-shaped pattern as + * a warning rather than silently missing it. Target-scoped import graph. */ export async function detectPosInterceptsSimple( entryFilePaths: string | string[], -): Promise { +): Promise { const ts = await loadTypeScript() const entries = uniq((Array.isArray(entryFilePaths) ? entryFilePaths : [entryFilePaths]).filter(Boolean)) @@ -134,13 +220,15 @@ export async function detectPosInterceptsSimple( const allFiles = uniq([...entries, ...importedByEntry.flat()]) const callsites: InterceptCallsite[] = [] + const warnings: InterceptWarning[] = [] for (const filePath of allFiles) { - callsites.push(...analyzeFile(ts, filePath)) + const {callsites: fileCallsites, warnings: fileWarnings} = analyzeFileSimple(ts, filePath) + callsites.push(...fileCallsites) + warnings.push(...fileWarnings) } - const events = uniq(callsites.filter((cs) => cs.event !== null).map((cs) => cs.event as string)).sort() - const unresolved = callsites.filter((cs) => cs.event === null) - return {events, callsites, unresolved, analyzedFiles: allFiles} + const events = uniq(callsites.map((cs) => cs.event as string)).sort() + return {events, callsites, warnings, analyzedFiles: allFiles} } /** Deploy-path parity with the full detector: config-driven, target-scoped. */ @@ -148,7 +236,7 @@ export async function deriveInterceptsFromConfigSimple( // eslint-disable-next-line @typescript-eslint/no-explicit-any config: {targeting?: any; extension_points?: any} | undefined, directory: string, -): Promise { +): Promise { const entryModules = findInterceptEntryModules(config, directory) if (entryModules.length === 0) return undefined return detectPosInterceptsSimple(entryModules) From d2839726b25b7232d6317808e2258d58735a438d Mon Sep 17 00:00:00 2001 From: Henry Stelle Date: Fri, 10 Jul 2026 12:07:09 -0700 Subject: [PATCH 4/6] SPIKE: match real 2-arg intercept('', callback) API in both detectors Event is the first arg (string literal resolves; non-literal = dynamic-arg unresolved/warn). A genuine registration requires a callback second arg (arrow fn, function expression, or a function reference). Edge cases: - literal event with NO callback -> surfaced as suspected MALFORMED (complex: unresolved; simple: missing-callback warning), never silently counted. - extra trailing args tolerated (only args[0]/args[1] inspected). Alias warnings (simple) and alias resolution (complex) unchanged; the callback check applies at the resolved callsite. Existing fixtures already used the 2-arg shape; added case-missing-callback and case-trailing-args fixtures and extended the three-way matrix (now includes missing-callback + trailing-args). Assisted-By: devx/76608ac5-410d-425f-93b9-9eea3cc49f3f --- .../case-missing-callback.ts | 5 ++ .../case-trailing-args.ts | 4 ++ .../pos_intercept_detection.DESIGN.md | 30 ++++++++++-- .../specifications/pos_intercept_detection.ts | 47 +++++++++++++++++-- ...pos_intercept_detection_comparison.test.ts | 41 ++++++++++++++-- .../pos_intercept_detection_simple.ts | 45 ++++++++++++++++-- 6 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-missing-callback.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-trailing-args.ts diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-missing-callback.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-missing-callback.ts new file mode 100644 index 00000000000..51a356e23f3 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-missing-callback.ts @@ -0,0 +1,5 @@ +// Literal event but NO callback second arg. The real API requires a callback, +// so this is a suspected MALFORMED registration: surfaced (complex: unresolved, +// simple: missing-callback warning), never silently counted as an event. +declare const shopify: {intercept: (event: string, handler?: () => void) => void} +shopify.intercept('missingcb') diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-trailing-args.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-trailing-args.ts new file mode 100644 index 00000000000..067520fcb29 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-trailing-args.ts @@ -0,0 +1,4 @@ +// Extra trailing args beyond ('event', callback) are tolerated — the event +// still resolves from the first arg with a callback present. +declare const shopify: {intercept: (event: string, handler: () => void, ...rest: unknown[]) => void} +shopify.intercept('trailing', () => {}, 'extra') diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md index 6c7c4086c12..132b486e20b 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.DESIGN.md @@ -84,22 +84,42 @@ LOUDLY (a warning with file:line + raw source telling the developer to declare that intercept in the TOML). It resolves nothing indirect and follows nothing cross-file — it recognizes intercept-shaped patterns *syntactically* and warns. +All calls use the real API shape `shopify.intercept('', callback)`. + Three-way matrix (`pos_intercept_detection_comparison.test.ts`): | pattern | complex resolves | simple resolves | simple warns | |---------|:-------:|:-------:|:-----| -| `shopify.intercept('beforecheckout')` | yes | yes | — | -| `const {intercept} = shopify; intercept('x')` | yes | no | `destructure` | -| `const s = shopify; s.intercept('x')` | yes | no | `object-alias-access` | -| `let fn; fn = shopify.intercept; fn('x')` | yes | no | `function-reference` | +| `shopify.intercept('beforecheckout', cb)` | yes | yes | — | +| `const {intercept} = shopify; intercept('x', cb)` | yes | no | `destructure` | +| `const s = shopify; s.intercept('x', cb)` | yes | no | `object-alias-access` | +| `let fn; fn = shopify.intercept; fn('x', cb)` | yes | no | `function-reference` | | cross-file `export const block = shopify.intercept` | yes | no | `function-reference` (at creation site) | | `if/else` both branches | yes | yes | — | -| dynamic variable arg | unresolved | no | `dynamic-arg` | +| dynamic variable first arg | unresolved | no | `dynamic-arg` | +| literal event, NO callback (malformed) | unresolved | no | `missing-callback` | +| `('event', cb, extraArg)` trailing args | yes | yes | — | | `const s = shopify; s.toast.show()` (noise) | n/a | no | **— (no false positive)** | **Key property:** the simple detector has ZERO silent misses on the alias patterns — where it can't resolve, it warns. +### Callback-presence handling (2-arg API `intercept('', callback)`) + +The event is the FIRST arg; a genuine registration has a callback SECOND arg. +Both detectors accept as a callback: an arrow function, a function expression, +or a function reference (identifier or property access). Handling: + +- **Literal event + callback** → resolved event (counted). This is the only path + that counts an event. +- **Literal event, NO/invalid callback** → surfaced but NOT counted: a suspected + MALFORMED registration. Complex reports it in `unresolved`; simple emits a + `missing-callback` warning. (Henry's lean: surface, don't silently count.) +- **Non-literal first arg** → `dynamic-arg` unresolved/warn as before (callback + presence is moot — the event name is unknowable). +- **Extra trailing args** beyond `(event, callback)` → tolerated; only `args[0]` + and `args[1]` are inspected. + ### Is warn-on-alias simpler or more complex than full resolution? **Honest assessment: simpler, and meaningfully so.** Code-ish (non-comment) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts index 39af743abe0..064f24f114b 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection.ts @@ -33,9 +33,14 @@ import type ts from 'typescript' // const s = shopify; s.intercept('...') // let fn; fn = shopify.intercept (reassignment) // export const intercept = shopify.intercept (re-export, cross-file) -// * Only STRING-LITERAL first args are statically resolvable. Anything else -// (variables, member expressions, template strings with substitutions) is -// surfaced as UNRESOLVED and never silently dropped. +// * The API is `shopify.intercept('', callback)`: the event is the +// FIRST arg and a callback is the SECOND. Only STRING-LITERAL first args are +// statically resolvable; anything else (variables, member expressions, +// template strings with substitutions) is surfaced as UNRESOLVED, never +// dropped. A resolved event is counted ONLY when a callback second arg is +// present; a literal-event call missing its callback is surfaced as a +// suspected MALFORMED registration (not counted). Extra trailing args are +// tolerated. // --------------------------------------------------------------------------- async function loadTypeScript(): Promise { @@ -274,6 +279,28 @@ function collectImportsAndExports(ts: typeof import('typescript'), analysis: Fil visit(analysis.sourceFile) } +/** + * The real API is `shopify.intercept('', callback)`. A genuine intercept + * registration has a SECOND argument that is a function/callback. We accept: + * - arrow function shopify.intercept('x', () => {}) + * - function expression shopify.intercept('x', function () {}) + * - a reference to a function (identifier or property access) + * shopify.intercept('x', handler) / handlers.onX + * Anything else (missing, or a non-function literal like a string/number/object) + * is treated as a missing/invalid callback — a suspected malformed registration. + * Extra trailing args are tolerated (we only inspect args[0] and args[1]). + */ +function hasCallbackArg(ts: typeof import('typescript'), call: ts.CallExpression): boolean { + const second = call.arguments[1] + if (!second) return false + return ( + ts.isArrowFunction(second) || + ts.isFunctionExpression(second) || + ts.isIdentifier(second) || + ts.isPropertyAccessExpression(second) + ) +} + /** Final pass: collect every intercept callsite using the resolved alias sets. */ function collectCallsites(ts: typeof import('typescript'), analysis: FileAnalysis): InterceptCallsite[] { const callsites: InterceptCallsite[] = [] @@ -288,6 +315,7 @@ function collectCallsites(ts: typeof import('typescript'), analysis: FileAnalysi if (isInterceptCall) { const firstArg = node.arguments[0] + const callbackPresent = hasCallbackArg(ts, node) const {line, character} = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) const base = {file: analysis.path, line: line + 1, column: character + 1} @@ -300,7 +328,18 @@ function collectCallsites(ts: typeof import('typescript'), analysis: FileAnalysi }) } else if (ts.isStringLiteralLike(firstArg)) { // string literal or no-substitution template literal - callsites.push({...base, event: firstArg.text, argText: firstArg.getText(sourceFile)}) + if (callbackPresent) { + callsites.push({...base, event: firstArg.text, argText: firstArg.getText(sourceFile)}) + } else { + // Literal event but no callback — surface it, but do NOT count it as a + // resolved event: it's a suspected malformed intercept registration. + callsites.push({ + ...base, + event: null, + argText: firstArg.getText(sourceFile), + unresolvedReason: `intercept('${firstArg.text}', ...) is missing its callback argument (suspected malformed registration)`, + }) + } } else { callsites.push({ ...base, diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts index 541fac0bcf1..2df385127a9 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts @@ -17,15 +17,19 @@ const fixture = (name: string) => joinPath(fixtures, name) // [S] simple RESOLVES the event (direct string-literal calls only) // [W] simple WARNS instead (kind of warning emitted) // +// All calls use the real API shape: shopify.intercept('', callback). +// // pattern | C resolves | S resolves | S warns // -------------------------------------------------|------------|------------|-------------------- -// shopify.intercept('beforecheckout') | yes | yes | — -// const {intercept}=shopify; intercept('x') | yes | no | destructure -// const s=shopify; s.intercept('x') | yes | no | object-alias-access -// let fn; fn=shopify.intercept; fn('x') | yes | no | function-reference +// shopify.intercept('beforecheckout', cb) | yes | yes | — +// const {intercept}=shopify; intercept('x', cb) | yes | no | destructure +// const s=shopify; s.intercept('x', cb) | yes | no | object-alias-access +// let fn; fn=shopify.intercept; fn('x', cb) | yes | no | function-reference // cross-file export const block=shopify.intercept | yes | no | function-reference // if/else both branches | yes | yes | — -// dynamic variable arg | unresolved | no | dynamic-arg +// dynamic variable first arg | unresolved | no | dynamic-arg +// literal event, NO callback (malformed) | unresolved | no | missing-callback +// ('event', cb, extraArg) trailing args tolerated | yes | yes | — // const s=shopify; s.toast.show() (noise) | n/a | no | — (no false positive) // // KEY PROPERTY under test: the simple detector has ZERO SILENT MISSES on the @@ -93,6 +97,20 @@ const cases: Case[] = [ simpleEvents: [], simpleWarnKinds: ['dynamic-arg'], }, + { + name: 'literal event but NO callback (malformed) — C:unresolved S:WARN(missing-callback)', + file: 'case-missing-callback.ts', + complexEvents: [], + simpleEvents: [], + simpleWarnKinds: ['missing-callback'], + }, + { + name: 'trailing args tolerated — C:resolve S:resolve (no warn)', + file: 'case-trailing-args.ts', + complexEvents: ['trailing'], + simpleEvents: ['trailing'], + simpleWarnKinds: [], + }, { name: 'noise: const s=shopify used for non-intercept — S: no resolve, NO false-positive warn', file: 'case-alias-noise.ts', @@ -123,6 +141,19 @@ describe('complex vs safe-simplest POS intercept detector — three-way matrix', }) }) + test('malformed (literal event, no callback): BOTH surface it, NEITHER counts it as an event', async () => { + const [complex, simple] = await Promise.all([ + detectPosIntercepts(fixture('case-missing-callback.ts')), + detectPosInterceptsSimple(fixture('case-missing-callback.ts')), + ]) + // Not counted as a real event by either detector. + expect(complex.events).toEqual([]) + expect(simple.events).toEqual([]) + // But surfaced by both — complex as an unresolved callsite, simple as a warning. + expect(complex.unresolved.some((cs) => /missing its callback/.test(cs.unresolvedReason ?? ''))).toBe(true) + expect(simple.warnings.map((warning) => warning.kind)).toEqual(['missing-callback']) + }) + test('KEY PROPERTY: zero silent misses — simple either resolves or warns for every alias pattern', async () => { const aliasPatterns = cases.filter((testCase) => testCase.simpleWarnKinds.length > 0 || testCase.complexEvents.length > 0) for (const testCase of aliasPatterns) { diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts index 1b8317e5ba4..e6217df8648 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts @@ -26,7 +26,13 @@ import type ts from 'typescript' // * function-reference: const f = shopify.intercept ; x = shopify.intercept ; // wrap(shopify.intercept) (any non-call use of the fn) // * object-alias-access: const s = shopify; ... s.intercept(...) (call or ref) -// * dynamic-arg: shopify.intercept() +// * dynamic-arg: shopify.intercept(, cb) +// * missing-callback: shopify.intercept('literal') with no callback +// +// The real API is `shopify.intercept('', callback)`. A direct literal +// call is DERIVED only when a callback second arg is present; a literal call +// with no callback is flagged (missing-callback) as a suspected malformed +// registration rather than counted. Extra trailing args are tolerated. // // Control flow is ignored (every direct literal call counts, any branch). // --------------------------------------------------------------------------- @@ -43,7 +49,28 @@ function scriptKindFor(ts: typeof import('typescript'), filePath: string): ts.Sc return ts.ScriptKind.JSX } -export type InterceptWarningKind = 'destructure' | 'function-reference' | 'object-alias-access' | 'dynamic-arg' +export type InterceptWarningKind = + | 'destructure' + | 'function-reference' + | 'object-alias-access' + | 'dynamic-arg' + | 'missing-callback' + +/** + * A genuine intercept registration has a callback as its SECOND argument. + * Accepts arrow fn, function expression, or a function reference (identifier / + * property access). See the full detector for the shared rationale. + */ +function hasCallbackArg(ts: typeof import('typescript'), call: ts.CallExpression): boolean { + const second = call.arguments[1] + if (!second) return false + return ( + ts.isArrowFunction(second) || + ts.isFunctionExpression(second) || + ts.isIdentifier(second) || + ts.isPropertyAccessExpression(second) + ) +} /** An intercept-shaped pattern the simple scan can't statically resolve. */ export interface InterceptWarning { @@ -165,11 +192,21 @@ function analyzeFileSimple( const isDirectCallee = parent && ts.isCallExpression(parent) && parent.expression === node if (isShopify && isDirectCallee) { - // Direct `shopify.intercept(...)` — resolve literal, else dynamic-arg warn. + // Direct `shopify.intercept('', callback)`. const call = parent as ts.CallExpression const firstArg = call.arguments[0] + const callbackPresent = hasCallbackArg(ts, call) if (firstArg && ts.isStringLiteralLike(firstArg)) { - callsites.push({...posOf(call), event: firstArg.text, argText: firstArg.getText(sourceFile)}) + if (callbackPresent) { + callsites.push({...posOf(call), event: firstArg.text, argText: firstArg.getText(sourceFile)}) + } else { + warnings.push({ + kind: 'missing-callback', + ...posOf(call), + raw: truncate(call.getText(sourceFile)), + message: `shopify.intercept('${firstArg.text}') is missing its callback argument (suspected malformed registration). ${DECLARE_HINT}`, + }) + } } else { warnings.push({ kind: 'dynamic-arg', From c6c4fa403f690e821d79f5a1525f2230dc18ec09 Mon Sep 17 00:00:00 2001 From: Henry Stelle Date: Fri, 10 Jul 2026 12:23:57 -0700 Subject: [PATCH 5/6] SPIKE: add runnable side-by-side detector report + close simple alias-chain gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Report (pos_intercept_detection_report.test.ts) prints, per curated sample: verbatim source, simple detector's events+warnings, complex detector's events+unresolved, and a one-line verdict. Curated to tell the story, including two cases where the COMPLEX detector ALSO FAILS with a silent miss (HOF register(shopify.intercept) and object-storage const m={i:shopify.intercept}) plus const-fold (complex unresolved). Optional REPORT_PATH env to run on a real file. While building the report it surfaced that the simple detector silently missed same-file alias CHAINS (const a=shopify; const b=a; b.intercept(...)) — a violation of its 'never silently miss' guarantee. Closed it with a cheap same-file identifier-to-identifier fixpoint in collectShopifyAliases (still no cross-file data-flow). Added case-alias-chain fixture + matrix case. Assisted-By: devx/76608ac5-410d-425f-93b9-9eea3cc49f3f --- .../pos-intercept-compare/case-alias-chain.ts | 6 + ...pos_intercept_detection_comparison.test.ts | 7 + .../pos_intercept_detection_report.test.ts | 191 ++++++++++++++++++ .../pos_intercept_detection_simple.ts | 64 +++--- 4 files changed, 243 insertions(+), 25 deletions(-) create mode 100644 packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-chain.ts create mode 100644 packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts diff --git a/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-chain.ts b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-chain.ts new file mode 100644 index 00000000000..b3d0be65800 --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/fixtures/pos-intercept-compare/case-alias-chain.ts @@ -0,0 +1,6 @@ +// Alias CHAIN: const a = shopify; const b = a. Complex resolves through the +// chain; simple can't read the event but WARNS (never a silent miss). +declare const shopify: {intercept: (event: string, handler: () => void) => void} +const a = shopify +const b = a +b.intercept('aliaschain', () => {}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts index 2df385127a9..15db8a2c8c5 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_comparison.test.ts @@ -83,6 +83,13 @@ const cases: Case[] = [ simpleEvents: [], simpleWarnKinds: ['function-reference'], }, + { + name: 'alias chain const a=shopify; const b=a — C:resolve S:WARN(object-alias-access, chain closed)', + file: 'case-alias-chain.ts', + complexEvents: ['aliaschain'], + simpleEvents: [], + simpleWarnKinds: ['object-alias-access'], + }, { name: 'if/else both branches — C:resolve S:resolve (no warn)', file: 'case-branches.ts', diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts new file mode 100644 index 00000000000..e79d4f9da9b --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts @@ -0,0 +1,191 @@ +import {detectPosIntercepts} from './pos_intercept_detection.js' +import {detectPosInterceptsSimple} from './pos_intercept_detection_simple.js' +import {mkdtempSync, mkdirSync, writeFileSync} from 'node:fs' +import {tmpdir} from 'node:os' +import {join, dirname} from 'node:path' +import {test} from 'vitest' + +// --------------------------------------------------------------------------- +// RUNNABLE REPORT (not an assertion suite — it prints and exits 0). +// +// Shows the SIMPLE vs COMPLEX POS intercept detectors side-by-side over a +// curated set of samples that tell the story, including cases where the COMPLEX +// detector ALSO fails (its shallow data-flow has real limits). +// +// RUN: +// cd packages/app && ../../node_modules/.bin/vitest run \ +// src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts +// +// Optional: run against a real file instead of the built-in samples: +// REPORT_PATH=/abs/path/to/entry.ts ../../node_modules/.bin/vitest run \ +// src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts +// --------------------------------------------------------------------------- + +interface Sample { + name: string + label: string + /** entry filename within the sample dir. */ + entry: string + /** filename -> source content (verbatim, printed in the report). */ + files: {[filename: string]: string} +} + +const CB = '() => {}' + +const SAMPLES: Sample[] = [ + { + name: 'plain direct call', + label: 'BOTH RESOLVE', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nshopify.intercept('beforecheckout', ${CB})\n`, + }, + }, + { + name: 'same-file destructure', + label: 'simple WARNS, complex RESOLVES', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst {intercept} = shopify\nintercept('beforediscount', ${CB})\n`, + }, + }, + { + name: 'object alias then member call', + label: 'simple WARNS, complex RESOLVES', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst s = shopify\ns.intercept('beforeexchange', ${CB})\n`, + }, + }, + { + name: 'cross-file re-exported reference', + label: 'simple WARNS, complex RESOLVES', + entry: 'index.ts', + files: { + 'dep.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nexport const block = shopify.intercept\n`, + 'index.ts': `import {block} from './dep.js'\nblock('beforecancel', ${CB})\n`, + }, + }, + { + name: 'alias chain const a = shopify; const b = a', + label: 'simple WARNS (chain closed), complex RESOLVES', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst a = shopify\nconst b = a\nb.intercept('beforecapture', ${CB})\n`, + }, + }, + { + name: 'dynamic event argument (variable)', + label: 'BOTH UNRESOLVED (dynamic) — simple WARNS', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst evt = 'beforetax'\nshopify.intercept(evt, ${CB})\n`, + }, + }, + { + name: 'const-folded event name const E = "..."; intercept(E, cb)', + label: 'COMPLEX ALSO FAILS (no constant folding) — simple WARNS', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst E = 'beforeshipping'\nshopify.intercept(E, ${CB})\n`, + }, + }, + { + name: 'higher-order passing register(shopify.intercept)', + label: 'COMPLEX ALSO FAILS (silent miss on HOF) — simple WARNS', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nfunction register(fn: typeof shopify.intercept) {\n fn('beforepayment', ${CB})\n}\nregister(shopify.intercept)\n`, + }, + }, + { + name: 'stored in object then called const m = { i: shopify.intercept }', + label: 'COMPLEX ALSO FAILS (silent miss via object storage) — simple WARNS', + entry: 'index.ts', + files: { + 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst m = {i: shopify.intercept}\nm.i('beforerefund', ${CB})\n`, + }, + }, +] + +function writeSample(sample: Sample): string { + const dir = mkdtempSync(join(tmpdir(), 'pos-intercept-report-')) + for (const [filename, content] of Object.entries(sample.files)) { + const filePath = join(dir, filename) + mkdirSync(dirname(filePath), {recursive: true}) + writeFileSync(filePath, content) + } + return join(dir, sample.entry) +} + +const list = (items: string[]) => (items.length ? `[${items.join(', ')}]` : '[]') + +function indent(text: string): string { + return text + .replace(/\n+$/, '') + .split('\n') + .map((line) => ` ${line}`) + .join('\n') +} + +async function reportOne(sourceLabel: string, entryPath: string, printedSource?: string): Promise { + const [simple, complex] = await Promise.all([detectPosInterceptsSimple(entryPath), detectPosIntercepts(entryPath)]) + + const simpleWarns = simple.warnings.map((warning) => `${warning.kind}: ${warning.message.split('.')[0]}`) + const complexUnresolved = complex.unresolved.map((entry) => entry.unresolvedReason ?? entry.argText) + + // Verdict. + const simpleStr = simple.events.length + ? `resolved ${list(simple.events)}` + : simpleWarns.length + ? `WARN(${simple.warnings.map((warning) => warning.kind).join(', ')})` + : 'nothing' + const complexStr = complex.events.length + ? `resolved ${list(complex.events)}` + : complexUnresolved.length + ? `unresolved(${complexUnresolved.length})` + : 'NOTHING — silent miss' + const complexFailed = complex.events.length === 0 + const verdict = `${complexFailed && complexUnresolved.length === 0 ? '⚠ COMPLEX ALSO FAILS — ' : ''}simple ${simpleStr} | complex ${complexStr}` + + const lines: string[] = [] + lines.push('═'.repeat(72)) + lines.push(sourceLabel) + lines.push('─'.repeat(72)) + if (printedSource !== undefined) { + lines.push('SOURCE:') + lines.push(indent(printedSource)) + lines.push('') + } + lines.push(`SIMPLE events=${list(simple.events)}`) + if (simpleWarns.length) simpleWarns.forEach((warning) => lines.push(` warn ${warning}`)) + lines.push(`COMPLEX events=${list(complex.events)}`) + if (complexUnresolved.length) complexUnresolved.forEach((reason) => lines.push(` unresolved ${reason}`)) + lines.push(`VERDICT ${verdict}`) + // eslint-disable-next-line no-console + console.log(lines.join('\n')) +} + +test('POS intercept detector report (simple vs complex)', async () => { + const overridePath = process.env.REPORT_PATH + // eslint-disable-next-line no-console + console.log(`\n\nPOS INTERCEPT DETECTOR REPORT — simple (safe-simplest) vs complex (alias-resolving)\n`) + + if (overridePath) { + // eslint-disable-next-line no-console + console.log(`Running against REPORT_PATH=${overridePath}\n`) + await reportOne(`FILE: ${overridePath}`, overridePath) + } else { + for (let index = 0; index < SAMPLES.length; index++) { + const sample = SAMPLES[index]! + const entryPath = writeSample(sample) + const combinedSource = Object.entries(sample.files) + .map(([filename, content]) => `// ── ${filename} ──\n${content}`) + .join('\n') + // eslint-disable-next-line no-await-in-loop + await reportOne(`SAMPLE ${index + 1}: ${sample.name} [${sample.label}]`, entryPath, combinedSource) + } + } + // eslint-disable-next-line no-console + console.log(`\n${'═'.repeat(72)}\nLEGEND: "silent miss" = a real intercept('event', cb) call the detector\nreturned NOTHING for (not even unresolved). The safe-simplest detector never\nsilently misses — it warns and tells the developer to declare it in TOML.\n`) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts index e6217df8648..fd987b52db5 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_simple.ts @@ -96,36 +96,50 @@ export interface SimpleDetectionResult { const DECLARE_HINT = 'Declare this intercept explicitly under capabilities.intercepts in the extension TOML.' /** - * Collect identifiers directly aliased to the `shopify` global in this file: - * `const s = shopify` and `s = shopify`. Intentionally single-level (no alias - * chains) — keeping the simple detector simple. Used ONLY to scope the - * object-alias-access warning so a bare `const s = shopify` used for unrelated - * reasons never warns; we only flag when `.intercept` is actually accessed on it. + * Collect identifiers aliased to the `shopify` global WITHIN this file: + * `const s = shopify` / `s = shopify`, plus same-file alias CHAINS + * (`const a = shopify; const b = a`). This is a cheap same-file fixpoint over + * identifier-to-identifier assignments — NOT the complex detector's whole-graph, + * cross-file data-flow. It exists so the "never silently miss" guarantee holds: + * without chain handling, `b.intercept(...)` would be neither resolved nor + * warned. Aliases are used ONLY to scope the object-alias-access warning — a + * bare `const s = shopify` used for unrelated reasons still never warns; we flag + * only when `.intercept` is actually accessed on the alias. */ function collectShopifyAliases(ts: typeof import('typescript'), sourceFile: ts.SourceFile): Set { const aliases = new Set() - const visit = (node: ts.Node): void => { - if ( - ts.isVariableDeclaration(node) && - ts.isIdentifier(node.name) && - node.initializer && - ts.isIdentifier(node.initializer) && - node.initializer.text === 'shopify' - ) { - aliases.add(node.name.text) - } - if ( - ts.isBinaryExpression(node) && - node.operatorToken.kind === ts.SyntaxKind.EqualsToken && - ts.isIdentifier(node.left) && - ts.isIdentifier(node.right) && - node.right.text === 'shopify' - ) { - aliases.add(node.left.text) + + // One syntactic pass, applied to a fixpoint so chains resolve regardless of + // declaration order. + const pass = (): boolean => { + const before = aliases.size + const visit = (node: ts.Node): void => { + if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.initializer && + ts.isIdentifier(node.initializer) && + (node.initializer.text === 'shopify' || aliases.has(node.initializer.text)) + ) { + aliases.add(node.name.text) + } + if ( + ts.isBinaryExpression(node) && + node.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isIdentifier(node.left) && + ts.isIdentifier(node.right) && + (node.right.text === 'shopify' || aliases.has(node.right.text)) + ) { + aliases.add(node.left.text) + } + ts.forEachChild(node, visit) } - ts.forEachChild(node, visit) + visit(sourceFile) + return aliases.size > before + } + while (pass()) { + // repeat until no new aliases are discovered } - visit(sourceFile) return aliases } From 64c52afe0935869adbb14db7cbd10583c347632a Mon Sep 17 00:00:00 2001 From: Henry Stelle Date: Fri, 10 Jul 2026 12:34:27 -0700 Subject: [PATCH 6/6] SPIKE: tighten detector report to symbol grid + tally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each sample now shows only: short description, minimal code, and two result cells (SIMPLE / COMPLEX) each rendered as one symbol — ✅ resolved, 🟠 surfaced but not resolved (simple warning / complex unresolved), ❌ silent miss. Dropped the verbose verdict/events/warn-detail lines. Added a bottom tally counting ✅/🟠/❌ per detector: SIMPLE ✅1 🟠8 ❌0 vs COMPLEX ✅5 🟠2 ❌2 — makes it obvious simple never silently misses. Assisted-By: devx/76608ac5-410d-425f-93b9-9eea3cc49f3f --- .../pos_intercept_detection_report.test.ts | 105 ++++++++++-------- 1 file changed, 60 insertions(+), 45 deletions(-) diff --git a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts index e79d4f9da9b..e858afa00de 100644 --- a/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts @@ -24,9 +24,11 @@ import {test} from 'vitest' interface Sample { name: string label: string + /** Minimal code shown in the report (just enough to understand the sample). */ + show: string /** entry filename within the sample dir. */ entry: string - /** filename -> source content (verbatim, printed in the report). */ + /** filename -> source content (verbatim) actually written + analyzed. */ files: {[filename: string]: string} } @@ -36,6 +38,7 @@ const SAMPLES: Sample[] = [ { name: 'plain direct call', label: 'BOTH RESOLVE', + show: `shopify.intercept('beforecheckout', cb)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nshopify.intercept('beforecheckout', ${CB})\n`, @@ -44,6 +47,7 @@ const SAMPLES: Sample[] = [ { name: 'same-file destructure', label: 'simple WARNS, complex RESOLVES', + show: `const {intercept} = shopify\nintercept('beforediscount', cb)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst {intercept} = shopify\nintercept('beforediscount', ${CB})\n`, @@ -52,6 +56,7 @@ const SAMPLES: Sample[] = [ { name: 'object alias then member call', label: 'simple WARNS, complex RESOLVES', + show: `const s = shopify\ns.intercept('beforeexchange', cb)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst s = shopify\ns.intercept('beforeexchange', ${CB})\n`, @@ -60,6 +65,7 @@ const SAMPLES: Sample[] = [ { name: 'cross-file re-exported reference', label: 'simple WARNS, complex RESOLVES', + show: `// dep.ts\nexport const block = shopify.intercept\n// index.ts\nblock('beforecancel', cb)`, entry: 'index.ts', files: { 'dep.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nexport const block = shopify.intercept\n`, @@ -67,8 +73,9 @@ const SAMPLES: Sample[] = [ }, }, { - name: 'alias chain const a = shopify; const b = a', + name: 'alias chain', label: 'simple WARNS (chain closed), complex RESOLVES', + show: `const a = shopify\nconst b = a\nb.intercept('beforecapture', cb)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst a = shopify\nconst b = a\nb.intercept('beforecapture', ${CB})\n`, @@ -77,30 +84,34 @@ const SAMPLES: Sample[] = [ { name: 'dynamic event argument (variable)', label: 'BOTH UNRESOLVED (dynamic) — simple WARNS', + show: `const evt = 'beforetax'\nshopify.intercept(evt, cb)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst evt = 'beforetax'\nshopify.intercept(evt, ${CB})\n`, }, }, { - name: 'const-folded event name const E = "..."; intercept(E, cb)', + name: 'const-folded event name', label: 'COMPLEX ALSO FAILS (no constant folding) — simple WARNS', + show: `const E = 'beforeshipping'\nshopify.intercept(E, cb)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst E = 'beforeshipping'\nshopify.intercept(E, ${CB})\n`, }, }, { - name: 'higher-order passing register(shopify.intercept)', + name: 'higher-order passing', label: 'COMPLEX ALSO FAILS (silent miss on HOF) — simple WARNS', + show: `function register(fn) { fn('beforepayment', cb) }\nregister(shopify.intercept)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nfunction register(fn: typeof shopify.intercept) {\n fn('beforepayment', ${CB})\n}\nregister(shopify.intercept)\n`, }, }, { - name: 'stored in object then called const m = { i: shopify.intercept }', + name: 'stored in object then called', label: 'COMPLEX ALSO FAILS (silent miss via object storage) — simple WARNS', + show: `const m = {i: shopify.intercept}\nm.i('beforerefund', cb)`, entry: 'index.ts', files: { 'index.ts': `declare const shopify: {intercept: (e: string, cb: () => void) => void}\nconst m = {i: shopify.intercept}\nm.i('beforerefund', ${CB})\n`, @@ -118,7 +129,14 @@ function writeSample(sample: Sample): string { return join(dir, sample.entry) } -const list = (items: string[]) => (items.length ? `[${items.join(', ')}]` : '[]') +type Mark = '✅' | '🟠' | '❌' + +/** ✅ resolved · 🟠 surfaced/warned but not resolved · ❌ silent miss. */ +function symbolFor(resolved: boolean, surfaced: boolean): Mark { + if (resolved) return '✅' + if (surfaced) return '🟠' + return '❌' +} function indent(text: string): string { return text @@ -128,64 +146,61 @@ function indent(text: string): string { .join('\n') } -async function reportOne(sourceLabel: string, entryPath: string, printedSource?: string): Promise { - const [simple, complex] = await Promise.all([detectPosInterceptsSimple(entryPath), detectPosIntercepts(entryPath)]) +interface Tally { + '✅': number + '🟠': number + '❌': number +} - const simpleWarns = simple.warnings.map((warning) => `${warning.kind}: ${warning.message.split('.')[0]}`) - const complexUnresolved = complex.unresolved.map((entry) => entry.unresolvedReason ?? entry.argText) +async function reportOne( + heading: string, + entryPath: string, + code: string | undefined, + tally: {simple: Tally; complex: Tally}, +): Promise { + const [simple, complex] = await Promise.all([detectPosInterceptsSimple(entryPath), detectPosIntercepts(entryPath)]) - // Verdict. - const simpleStr = simple.events.length - ? `resolved ${list(simple.events)}` - : simpleWarns.length - ? `WARN(${simple.warnings.map((warning) => warning.kind).join(', ')})` - : 'nothing' - const complexStr = complex.events.length - ? `resolved ${list(complex.events)}` - : complexUnresolved.length - ? `unresolved(${complexUnresolved.length})` - : 'NOTHING — silent miss' - const complexFailed = complex.events.length === 0 - const verdict = `${complexFailed && complexUnresolved.length === 0 ? '⚠ COMPLEX ALSO FAILS — ' : ''}simple ${simpleStr} | complex ${complexStr}` + const simpleSymbol = symbolFor(simple.events.length > 0, simple.warnings.length > 0) + const complexSymbol = symbolFor(complex.events.length > 0, complex.unresolved.length > 0) + tally.simple[simpleSymbol]++ + tally.complex[complexSymbol]++ const lines: string[] = [] - lines.push('═'.repeat(72)) - lines.push(sourceLabel) - lines.push('─'.repeat(72)) - if (printedSource !== undefined) { - lines.push('SOURCE:') - lines.push(indent(printedSource)) - lines.push('') - } - lines.push(`SIMPLE events=${list(simple.events)}`) - if (simpleWarns.length) simpleWarns.forEach((warning) => lines.push(` warn ${warning}`)) - lines.push(`COMPLEX events=${list(complex.events)}`) - if (complexUnresolved.length) complexUnresolved.forEach((reason) => lines.push(` unresolved ${reason}`)) - lines.push(`VERDICT ${verdict}`) + lines.push(heading) + if (code !== undefined) lines.push(indent(code)) + lines.push(` SIMPLE ${simpleSymbol} COMPLEX ${complexSymbol}`) + lines.push('') // eslint-disable-next-line no-console console.log(lines.join('\n')) } test('POS intercept detector report (simple vs complex)', async () => { const overridePath = process.env.REPORT_PATH + const tally = { + simple: {'✅': 0, '🟠': 0, '❌': 0}, + complex: {'✅': 0, '🟠': 0, '❌': 0}, + } // eslint-disable-next-line no-console - console.log(`\n\nPOS INTERCEPT DETECTOR REPORT — simple (safe-simplest) vs complex (alias-resolving)\n`) + console.log( + `\n\nPOS INTERCEPT DETECTOR REPORT — simple (safe-simplest) vs complex (alias-resolving)\n` + + `✅ resolved · 🟠 not resolved but surfaced/warned · ❌ silent miss (returned nothing)\n`, + ) if (overridePath) { - // eslint-disable-next-line no-console - console.log(`Running against REPORT_PATH=${overridePath}\n`) - await reportOne(`FILE: ${overridePath}`, overridePath) + await reportOne(`FILE: ${overridePath}`, overridePath, undefined, tally) } else { for (let index = 0; index < SAMPLES.length; index++) { const sample = SAMPLES[index]! const entryPath = writeSample(sample) - const combinedSource = Object.entries(sample.files) - .map(([filename, content]) => `// ── ${filename} ──\n${content}`) - .join('\n') // eslint-disable-next-line no-await-in-loop - await reportOne(`SAMPLE ${index + 1}: ${sample.name} [${sample.label}]`, entryPath, combinedSource) + await reportOne(`${index + 1}. ${sample.name}`, entryPath, sample.show, tally) } } + + const row = (t: Tally) => `✅ ${t['✅']} 🟠 ${t['🟠']} ❌ ${t['❌']}` // eslint-disable-next-line no-console - console.log(`\n${'═'.repeat(72)}\nLEGEND: "silent miss" = a real intercept('event', cb) call the detector\nreturned NOTHING for (not even unresolved). The safe-simplest detector never\nsilently misses — it warns and tells the developer to declare it in TOML.\n`) + console.log( + `${'─'.repeat(56)}\nTALLY\n SIMPLE : ${row(tally.simple)}\n COMPLEX: ${row(tally.complex)}\n` + + `${'─'.repeat(56)}\nSimple has ZERO ❌ (never a silent miss); complex trades some 🟠/❌\nfor silent derivation of the alias cases.\n`, + ) })