Skip to content

Expand app configuration through extension specifications - #8686

Open
isaacroldan wants to merge 9 commits into
mainfrom
isaac/specification-config-expansion
Open

isaacroldan wants to merge 9 commits into
mainfrom
isaac/specification-config-expansion

Conversation

@isaacroldan

@isaacroldan isaacroldan commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Alternative to #8426, built on #8425. Deploying one events module per subscription needs support for multiple modules from one app configuration section. The events contract and identity rules belong in its specification.

WHAT is this pull request doing?

Add optional specification hooks for configuration expansion, module identity, and module target. The loader creates and validates every result through the same path, while the events specification owns the subscription split.

Expand event subscriptions by default, without an organization flag lookup. Require valid, unique subscription handles and reject the reserved legacy handle events. Keep app branding handles out of events validation, preserve extra subscription fields, and return no modules for an empty subscription list.

Validated with 350 tests across eight files, app type checking, lint on changed files, and Knip for the app workspace. Includes deploy/config-link round trips, identity stability, and invalid configuration cases. A live deploy has not been tested. The server must support single-subscription events modules before this CLI change is released.

How to manually test your changes?

pnpm shopify app deploy --path /path/to/app
pnpm shopify app config link --path /path/to/app

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing and includes a patch changeset

@github-actions github-actions Bot added the Area: @shopify/app @shopify/app package issues label Sep 28, 2026
@cdarne
cdarne marked this pull request as ready for review September 29, 2026 18:40
@cdarne
cdarne requested a review from a team as a code owner September 29, 2026 18:40
@isaacroldan
isaacroldan force-pushed the isaac/specification-config-expansion branch 3 times, most recently from a63015f to 6809ca7 Compare September 30, 2026 15:01
isaacroldan and others added 4 commits September 30, 2026 17:02
Keep event subscription expansion and identity in the events specification. Use the shared loader validation path for each expanded module and validate subscription handles before resolving identity.

Co-authored-by: Rezaan Syed <rezaan.syed@shopify.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@isaacroldan
isaacroldan force-pushed the isaac/specification-config-expansion branch from 6809ca7 to 2241ebd Compare September 30, 2026 15:02
@github-actions github-actions Bot added no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. and removed Area: @shopify/app @shopify/app package issues labels Sep 30, 2026
@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/api.d.ts
@@ -37,21 +37,6 @@ export declare function isTransientNetworkError(error: unknown): boolean;
  * - Permanent: certificate validation failures, misconfigured SSL
  */
 export declare function isNetworkError(error: unknown): boolean;
-/**
- * Checks if an error is an aborted request: a user cancelling the command, the host process
- * cancelling it, or one of the CLI's own request timeouts firing.
- *
- * Not used by the retry logic, because a user-cancelled request must not be retried.
- * `isTransientNetworkError` separately matches the CLI's own timeout message, so timeouts do
- * still retry.
- *
- * The `name` check matches the `AbortError` shape that fetch throws, not cli-kit's own
- * `AbortError`, which leaves `name` as 'Error'.
- *
- * @param error - Error to be checked.
- * @returns A boolean indicating if the request was aborted.
- */
-export declare function isAbortedFetchError(error: unknown): boolean;
 export declare function simpleRequestWithDebugLog<T extends {
     headers: Headers;
     status: number;
packages/cli-kit/dist/public/common/command-events.d.ts
@@ -23,14 +23,14 @@ export declare const commandDiagnosticEventSchema: z.ZodObject<{
 export declare const commandProgressEventSchema: z.ZodObject<{
     type: z.ZodLiteral<"progress">;
     timestamp: z.ZodString;
-    status: z.ZodEnum<["started", "updated", "retrying", "completed", "failed"]>;
+    status: z.ZodEnum<["started", "updated", "completed"]>;
     operation: z.ZodString;
     message: z.ZodOptional<z.ZodString>;
     current: z.ZodOptional<z.ZodNumber>;
     total: z.ZodOptional<z.ZodNumber>;
 }, "strict", z.ZodTypeAny, {
     type: "progress";
-    status: "started" | "updated" | "retrying" | "completed" | "failed";
+    status: "started" | "updated" | "completed";
     timestamp: string;
     operation: string;
     message?: string | undefined;
@@ -38,7 +38,7 @@ export declare const commandProgressEventSchema: z.ZodObject<{
     total?: number | undefined;
 }, {
     type: "progress";
-    status: "started" | "updated" | "retrying" | "completed" | "failed";
+    status: "started" | "updated" | "completed";
     timestamp: string;
     operation: string;
     message?: string | undefined;
@@ -67,14 +67,14 @@ export declare const commandEventSchema: z.ZodDiscriminatedUnion<"type", [z.ZodO
 }>, z.ZodObject<{
     type: z.ZodLiteral<"progress">;
     timestamp: z.ZodString;
-    status: z.ZodEnum<["started", "updated", "retrying", "completed", "failed"]>;
+    status: z.ZodEnum<["started", "updated", "completed"]>;
     operation: z.ZodString;
     message: z.ZodOptional<z.ZodString>;
     current: z.ZodOptional<z.ZodNumber>;
     total: z.ZodOptional<z.ZodNumber>;
 }, "strict", z.ZodTypeAny, {
     type: "progress";
-    status: "started" | "updated" | "retrying" | "completed" | "failed";
+    status: "started" | "updated" | "completed";
     timestamp: string;
     operation: string;
     message?: string | undefined;
@@ -82,7 +82,7 @@ export declare const commandEventSchema: z.ZodDiscriminatedUnion<"type", [z.ZodO
     total?: number | undefined;
 }, {
     type: "progress";
-    status: "started" | "updated" | "retrying" | "completed" | "failed";
+    status: "started" | "updated" | "completed";
     timestamp: string;
     operation: string;
     message?: string | undefined;
packages/cli-kit/dist/public/node/command-events.d.ts
@@ -29,14 +29,14 @@ export declare const commandEventOutputSchema: import("./json-output-schema.js")
 }>, import("zod").ZodObject<{
     type: import("zod").ZodLiteral<"progress">;
     timestamp: import("zod").ZodString;
-    status: import("zod").ZodEnum<["started", "updated", "retrying", "completed", "failed"]>;
+    status: import("zod").ZodEnum<["started", "updated", "completed"]>;
     operation: import("zod").ZodString;
     message: import("zod").ZodOptional<import("zod").ZodString>;
     current: import("zod").ZodOptional<import("zod").ZodNumber>;
     total: import("zod").ZodOptional<import("zod").ZodNumber>;
 }, "strict", import("zod").ZodTypeAny, {
     type: "progress";
-    status: "started" | "updated" | "retrying" | "completed" | "failed";
+    status: "started" | "updated" | "completed";
     timestamp: string;
     operation: string;
     message?: string | undefined;
@@ -44,7 +44,7 @@ export declare const commandEventOutputSchema: import("./json-output-schema.js")
     total?: number | undefined;
 }, {
     type: "progress";
-    status: "started" | "updated" | "retrying" | "completed" | "failed";
+    status: "started" | "updated" | "completed";
     timestamp: string;
     operation: string;
     message?: string | undefined;
packages/cli-kit/dist/public/node/local-storage.d.ts
@@ -48,7 +48,6 @@ export declare class LocalStorage<T extends Record<string, any>> {
      *
      * @param error - The error that occurred.
      * @param operation - The operation that failed.
-     * @param configPath - The local storage configuration file path.
      * @throws AbortError if the error is permission-related.
      * @throws BugError if the error is not permission-related.
      */
packages/cli-kit/dist/public/node/ui.d.ts
@@ -318,8 +318,6 @@ export declare function renderTasks<TContext>(tasks: Task<TContext>[], { renderO
 export interface RenderSingleTaskOptions<T> {
     title: TokenizedString;
     task: (updateStatus: (status: TokenizedString) => void) => Promise<T>;
-    /** The number of additional attempts after a failure. Defaults to zero. */
-    retry?: number;
     onAbort?: () => void;
     renderOptions?: RenderOptions;
 }
@@ -328,13 +326,12 @@ export interface RenderSingleTaskOptions<T> {
  * @param options - Configuration object
  * @param options.title - The initial title to display with the loading bar
  * @param options.task - The async task to execute. Receives an updateStatus callback to change the displayed title.
- * @param options.retry - The number of additional attempts after a failure. Defaults to zero.
  * @param options.renderOptions - Optional render configuration
  * @returns The result of the task
  * @example
  * Loading app ...
  */
-export declare function renderSingleTask<T>({ title, task, retry, onAbort, renderOptions, }: RenderSingleTaskOptions<T>): Promise<T>;
+export declare function renderSingleTask<T>({ title, task, onAbort, renderOptions, }: RenderSingleTaskOptions<T>): Promise<T>;
 export interface RenderTextPromptOptions extends Omit<TextPromptProps, 'onSubmit'> {
     renderOptions?: RenderOptions;
 }
packages/cli-kit/dist/public/node/context/local.d.ts
@@ -11,12 +11,6 @@ export declare function isTerminalInteractive(): boolean;
  * @returns The path to the user's home directory.
  */
 export declare function homeDirectory(): string;
-/**
- * Clears the memoized result of isUnitTest so the environment variable is re-read.
- *
- * Only intended for test helpers that temporarily toggle unit-test detection.
- */
-export declare function resetMemoizedIsUnitTest(): void;
 /**
  * Returns true if the CLI is running in debug mode.
  *
packages/cli-kit/dist/public/node/testing/output.d.ts
@@ -8,36 +8,6 @@ interface OutputMock {
     error: () => string;
     clear: () => void;
 }
-interface StandardStreamsMock {
-    stdout: () => string;
-    stderr: () => string;
-    restore: () => void;
-}
-export interface CapturedStandardStreams {
-    stdout: () => string;
-    stderr: () => string;
-}
-/**
- * Runs a callback with process stdout/stderr captured and unit-test output suppression disabled,
- * so tests can assert on what a command actually writes to the standard streams.
- *
- * The callback receives accessors instead of the function returning captured output so that
- * assertions remain possible when the callback throws (for example commands that abort).
- * Streams, console.warn and unit-test detection are restored afterwards.
- * Not safe for concurrent tests.
- *
- * @param run - Callback receiving accessors for the captured stdout and stderr.
- * @returns The value returned by the callback.
- */
-export declare function withCapturedStandardStreams<T>(run: (streams: CapturedStandardStreams) => T | Promise<T>): Promise<T>;
-/**
- * Captures writes to stdout and stderr, including console warnings intercepted by Vitest.
- * Call restore in a finally block. This replaces process globals and must not be used in concurrent tests.
- * Prefer withCapturedStandardStreams, which also disables unit-test output suppression while it runs.
- *
- * @returns Captured output and a function to restore the original writers.
- */
-export declare function mockAndCaptureStandardStreams(): StandardStreamsMock;
 /**
  * Returns a set of functions to get the outputs ocurred during a test run.
  *
packages/cli-kit/dist/private/node/ui/hooks/use-async-and-unmount.d.ts
@@ -1,6 +1,6 @@
-interface Options<T> {
-    onFulfilled?: (result: T) => unknown;
+interface Options {
+    onFulfilled?: () => unknown;
     onRejected?: (error: Error) => void;
 }
-export default function useAsyncAndUnmount<T>(asyncFunction: () => Promise<T>, { onFulfilled, onRejected }?: Options<T>): void;
+export default function useAsyncAndUnmount(asyncFunction: () => Promise<unknown>, { onFulfilled, onRejected }?: Options): void;
 export {};
\ No newline at end of file
packages/cli-kit/dist/private/node/ui/components/Tasks.d.ts
@@ -1,7 +1,14 @@
 import { AbortSignal } from '../../../../public/node/abort.js';
-import { Task } from '../tasks.js';
+import { TokenizedString } from '../../../../public/node/output.js';
 import React from 'react';
-export type { Task } from '../tasks.js';
+export interface Task<TContext = unknown> {
+    title: string | TokenizedString;
+    task: (ctx: TContext, task: Task<TContext>) => Promise<void | Task<TContext>[]>;
+    retry?: number;
+    retryCount?: number;
+    errors?: Error[];
+    skip?: (ctx: TContext) => boolean;
+}
 interface TasksProps<TContext> {
     tasks: Task<TContext>[];
     silent?: boolean;

Rename the flag to f_single_subscription_events_modules and drop the
SHOPIFY_CLI_EVENTS_SUBSCRIPTION_FANOUT environment opt-in.
Business Platform resolves hashed client handles rather than readable
Verdict handles, so the readable name always evaluated to false.
Comment thread packages/app/src/cli/models/extensions/specifications/app_config_events.ts Outdated
The platform is removing the nested handle from the single-subscription
contract, so an expanded configuration that still carried it inside the
subscription failed validation before the deploy transform stripped it.
Expand each subscription into a module with its handle at the top level,
like every other module, and derive identity and target from there.
@rezaansyed
rezaansyed requested a review from dpeacock September 30, 2026 20:35
Base automatically changed from events-config-link-shape-tolerance to main October 1, 2026 09:25
@isaacroldan

Copy link
Copy Markdown
Contributor Author

/snapit

Comment on lines +27 to +30
const EventsSchema = BaseSchemaWithoutHandle.extend({
events: zod.any().optional(),
handle: ModuleHandleSchema.optional(),
events: EventsSectionSchema.optional(),
}).superRefine((config, context) => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't include handle here outside of events. This will receive the full app toml config, and handle is a field belonging to the branding module. We can only validate events and its children here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rezaansyed this is a limitation of the expansion approach, i'm looking on ways to improve this.

@github-actions github-actions Bot added Area: @shopify/app @shopify/app package issues and removed no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. labels Oct 1, 2026

This branch has not been deployed

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

Labels

Area: @shopify/app @shopify/app package issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants