fix(server): isolate paseo.json parse failures so they don't block fetch_workspaces
A malformed paseo.json (e.g. git-conflict markers) in any registered
workspace caused readPaseoConfig to throw, which propagated through
getScriptConfigs -> buildWorkspaceScriptPayloads ->
Session.describeWorkspaceRecord and failed the entire
fetch_workspaces_request with code=fetch_workspaces_failed. The client
showed no workspaces at all.
Reshape: separate loading (mechanism) from policy (per-caller).
- readPaseoConfig now returns a discriminated result
({ ok: true; config } | { ok: false; configPath; error }). Pure load,
no logging, no opinions.
- Workspace projection owns its own policy via readPaseoConfigForProjection
in script-status-projection.ts: warn via pino with configPath +
workspaceDirectory + err, treat workspace as having no scripts, keep the
list alive.
- spawnWorkspaceScript throws with cause so the existing
handleStartWorkspaceScriptRequest catch logs it once with full context.
- The setup/teardown/terminal wrappers retain their pre-existing silent
degradation on missing/broken config.
- getScriptConfigs takes a parsed PaseoConfig | null instead of repoRoot
so the projection can pass the result through without double-reading.
Closes https://github.com/getpaseo/paseo/issues/598
This commit is contained in:
@@ -269,6 +269,7 @@ export async function createPaseoDaemon(
|
||||
daemonPort: () => (boundListenTarget?.type === "tcp" ? boundListenTarget.port : null),
|
||||
resolveWorkspaceDirectory: async (workspaceId) =>
|
||||
(await workspaceRegistry?.get(workspaceId))?.cwd ?? null,
|
||||
logger,
|
||||
}),
|
||||
});
|
||||
const handleBranchChange = createBranchChangeRouteHandler({
|
||||
|
||||
@@ -11,6 +11,9 @@ import {
|
||||
import { WorkspaceScriptPayloadSchema } from "../shared/messages.js";
|
||||
import type { ScriptHealthState } from "./script-health-monitor.js";
|
||||
import { WorkspaceScriptRuntimeStore } from "./workspace-script-runtime-store.js";
|
||||
import { readPaseoConfig } from "../utils/worktree.js";
|
||||
import type { PaseoConfig } from "../utils/paseo-config-schema.js";
|
||||
import { createTestLogger } from "../test-utils/test-logger.js";
|
||||
|
||||
function createWorkspaceRepo(options?: {
|
||||
branchName?: string;
|
||||
@@ -41,13 +44,21 @@ function createWorkspaceRepo(options?: {
|
||||
function buildPayloads(input: {
|
||||
workspaceId: string;
|
||||
workspaceDirectory: string;
|
||||
paseoConfig?: PaseoConfig | null;
|
||||
routeStore: ScriptRouteStore;
|
||||
runtimeStore: WorkspaceScriptRuntimeStore;
|
||||
daemonPort: number | null;
|
||||
gitMetadata?: { projectSlug: string; currentBranch: string | null };
|
||||
resolveHealth?: (hostname: string) => ScriptHealthState | null;
|
||||
}) {
|
||||
return buildWorkspaceScriptPayloads(input);
|
||||
const paseoConfig =
|
||||
input.paseoConfig !== undefined ? input.paseoConfig : loadConfig(input.workspaceDirectory);
|
||||
return buildWorkspaceScriptPayloads({ ...input, paseoConfig });
|
||||
}
|
||||
|
||||
function loadConfig(repoRoot: string): PaseoConfig | null {
|
||||
const result = readPaseoConfig(repoRoot);
|
||||
return result.ok ? result.config : null;
|
||||
}
|
||||
|
||||
describe("script-status-projection", () => {
|
||||
@@ -369,6 +380,67 @@ describe("script-status-projection", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("readPaseoConfig fails with configPath and error when paseo.json is malformed", () => {
|
||||
const workspace = createWorkspaceRepo();
|
||||
const configPath = path.join(workspace.repoDir, "paseo.json");
|
||||
writeFileSync(
|
||||
configPath,
|
||||
'{\n<<<<<<< HEAD\n "scripts": {}\n=======\n "scripts": {}\n>>>>>>> origin/main\n}\n',
|
||||
);
|
||||
|
||||
try {
|
||||
const result = readPaseoConfig(workspace.repoDir);
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) throw new Error("unreachable");
|
||||
expect(result.configPath).toBe(configPath);
|
||||
expect(result.error).toBeInstanceOf(SyntaxError);
|
||||
} finally {
|
||||
workspace.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it("buildWorkspaceScriptPayloads given paseoConfig=null still surfaces orphaned runtime scripts", () => {
|
||||
const workspaceId = "workspace-null-config";
|
||||
const workspace = createWorkspaceRepo();
|
||||
const routeStore = new ScriptRouteStore();
|
||||
const runtimeStore = new WorkspaceScriptRuntimeStore();
|
||||
runtimeStore.set({
|
||||
workspaceId,
|
||||
scriptName: "typecheck",
|
||||
type: "script",
|
||||
lifecycle: "running",
|
||||
terminalId: "term-typecheck",
|
||||
exitCode: null,
|
||||
});
|
||||
|
||||
try {
|
||||
expect(
|
||||
buildPayloads({
|
||||
workspaceId,
|
||||
workspaceDirectory: workspace.repoDir,
|
||||
paseoConfig: null,
|
||||
routeStore,
|
||||
runtimeStore,
|
||||
daemonPort: 6767,
|
||||
}),
|
||||
).toEqual([
|
||||
{
|
||||
scriptName: "typecheck",
|
||||
type: "script",
|
||||
hostname: "typecheck",
|
||||
port: null,
|
||||
proxyUrl: null,
|
||||
lifecycle: "running",
|
||||
health: null,
|
||||
exitCode: null,
|
||||
terminalId: "term-typecheck",
|
||||
},
|
||||
]);
|
||||
} finally {
|
||||
workspace.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it("createScriptStatusEmitter overlays health onto the projected workspace script list", async () => {
|
||||
const workspaceId = "workspace-emitter";
|
||||
const workspace = createWorkspaceRepo({
|
||||
@@ -405,6 +477,7 @@ describe("script-status-projection", () => {
|
||||
daemonPort: 6767,
|
||||
resolveWorkspaceDirectory: async (requestedWorkspaceId) =>
|
||||
requestedWorkspaceId === "workspace-emitter" ? workspace.repoDir : null,
|
||||
logger: createTestLogger(),
|
||||
});
|
||||
|
||||
try {
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
import type { Logger } from "pino";
|
||||
import type {
|
||||
ScriptStatusUpdateMessage,
|
||||
SessionOutboundMessage,
|
||||
WorkspaceScriptPayload,
|
||||
} from "../shared/messages.js";
|
||||
import type { PaseoConfig } from "../utils/paseo-config-schema.js";
|
||||
import { buildScriptHostname } from "../utils/script-hostname.js";
|
||||
import { getScriptConfigs, isServiceScript } from "../utils/worktree.js";
|
||||
import { getScriptConfigs, isServiceScript, readPaseoConfig } from "../utils/worktree.js";
|
||||
import { deriveProjectSlug } from "./workspace-git-metadata.js";
|
||||
import type { ScriptHealthEntry, ScriptHealthState } from "./script-health-monitor.js";
|
||||
import type { ScriptRouteStore } from "./script-proxy.js";
|
||||
@@ -17,6 +19,7 @@ interface SessionEmitter {
|
||||
interface BuildWorkspaceScriptPayloadsOptions {
|
||||
workspaceId: string;
|
||||
workspaceDirectory: string;
|
||||
paseoConfig: PaseoConfig | null;
|
||||
routeStore: ScriptRouteStore;
|
||||
runtimeStore: WorkspaceScriptRuntimeStore;
|
||||
daemonPort: number | null;
|
||||
@@ -27,6 +30,21 @@ interface BuildWorkspaceScriptPayloadsOptions {
|
||||
resolveHealth?: (hostname: string) => ScriptHealthState | null;
|
||||
}
|
||||
|
||||
export function readPaseoConfigForProjection(
|
||||
workspaceDirectory: string,
|
||||
logger: Logger,
|
||||
): PaseoConfig | null {
|
||||
const result = readPaseoConfig(workspaceDirectory);
|
||||
if (result.ok) {
|
||||
return result.config;
|
||||
}
|
||||
logger.warn(
|
||||
{ configPath: result.configPath, workspaceDirectory, err: result.error },
|
||||
"Failed to parse paseo.json; treating workspace as having no scripts",
|
||||
);
|
||||
return null;
|
||||
}
|
||||
|
||||
function resolveDaemonPort(daemonPort: number | null | (() => number | null)): number | null {
|
||||
if (typeof daemonPort === "function") {
|
||||
return daemonPort();
|
||||
@@ -136,7 +154,7 @@ export function buildWorkspaceScriptPayloads(
|
||||
const workspaceDirectory = options.workspaceDirectory;
|
||||
const projectSlug = options.gitMetadata?.projectSlug ?? deriveProjectSlug(workspaceDirectory);
|
||||
const branchName = options.gitMetadata?.currentBranch ?? null;
|
||||
const scriptConfigs = getScriptConfigs(workspaceDirectory);
|
||||
const scriptConfigs = getScriptConfigs(options.paseoConfig);
|
||||
const runtimeEntries = new Map(
|
||||
options.runtimeStore
|
||||
.listForWorkspace(workspaceId)
|
||||
@@ -193,12 +211,14 @@ export function createScriptStatusEmitter({
|
||||
runtimeStore,
|
||||
daemonPort,
|
||||
resolveWorkspaceDirectory,
|
||||
logger,
|
||||
}: {
|
||||
sessions: () => SessionEmitter[];
|
||||
routeStore: ScriptRouteStore;
|
||||
runtimeStore: WorkspaceScriptRuntimeStore;
|
||||
daemonPort: number | null | (() => number | null);
|
||||
resolveWorkspaceDirectory: (workspaceId: string) => string | null | Promise<string | null>;
|
||||
logger: Logger;
|
||||
}): (workspaceId: string, scripts: ScriptHealthEntry[]) => void {
|
||||
return (workspaceId, scripts) => {
|
||||
void (async () => {
|
||||
@@ -215,6 +235,7 @@ export function createScriptStatusEmitter({
|
||||
const projected = buildWorkspaceScriptPayloads({
|
||||
workspaceId,
|
||||
workspaceDirectory,
|
||||
paseoConfig: readPaseoConfigForProjection(workspaceDirectory, logger),
|
||||
routeStore,
|
||||
runtimeStore,
|
||||
daemonPort: resolvedDaemonPort,
|
||||
|
||||
@@ -77,7 +77,10 @@ import { sendPromptToAgent, unarchiveAgentState } from "./agent/mcp-shared.js";
|
||||
import { experimental_createMCPClient } from "ai";
|
||||
import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js";
|
||||
import type { VoiceCallerContext, VoiceSpeakHandler } from "./voice-types.js";
|
||||
import { buildWorkspaceScriptPayloads } from "./script-status-projection.js";
|
||||
import {
|
||||
buildWorkspaceScriptPayloads,
|
||||
readPaseoConfigForProjection,
|
||||
} from "./script-status-projection.js";
|
||||
import { deriveProjectSlug } from "./workspace-git-metadata.js";
|
||||
import type { ScriptHealthState } from "./script-health-monitor.js";
|
||||
import { spawnWorkspaceScript } from "./worktree-bootstrap.js";
|
||||
@@ -6069,6 +6072,7 @@ export class Session {
|
||||
? buildWorkspaceScriptPayloads({
|
||||
workspaceId: workspace.workspaceId,
|
||||
workspaceDirectory: workspace.cwd,
|
||||
paseoConfig: readPaseoConfigForProjection(workspace.cwd, this.sessionLogger),
|
||||
routeStore: this.scriptRouteStore,
|
||||
runtimeStore: this.scriptRuntimeStore,
|
||||
daemonPort: this.getDaemonTcpPort?.() ?? null,
|
||||
@@ -7133,6 +7137,7 @@ export class Session {
|
||||
return buildWorkspaceScriptPayloads({
|
||||
workspaceId,
|
||||
workspaceDirectory,
|
||||
paseoConfig: readPaseoConfigForProjection(workspaceDirectory, this.sessionLogger),
|
||||
routeStore: this.scriptRouteStore,
|
||||
runtimeStore: this.scriptRuntimeStore,
|
||||
daemonPort: this.getDaemonTcpPort?.() ?? null,
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
getWorktreeTerminalSpecs,
|
||||
isServiceScript,
|
||||
processCarriageReturns,
|
||||
readPaseoConfig,
|
||||
resolveWorktreeRuntimeEnv,
|
||||
runWorktreeSetupCommands,
|
||||
WorktreeSetupError,
|
||||
@@ -832,7 +833,13 @@ export async function spawnWorkspaceScript(
|
||||
logger,
|
||||
onLifecycleChanged,
|
||||
} = options;
|
||||
const scriptConfigs = getScriptConfigs(repoRoot);
|
||||
const configResult = readPaseoConfig(repoRoot);
|
||||
if (!configResult.ok) {
|
||||
throw new Error(`Failed to parse ${configResult.configPath}`, {
|
||||
cause: configResult.error,
|
||||
});
|
||||
}
|
||||
const scriptConfigs = getScriptConfigs(configResult.config);
|
||||
const config = scriptConfigs.get(scriptName);
|
||||
if (!config) {
|
||||
throw new Error(`Script '${scriptName}' is not configured in paseo.json`);
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
isServiceScript,
|
||||
isPaseoOwnedWorktreeCwd,
|
||||
listPaseoWorktrees,
|
||||
readPaseoConfig,
|
||||
resolveWorktreeRuntimeEnv,
|
||||
type WorktreeSetupCommandProgressEvent,
|
||||
runWorktreeSetupCommands,
|
||||
@@ -18,6 +19,12 @@ import {
|
||||
type CreateWorktreeOptions,
|
||||
type WorktreeConfig,
|
||||
} from "./worktree";
|
||||
import type { PaseoConfig } from "./paseo-config-schema.js";
|
||||
|
||||
function loadConfigForTest(repoRoot: string): PaseoConfig | null {
|
||||
const result = readPaseoConfig(repoRoot);
|
||||
return result.ok ? result.config : null;
|
||||
}
|
||||
import { getPaseoWorktreeMetadataPath } from "./worktree-metadata.js";
|
||||
import { execSync } from "child_process";
|
||||
import {
|
||||
@@ -707,7 +714,7 @@ describe.skipIf(process.platform === "win32")("createWorktree", () => {
|
||||
}),
|
||||
);
|
||||
|
||||
const scriptConfigs = getScriptConfigs(repoDir);
|
||||
const scriptConfigs = getScriptConfigs(loadConfigForTest(repoDir));
|
||||
const typecheck = scriptConfigs.get("typecheck");
|
||||
|
||||
expect(typecheck).toEqual({
|
||||
@@ -731,7 +738,7 @@ describe.skipIf(process.platform === "win32")("createWorktree", () => {
|
||||
}),
|
||||
);
|
||||
|
||||
const scriptConfigs = getScriptConfigs(repoDir);
|
||||
const scriptConfigs = getScriptConfigs(loadConfigForTest(repoDir));
|
||||
const server = scriptConfigs.get("server");
|
||||
|
||||
expect(server).toEqual({
|
||||
@@ -771,7 +778,7 @@ describe.skipIf(process.platform === "win32")("createWorktree", () => {
|
||||
}),
|
||||
);
|
||||
|
||||
expect(getScriptConfigs(repoDir)).toEqual(
|
||||
expect(getScriptConfigs(loadConfigForTest(repoDir))).toEqual(
|
||||
new Map([
|
||||
["valid", { command: "npm run valid" }],
|
||||
["invalidType", { command: "npm run worker" }],
|
||||
|
||||
@@ -8,7 +8,7 @@ import { createHash } from "node:crypto";
|
||||
import * as pty from "node-pty";
|
||||
import stripAnsi from "strip-ansi";
|
||||
import { buildStringCommandShellInvocation } from "./string-command-shell.js";
|
||||
import { readPaseoConfigJson } from "./paseo-config-file.js";
|
||||
import { readPaseoConfigJson, resolvePaseoConfigPath } from "./paseo-config-file.js";
|
||||
export {
|
||||
PaseoConfigRawSchema,
|
||||
PaseoLifecycleCommandRawSchema,
|
||||
@@ -197,31 +197,35 @@ export class UnknownBranchError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
export function readPaseoConfig(repoRoot: string): PaseoConfig | null {
|
||||
export type ReadPaseoConfigResult =
|
||||
| { ok: true; config: PaseoConfig | null }
|
||||
| { ok: false; configPath: string; error: unknown };
|
||||
|
||||
export function readPaseoConfig(repoRoot: string): ReadPaseoConfigResult {
|
||||
try {
|
||||
const json = readPaseoConfigJson(repoRoot);
|
||||
if (json === null) {
|
||||
return null;
|
||||
return { ok: true, config: null };
|
||||
}
|
||||
return PaseoConfigSchema.parse(json);
|
||||
} catch {
|
||||
throw new Error(`Failed to parse paseo.json`);
|
||||
return { ok: true, config: PaseoConfigSchema.parse(json) };
|
||||
} catch (error) {
|
||||
return { ok: false, configPath: resolvePaseoConfigPath(repoRoot), error };
|
||||
}
|
||||
}
|
||||
|
||||
export function getWorktreeSetupCommands(repoRoot: string): string[] {
|
||||
const config = readPaseoConfig(repoRoot);
|
||||
return config?.worktree?.setup ?? [];
|
||||
const result = readPaseoConfig(repoRoot);
|
||||
return result.ok ? (result.config?.worktree?.setup ?? []) : [];
|
||||
}
|
||||
|
||||
export function getWorktreeTeardownCommands(repoRoot: string): string[] {
|
||||
const config = readPaseoConfig(repoRoot);
|
||||
return config?.worktree?.teardown ?? [];
|
||||
const result = readPaseoConfig(repoRoot);
|
||||
return result.ok ? (result.config?.worktree?.teardown ?? []) : [];
|
||||
}
|
||||
|
||||
export function getWorktreeTerminalSpecs(repoRoot: string): WorktreeTerminalConfig[] {
|
||||
const config = readPaseoConfig(repoRoot);
|
||||
const terminals = config?.worktree?.terminals;
|
||||
const result = readPaseoConfig(repoRoot);
|
||||
const terminals = result.ok ? result.config?.worktree?.terminals : undefined;
|
||||
if (!Array.isArray(terminals) || terminals.length === 0) {
|
||||
return [];
|
||||
}
|
||||
@@ -254,8 +258,7 @@ export function getWorktreeTerminalSpecs(repoRoot: string): WorktreeTerminalConf
|
||||
return specs;
|
||||
}
|
||||
|
||||
export function getScriptConfigs(repoRoot: string): Map<string, ScriptConfig> {
|
||||
const config = readPaseoConfig(repoRoot);
|
||||
export function getScriptConfigs(config: PaseoConfig | null): Map<string, ScriptConfig> {
|
||||
const scripts = config?.scripts;
|
||||
if (!scripts || typeof scripts !== "object") {
|
||||
return new Map();
|
||||
|
||||
Reference in New Issue
Block a user