Skip to content

Extract mergeAllWebhooks transform from Zod schema into breakdown-extensions#6952

Draft
ryancbahan wants to merge 1 commit intorcb/extract-scopes-transformfrom
rcb/extract-webhook-transform
Draft

Extract mergeAllWebhooks transform from Zod schema into breakdown-extensions#6952
ryancbahan wants to merge 1 commit intorcb/extract-scopes-transformfrom
rcb/extract-webhook-transform

Conversation

@ryancbahan
Copy link
Contributor

@ryancbahan ryancbahan commented Mar 7, 2026

WHY are these changes introduced?

WebhooksConfigSchema applies a mergeAllWebhooks Zod transform at parse time that explodes multi-topic subscription arrays into one-topic-per-subscription entries and reorders compliance subscriptions to the front. This means config.webhooks.subscriptions no longer matches what the user wrote in their TOML — and writing the config back to disk produces a different subscription order than the original file.

This was documented as a known round-trip bug by the snapshot tests in #6947: write→read→write reordered compliance subscriptions to the front. After the second cycle the output stabilized, but the first round-trip was lossy.

WHAT is this pull request doing?

Removes the mergeAllWebhooks transform from WebhooksConfigSchema so parsed webhook subscriptions preserve their raw TOML form (multi-topic arrays stay intact, original ordering preserved).

The only consumer that needs the exploded one-topic-per-subscription form is breakdown-extensions.ts, which compares local config against remote config during deploy. A new normalizeWebhooksForComparison() function applies the normalization there:

  • Explodes to one-topic-per-subscription via mergeAllWebhooks
  • Expands relative URIs to absolute (using application_url)
  • Sorts by URI + topics for stable, order-independent comparison
  • Returns a new object — never mutates the input (important since baselineConfig may reference app.configuration)

This replaces the old inline mutation code that was doing subscription.uri = ... directly on the config object.

Results:

  • The round-trip snapshot test now asserts firstWrite === secondWrite (clean round-trip)
  • The old reordered-subscriptions snapshot is deleted (-52 lines)
  • Loader tests now assert multi-topic arrays are preserved as-is after parse

Note: After this change, code that reads config.webhooks.subscriptions gets the raw TOML form (multi-topic arrays). Only the deploy comparison path calls mergeAllWebhooks to normalize.

How to test your changes?

npx vitest run packages/app/src/cli/services/app/config-pipeline-snapshot.test.ts
npx vitest run packages/app/src/cli/services/context/breakdown-extensions.test.ts
npx vitest run packages/app/src/cli/models/app/loader.test.ts

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 78.85% 14498/18386
🟡 Branches 73.19% 7197/9833
🟡 Functions 79.05% 3693/4672
🟡 Lines 79.18% 13696/17298

Test suite run success

3811 tests passing in 1456 suites.

Report generated by 🧪jest coverage report action from bb7208c

@ryancbahan ryancbahan force-pushed the rcb/extract-webhook-transform branch from 7a9cdb3 to 21a577c Compare March 7, 2026 23:38
@ryancbahan ryancbahan force-pushed the rcb/extract-scopes-transform branch from 65c98c2 to b1bf68f Compare March 9, 2026 15:38
@ryancbahan ryancbahan force-pushed the rcb/extract-webhook-transform branch from 21a577c to 34aab98 Compare March 9, 2026 15:38
@ryancbahan ryancbahan force-pushed the rcb/extract-scopes-transform branch from b1bf68f to f9cdcda Compare March 9, 2026 16:39
@ryancbahan ryancbahan force-pushed the rcb/extract-webhook-transform branch from 34aab98 to 2d9c5ca Compare March 9, 2026 16:39
@ryancbahan ryancbahan force-pushed the rcb/extract-scopes-transform branch from f9cdcda to c3b1deb Compare March 9, 2026 17:08
@ryancbahan ryancbahan force-pushed the rcb/extract-webhook-transform branch from 2d9c5ca to 9d40417 Compare March 9, 2026 17:08
@ryancbahan ryancbahan force-pushed the rcb/extract-scopes-transform branch from c3b1deb to 93b9d18 Compare March 11, 2026 14:12
@ryancbahan ryancbahan force-pushed the rcb/extract-webhook-transform branch from 9d40417 to bb7208c Compare March 11, 2026 14:12
@github-actions
Copy link
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

packages/cli-kit/dist/public/node/toml/codec.d.ts
import { JsonMap } from '../../../private/common/json.js';
export type JsonMapType = JsonMap;
/**
 * Given a TOML string, it returns a JSON object.
 *
 * @param input - TOML string.
 * @returns JSON object.
 */
export declare function decodeToml(input: string): JsonMapType;
/**
 * Given a JSON object, it returns a TOML string.
 *
 * @param content - JSON object.
 * @returns TOML string.
 */
export declare function encodeToml(content: JsonMap | object): string;
packages/cli-kit/dist/public/node/toml/index.d.ts
export type { JsonMapType } from './codec.js';
packages/cli-kit/dist/public/node/toml/toml-file.d.ts
import { JsonMapType } from './codec.js';
/**
 * Thrown when a TOML file cannot be parsed. Includes the file path for context.
 */
export declare class TomlParseError extends Error {
    readonly path: string;
    constructor(path: string, cause: Error);
}
/**
 * General-purpose TOML file abstraction.
 *
 * Provides a unified interface for reading, patching, removing keys from, and replacing
 * the content of TOML files on disk.
 *
 * - `read` populates content from disk
 * - `patch` does surgical WASM-based edits (preserves comments and formatting)
 * - `remove` deletes a key by dotted path (preserves comments and formatting)
 * - `replace` does a full re-serialization (comments and formatting are NOT preserved).
 * - `transformRaw` applies a function to the raw TOML string on disk.
 */
export declare class TomlFile {
    /**
     * Read and parse a TOML file from disk. Throws if the file doesn't exist or contains invalid TOML.
     * Parse errors are wrapped in {@link TomlParseError} with the file path for context.
     *
     * @param path - Absolute path to the TOML file.
     * @returns A TomlFile instance with parsed content.
     */
    static read(path: string): Promise<TomlFile>;
    readonly path: string;
    content: JsonMapType;
    constructor(path: string, content: JsonMapType);
    /**
     * Surgically patch values in the TOML file, preserving comments and formatting.
     *
     * Accepts a nested object whose leaf values are set in the TOML. Intermediate tables are
     * created automatically. Setting a leaf to `undefined` removes it (use `remove()` for a
     * clearer API when deleting keys).
     *
     * @example
     * ```ts
     * await file.patch({build: {dev_store_url: 'my-store.myshopify.com'}})
     * await file.patch({application_url: 'https://example.com', auth: {redirect_urls: ['...']}})
     * ```
     */
    patch(changes: {
        [key: string]: unknown;
    }): Promise<void>;
    /**
     * Remove a key from the TOML file by dotted path, preserving comments and formatting.
     *
     * @param keyPath - Dotted key path to remove (e.g. 'build.include_config_on_deploy').
     * @example
     * ```ts
     * await file.remove('build.include_config_on_deploy')
     * ```
     */
    remove(keyPath: string): Promise<void>;
    /**
     * Replace the entire file content. The file is fully re-serialized — comments and formatting
     * are NOT preserved.
     *
     * @param content - The new content to write.
     * @example
     * ```ts
     * await file.replace({client_id: 'abc', name: 'My App'})
     * ```
     */
    replace(content: JsonMapType): Promise<void>;
    /**
     * Transform the raw TOML string on disk. Reads the file, applies the transform function
     * to the raw text, writes back, and re-parses to keep `content` in sync.
     *
     * Use this for text-level operations that can't be expressed as structured edits —
     * e.g. Injecting comments or positional insertion of keys in arrays-of-tables.
     * Subsequent `patch()` calls will preserve any comments added this way.
     *
     * @param transform - A function that receives the raw TOML string and returns the modified string.
     * @example
     * ```ts
     * await file.transformRaw((raw) => `# Header comment\n${raw}`)
     * ```
     */
    transformRaw(transform: (raw: string) => string): Promise<void>;
    private decode;
}

Existing type declarations

We found no diffs with existing type declarations

…ensions

Remove the parse-time mergeAllWebhooks transform from WebhooksConfigSchema
so that parsed webhook subscriptions preserve their raw TOML form (multi-topic
arrays stay intact). This fixes the round-trip asymmetry where compliance
subscriptions got reordered to the front on re-parse.

The only consumer that needs canonical one-topic-per-subscription form is
breakdown-extensions.ts (for comparing local vs remote config), so the
mergeAllWebhooks normalization is applied there before diffing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ryancbahan ryancbahan force-pushed the rcb/extract-scopes-transform branch from 93b9d18 to 01deb4a Compare March 11, 2026 22:13
@ryancbahan ryancbahan force-pushed the rcb/extract-webhook-transform branch from bb7208c to 0dbbbb9 Compare March 11, 2026 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant