Repository navigation
Add reusable dev session files to CLI Kit - #8952
Open
isaacroldan wants to merge 12 commits into
Open
isaacroldan wants to merge 12 commits into
isaacroldan wants to merge 12 commits into
Conversation
1 of 4 tasks
isaacroldan
marked this pull request as ready for review
October 20, 2026 14:00
| return [command, key] | ||
| .filter(Boolean) | ||
| .join('-') | ||
| .toLowerCase() |
Contributor
There was a problem hiding this comment.
[suggestion] Could we append a hash of the original command/key to the filename slug?
toLowerCase() maps valid config names such as shopify.app.Production.toml and shopify.app.production.toml to identical session files. These can be separate configs on Linux, but starting one blocks the other. This is a rare edge case, so I consider it non-blocking; a hash would keep distinct identities separate.
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/dev-instance.d.tsimport { zod } from './schema.js';
import { Writable } from 'stream';
/**
* A stable error code for JSON consumers when a long-running command refuses to start because another instance of
* it is already running for the same project.
*/
export declare const DEV_INSTANCE_ALREADY_RUNNING_CODE = "dev_instance_already_running";
/**
* Describes a running instance of a long-running command such as `app dev` or `theme dev`.
* It's written to `.shopify/dev/<slug>.json` so agents and other tools can discover the instance.
*/
export interface DevInstanceInfo {
schemaVersion: 1;
/** Identifies this run, including when the OS reuses a PID. */
sessionId: string;
/** The command that owns the instance, for example `app dev`. */
command: string;
/** Distinguishes instances of the same command in the same directory, for example the app configuration file. */
key?: string;
pid: number;
status: 'starting' | 'running';
startedAt: string;
updatedAt: string;
/** The project directory the instance runs for. */
directory: string;
/** The JSON Lines file the instance appends its output to. */
logFile: string;
/** Command-specific, JSON-serializable details such as URLs and ports. */
details: {
[key: string]: unknown;
};
}
export declare const devInstanceLogOutputSchema: import("./json-output-schema.js").JsonOutputSchema<zod.ZodObject<{
timestamp: zod.ZodString;
source: zod.ZodString;
level: zod.ZodOptional<zod.ZodEnum<["debug", "info", "warning", "error"]>>;
message: zod.ZodString;
}, "strict", zod.ZodTypeAny, {
source: string;
message: string;
timestamp: string;
level?: "info" | "error" | "debug" | "warning" | undefined;
}, {
source: string;
message: string;
timestamp: string;
level?: "info" | "error" | "debug" | "warning" | undefined;
}>>;
/** One line of a dev instance's JSON Lines log. */
export type DevInstanceLogEntry = zod.infer<typeof devInstanceLogOutputSchema.schema>;
export interface DevInstancePaths {
/** The folder that holds every dev instance of the project: `.shopify/dev`. */
folder: string;
/** The instance description. */
infoFile: string;
/** The lock directory, owned by `proper-lockfile`. */
lockFile: string;
/** The JSON Lines log. */
logFile: string;
}
export interface DevInstanceIdentity {
/** The command that owns the instance, for example `app dev`. */
command: string;
/** The project directory. */
directory: string;
/** Distinguishes several instances of the same command in the same directory. */
key?: string;
}
export interface AcquireDevInstanceOptions extends DevInstanceIdentity {
/** Initial command-specific details. */
details?: {
[key: string]: unknown;
};
/** Receives the exact JSONL bytes written to the session log. The caller owns this stream. */
logOutput?: Writable;
}
export interface DevInstanceUpdate {
status?: DevInstanceInfo['status'];
/** Merged into the existing details. */
details?: {
[key: string]: unknown;
};
}
export interface DevInstance {
readonly info: DevInstanceInfo;
readonly paths: DevInstancePaths;
/** Aborts when this session loses its lock or output fails. Consumers must stop their owned work. */
readonly abortSignal: AbortSignal;
/** Updates and persists the instance description. */
update: (changes: DevInstanceUpdate) => Promise<void>;
/** Appends one entry to the JSON Lines log. */
log: (entry: Omit<DevInstanceLogEntry, 'timestamp'> & {
timestamp?: string;
}) => Promise<void>;
/**
* Wraps a stream so every line written to it is also appended to the log, without ANSI styling.
* The returned stream forwards the original bytes to `target` unchanged.
*/
tee: (target: Writable, options: {
source: string;
/** Treat each string write as a complete message. Byte writes remain buffered into lines. */
stringWritesAreMessages?: boolean;
}) => Writable;
/** Closes the log, removes the instance description and releases the lock. Safe to call more than once. */
release: () => Promise<void>;
}
/**
* Returns the files used by a dev instance. Every instance of a project shares `.shopify/dev`,
* with one `<slug>.json`, `<slug>.lock` and `<slug>.jsonl` per command and key.
*
* @param identity - The command, directory and optional key of the instance.
* @returns The paths of the instance files.
*/
export declare function devInstancePaths(identity: DevInstanceIdentity): DevInstancePaths;
/**
* Marks this process as the running instance of a long-running command for a project.
*
* Mutual exclusion uses `proper-lockfile`: an atomic `mkdir` lock whose mtime this process refreshes as a heartbeat.
* An exclusive startup directory serializes abandoned-lock recovery. An interrupted startup requires explicit
* cleanup of that directory. A live owner blocks takeover, including when its heartbeat is stale.
*
* @param options - The instance identity, initial details.
* @returns The acquired instance.
* @throws AbortError with code {@link DEV_INSTANCE_ALREADY_RUNNING_CODE} when another live instance holds the lock.
*/
export declare function acquireDevInstance(options: AcquireDevInstanceOptions): Promise<DevInstance>;
/**
* Returns the running instance of a command for a project, if there is one.
*
* @param identity - The command, directory and optional key of the instance.
* @returns The instance description, or undefined when no live instance holds the lock.
*/
export declare function readDevInstance(identity: DevInstanceIdentity): Promise<DevInstanceInfo | undefined>;
/**
* Lists every running dev instance of a project, across commands and keys.
*
* @param directory - The project directory.
* @returns The descriptions of the live instances.
*/
export declare function listDevInstances(directory: string): Promise<DevInstanceInfo[]>;
/**
* Checks whether a process exists. A process owned by another user counts as alive.
*
* @param pid - The process ID.
* @returns True when the process exists.
*/
export declare function isProcessAlive(pid: number): boolean;
Existing type declarationspackages/cli-kit/dist/private/node/ui/components/ConcurrentOutput.d.ts@@ -19,6 +19,8 @@ interface ConcurrentOutputContext {
stripAnsi?: boolean;
}
declare function useConcurrentOutputContext<T>(context: ConcurrentOutputContext, callback: () => T): T;
+/** Returns the source and styling context for the current output write. */
+export declare function getConcurrentOutputContext(): ConcurrentOutputContext | undefined;
/** Runs finite processes concurrently and routes their output through the shared diagnostic context. */
export declare function runConcurrentProcessesForJson({ processes, abortSignal, }: Pick<ConcurrentOutputProps, 'processes' | 'abortSignal'>): Promise<void>;
/**
|
This branch has not been deployed
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?
Long-running commands need a reusable way to expose their current session and output to other processes. This is the base layer for #8926.
WHAT is this pull request doing?
Add a CLI Kit API for session ownership, atomic status files, and JSONL output. It handles stream backpressure and cleanup, and signals consumers to stop when ownership or output fails. The fixed stale interval is 15 seconds. A live PID blocks takeover; an interrupted acquisition requires explicit cleanup of the reported startup lock.
Each run has a new session ID and starts with an empty log. Filenames include a short hash of the original command/key pair, so case changes, punctuation, and command/key boundaries do not merge distinct identities. Cleanup drains queued status updates before releasing ownership, including when a tee fails. This PR owns the complete
dev-instance.tsimplementation and its unit tests, including the canonical record schema and caller-ownedlogOutputstream. The writer validates and serializes each record once, then writes identical JSONL bytes to the file and mirror. Records have notype,stream, or repeated session ID. The schema contains only dev stream fields; fatal CLI error documents stay outside the session log. #8926 connects this API toapp dev; #8957 adds the command's JSON output.How to manually test your changes?
From a built checkout, run:
Read the reported JSON and JSONL files from another terminal. Run the same script there and confirm it reports the existing session. Press Enter in the first terminal, then confirm the status file is removed and the log remains. Restart the script and confirm the session ID changes and the log starts fresh. Set
keytoshopify.app.Production.tomlin one copy andshopify.app.production.tomlin another; both should start with separate session files.Validation
The shared API passes all 33 dev-instance tests, focused lint, and CLI Kit type checking. The historical stress checks below cover the shared API; the newer JSONL mirror check and command output tests are described in #8957.
The tee writer preserves completion callbacks when callers pass an undefined encoding, and it decodes explicit string encodings before recording their text. Four new regression cases first failed on the previous implementation and now pass: callbacks in line/message modes, and hex/base64 data through both
writeandend. The updated stack passes 107 focused tests across dev-instance behavior, app dev, real-bootstrap shutdown, and function builds.The cleanup regression queues 500 status updates, fails the tee destination, and checks that release preserves the original output error, removes metadata and the lock, and allows immediate acquisition in the same process. Three identity cases start distinct sessions concurrently despite identical normalized names: case changes, punctuation, and command/key boundaries. These regressions failed on the reviewed head and pass with the fixes. A compiled-runtime probe independently confirmed cleanup and case-distinct sessions; the rebuilt-CLI bootstrap probe retained all 5,104 stdout/file records after SIGTERM.
Stress testing
Used one-off Node harnesses against the built CLI modules, with real temporary files and separate child processes. The probes used readiness signals, timeouts, and cleanup of their own processes and directories.
write()/drainstopped while blocked, then delivered all 20,000 records after reading resumed.Regression tests for the identified failures are committed. The output-source and app-dev integration regressions were also checked against the old implementation, where they failed. The larger stress loops were local, ad hoc scripts and are not part of CI.
These checks ran on macOS arm64 with Node v26.1.0. Windows/Linux behavior and remote Shopify updates were not exercised by the stress harnesses. Logs remain unbounded by design.
Checklist