Skip to content

[Spike] Derive POS intercept events from source (AST) instead of TOML#8058

Draft
henryStelle wants to merge 6 commits into
henry/pos-intercept-failsafe-configfrom
henry/pos-intercept-ast-derive
Draft

[Spike] Derive POS intercept events from source (AST) instead of TOML#8058
henryStelle wants to merge 6 commits into
henry/pos-intercept-failsafe-configfrom
henry/pos-intercept-ast-derive

Conversation

@henryStelle

@henryStelle henryStelle commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Spike / exploration — not for merge. Draft opened against henry/pos-intercept-failsafe-config so the diff is exactly the AST-derivation delta on top of the config-flow prototype.

What this explores

Deriving the POS intercepts events from extension source code (AST) instead of a hand-authored capabilities.intercepts TOML array — per the CLI's 2022 "Configuration source of truth" ADR ("derive if it can be derived").

What it does

  • pos_intercept_detection.ts — reuses the CLI's existing AST tooling (findAllImportedFiles + TS compiler API) to walk the extension's import graph and find every shopify.intercept('<event>') callsite.
  • Ignores control flow — both branches of an if/else/ternary are counted.
  • Tracks the shopify.intercept reference through aliasing via a whole-graph fixpoint: const guard = shopify.intercept, const {intercept} = shopify, renamed destructures, const s = shopify; s.intercept(...), reassignment, and cross-file export const block = shopify.intercept imported + called elsewhere — all resolve back to the event.
  • Dynamic/computed args (variables, template strings) reported as UNRESOLVED with file:line, never dropped.
  • Wired into pos_ui_extension.ts deployConfig via deriveAndMergeIntercepts() (unions derived + TOML).

Key design answer — derived events → backend (no new mechanism)

Derived events ride the existing capabilities.intercepts wire field: deployConfigbundleConfig() JSON → deploy upload → backend persists the config blob. Byte-identical to a TOML-declared array, so the backend consumer can't tell the source and needs zero change. TOML becomes an optional override/escape hatch.

Reliability gaps (see pos_intercept_detection.DESIGN.md)

Shallow data-flow (doesn't follow fn-params/HOF/.bind/object storage); no const-folding; tsconfig path aliases not resolved for alias-propagation. A TypeChecker-backed analysis + esbuild metafile graph would harden it. Dynamic args need a product decision (warn vs hard-fail vs require TOML).


Simple vs. complex — detection coverage

Each row is a code pattern containing a real shopify.intercept('<event>', cb) call. ✅ = resolved the event · 🟠 = didn't resolve but surfaced/warned (loud, not silent) · ❌ = silent miss (returned nothing).

# pattern simple complex
1 plain direct call
2 same-file destructure (const {intercept} = shopify) 🟠
3 object alias then member call (const s = shopify; s.intercept(...)) 🟠
4 cross-file re-exported reference 🟠
5 alias chain (const a = shopify; const b = a) 🟠
6 dynamic event arg (variable) 🟠 🟠
7 const-folded event name 🟠 🟠
8 higher-order passing (register(shopify.intercept)) 🟠
9 stored in object then called (const m = {i: shopify.intercept}) 🟠
tally ✅1 · 🟠8 · ❌0 ✅5 · 🟠2 · ❌2

Takeaway: the simple detector never silently misses (❌0) — it resolves the one plain case and loudly flags the other 8 for explicit TOML declaration. The complex detector resolves 5 but silently misses 2 (higher-order passing, object storage), since those defeat its shallow data-flow. The extra machinery buys 4 more silent resolves at the cost of 2 silent misses.

Run it yourself

cd packages/app
../../node_modules/.bin/vitest run src/cli/models/extensions/specifications/pos_intercept_detection_report.test.ts

Prints the same grid from the two detectors run over live fixtures. Related tests (detector + comparison matrix):

../../node_modules/.bin/vitest run src/cli/models/extensions/specifications/pos_intercept_detection

…ploy

Prototype AST detector that walks the extension import graph from its entry
and detects every shopify.intercept('<event>') 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
@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Jul 10, 2026
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
Simple detector now resolves ONLY direct shopify.intercept('<literal>') 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
…tectors

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
…-chain gap

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
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
@github-actions

Copy link
Copy Markdown
Contributor

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/private/node/constants.d.ts
@@ -6,6 +6,7 @@ export declare const environmentVariables: {
     doctor: string;
     enableCliRedirect: string;
     env: string;
+    firstPartyDev: string;
     noAnalytics: string;
     optOutInstrumentation: string;
     appAutomationToken: string;
packages/cli-kit/dist/private/node/session.d.ts
@@ -90,7 +90,6 @@ export declare function setLastSeenUserIdAfterAuth(id: string): void;
  */
 export declare function getLastSeenAuthMethod(): Promise<AuthMethod>;
 export declare function setLastSeenAuthMethod(method: AuthMethod): void;
-export declare function setCommandSessionId(sessionId: string | undefined): void;
 export interface EnsureAuthenticatedAdditionalOptions {
     noPrompt?: boolean;
     forceRefresh?: boolean;
packages/cli-kit/dist/public/common/object.d.ts
@@ -38,14 +38,6 @@ export declare function mapValues<T extends object, TResult>(source: T | null |
  * @returns True if the objects are equal, false otherwise.
  */
 export declare function deepCompare(one: object, two: object): boolean;
-/**
- * Deeply compares two values and treats arrays as order-insensitive.
- *
- * @param one - The first value to be compared.
- * @param two - The second value to be compared.
- * @returns True if the normalized values are equal, false otherwise.
- */
-export declare function deepCompareWithOrderInsensitiveArrays(one: unknown, two: unknown): boolean;
 /**
  * Return the difference between two nested objects.
  *
packages/cli-kit/dist/public/common/version.d.ts
@@ -1 +1 @@
-export declare const CLI_KIT_VERSION = "4.4.0";
\ No newline at end of file
+export declare const CLI_KIT_VERSION = "4.1.0";
\ No newline at end of file
packages/cli-kit/dist/public/node/base-command.d.ts
@@ -16,7 +16,6 @@ declare abstract class BaseCommand extends Command {
     protected parse<TFlags extends FlagOutput & {
         path?: string;
         verbose?: boolean;
-        'auth-alias'?: string;
     }, TGlobalFlags extends FlagOutput, TArgs extends ArgOutput>(options?: Input<TFlags, TGlobalFlags, TArgs>, argv?: string[]): Promise<ParserOutput<TFlags, TGlobalFlags, TArgs> & {
         argv: string[];
     }>;
packages/cli-kit/dist/public/node/cli.d.ts
@@ -39,9 +39,6 @@ export declare const globalFlags: {
 export declare const jsonFlag: {
     json: import("@oclif/core/interfaces").BooleanFlag<boolean>;
 };
-export declare const authAliasFlag: {
-    'auth-alias': import("@oclif/core/interfaces").OptionFlag<string | undefined, import("@oclif/core/interfaces").CustomOptions>;
-};
 /**
  * Builds a  flag that only accepts a valid port number. The flag parses its
  * value as an integer and rejects anything that isn't a whole number between 1 and
packages/cli-kit/dist/public/node/monorail.d.ts
@@ -2,7 +2,7 @@ import { JsonMap } from '../../private/common/json.js';
 import { DeepRequired } from '../common/ts/deep-required.js';
 export { DeepRequired };
 type Optional<T> = T | null;
-export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.28";
+export declare const MONORAIL_COMMAND_TOPIC = "app_cli3_command/1.26";
 export interface Schemas {
     [MONORAIL_COMMAND_TOPIC]: {
         sensitive: {
@@ -19,7 +19,6 @@ export interface Schemas {
         public: {
             business_platform_id?: Optional<number>;
             partner_id?: Optional<number>;
-            store_id?: Optional<number>;
             command: string;
             project_type?: Optional<string>;
             time_start: number;
packages/cli-kit/dist/public/node/session.d.ts
@@ -22,19 +22,6 @@ export type AccountInfo = UserAccountInfo | ServiceAccountInfo | UnknownAccountI
  * @param userId - User identifier to report on the command analytics event.
  */
 export declare function setLastSeenUserId(userId: string): void;
-/**
- * Finds a stored Shopify account session by alias without changing the current session.
- *
- * @param alias - The account alias to find.
- * @returns The matching session ID, or undefined if no session matches.
- */
-export declare function findSessionIdByAlias(alias: string): Promise<string | undefined>;
-/**
- * Selects a stored Shopify account session by alias for the current command process.
- *
- * @param alias - The account alias to select. Passing undefined clears the command selection.
- */
-export declare function setCurrentSessionAlias(alias?: string): Promise<void>;
 interface UserAccountInfo {
     type: 'UserAccount';
     email: string;
@@ -132,10 +119,9 @@ export declare function ensureAuthenticatedThemes(store: string, password: strin
  * Ensure that we have a valid session to access the Business Platform API.
  *
  * @param scopes - Optional array of extra scopes to authenticate with.
- * @param options - Optional extra options to use.
  * @returns The access token for the Business Platform API.
  */
-export declare function ensureAuthenticatedBusinessPlatform(scopes?: BusinessPlatformScope[], options?: EnsureAuthenticatedAdditionalOptions): Promise<string>;
+export declare function ensureAuthenticatedBusinessPlatform(scopes?: BusinessPlatformScope[]): Promise<string>;
 /**
  * Logout from Shopify.
  *
packages/cli-kit/dist/public/node/ui.d.ts
@@ -81,7 +81,10 @@ To see a list of supported npm commands, run:
  * │                                                          │
  * ╰──────────────────────────────────────────────────────────╯
  * [1] https://shopify.dev
- * [2] https://www.google.com/search?q=jh56t9l34kpo35tw8s28hn7s9s2xvzla01d8cn6j7yq&rlz=1C1GCEU_enUS832US832&oq=jh56t9l34kpo35tw8s28hn7s9s2xvzla01d8cn6j7yq&aqs=chrome.0.35i39l2j0l4j46j69i60.2711j0j7&sourceid=chrome&ie=UTF-8
+ * [2] https://www.google.com/search?q=jh56t9l34kpo35tw8s28hn7s
+ * 9s2xvzla01d8cn6j7yq&rlz=1C1GCEU_enUS832US832&oq=jh56t9l34kpo
+ * 35tw8s28hn7s9s2xvzla01d8cn6j7yq&aqs=chrome.0.35i39l2j0l4j46j
+ * 69i60.2711j0j7&sourceid=chrome&ie=UTF-8
  * [3] https://shopify.com
  *
  */
@@ -109,7 +112,8 @@ export declare function renderInfo(options: RenderAlertOptions): string | undefi
  * │    • See your deployment and set it live [1]             │
  * │                                                          │
  * ╰──────────────────────────────────────────────────────────╯
- * [1] https://partners.shopify.com/1797046/apps/4523695/deployments
+ * [1] https://partners.shopify.com/1797046/apps/4523695/deploy
+ * ments
  *
  */
 export declare function renderSuccess(options: RenderAlertOptions): string | undefined;
packages/cli-kit/dist/public/node/api/admin.d.ts
@@ -9,10 +9,9 @@ import { TypedDocumentNode } from '@graphql-typed-document-node/core';
  * @param query - GraphQL query to execute.
  * @param session - Shopify admin session including token and Store FQDN.
  * @param variables - GraphQL variables to pass to the query.
- * @param version - API version.
  * @returns The response of the query of generic type <T>.
  */
-export declare function adminRequest<T>(query: string, session: AdminSession, variables?: GraphQLVariables, version?: string): Promise<T>;
+export declare function adminRequest<T>(query: string, session: AdminSession, variables?: GraphQLVariables): Promise<T>;
 export interface AdminRequestOptions<TResult, TVariables extends Variables> {
     /** GraphQL query to execute. */
     query: TypedDocumentNode<TResult, TVariables>;
packages/cli-kit/dist/public/node/context/local.d.ts
@@ -63,6 +63,13 @@ export declare function alwaysLogAnalytics(env?: NodeJS.ProcessEnv): boolean;
  * @returns True if SHOPIFY_CLI_ALWAYS_LOG_METRICS is truthy.
  */
 export declare function alwaysLogMetrics(env?: NodeJS.ProcessEnv): boolean;
+/**
+ * Returns true if the CLI User is 1P.
+ *
+ * @param env - The environment variables from the environment of the current process.
+ * @returns True if SHOPIFY_CLI_1P is truthy.
+ */
+export declare function firstPartyDev(env?: NodeJS.ProcessEnv): boolean;
 /**
  * Returns true if the CLI can run the "doctor-release" command.
  *

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant