mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-04-28 03:49:31 +00:00
refactor: eliminate process-global mock.module() pollution in tests (#2490)
Replace mock.module() calls with dependency injection to prevent cross-file test pollution in Bun's shared worker process. Changes: - orchestrate.ts: add getApiKey to OrchestrationOptions - billing-guidance.ts: add injectable BillingGuidanceDeps parameter - delete.ts: add optional deleteHandler parameter to confirmAndDelete - update.ts: add UpdateOptions with injectable runUpdate function - sprite.ts: add optional spawnFn parameter to interactiveSession - Remove unnecessary oauth mocks from junie-agent and do-snapshot tests Only @clack/prompts mock (shared via test-helpers.ts) and do-payment-warning.test.ts (safe spread pattern) remain. Co-authored-by: lab <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6439cba58c
commit
5031d84e6c
13 changed files with 223 additions and 193 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@openrouter/spawn",
|
||||
"version": "0.16.8",
|
||||
"version": "0.16.9",
|
||||
"type": "module",
|
||||
"bin": {
|
||||
"spawn": "cli.js"
|
||||
|
|
|
|||
|
|
@ -1,19 +1,22 @@
|
|||
import { beforeEach, describe, expect, it, mock, spyOn } from "bun:test";
|
||||
import type { BillingGuidanceDeps } from "../shared/billing-guidance";
|
||||
|
||||
import { beforeEach, describe, expect, it, mock, spyOn } from "bun:test";
|
||||
import { handleBillingError, isBillingError, showNonBillingError } from "../shared/billing-guidance";
|
||||
|
||||
// ── Mock deps (injected via DI, not mock.module) ──────────────────────────
|
||||
|
||||
// Mock the ui module before importing billing-guidance
|
||||
const mockOpenBrowser = mock(() => {});
|
||||
const mockPrompt = mock(() => Promise.resolve(""));
|
||||
|
||||
mock.module("../shared/ui", () => ({
|
||||
logError: mock(() => {}),
|
||||
logInfo: mock(() => {}),
|
||||
logStep: mock(() => {}),
|
||||
logWarn: mock(() => {}),
|
||||
openBrowser: mockOpenBrowser,
|
||||
prompt: mockPrompt,
|
||||
}));
|
||||
|
||||
const { handleBillingError, isBillingError, showNonBillingError } = await import("../shared/billing-guidance");
|
||||
function createMockDeps(): BillingGuidanceDeps {
|
||||
return {
|
||||
logInfo: mock(() => {}),
|
||||
logStep: mock(() => {}),
|
||||
logWarn: mock(() => {}),
|
||||
openBrowser: mockOpenBrowser,
|
||||
prompt: mockPrompt,
|
||||
};
|
||||
}
|
||||
|
||||
describe("isBillingError", () => {
|
||||
describe("hetzner", () => {
|
||||
|
|
@ -100,24 +103,26 @@ describe("handleBillingError", () => {
|
|||
|
||||
it("opens billing URL and returns true when user presses Enter", async () => {
|
||||
mockPrompt.mockImplementation(() => Promise.resolve(""));
|
||||
const result = await handleBillingError("hetzner");
|
||||
const deps = createMockDeps();
|
||||
const result = await handleBillingError("hetzner", deps);
|
||||
expect(result).toBe(true);
|
||||
expect(mockOpenBrowser).toHaveBeenCalledWith("https://console.hetzner.cloud/");
|
||||
expect(deps.openBrowser).toHaveBeenCalledWith("https://console.hetzner.cloud/");
|
||||
stderrSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("returns false when prompt throws (Ctrl+C)", async () => {
|
||||
mockPrompt.mockImplementation(() => Promise.reject(new Error("cancelled")));
|
||||
const result = await handleBillingError("digitalocean");
|
||||
const result = await handleBillingError("digitalocean", createMockDeps());
|
||||
expect(result).toBe(false);
|
||||
stderrSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("works for clouds without billing URL", async () => {
|
||||
mockPrompt.mockImplementation(() => Promise.resolve(""));
|
||||
const result = await handleBillingError("unknown");
|
||||
const deps = createMockDeps();
|
||||
const result = await handleBillingError("unknown", deps);
|
||||
expect(result).toBe(true);
|
||||
expect(mockOpenBrowser).not.toHaveBeenCalled();
|
||||
expect(deps.openBrowser).not.toHaveBeenCalled();
|
||||
stderrSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
|
@ -130,10 +135,15 @@ describe("showNonBillingError", () => {
|
|||
});
|
||||
|
||||
it("does not throw", () => {
|
||||
const deps = createMockDeps();
|
||||
expect(() => {
|
||||
showNonBillingError("hetzner", [
|
||||
"Server limit reached for your account",
|
||||
]);
|
||||
showNonBillingError(
|
||||
"hetzner",
|
||||
[
|
||||
"Server limit reached for your account",
|
||||
],
|
||||
deps,
|
||||
);
|
||||
}).not.toThrow();
|
||||
stderrSpy.mockRestore();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,31 +8,17 @@ const VERSION = pkg.version;
|
|||
/**
|
||||
* Tests for cmdUpdate (commands/update.ts).
|
||||
*
|
||||
* Script download/execution tests live in:
|
||||
* - download-and-failure.test.ts (failure paths: both-404, both-500, network errors)
|
||||
* - cmdrun-happy-path.test.ts (success paths: primary/fallback download, history, env vars)
|
||||
* Uses dependency injection (UpdateOptions.runUpdate) instead of mock.module
|
||||
* for node:child_process to avoid process-global mock pollution.
|
||||
*/
|
||||
|
||||
const { spinnerStart: mockSpinnerStart, spinnerStop: mockSpinnerStop } = mockClackPrompts();
|
||||
|
||||
// Mock node:child_process to prevent real subprocess calls in tests:
|
||||
// - execSync: used by performUpdate() to run curl|bash install — without this mock,
|
||||
// "should handle update failure gracefully" downloads the real install script from
|
||||
// the network, causing a 58s timeout under full-suite concurrency (CLAUDE.md violation).
|
||||
// - spawnSync: used by spawnBash() to run downloaded scripts — returns exit code 0
|
||||
// so callers see a successful execution.
|
||||
mock.module("node:child_process", () => ({
|
||||
execSync: mock(() => {}),
|
||||
execFileSync: mock(() => {}),
|
||||
spawnSync: mock(() => ({
|
||||
status: 0,
|
||||
signal: null,
|
||||
error: null,
|
||||
})),
|
||||
}));
|
||||
// ── Import commands directly (no mock.module needed) ──────────────────────
|
||||
import { cmdUpdate } from "../commands/index.js";
|
||||
|
||||
// Import commands after mock setup
|
||||
const { cmdUpdate } = await import("../commands/index.js");
|
||||
/** No-op runUpdate to prevent real subprocess calls in tests. */
|
||||
const mockRunUpdate = mock(() => {});
|
||||
|
||||
describe("cmdUpdate", () => {
|
||||
let consoleMocks: ReturnType<typeof createConsoleMocks>;
|
||||
|
|
@ -43,6 +29,7 @@ describe("cmdUpdate", () => {
|
|||
consoleMocks = createConsoleMocks();
|
||||
mockSpinnerStart.mockClear();
|
||||
mockSpinnerStop.mockClear();
|
||||
mockRunUpdate.mockClear();
|
||||
|
||||
processExitSpy = spyOn(process, "exit").mockImplementation(() => {
|
||||
throw new Error("process.exit");
|
||||
|
|
@ -67,7 +54,9 @@ describe("cmdUpdate", () => {
|
|||
});
|
||||
});
|
||||
|
||||
await cmdUpdate();
|
||||
await cmdUpdate({
|
||||
runUpdate: mockRunUpdate,
|
||||
});
|
||||
|
||||
expect(mockSpinnerStart).toHaveBeenCalled();
|
||||
expect(mockSpinnerStop).toHaveBeenCalled();
|
||||
|
|
@ -86,7 +75,9 @@ describe("cmdUpdate", () => {
|
|||
});
|
||||
});
|
||||
|
||||
await cmdUpdate();
|
||||
await cmdUpdate({
|
||||
runUpdate: mockRunUpdate,
|
||||
});
|
||||
|
||||
expect(mockSpinnerStart).toHaveBeenCalled();
|
||||
// Should show update message with version transition
|
||||
|
|
@ -102,7 +93,9 @@ describe("cmdUpdate", () => {
|
|||
}),
|
||||
);
|
||||
|
||||
await cmdUpdate();
|
||||
await cmdUpdate({
|
||||
runUpdate: mockRunUpdate,
|
||||
});
|
||||
|
||||
expect(mockSpinnerStart).toHaveBeenCalled();
|
||||
// Should show failed message
|
||||
|
|
@ -118,7 +111,9 @@ describe("cmdUpdate", () => {
|
|||
throw new TypeError("Failed to fetch");
|
||||
});
|
||||
|
||||
await cmdUpdate();
|
||||
await cmdUpdate({
|
||||
runUpdate: mockRunUpdate,
|
||||
});
|
||||
|
||||
expect(mockSpinnerStart).toHaveBeenCalled();
|
||||
const stopCalls = mockSpinnerStop.mock.calls.map((c: unknown[]) => c.join(" "));
|
||||
|
|
@ -135,9 +130,14 @@ describe("cmdUpdate", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// cmdUpdate now runs execSync which will fail in test env
|
||||
// The function catches errors internally, so it should not throw
|
||||
await cmdUpdate();
|
||||
// Mock runUpdate that throws to simulate failure
|
||||
const failingRunUpdate = mock(() => {
|
||||
throw new Error("curl failed");
|
||||
});
|
||||
|
||||
await cmdUpdate({
|
||||
runUpdate: failingRunUpdate,
|
||||
});
|
||||
|
||||
// Should show the update version in spinner stop
|
||||
const stopCalls = mockSpinnerStop.mock.calls.map((c: unknown[]) => c.join(" "));
|
||||
|
|
@ -154,7 +154,9 @@ describe("cmdUpdate", () => {
|
|||
),
|
||||
);
|
||||
|
||||
await cmdUpdate();
|
||||
await cmdUpdate({
|
||||
runUpdate: mockRunUpdate,
|
||||
});
|
||||
|
||||
const startCalls = mockSpinnerStart.mock.calls.map((c: unknown[]) => c.join(" "));
|
||||
expect(startCalls.some((msg: string) => msg.includes("Checking"))).toBe(true);
|
||||
|
|
@ -170,7 +172,9 @@ describe("cmdUpdate", () => {
|
|||
});
|
||||
});
|
||||
|
||||
await cmdUpdate();
|
||||
await cmdUpdate({
|
||||
runUpdate: mockRunUpdate,
|
||||
});
|
||||
|
||||
// cmdUpdate now uses s.stop() with version info instead of s.message()
|
||||
const stopCalls = mockSpinnerStop.mock.calls.map((c: unknown[]) => c.join(" "));
|
||||
|
|
|
|||
|
|
@ -2,11 +2,17 @@
|
|||
* delete-spinner.test.ts — Tests that confirmAndDelete feeds cloud destroy
|
||||
* stderr output into the spinner message, then clears the spinner and shows
|
||||
* the final result via p.log.success/error with the last stderr message.
|
||||
*
|
||||
* Uses dependency injection (deleteHandler param) instead of mock.module
|
||||
* to avoid process-global mock pollution.
|
||||
*/
|
||||
|
||||
import type { SpawnRecord } from "../history.js";
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test";
|
||||
import { mkdirSync, rmSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { markRecordDeleted } from "../history.js";
|
||||
import { mockClackPrompts } from "./test-helpers.js";
|
||||
|
||||
// ── Mock @clack/prompts (must be before importing the module under test) ──
|
||||
|
|
@ -14,24 +20,12 @@ const clack = mockClackPrompts({
|
|||
confirm: mock(async () => true),
|
||||
});
|
||||
|
||||
// ── Mock only hetzner (the cloud used by test records) ──────────────────
|
||||
// Other cloud modules are left un-mocked to avoid process-global pollution
|
||||
// (mock.module is global in Bun and would break other test files).
|
||||
// They import fine but are never called since test records use cloud: "hetzner".
|
||||
const mockHetznerDestroy = mock(() => Promise.resolve());
|
||||
mock.module("../hetzner/hetzner.js", () => ({
|
||||
ensureHcloudToken: mock(() => Promise.resolve()),
|
||||
destroyServer: mockHetznerDestroy,
|
||||
}));
|
||||
|
||||
// History uses real module — SPAWN_HOME is pointed at a temp dir in beforeEach
|
||||
|
||||
// ── Import the module under test ──────────────────────────────────────────
|
||||
const { confirmAndDelete } = await import("../commands/delete.js");
|
||||
// ── Import the module under test (no mock.module needed) ──────────────────
|
||||
import { confirmAndDelete } from "../commands/delete.js";
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
function makeRecord(cloud: string, serverName: string) {
|
||||
function makeRecord(cloud: string, serverName: string): SpawnRecord {
|
||||
return {
|
||||
id: "test-id",
|
||||
agent: "claude",
|
||||
|
|
@ -46,6 +40,19 @@ function makeRecord(cloud: string, serverName: string) {
|
|||
};
|
||||
}
|
||||
|
||||
/** Create a mock deleteHandler that writes to stderr (simulating cloud output). */
|
||||
function createMockDeleteHandler(stderrLines: string[], shouldSucceed = true) {
|
||||
return mock(async (record: SpawnRecord): Promise<boolean> => {
|
||||
for (const line of stderrLines) {
|
||||
process.stderr.write(line);
|
||||
}
|
||||
if (shouldSucceed) {
|
||||
markRecordDeleted(record);
|
||||
}
|
||||
return shouldSucceed;
|
||||
});
|
||||
}
|
||||
|
||||
// ── Tests ─────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("confirmAndDelete spinner behavior", () => {
|
||||
|
|
@ -67,7 +74,6 @@ describe("confirmAndDelete spinner behavior", () => {
|
|||
clack.spinnerClear.mockClear();
|
||||
clack.logSuccess.mockClear();
|
||||
clack.logError.mockClear();
|
||||
mockHetznerDestroy.mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
|
@ -83,14 +89,13 @@ describe("confirmAndDelete spinner behavior", () => {
|
|||
});
|
||||
|
||||
it("feeds stderr output from destroy into spinner.message()", async () => {
|
||||
// Simulate a cloud destroy function that writes progress to stderr
|
||||
mockHetznerDestroy.mockImplementation(async () => {
|
||||
process.stderr.write("\x1b[36mDestroying Hetzner server srv-123...\x1b[0m\n");
|
||||
process.stderr.write("\x1b[32mServer srv-123 destroyed\x1b[0m\n");
|
||||
});
|
||||
const handler = createMockDeleteHandler([
|
||||
"\x1b[36mDestroying Hetzner server srv-123...\x1b[0m\n",
|
||||
"\x1b[32mServer srv-123 destroyed\x1b[0m\n",
|
||||
]);
|
||||
|
||||
const record = makeRecord("hetzner", "srv-123");
|
||||
const result = await confirmAndDelete(record, null);
|
||||
const result = await confirmAndDelete(record, null, handler);
|
||||
|
||||
expect(result).toBe(true);
|
||||
|
||||
|
|
@ -101,25 +106,25 @@ describe("confirmAndDelete spinner behavior", () => {
|
|||
});
|
||||
|
||||
it("calls spinner.clear() instead of spinner.stop()", async () => {
|
||||
mockHetznerDestroy.mockImplementation(async () => {
|
||||
process.stderr.write("Server srv-123 destroyed\n");
|
||||
});
|
||||
const handler = createMockDeleteHandler([
|
||||
"Server srv-123 destroyed\n",
|
||||
]);
|
||||
|
||||
const record = makeRecord("hetzner", "srv-123");
|
||||
await confirmAndDelete(record, null);
|
||||
await confirmAndDelete(record, null, handler);
|
||||
|
||||
expect(clack.spinnerClear).toHaveBeenCalledTimes(1);
|
||||
expect(clack.spinnerStop).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows success with last stderr message as detail", async () => {
|
||||
mockHetznerDestroy.mockImplementation(async () => {
|
||||
process.stderr.write("Destroying Hetzner server srv-123...\n");
|
||||
process.stderr.write("Server srv-123 destroyed\n");
|
||||
});
|
||||
const handler = createMockDeleteHandler([
|
||||
"Destroying Hetzner server srv-123...\n",
|
||||
"Server srv-123 destroyed\n",
|
||||
]);
|
||||
|
||||
const record = makeRecord("hetzner", "srv-123");
|
||||
await confirmAndDelete(record, null);
|
||||
await confirmAndDelete(record, null, handler);
|
||||
|
||||
expect(clack.logSuccess).toHaveBeenCalledTimes(1);
|
||||
const msg = clack.logSuccess.mock.calls[0][0];
|
||||
|
|
@ -128,13 +133,15 @@ describe("confirmAndDelete spinner behavior", () => {
|
|||
});
|
||||
|
||||
it("shows error with detail on delete failure", async () => {
|
||||
mockHetznerDestroy.mockImplementation(async () => {
|
||||
process.stderr.write("Connection refused\n");
|
||||
throw new Error("API timeout");
|
||||
});
|
||||
const handler = createMockDeleteHandler(
|
||||
[
|
||||
"Connection refused\n",
|
||||
],
|
||||
false,
|
||||
);
|
||||
|
||||
const record = makeRecord("hetzner", "srv-123");
|
||||
const result = await confirmAndDelete(record, null);
|
||||
const result = await confirmAndDelete(record, null, handler);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(clack.spinnerClear).toHaveBeenCalledTimes(1);
|
||||
|
|
@ -144,12 +151,12 @@ describe("confirmAndDelete spinner behavior", () => {
|
|||
it("restores process.stderr.write after delete", async () => {
|
||||
const origWrite = process.stderr.write;
|
||||
|
||||
mockHetznerDestroy.mockImplementation(async () => {
|
||||
process.stderr.write("done\n");
|
||||
});
|
||||
const handler = createMockDeleteHandler([
|
||||
"done\n",
|
||||
]);
|
||||
|
||||
const record = makeRecord("hetzner", "srv-123");
|
||||
await confirmAndDelete(record, null);
|
||||
await confirmAndDelete(record, null, handler);
|
||||
|
||||
expect(process.stderr.write).toBe(origWrite);
|
||||
});
|
||||
|
|
@ -157,23 +164,23 @@ describe("confirmAndDelete spinner behavior", () => {
|
|||
it("restores process.stderr.write even on error", async () => {
|
||||
const origWrite = process.stderr.write;
|
||||
|
||||
mockHetznerDestroy.mockImplementation(async () => {
|
||||
const handler = mock(async () => {
|
||||
process.stderr.write("boom\n");
|
||||
throw new Error("kaboom");
|
||||
});
|
||||
|
||||
const record = makeRecord("hetzner", "srv-123");
|
||||
await confirmAndDelete(record, null);
|
||||
await confirmAndDelete(record, null, handler);
|
||||
|
||||
expect(process.stderr.write).toBe(origWrite);
|
||||
});
|
||||
|
||||
it("works with no stderr output from destroy", async () => {
|
||||
// Destroy succeeds silently
|
||||
mockHetznerDestroy.mockImplementation(async () => {});
|
||||
const handler = createMockDeleteHandler([]);
|
||||
|
||||
const record = makeRecord("hetzner", "srv-123");
|
||||
const result = await confirmAndDelete(record, null);
|
||||
const result = await confirmAndDelete(record, null, handler);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(clack.spinnerClear).toHaveBeenCalledTimes(1);
|
||||
|
|
|
|||
|
|
@ -7,13 +7,8 @@
|
|||
|
||||
import { afterAll, afterEach, describe, expect, it, mock } from "bun:test";
|
||||
|
||||
// ── Mock oauth (prevent interactive prompts) ──────────────────────────────
|
||||
|
||||
mock.module("../shared/oauth", () => ({
|
||||
getOrPromptApiKey: mock(() => Promise.resolve("sk-test")),
|
||||
}));
|
||||
|
||||
// ── Import under test ─────────────────────────────────────────────────────
|
||||
// digitalocean.ts only imports a CSS constant from oauth, so no mock needed.
|
||||
|
||||
const { findSpawnSnapshot } = await import("../digitalocean/digitalocean");
|
||||
|
||||
|
|
|
|||
|
|
@ -18,13 +18,8 @@ beforeEach(() => {
|
|||
stderrSpy = spyOn(process.stderr, "write").mockImplementation(() => true);
|
||||
});
|
||||
|
||||
// ── Mock oauth to avoid interactive prompts ──────────────────────────────────
|
||||
|
||||
mock.module("../shared/oauth", () => ({
|
||||
getOrPromptApiKey: mock(() => Promise.resolve("sk-or-v1-test-key")),
|
||||
}));
|
||||
|
||||
// ── Import module under test ──────────────────────────────────────────────────
|
||||
// agent-setup.ts doesn't import oauth, so no mock needed.
|
||||
|
||||
const { createCloudAgents } = await import("../shared/agent-setup");
|
||||
|
||||
|
|
|
|||
|
|
@ -5,9 +5,8 @@
|
|||
* handles optional hooks (preProvision, configure, preLaunch), model selection,
|
||||
* and restart loop wrapping for non-local clouds.
|
||||
*
|
||||
* IMPORTANT: We only mock ../shared/oauth (not ../shared/agent-setup or
|
||||
* ../shared/ui) because Bun's mock.module is process-global and would
|
||||
* bleed into with-retry-result.test.ts which tests the real wrapSshCall.
|
||||
* Uses dependency injection (OrchestrationOptions.getApiKey) instead of
|
||||
* mock.module to avoid process-global mock pollution.
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test";
|
||||
|
|
@ -16,21 +15,15 @@ import { join } from "node:path";
|
|||
import { asyncTryCatch, tryCatch } from "@openrouter/spawn-shared";
|
||||
import { isNumber } from "../shared/type-guards.js";
|
||||
|
||||
// ── Mock oauth + tarball (needed to avoid interactive prompts / network) ──
|
||||
|
||||
const mockGetOrPromptApiKey = mock(() => Promise.resolve("sk-or-v1-test-key"));
|
||||
|
||||
mock.module("../shared/oauth", () => ({
|
||||
getOrPromptApiKey: mockGetOrPromptApiKey,
|
||||
}));
|
||||
|
||||
// ── Import the real module under test ─────────────────────────────────────
|
||||
|
||||
const { runOrchestration } = await import("../shared/orchestrate");
|
||||
|
||||
import type { AgentConfig } from "../shared/agents";
|
||||
import type { CloudOrchestrator, OrchestrationOptions } from "../shared/orchestrate";
|
||||
|
||||
import { runOrchestration } from "../shared/orchestrate";
|
||||
|
||||
const mockTryTarballInstall = mock(() => Promise.resolve(false));
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────
|
||||
|
|
@ -75,9 +68,10 @@ function createMockAgent(overrides: Partial<AgentConfig> = {}): AgentConfig {
|
|||
};
|
||||
}
|
||||
|
||||
/** Default options that inject the mock tarball function. */
|
||||
/** Default options that inject mock dependencies via DI. */
|
||||
const defaultOpts: OrchestrationOptions = {
|
||||
tryTarball: mockTryTarballInstall,
|
||||
getApiKey: mockGetOrPromptApiKey,
|
||||
};
|
||||
|
||||
/** Run orchestration and catch the process.exit throw. */
|
||||
|
|
|
|||
|
|
@ -6,29 +6,15 @@
|
|||
* - installSpriteKeepAlive() is gracefully non-fatal when download fails
|
||||
* - interactiveSession() wraps the cmd in a session script with keep-alive support
|
||||
*
|
||||
* IMPORTANT: Only mock.module "../shared/ssh" here — NOT "../shared/ui" or
|
||||
* "../shared/paths", as those are shared with other test files and would
|
||||
* cause failures in history.test.ts, paths.test.ts, etc.
|
||||
* Uses dependency injection (spawnFn param) for interactiveSession instead of
|
||||
* mock.module to avoid process-global mock pollution.
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test";
|
||||
|
||||
// ── Mock only ../shared/ssh (not used directly by any other test file) ────────
|
||||
// ── Import module under test directly (no mock.module needed) ────────────────
|
||||
|
||||
const mockSpawnInteractive = mock((_args: string[]) => 0);
|
||||
const mockKillWithTimeout = mock(() => {});
|
||||
const mockSleep = mock(() => Promise.resolve());
|
||||
|
||||
mock.module("../shared/ssh", () => ({
|
||||
spawnInteractive: mockSpawnInteractive,
|
||||
killWithTimeout: mockKillWithTimeout,
|
||||
sleep: mockSleep,
|
||||
SSH_INTERACTIVE_OPTS: [],
|
||||
}));
|
||||
|
||||
// ── Import module under test after mocks ──────────────────────────────────────
|
||||
|
||||
const { installSpriteKeepAlive, interactiveSession } = await import("../sprite/sprite");
|
||||
import { installSpriteKeepAlive, interactiveSession } from "../sprite/sprite";
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -131,6 +117,7 @@ describe("installSpriteKeepAlive", () => {
|
|||
describe("interactiveSession (keep-alive wrapper)", () => {
|
||||
let spawnSyncSpy: ReturnType<typeof spyOn>;
|
||||
let stderrSpy: ReturnType<typeof spyOn>;
|
||||
const mockSpawnInteractive = mock((_args: string[]) => 0);
|
||||
|
||||
beforeEach(() => {
|
||||
mockSpawnInteractive.mockClear();
|
||||
|
|
@ -165,7 +152,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
return 0;
|
||||
});
|
||||
|
||||
await interactiveSession(testCmd);
|
||||
await interactiveSession(testCmd, mockSpawnInteractive);
|
||||
|
||||
expect(capturedSessionScript).toContain(expectedB64);
|
||||
});
|
||||
|
|
@ -180,7 +167,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
return 0;
|
||||
});
|
||||
|
||||
await interactiveSession("my-agent --start");
|
||||
await interactiveSession("my-agent --start", mockSpawnInteractive);
|
||||
|
||||
expect(capturedSessionScript).toContain("sprite-keep-running");
|
||||
expect(capturedSessionScript).toContain("command -v sprite-keep-running");
|
||||
|
|
@ -196,7 +183,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
return 0;
|
||||
});
|
||||
|
||||
await interactiveSession("agent cmd");
|
||||
await interactiveSession("agent cmd", mockSpawnInteractive);
|
||||
|
||||
expect(capturedSessionScript).toContain("mktemp");
|
||||
expect(capturedSessionScript).toContain("base64 -d");
|
||||
|
|
@ -213,7 +200,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
return 0;
|
||||
});
|
||||
|
||||
await interactiveSession("fallback-agent");
|
||||
await interactiveSession("fallback-agent", mockSpawnInteractive);
|
||||
|
||||
expect(capturedSessionScript).toContain("else");
|
||||
expect(capturedSessionScript).toMatch(/else[\s\S]*bash/);
|
||||
|
|
@ -239,7 +226,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
return 0;
|
||||
});
|
||||
|
||||
await interactiveSession(multilineCmd);
|
||||
await interactiveSession(multilineCmd, mockSpawnInteractive);
|
||||
|
||||
expect(capturedSessionScript).toContain(expectedB64);
|
||||
});
|
||||
|
|
@ -253,7 +240,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
return 0;
|
||||
});
|
||||
|
||||
await interactiveSession("agent-cmd");
|
||||
await interactiveSession("agent-cmd", mockSpawnInteractive);
|
||||
|
||||
expect(capturedArgs).toContain("-tty");
|
||||
});
|
||||
|
|
@ -267,7 +254,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
return 0;
|
||||
});
|
||||
|
||||
await interactiveSession("agent-cmd");
|
||||
await interactiveSession("agent-cmd", mockSpawnInteractive);
|
||||
|
||||
expect(capturedArgs).not.toContain("-tty");
|
||||
});
|
||||
|
|
@ -275,7 +262,7 @@ describe("interactiveSession (keep-alive wrapper)", () => {
|
|||
it("returns the exit code from spawnInteractive", async () => {
|
||||
mockSpawnInteractive.mockImplementation(() => 42);
|
||||
|
||||
const exitCode = await interactiveSession("agent-cmd");
|
||||
const exitCode = await interactiveSession("agent-cmd", mockSpawnInteractive);
|
||||
|
||||
expect(exitCode).toBe(42);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -174,7 +174,11 @@ async function execDeleteServer(record: SpawnRecord): Promise<boolean> {
|
|||
}
|
||||
|
||||
/** Prompt for delete confirmation and execute. Returns true if deleted. */
|
||||
export async function confirmAndDelete(record: SpawnRecord, manifest: Manifest | null): Promise<boolean> {
|
||||
export async function confirmAndDelete(
|
||||
record: SpawnRecord,
|
||||
manifest: Manifest | null,
|
||||
deleteHandler?: (record: SpawnRecord) => Promise<boolean>,
|
||||
): Promise<boolean> {
|
||||
const conn = record.connection!;
|
||||
const label = conn.server_name || conn.server_id || conn.ip;
|
||||
const cloudLabel = manifest?.clouds[conn.cloud!]?.name || conn.cloud;
|
||||
|
|
@ -191,7 +195,10 @@ export async function confirmAndDelete(record: SpawnRecord, manifest: Manifest |
|
|||
|
||||
// Ensure credentials before starting the spinner so interactive
|
||||
// prompts (e.g. expired API key entry) don't overlap with it.
|
||||
await ensureDeleteCredentials(record);
|
||||
// Skip when a custom deleteHandler is provided (it manages its own deps).
|
||||
if (!deleteHandler) {
|
||||
await ensureDeleteCredentials(record);
|
||||
}
|
||||
|
||||
const s = p.spinner();
|
||||
s.start(`Deleting ${label}...`);
|
||||
|
|
@ -213,7 +220,8 @@ export async function confirmAndDelete(record: SpawnRecord, manifest: Manifest |
|
|||
return true;
|
||||
};
|
||||
|
||||
const deleteResult = await asyncTryCatch(() => execDeleteServer(record));
|
||||
const deleteFn = deleteHandler ?? execDeleteServer;
|
||||
const deleteResult = await asyncTryCatch(() => deleteFn(record));
|
||||
process.stderr.write = origStderrWrite;
|
||||
|
||||
const success = deleteResult.ok ? deleteResult.data : false;
|
||||
|
|
|
|||
|
|
@ -41,38 +41,40 @@ async function fetchRemoteVersion(): Promise<string> {
|
|||
return data.version;
|
||||
}
|
||||
|
||||
async function performUpdate(_remoteVersion: string): Promise<void> {
|
||||
const r = tryCatch(() => {
|
||||
// Two-step: fetch with --proto '=https', then execute via bash -c
|
||||
// Prevents protocol downgrade on hostile networks (matches update-check.ts pattern)
|
||||
const scriptContent = execFileSync(
|
||||
"curl",
|
||||
[
|
||||
"--proto",
|
||||
"=https",
|
||||
"-fsSL",
|
||||
INSTALL_URL,
|
||||
function defaultRunUpdate(): void {
|
||||
// Two-step: fetch with --proto '=https', then execute via bash -c
|
||||
// Prevents protocol downgrade on hostile networks (matches update-check.ts pattern)
|
||||
const scriptContent = execFileSync(
|
||||
"curl",
|
||||
[
|
||||
"--proto",
|
||||
"=https",
|
||||
"-fsSL",
|
||||
INSTALL_URL,
|
||||
],
|
||||
{
|
||||
encoding: "utf8",
|
||||
stdio: [
|
||||
"pipe",
|
||||
"pipe",
|
||||
"inherit",
|
||||
],
|
||||
{
|
||||
encoding: "utf8",
|
||||
stdio: [
|
||||
"pipe",
|
||||
"pipe",
|
||||
"inherit",
|
||||
],
|
||||
},
|
||||
);
|
||||
execFileSync(
|
||||
"bash",
|
||||
[
|
||||
"-c",
|
||||
scriptContent ?? "",
|
||||
],
|
||||
{
|
||||
stdio: "inherit",
|
||||
},
|
||||
);
|
||||
});
|
||||
},
|
||||
);
|
||||
execFileSync(
|
||||
"bash",
|
||||
[
|
||||
"-c",
|
||||
scriptContent ?? "",
|
||||
],
|
||||
{
|
||||
stdio: "inherit",
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
async function performUpdate(_remoteVersion: string, runUpdate: () => void = defaultRunUpdate): Promise<void> {
|
||||
const r = tryCatch(() => runUpdate());
|
||||
if (r.ok) {
|
||||
console.log();
|
||||
p.log.success("Updated successfully!");
|
||||
|
|
@ -85,7 +87,11 @@ async function performUpdate(_remoteVersion: string): Promise<void> {
|
|||
}
|
||||
}
|
||||
|
||||
export async function cmdUpdate(): Promise<void> {
|
||||
export interface UpdateOptions {
|
||||
runUpdate?: () => void;
|
||||
}
|
||||
|
||||
export async function cmdUpdate(options?: UpdateOptions): Promise<void> {
|
||||
const s = p.spinner();
|
||||
s.start("Checking for updates...");
|
||||
|
||||
|
|
@ -107,5 +113,5 @@ export async function cmdUpdate(): Promise<void> {
|
|||
}
|
||||
|
||||
s.stop(`Updating: v${VERSION} -> v${remoteVersion}`);
|
||||
await performUpdate(remoteVersion);
|
||||
await performUpdate(remoteVersion, options?.runUpdate);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -82,34 +82,51 @@ export function isBillingError(cloud: string, errorMsg: string): boolean {
|
|||
return patterns.some((p) => p.test(errorMsg));
|
||||
}
|
||||
|
||||
/** Dependencies for billing-guidance functions (injectable for testing). */
|
||||
export interface BillingGuidanceDeps {
|
||||
logInfo: typeof logInfo;
|
||||
logStep: typeof logStep;
|
||||
logWarn: typeof logWarn;
|
||||
openBrowser: typeof openBrowser;
|
||||
prompt: typeof prompt;
|
||||
}
|
||||
|
||||
const defaultDeps: BillingGuidanceDeps = {
|
||||
logInfo,
|
||||
logStep,
|
||||
logWarn,
|
||||
openBrowser,
|
||||
prompt,
|
||||
};
|
||||
|
||||
/**
|
||||
* Show billing guidance, open the billing page, and prompt user to retry.
|
||||
* Returns true if user wants to retry, false otherwise.
|
||||
*/
|
||||
export async function handleBillingError(cloud: string): Promise<boolean> {
|
||||
export async function handleBillingError(cloud: string, deps: BillingGuidanceDeps = defaultDeps): Promise<boolean> {
|
||||
const billingUrl = BILLING_URLS[cloud];
|
||||
const steps = SETUP_STEPS[cloud] || [];
|
||||
|
||||
process.stderr.write("\n");
|
||||
logWarn("Your account needs a payment method to create servers.");
|
||||
deps.logWarn("Your account needs a payment method to create servers.");
|
||||
|
||||
if (steps.length > 0) {
|
||||
process.stderr.write("\n");
|
||||
for (const step of steps) {
|
||||
logStep(` ${step}`);
|
||||
deps.logStep(` ${step}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (billingUrl) {
|
||||
process.stderr.write("\n");
|
||||
logStep("Opening your billing page...");
|
||||
openBrowser(billingUrl);
|
||||
deps.logStep("Opening your billing page...");
|
||||
deps.openBrowser(billingUrl);
|
||||
}
|
||||
|
||||
process.stderr.write("\n");
|
||||
return unwrapOr(
|
||||
await asyncTryCatch(async () => {
|
||||
await prompt("Press Enter after adding a payment method to retry (or Ctrl+C to exit)");
|
||||
await deps.prompt("Press Enter after adding a payment method to retry (or Ctrl+C to exit)");
|
||||
return true;
|
||||
}),
|
||||
false,
|
||||
|
|
@ -119,15 +136,19 @@ export async function handleBillingError(cloud: string): Promise<boolean> {
|
|||
/**
|
||||
* Show non-billing error guidance with cloud-specific causes and dashboard link.
|
||||
*/
|
||||
export function showNonBillingError(cloud: string, causes: string[]): void {
|
||||
export function showNonBillingError(
|
||||
cloud: string,
|
||||
causes: string[],
|
||||
deps: Pick<BillingGuidanceDeps, "logInfo" | "logWarn"> = defaultDeps,
|
||||
): void {
|
||||
if (causes.length > 0) {
|
||||
logWarn("Possible causes:");
|
||||
deps.logWarn("Possible causes:");
|
||||
for (const cause of causes) {
|
||||
logWarn(` - ${cause}`);
|
||||
deps.logWarn(` - ${cause}`);
|
||||
}
|
||||
}
|
||||
const billingUrl = BILLING_URLS[cloud];
|
||||
if (billingUrl) {
|
||||
logInfo(`Dashboard: ${billingUrl}`);
|
||||
deps.logInfo(`Dashboard: ${billingUrl}`);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -75,6 +75,7 @@ function wrapWithRestartLoop(cmd: string): string {
|
|||
/** Options for runOrchestration (used in tests to inject mock dependencies). */
|
||||
export interface OrchestrationOptions {
|
||||
tryTarball?: (runner: CloudRunner, agentName: string) => Promise<boolean>;
|
||||
getApiKey?: (agentSlug?: string, cloudSlug?: string) => Promise<string>;
|
||||
}
|
||||
|
||||
export async function runOrchestration(
|
||||
|
|
@ -101,7 +102,8 @@ export async function runOrchestration(
|
|||
|
||||
// 2. Get API key (immediately after cloud auth — before any other prompts
|
||||
// so the "opening browser" message leads directly to OpenRouter OAuth)
|
||||
const apiKey = await getOrPromptApiKey(agentName, cloud.cloudName);
|
||||
const resolveApiKey = options?.getApiKey ?? getOrPromptApiKey;
|
||||
const apiKey = await resolveApiKey(agentName, cloud.cloudName);
|
||||
|
||||
// 3. Pre-provision hooks (e.g., GitHub auth prompt — non-fatal)
|
||||
// Uses try/catch (not guarded) because hooks can throw ANY provider-specific error.
|
||||
|
|
|
|||
|
|
@ -580,7 +580,7 @@ export async function installSpriteKeepAlive(): Promise<void> {
|
|||
* is installed, it wraps the command to keep the sprite alive via Sprite's
|
||||
* /v1/tasks API for the duration of the session.
|
||||
*/
|
||||
export async function interactiveSession(cmd: string): Promise<number> {
|
||||
export async function interactiveSession(cmd: string, spawnFn?: (args: string[]) => number): Promise<number> {
|
||||
const spriteCmd = getSpriteCmd()!;
|
||||
|
||||
// Encode the session command to handle multi-line restart loop scripts safely
|
||||
|
|
@ -624,7 +624,8 @@ export async function interactiveSession(cmd: string): Promise<number> {
|
|||
sessionScript,
|
||||
];
|
||||
|
||||
const exitCode = spawnInteractive(args);
|
||||
const spawn = spawnFn ?? spawnInteractive;
|
||||
const exitCode = spawn(args);
|
||||
|
||||
// Post-session summary
|
||||
process.stderr.write("\n");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue