Repository navigation
Add JSON streaming to app function replay watch mode - #9007
gonzaloriestra wants to merge 7 commits into
Conversation
|
/snapit |
|
🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261027114600Caution After installing, validate the version by running |
|
/snapit |
|
🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261027123425Caution After installing, validate the version by running |
83d41bb to
3ba15e0
Compare
249da1b to
cd2ee97
Compare
08fe311 to
5bbafa4
Compare
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 declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/base-command.d.ts@@ -13,7 +13,7 @@ export interface NonTTYFlagRequirement {
declare abstract class BaseCommand extends Command {
static baseFlags: FlagInput<{}>;
static descriptionWithMarkdown?: string;
- static get jsonOutputSchema(): JsonOutputSchema | undefined;
+ static jsonOutputSchema?: JsonOutputSchema;
static get requiresSyncAnalytics(): boolean;
static nonTTYFlagRequirements(_flags: FlagOutput): NonTTYFlagRequirement[];
static descriptionForHelp(): string | undefined;
|
isaacroldan
left a comment
There was a problem hiding this comment.
Found two issues with watcher readiness and build cancellation.
🤖 Generated by the review-pr skill
| if (signal.aborted) return | ||
| await watcher.start({stdout: diagnostics, stderr: diagnostics, signal}, false) | ||
| if (!signal.aborted) { | ||
| writeEvent({type: 'status', timestamp: formatJsonOutputTimestamp(new Date()), status: 'watching'}) |
There was a problem hiding this comment.
[P2] watching is emitted before Chokidar finishes its initial scan, so the first edit can be missed. I reproduced this with a real watcher: an edit right after the record did nothing, while a second edit after ready triggered a rebuild. Can we wait for Chokidar's ready event here and add a test for an edit immediately after watching?
🤖 Generated by the review-pr skill
There was a problem hiding this comment.
Fixed. FileWatcher.start() now waits for Chokidar’s ready event before replay emits watching. Startup also settles on cancellation or scan errors. Added a real-watcher regression that edits the Function immediately when watching is emitted and verifies a rebuild and second execution.
| async stop(): Promise<void> { | ||
| this.stopping = true | ||
| await this.fileWatcher?.stop() | ||
| await Promise.all(this.pendingEvents) |
There was a problem hiding this comment.
[P2] If Ctrl-C arrives while a rebuild is waiting for .build-lock, the build still starts once the lock is released. exec() only listens for future abort events, so it misses the earlier abort, and this wait keeps replay open until the new command finishes. I reproduced this through watchReplay with a real lock and subprocess. Can we make the lock wait cancellable and check for an already-aborted signal before starting the build?
🤖 Generated by the review-pr skill
There was a problem hiding this comment.
Fixed. Lock retries now use abortable waits, with signal checks before and after lock acquisition and between build stages. exec() also skips commands whose signal is already aborted. Added real-lock and subprocess coverage for cancellation while blocked, cancellation on acquisition, and a successful build after the lock is released.
WHY are these changes introduced?
Fixes https://github.com/shop/issues-develop/issues/24315.
Function replay watches by default, but currently rejects
--jsonunless--no-watchis supplied.WHAT is this pull request doing?
Add a typed JSONL watch stream with lifecycle, rebuild, and execution records. Keep diagnostics on stderr and preserve native runner fields and numeric tokens inside execution results. Recoverable build and Function failures keep watching; fatal failures stop the session. Cancellation waits for the watcher, builds, and runner to finish.
The finite
--no-watch --jsonresult and text watch UI stay compatible. Help and schema discovery describe both output modes.Representative text output:
JSON watch output:
{"type":"status","timestamp":"2026-10-26T09:00:00Z","status":"watching"} {"type":"execution","timestamp":"2026-10-26T09:00:00Z","result":{"name":"function.wasm","size":1,"memory_usage":64,"instructions":1000,"logs":"","input":{},"output":{"operations":[]},"success":true},"exitCode":0}Declare the replay JSON schema as a static field.
How to manually test your changes?
pnpm shopify app function replay --path <function-directory> --log <log-identifier> --json --no-input > replay.jsonl 2> replay.stderr.--no-watch --jsonto inspect the single native result, and without--jsonto inspect the terminal UI.pnpm shopify app function replay --json-schemato inspect both contracts without starting replay.Checklist
minorfor CLI streaming support,patchfor CLI Kit process cancellation).