Skip to content

Commit 85ace06

Browse files
committed
🤖 refactor: DRY cleanup for Docker runtime implementation
Extract shared utilities and consolidate duplicate code: 1. streamUtils.ts - Extract streamToString helper used by SSH and Docker 2. streamUtils.ts - Extract shescape shell escaping helper used by SSH and Docker 3. initHook.ts - Add runInitHookOnRuntime() shared by SSH and Docker runtimes 4. chatCommands.ts - Use parseRuntimeModeAndHost from common/types/runtime Net reduction: ~79 lines of duplicate code removed while improving maintainability. _Generated with mux_
1 parent 1610068 commit 85ace06

File tree

5 files changed

+166
-245
lines changed

5 files changed

+166
-245
lines changed

src/browser/utils/chatCommands.ts

Lines changed: 37 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type {
1616
} from "@/common/types/message";
1717
import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
1818
import type { RuntimeConfig } from "@/common/types/runtime";
19-
import { RUNTIME_MODE, SSH_RUNTIME_PREFIX, DOCKER_RUNTIME_PREFIX } from "@/common/types/runtime";
19+
import { RUNTIME_MODE, parseRuntimeModeAndHost } from "@/common/types/runtime";
2020
import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events";
2121
import { WORKSPACE_ONLY_COMMANDS } from "@/constants/slashCommands";
2222
import type { Toast } from "@/browser/components/ChatInputToast";
@@ -430,7 +430,9 @@ async function handleForkCommand(
430430
}
431431

432432
/**
433-
* Parse runtime string from -r flag into RuntimeConfig for backend
433+
* Parse runtime string from -r flag into RuntimeConfig for backend.
434+
* Uses shared parseRuntimeModeAndHost for parsing, then converts to RuntimeConfig.
435+
*
434436
* Supports formats:
435437
* - "ssh <host>" or "ssh <user@host>" -> SSH runtime
436438
* - "docker <image>" -> Docker container runtime
@@ -442,57 +444,45 @@ export function parseRuntimeString(
442444
runtime: string | undefined,
443445
_workspaceName: string
444446
): RuntimeConfig | undefined {
445-
if (!runtime) {
446-
return undefined; // Default to worktree (backend decides)
447-
}
448-
449-
const trimmed = runtime.trim();
450-
const lowerTrimmed = trimmed.toLowerCase();
451-
452-
// Worktree runtime (explicit or default)
453-
if (lowerTrimmed === RUNTIME_MODE.WORKTREE) {
454-
return undefined; // Explicit worktree - let backend use default
455-
}
456-
457-
// Local runtime (project-dir, no isolation)
458-
if (lowerTrimmed === RUNTIME_MODE.LOCAL) {
459-
// Return "local" type without srcBaseDir to indicate project-dir runtime
460-
return { type: RUNTIME_MODE.LOCAL };
461-
}
462-
463-
// Parse "ssh <host>" or "ssh <user@host>" format
464-
if (lowerTrimmed === RUNTIME_MODE.SSH || lowerTrimmed.startsWith(SSH_RUNTIME_PREFIX)) {
465-
const hostPart = trimmed.slice(SSH_RUNTIME_PREFIX.length - 1).trim(); // Preserve original case for host
466-
if (!hostPart) {
447+
// Use shared parser from common/types/runtime
448+
const parsed = parseRuntimeModeAndHost(runtime);
449+
450+
// null means invalid input (e.g., "ssh" without host, "docker" without image)
451+
if (parsed === null) {
452+
// Determine which error to throw based on input
453+
const trimmed = runtime?.trim().toLowerCase() ?? "";
454+
if (trimmed === RUNTIME_MODE.SSH || trimmed.startsWith("ssh ")) {
467455
throw new Error("SSH runtime requires host (e.g., 'ssh hostname' or 'ssh user@host')");
468456
}
469-
470-
// Accept both "hostname" and "user@hostname" formats
471-
// SSH will use current user or ~/.ssh/config if user not specified
472-
// Use tilde path - backend will resolve it via runtime.resolvePath()
473-
return {
474-
type: RUNTIME_MODE.SSH,
475-
host: hostPart,
476-
srcBaseDir: "~/mux", // Default remote base directory (tilde will be resolved by backend)
477-
};
478-
}
479-
480-
// Parse "docker <image>" format
481-
if (lowerTrimmed === RUNTIME_MODE.DOCKER || lowerTrimmed.startsWith(DOCKER_RUNTIME_PREFIX)) {
482-
const imagePart = trimmed.slice(DOCKER_RUNTIME_PREFIX.length - 1).trim(); // Preserve original case for image
483-
if (!imagePart) {
457+
if (trimmed === RUNTIME_MODE.DOCKER || trimmed.startsWith("docker ")) {
484458
throw new Error("Docker runtime requires image (e.g., 'docker ubuntu:22.04')");
485459
}
486-
487-
return {
488-
type: RUNTIME_MODE.DOCKER,
489-
image: imagePart,
490-
};
460+
throw new Error(
461+
`Unknown runtime type: '${runtime ?? ""}'. Use 'ssh <host>', 'docker <image>', 'worktree', or 'local'`
462+
);
491463
}
492464

493-
throw new Error(
494-
`Unknown runtime type: '${runtime}'. Use 'ssh <host>', 'docker <image>', 'worktree', or 'local'`
495-
);
465+
// Convert ParsedRuntime to RuntimeConfig
466+
switch (parsed.mode) {
467+
case RUNTIME_MODE.WORKTREE:
468+
return undefined; // Let backend use default worktree config
469+
470+
case RUNTIME_MODE.LOCAL:
471+
return { type: RUNTIME_MODE.LOCAL };
472+
473+
case RUNTIME_MODE.SSH:
474+
return {
475+
type: RUNTIME_MODE.SSH,
476+
host: parsed.host,
477+
srcBaseDir: "~/mux", // Default remote base directory (tilde resolved by backend)
478+
};
479+
480+
case RUNTIME_MODE.DOCKER:
481+
return {
482+
type: RUNTIME_MODE.DOCKER,
483+
image: parsed.image,
484+
};
485+
}
496486
}
497487

498488
export interface CreateWorkspaceOptions {

src/node/runtime/DockerRuntime.ts

Lines changed: 4 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -28,29 +28,19 @@ import type {
2828
import { RuntimeError } from "./Runtime";
2929
import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/common/constants/exitCodes";
3030
import { log } from "@/node/services/log";
31-
import { checkInitHookExists, createLineBufferedLoggers, getMuxEnv } from "./initHook";
31+
import { checkInitHookExists, getMuxEnv, runInitHookOnRuntime } from "./initHook";
3232
import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env";
3333
import { getProjectName } from "@/node/utils/runtime/helpers";
3434
import { getErrorMessage } from "@/common/utils/errors";
3535
import { DisposableProcess } from "@/node/utils/disposableExec";
36+
import { streamToString, shescape } from "./streamUtils";
3637

3738
/** Hardcoded source directory inside container */
3839
const CONTAINER_SRC_DIR = "/src";
3940

4041
/** Hardcoded background output directory inside container */
4142
const _CONTAINER_BG_OUTPUT_DIR = "/tmp/mux-bashes";
4243

43-
/**
44-
* Shell-escape helper for container bash.
45-
*/
46-
const shescape = {
47-
quote(value: unknown): string {
48-
const s = String(value);
49-
if (s.length === 0) return "''";
50-
return "'" + s.replace(/'/g, "'\"'\"'") + "'";
51-
},
52-
};
53-
5444
/**
5545
* Result of running a Docker command
5646
*/
@@ -568,7 +558,8 @@ export class DockerRuntime implements Runtime {
568558
const hookExists = await checkInitHookExists(projectPath);
569559
if (hookExists) {
570560
const muxEnv = getMuxEnv(projectPath, "docker", branchName);
571-
await this.runInitHook(workspacePath, muxEnv, initLogger, abortSignal);
561+
const hookPath = `${workspacePath}/.mux/init`;
562+
await runInitHookOnRuntime(this, hookPath, workspacePath, muxEnv, initLogger, abortSignal);
572563
} else {
573564
initLogger.logComplete(0);
574565
}
@@ -724,61 +715,6 @@ export class DockerRuntime implements Runtime {
724715
}
725716
}
726717

727-
/**
728-
* Run .mux/init hook inside container
729-
*/
730-
private async runInitHook(
731-
workspacePath: string,
732-
muxEnv: Record<string, string>,
733-
initLogger: InitLogger,
734-
abortSignal?: AbortSignal
735-
): Promise<void> {
736-
const hookPath = `${workspacePath}/.mux/init`;
737-
initLogger.logStep(`Running init hook: ${hookPath}`);
738-
739-
const hookStream = await this.exec(hookPath, {
740-
cwd: workspacePath,
741-
timeout: 3600,
742-
abortSignal,
743-
env: muxEnv,
744-
});
745-
746-
const loggers = createLineBufferedLoggers(initLogger);
747-
const stdoutReader = hookStream.stdout.getReader();
748-
const stderrReader = hookStream.stderr.getReader();
749-
const decoder = new TextDecoder();
750-
751-
const readStdout = async () => {
752-
try {
753-
while (true) {
754-
const { done, value } = await stdoutReader.read();
755-
if (done) break;
756-
loggers.stdout.append(decoder.decode(value, { stream: true }));
757-
}
758-
loggers.stdout.flush();
759-
} finally {
760-
stdoutReader.releaseLock();
761-
}
762-
};
763-
764-
const readStderr = async () => {
765-
try {
766-
while (true) {
767-
const { done, value } = await stderrReader.read();
768-
if (done) break;
769-
loggers.stderr.append(decoder.decode(value, { stream: true }));
770-
}
771-
loggers.stderr.flush();
772-
} finally {
773-
stderrReader.releaseLock();
774-
}
775-
};
776-
777-
const [exitCode] = await Promise.all([hookStream.exitCode, readStdout(), readStderr()]);
778-
779-
initLogger.logComplete(exitCode);
780-
}
781-
782718
// eslint-disable-next-line @typescript-eslint/require-await
783719
async renameWorkspace(
784720
_projectPath: string,
@@ -877,23 +813,3 @@ export class DockerRuntime implements Runtime {
877813
return Promise.resolve("/tmp");
878814
}
879815
}
880-
/**
881-
* Helper to convert a ReadableStream to a string
882-
*/
883-
async function streamToString(stream: ReadableStream<Uint8Array>): Promise<string> {
884-
const reader = stream.getReader();
885-
const decoder = new TextDecoder("utf-8");
886-
let result = "";
887-
888-
try {
889-
while (true) {
890-
const { done, value } = await reader.read();
891-
if (done) break;
892-
result += decoder.decode(value, { stream: true });
893-
}
894-
result += decoder.decode();
895-
return result;
896-
} finally {
897-
reader.releaseLock();
898-
}
899-
}

src/node/runtime/SSHRuntime.ts

Lines changed: 9 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ import type {
1717
import { RuntimeError as RuntimeErrorClass } from "./Runtime";
1818
import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/common/constants/exitCodes";
1919
import { log } from "@/node/services/log";
20-
import { checkInitHookExists, createLineBufferedLoggers, getMuxEnv } from "./initHook";
20+
import { checkInitHookExists, getMuxEnv, runInitHookOnRuntime } from "./initHook";
21+
import { expandTildeForSSH as expandHookPath } from "./tildeExpansion";
2122
import { NON_INTERACTIVE_ENV_VARS } from "@/common/constants/env";
2223
import { streamProcessToLogger } from "./streamProcess";
2324
import { expandTildeForSSH, cdCommandForSSH } from "./tildeExpansion";
@@ -26,20 +27,7 @@ import { getErrorMessage } from "@/common/utils/errors";
2627
import { execAsync, DisposableProcess } from "@/node/utils/disposableExec";
2728
import { getControlPath, sshConnectionPool, type SSHRuntimeConfig } from "./sshConnectionPool";
2829
import { getBashPath } from "@/node/utils/main/bashPath";
29-
30-
/**
31-
* Shell-escape helper for remote bash.
32-
* Reused across all SSH runtime operations for performance.
33-
* Note: For background process commands, use shellQuote from backgroundCommands for parity.
34-
*/
35-
const shescape = {
36-
quote(value: unknown): string {
37-
const s = String(value);
38-
if (s.length === 0) return "''";
39-
// Use POSIX-safe pattern to embed single quotes within single-quoted strings
40-
return "'" + s.replace(/'/g, "'\"'\"'") + "'";
41-
},
42-
};
30+
import { streamToString, shescape } from "./streamUtils";
4331

4432
// Re-export SSHRuntimeConfig from connection pool (defined there to avoid circular deps)
4533
export type { SSHRuntimeConfig } from "./sshConnectionPool";
@@ -771,78 +759,6 @@ export class SSHRuntime implements Runtime {
771759
}
772760
}
773761

774-
/**
775-
* Run .mux/init hook on remote machine if it exists
776-
* @param workspacePath - Path to the workspace directory on remote
777-
* @param muxEnv - MUX_ environment variables (from getMuxEnv)
778-
* @param initLogger - Logger for streaming output
779-
* @param abortSignal - Optional abort signal
780-
*/
781-
private async runInitHook(
782-
workspacePath: string,
783-
muxEnv: Record<string, string>,
784-
initLogger: InitLogger,
785-
abortSignal?: AbortSignal
786-
): Promise<void> {
787-
// Construct hook path - expand tilde if present
788-
const remoteHookPath = `${workspacePath}/.mux/init`;
789-
initLogger.logStep(`Running init hook: ${remoteHookPath}`);
790-
791-
// Expand tilde in hook path for execution
792-
// Tilde won't be expanded when the path is quoted, so we need to expand it ourselves
793-
const hookCommand = expandTildeForSSH(remoteHookPath);
794-
795-
// Run hook remotely and stream output
796-
// No timeout - user init hooks can be arbitrarily long
797-
const hookStream = await this.exec(hookCommand, {
798-
cwd: workspacePath, // Run in the workspace directory
799-
timeout: 3600, // 1 hour - generous timeout for init hooks
800-
abortSignal,
801-
env: muxEnv,
802-
});
803-
804-
// Create line-buffered loggers
805-
const loggers = createLineBufferedLoggers(initLogger);
806-
807-
// Stream stdout/stderr through line-buffered loggers
808-
const stdoutReader = hookStream.stdout.getReader();
809-
const stderrReader = hookStream.stderr.getReader();
810-
const decoder = new TextDecoder();
811-
812-
// Read stdout in parallel
813-
const readStdout = async () => {
814-
try {
815-
while (true) {
816-
const { done, value } = await stdoutReader.read();
817-
if (done) break;
818-
loggers.stdout.append(decoder.decode(value, { stream: true }));
819-
}
820-
loggers.stdout.flush();
821-
} finally {
822-
stdoutReader.releaseLock();
823-
}
824-
};
825-
826-
// Read stderr in parallel
827-
const readStderr = async () => {
828-
try {
829-
while (true) {
830-
const { done, value } = await stderrReader.read();
831-
if (done) break;
832-
loggers.stderr.append(decoder.decode(value, { stream: true }));
833-
}
834-
loggers.stderr.flush();
835-
} finally {
836-
stderrReader.releaseLock();
837-
}
838-
};
839-
840-
// Wait for completion
841-
const [exitCode] = await Promise.all([hookStream.exitCode, readStdout(), readStderr()]);
842-
843-
initLogger.logComplete(exitCode);
844-
}
845-
846762
getWorkspacePath(projectPath: string, workspaceName: string): string {
847763
const projectName = getProjectName(projectPath);
848764
return path.posix.join(this.config.srcBaseDir, projectName, workspaceName);
@@ -957,11 +873,13 @@ export class SSHRuntime implements Runtime {
957873
await this.pullLatestFromOrigin(workspacePath, trunkBranch, initLogger, abortSignal);
958874

959875
// 4. Run .mux/init hook if it exists
960-
// Note: runInitHook calls logComplete() internally if hook exists
876+
// Note: runInitHookOnRuntime calls logComplete() internally
961877
const hookExists = await checkInitHookExists(projectPath);
962878
if (hookExists) {
963879
const muxEnv = getMuxEnv(projectPath, "ssh", branchName);
964-
await this.runInitHook(workspacePath, muxEnv, initLogger, abortSignal);
880+
// Expand tilde in hook path (quoted paths don't auto-expand on remote)
881+
const hookPath = expandHookPath(`${workspacePath}/.mux/init`);
882+
await runInitHookOnRuntime(this, hookPath, workspacePath, muxEnv, initLogger, abortSignal);
965883
} else {
966884
// No hook - signal completion immediately
967885
initLogger.logComplete(0);
@@ -1330,23 +1248,5 @@ export class SSHRuntime implements Runtime {
13301248
}
13311249
}
13321250

1333-
/**
1334-
* Helper to convert a ReadableStream to a string
1335-
*/
1336-
export async function streamToString(stream: ReadableStream<Uint8Array>): Promise<string> {
1337-
const reader = stream.getReader();
1338-
const decoder = new TextDecoder("utf-8");
1339-
let result = "";
1340-
1341-
try {
1342-
while (true) {
1343-
const { done, value } = await reader.read();
1344-
if (done) break;
1345-
result += decoder.decode(value, { stream: true });
1346-
}
1347-
result += decoder.decode();
1348-
return result;
1349-
} finally {
1350-
reader.releaseLock();
1351-
}
1352-
}
1251+
// Re-export for backward compatibility with existing imports
1252+
export { streamToString } from "./streamUtils";

0 commit comments

Comments
 (0)