Skip to content

[Feature] Add Dev Instance support to theme dev - #8976

Open
isaacroldan wants to merge 8 commits into
mainfrom
codex/theme-dev-instance
Open

isaacroldan wants to merge 8 commits into
mainfrom
codex/theme-dev-instance

Conversation

@isaacroldan

@isaacroldan isaacroldan commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Fixes https://github.com/shop/issues-develop/issues/24318

Agents cannot find a running theme dev session or read its output after the command starts in another terminal.

WHAT is this pull request doing?

Use Dev Instance to record the session, preview URLs, and JSONL logs under the theme's .shopify/dev/ directory. Ownership uses the store and theme ID, so a duplicate start can report the existing session. Acquisition stays in the service.

Capture theme messages before terminal rendering, including while Ink progress is active. Theme log records contain timestamp and message, without repeating a source field. The shared log schema allows this field to be omitted; app-dev continues to include its process sources. On Ctrl+C, SIGTERM, or loss of instance ownership, cancel dev work and drain queued output before releasing the instance. This also adds cancellation for the file watcher, theme-editor polling, and session refresh timer.

This PR does not add theme dev --json.

The shared agent workflow is in docs/cli/dev-instance-agent-workflow.md, with separate app and theme sections.

How to manually test your changes?

  1. In a test store, start a development theme with pnpm shopify theme dev --path /path/to/theme --store your-store.myshopify.com --theme DEVELOPMENT_THEME_ID.
  2. Read the matching instance JSON in /path/to/theme/.shopify/dev/. Check the store, theme ID, status, and preview URLs. Open its logFile, change a theme asset, and visit the local preview. Confirm that sync messages and request lines appear in the terminal and log.
  3. Run the same command in another terminal. It should report the existing instance. A different port or listing does not allow a second session for the same store and theme in that directory.
  4. Press Ctrl+C once. Check that the process exits, the instance JSON and lock are removed, and the JSONL log remains.

Validation and stress tests

  • 480 focused tests pass across 32 files, including related theme environment, Dev Instance, output, and app-dev contract tests. Theme and CLI Kit type checking and changed-source lint pass.
  • Real child processes load the service and bootstrap from workspace sources. They preserve 1,000 queued messages plus shutdown output, exit on the first signal, and remove ownership files. New cases stall crawler lookup or session initialization and stop before those requests finish.
  • The output test enables Ink's normal console patching and checks synchronization messages, warnings, and errors while progress is active. Both new regressions fail without their fixes.
  • No live-store synchronization or manual Windows console test was performed. Subprocess signal tests skip Windows.

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

@isaacroldan
isaacroldan requested review from a team as code owners October 21, 2026 16:07
@github-actions github-actions Bot added Area: @shopify/theme @shopify/theme package issues Area: @shopify/cli @shopify/cli package issues and removed Area: @shopify/theme @shopify/theme package issues labels Oct 21, 2026
@isaacroldan
isaacroldan added this pull request to stack #8953 October 22, 2026 08:41
Base automatically changed from codex/app-dev-json to main October 22, 2026 15:58
@isaacroldan
isaacroldan force-pushed the codex/theme-dev-instance branch from 5bbee4e to ef7d27a Compare October 22, 2026 16:06
@gonzaloriestra

Copy link
Copy Markdown
Contributor

You have to remove this command (and app dev) from the exception list.

@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/cli @shopify/cli package issues labels Oct 26, 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/dev-instance-output.d.ts
@@ -2,18 +2,18 @@ import { zod } from '../../public/node/schema.js';
 import { Writable, type WritableOptions } from 'stream';
 export declare const devInstanceLogOutputSchema: import("../../public/node/json-output-schema.js").JsonOutputSchema<zod.ZodObject<{
     timestamp: zod.ZodString;
-    source: zod.ZodString;
+    source: zod.ZodOptional<zod.ZodString>;
     level: zod.ZodOptional<zod.ZodEnum<["debug", "info", "warning", "error"]>>;
     message: zod.ZodString;
 }, "strict", zod.ZodTypeAny, {
-    source: string;
     message: string;
     timestamp: string;
+    source?: string | undefined;
     level?: "info" | "error" | "debug" | "warning" | undefined;
 }, {
-    source: string;
     message: string;
     timestamp: string;
+    source?: string | undefined;
     level?: "info" | "error" | "debug" | "warning" | undefined;
 }>>;
 /** One line of a dev instance's JSON Lines log. */
@@ -30,7 +30,7 @@ export declare function createDevInstanceOutput({ logFile, logOutput, isActive,
         timestamp?: string;
     }) => Promise<void>;
     tee: (target: Writable, { source, stringWritesAreMessages }: {
-        source: string;
+        source?: string;
         stringWritesAreMessages?: boolean;
     }) => DevInstanceOutput;
     flush: () => Promise<PromiseSettledResult<void>[]>;
packages/cli-kit/dist/private/node/output-context.d.ts
@@ -2,6 +2,8 @@ export interface ConcurrentOutputContext {
     outputPrefix?: string;
     /** Controls ANSI stripping for terminal output. JSON diagnostics are always unstyled. */
     stripAnsi?: boolean;
+    /** Captures console messages before the terminal renderer handles them. */
+    consoleWarn?: (message: string) => void;
 }
 export declare function useConcurrentOutputContext<T>(context: ConcurrentOutputContext, callback: () => T): T;
 /** Returns the source and styling context for the current output write. */
packages/cli-kit/dist/public/node/dev-instance.d.ts
@@ -80,7 +80,8 @@ export interface DevInstance {
      * The returned stream forwards the original bytes to `target` unchanged.
      */
     tee: (target: Writable, options: {
-        source: string;
+        /** Omit the source field when the stream does not need to distinguish producers. */
+        source?: string;
         /** Treat each string write as a complete message. Byte writes remain buffered into lines. */
         stringWritesAreMessages?: boolean;
     }) => Writable;

@gonzaloriestra

Copy link
Copy Markdown
Contributor

[blocking, P1] Stop pending synchronization before releasing ownership

At packages/theme/src/cli/services/dev.ts:252, shutdown closes the server but leaves an in-flight Theme Editor poll running. I reproduced the old poll deleting a newly edited local template after the instance JSON and lock had been removed, while shutdown analytics was still pending. Another session can already start editing at that point.

Please honor cancellation after remote requests and before local writes/deletions, and retain ownership until pending synchronization has stopped.

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

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.

2 participants