Extract mergeAllWebhooks transform from Zod schema into breakdown-extensions#6952
Draft
ryancbahan wants to merge 1 commit intorcb/extract-scopes-transformfrom
Draft
Extract mergeAllWebhooks transform from Zod schema into breakdown-extensions#6952ryancbahan wants to merge 1 commit intorcb/extract-scopes-transformfrom
ryancbahan wants to merge 1 commit intorcb/extract-scopes-transformfrom
Conversation
This was referenced Mar 7, 2026
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
3 tasks
Contributor
Coverage report
Test suite run success3811 tests passing in 1456 suites. Report generated by 🧪jest coverage report action from bb7208c |
7a9cdb3 to
21a577c
Compare
3 tasks
65c98c2 to
b1bf68f
Compare
21a577c to
34aab98
Compare
b1bf68f to
f9cdcda
Compare
34aab98 to
2d9c5ca
Compare
f9cdcda to
c3b1deb
Compare
2d9c5ca to
9d40417
Compare
c3b1deb to
93b9d18
Compare
9d40417 to
bb7208c
Compare
Contributor
Differences in type declarationsWe 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:
New type declarationspackages/cli-kit/dist/public/node/toml/codec.d.tsimport { 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.tsexport type { JsonMapType } from './codec.js';
packages/cli-kit/dist/public/node/toml/toml-file.d.tsimport { 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 declarationsWe 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>
93b9d18 to
01deb4a
Compare
bb7208c to
0dbbbb9
Compare
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

WHY are these changes introduced?
WebhooksConfigSchemaapplies amergeAllWebhooksZod transform at parse time that explodes multi-topic subscription arrays into one-topic-per-subscription entries and reorders compliance subscriptions to the front. This meansconfig.webhooks.subscriptionsno 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
mergeAllWebhookstransform fromWebhooksConfigSchemaso 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 newnormalizeWebhooksForComparison()function applies the normalization there:mergeAllWebhooksapplication_url)baselineConfigmay referenceapp.configuration)This replaces the old inline mutation code that was doing
subscription.uri = ...directly on the config object.Results:
firstWrite === secondWrite(clean round-trip)Note: After this change, code that reads
config.webhooks.subscriptionsgets the raw TOML form (multi-topic arrays). Only the deploy comparison path callsmergeAllWebhooksto normalize.How to test your changes?
Measuring impact
Checklist