From 00b77421dda830aaba9f69d668bca4eeb891cffa Mon Sep 17 00:00:00 2001 From: sheeek Date: Fri, 9 Jan 2026 09:45:45 +0100 Subject: [PATCH] refactor: improve sandbox commands code structure Improvements: - Extract validation into separate function - Split display logic from business logic - Create reusable container matcher for agent filtering - Abstract status/image formatting into helpers - Reduce code duplication between containers and browsers - Extract container removal into generic function - Add type safety with FilteredContainers type - Improve readability with smaller, focused functions Changes: - validateRecreateOptions(): Validate mutual exclusivity - fetchAndFilterContainers(): Fetch + filter in one place - createAgentMatcher(): Reusable agent filter predicate - displayContainers/Browsers(): Dedicated display functions - displaySummary/RecreatePreview/Result(): Clear separation - removeContainer(): Generic removal with error handling - Format helpers: formatStatus, formatImageMatch, etc. - Count helpers: countRunning, countMismatches Result: 85 more lines but much better maintainability and testability. --- src/commands/sandbox.ts | 449 ++++++++++++++++++++++++---------------- 1 file changed, 267 insertions(+), 182 deletions(-) diff --git a/src/commands/sandbox.ts b/src/commands/sandbox.ts index e7564e801..92b150369 100644 --- a/src/commands/sandbox.ts +++ b/src/commands/sandbox.ts @@ -1,21 +1,39 @@ import { confirm as clackConfirm } from "@clack/prompts"; import { + type SandboxBrowserInfo, + type SandboxContainerInfo, listSandboxBrowsers, listSandboxContainers, removeSandboxBrowserContainer, removeSandboxContainer, } from "../agents/sandbox.js"; import type { RuntimeEnv } from "../runtime.js"; -import { stylePromptTitle } from "../terminal/prompt-style.js"; -// --- List Command --- +// --- Types --- type SandboxListOptions = { browser: boolean; json: boolean; }; +type SandboxRecreateOptions = { + all: boolean; + session?: string; + agent?: string; + browser: boolean; + force: boolean; +}; + +type ContainerItem = SandboxContainerInfo | SandboxBrowserInfo; + +type FilteredContainers = { + containers: SandboxContainerInfo[]; + browsers: SandboxBrowserInfo[]; +}; + +// --- List Command --- + export async function sandboxListCommand( opts: SandboxListOptions, runtime: RuntimeEnv, @@ -28,74 +46,157 @@ export async function sandboxListCommand( : []; if (opts.json) { - runtime.log( - JSON.stringify( - { containers, browsers }, - null, - 2, - ), - ); + runtime.log(JSON.stringify({ containers, browsers }, null, 2)); return; } if (opts.browser) { - if (browsers.length === 0) { - runtime.log("No sandbox browser containers found."); - return; - } - - runtime.log("\nšŸŒ Sandbox Browser Containers:\n"); - for (const browser of browsers) { - const status = browser.running ? "šŸŸ¢ running" : "āš« stopped"; - const imageStatus = browser.imageMatch ? "āœ“" : "āš ļø mismatch"; - const age = formatAge(Date.now() - browser.createdAtMs); - const idle = formatAge(Date.now() - browser.lastUsedAtMs); - - runtime.log(` ${browser.containerName}`); - runtime.log(` Status: ${status}`); - runtime.log(` Image: ${browser.image} ${imageStatus}`); - runtime.log(` CDP: ${browser.cdpPort}`); - if (browser.noVncPort) { - runtime.log(` noVNC: ${browser.noVncPort}`); - } - runtime.log(` Age: ${age}`); - runtime.log(` Idle: ${idle}`); - runtime.log(` Session: ${browser.sessionKey}`); - runtime.log(""); - } + displayBrowsers(browsers, runtime); } else { - if (containers.length === 0) { - runtime.log("No sandbox containers found."); - return; - } - - runtime.log("\nšŸ“¦ Sandbox Containers:\n"); - for (const container of containers) { - const status = container.running ? "šŸŸ¢ running" : "āš« stopped"; - const imageStatus = container.imageMatch ? "āœ“" : "āš ļø mismatch"; - const age = formatAge(Date.now() - container.createdAtMs); - const idle = formatAge(Date.now() - container.lastUsedAtMs); - - runtime.log(` ${container.containerName}`); - runtime.log(` Status: ${status}`); - runtime.log(` Image: ${container.image} ${imageStatus}`); - runtime.log(` Age: ${age}`); - runtime.log(` Idle: ${idle}`); - runtime.log(` Session: ${container.sessionKey}`); - runtime.log(""); - } + displayContainers(containers, runtime); } - // Summary - const totalContainers = containers.length + browsers.length; - const runningCount = - containers.filter((c) => c.running).length + - browsers.filter((b) => b.running).length; - const mismatchCount = - containers.filter((c) => !c.imageMatch).length + - browsers.filter((b) => !b.imageMatch).length; + displaySummary(containers, browsers, runtime); +} + +// --- Recreate Command --- + +export async function sandboxRecreateCommand( + opts: SandboxRecreateOptions, + runtime: RuntimeEnv, +): Promise { + validateRecreateOptions(opts, runtime); + + const filtered = await fetchAndFilterContainers(opts); + + if (filtered.containers.length + filtered.browsers.length === 0) { + runtime.log("No containers found matching the criteria."); + return; + } + + displayRecreatePreview(filtered, runtime); + + if (!opts.force && !(await confirmRecreate())) { + runtime.log("Cancelled."); + return; + } + + const result = await removeContainers(filtered, runtime); + displayRecreateResult(result, runtime); + + if (result.failCount > 0) { + runtime.exit(1); + } +} + +// --- Validation --- + +function validateRecreateOptions( + opts: SandboxRecreateOptions, + runtime: RuntimeEnv, +): void { + if (!opts.all && !opts.session && !opts.agent) { + runtime.error("Please specify --all, --session , or --agent "); + runtime.exit(1); + } + + const exclusiveCount = [opts.all, opts.session, opts.agent].filter(Boolean) + .length; + if (exclusiveCount > 1) { + runtime.error("Please specify only one of: --all, --session, --agent"); + runtime.exit(1); + } +} + +// --- Filtering --- + +async function fetchAndFilterContainers( + opts: SandboxRecreateOptions, +): Promise { + const allContainers = await listSandboxContainers().catch(() => []); + const allBrowsers = await listSandboxBrowsers().catch(() => []); + + let containers = opts.browser ? [] : allContainers; + let browsers = opts.browser ? allBrowsers : []; + + if (opts.session) { + containers = containers.filter((c) => c.sessionKey === opts.session); + browsers = browsers.filter((b) => b.sessionKey === opts.session); + } else if (opts.agent) { + const matchesAgent = createAgentMatcher(opts.agent); + containers = containers.filter(matchesAgent); + browsers = browsers.filter(matchesAgent); + } + + return { containers, browsers }; +} + +function createAgentMatcher(agentId: string) { + const agentPrefix = `agent:${agentId}`; + return (item: ContainerItem) => + item.sessionKey === agentPrefix || + item.sessionKey.startsWith(`${agentPrefix}:`); +} + +// --- Display Functions --- + +function displayContainers( + containers: SandboxContainerInfo[], + runtime: RuntimeEnv, +): void { + if (containers.length === 0) { + runtime.log("No sandbox containers found."); + return; + } + + runtime.log("\nšŸ“¦ Sandbox Containers:\n"); + for (const container of containers) { + runtime.log(` ${container.containerName}`); + runtime.log(` Status: ${formatStatus(container.running)}`); + runtime.log(` Image: ${container.image} ${formatImageMatch(container.imageMatch)}`); + runtime.log(` Age: ${formatAge(Date.now() - container.createdAtMs)}`); + runtime.log(` Idle: ${formatAge(Date.now() - container.lastUsedAtMs)}`); + runtime.log(` Session: ${container.sessionKey}`); + runtime.log(""); + } +} + +function displayBrowsers( + browsers: SandboxBrowserInfo[], + runtime: RuntimeEnv, +): void { + if (browsers.length === 0) { + runtime.log("No sandbox browser containers found."); + return; + } + + runtime.log("\nšŸŒ Sandbox Browser Containers:\n"); + for (const browser of browsers) { + runtime.log(` ${browser.containerName}`); + runtime.log(` Status: ${formatStatus(browser.running)}`); + runtime.log(` Image: ${browser.image} ${formatImageMatch(browser.imageMatch)}`); + runtime.log(` CDP: ${browser.cdpPort}`); + if (browser.noVncPort) { + runtime.log(` noVNC: ${browser.noVncPort}`); + } + runtime.log(` Age: ${formatAge(Date.now() - browser.createdAtMs)}`); + runtime.log(` Idle: ${formatAge(Date.now() - browser.lastUsedAtMs)}`); + runtime.log(` Session: ${browser.sessionKey}`); + runtime.log(""); + } +} + +function displaySummary( + containers: SandboxContainerInfo[], + browsers: SandboxBrowserInfo[], + runtime: RuntimeEnv, +): void { + const totalCount = containers.length + browsers.length; + const runningCount = countRunning(containers) + countRunning(browsers); + const mismatchCount = countMismatches(containers) + countMismatches(browsers); + + runtime.log(`Total: ${totalCount} (${runningCount} running)`); - runtime.log(`Total: ${totalContainers} (${runningCount} running)`); if (mismatchCount > 0) { runtime.log( `\nāš ļø ${mismatchCount} container(s) with image mismatch detected.`, @@ -106,152 +207,126 @@ export async function sandboxListCommand( } } -// --- Recreate Command --- - -type SandboxRecreateOptions = { - all: boolean; - session?: string; - agent?: string; - browser: boolean; - force: boolean; -}; - -export async function sandboxRecreateCommand( - opts: SandboxRecreateOptions, +function displayRecreatePreview( + filtered: FilteredContainers, runtime: RuntimeEnv, -): Promise { - // Validation - if (!opts.all && !opts.session && !opts.agent) { - runtime.error( - "Please specify --all, --session , or --agent ", - ); - runtime.exit(1); - return; - } - - if ( - (opts.all && opts.session) || - (opts.all && opts.agent) || - (opts.session && opts.agent) - ) { - runtime.error("Please specify only one of: --all, --session, --agent"); - runtime.exit(1); - return; - } - - // Fetch containers - const allContainers = await listSandboxContainers().catch(() => []); - const allBrowsers = await listSandboxBrowsers().catch(() => []); - - // Filter based on options - let containersToRemove = opts.browser ? [] : allContainers; - let browsersToRemove = opts.browser ? allBrowsers : []; - - if (opts.session) { - containersToRemove = containersToRemove.filter( - (c) => c.sessionKey === opts.session, - ); - browsersToRemove = browsersToRemove.filter( - (b) => b.sessionKey === opts.session, - ); - } else if (opts.agent) { - const agentPrefix = `agent:${opts.agent}`; - containersToRemove = containersToRemove.filter( - (c) => c.sessionKey === agentPrefix || c.sessionKey.startsWith(`${agentPrefix}:`), - ); - browsersToRemove = browsersToRemove.filter( - (b) => b.sessionKey === agentPrefix || b.sessionKey.startsWith(`${agentPrefix}:`), - ); - } - - const totalToRemove = containersToRemove.length + browsersToRemove.length; - - if (totalToRemove === 0) { - runtime.log("No containers found matching the criteria."); - return; - } - - // Show what will be removed +): void { runtime.log("\nContainers to be recreated:\n"); - if (containersToRemove.length > 0) { + if (filtered.containers.length > 0) { runtime.log("šŸ“¦ Sandbox Containers:"); - for (const container of containersToRemove) { - const status = container.running ? "running" : "stopped"; - runtime.log(` - ${container.containerName} (${status})`); + for (const container of filtered.containers) { + runtime.log( + ` - ${container.containerName} (${formatSimpleStatus(container.running)})`, + ); } } - if (browsersToRemove.length > 0) { + if (filtered.browsers.length > 0) { runtime.log("\nšŸŒ Browser Containers:"); - for (const browser of browsersToRemove) { - const status = browser.running ? "running" : "stopped"; - runtime.log(` - ${browser.containerName} (${status})`); + for (const browser of filtered.browsers) { + runtime.log( + ` - ${browser.containerName} (${formatSimpleStatus(browser.running)})`, + ); } } - runtime.log(`\nTotal: ${totalToRemove} container(s)`); + const total = filtered.containers.length + filtered.browsers.length; + runtime.log(`\nTotal: ${total} container(s)`); +} - // Confirmation - if (!opts.force) { - const shouldContinue = await clackConfirm({ - message: "This will stop and remove these containers. Continue?", - initialValue: false, - }); +function displayRecreateResult( + result: { successCount: number; failCount: number }, + runtime: RuntimeEnv, +): void { + runtime.log( + `\nDone: ${result.successCount} removed, ${result.failCount} failed`, + ); - if (!shouldContinue || shouldContinue === Symbol.for("clack:cancel")) { - runtime.log("Cancelled."); - return; - } + if (result.successCount > 0) { + runtime.log( + "\nContainers will be automatically recreated when the agent is next used.", + ); } +} - // Remove containers +// --- Container Operations --- + +async function confirmRecreate(): Promise { + const result = await clackConfirm({ + message: "This will stop and remove these containers. Continue?", + initialValue: false, + }); + + return result !== false && result !== Symbol.for("clack:cancel"); +} + +async function removeContainers( + filtered: FilteredContainers, + runtime: RuntimeEnv, +): Promise<{ successCount: number; failCount: number }> { runtime.log("\nRemoving containers...\n"); let successCount = 0; let failCount = 0; - for (const container of containersToRemove) { - try { - await removeSandboxContainer(container.containerName); - runtime.log(`āœ“ Removed ${container.containerName}`); - successCount++; - } catch (err) { - runtime.error( - `āœ— Failed to remove ${container.containerName}: ${String(err)}`, - ); - failCount++; - } - } - - for (const browser of browsersToRemove) { - try { - await removeSandboxBrowserContainer(browser.containerName); - runtime.log(`āœ“ Removed ${browser.containerName}`); - successCount++; - } catch (err) { - runtime.error( - `āœ— Failed to remove ${browser.containerName}: ${String(err)}`, - ); - failCount++; - } - } - - // Summary - runtime.log(`\nDone: ${successCount} removed, ${failCount} failed`); - - if (successCount > 0) { - runtime.log( - "\nContainers will be automatically recreated when the agent is next used.", + for (const container of filtered.containers) { + const result = await removeContainer( + container.containerName, + removeSandboxContainer, + runtime, ); + if (result.success) { + successCount++; + } else { + failCount++; + } } - if (failCount > 0) { - runtime.exit(1); + for (const browser of filtered.browsers) { + const result = await removeContainer( + browser.containerName, + removeSandboxBrowserContainer, + runtime, + ); + if (result.success) { + successCount++; + } else { + failCount++; + } + } + + return { successCount, failCount }; +} + +async function removeContainer( + containerName: string, + removeFn: (name: string) => Promise, + runtime: RuntimeEnv, +): Promise<{ success: boolean }> { + try { + await removeFn(containerName); + runtime.log(`āœ“ Removed ${containerName}`); + return { success: true }; + } catch (err) { + runtime.error(`āœ— Failed to remove ${containerName}: ${String(err)}`); + return { success: false }; } } -// --- Helpers --- +// --- Formatting Helpers --- + +function formatStatus(running: boolean): string { + return running ? "šŸŸ¢ running" : "āš« stopped"; +} + +function formatSimpleStatus(running: boolean): string { + return running ? "running" : "stopped"; +} + +function formatImageMatch(matches: boolean): string { + return matches ? "āœ“" : "āš ļø mismatch"; +} function formatAge(ms: number): string { const seconds = Math.floor(ms / 1000); @@ -264,3 +339,13 @@ function formatAge(ms: number): string { if (minutes > 0) return `${minutes}m`; return `${seconds}s`; } + +// --- Counting Helpers --- + +function countRunning(items: ContainerItem[]): number { + return items.filter((item) => item.running).length; +} + +function countMismatches(items: ContainerItem[]): number { + return items.filter((item) => !item.imageMatch).length; +}