Fix CLI failure event review issues
This commit is contained in:
parent
fd33d9381e
commit
91a69b372b
|
|
@ -19,7 +19,7 @@ import { join } from "node:path";
|
|||
import { tmpdir } from "node:os";
|
||||
import { parse as parseYaml } from "yaml";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { SessionManager } from "@aoagents/ao-core";
|
||||
import { recordActivityEvent, type SessionManager } from "@aoagents/ao-core";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Hoisted mocks
|
||||
|
|
@ -153,6 +153,7 @@ vi.mock("@aoagents/ao-core", async (importOriginal) => {
|
|||
sweepDaemonChildren: mockSweepDaemonChildren,
|
||||
scanAoOrphans: mockScanAoOrphans,
|
||||
reapAoOrphans: mockReapAoOrphans,
|
||||
recordActivityEvent: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
|
|
@ -336,6 +337,7 @@ beforeEach(async () => {
|
|||
program.exitOverride();
|
||||
registerStart(program);
|
||||
registerStop(program);
|
||||
vi.mocked(recordActivityEvent).mockClear();
|
||||
|
||||
vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
|
@ -505,6 +507,9 @@ function makeProject(overrides: Record<string, unknown> = {}): Record<string, un
|
|||
};
|
||||
}
|
||||
|
||||
const recordedEvents = (): Array<Record<string, unknown>> =>
|
||||
vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0] as Record<string, unknown>);
|
||||
|
||||
/** Mock process.cwd() to return a specific directory (avoids process.chdir in workers). */
|
||||
function mockCwd(dir: string): void {
|
||||
cwdSpy = vi.spyOn(process, "cwd").mockReturnValue(dir);
|
||||
|
|
@ -1571,6 +1576,19 @@ describe("start command — orchestrator session strategy display", () => {
|
|||
await expect(program.parseAsync(["node", "test", "start"])).rejects.toThrow("process.exit(1)");
|
||||
|
||||
expect(releaseStartupLock).toHaveBeenCalledTimes(1);
|
||||
const startFailedEvents = recordedEvents().filter((e) => e.kind === "cli.start_failed");
|
||||
expect(startFailedEvents).toHaveLength(1);
|
||||
expect(startFailedEvents[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
projectId: "my-app",
|
||||
source: "cli",
|
||||
level: "error",
|
||||
data: expect.objectContaining({
|
||||
reason: "orchestrator_setup",
|
||||
errorMessage: "Spawn failed",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("fails and cleans up dashboard when sm.restore throws on a killed orchestrator", async () => {
|
||||
|
|
@ -1651,6 +1669,56 @@ describe("start command — orchestrator session strategy display", () => {
|
|||
expect(written.stoppedAt).toBe("2026-04-28T10:00:00.000Z");
|
||||
});
|
||||
|
||||
it("attributes other-project restore failures to the owning project", async () => {
|
||||
mockReadLastStop.mockResolvedValue({
|
||||
stoppedAt: "2026-04-28T10:00:00.000Z",
|
||||
projectId: "my-app",
|
||||
sessionIds: ["app-1"],
|
||||
otherProjects: [{ projectId: "other-app", sessionIds: ["other-1"] }],
|
||||
});
|
||||
|
||||
mockConfigRef.current = makeConfig({
|
||||
"my-app": makeProject(),
|
||||
"other-app": makeProject({ name: "Other App", sessionPrefix: "other" }),
|
||||
});
|
||||
const { findWebDir } = await import("../../src/lib/web-dir.js");
|
||||
vi.mocked(findWebDir).mockReturnValue(tmpDir);
|
||||
writeFileSync(join(tmpDir, "package.json"), "{}");
|
||||
|
||||
const fakeDashboard = { on: vi.fn(), kill: vi.fn(), emit: vi.fn() };
|
||||
mockSpawn.mockReturnValue(fakeDashboard);
|
||||
|
||||
mockSessionManager.restore.mockImplementation((id: string) => {
|
||||
if (id === "other-1") return Promise.reject(new Error("workspace gone"));
|
||||
return Promise.resolve(undefined);
|
||||
});
|
||||
|
||||
await program.parseAsync(["node", "test", "start", "my-app", "--no-orchestrator"]);
|
||||
|
||||
const restoreFailedEvents = recordedEvents().filter(
|
||||
(e) => e.kind === "cli.restore_session_failed",
|
||||
);
|
||||
expect(restoreFailedEvents).toHaveLength(1);
|
||||
expect(restoreFailedEvents[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
projectId: "other-app",
|
||||
sessionId: "other-1",
|
||||
source: "cli",
|
||||
level: "warn",
|
||||
data: expect.objectContaining({ errorMessage: "workspace gone" }),
|
||||
}),
|
||||
);
|
||||
|
||||
const written = mockWriteLastStop.mock.calls[0][0];
|
||||
expect(written).toEqual(
|
||||
expect.objectContaining({
|
||||
projectId: "my-app",
|
||||
sessionIds: [],
|
||||
otherProjects: [{ projectId: "other-app", sessionIds: ["other-1"] }],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("clears last-stop record when every session restored successfully", async () => {
|
||||
mockReadLastStop.mockResolvedValue({
|
||||
stoppedAt: "2026-04-28T10:00:00.000Z",
|
||||
|
|
|
|||
|
|
@ -209,13 +209,25 @@ describe("shutdown handlers — activity events", () => {
|
|||
const events = recordedEvents();
|
||||
expect(events).toContainEqual(
|
||||
expect.objectContaining({
|
||||
kind: "cli.shutdown_failed",
|
||||
kind: "cli.last_stop_write_failed",
|
||||
source: "cli",
|
||||
level: "error",
|
||||
data: expect.objectContaining({ errorMessage: "disk full" }),
|
||||
data: expect.objectContaining({
|
||||
targetSessionCount: 1,
|
||||
otherProjectCount: 0,
|
||||
totalKilled: 1,
|
||||
errorMessage: "disk full",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(events.filter((e) => e.kind === "cli.shutdown_completed")).toHaveLength(0);
|
||||
expect(events).toContainEqual(
|
||||
expect.objectContaining({
|
||||
kind: "cli.shutdown_completed",
|
||||
source: "cli",
|
||||
projectId: "p1",
|
||||
}),
|
||||
);
|
||||
expect(events.filter((e) => e.kind === "cli.shutdown_failed")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("emits cli.shutdown_force_exit when the 10s timer fires", async () => {
|
||||
|
|
|
|||
|
|
@ -117,6 +117,17 @@ import { projectSessionUrl } from "../lib/routes.js";
|
|||
// HELPERS
|
||||
// =============================================================================
|
||||
|
||||
class CliFailureEventRecordedError extends Error {
|
||||
constructor(message: string, options?: ErrorOptions) {
|
||||
super(message, options);
|
||||
this.name = "CliFailureEventRecordedError";
|
||||
}
|
||||
}
|
||||
|
||||
function isCliFailureEventRecordedError(err: unknown): boolean {
|
||||
return err instanceof CliFailureEventRecordedError;
|
||||
}
|
||||
|
||||
function readProjectBehaviorConfig(projectPath: string): LocalProjectConfig {
|
||||
const localConfig = loadLocalProjectConfigDetailed(projectPath);
|
||||
if (localConfig.kind === "loaded") {
|
||||
|
|
@ -964,7 +975,7 @@ async function runStartup(
|
|||
if (dashboardProcess) {
|
||||
dashboardProcess.kill();
|
||||
}
|
||||
throw new Error(
|
||||
throw new CliFailureEventRecordedError(
|
||||
`Failed to setup orchestrator: ${err instanceof Error ? err.message : String(err)}`,
|
||||
{ cause: err },
|
||||
);
|
||||
|
|
@ -992,7 +1003,7 @@ async function runStartup(
|
|||
if (dashboardProcess) {
|
||||
dashboardProcess.kill();
|
||||
}
|
||||
throw new Error(
|
||||
throw new CliFailureEventRecordedError(
|
||||
`Failed to start project supervisor: ${err instanceof Error ? err.message : String(err)}`,
|
||||
{ cause: err },
|
||||
);
|
||||
|
|
@ -1006,12 +1017,21 @@ async function runStartup(
|
|||
if (lastStop && lastStop.sessionIds.length > 0) {
|
||||
const stoppedAgo = `stopped at ${new Date(lastStop.stoppedAt).toLocaleString()}`;
|
||||
const otherProjects = lastStop.otherProjects ?? [];
|
||||
const restoreProjectBySessionId = new Map<string, string>();
|
||||
|
||||
// Build flat list of all sessions to restore, grouped for display
|
||||
const allRestoreSessions: string[] = [
|
||||
...(lastStop.projectId === projectId ? lastStop.sessionIds : []),
|
||||
...otherProjects.flatMap((p) => p.sessionIds),
|
||||
];
|
||||
for (const sessionId of lastStop.sessionIds) {
|
||||
restoreProjectBySessionId.set(sessionId, lastStop.projectId);
|
||||
}
|
||||
for (const otherProject of otherProjects) {
|
||||
for (const sessionId of otherProject.sessionIds) {
|
||||
restoreProjectBySessionId.set(sessionId, otherProject.projectId);
|
||||
}
|
||||
}
|
||||
|
||||
// Display grouped by project
|
||||
const currentProjectSessions = lastStop.projectId === projectId ? lastStop.sessionIds : [];
|
||||
|
|
@ -1072,8 +1092,9 @@ async function runStartup(
|
|||
restoredCount++;
|
||||
} catch (err) {
|
||||
failedSessionIds.add(sessionId);
|
||||
const restoreProjectId = restoreProjectBySessionId.get(sessionId) ?? projectId;
|
||||
recordActivityEvent({
|
||||
projectId,
|
||||
projectId: restoreProjectId,
|
||||
sessionId,
|
||||
source: "cli",
|
||||
kind: "cli.restore_session_failed",
|
||||
|
|
@ -1743,16 +1764,18 @@ export function registerStart(program: Command): void {
|
|||
// Ctrl+C and `ao stop` (which sends SIGTERM) perform a full
|
||||
// graceful shutdown via the handler installed inside runStartup().
|
||||
} catch (err) {
|
||||
recordActivityEvent({
|
||||
source: "cli",
|
||||
kind: "cli.start_failed",
|
||||
level: "error",
|
||||
summary: `ao start action failed`,
|
||||
data: {
|
||||
reason: "outer",
|
||||
errorMessage: err instanceof Error ? err.message : String(err),
|
||||
},
|
||||
});
|
||||
if (!isCliFailureEventRecordedError(err)) {
|
||||
recordActivityEvent({
|
||||
source: "cli",
|
||||
kind: "cli.start_failed",
|
||||
level: "error",
|
||||
summary: `ao start action failed`,
|
||||
data: {
|
||||
reason: "outer",
|
||||
errorMessage: err instanceof Error ? err.message : String(err),
|
||||
},
|
||||
});
|
||||
}
|
||||
if (err instanceof Error) {
|
||||
console.error(chalk.red("\nError:"), err.message);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -119,7 +119,6 @@ export function installShutdownHandlers(ctx: ShutdownContext): void {
|
|||
}
|
||||
}
|
||||
|
||||
let lastStopWriteError: unknown;
|
||||
if (killedSessionIds.length > 0) {
|
||||
const targetIds = killedSessionIds.filter((id) =>
|
||||
activeSessions.some((s) => s.id === id && s.projectId === ctx.projectId),
|
||||
|
|
@ -144,15 +143,24 @@ export function installShutdownHandlers(ctx: ShutdownContext): void {
|
|||
otherProjects: otherProjects.length > 0 ? otherProjects : undefined,
|
||||
});
|
||||
} catch (err) {
|
||||
lastStopWriteError = err;
|
||||
recordActivityEvent({
|
||||
projectId: ctx.projectId,
|
||||
source: "cli",
|
||||
kind: "cli.last_stop_write_failed",
|
||||
level: "error",
|
||||
summary: `failed to write last-stop state during shutdown`,
|
||||
data: {
|
||||
targetSessionCount: targetIds.length,
|
||||
otherProjectCount: otherProjects.length,
|
||||
totalKilled: killedSessionIds.length,
|
||||
errorMessage: err instanceof Error ? err.message : String(err),
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
await sweepDaemonChildren({ ownerPid: process.pid });
|
||||
await unregister();
|
||||
if (lastStopWriteError) {
|
||||
throw lastStopWriteError;
|
||||
}
|
||||
recordActivityEvent({
|
||||
projectId: ctx.projectId,
|
||||
source: "cli",
|
||||
|
|
|
|||
Loading…
Reference in New Issue