From fcedb25031252d44219ef1aa88acc069c0683a09 Mon Sep 17 00:00:00 2001 From: Dhruv Sharma Date: Mon, 18 May 2026 17:27:36 +0530 Subject: [PATCH 1/6] feat(core): activity events for recovery, metadata corruption, agent-report (#1692) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(core): activity events for recovery, metadata corruption, agent-report Wires activity events into three forensic-critical paths so RCA can reconstruct what happened after the fact: 1. recovery subsystem - recovery.session_failed (MUST) per failed session in runRecovery - recovery.action_failed (SHOULD) on recoverSession outer catch Adds "recovery" to ActivityEventSource so `ao events list --source recovery` reconstructs an `ao recover` invocation timeline. 2. metadata corruption - metadata.corrupt_detected (MUST) when mutateMetadata renames a corrupt session-metadata file to .corrupt-{ts}. Includes data.renamedTo and a 200-char data.contentSample (B11) for forensic recovery. Previously only console.warn — silent overwrites had no queryable signal. 3. agent-report apply path - api.agent_report.transition_rejected (SHOULD) - api.agent_report.session_not_found (COULD) Per B22, recovery events fire per session, not per probe step. Per B11, metadata.corrupt_detected truncates contentSample to 200 chars (full file would exceed the 16KB sanitizer cap). Closes #1660 * fix(core): align forensic activity event metadata * fix(core): cover single-session recovery failures * fix(core): improve corrupt metadata event attribution * fix(cli): expose activity event source filter --------- Co-authored-by: whoisasx --- .../issue-1660-recovery-metadata-events.md | 6 + .../cli/__tests__/commands/events.test.ts | 87 ++++++ packages/cli/src/commands/events.ts | 21 +- .../core/src/__tests__/agent-report.test.ts | 28 +- packages/core/src/__tests__/metadata.test.ts | 139 ++++++++- .../src/__tests__/recovery-manager.test.ts | 269 ++++++++++++++++++ packages/core/src/activity-events.ts | 11 +- packages/core/src/agent-report.ts | 46 ++- packages/core/src/metadata.ts | 37 ++- packages/core/src/recovery/actions.ts | 17 +- packages/core/src/recovery/manager.ts | 34 ++- 11 files changed, 676 insertions(+), 19 deletions(-) create mode 100644 .changeset/issue-1660-recovery-metadata-events.md create mode 100644 packages/cli/__tests__/commands/events.test.ts create mode 100644 packages/core/src/__tests__/recovery-manager.test.ts diff --git a/.changeset/issue-1660-recovery-metadata-events.md b/.changeset/issue-1660-recovery-metadata-events.md new file mode 100644 index 000000000..4ad89f8b0 --- /dev/null +++ b/.changeset/issue-1660-recovery-metadata-events.md @@ -0,0 +1,6 @@ +--- +"@aoagents/ao-cli": patch +"@aoagents/ao-core": minor +--- + +Wire activity events for the recovery subsystem, metadata-corruption detection, and agent-report apply path. New event kinds: `recovery.session_failed`, `recovery.action_failed`, `metadata.corrupt_detected`, `api.agent_report.session_not_found`, `api.agent_report.transition_rejected`. Adds `"recovery"` to the `ActivityEventSource` union. Lets RCA reconstruct `ao recover` invocations, find every silent metadata overwrite, and audit rejected agent transitions. Adds `ao events list --source` and `--kind` so these forensic event queries are available from the CLI. diff --git a/packages/cli/__tests__/commands/events.test.ts b/packages/cli/__tests__/commands/events.test.ts new file mode 100644 index 000000000..e96633234 --- /dev/null +++ b/packages/cli/__tests__/commands/events.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { Command } from "commander"; + +const { mockQueryActivityEvents, mockSearchActivityEvents, mockGetActivityEventStats } = vi.hoisted( + () => ({ + mockQueryActivityEvents: vi.fn(), + mockSearchActivityEvents: vi.fn(), + mockGetActivityEventStats: vi.fn(), + }), +); + +vi.mock("@aoagents/ao-core", () => ({ + queryActivityEvents: (...args: unknown[]) => mockQueryActivityEvents(...args), + searchActivityEvents: (...args: unknown[]) => mockSearchActivityEvents(...args), + getActivityEventStats: (...args: unknown[]) => mockGetActivityEventStats(...args), + droppedEventCount: () => 0, + isActivityEventsFtsEnabled: () => true, +})); + +import { registerEvents } from "../../src/commands/events.js"; + +describe("events command", () => { + let program: Command; + let consoleLogSpy: ReturnType; + + beforeEach(() => { + program = new Command(); + program.exitOverride(); + registerEvents(program); + + consoleLogSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + mockQueryActivityEvents.mockReset(); + mockSearchActivityEvents.mockReset(); + mockGetActivityEventStats.mockReset(); + mockQueryActivityEvents.mockReturnValue([]); + }); + + afterEach(() => { + consoleLogSpy.mockRestore(); + }); + + it("filters list output by source and --kind alias", async () => { + await program.parseAsync([ + "node", + "test", + "events", + "list", + "--source", + "recovery", + "--kind", + "metadata.corrupt_detected", + "--limit", + "1", + "--json", + ]); + + expect(mockQueryActivityEvents).toHaveBeenCalledWith( + expect.objectContaining({ + source: "recovery", + kind: "metadata.corrupt_detected", + limit: 1, + }), + ); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining('"source": "recovery"')); + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining('"kind": "metadata.corrupt_detected"'), + ); + }); + + it("keeps --type as the existing event-kind filter", async () => { + await program.parseAsync([ + "node", + "test", + "events", + "list", + "--type", + "recovery.session_failed", + "--json", + ]); + + expect(mockQueryActivityEvents).toHaveBeenCalledWith( + expect.objectContaining({ + kind: "recovery.session_failed", + }), + ); + }); +}); diff --git a/packages/cli/src/commands/events.ts b/packages/cli/src/commands/events.ts index 87430d2a3..bbb72a7bd 100644 --- a/packages/cli/src/commands/events.ts +++ b/packages/cli/src/commands/events.ts @@ -9,6 +9,7 @@ import { type ActivityEvent, type ActivityEventLevel, type ActivityEventKind, + type ActivityEventSource, } from "@aoagents/ao-core"; interface JsonEnvelope { @@ -80,7 +81,12 @@ export function registerEvents(program: Command): void { .description("List recent activity events") .option("-p, --project ", "Filter by project ID") .option("-s, --session ", "Filter by session ID") - .option("-t, --type ", "Filter by event kind (e.g. session.spawned, lifecycle.transition)") + .option( + "-t, --type ", + "Filter by event kind (e.g. session.spawned, lifecycle.transition)", + ) + .option("--kind ", "Alias for --type") + .option("--source ", "Filter by event source (e.g. lifecycle, recovery, api)") .option("--log-level ", "Filter by log level (debug, info, warn, error)") .option("--since ", "Show events from last N minutes/hours/days (e.g. 30m, 2h, 1d)") .option("-n, --limit ", "Max results", "50") @@ -91,15 +97,21 @@ export function registerEvents(program: Command): void { if (sinceRaw) { since = parseSinceDuration(sinceRaw); if (!since) { - console.error(chalk.yellow(`Warning: unrecognised --since format "${sinceRaw}" (use e.g. 30m, 2h, 1d). No time filter applied.`)); + console.error( + chalk.yellow( + `Warning: unrecognised --since format "${sinceRaw}" (use e.g. 30m, 2h, 1d). No time filter applied.`, + ), + ); } } const limit = parseInt(opts["limit"] ?? "50", 10); + const kind = opts["type"] ?? opts["kind"]; const results = queryActivityEvents({ projectId: opts["project"], sessionId: opts["session"], - kind: opts["type"] as ActivityEventKind, + kind: kind as ActivityEventKind, + source: opts["source"] as ActivityEventSource, level: opts["logLevel"] as ActivityEventLevel, since, limit, @@ -111,7 +123,8 @@ export function registerEvents(program: Command): void { query: { projectId: opts["project"] ?? null, sessionId: opts["session"] ?? null, - kind: opts["type"] ?? null, + kind: kind ?? null, + source: opts["source"] ?? null, level: opts["logLevel"] ?? null, since: sinceRaw ?? null, limit, diff --git a/packages/core/src/__tests__/agent-report.test.ts b/packages/core/src/__tests__/agent-report.test.ts index 7e3fddce1..f49483b9d 100644 --- a/packages/core/src/__tests__/agent-report.test.ts +++ b/packages/core/src/__tests__/agent-report.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { mkdirSync, readFileSync, rmSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; @@ -18,17 +18,25 @@ import { } from "../agent-report.js"; import { writeMetadata, writeCanonicalLifecycle, readMetadataRaw } from "../metadata.js"; import { createInitialCanonicalLifecycle } from "../lifecycle-state.js"; +import { recordActivityEvent } from "../activity-events.js"; import type { CanonicalSessionLifecycle } from "../types.js"; +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); + let dataDir: string; +let tempRoot: string; beforeEach(() => { - dataDir = join(tmpdir(), `ao-test-agent-report-${randomUUID()}`); + tempRoot = join(tmpdir(), `ao-test-agent-report-${randomUUID()}`); + dataDir = join(tempRoot, "projects", "project-alpha", "sessions"); mkdirSync(dataDir, { recursive: true }); + vi.mocked(recordActivityEvent).mockClear(); }); afterEach(() => { - rmSync(dataDir, { recursive: true, force: true }); + rmSync(tempRoot, { recursive: true, force: true }); }); function seedWorkerSession( @@ -433,6 +441,13 @@ describe("applyAgentReport", () => { sessionState: "terminated", }, }); + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + projectId: "project-alpha", + sessionId, + kind: "api.agent_report.transition_rejected", + }), + ); }); it("throws when the session does not exist", () => { @@ -442,6 +457,13 @@ describe("applyAgentReport", () => { now: new Date(), }), ).toThrow(/not found/); + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + projectId: "project-alpha", + sessionId: "missing-session", + kind: "api.agent_report.session_not_found", + }), + ); }); // 260 atomic-write cycles are slow on Windows (rename + AV scan); bump the diff --git a/packages/core/src/__tests__/metadata.test.ts b/packages/core/src/__tests__/metadata.test.ts index 03ac1c16b..47c66b0e0 100644 --- a/packages/core/src/__tests__/metadata.test.ts +++ b/packages/core/src/__tests__/metadata.test.ts @@ -1,5 +1,6 @@ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { mkdirSync, rmSync, readFileSync, existsSync, writeFileSync, readdirSync } from "node:fs"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { mkdirSync, rmSync, readFileSync, existsSync, writeFileSync, readdirSync, renameSync } from "node:fs"; +import type * as NodeFs from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { randomUUID } from "node:crypto"; @@ -13,12 +14,29 @@ import { deleteMetadata, listMetadata, } from "../metadata.js"; +import { recordActivityEvent } from "../activity-events.js"; + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + renameSync: vi.fn((...args: Parameters) => + actual.renameSync(...args), + ), + }; +}); + +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); let dataDir: string; beforeEach(() => { dataDir = join(tmpdir(), `ao-test-metadata-${randomUUID()}`); mkdirSync(dataDir, { recursive: true }); + vi.mocked(recordActivityEvent).mockClear(); + vi.mocked(renameSync).mockClear(); }); afterEach(() => { @@ -390,6 +408,123 @@ describe("mutateMetadata corrupt-file handling", () => { const corruptCopies = readdirSync(dataDir).filter((f) => f.includes(".corrupt-")); expect(corruptCopies).toHaveLength(0); }); + + it("emits metadata.corrupt_detected when JSON parse fails and file is renamed", () => { + const sessionPath = join(dataDir, "ao-3.json"); + writeFileSync(sessionPath, "{ broken json", "utf-8"); + + const result = mutateMetadata( + dataDir, + "ao-3", + (existing) => ({ ...existing, branch: "feat/x" }), + { createIfMissing: true }, + ); + + expect(result).not.toBeNull(); + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + sessionId: "ao-3", + source: "session-manager", + kind: "metadata.corrupt_detected", + level: "error", + summary: expect.stringContaining("renamed to"), + data: expect.objectContaining({ + renamedTo: expect.stringContaining(`${sessionPath}.corrupt-`), + renameSucceeded: true, + contentSample: "{ broken json", + path: sessionPath, + }), + }), + ); + }); + + it("emits a rename-failed summary when corrupt metadata cannot be renamed", () => { + const sessionPath = join(dataDir, "ao-rename-failed.json"); + writeFileSync(sessionPath, "{ broken json", "utf-8"); + vi.mocked(renameSync).mockImplementationOnce(() => { + throw new Error("rename denied"); + }); + + const result = mutateMetadata( + dataDir, + "ao-rename-failed", + (existing) => ({ ...existing, branch: "feat/x" }), + { createIfMissing: true }, + ); + + expect(result).not.toBeNull(); + const call = vi + .mocked(recordActivityEvent) + .mock.calls.find((c) => c[0].kind === "metadata.corrupt_detected"); + expect(call).toBeDefined(); + expect(call![0]).toMatchObject({ + sessionId: "ao-rename-failed", + summary: expect.stringContaining("failed to rename"), + data: expect.objectContaining({ + renamedTo: null, + renameSucceeded: false, + path: sessionPath, + }), + }); + expect(call![0].summary).not.toContain("renamed to"); + }); + + it("uses the provided source for metadata.corrupt_detected", () => { + const sessionPath = join(dataDir, "ao-api-source.json"); + writeFileSync(sessionPath, "{ broken json", "utf-8"); + + mutateMetadata( + dataDir, + "ao-api-source", + (existing) => ({ ...existing, branch: "feat/api" }), + { createIfMissing: true, activityEventSource: "api" }, + ); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + sessionId: "ao-api-source", + source: "api", + kind: "metadata.corrupt_detected", + }), + ); + }); + + it("truncates contentSample to 200 chars in metadata.corrupt_detected", () => { + const sessionPath = join(dataDir, "ao-4.json"); + // 250 char garbage payload — sanitizer cap is 16KB but invariant B11 caps + // forensic sample at 200 chars. + const huge = "x".repeat(250); + writeFileSync(sessionPath, huge, "utf-8"); + + mutateMetadata( + dataDir, + "ao-4", + (existing) => ({ ...existing, branch: "feat/y" }), + { createIfMissing: true }, + ); + + const call = vi + .mocked(recordActivityEvent) + .mock.calls.find((c) => c[0].kind === "metadata.corrupt_detected"); + expect(call).toBeDefined(); + const sample = (call![0].data as Record)["contentSample"] as string; + expect(sample.length).toBe(200); + expect((call![0].data as Record)["contentLength"]).toBe(250); + }); + + it("does not emit metadata.corrupt_detected for healthy JSON", () => { + writeMetadata(dataDir, "ao-5", { + worktree: "/tmp/w", + branch: "main", + status: "working", + }); + mutateMetadata(dataDir, "ao-5", (existing) => ({ ...existing, summary: "hi" })); + + const corruptCalls = vi + .mocked(recordActivityEvent) + .mock.calls.filter((c) => c[0].kind === "metadata.corrupt_detected"); + expect(corruptCalls).toHaveLength(0); + }); }); describe("readCanonicalLifecycle", () => { diff --git a/packages/core/src/__tests__/recovery-manager.test.ts b/packages/core/src/__tests__/recovery-manager.test.ts new file mode 100644 index 000000000..80647dc29 --- /dev/null +++ b/packages/core/src/__tests__/recovery-manager.test.ts @@ -0,0 +1,269 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { randomUUID } from "node:crypto"; +import { recoverSessionById, runRecovery } from "../recovery/manager.js"; +import { recordActivityEvent } from "../activity-events.js"; +import { getProjectDir, getProjectSessionsDir } from "../paths.js"; +import { + DEFAULT_RECOVERY_CONFIG, + type RecoveryAssessment, + type RecoveryResult, +} from "../recovery/types.js"; +import * as actionsModule from "../recovery/actions.js"; +import * as validatorModule from "../recovery/validator.js"; +import type { OrchestratorConfig, PluginRegistry } from "../types.js"; + +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); + +const PROJECT_ID = "app"; + +function makeConfig(rootDir: string): OrchestratorConfig { + return { + configPath: join(rootDir, "agent-orchestrator.yaml"), + port: 3000, + readyThresholdMs: 300_000, + power: { preventIdleSleep: false }, + defaults: { + runtime: "tmux", + agent: "claude-code", + workspace: "worktree", + notifiers: ["desktop"], + }, + projects: { + app: { + name: "app", + repo: "org/repo", + path: join(rootDir, "project"), + defaultBranch: "main", + sessionPrefix: "app", + }, + }, + notifiers: {}, + notificationRouting: { + urgent: ["desktop"], + action: ["desktop"], + warning: ["desktop"], + info: ["desktop"], + }, + reactions: {}, + }; +} + +function makeRegistry(): PluginRegistry { + return { + register: vi.fn(), + get: vi.fn().mockReturnValue(null), + list: vi.fn().mockReturnValue([]), + loadBuiltins: vi.fn().mockResolvedValue(undefined), + loadFromConfig: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeAssessment(sessionId: string): RecoveryAssessment { + return { + sessionId, + projectId: PROJECT_ID, + classification: "live", + action: "recover", + reason: "needs recovery", + runtimeProbeSucceeded: true, + processProbeSucceeded: true, + signalDisagreement: false, + recoveryRule: "auto", + runtimeAlive: true, + runtimeHandle: null, + workspaceExists: true, + workspacePath: "/tmp/worktree", + agentProcessRunning: true, + agentActivity: "active", + metadataValid: true, + metadataStatus: "working", + rawMetadata: { project: PROJECT_ID, status: "working" }, + }; +} + +describe("runRecovery activity events", () => { + let rootDir: string; + let previousHome: string | undefined; + + beforeEach(() => { + rootDir = join(tmpdir(), `ao-recovery-events-${randomUUID()}`); + mkdirSync(rootDir, { recursive: true }); + mkdirSync(join(rootDir, "project"), { recursive: true }); + writeFileSync(join(rootDir, "agent-orchestrator.yaml"), "projects: {}\n", "utf-8"); + previousHome = process.env["HOME"]; + process.env["HOME"] = rootDir; + + const sessionsDir = getProjectSessionsDir(PROJECT_ID); + mkdirSync(sessionsDir, { recursive: true }); + writeFileSync( + join(sessionsDir, "app-1.json"), + JSON.stringify({ project: PROJECT_ID, status: "working" }) + "\n", + "utf-8", + ); + writeFileSync( + join(sessionsDir, "app-2.json"), + JSON.stringify({ project: PROJECT_ID, status: "working" }) + "\n", + "utf-8", + ); + + vi.mocked(recordActivityEvent).mockClear(); + vi.restoreAllMocks(); + }); + + afterEach(() => { + if (previousHome === undefined) { + delete process.env["HOME"]; + } else { + process.env["HOME"] = previousHome; + } + if (rootDir) { + const projectBaseDir = getProjectDir(PROJECT_ID); + if (existsSync(projectBaseDir)) { + rmSync(projectBaseDir, { recursive: true, force: true }); + } + rmSync(rootDir, { recursive: true, force: true }); + } + vi.restoreAllMocks(); + }); + + it("emits recovery.session_failed for each session that recovery couldn't fix", async () => { + vi.spyOn(validatorModule, "validateSession").mockImplementation(async (scanned) => + makeAssessment(scanned.sessionId), + ); + + const successResult: RecoveryResult = { + success: true, + sessionId: "app-1", + action: "recover", + }; + const failedResult: RecoveryResult = { + success: false, + sessionId: "app-2", + action: "recover", + error: "worktree missing", + }; + + vi.spyOn(actionsModule, "executeAction") + .mockResolvedValueOnce(successResult) + .mockResolvedValueOnce(failedResult); + + const config = makeConfig(rootDir); + const registry = makeRegistry(); + + const { report } = await runRecovery({ + config, + registry, + recoveryConfig: { + ...DEFAULT_RECOVERY_CONFIG, + logPath: join(rootDir, "recovery.log"), + }, + }); + + expect(report.errors).toHaveLength(1); + expect(report.errors[0]?.sessionId).toBe("app-2"); + + const emitCalls = vi + .mocked(recordActivityEvent) + .mock.calls.map((c) => c[0]) + .filter((e) => e.kind === "recovery.session_failed"); + + expect(emitCalls).toHaveLength(1); + expect(emitCalls[0]).toEqual( + expect.objectContaining({ + sessionId: "app-2", + projectId: PROJECT_ID, + source: "recovery", + kind: "recovery.session_failed", + level: "error", + data: expect.objectContaining({ + action: "recover", + errorMessage: "worktree missing", + }), + }), + ); + }); + + it("emits recovery.session_failed when single-session recovery fails", async () => { + vi.spyOn(validatorModule, "validateSession").mockImplementation(async (scanned) => + makeAssessment(scanned.sessionId), + ); + + const failedResult: RecoveryResult = { + success: false, + sessionId: "app-2", + action: "recover", + error: "agent process missing", + }; + + vi.spyOn(actionsModule, "executeAction").mockResolvedValueOnce(failedResult); + + const config = makeConfig(rootDir); + const registry = makeRegistry(); + + const result = await recoverSessionById("app-2", { + config, + registry, + recoveryConfig: { + ...DEFAULT_RECOVERY_CONFIG, + logPath: join(rootDir, "recovery.log"), + }, + }); + + expect(result).toEqual(failedResult); + + const emitCalls = vi + .mocked(recordActivityEvent) + .mock.calls.map((c) => c[0]) + .filter((e) => e.kind === "recovery.session_failed"); + + expect(emitCalls).toHaveLength(1); + expect(emitCalls[0]).toEqual( + expect.objectContaining({ + sessionId: "app-2", + projectId: PROJECT_ID, + source: "recovery", + kind: "recovery.session_failed", + level: "error", + data: expect.objectContaining({ + action: "recover", + errorMessage: "agent process missing", + }), + }), + ); + }); + + it("does not emit recovery.session_failed when every session recovers cleanly", async () => { + vi.spyOn(validatorModule, "validateSession").mockImplementation(async (scanned) => + makeAssessment(scanned.sessionId), + ); + + vi.spyOn(actionsModule, "executeAction").mockImplementation(async (assessment) => ({ + success: true, + sessionId: assessment.sessionId, + action: "recover", + })); + + const config = makeConfig(rootDir); + const registry = makeRegistry(); + + await runRecovery({ + config, + registry, + recoveryConfig: { + ...DEFAULT_RECOVERY_CONFIG, + logPath: join(rootDir, "recovery.log"), + }, + }); + + const failedEmits = vi + .mocked(recordActivityEvent) + .mock.calls.map((c) => c[0]) + .filter((e) => e.kind === "recovery.session_failed"); + expect(failedEmits).toHaveLength(0); + }); +}); diff --git a/packages/core/src/activity-events.ts b/packages/core/src/activity-events.ts index 0c1072e88..789e2616d 100644 --- a/packages/core/src/activity-events.ts +++ b/packages/core/src/activity-events.ts @@ -24,7 +24,8 @@ export type ActivityEventSource = | "workspace" | "notifier" | "reaction" - | "report-watcher"; + | "report-watcher" + | "recovery"; export type ActivityEventKind = | "session.spawn_started" @@ -69,7 +70,13 @@ export type ActivityEventKind = | "lifecycle.poll_failed" | "detecting.escalated" // Report watcher - | "report_watcher.triggered"; + | "report_watcher.triggered" + // Recovery/forensic instrumentation + | "recovery.session_failed" + | "recovery.action_failed" + | "metadata.corrupt_detected" + | "api.agent_report.session_not_found" + | "api.agent_report.transition_rejected"; export type ActivityEventLevel = "debug" | "info" | "warn" | "error"; diff --git a/packages/core/src/agent-report.ts b/packages/core/src/agent-report.ts index 07e380e30..13b1a187a 100644 --- a/packages/core/src/agent-report.ts +++ b/packages/core/src/agent-report.ts @@ -18,7 +18,7 @@ import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { readFile } from "node:fs/promises"; -import { join } from "node:path"; +import { basename, dirname, join } from "node:path"; import type { CanonicalSessionLifecycle, CanonicalSessionReason, @@ -26,6 +26,7 @@ import type { SessionId, SessionStatus, } from "./types.js"; +import { recordActivityEvent } from "./activity-events.js"; import { mutateMetadata, readMetadataRaw } from "./metadata.js"; import { buildLifecycleMetadataPatch, cloneLifecycle, deriveLegacyStatus, parseCanonicalLifecycle } from "./lifecycle-state.js"; import { parsePrFromUrl } from "./utils/pr.js"; @@ -254,6 +255,11 @@ function buildAuditDir(dataDir: string): string { return join(dataDir, ".agent-report-audit"); } +function inferProjectIdFromDataDir(dataDir: string): string | undefined { + // dataDir is `.../projects/{projectId}/sessions`; recover projectId for filtering. + return basename(dirname(dataDir)) || undefined; +} + const AGENT_REPORT_AUDIT_MAX_BYTES = 256 * 1024; const AGENT_REPORT_AUDIT_MAX_ENTRIES = 200; @@ -383,8 +389,22 @@ export function applyAgentReport( sessionId: SessionId, input: ApplyAgentReportInput, ): ApplyAgentReportResult { + const projectId = inferProjectIdFromDataDir(dataDir); const raw = readMetadataRaw(dataDir, sessionId); if (!raw) { + recordActivityEvent({ + projectId, + sessionId, + source: "api", + kind: "api.agent_report.session_not_found", + level: "warn", + summary: `applyAgentReport: session not found: ${sessionId}`, + data: { + reportState: input.state, + actor: input.actor, + source: input.source, + }, + }); throw new Error(`Session not found: ${sessionId}`); } @@ -439,6 +459,7 @@ export function applyAgentReport( before = buildAuditSnapshot(current, previousLegacyStatus); const validation = validateAgentReportTransition(current, input.state); if (!validation.ok) { + const rejectionReason = validation.reason ?? "transition rejected"; appendAgentReportAuditEntry(dataDir, sessionId, { timestamp: now, actor, @@ -449,11 +470,28 @@ export function applyAgentReport( prUrl: trimmedPrUrl, prIsDraft, accepted: false, - rejectionReason: validation.reason ?? "transition rejected", + rejectionReason, before, after: before, }); - throw new Error(validation.reason ?? "transition rejected"); + recordActivityEvent({ + projectId, + sessionId, + source: "api", + kind: "api.agent_report.transition_rejected", + level: "warn", + summary: `applyAgentReport rejected ${input.state} for ${sessionId}: ${rejectionReason}`, + data: { + reportState: input.state, + rejectionReason, + actor, + reportSource: source, + fromState: current.session.state, + fromReason: current.session.reason, + legacyStatus: previousLegacyStatus, + }, + }); + throw new Error(rejectionReason); } const mapped = mapAgentReportToLifecycle(input.state); previousState = current.session.state; @@ -512,7 +550,7 @@ export function applyAgentReport( } } return next; - }); + }, { activityEventSource: "api" }); if (!nextMetadata || !before || !previousState || !nextState || !legacyStatus) { throw new Error(`Failed to apply agent report for session ${sessionId}`); diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index a4f3a19c5..451c127b5 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -22,9 +22,10 @@ import { closeSync, constants, } from "node:fs"; -import { join, dirname } from "node:path"; +import { basename, join, dirname } from "node:path"; import type { CanonicalSessionLifecycle, RuntimeHandle, SessionId, SessionMetadata } from "./types.js"; import { atomicWriteFileSync } from "./atomic-write.js"; +import { recordActivityEvent, type ActivityEventSource } from "./activity-events.js"; import { buildLifecycleMetadataPatch, cloneLifecycle, @@ -330,6 +331,11 @@ export function applyMetadataUpdates( return next; } +interface MutateMetadataOptions { + createIfMissing?: boolean; + activityEventSource?: ActivityEventSource; +} + function normalizeMetadataRecord(data: Record): Record { return Object.fromEntries( Object.entries(data).filter(([, value]) => value !== undefined && value !== ""), @@ -340,7 +346,7 @@ export function mutateMetadata( dataDir: string, sessionId: SessionId, updater: (existing: Record) => Record, - options: { createIfMissing?: boolean } = {}, + options: MutateMetadataOptions = {}, ): Record | null { const path = metadataPath(dataDir, sessionId); const lockPath = `${path}.lock`; @@ -368,8 +374,10 @@ export function mutateMetadata( // that anything was wrong — the file just becomes "not // corrupt anymore — and missing fields". const corruptPath = `${path}.corrupt-${Date.now()}`; + let renamed = false; try { renameSync(path, corruptPath); + renamed = true; // eslint-disable-next-line no-console console.warn( `[metadata] corrupt JSON at ${path}; preserved as ${corruptPath} before rewriting`, @@ -377,6 +385,31 @@ export function mutateMetadata( } catch { // best effort — proceed even if the rename fails (e.g. EACCES) } + // Forensic activity event so RCA can find every silent overwrite. + // Truncate the bad-JSON sample to 200 chars (B11 invariant — full file + // could be 16KB+ and would be dropped by the sanitizer cap). + const contentSample = + content.length > 200 ? content.slice(0, 200) : content; + // dataDir is `.../projects/{projectId}/sessions`; recover projectId for filtering. + const inferredProjectId = basename(dirname(dataDir)); + const summary = renamed + ? `Corrupt metadata for session ${sessionId} renamed to ${basename(corruptPath)}` + : `Corrupt metadata detected for session ${sessionId}; failed to rename forensic copy before rewrite`; + recordActivityEvent({ + projectId: inferredProjectId || undefined, + sessionId, + source: options.activityEventSource ?? "session-manager", + kind: "metadata.corrupt_detected", + level: "error", + summary, + data: { + path, + renamedTo: renamed ? corruptPath : null, + renameSucceeded: renamed, + contentSample, + contentLength: content.length, + }, + }); } } } else if (!options.createIfMissing) { diff --git a/packages/core/src/recovery/actions.ts b/packages/core/src/recovery/actions.ts index 43a9eec87..f7b57980a 100644 --- a/packages/core/src/recovery/actions.ts +++ b/packages/core/src/recovery/actions.ts @@ -5,6 +5,7 @@ import type { Runtime, Workspace, } from "../types.js"; +import { recordActivityEvent } from "../activity-events.js"; import { updateMetadata } from "../metadata.js"; import { getProjectSessionsDir } from "../paths.js"; import { validateStatus } from "../utils/validation.js"; @@ -146,11 +147,25 @@ export async function recoverSession( session, }; } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + recordActivityEvent({ + projectId, + sessionId, + source: "recovery", + kind: "recovery.action_failed", + level: "error", + summary: `recoverSession threw for ${sessionId}: ${errorMessage}`, + data: { + action: "recover", + recoveryCount, + errorMessage, + }, + }); return { success: false, sessionId, action: "recover", - error: error instanceof Error ? error.message : String(error), + error: errorMessage, }; } } diff --git a/packages/core/src/recovery/manager.ts b/packages/core/src/recovery/manager.ts index 1c90e007f..330922e7f 100644 --- a/packages/core/src/recovery/manager.ts +++ b/packages/core/src/recovery/manager.ts @@ -1,4 +1,5 @@ import type { OrchestratorConfig, PluginRegistry, Session } from "../types.js"; +import { recordActivityEvent } from "../activity-events.js"; import { scanAllSessions, getRecoveryLogPath } from "./scanner.js"; import { validateSession } from "./validator.js"; import { executeAction } from "./actions.js"; @@ -106,9 +107,10 @@ export async function runRecovery(options: RecoveryManagerOptions): Promise Date: Mon, 18 May 2026 17:28:10 +0530 Subject: [PATCH 2/6] feat(web): record activity events for API mutation routes (#1695) * feat(web): record activity events for API mutation routes Wire recordActivityEvent calls into all POST/PATCH/DELETE handlers under packages/web/src/app/api/ so RCA can answer "did the user click X?" beyond just success/failure status codes. Source: "api" (added in #1620). Coverage: - session mutations: spawn, kill, send, message, restore, remap (with failure variants for SessionNotRestorable/WorkspaceMissing/etc.) - orchestrator + PR: orchestrator spawn, PR merge (with rejected/failed) - project + config: add, update, remove, repair, reload - issue + verify + labels: issue create, verify, label setup Sanitization rules preserved: - never include request/response bodies - *_message_* events include messageLength only, never the message text - project_updated records changedKeys, never values (config can carry tokens) Tests: regression coverage for the 12 MUST emits across two new test files, plus negative-path sanitization assertions. Closes #1655 * fix(web): refine api activity failure events * fix(web): align project activity event tests * fix(web): avoid orchestrator spawn failure double emit --------- Co-authored-by: whoisasx --- .../activity-events-projects.test.ts | 164 ++++++ .../__tests__/activity-events-routes.test.ts | 498 ++++++++++++++++++ packages/web/src/app/api/issues/route.ts | 24 +- .../web/src/app/api/orchestrators/route.ts | 15 +- .../web/src/app/api/projects/[id]/route.ts | 95 ++-- .../web/src/app/api/projects/reload/route.ts | 20 +- packages/web/src/app/api/projects/route.ts | 23 + .../web/src/app/api/prs/[id]/merge/route.ts | 48 +- .../src/app/api/sessions/[id]/kill/route.ts | 18 +- .../app/api/sessions/[id]/message/route.ts | 19 +- .../src/app/api/sessions/[id]/remap/route.ts | 27 +- .../app/api/sessions/[id]/restore/route.ts | 59 ++- .../src/app/api/sessions/[id]/send/route.ts | 19 +- .../web/src/app/api/setup-labels/route.ts | 10 + packages/web/src/app/api/spawn/route.ts | 20 + packages/web/src/app/api/verify/route.ts | 32 +- 16 files changed, 1028 insertions(+), 63 deletions(-) create mode 100644 packages/web/src/__tests__/activity-events-projects.test.ts create mode 100644 packages/web/src/__tests__/activity-events-routes.test.ts diff --git a/packages/web/src/__tests__/activity-events-projects.test.ts b/packages/web/src/__tests__/activity-events-projects.test.ts new file mode 100644 index 000000000..2aad007da --- /dev/null +++ b/packages/web/src/__tests__/activity-events-projects.test.ts @@ -0,0 +1,164 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { execSync } from "node:child_process"; +import { NextRequest } from "next/server"; +import { recordActivityEvent, registerProjectInGlobalConfig } from "@aoagents/ao-core"; + +vi.mock("@aoagents/ao-core", async () => { + const actual = await vi.importActual("@aoagents/ao-core"); + return { + ...(actual as Record), + recordActivityEvent: vi.fn(), + }; +}); + +const invalidatePortfolioServicesCache = vi.fn(); +const getServices = vi.fn(); + +vi.mock("@/lib/services", () => ({ + invalidatePortfolioServicesCache, + getServices, +})); + +function makeRequest(method: string, url: string, body?: Record): NextRequest { + return new NextRequest(url, { + method, + body: body ? JSON.stringify(body) : undefined, + headers: body ? { "Content-Type": "application/json" } : undefined, + }); +} + +const recorded = vi.mocked(recordActivityEvent); + +describe("Activity events — project mutation routes", () => { + let oldGlobalConfig: string | undefined; + let oldConfigPath: string | undefined; + let oldHome: string | undefined; + let tempRoot: string; + let configPath: string; + + beforeEach(() => { + vi.resetModules(); + recorded.mockClear(); + invalidatePortfolioServicesCache.mockReset(); + getServices.mockReset(); + getServices.mockResolvedValue({ + registry: { get: vi.fn().mockReturnValue(null) }, + sessionManager: { + list: vi.fn().mockResolvedValue([]), + kill: vi.fn().mockResolvedValue(undefined), + }, + }); + oldGlobalConfig = process.env["AO_GLOBAL_CONFIG"]; + oldConfigPath = process.env["AO_CONFIG_PATH"]; + oldHome = process.env["HOME"]; + tempRoot = mkdtempSync(path.join(tmpdir(), "ao-activity-projects-")); + configPath = path.join(tempRoot, "config.yaml"); + process.env["AO_GLOBAL_CONFIG"] = configPath; + process.env["AO_CONFIG_PATH"] = configPath; + process.env["HOME"] = tempRoot; + }); + + afterEach(() => { + if (oldGlobalConfig === undefined) delete process.env["AO_GLOBAL_CONFIG"]; + else process.env["AO_GLOBAL_CONFIG"] = oldGlobalConfig; + if (oldConfigPath === undefined) delete process.env["AO_CONFIG_PATH"]; + else process.env["AO_CONFIG_PATH"] = oldConfigPath; + if (oldHome === undefined) delete process.env["HOME"]; + else process.env["HOME"] = oldHome; + rmSync(tempRoot, { recursive: true, force: true }); + }); + + it("POST /api/projects emits api.project_added on success", async () => { + const repoDir = path.join(tempRoot, "demo-add"); + mkdirSync(repoDir, { recursive: true }); + execSync("git init -q", { cwd: repoDir }); + + const { POST } = await import("@/app/api/projects/route"); + const res = await POST( + makeRequest("POST", "http://localhost:3000/api/projects", { + path: repoDir, + projectId: "demo-add", + name: "Demo Add", + }), + ); + + expect(res.status).toBe(201); + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.project_added", + }), + ); + }); + + it("PATCH /api/projects/:id emits api.project_updated with changed keys (not values)", async () => { + const repoDir = path.join(tempRoot, "demo-patch"); + mkdirSync(repoDir, { recursive: true }); + const effectiveId = registerProjectInGlobalConfig("demo-patch", "Demo Patch", repoDir); + + const { PATCH } = await import("@/app/api/projects/[id]/route"); + const res = await PATCH( + makeRequest("PATCH", `http://localhost:3000/api/projects/${effectiveId}`, { + agent: "codex", + runtime: "tmux", + someUnknownField: "do-not-record", + }), + { params: Promise.resolve({ id: effectiveId }) }, + ); + + expect(res.status).toBe(200); + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.project_updated", + projectId: effectiveId, + data: expect.objectContaining({ + changedKeys: expect.arrayContaining(["agent", "runtime"]), + }), + }), + ); + + // Ensure no value content (e.g. "codex", "tmux") leaked into the event payload + const calls = recorded.mock.calls.filter( + (c) => (c[0] as { kind: string }).kind === "api.project_updated", + ); + expect(calls[0]?.[0]).toEqual( + expect.objectContaining({ + data: expect.objectContaining({ + changedKeys: ["agent", "runtime"], + }), + }), + ); + for (const [event] of calls) { + const json = JSON.stringify(event); + expect(json).not.toContain("codex"); + expect(json).not.toContain('"tmux"'); + expect(json).not.toContain("someUnknownField"); + expect(json).not.toContain("do-not-record"); + } + }); + + it("DELETE /api/projects/:id emits api.project_removed on success", async () => { + const repoDir = path.join(tempRoot, "demo-delete"); + mkdirSync(repoDir, { recursive: true }); + const effectiveId = registerProjectInGlobalConfig("demo-delete", "Demo Delete", repoDir); + + const { DELETE } = await import("@/app/api/projects/[id]/route"); + const res = await DELETE( + makeRequest("DELETE", `http://localhost:3000/api/projects/${effectiveId}`), + { params: Promise.resolve({ id: effectiveId }) }, + ); + + expect(res.status).toBe(200); + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.project_removed", + projectId: effectiveId, + }), + ); + }); +}); diff --git a/packages/web/src/__tests__/activity-events-routes.test.ts b/packages/web/src/__tests__/activity-events-routes.test.ts new file mode 100644 index 000000000..fbd2b474c --- /dev/null +++ b/packages/web/src/__tests__/activity-events-routes.test.ts @@ -0,0 +1,498 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; +import { + SessionNotFoundError, + SessionNotRestorableError, + WorkspaceMissingError, + recordActivityEvent, + createInitialCanonicalLifecycle, + createActivitySignal, + type Session, + type SessionManager, + type OrchestratorConfig, + type PluginRegistry, + type SCM, +} from "@aoagents/ao-core"; + +// Partial mock so we replace recordActivityEvent but keep types/helpers +vi.mock("@aoagents/ao-core", async () => { + const actual = await vi.importActual("@aoagents/ao-core"); + return { + ...(actual as Record), + recordActivityEvent: vi.fn(), + }; +}); + +vi.mock("@/lib/observability", async () => { + const actual = await vi.importActual("@/lib/observability"); + return { + ...(actual as Record), + recordApiObservation: vi.fn(), + }; +}); + +function makeSession(overrides: Partial & { id: string }): Session { + const lifecycle = createInitialCanonicalLifecycle("worker", new Date()); + lifecycle.session.state = "working"; + lifecycle.session.reason = "task_in_progress"; + lifecycle.session.startedAt = lifecycle.session.lastTransitionAt; + lifecycle.runtime.state = "alive"; + lifecycle.runtime.reason = "process_running"; + return { + projectId: "my-app", + status: "working", + activity: "active", + activitySignal: createActivitySignal("valid", { + activity: "active", + timestamp: new Date(), + source: "native", + }), + lifecycle, + branch: null, + issueId: null, + pr: null, + workspacePath: null, + runtimeHandle: null, + agentInfo: null, + createdAt: new Date(), + lastActivityAt: new Date(), + metadata: {}, + ...overrides, + }; +} + +const baseSessions: Session[] = [ + makeSession({ id: "backend-3" }), + makeSession({ + id: "backend-7", + pr: { + number: 432, + url: "https://github.com/acme/my-app/pull/432", + title: "feat: health check", + owner: "acme", + repo: "my-app", + branch: "feat/health-check", + baseBranch: "main", + isDraft: false, + }, + }), + makeSession({ id: "frontend-1", status: "killed", activity: "exited" }), +]; + +const mockSessionManager: SessionManager = { + list: vi.fn(async () => baseSessions), + listCached: vi.fn(async () => baseSessions), + invalidateCache: vi.fn(), + get: vi.fn(async (id: string) => baseSessions.find((s) => s.id === id) ?? null), + spawn: vi.fn(async (cfg) => + makeSession({ + id: `session-${Date.now()}`, + projectId: cfg.projectId, + issueId: cfg.issueId ?? null, + status: "spawning", + }), + ), + kill: vi.fn(async (id: string) => { + if (!baseSessions.find((s) => s.id === id)) { + throw new SessionNotFoundError(id); + } + }), + send: vi.fn(async (id: string) => { + if (!baseSessions.find((s) => s.id === id)) { + throw new SessionNotFoundError(id); + } + }), + cleanup: vi.fn(async () => ({ killed: [], skipped: [], errors: [] })), + spawnOrchestrator: vi.fn(async () => + makeSession({ + id: "my-app-orchestrator", + projectId: "my-app", + metadata: { role: "orchestrator" }, + }), + ), + relaunchOrchestrator: vi.fn(async () => + makeSession({ + id: "my-app-orchestrator", + projectId: "my-app", + metadata: { role: "orchestrator" }, + }), + ), + ensureOrchestrator: vi.fn(), + remap: vi.fn(async () => "ses_mock"), + restore: vi.fn(async (id: string) => { + const session = baseSessions.find((s) => s.id === id); + if (!session) throw new SessionNotFoundError(id); + return { ...session, status: "spawning" as const, activity: "active" as const }; + }), +}; + +const mockSCM: SCM = { + name: "github", + detectPR: vi.fn(async () => null), + getPRState: vi.fn(async () => "open" as const), + mergePR: vi.fn(async () => {}), + closePR: vi.fn(async () => {}), + getCIChecks: vi.fn(async () => []), + getCISummary: vi.fn(async () => "passing" as const), + getReviews: vi.fn(async () => []), + getReviewDecision: vi.fn(async () => "approved" as const), + getPendingComments: vi.fn(async () => []), + getAutomatedComments: vi.fn(async () => []), + getMergeability: vi.fn(async () => ({ + mergeable: true, + ciPassing: true, + approved: true, + noConflicts: true, + blockers: [], + })), +}; + +const mockRegistry: PluginRegistry = { + register: vi.fn(), + get: vi.fn(() => mockSCM) as PluginRegistry["get"], + list: vi.fn(() => []), + loadBuiltins: vi.fn(async () => {}), + loadFromConfig: vi.fn(async () => {}), +}; + +const mockConfig: OrchestratorConfig = { + configPath: "/tmp/ao-test/agent-orchestrator.yaml", + port: 3000, + readyThresholdMs: 300_000, + defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + projects: { + "my-app": { + name: "My App", + repo: "acme/my-app", + path: "/tmp/my-app", + defaultBranch: "main", + sessionPrefix: "my-app", + scm: { plugin: "github" }, + }, + }, + notifiers: {}, + notificationRouting: { urgent: [], action: [], warning: [], info: [] }, + reactions: {}, +}; + +vi.mock("@/lib/services", () => ({ + getServices: vi.fn(async () => ({ + config: mockConfig, + registry: mockRegistry, + sessionManager: mockSessionManager, + })), + getVerifyIssues: vi.fn(async () => []), + getSCM: vi.fn(() => mockSCM), + invalidatePortfolioServicesCache: vi.fn(), +})); + +import { getServices } from "@/lib/services"; +import { recordApiObservation } from "@/lib/observability"; +import { POST as spawnPOST } from "@/app/api/spawn/route"; +import { POST as killPOST } from "@/app/api/sessions/[id]/kill/route"; +import { POST as sendPOST } from "@/app/api/sessions/[id]/send/route"; +import { POST as messagePOST } from "@/app/api/sessions/[id]/message/route"; +import { POST as restorePOST } from "@/app/api/sessions/[id]/restore/route"; +import { POST as orchestratorsPOST } from "@/app/api/orchestrators/route"; +import { POST as mergePOST } from "@/app/api/prs/[id]/merge/route"; + +function makeRequest(url: string, init?: RequestInit): NextRequest { + return new NextRequest( + new URL(url, "http://localhost:3000"), + init as ConstructorParameters[1], + ); +} + +const recorded = vi.mocked(recordActivityEvent); + +beforeEach(() => { + recorded.mockClear(); + vi.mocked(recordApiObservation).mockClear(); + vi.mocked(getServices).mockClear(); +}); + +describe("API mutation routes emit activity events (api source)", () => { + describe("MUST emits — session mutations", () => { + it("POST /api/spawn emits api.session_spawn_requested on success", async () => { + const req = makeRequest("/api/spawn", { + method: "POST", + body: JSON.stringify({ projectId: "my-app", issueId: "INT-100" }), + headers: { "Content-Type": "application/json" }, + }); + await spawnPOST(req); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_spawn_requested", + projectId: "my-app", + }), + ); + }); + + it("POST /api/sessions/:id/kill emits api.session_kill_requested on success", async () => { + const req = makeRequest("/api/sessions/backend-3/kill", { method: "POST" }); + await killPOST(req, { params: Promise.resolve({ id: "backend-3" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_kill_requested", + sessionId: "backend-3", + }), + ); + }); + + it("POST /api/sessions/:id/send emits api.session_message_sent with messageLength", async () => { + const req = makeRequest("/api/sessions/backend-3/send", { + method: "POST", + body: JSON.stringify({ message: "Fix the tests" }), + headers: { "Content-Type": "application/json" }, + }); + await sendPOST(req, { params: Promise.resolve({ id: "backend-3" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_message_sent", + sessionId: "backend-3", + data: expect.objectContaining({ messageLength: "Fix the tests".length }), + }), + ); + }); + + it("POST /api/sessions/:id/send does NOT include the raw message in data", async () => { + const secret = "very-secret-PII content"; + const req = makeRequest("/api/sessions/backend-3/send", { + method: "POST", + body: JSON.stringify({ message: secret }), + headers: { "Content-Type": "application/json" }, + }); + await sendPOST(req, { params: Promise.resolve({ id: "backend-3" }) }); + + const calls = recorded.mock.calls.filter( + (c) => (c[0] as { kind: string }).kind === "api.session_message_sent", + ); + expect(calls.length).toBeGreaterThan(0); + for (const [event] of calls) { + const json = JSON.stringify(event); + expect(json).not.toContain(secret); + } + }); + + it("POST /api/sessions/:id/message emits api.session_message_sent with messageLength", async () => { + const req = makeRequest("/api/sessions/backend-3/message", { + method: "POST", + body: JSON.stringify({ message: "Hi" }), + headers: { "Content-Type": "application/json" }, + }); + await messagePOST(req, { params: Promise.resolve({ id: "backend-3" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_message_sent", + sessionId: "backend-3", + data: expect.objectContaining({ messageLength: 2 }), + }), + ); + }); + + it("POST /api/sessions/:id/restore emits api.session_restore_requested on success", async () => { + const req = makeRequest("/api/sessions/frontend-1/restore", { method: "POST" }); + await restorePOST(req, { params: Promise.resolve({ id: "frontend-1" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_restore_requested", + sessionId: "frontend-1", + }), + ); + }); + }); + + describe("MUST emits — orchestrator + PR mutations", () => { + it("POST /api/orchestrators emits api.orchestrator_spawn_requested on success", async () => { + const req = makeRequest("/api/orchestrators", { + method: "POST", + body: JSON.stringify({ projectId: "my-app" }), + headers: { "Content-Type": "application/json" }, + }); + await orchestratorsPOST(req); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.orchestrator_spawn_requested", + projectId: "my-app", + }), + ); + }); + + it("POST /api/prs/:id/merge emits api.pr_merge_requested on success", async () => { + const req = makeRequest("/api/prs/432/merge", { method: "POST" }); + await mergePOST(req, { params: Promise.resolve({ id: "432" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.pr_merge_requested", + data: expect.objectContaining({ prNumber: 432 }), + }), + ); + }); + }); + + describe("SHOULD emits — failure paths", () => { + it("POST /api/spawn emits api.session_spawn_rejected for unknown project", async () => { + const req = makeRequest("/api/spawn", { + method: "POST", + body: JSON.stringify({ projectId: "unknown-app" }), + headers: { "Content-Type": "application/json" }, + }); + await spawnPOST(req); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_spawn_rejected", + projectId: "unknown-app", + }), + ); + }); + + it("POST /api/spawn does not emit api.session_spawn_failed when core spawn throws", async () => { + (mockSessionManager.spawn as ReturnType).mockRejectedValueOnce( + new Error("runtime failed"), + ); + const req = makeRequest("/api/spawn", { + method: "POST", + body: JSON.stringify({ projectId: "my-app", issueId: "INT-101" }), + headers: { "Content-Type": "application/json" }, + }); + const res = await spawnPOST(req); + + expect(res.status).toBe(500); + expect( + recorded.mock.calls.some( + ([event]) => (event as { kind: string }).kind === "api.session_spawn_failed", + ), + ).toBe(false); + }); + + it.each([ + ["spawn", false, mockSessionManager.spawnOrchestrator], + ["clean relaunch", true, mockSessionManager.relaunchOrchestrator], + ])( + "POST /api/orchestrators does not emit api.orchestrator_spawn_failed when core %s throws", + async (_name, clean, method) => { + (method as ReturnType).mockRejectedValueOnce(new Error("runtime failed")); + const req = makeRequest("/api/orchestrators", { + method: "POST", + body: JSON.stringify({ projectId: "my-app", clean }), + headers: { "Content-Type": "application/json" }, + }); + const res = await orchestratorsPOST(req); + + expect(res.status).toBe(500); + expect( + recorded.mock.calls.some( + ([event]) => (event as { kind: string }).kind === "api.orchestrator_spawn_failed", + ), + ).toBe(false); + }, + ); + + it("POST /api/sessions/:id/send emits api.session_message_failed on unexpected error", async () => { + (mockSessionManager.send as ReturnType).mockRejectedValueOnce( + new Error("write failed"), + ); + const req = makeRequest("/api/sessions/backend-3/send", { + method: "POST", + body: JSON.stringify({ message: "hi" }), + headers: { "Content-Type": "application/json" }, + }); + await sendPOST(req, { params: Promise.resolve({ id: "backend-3" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_message_failed", + sessionId: "backend-3", + data: expect.objectContaining({ messageLength: 2 }), + }), + ); + }); + + it.each([ + ["non-restorable session", new SessionNotRestorableError("my-app-123", "still working"), 409], + ["missing workspace", new WorkspaceMissingError("/tmp/missing-workspace"), 422], + ["unexpected restore error", new Error("restore failed"), 500], + ])( + "POST /api/sessions/:id/restore emits attributed api.session_restore_failed for %s", + async (_name, error, statusCode) => { + (mockSessionManager.restore as ReturnType).mockRejectedValueOnce(error); + const req = makeRequest("/api/sessions/my-app-123/restore", { method: "POST" }); + const res = await restorePOST(req, { params: Promise.resolve({ id: "my-app-123" }) }); + + expect(res.status).toBe(statusCode); + expect(vi.mocked(getServices)).toHaveBeenCalledTimes(1); + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.session_restore_failed", + projectId: "my-app", + sessionId: "my-app-123", + data: expect.objectContaining({ statusCode }), + }), + ); + }, + ); + + it("POST /api/prs/:id/merge emits api.pr_merge_rejected for non-mergeable PR", async () => { + (mockSCM.getMergeability as ReturnType).mockResolvedValueOnce({ + mergeable: false, + ciPassing: false, + approved: false, + noConflicts: true, + blockers: ["CI checks failing"], + }); + const req = makeRequest("/api/prs/432/merge", { method: "POST" }); + await mergePOST(req, { params: Promise.resolve({ id: "432" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.pr_merge_rejected", + data: expect.objectContaining({ prNumber: 432 }), + }), + ); + }); + + it("POST /api/prs/:id/merge emits api.pr_merge_failed when mergePR throws", async () => { + (mockSCM.mergePR as ReturnType).mockRejectedValueOnce(new Error("github 500")); + const req = makeRequest("/api/prs/432/merge", { method: "POST" }); + await mergePOST(req, { params: Promise.resolve({ id: "432" }) }); + + expect(recorded).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.pr_merge_failed", + projectId: "my-app", + sessionId: "backend-7", + data: expect.objectContaining({ prNumber: 432, reason: "github 500" }), + }), + ); + expect(recordApiObservation).toHaveBeenCalledWith( + expect.objectContaining({ + outcome: "failure", + statusCode: 500, + projectId: "my-app", + sessionId: "backend-7", + data: expect.objectContaining({ prNumber: 432 }), + }), + ); + }); + }); +}); diff --git a/packages/web/src/app/api/issues/route.ts b/packages/web/src/app/api/issues/route.ts index 6063a67f1..07b18847a 100644 --- a/packages/web/src/app/api/issues/route.ts +++ b/packages/web/src/app/api/issues/route.ts @@ -1,7 +1,7 @@ import { type NextRequest, NextResponse } from "next/server"; import { getServices } from "@/lib/services"; import { validateString, validateConfiguredProject } from "@/lib/validation"; -import type { Tracker } from "@aoagents/ao-core"; +import { recordActivityEvent, type Tracker } from "@aoagents/ao-core"; export const dynamic = "force-dynamic"; @@ -95,11 +95,25 @@ export async function POST(request: NextRequest) { project, ); + recordActivityEvent({ + projectId, + source: "api", + kind: "api.issue_created", + summary: `issue created: ${issue.id}`, + data: { issueId: issue.id, addToBacklog: Boolean(body.addToBacklog) }, + }); + return NextResponse.json({ issue: { projectId, ...issue } }, { status: 201 }); } catch (err) { - return NextResponse.json( - { error: err instanceof Error ? err.message : "Failed to create issue" }, - { status: 500 }, - ); + const reason = err instanceof Error ? err.message : "Failed to create issue"; + recordActivityEvent({ + projectId, + source: "api", + kind: "api.issue_create_failed", + level: "error", + summary: `issue create failed: ${reason}`, + data: { reason }, + }); + return NextResponse.json({ error: reason }, { status: 500 }); } } diff --git a/packages/web/src/app/api/orchestrators/route.ts b/packages/web/src/app/api/orchestrators/route.ts index 54890fa85..40e6c3972 100644 --- a/packages/web/src/app/api/orchestrators/route.ts +++ b/packages/web/src/app/api/orchestrators/route.ts @@ -1,9 +1,12 @@ import { type NextRequest, NextResponse } from "next/server"; -import { generateOrchestratorPrompt } from "@aoagents/ao-core"; +import { generateOrchestratorPrompt, recordActivityEvent } from "@aoagents/ao-core"; import { getServices } from "@/lib/services"; import { validateIdentifier, validateConfiguredProject } from "@/lib/validation"; -function classifySpawnError(projectId: string, error: unknown): { +function classifySpawnError( + projectId: string, + error: unknown, +): { status: number; payload: Record; } { @@ -61,6 +64,14 @@ export async function POST(request: NextRequest) { ? await sessionManager.relaunchOrchestrator({ projectId, systemPrompt }) : await sessionManager.spawnOrchestrator({ projectId, systemPrompt }); + recordActivityEvent({ + projectId, + sessionId: session.id, + source: "api", + kind: "api.orchestrator_spawn_requested", + summary: `orchestrator spawn requested for ${projectId}`, + }); + return NextResponse.json( { orchestrator: { diff --git a/packages/web/src/app/api/projects/[id]/route.ts b/packages/web/src/app/api/projects/[id]/route.ts index c98964402..3ac591fb9 100644 --- a/packages/web/src/app/api/projects/[id]/route.ts +++ b/packages/web/src/app/api/projects/[id]/route.ts @@ -9,6 +9,7 @@ import { loadConfig, loadGlobalConfig, loadLocalProjectConfigDetailed, + recordActivityEvent, repairWrappedLocalProjectConfig, unregisterProject, writeLocalProjectConfig, @@ -23,6 +24,7 @@ import { stopStaleWindowsPtyHosts } from "@/lib/windows-pty-cleanup"; export const dynamic = "force-dynamic"; const IDENTITY_FIELDS = new Set(["projectId", "path", "repo", "defaultBranch"]); +const EDITABLE_CONFIG_FIELDS = new Set(["agent", "runtime", "tracker", "scm", "reactions"]); function sanitizeString(value: unknown): string | undefined { if (typeof value !== "string") return undefined; @@ -112,7 +114,10 @@ function getProjectState(projectId: string) { }; } -function degradedPayload(projectId: string, degradedProject: NonNullable["degradedProject"]>) { +function degradedPayload( + projectId: string, + degradedProject: NonNullable["degradedProject"]>, +) { return { error: degradedProject.resolveError, projectId, @@ -126,10 +131,7 @@ function degradedPayload(projectId: string, degradedProject: NonNullable }, -) { +export async function GET(_request: NextRequest, context: { params: Promise<{ id: string }> }) { try { const { id } = await context.params; const state = getProjectState(id); @@ -172,10 +174,7 @@ export async function GET( } } -export async function PATCH( - request: NextRequest, - context: { params: Promise<{ id: string }> }, -) { +export async function PATCH(request: NextRequest, context: { params: Promise<{ id: string }> }) { try { const { id } = await context.params; const body = (await request.json().catch(() => null)) as Record | null; @@ -200,7 +199,10 @@ export async function PATCH( } const projectPath = state.globalEntry?.path; if (!projectPath) { - return NextResponse.json({ error: `Project "${id}" is missing a registry path.` }, { status: 409 }); + return NextResponse.json( + { error: `Project "${id}" is missing a registry path.` }, + { status: 409 }, + ); } const localConfigResult = loadLocalProjectConfigDetailed(projectPath); @@ -211,7 +213,8 @@ export async function PATCH( return NextResponse.json({ error: localConfigResult.error }, { status: 400 }); } - const currentConfig: LocalProjectConfig = localConfigResult.kind === "loaded" ? { ...localConfigResult.config } : {}; + const currentConfig: LocalProjectConfig = + localConfigResult.kind === "loaded" ? { ...localConfigResult.config } : {}; const nextConfig: LocalProjectConfig = { ...currentConfig, }; @@ -231,8 +234,7 @@ export async function PATCH( ...(body["tracker"] as Record), } as LocalProjectConfig["tracker"]) : undefined; - nextConfig.tracker = - nextTracker; + nextConfig.tracker = nextTracker; } if (hasOwn("scm")) { const nextScm = @@ -242,8 +244,7 @@ export async function PATCH( ...(body["scm"] as Record), } as LocalProjectConfig["scm"]) : undefined; - nextConfig.scm = - nextScm; + nextConfig.scm = nextScm; } if (hasOwn("reactions")) { nextConfig.reactions = @@ -261,6 +262,16 @@ export async function PATCH( invalidatePortfolioServicesCache(); revalidateProjectPaths(id); + // Record only changed *keys*, never values — config can contain tokens. + const changedKeys = Object.keys(body).filter((k) => EDITABLE_CONFIG_FIELDS.has(k)); + recordActivityEvent({ + projectId: id, + source: "api", + kind: "api.project_updated", + summary: `project updated: ${id}`, + data: { changedKeys }, + }); + return NextResponse.json({ ok: true }); } catch (error) { return NextResponse.json( @@ -270,19 +281,14 @@ export async function PATCH( } } -export async function PUT( - request: NextRequest, - context: { params: Promise<{ id: string }> }, -) { +export async function PUT(request: NextRequest, context: { params: Promise<{ id: string }> }) { return PATCH(request, context); } -export async function DELETE( - _request: NextRequest, - context: { params: Promise<{ id: string }> }, -) { +export async function DELETE(_request: NextRequest, context: { params: Promise<{ id: string }> }) { + let id: string | undefined; try { - const { id } = await context.params; + ({ id } = await context.params); const state = getProjectState(id); if (!state.globalEntry && !state.project && !state.degradedProject) { return NextResponse.json({ error: `Unknown project: ${id}` }, { status: 404 }); @@ -299,7 +305,8 @@ export async function DELETE( return NextResponse.json({ error: `Invalid project ID: ${id}` }, { status: 400 }); } - const workspacePluginName = state.project?.workspace ?? state.config.defaults.workspace ?? "worktree"; + const workspacePluginName = + state.project?.workspace ?? state.config.defaults.workspace ?? "worktree"; const { registry, sessionManager } = await getServices(); await stopProjectSessions(id, sessionManager); await stopStaleWindowsPtyHosts(projectDir); @@ -310,23 +317,34 @@ export async function DELETE( invalidatePortfolioServicesCache(); revalidateProjectPaths(id); + recordActivityEvent({ + projectId: id, + source: "api", + kind: "api.project_removed", + summary: `project removed: ${id}`, + data: { removedStorageDir: hadStorageDir }, + }); + return NextResponse.json({ ok: true, projectId: id, removedStorageDir: hadStorageDir, }); } catch (error) { - return NextResponse.json( - { error: error instanceof Error ? error.message : "Failed to delete project" }, - { status: 500 }, - ); + const reason = error instanceof Error ? error.message : "Failed to delete project"; + recordActivityEvent({ + projectId: id, + source: "api", + kind: "api.project_remove_failed", + level: "error", + summary: `project remove failed: ${reason}`, + data: { reason }, + }); + return NextResponse.json({ error: reason }, { status: 500 }); } } -export async function POST( - _request: NextRequest, - context: { params: Promise<{ id: string }> }, -) { +export async function POST(_request: NextRequest, context: { params: Promise<{ id: string }> }) { try { const { id } = await context.params; const state = getProjectState(id); @@ -337,7 +355,9 @@ export async function POST( return NextResponse.json({ error: "Project does not need repair." }, { status: 400 }); } - const isWrappedConfigError = state.degradedProject.resolveError.includes("wrapped projects: format"); + const isWrappedConfigError = state.degradedProject.resolveError.includes( + "wrapped projects: format", + ); if (!isWrappedConfigError) { return NextResponse.json( { error: "Automatic repair is not available for this degraded config." }, @@ -349,6 +369,13 @@ export async function POST( invalidatePortfolioServicesCache(); revalidateProjectPaths(id); + recordActivityEvent({ + projectId: id, + source: "api", + kind: "api.project_repaired", + summary: `project repaired: ${id}`, + }); + return NextResponse.json({ ok: true, repaired: true, projectId: id }); } catch (error) { return NextResponse.json( diff --git a/packages/web/src/app/api/projects/reload/route.ts b/packages/web/src/app/api/projects/reload/route.ts index e1a13f333..ecacc9dde 100644 --- a/packages/web/src/app/api/projects/reload/route.ts +++ b/packages/web/src/app/api/projects/reload/route.ts @@ -1,5 +1,10 @@ import { NextResponse } from "next/server"; -import { ConfigNotFoundError, getGlobalConfigPath, loadConfig } from "@aoagents/ao-core"; +import { + ConfigNotFoundError, + getGlobalConfigPath, + loadConfig, + recordActivityEvent, +} from "@aoagents/ao-core"; import { invalidatePortfolioServicesCache } from "@/lib/services"; export const dynamic = "force-dynamic"; @@ -25,10 +30,19 @@ export async function POST() { invalidatePortfolioServicesCache(); const config = loadReloadConfig(); + const projectCount = Object.keys(config.projects).length; + const degradedCount = Object.keys(config.degradedProjects).length; + recordActivityEvent({ + source: "api", + kind: "api.config_reloaded", + summary: `config reloaded: ${projectCount} projects, ${degradedCount} degraded`, + data: { projectCount, degradedCount }, + }); + return NextResponse.json({ reloaded: true, - projectCount: Object.keys(config.projects).length, - degradedCount: Object.keys(config.degradedProjects).length, + projectCount, + degradedCount, }); } catch (error) { return NextResponse.json( diff --git a/packages/web/src/app/api/projects/route.ts b/packages/web/src/app/api/projects/route.ts index 2bafa5212..ca9c1c9ba 100644 --- a/packages/web/src/app/api/projects/route.ts +++ b/packages/web/src/app/api/projects/route.ts @@ -8,6 +8,7 @@ import { getGlobalConfigPath, loadConfig, migrateToGlobalConfig, + recordActivityEvent, registerProjectInGlobalConfig, } from "@aoagents/ao-core"; import { revalidatePath } from "next/cache"; @@ -108,6 +109,12 @@ export async function POST(request: NextRequest) { ); invalidatePortfolioServicesCache(); revalidatePortfolioPaths(registeredProjectId); + recordActivityEvent({ + projectId: registeredProjectId, + source: "api", + kind: "api.project_added", + summary: `project added: ${registeredProjectId}`, + }); return NextResponse.json({ ok: true, projectId: registeredProjectId }, { status: 201 }); } catch (err) { const message = err instanceof Error ? err.message : "Failed to add project"; @@ -124,6 +131,14 @@ export async function POST(request: NextRequest) { if (pathAlreadyRegistered) { const existingProjectId = pathAlreadyRegistered[1]; const suggestedProjectId = generateExternalId(resolvedPath); + recordActivityEvent({ + projectId, + source: "api", + kind: "api.project_add_rejected", + level: "warn", + summary: `project add rejected: path already registered`, + data: { reason: "path_already_registered", existingProjectId, statusCode: 409 }, + }); return NextResponse.json( { error: message, @@ -138,6 +153,14 @@ export async function POST(request: NextRequest) { if (idAlreadyRegistered) { const existingProjectId = idAlreadyRegistered[1]; const suggestedProjectId = generateExternalId(resolvedPath); + recordActivityEvent({ + projectId, + source: "api", + kind: "api.project_add_rejected", + level: "warn", + summary: `project add rejected: id already registered`, + data: { reason: "id_already_registered", existingProjectId, statusCode: 409 }, + }); return NextResponse.json( { error: message, diff --git a/packages/web/src/app/api/prs/[id]/merge/route.ts b/packages/web/src/app/api/prs/[id]/merge/route.ts index 9ab5aed1a..c996a60f9 100644 --- a/packages/web/src/app/api/prs/[id]/merge/route.ts +++ b/packages/web/src/app/api/prs/[id]/merge/route.ts @@ -1,4 +1,5 @@ import { type NextRequest } from "next/server"; +import { recordActivityEvent, type OrchestratorConfig } from "@aoagents/ao-core"; import { getServices, getSCM } from "@/lib/services"; import { getCorrelationId, jsonWithCorrelation, recordApiObservation } from "@/lib/observability"; @@ -11,15 +12,21 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< return jsonWithCorrelation({ error: "Invalid PR number" }, { status: 400 }, correlationId); } const prNumber = Number(id); + let configForObservation: OrchestratorConfig | undefined; + let projectId: string | undefined; + let sessionId: string | undefined; try { const { config, registry, sessionManager } = await getServices(); + configForObservation = config; const sessions = await sessionManager.list(); const session = sessions.find((s) => s.pr?.number === prNumber); if (!session?.pr) { return jsonWithCorrelation({ error: "PR not found" }, { status: 404 }, correlationId); } + projectId = session.projectId; + sessionId = session.id; const project = config.projects[session.projectId]; const scm = getSCM(registry, project); @@ -34,6 +41,15 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< // Validate PR is in a mergeable state const state = await scm.getPRState(session.pr); if (state !== "open") { + recordActivityEvent({ + projectId: session.projectId, + sessionId: session.id, + source: "api", + kind: "api.pr_merge_rejected", + level: "warn", + summary: `PR ${prNumber} merge rejected: state is ${state}`, + data: { prNumber, prState: state, statusCode: 409 }, + }); return jsonWithCorrelation( { error: `PR is ${state}, not open` }, { status: 409 }, @@ -43,6 +59,15 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< const mergeability = await scm.getMergeability(session.pr); if (!mergeability.mergeable) { + recordActivityEvent({ + projectId: session.projectId, + sessionId: session.id, + source: "api", + kind: "api.pr_merge_rejected", + level: "warn", + summary: `PR ${prNumber} merge rejected: not mergeable`, + data: { prNumber, blockers: mergeability.blockers, statusCode: 422 }, + }); return jsonWithCorrelation( { error: "PR is not mergeable", blockers: mergeability.blockers }, { status: 422 }, @@ -63,13 +88,22 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< sessionId: session.id, data: { prNumber }, }); + recordActivityEvent({ + projectId: session.projectId, + sessionId: session.id, + source: "api", + kind: "api.pr_merge_requested", + summary: `PR ${prNumber} merge requested`, + data: { prNumber, method: "squash" }, + }); return jsonWithCorrelation( { ok: true, prNumber, method: "squash" }, { status: 200 }, correlationId, ); } catch (err) { - const { config } = await getServices().catch(() => ({ config: undefined })); + const config = + configForObservation ?? (await getServices().catch(() => ({ config: undefined }))).config; if (config) { recordApiObservation({ config, @@ -79,10 +113,22 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< startedAt, outcome: "failure", statusCode: 500, + projectId, + sessionId, reason: err instanceof Error ? err.message : "Failed to merge PR", data: { prNumber }, }); } + const reason = err instanceof Error ? err.message : "Failed to merge PR"; + recordActivityEvent({ + projectId, + sessionId, + source: "api", + kind: "api.pr_merge_failed", + level: "error", + summary: `PR ${prNumber} merge failed: ${reason}`, + data: { prNumber, reason }, + }); return jsonWithCorrelation( { error: err instanceof Error ? err.message : "Failed to merge PR" }, { status: 500 }, diff --git a/packages/web/src/app/api/sessions/[id]/kill/route.ts b/packages/web/src/app/api/sessions/[id]/kill/route.ts index cf4a6b117..a71bac6e9 100644 --- a/packages/web/src/app/api/sessions/[id]/kill/route.ts +++ b/packages/web/src/app/api/sessions/[id]/kill/route.ts @@ -1,7 +1,7 @@ import { type NextRequest } from "next/server"; import { validateIdentifier } from "@/lib/validation"; import { getServices } from "@/lib/services"; -import { SessionNotFoundError } from "@aoagents/ao-core"; +import { SessionNotFoundError, recordActivityEvent } from "@aoagents/ao-core"; import { getCorrelationId, jsonWithCorrelation, @@ -34,6 +34,13 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< projectId, sessionId: id, }); + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_kill_requested", + summary: `session kill requested: ${id}`, + }); return jsonWithCorrelation({ ok: true, sessionId: id }, { status: 200 }, correlationId); } catch (err) { if (err instanceof SessionNotFoundError) { @@ -56,6 +63,15 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< }); } const msg = err instanceof Error ? err.message : "Failed to kill session"; + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_kill_failed", + level: "error", + summary: `session kill failed: ${msg}`, + data: { reason: msg }, + }); return jsonWithCorrelation({ error: msg }, { status: 500 }, correlationId); } } diff --git a/packages/web/src/app/api/sessions/[id]/message/route.ts b/packages/web/src/app/api/sessions/[id]/message/route.ts index 463927af5..1df3e4bae 100644 --- a/packages/web/src/app/api/sessions/[id]/message/route.ts +++ b/packages/web/src/app/api/sessions/[id]/message/route.ts @@ -1,7 +1,7 @@ import { type NextRequest } from "next/server"; import { getServices } from "@/lib/services"; import { stripControlChars, validateIdentifier, validateString } from "@/lib/validation"; -import { SessionNotFoundError } from "@aoagents/ao-core"; +import { SessionNotFoundError, recordActivityEvent } from "@aoagents/ao-core"; import { getCorrelationId, jsonWithCorrelation, @@ -79,6 +79,14 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ sessionId: id, data: { messageLength: message.length }, }); + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_message_sent", + summary: `message sent to session ${id}`, + data: { messageLength: message.length }, + }); return jsonWithCorrelation({ success: true }, { status: 200 }, correlationId); } catch (err) { const errorMsg = err instanceof Error ? err.message : String(err); @@ -98,6 +106,15 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ if (err instanceof SessionNotFoundError) { return jsonWithCorrelation({ error: err.message }, { status: 404 }, correlationId); } + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_message_failed", + level: "error", + summary: `session message failed: ${errorMsg}`, + data: { messageLength: message.length, reason: errorMsg }, + }); console.error("Failed to send message:", errorMsg); return jsonWithCorrelation( { error: `Failed to send message: ${errorMsg}` }, diff --git a/packages/web/src/app/api/sessions/[id]/remap/route.ts b/packages/web/src/app/api/sessions/[id]/remap/route.ts index d71ea32f9..5c5f0bb83 100644 --- a/packages/web/src/app/api/sessions/[id]/remap/route.ts +++ b/packages/web/src/app/api/sessions/[id]/remap/route.ts @@ -1,7 +1,7 @@ import { type NextRequest } from "next/server"; import { validateIdentifier } from "@/lib/validation"; import { getServices } from "@/lib/services"; -import { SessionNotFoundError } from "@aoagents/ao-core"; +import { SessionNotFoundError, recordActivityEvent } from "@aoagents/ao-core"; import { getCorrelationId, jsonWithCorrelation, @@ -33,6 +33,13 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ projectId, sessionId: id, }); + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_remap_requested", + summary: `session remap requested: ${id}`, + }); return jsonWithCorrelation( { ok: true, sessionId: id, opencodeSessionId }, { status: 200 }, @@ -74,6 +81,15 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ reason: msg, }); } + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_remap_failed", + level: "warn", + summary: `session remap failed: ${msg}`, + data: { reason: msg, statusCode: 422 }, + }); return jsonWithCorrelation({ error: msg }, { status: 422 }, correlationId); } if (config) { @@ -90,6 +106,15 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ reason: msg, }); } + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_remap_failed", + level: "error", + summary: `session remap failed: ${msg}`, + data: { reason: msg, statusCode: 500 }, + }); return jsonWithCorrelation({ error: msg }, { status: 500 }, correlationId); } } diff --git a/packages/web/src/app/api/sessions/[id]/restore/route.ts b/packages/web/src/app/api/sessions/[id]/restore/route.ts index 5372b9012..7c1bea560 100644 --- a/packages/web/src/app/api/sessions/[id]/restore/route.ts +++ b/packages/web/src/app/api/sessions/[id]/restore/route.ts @@ -6,6 +6,8 @@ import { SessionNotRestorableError, WorkspaceMissingError, SessionNotFoundError, + recordActivityEvent, + type OrchestratorConfig, } from "@aoagents/ao-core"; import { getCorrelationId, @@ -24,9 +26,13 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< return jsonWithCorrelation({ error: idErr }, { status: 400 }, correlationId); } + let configForAttribution: OrchestratorConfig | undefined; + let projectIdForAttribution: string | undefined; + try { const { config, sessionManager } = await getServices(); - const projectId = resolveProjectIdForSessionId(config, id); + configForAttribution = config; + projectIdForAttribution = resolveProjectIdForSessionId(config, id); const restored = await sessionManager.restore(id); recordApiObservation({ @@ -37,9 +43,16 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< startedAt, outcome: "success", statusCode: 200, - projectId: restored.projectId ?? projectId, + projectId: restored.projectId ?? projectIdForAttribution, sessionId: id, }); + recordActivityEvent({ + projectId: restored.projectId ?? projectIdForAttribution, + sessionId: id, + source: "api", + kind: "api.session_restore_requested", + summary: `session restore requested: ${id}`, + }); return jsonWithCorrelation( { @@ -54,29 +67,61 @@ export async function POST(_request: NextRequest, { params }: { params: Promise< if (err instanceof SessionNotFoundError) { return jsonWithCorrelation({ error: err.message }, { status: 404 }, correlationId); } + if (!configForAttribution) { + const serviceContext = await getServices().catch(() => undefined); + configForAttribution = serviceContext?.config; + projectIdForAttribution = configForAttribution + ? resolveProjectIdForSessionId(configForAttribution, id) + : undefined; + } if (err instanceof SessionNotRestorableError) { + recordActivityEvent({ + projectId: projectIdForAttribution, + sessionId: id, + source: "api", + kind: "api.session_restore_failed", + level: "warn", + summary: `session restore failed: ${err.message}`, + data: { reason: err.message, statusCode: 409 }, + }); return jsonWithCorrelation({ error: err.message }, { status: 409 }, correlationId); } if (err instanceof WorkspaceMissingError) { + recordActivityEvent({ + projectId: projectIdForAttribution, + sessionId: id, + source: "api", + kind: "api.session_restore_failed", + level: "warn", + summary: `session restore failed: ${err.message}`, + data: { reason: err.message, statusCode: 422 }, + }); return jsonWithCorrelation({ error: err.message }, { status: 422 }, correlationId); } - const { config } = await getServices().catch(() => ({ config: undefined })); - const projectId = config ? resolveProjectIdForSessionId(config, id) : undefined; - if (config) { + if (configForAttribution) { recordApiObservation({ - config, + config: configForAttribution, method: "POST", path: "/api/sessions/[id]/restore", correlationId, startedAt, outcome: "failure", statusCode: 500, - projectId, + projectId: projectIdForAttribution, sessionId: id, reason: err instanceof Error ? err.message : "Failed to restore session", }); } const msg = err instanceof Error ? err.message : "Failed to restore session"; + recordActivityEvent({ + projectId: projectIdForAttribution, + sessionId: id, + source: "api", + kind: "api.session_restore_failed", + level: "error", + summary: `session restore failed: ${msg}`, + data: { reason: msg, statusCode: 500 }, + }); return jsonWithCorrelation({ error: msg }, { status: 500 }, correlationId); } } diff --git a/packages/web/src/app/api/sessions/[id]/send/route.ts b/packages/web/src/app/api/sessions/[id]/send/route.ts index 44a8198df..8c0b67cd2 100644 --- a/packages/web/src/app/api/sessions/[id]/send/route.ts +++ b/packages/web/src/app/api/sessions/[id]/send/route.ts @@ -1,7 +1,7 @@ import { type NextRequest } from "next/server"; import { validateIdentifier, validateString, stripControlChars } from "@/lib/validation"; import { getServices } from "@/lib/services"; -import { SessionNotFoundError } from "@aoagents/ao-core"; +import { SessionNotFoundError, recordActivityEvent } from "@aoagents/ao-core"; import { getCorrelationId, jsonWithCorrelation, @@ -55,6 +55,14 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ sessionId: id, data: { messageLength: message.length }, }); + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_message_sent", + summary: `message sent to session ${id}`, + data: { messageLength: message.length }, + }); return jsonWithCorrelation( { ok: true, sessionId: id, message }, { status: 200 }, @@ -82,6 +90,15 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{ }); } const msg = err instanceof Error ? err.message : "Failed to send message"; + recordActivityEvent({ + projectId, + sessionId: id, + source: "api", + kind: "api.session_message_failed", + level: "error", + summary: `session message failed: ${msg}`, + data: { messageLength: message.length, reason: msg }, + }); return jsonWithCorrelation({ error: msg }, { status: 500 }, correlationId); } } diff --git a/packages/web/src/app/api/setup-labels/route.ts b/packages/web/src/app/api/setup-labels/route.ts index 7b0c32465..75a3b597e 100644 --- a/packages/web/src/app/api/setup-labels/route.ts +++ b/packages/web/src/app/api/setup-labels/route.ts @@ -1,4 +1,5 @@ import { NextResponse } from "next/server"; +import { recordActivityEvent } from "@aoagents/ao-core"; import { getServices } from "@/lib/services"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; @@ -42,6 +43,15 @@ export async function POST() { } } + const created = results.filter((r) => r.status === "created").length; + const exists = results.filter((r) => r.status === "exists").length; + recordActivityEvent({ + source: "api", + kind: "api.labels_setup", + summary: `labels setup complete: ${created} created, ${exists} exists`, + data: { created, exists, total: results.length }, + }); + return NextResponse.json({ results }); } catch (err) { return NextResponse.json( diff --git a/packages/web/src/app/api/spawn/route.ts b/packages/web/src/app/api/spawn/route.ts index 192a4a557..8fcc37fe5 100644 --- a/packages/web/src/app/api/spawn/route.ts +++ b/packages/web/src/app/api/spawn/route.ts @@ -1,4 +1,5 @@ import { type NextRequest } from "next/server"; +import { recordActivityEvent } from "@aoagents/ao-core"; import { validateIdentifier, validateString, validateConfiguredProject } from "@/lib/validation"; import { getServices } from "@/lib/services"; import { sessionToDashboard } from "@/lib/serialize"; @@ -50,6 +51,14 @@ export async function POST(request: NextRequest) { reason: projectErr, data: { issueId: body.issueId }, }); + recordActivityEvent({ + projectId, + source: "api", + kind: "api.session_spawn_rejected", + level: "warn", + summary: `session spawn rejected: ${projectErr}`, + data: { reason: "project_not_configured" }, + }); return jsonWithCorrelation({ error: projectErr }, { status: 404 }, correlationId); } @@ -75,6 +84,17 @@ export async function POST(request: NextRequest) { sessionId: session.id, data: { issueId: session.issueId }, }); + recordActivityEvent({ + projectId: session.projectId, + sessionId: session.id, + source: "api", + kind: "api.session_spawn_requested", + summary: `session spawn requested for ${session.projectId}`, + data: { + issueId: session.issueId ?? undefined, + hasPrompt: Boolean(prompt), + }, + }); return jsonWithCorrelation( { session: sessionToDashboard(session) }, diff --git a/packages/web/src/app/api/verify/route.ts b/packages/web/src/app/api/verify/route.ts index ef2ab0661..b713aaf52 100644 --- a/packages/web/src/app/api/verify/route.ts +++ b/packages/web/src/app/api/verify/route.ts @@ -1,7 +1,7 @@ import { type NextRequest, NextResponse } from "next/server"; import { getVerifyIssues, getServices } from "@/lib/services"; import { validateConfiguredProject } from "@/lib/validation"; -import type { Tracker } from "@aoagents/ao-core"; +import { recordActivityEvent, type Tracker } from "@aoagents/ao-core"; export const dynamic = "force-dynamic"; @@ -25,6 +25,9 @@ export async function GET() { * Body: { issueId: string, projectId: string, action: "verify" | "fail", comment?: string } */ export async function POST(req: NextRequest) { + let issueId: string | undefined; + let projectId: string | undefined; + let action: "verify" | "fail" | undefined; try { const body = (await req.json().catch(() => null)) as | { @@ -37,12 +40,13 @@ export async function POST(req: NextRequest) { if (!body) { return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); } - const { issueId, projectId, action, comment } = body as { + let comment: string | undefined; + ({ issueId, projectId, action, comment } = body as { issueId: string; projectId: string; action: "verify" | "fail"; comment?: string; - }; + }); if (!issueId || !projectId || !action) { return NextResponse.json( @@ -96,11 +100,25 @@ export async function POST(req: NextRequest) { ); } + recordActivityEvent({ + projectId, + source: "api", + kind: "api.issue_verified", + summary: `issue ${issueId} ${action}`, + data: { issueId, action }, + }); + return NextResponse.json({ ok: true }); } catch (err) { - return NextResponse.json( - { error: err instanceof Error ? err.message : "Failed to update issue" }, - { status: 500 }, - ); + const reason = err instanceof Error ? err.message : "Failed to update issue"; + recordActivityEvent({ + projectId, + source: "api", + kind: "api.issue_verify_failed", + level: "error", + summary: `issue verify failed: ${reason}`, + data: { issueId, action, reason }, + }); + return NextResponse.json({ error: reason }, { status: 500 }); } } From 73bed33c2ebe368d51c5b18a36307dab430f7ab4 Mon Sep 17 00:00:00 2001 From: Dhruv Sharma Date: Mon, 18 May 2026 18:54:00 +0530 Subject: [PATCH 3/6] feat(web): activity events for webhooks and mux WebSocket (#1656) (#1693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web): activity events for webhooks and mux WebSocket (#1656) Closes #1656 — adds the 10 activity events called out in the issue, covering webhook ingress (4) and the mux WebSocket terminal server (6). Builds on the ActivityEvent infrastructure landed in #1620. Webhook events (api source): - api.webhook_unverified (warn) — 401 signature verification failure - api.webhook_rejected (warn) — 413 payload exceeds maxBodyBytes - api.webhook_received (info|warn) — 202 success, with parse/lifecycle error counts - api.webhook_failed (error) — 500 outer catch / pipeline crash Mux WS events (ui source — Node-side server only): - ui.terminal_connected — one per mux WS connection - ui.terminal_disconnected — one per close - ui.terminal_heartbeat_lost (warn) — once on 3 missed pongs (was console-only) - ui.terminal_pty_lost (warn) — fires only when subscribers are still attached, distinguishing "PTY actually died" from "user closed browser" - ui.terminal_protocol_error (warn) — invalid mux client message - ui.session_broadcast_failed (warn) — emitted on the healthy→failing transition only; re-arms after a successful poll so a long outage yields one event, not 20/min Invariants honored: no raw payloads or signatures in `data`, no client IPs in `summary` (kept in `data` only), no per-keystroke / per-pong fan-out — only on state transitions. `api.webhook_unverified` is the security-audit event; data captures `slug` and `remoteAddr` but never the failed signature. Co-Authored-By: Claude Opus 4.7 * fix(web): harden webhook and PTY activity events * chore(ci): retrigger checks * fix(web): record PTY loss across mux exit paths --------- Co-authored-by: Claude Opus 4.7 Co-authored-by: whoisasx --- .changeset/activity-events-webhooks-mux.md | 18 + packages/core/src/activity-events.ts | 12 + .../server/__tests__/mux-websocket.test.ts | 526 +++++++++++++++++- packages/web/server/mux-websocket.ts | 237 +++++++- .../web/src/__tests__/webhook-route.test.ts | 328 +++++++++++ .../src/app/api/webhooks/[...slug]/route.ts | 108 +++- packages/web/src/lib/scm-webhooks.ts | 2 +- 7 files changed, 1210 insertions(+), 21 deletions(-) create mode 100644 .changeset/activity-events-webhooks-mux.md create mode 100644 packages/web/src/__tests__/webhook-route.test.ts diff --git a/.changeset/activity-events-webhooks-mux.md b/.changeset/activity-events-webhooks-mux.md new file mode 100644 index 000000000..6f4dcc73f --- /dev/null +++ b/.changeset/activity-events-webhooks-mux.md @@ -0,0 +1,18 @@ +--- +"@aoagents/ao-core": minor +"@aoagents/ao-web": minor +--- + +Wire activity events into webhook ingress and the mux WebSocket terminal server (sub-issue of #1511, follows #1620). + +- `api.webhook_unverified` (warn) — signature verification failed; data includes `slug`, `remoteAddr`, `candidateCount` (never the failed signature) +- `api.webhook_rejected` (warn) — payload exceeded `maxBodyBytes`; data includes counts and `maxBodyBytes` (never the body) +- `api.webhook_received` (info|warn) — accepted webhook; data includes `projectIds`, `matchedSessions`, `parseErrorCount`, `lifecycleErrorCount` (never the body) +- `api.webhook_failed` (error) — outer pipeline crash with `errorMessage` +- `ui.terminal_connected` / `ui.terminal_disconnected` — one event per mux WS connection lifecycle +- `ui.terminal_heartbeat_lost` (warn) — fires once on 3 missed pongs (was console-only) +- `ui.terminal_pty_lost` (warn) — fires when PTY exits with subscribers attached (distinguishes "PTY died" from "user closed browser") +- `ui.terminal_protocol_error` (warn) — invalid mux client message +- `ui.session_broadcast_failed` (warn) — emitted on the healthy→failing transition only (re-arms after a successful poll), so a long outage produces one event, not 20/min + +`api.webhook_unverified` is the security-audit event; treat 401s on webhooks as a signal worth retaining for the full 7-day window. diff --git a/packages/core/src/activity-events.ts b/packages/core/src/activity-events.ts index 789e2616d..aafcc4f42 100644 --- a/packages/core/src/activity-events.ts +++ b/packages/core/src/activity-events.ts @@ -71,6 +71,18 @@ export type ActivityEventKind = | "detecting.escalated" // Report watcher | "report_watcher.triggered" + // Webhook ingress (api source) + | "api.webhook_unverified" + | "api.webhook_rejected" + | "api.webhook_received" + | "api.webhook_failed" + // WebSocket terminal mux (ui source — Node-side server only) + | "ui.terminal_connected" + | "ui.terminal_disconnected" + | "ui.terminal_heartbeat_lost" + | "ui.terminal_pty_lost" + | "ui.terminal_protocol_error" + | "ui.session_broadcast_failed" // Recovery/forensic instrumentation | "recovery.session_failed" | "recovery.action_failed" diff --git a/packages/web/server/__tests__/mux-websocket.test.ts b/packages/web/server/__tests__/mux-websocket.test.ts index 48f38e96f..3e9c41377 100644 --- a/packages/web/server/__tests__/mux-websocket.test.ts +++ b/packages/web/server/__tests__/mux-websocket.test.ts @@ -1,15 +1,26 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { EventEmitter } from "node:events"; +import type { Socket } from "node:net"; +import { WebSocket } from "ws"; import type { SessionBroadcaster as SessionBroadcasterType } from "../mux-websocket"; // vi.mock factories run before module-level statements. Hoist the mock // fns so the factories close over the same instances the tests use. -const { mockSpawn, mockPtySpawn, mockTmuxHasSession } = vi.hoisted(() => ({ +const { mockSpawn, mockPtySpawn, mockTmuxHasSession, recordActivityEvent } = vi.hoisted(() => ({ mockSpawn: vi.fn(), mockPtySpawn: vi.fn(), mockTmuxHasSession: vi.fn(), + recordActivityEvent: vi.fn(), })); +vi.mock("@aoagents/ao-core", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + recordActivityEvent: (event: unknown) => recordActivityEvent(event), + }; +}); + vi.mock("node:child_process", async (importOriginal) => { const actual = (await importOriginal()) as Record; const spawnFn = (...args: unknown[]) => mockSpawn(...args); @@ -34,21 +45,72 @@ vi.mock("../tmux-utils.js", () => ({ findTmux: () => "/usr/bin/tmux", validateSessionId: () => true, resolveTmuxSession: () => "ao-177", + resolvePipePath: () => null, tmuxHasSession: (...args: unknown[]) => mockTmuxHasSession(...args), })); -const { SessionBroadcaster, TerminalManager } = await import("../mux-websocket"); +const { SessionBroadcaster, TerminalManager, createMuxWebSocket, handleWindowsPipeMessage } = + await import("../mux-websocket"); // Mock global fetch const mockFetch = vi.fn(); global.fetch = mockFetch; +type MockPty = { + dataHandlers: Array<(data: string) => void>; + exitHandlers: Array<(event: { exitCode: number }) => void>; + onData: ReturnType; + onExit: ReturnType; + write: ReturnType; + resize: ReturnType; + kill: ReturnType; + emitData: (data: string) => void; + emitExit: (exitCode: number) => Promise; +}; + +const ptyInstances: MockPty[] = []; + +function createMockPty(): MockPty { + const pty = {} as MockPty; + pty.dataHandlers = []; + pty.exitHandlers = []; + pty.onData = vi.fn((handler: (data: string) => void) => { + pty.dataHandlers.push(handler); + }); + pty.onExit = vi.fn((handler: (event: { exitCode: number }) => void) => { + pty.exitHandlers.push(handler); + }); + pty.write = vi.fn(); + pty.resize = vi.fn(); + pty.kill = vi.fn(); + pty.emitData = (data: string) => { + for (const handler of pty.dataHandlers) handler(data); + }; + pty.emitExit = async (exitCode: number) => { + await Promise.all([...pty.exitHandlers].map((handler) => handler({ exitCode }))); + }; + ptyInstances.push(pty); + return pty; +} + +function resetPtyMock(): void { + ptyInstances.length = 0; + mockSpawn.mockReset(); + mockPtySpawn.mockReset(); + mockTmuxHasSession.mockReset(); + mockTmuxHasSession.mockResolvedValue(true); + mockSpawn.mockImplementation(() => new EventEmitter()); + mockPtySpawn.mockImplementation(createMockPty); +} + describe("SessionBroadcaster", () => { let broadcaster: SessionBroadcasterType; beforeEach(() => { vi.useFakeTimers(); mockFetch.mockReset(); + recordActivityEvent.mockClear(); + resetPtyMock(); broadcaster = new SessionBroadcaster("3000"); }); @@ -262,6 +324,451 @@ describe("SessionBroadcaster", () => { expect(mockFetch).toHaveBeenCalledTimes(1); }); }); + + describe("ui.session_broadcast_failed activity events", () => { + function failedKinds(): string[] { + return recordActivityEvent.mock.calls + .map(([e]) => (e as { kind: string }).kind) + .filter((k) => k === "ui.session_broadcast_failed"); + } + + it("emits exactly once on the healthy→failing transition", async () => { + // First fetch fails — triggers emission + mockFetch.mockRejectedValueOnce(new Error("ECONNREFUSED")); + // Second fetch (3s later) also fails — should NOT emit again + mockFetch.mockRejectedValueOnce(new Error("ECONNREFUSED")); + + broadcaster.subscribe(vi.fn()); + await vi.advanceTimersByTimeAsync(10); + await vi.advanceTimersByTimeAsync(3010); + + expect(failedKinds()).toEqual(["ui.session_broadcast_failed"]); + }); + + it("re-arms after recovery (success → failure emits again)", async () => { + // fail → succeed → fail + mockFetch.mockRejectedValueOnce(new Error("net down")); + mockFetch.mockResolvedValueOnce({ ok: true, json: async () => ({ sessions: [] }) }); + mockFetch.mockRejectedValueOnce(new Error("net down again")); + + broadcaster.subscribe(vi.fn()); + await vi.advanceTimersByTimeAsync(10); + await vi.advanceTimersByTimeAsync(3010); // poll #1 → success + await vi.advanceTimersByTimeAsync(3010); // poll #2 → failure + + expect(failedKinds().length).toBe(2); + }); + + it("emits with source=ui, level=warn, and the failure URL in data", async () => { + mockFetch.mockRejectedValueOnce(new Error("ETIMEDOUT")); + + broadcaster.subscribe(vi.fn()); + await vi.advanceTimersByTimeAsync(10); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "ui", + kind: "ui.session_broadcast_failed", + level: "warn", + }), + ); + const call = recordActivityEvent.mock.calls.find( + ([e]) => (e as { kind: string }).kind === "ui.session_broadcast_failed", + )![0] as { data: Record }; + expect(call.data["url"]).toContain("/api/sessions/patches"); + expect(call.data["errorMessage"]).toContain("ETIMEDOUT"); + }); + + it("includes httpStatus when fetch returns non-OK response", async () => { + mockFetch.mockResolvedValueOnce({ ok: false, status: 503 }); + + broadcaster.subscribe(vi.fn()); + await vi.advanceTimersByTimeAsync(10); + + const call = recordActivityEvent.mock.calls.find( + ([e]) => (e as { kind: string }).kind === "ui.session_broadcast_failed", + )![0] as { data: Record }; + expect(call.data["httpStatus"]).toBe(503); + }); + }); +}); + +// ── Connection-level activity events ────────────────────────────────── +// These verify ui.terminal_* events fire at the right WS lifecycle points. +// We exercise the connection handler directly by emitting "connection" on +// the WebSocketServer and feeding a fake ws + IncomingMessage stand-in. + +class FakeWS extends EventEmitter { + readyState: 0 | 1 | 2 | 3 = WebSocket.OPEN; + bufferedAmount = 0; + ping = vi.fn(); + terminate = vi.fn(() => { + this.readyState = WebSocket.CLOSED; + }); + send = vi.fn(); +} + +class FakePipeSocket extends EventEmitter { + write = vi.fn(); + end = vi.fn(() => { + this.emit("close"); + }); + destroy = vi.fn(() => { + this.emit("close"); + }); +} + +function makeFakeRequest(opts?: { remoteAddress?: string; xff?: string }) { + return { + headers: opts?.xff ? { "x-forwarded-for": opts.xff } : {}, + socket: { remoteAddress: opts?.remoteAddress ?? "127.0.0.1" }, + }; +} + +describe("mux WebSocket connection events", () => { + beforeEach(() => { + vi.useFakeTimers(); + recordActivityEvent.mockClear(); + resetPtyMock(); + }); + + afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); + }); + + function emitConnection(opts?: Parameters[0]) { + const wss = createMuxWebSocket(); + if (!wss) { + throw new Error("mux WS server not created — node-pty unavailable"); + } + const ws = new FakeWS(); + wss.emit("connection", ws as unknown as WebSocket, makeFakeRequest(opts)); + return { wss, ws }; + } + + function findEvent(kind: string): { data: Record } | undefined { + const found = recordActivityEvent.mock.calls.find( + ([e]) => (e as { kind: string }).kind === kind, + ); + return found?.[0] as { data: Record } | undefined; + } + + it("emits ui.terminal_connected on a new mux connection (with remoteAddr)", () => { + emitConnection({ xff: "198.51.100.5, 10.0.0.1" }); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ source: "ui", kind: "ui.terminal_connected" }), + ); + const evt = findEvent("ui.terminal_connected")!; + expect(evt.data["remoteAddr"]).toBe("198.51.100.5"); + }); + + it("emits ui.terminal_disconnected exactly once on close", () => { + const { ws } = emitConnection(); + recordActivityEvent.mockClear(); + + ws.emit("close", 1000, Buffer.from("normal")); + + const calls = recordActivityEvent.mock.calls.filter( + ([e]) => (e as { kind: string }).kind === "ui.terminal_disconnected", + ); + expect(calls.length).toBe(1); + const evt = findEvent("ui.terminal_disconnected")!; + expect(evt.data["code"]).toBe(1000); + expect(evt.data["reason"]).toBe("normal"); + }); + + it("emits ui.terminal_heartbeat_lost once on 3 missed pongs and terminates", () => { + const { ws } = emitConnection(); + recordActivityEvent.mockClear(); + + // Each 15s interval sends a ping and increments missedPongs by 1. + // After 3 ticks (45s) it should hit MAX_MISSED_PONGS=3 and terminate. + vi.advanceTimersByTime(15_000); + vi.advanceTimersByTime(15_000); + vi.advanceTimersByTime(15_000); + + const calls = recordActivityEvent.mock.calls.filter( + ([e]) => (e as { kind: string }).kind === "ui.terminal_heartbeat_lost", + ); + expect(calls.length).toBe(1); + expect(ws.terminate).toHaveBeenCalled(); + + // Issue invariant: at most one emit per state change — extra ticks must not + // produce another event. + vi.advanceTimersByTime(15_000); + expect( + recordActivityEvent.mock.calls.filter( + ([e]) => (e as { kind: string }).kind === "ui.terminal_heartbeat_lost", + ).length, + ).toBe(1); + }); + + it("does NOT emit heartbeat_lost when pong arrives before 3 missed pings", () => { + const { ws } = emitConnection(); + recordActivityEvent.mockClear(); + + vi.advanceTimersByTime(15_000); // missedPongs=1 + ws.emit("pong"); // resets to 0 + vi.advanceTimersByTime(15_000); // missedPongs=1 + vi.advanceTimersByTime(15_000); // missedPongs=2 + + expect( + recordActivityEvent.mock.calls.filter( + ([e]) => (e as { kind: string }).kind === "ui.terminal_heartbeat_lost", + ).length, + ).toBe(0); + expect(ws.terminate).not.toHaveBeenCalled(); + }); + + it("emits ui.terminal_protocol_error on malformed client message", () => { + const { ws } = emitConnection(); + recordActivityEvent.mockClear(); + + ws.emit("message", Buffer.from("not-json{{{")); + + const calls = recordActivityEvent.mock.calls.filter( + ([e]) => (e as { kind: string }).kind === "ui.terminal_protocol_error", + ); + expect(calls.length).toBe(1); + const evt = findEvent("ui.terminal_protocol_error")!; + expect(evt.data["errorMessage"]).toBeTruthy(); + }); +}); + +describe("Windows pipe ui.terminal_pty_lost activity events", () => { + beforeEach(() => { + recordActivityEvent.mockClear(); + }); + + function framedMessage(type: number, payload: unknown): Buffer { + const body = Buffer.from(JSON.stringify(payload), "utf-8"); + const header = Buffer.alloc(5); + header.writeUInt8(type, 0); + header.writeUInt32BE(body.length, 1); + return Buffer.concat([header, body]); + } + + function openPipe() { + const ws = new FakeWS(); + const pipe = new FakePipeSocket(); + const winPipes = new Map(); + const winPipeBuffers = new Map(); + const deps = { + connect: vi.fn(() => pipe as unknown as Socket), + resolvePipePath: vi.fn(() => "\\\\.\\pipe\\ao-pty-app-1"), + }; + + handleWindowsPipeMessage( + { id: "app-1", type: "open", projectId: "proj-1" }, + ws, + winPipes, + winPipeBuffers, + deps, + ); + pipe.emit("connect"); + recordActivityEvent.mockClear(); + ws.send.mockClear(); + + return { ws, pipe, winPipes, winPipeBuffers, deps }; + } + + function ptyLostEvents(): Array<{ data: Record; sessionId?: string }> { + return recordActivityEvent.mock.calls + .map(([e]) => e as { kind: string; data: Record; sessionId?: string }) + .filter((event) => event.kind === "ui.terminal_pty_lost"); + } + + it("emits ui.terminal_pty_lost when the PTY host pipe closes while the socket is open", () => { + const { ws, pipe } = openPipe(); + + pipe.emit("close"); + + expect(ptyLostEvents()).toEqual([ + expect.objectContaining({ + sessionId: "app-1", + data: expect.objectContaining({ + sessionId: "app-1", + transport: "windows_pipe", + reason: "pipe_closed", + }), + }), + ]); + expect(ws.send).toHaveBeenCalledWith( + JSON.stringify({ ch: "terminal", id: "app-1", type: "exited", code: 0, projectId: "proj-1" }), + ); + }); + + it("emits ui.terminal_pty_lost when the PTY host reports not alive", () => { + const { ws, pipe } = openPipe(); + + pipe.emit("data", framedMessage(0x07, { alive: false })); + pipe.emit("close"); + + const events = ptyLostEvents(); + expect(events).toHaveLength(1); + expect(events[0]).toEqual( + expect.objectContaining({ + sessionId: "app-1", + data: expect.objectContaining({ + sessionId: "app-1", + transport: "windows_pipe", + reason: "host_not_alive", + }), + }), + ); + expect(ws.send).toHaveBeenCalledWith( + JSON.stringify({ ch: "terminal", id: "app-1", type: "exited", code: 0, projectId: "proj-1" }), + ); + }); + + it("does not emit ui.terminal_pty_lost for an intentional client close", () => { + const { ws, winPipes, winPipeBuffers, deps } = openPipe(); + + handleWindowsPipeMessage( + { id: "app-1", type: "close", projectId: "proj-1" }, + ws, + winPipes, + winPipeBuffers, + deps, + ); + + expect(ptyLostEvents()).toEqual([]); + }); +}); + +describe("TerminalManager ui.terminal_pty_lost activity events", () => { + beforeEach(() => { + recordActivityEvent.mockClear(); + resetPtyMock(); + }); + + function ptyLostEvents(): Array<{ data: Record; sessionId?: string }> { + return recordActivityEvent.mock.calls + .map(([e]) => e as { kind: string; data: Record; sessionId?: string }) + .filter((event) => event.kind === "ui.terminal_pty_lost"); + } + + it("emits ui.terminal_pty_lost when a subscribed PTY exits and reattach fails", async () => { + const manager = new TerminalManager("/usr/bin/tmux"); + manager.open("app-1", "proj-1", "tmux-app-1"); + const pty = ptyInstances[0]; + expect(pty).toBeDefined(); + + manager.subscribe("app-1", "proj-1", vi.fn(), vi.fn()); + mockPtySpawn.mockImplementationOnce(() => { + throw new Error("reattach unavailable"); + }); + + await pty!.emitExit(9); + + const events = ptyLostEvents(); + expect(events).toHaveLength(1); + expect(events[0]).toEqual( + expect.objectContaining({ + sessionId: "app-1", + data: expect.objectContaining({ + sessionId: "app-1", + exitCode: 9, + subscriberCount: 1, + reattachError: "reattach unavailable", + }), + }), + ); + }); + + it("emits ui.terminal_pty_lost when a subscribed PTY exits and reattach succeeds", async () => { + const manager = new TerminalManager("/usr/bin/tmux"); + manager.open("app-1", "proj-1", "tmux-app-1"); + const pty = ptyInstances[0]; + expect(pty).toBeDefined(); + + manager.subscribe("app-1", "proj-1", vi.fn(), vi.fn()); + + await pty!.emitExit(9); + + const events = ptyLostEvents(); + expect(mockPtySpawn).toHaveBeenCalledTimes(2); + expect(events).toHaveLength(1); + expect(events[0]).toEqual( + expect.objectContaining({ + sessionId: "app-1", + data: expect.objectContaining({ + sessionId: "app-1", + exitCode: 9, + subscriberCount: 1, + reattachRecovered: true, + reattachExhausted: false, + }), + }), + ); + }); + + it("does not emit ui.terminal_pty_lost when the last subscriber already left", async () => { + const manager = new TerminalManager("/usr/bin/tmux"); + manager.open("app-1", "proj-1", "tmux-app-1"); + const pty = ptyInstances[0]; + expect(pty).toBeDefined(); + + const unsubscribe = manager.subscribe("app-1", "proj-1", vi.fn(), vi.fn()); + unsubscribe(); + + await pty!.emitExit(0); + + expect(ptyLostEvents()).toEqual([]); + }); + + it("emits ui.terminal_pty_lost at most once across reattach cycles", async () => { + const manager = new TerminalManager("/usr/bin/tmux"); + manager.open("app-1", "proj-1", "tmux-app-1"); + const firstPty = ptyInstances[0]; + expect(firstPty).toBeDefined(); + + manager.subscribe("app-1", "proj-1", vi.fn(), vi.fn()); + mockPtySpawn.mockImplementationOnce(() => { + throw new Error("first reattach unavailable"); + }); + await firstPty!.emitExit(7); + expect(ptyLostEvents()).toHaveLength(1); + + // A client may try to re-open the terminal after the first PTY loss. + // A second failed reattach should not produce another activity event for + // the same terminal entry. + manager.open("app-1", "proj-1", "tmux-app-1"); + const secondPty = ptyInstances[1]; + expect(secondPty).toBeDefined(); + mockPtySpawn.mockImplementationOnce(() => { + throw new Error("second reattach unavailable"); + }); + await secondPty!.emitExit(8); + + expect(ptyLostEvents()).toHaveLength(1); + }); + + it("re-arms ui.terminal_pty_lost after a successful reattach survives the grace period", async () => { + vi.useFakeTimers(); + try { + const manager = new TerminalManager("/usr/bin/tmux"); + manager.open("app-1", "proj-1", "tmux-app-1"); + const firstPty = ptyInstances[0]; + expect(firstPty).toBeDefined(); + + manager.subscribe("app-1", "proj-1", vi.fn(), vi.fn()); + await firstPty!.emitExit(7); + expect(ptyLostEvents()).toHaveLength(1); + + await vi.advanceTimersByTimeAsync(5_000); + + const secondPty = ptyInstances[1]; + expect(secondPty).toBeDefined(); + await secondPty!.emitExit(8); + + expect(ptyLostEvents()).toHaveLength(2); + } finally { + vi.useRealTimers(); + } + }); }); describe("TerminalManager.open — tmux target args (regression for #1714)", () => { @@ -325,6 +832,7 @@ describe("TerminalManager.open — re-attach skipped when tmux session is gone ( mockSpawn.mockReset(); mockPtySpawn.mockReset(); mockTmuxHasSession.mockReset(); + recordActivityEvent.mockClear(); capturedOnExit = undefined; mockSpawn.mockImplementation(() => new EventEmitter()); @@ -355,6 +863,20 @@ describe("TerminalManager.open — re-attach skipped when tmux session is gone ( // Subscribers were notified with the original exit code. expect(exitCb).toHaveBeenCalledTimes(1); expect(exitCb).toHaveBeenCalledWith(0); + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "ui", + kind: "ui.terminal_pty_lost", + level: "warn", + sessionId: "ao-177", + data: expect.objectContaining({ + sessionId: "ao-177", + exitCode: 0, + reattachSkipped: true, + tmuxSessionPresent: false, + }), + }), + ); }); it("still re-attaches when has-session reports the tmux session is alive", async () => { diff --git a/packages/web/server/mux-websocket.ts b/packages/web/server/mux-websocket.ts index 5515574df..5db3d4630 100644 --- a/packages/web/server/mux-websocket.ts +++ b/packages/web/server/mux-websocket.ts @@ -16,7 +16,7 @@ import { tmuxHasSession, validateSessionId, } from "./tmux-utils.js"; -import { getEnvDefaults, isWindows } from "@aoagents/ao-core"; +import { getEnvDefaults, isWindows, recordActivityEvent } from "@aoagents/ao-core"; // These types mirror src/lib/mux-protocol.ts exactly. // tsconfig.server.json constrains rootDir to "server/", so we cannot import @@ -63,6 +63,9 @@ export class SessionBroadcaster { private errorSubscribers = new Set<(error: string) => void>(); private intervalId: ReturnType | null = null; private polling = false; + // Tracks the last fetch outcome so we only emit ui.session_broadcast_failed on + // the healthy → failing transition (not every 3s during an outage). + private lastFetchOk = true; private readonly baseUrl: string; constructor(nextPort: string) { @@ -158,18 +161,42 @@ export class SessionBroadcaster { if (!res.ok) { const msg = `Session fetch failed: HTTP ${res.status}`; console.warn(`[SessionBroadcaster] ${msg}`); + this.recordFetchFailure(msg, { httpStatus: res.status }); return { sessions: null, error: msg }; } const data = (await res.json()) as { sessions?: SessionPatch[] }; + this.lastFetchOk = true; return { sessions: data.sessions ?? null, error: null }; } catch (err) { clearTimeout(timeoutId); const msg = err instanceof Error ? err.message : String(err); console.warn("[SessionBroadcaster] fetchSnapshot error:", msg); + this.recordFetchFailure(msg); return { sessions: null, error: msg }; } } + /** + * Emit ui.session_broadcast_failed once per healthy→failing transition. + * The broadcaster polls every 3s; emitting on every failure during a long + * outage would flood the events table (~20/min). Recovery resets the flag. + */ + private recordFetchFailure(message: string, extra?: Record): void { + if (!this.lastFetchOk) return; + this.lastFetchOk = false; + recordActivityEvent({ + source: "ui", + kind: "ui.session_broadcast_failed", + level: "warn", + summary: `session broadcaster fetch failed: ${message}`, + data: { + url: `${this.baseUrl}/api/sessions/patches`, + errorMessage: message, + ...extra, + }, + }); + } + private disconnect(): void { if (this.intervalId !== null) { clearInterval(this.intervalId); @@ -199,6 +226,7 @@ interface ManagedTerminal { buffer: string[]; bufferBytes: number; reattachAttempts: number; + ptyLostEmitted: boolean; /** * Pending grace-period timer that resets reattachAttempts when the * currently-attached PTY survives REATTACH_RESET_GRACE_MS. Tracked so @@ -276,6 +304,7 @@ export class TerminalManager { buffer: [], bufferBytes: 0, reattachAttempts: 0, + ptyLostEmitted: false, }; this.terminals.set(key, terminal); } @@ -346,6 +375,7 @@ export class TerminalManager { terminal.resetTimer = undefined; if (terminal.pty === pty) { terminal.reattachAttempts = 0; + terminal.ptyLostEmitted = false; } }, REATTACH_RESET_GRACE_MS); terminal.resetTimer.unref(); @@ -383,6 +413,7 @@ export class TerminalManager { pty.onExit(async ({ exitCode }) => { console.log(`[MuxServer] PTY exited for ${id} with code ${exitCode}`); terminal.pty = null; + let reattachError: string | undefined; // Skip the re-attach loop entirely when the underlying tmux session is // gone (e.g. user pressed Ctrl-C in the pane and the launch command @@ -392,15 +423,33 @@ export class TerminalManager { // clean user-initiated termination — see issue #1756. The // MAX_REATTACH_ATTEMPTS bound from #1640 still covers tmux server // hiccups where the session does still exist. - if ( - terminal.subscribers.size > 0 && - !(await tmuxHasSession(this.TMUX, tmuxSessionId)) - ) { + if (terminal.subscribers.size > 0 && !(await tmuxHasSession(this.TMUX, tmuxSessionId))) { console.log(`[MuxServer] tmux session ${tmuxSessionId} is gone, not re-attaching`); if (terminal.resetTimer) { clearTimeout(terminal.resetTimer); terminal.resetTimer = undefined; } + if (!terminal.ptyLostEmitted) { + terminal.ptyLostEmitted = true; + recordActivityEvent({ + projectId, + sessionId: id, + source: "ui", + kind: "ui.terminal_pty_lost", + level: "warn", + summary: `terminal PTY exited (code ${exitCode}) — tmux session gone`, + data: { + sessionId: id, + exitCode, + reattachAttempts: terminal.reattachAttempts, + maxReattachAttempts: MAX_REATTACH_ATTEMPTS, + reattachExhausted: false, + reattachSkipped: true, + tmuxSessionPresent: false, + subscriberCount: terminal.subscribers.size, + }, + }); + } for (const cb of terminal.exitCallbacks) { cb(exitCode); } @@ -423,14 +472,63 @@ export class TerminalManager { ); try { this.open(id, projectId, tmuxSessionId); + if (!terminal.ptyLostEmitted) { + terminal.ptyLostEmitted = true; + recordActivityEvent({ + projectId, + sessionId: id, + source: "ui", + kind: "ui.terminal_pty_lost", + level: "warn", + summary: `terminal PTY exited (code ${exitCode}) — reattached`, + data: { + sessionId: id, + exitCode, + reattachAttempts: terminal.reattachAttempts, + maxReattachAttempts: MAX_REATTACH_ATTEMPTS, + reattachExhausted: false, + reattachRecovered: true, + subscriberCount: terminal.subscribers.size, + }, + }); + } return; // re-attached — don't notify exit } catch (err) { + reattachError = err instanceof Error ? err.message : String(err); console.error(`[MuxServer] Failed to re-attach ${id}:`, err); } } else if (terminal.reattachAttempts >= MAX_REATTACH_ATTEMPTS) { console.error(`[MuxServer] Max re-attach attempts reached for ${id}, giving up`); } + // PTY actually died (vs user closed browser): only emit when subscribers + // are still attached — otherwise the exit is just normal cleanup. + // Keep this event one-shot for the terminal entry. Clients may re-open + // the same terminal after a failed reattach; repeated PTY exits should + // not flood the activity log for the same loss condition. + if (terminal.subscribers.size > 0 && !terminal.ptyLostEmitted) { + terminal.ptyLostEmitted = true; + recordActivityEvent({ + projectId, + sessionId: id, + source: "ui", + kind: "ui.terminal_pty_lost", + level: "warn", + summary: `terminal PTY exited (code ${exitCode})${ + terminal.reattachAttempts >= MAX_REATTACH_ATTEMPTS ? " — reattach exhausted" : "" + }`, + data: { + sessionId: id, + exitCode, + reattachAttempts: terminal.reattachAttempts, + maxReattachAttempts: MAX_REATTACH_ATTEMPTS, + reattachExhausted: terminal.reattachAttempts >= MAX_REATTACH_ATTEMPTS, + subscriberCount: terminal.subscribers.size, + ...(reattachError ? { reattachError } : {}), + }, + }); + } + // Notify subscribers that the terminal has exited (re-attach failed or no subscribers) for (const cb of terminal.exitCallbacks) { cb(exitCode); @@ -515,6 +613,8 @@ export class TerminalManager { // ── Windows Pipe Relay (extracted for testability) ── +const intentionalWinPipeCloses = new WeakSet(); + /** Minimal WebSocket-like interface for the pipe relay handler */ export interface WsSink { send(data: string): void; @@ -586,8 +686,36 @@ export function handleWindowsPipeMessage( const pipeSocket = deps.connect(pipePath); winPipes.set(pipeKey, pipeSocket); winPipeBuffers.set(pipeKey, Buffer.alloc(0)); + let ptyLostEmitted = false; + const recordWindowsPtyLost = ( + reason: "pipe_closed" | "host_not_alive" | "pipe_error", + extra?: Record, + ): void => { + if (ptyLostEmitted || ws.readyState !== WS_OPEN) return; + ptyLostEmitted = true; + recordActivityEvent({ + projectId, + sessionId: id, + source: "ui", + kind: "ui.terminal_pty_lost", + level: "warn", + summary: + reason === "host_not_alive" + ? `terminal PTY host reported not alive for ${id}` + : reason === "pipe_error" + ? `terminal PTY host pipe errored for ${id}` + : `terminal PTY host pipe closed for ${id}`, + data: { + sessionId: id, + transport: "windows_pipe", + reason, + ...extra, + }, + }); + }; pipeSocket.on("error", (err) => { + recordWindowsPtyLost("pipe_error", { errorMessage: err.message }); winPipes.delete(pipeKey); winPipeBuffers.delete(pipeKey); pipeSocket.destroy(); @@ -637,9 +765,8 @@ export function handleWindowsPipeMessage( try { const status = JSON.parse(payload.toString("utf-8")) as { alive: boolean }; if (!status.alive && ws.readyState === WS_OPEN) { - ws.send( - JSON.stringify({ ch: "terminal", id, type: "exited", code: 0, ...echo }), - ); + recordWindowsPtyLost("host_not_alive"); + ws.send(JSON.stringify({ ch: "terminal", id, type: "exited", code: 0, ...echo })); } } catch { /* ignore parse errors */ @@ -651,7 +778,11 @@ export function handleWindowsPipeMessage( pipeSocket.on("close", () => { winPipes.delete(pipeKey); winPipeBuffers.delete(pipeKey); + const intentionalClose = intentionalWinPipeCloses.delete(pipeSocket); if (ws.readyState === WS_OPEN) { + if (!intentionalClose) { + recordWindowsPtyLost("pipe_closed"); + } ws.send(JSON.stringify({ ch: "terminal", id, type: "exited", code: 0, ...echo })); } }); @@ -678,6 +809,7 @@ export function handleWindowsPipeMessage( } else if (type === "close") { const pipeSocket = winPipes.get(pipeKey); if (pipeSocket) { + intentionalWinPipeCloses.add(pipeSocket); pipeSocket.end(); winPipes.delete(pipeKey); winPipeBuffers.delete(pipeKey); @@ -706,9 +838,26 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | const wss = new WebSocketServer({ noServer: true }); - wss.on("connection", (ws) => { + wss.on("connection", (ws, request) => { console.log("[MuxServer] New mux connection"); + const connectedAt = Date.now(); + // Best-effort remote addr — proxy headers if present, else socket peer. + const xff = request?.headers["x-forwarded-for"]; + const xffStr = Array.isArray(xff) ? xff[0] : xff; + const remoteAddr = + (typeof xffStr === "string" ? xffStr.split(",")[0]?.trim() : undefined) ?? + request?.socket?.remoteAddress ?? + undefined; + + recordActivityEvent({ + source: "ui", + kind: "ui.terminal_connected", + level: "info", + summary: "mux WebSocket connection opened", + data: { remoteAddr }, + }); + const subscriptions = new Map void>(); // Windows: named pipe sockets keyed by session ID const winPipes = new Map>(); @@ -716,6 +865,7 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | const winPipeBuffers = new Map(); let sessionUnsubscribe: (() => void) | null = null; let missedPongs = 0; + let heartbeatLostEmitted = false; const MAX_MISSED_PONGS = 3; // Heartbeat: send native WebSocket ping every 15s. @@ -728,6 +878,22 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | missedPongs += 1; if (missedPongs >= MAX_MISSED_PONGS) { console.log("[MuxServer] Too many missed pongs, terminating connection"); + if (!heartbeatLostEmitted) { + heartbeatLostEmitted = true; + recordActivityEvent({ + source: "ui", + kind: "ui.terminal_heartbeat_lost", + level: "warn", + summary: `mux WebSocket heartbeat lost (${missedPongs} missed pongs)`, + data: { + missedPongs, + maxMissedPongs: MAX_MISSED_PONGS, + connectionAgeMs: Date.now() - connectedAt, + remoteAddr, + subscriberCount: subscriptions.size, + }, + }); + } ws.terminate(); } } @@ -759,7 +925,14 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | if (type === "open") { if (isWindows()) { handleWindowsPipeMessage( - msg as { id: string; type: string; projectId?: string; data?: string; cols?: number; rows?: number }, + msg as { + id: string; + type: string; + projectId?: string; + data?: string; + cols?: number; + rows?: number; + }, ws, winPipes, winPipeBuffers, @@ -841,7 +1014,13 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | } else if (type === "resize" && "cols" in msg && "rows" in msg) { if (isWindows()) { handleWindowsPipeMessage( - msg as { id: string; type: string; projectId?: string; cols: number; rows: number }, + msg as { + id: string; + type: string; + projectId?: string; + cols: number; + rows: number; + }, ws, winPipes, winPipeBuffers, @@ -853,7 +1032,14 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | } else if (type === "close") { if (isWindows()) { handleWindowsPipeMessage( - msg as { id: string; type: string; projectId?: string; data?: string; cols?: number; rows?: number }, + msg as { + id: string; + type: string; + projectId?: string; + data?: string; + cols?: number; + rows?: number; + }, ws, winPipes, winPipeBuffers, @@ -903,6 +1089,17 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | } } catch (err) { console.error("[MuxServer] Failed to parse message:", err); + recordActivityEvent({ + source: "ui", + kind: "ui.terminal_protocol_error", + level: "warn", + summary: "invalid mux client message — parse failed", + data: { + errorMessage: err instanceof Error ? err.message : String(err), + remoteAddr, + subscriberCount: subscriptions.size, + }, + }); const errorMsg: ServerMessage = { ch: "system", type: "error", @@ -917,8 +1114,22 @@ export function createMuxWebSocket(tmuxPath?: string | null): WebSocketServer | /** * Handle connection close */ - ws.on("close", () => { + ws.on("close", (code, reason) => { console.log("[MuxServer] Mux connection closed"); + recordActivityEvent({ + source: "ui", + kind: "ui.terminal_disconnected", + level: "info", + summary: "mux WebSocket connection closed", + data: { + code, + reason: reason?.toString("utf8") || undefined, + connectionAgeMs: Date.now() - connectedAt, + subscriberCount: subscriptions.size, + heartbeatLost: heartbeatLostEmitted, + remoteAddr, + }, + }); clearInterval(heartbeatInterval); sessionUnsubscribe?.(); sessionUnsubscribe = null; diff --git a/packages/web/src/__tests__/webhook-route.test.ts b/packages/web/src/__tests__/webhook-route.test.ts new file mode 100644 index 000000000..0bea074e5 --- /dev/null +++ b/packages/web/src/__tests__/webhook-route.test.ts @@ -0,0 +1,328 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { + createInitialCanonicalLifecycle, + createActivitySignal, + type Session, + type SessionManager, + type OrchestratorConfig, + type PluginRegistry, + type SCM, + type LifecycleManager, +} from "@aoagents/ao-core"; + +// Activity event recording is mocked so we can assert what fires without +// touching the real SQLite layer. +const recordActivityEvent = vi.fn(); +vi.mock("@aoagents/ao-core", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + recordActivityEvent: (event: unknown) => recordActivityEvent(event), + }; +}); + +// ── Mock services + plugin registry ─────────────────────────────────── + +function makeSession(id: string, overrides: Partial = {}): Session { + const lifecycle = createInitialCanonicalLifecycle("worker", new Date()); + lifecycle.session.state = "working"; + lifecycle.session.reason = "task_in_progress"; + lifecycle.session.startedAt = lifecycle.session.lastTransitionAt; + lifecycle.runtime.state = "alive"; + lifecycle.runtime.reason = "process_running"; + return { + id, + projectId: "my-app", + status: "working", + activity: "active", + activitySignal: createActivitySignal("valid", { + activity: "active", + timestamp: new Date(), + source: "native", + }), + lifecycle, + branch: "feat/x", + issueId: null, + pr: { + number: 42, + url: "u", + title: "t", + owner: "acme", + repo: "my-app", + branch: "feat/x", + baseBranch: "main", + isDraft: false, + }, + workspacePath: null, + runtimeHandle: null, + agentInfo: null, + createdAt: new Date(), + lastActivityAt: new Date(), + metadata: {}, + ...overrides, + }; +} + +const verifyWebhook = vi.fn(); +const parseWebhook = vi.fn(); +const mockSCM: SCM = { + name: "github", + detectPR: vi.fn(), + getPRState: vi.fn(), + mergePR: vi.fn(), + closePR: vi.fn(), + getCIChecks: vi.fn(), + getCISummary: vi.fn(), + getReviews: vi.fn(), + getReviewDecision: vi.fn(), + getPendingComments: vi.fn(), + getAutomatedComments: vi.fn(), + getMergeability: vi.fn(), + verifyWebhook, + parseWebhook, +} as unknown as SCM; + +const mockRegistry: PluginRegistry = { + register: vi.fn(), + get: vi.fn(() => mockSCM) as PluginRegistry["get"], + list: vi.fn(() => []), + loadBuiltins: vi.fn(), + loadFromConfig: vi.fn(), +}; + +const mockConfig: OrchestratorConfig = { + configPath: "/tmp/agent-orchestrator.yaml", + port: 3000, + readyThresholdMs: 300_000, + defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + projects: { + "my-app": { + name: "My App", + repo: "acme/my-app", + path: "/tmp/my-app", + defaultBranch: "main", + sessionPrefix: "my-app", + scm: { + plugin: "github", + webhook: { enabled: true, path: "/api/webhooks/github", maxBodyBytes: 1024 }, + }, + }, + }, + notifiers: {}, + notificationRouting: { urgent: [], action: [], warning: [], info: [] }, + reactions: {}, +}; + +const mockSessionManager = { + list: vi.fn(async () => [makeSession("s1")]), +} as unknown as SessionManager; + +const mockLifecycle = { + check: vi.fn(async () => {}), +} as unknown as LifecycleManager; + +vi.mock("@/lib/services", () => ({ + getServices: vi.fn(async () => ({ + config: mockConfig, + registry: mockRegistry, + sessionManager: mockSessionManager, + lifecycleManager: mockLifecycle, + })), +})); + +import { POST as webhookPOST } from "@/app/api/webhooks/[...slug]/route"; + +function makeWebhookRequest(opts?: { + body?: string; + contentLength?: number; + headers?: Record; +}): Request { + const body = opts?.body ?? JSON.stringify({ action: "synchronize", number: 42 }); + const headers: Record = { + "content-type": "application/json", + ...opts?.headers, + }; + if (opts?.contentLength !== undefined) { + headers["content-length"] = String(opts.contentLength); + } + return new Request("http://localhost:3000/api/webhooks/github", { + method: "POST", + headers, + body, + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + recordActivityEvent.mockClear(); + verifyWebhook.mockResolvedValue({ ok: true }); + parseWebhook.mockResolvedValue({ + provider: "github", + kind: "pull_request", + action: "synchronize", + rawEventType: "pull_request", + repository: { owner: "acme", name: "my-app" }, + prNumber: 42, + branch: "feat/x", + data: {}, + }); +}); + +// ── Tests ───────────────────────────────────────────────────────────── + +describe("POST /api/webhooks/[...slug] — activity events", () => { + it("rejects unverified webhook with 401 and emits api.webhook_unverified", async () => { + verifyWebhook.mockResolvedValueOnce({ ok: false, reason: "bad signature" }); + const req = makeWebhookRequest({ + headers: { "x-hub-signature-256": "sha256=bogus", "x-forwarded-for": "203.0.113.7" }, + }); + + const res = await webhookPOST(req); + expect(res.status).toBe(401); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.webhook_unverified", + level: "warn", + }), + ); + const call = recordActivityEvent.mock.calls[0]![0] as { + summary: string; + data: Record; + }; + // Critical: signature value must NOT be in data (or anywhere) + expect(JSON.stringify(call)).not.toContain("bogus"); + expect(call.data["slug"]).toBe("github"); + expect(call.data["remoteAddr"]).toBe("203.0.113.7"); + expect(call.data["verificationSupported"]).toBe(true); + expect(call.data["reason"]).toBe("bad signature"); + }); + + it("keeps unsupported webhook verification as 404 while recording audit context", async () => { + vi.mocked(mockRegistry.get).mockReturnValueOnce({ + ...mockSCM, + verifyWebhook: undefined, + } as unknown as SCM); + + const res = await webhookPOST(makeWebhookRequest()); + expect(res.status).toBe(404); + + expect(verifyWebhook).not.toHaveBeenCalled(); + expect(parseWebhook).not.toHaveBeenCalled(); + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.webhook_unverified", + level: "warn", + summary: expect.stringContaining("verification unsupported"), + }), + ); + const call = recordActivityEvent.mock.calls[0]![0] as { + data: Record; + }; + expect(call.data["slug"]).toBe("github"); + expect(call.data["verificationSupported"]).toBe(false); + expect(call.data["unsupportedVerificationCount"]).toBe(1); + expect(call.data["reason"]).toBe("verification_unsupported"); + }); + + it("emits api.webhook_rejected when content-length exceeds maxBodyBytes (413)", async () => { + const req = makeWebhookRequest({ contentLength: 2048 }); // > 1024 max + + const res = await webhookPOST(req); + expect(res.status).toBe(413); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.webhook_rejected", + level: "warn", + }), + ); + const call = recordActivityEvent.mock.calls[0]![0] as { data: Record }; + expect(call.data["slug"]).toBe("github"); + expect(call.data["contentLength"]).toBe(2048); + expect(call.data["maxBodyBytes"]).toBe(1024); + // No body content captured + expect(JSON.stringify(call)).not.toContain("synchronize"); + }); + + it("emits api.webhook_received with counts (not body) on 202 success", async () => { + const req = makeWebhookRequest({ + headers: { "x-forwarded-for": "192.0.2.1" }, + }); + + const res = await webhookPOST(req); + expect(res.status).toBe(202); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.webhook_received", + }), + ); + const call = recordActivityEvent.mock.calls[0]![0] as { data: Record }; + expect(call.data["slug"]).toBe("github"); + expect(call.data["remoteAddr"]).toBe("192.0.2.1"); + expect(call.data["matchedSessions"]).toBe(1); + expect(call.data["parseErrorCount"]).toBe(0); + expect(call.data["lifecycleErrorCount"]).toBe(0); + expect(call.data["projectIds"]).toEqual(["my-app"]); + // Critical: payload body is NOT included + expect(JSON.stringify(call)).not.toContain("synchronize"); + }); + + it("emits api.webhook_received with elevated level when parse/lifecycle errors occurred", async () => { + parseWebhook.mockRejectedValueOnce(new Error("boom")); + // Verification passes so we get into the parse path + verifyWebhook.mockResolvedValueOnce({ ok: true }); + + const res = await webhookPOST(makeWebhookRequest()); + expect(res.status).toBe(202); + + const received = recordActivityEvent.mock.calls.find( + ([e]) => (e as { kind: string }).kind === "api.webhook_received", + ); + expect(received).toBeDefined(); + expect((received![0] as { level: string }).level).toBe("warn"); + expect((received![0] as { data: Record }).data["parseErrorCount"]).toBe(1); + }); + + it("emits api.webhook_failed on 500 outer crash", async () => { + // Force the list() call inside POST to throw, hitting the outer catch. + (mockSessionManager.list as ReturnType).mockRejectedValueOnce( + new Error("session manager exploded"), + ); + + const res = await webhookPOST(makeWebhookRequest()); + expect(res.status).toBe(500); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "api", + kind: "api.webhook_failed", + level: "error", + }), + ); + const call = recordActivityEvent.mock.calls[0]![0] as { data: Record }; + expect(call.data["slug"]).toBe("github"); + expect(call.data["errorMessage"]).toContain("session manager exploded"); + }); + + it("does not emit any event for 404 (unknown path)", async () => { + // Empty config so no candidates match + vi.mocked(mockRegistry.get).mockReturnValueOnce(undefined as unknown as SCM); + const req = new Request("http://localhost:3000/api/webhooks/unknown", { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + + const res = await webhookPOST(req); + expect(res.status).toBe(404); + // No webhook events for 404 — it's a config issue, not an external signal + const kinds = recordActivityEvent.mock.calls.map(([e]) => (e as { kind: string }).kind); + expect(kinds.filter((k) => k.startsWith("api.webhook_"))).toEqual([]); + }); +}); diff --git a/packages/web/src/app/api/webhooks/[...slug]/route.ts b/packages/web/src/app/api/webhooks/[...slug]/route.ts index 9ccf878ad..08660dac8 100644 --- a/packages/web/src/app/api/webhooks/[...slug]/route.ts +++ b/packages/web/src/app/api/webhooks/[...slug]/route.ts @@ -1,4 +1,5 @@ import { NextResponse } from "next/server"; +import { recordActivityEvent } from "@aoagents/ao-core"; import { getServices } from "@/lib/services"; import { buildWebhookRequest, @@ -9,11 +10,35 @@ import { export const dynamic = "force-dynamic"; +const WEBHOOK_PATH_PREFIX = "/api/webhooks/"; + +function deriveSlug(pathname: string): string { + return pathname.startsWith(WEBHOOK_PATH_PREFIX) + ? pathname.slice(WEBHOOK_PATH_PREFIX.length) + : pathname; +} + +function deriveRemoteAddr(request: Request): string | undefined { + // Next.js does not expose the socket peer address on Request. The standard + // proxy headers are the only signal — first hop in x-forwarded-for is the + // original client. Sanitizer in recordActivityEvent does not redact IPs; + // they are intentionally retained for security audit (per issue #1656). + const xff = request.headers.get("x-forwarded-for"); + if (xff) { + const first = xff.split(",")[0]?.trim(); + if (first) return first; + } + return request.headers.get("x-real-ip") ?? undefined; +} + export async function POST(request: Request): Promise { + const pathname = new URL(request.url).pathname; + const slug = deriveSlug(pathname); + const remoteAddr = deriveRemoteAddr(request); + try { const services = await getServices(); - const path = new URL(request.url).pathname; - const candidates = findWebhookProjects(services.config, services.registry, path); + const candidates = findWebhookProjects(services.config, services.registry, pathname); if (candidates.length === 0) { return NextResponse.json( @@ -36,6 +61,19 @@ export async function POST(request: Request): Promise { Number.isFinite(contentLength) && contentLength > maxBodyBytes ) { + recordActivityEvent({ + source: "api", + kind: "api.webhook_rejected", + level: "warn", + summary: `webhook payload exceeded ${maxBodyBytes} bytes for ${slug}`, + data: { + slug, + remoteAddr, + contentLength, + maxBodyBytes, + reason: "payload_too_large", + }, + }); return NextResponse.json( { error: "Webhook payload exceeds configured maxBodyBytes" }, { status: 413 }, @@ -50,12 +88,21 @@ export async function POST(request: Request): Promise { const sessionIds = new Set(); const projectIds = new Set(); let verified = false; + let verificationSupported = false; + let unsupportedVerificationCount = 0; const errors: string[] = []; const parseErrors: string[] = []; const lifecycleErrors: string[] = []; for (const candidate of candidates) { - const verification = await candidate.scm.verifyWebhook?.(webhookRequest, candidate.project); + if (!candidate.scm.verifyWebhook) { + unsupportedVerificationCount += 1; + errors.push("Webhook verification not supported by SCM plugin"); + continue; + } + + verificationSupported = true; + const verification = await candidate.scm.verifyWebhook(webhookRequest, candidate.project); if (!verification?.ok) { if (verification?.reason) errors.push(verification.reason); continue; @@ -93,12 +140,52 @@ export async function POST(request: Request): Promise { } if (!verified) { + const unsupportedOnly = !verificationSupported && unsupportedVerificationCount > 0; + recordActivityEvent({ + source: "api", + kind: "api.webhook_unverified", + level: "warn", + summary: unsupportedOnly + ? `webhook verification unsupported for ${slug}` + : `webhook signature verification failed for ${slug}`, + data: { + slug, + remoteAddr, + candidateCount: candidates.length, + verificationSupported, + unsupportedVerificationCount, + reason: unsupportedOnly + ? "verification_unsupported" + : (errors[0] ?? "verification_failed"), + }, + }); return NextResponse.json( - { error: errors[0] ?? "Webhook verification failed", ok: false }, - { status: 401 }, + { + error: unsupportedOnly + ? "No SCM webhook configured for this path" + : (errors[0] ?? "Webhook verification failed"), + ok: false, + verificationSupported, + }, + { status: unsupportedOnly ? 404 : 401 }, ); } + recordActivityEvent({ + source: "api", + kind: "api.webhook_received", + level: parseErrors.length > 0 || lifecycleErrors.length > 0 ? "warn" : "info", + summary: `webhook accepted for ${slug}: ${sessionIds.size} session(s) matched`, + data: { + slug, + remoteAddr, + projectIds: [...projectIds], + matchedSessions: sessionIds.size, + parseErrorCount: parseErrors.length, + lifecycleErrorCount: lifecycleErrors.length, + }, + }); + return NextResponse.json( { ok: true, @@ -111,6 +198,17 @@ export async function POST(request: Request): Promise { { status: 202 }, ); } catch (err) { + recordActivityEvent({ + source: "api", + kind: "api.webhook_failed", + level: "error", + summary: `webhook pipeline crashed for ${slug}`, + data: { + slug, + remoteAddr, + errorMessage: err instanceof Error ? err.message : String(err), + }, + }); return NextResponse.json( { error: err instanceof Error ? err.message : "Failed to process SCM webhook" }, { status: 500 }, diff --git a/packages/web/src/lib/scm-webhooks.ts b/packages/web/src/lib/scm-webhooks.ts index 63f8d301a..360a8ff6c 100644 --- a/packages/web/src/lib/scm-webhooks.ts +++ b/packages/web/src/lib/scm-webhooks.ts @@ -40,7 +40,7 @@ export function findWebhookProjects( const webhookPath = getProjectWebhookPath(project); if (!webhookPath || webhookPath !== pathname) return []; const scm = registry.get("scm", project.scm.plugin); - if (!scm?.parseWebhook || !scm.verifyWebhook) return []; + if (!scm?.parseWebhook) return []; return [{ projectId, project, scm }]; }); } From be9afb36a40e0b75b38bd978a6f368314784b905 Mon Sep 17 00:00:00 2001 From: Dhruv Sharma Date: Mon, 18 May 2026 19:16:34 +0530 Subject: [PATCH 4/6] feat(core): record activity events for config, plugin-registry, and storage migration (#1696) * feat(core): record activity events for config, plugin-registry, and storage migration Closes #1658. Surfaces failures that previously left no trace in `ao events list`: - Config: project_resolve_failed (degraded project), project_malformed, project_invalid, migrated. Local-config errors carry the project's projectId so `ao events list --project ` finds them. - Plugin registry: load_failed (built-in and external), validation_failed, specifier_failed. External plugin failures are now queryable instead of living only in stderr. - Migration: blocked (active sessions), project_failed (per-project), rename_failed (.migrated rename), completed (one summary with totals), rollback_skipped (post-migration sessions present). - Agent report: api.agent_report.transition_rejected and apply_failed for observability into rejected reports. Migration emits at most one event per milestone (per project failure + single completion summary) so event volume scales with errors, not input size. Config events redact sensitive keys via the existing sanitizer. Adds regression tests for all four MUST emits. Co-Authored-By: Claude Opus 4.7 * fix(core): avoid duplicate config activity events * test(core): cover blocked storage migration event * test(core): satisfy lint in migration event mock * fix(core): emit migration completed for no-op runs --------- Co-authored-by: Claude Opus 4.7 Co-authored-by: whoisasx --- .../activity-events-agent-report.test.ts | 80 +++++++ .../__tests__/activity-events-config.test.ts | 205 ++++++++++++++++ .../activity-events-migration.test.ts | 223 ++++++++++++++++++ .../activity-events-plugin-registry.test.ts | 129 ++++++++++ packages/core/src/activity-events.ts | 40 +++- packages/core/src/agent-report.ts | 210 +++++++++-------- packages/core/src/config.ts | 12 + packages/core/src/global-config.ts | 43 +++- packages/core/src/migration/storage-v2.ts | 86 ++++++- packages/core/src/plugin-registry.ts | 57 +++++ packages/core/src/project-resolver.ts | 2 +- packages/core/src/types.ts | 3 + 12 files changed, 982 insertions(+), 108 deletions(-) create mode 100644 packages/core/src/__tests__/activity-events-agent-report.test.ts create mode 100644 packages/core/src/__tests__/activity-events-config.test.ts create mode 100644 packages/core/src/__tests__/activity-events-migration.test.ts create mode 100644 packages/core/src/__tests__/activity-events-plugin-registry.test.ts diff --git a/packages/core/src/__tests__/activity-events-agent-report.test.ts b/packages/core/src/__tests__/activity-events-agent-report.test.ts new file mode 100644 index 000000000..88c56d71a --- /dev/null +++ b/packages/core/src/__tests__/activity-events-agent-report.test.ts @@ -0,0 +1,80 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdirSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { randomUUID } from "node:crypto"; +import { applyAgentReport } from "../agent-report.js"; +import { writeMetadata, writeCanonicalLifecycle } from "../metadata.js"; +import { createInitialCanonicalLifecycle } from "../lifecycle-state.js"; +import { recordActivityEvent } from "../activity-events.js"; + +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); + +let dataDir: string; +let sessionId: string; + +beforeEach(() => { + dataDir = join(tmpdir(), `ao-test-agent-report-events-${randomUUID()}`); + mkdirSync(dataDir, { recursive: true }); + sessionId = "ao-1"; + vi.mocked(recordActivityEvent).mockClear(); +}); + +afterEach(() => { + rmSync(dataDir, { recursive: true, force: true }); +}); + +describe("activity events: agent-report", () => { + it("emits api.agent_report.transition_rejected when the transition is invalid", () => { + const lifecycle = createInitialCanonicalLifecycle("worker"); + lifecycle.session.state = "terminated"; + writeMetadata(dataDir, sessionId, { + worktree: "/tmp/worktree", + branch: "feat/x", + status: "terminated", + project: "demo", + }); + writeCanonicalLifecycle(dataDir, sessionId, lifecycle); + + expect(() => + applyAgentReport(dataDir, sessionId, { + state: "working", + now: new Date(), + }), + ).toThrow(); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const rejected = calls.find( + (c) => c.kind === "api.agent_report.transition_rejected", + ); + expect(rejected).toBeDefined(); + expect(rejected?.sessionId).toBe(sessionId); + expect(rejected?.source).toBe("api"); + }); + + it("does not emit transition_rejected on a valid transition", () => { + const lifecycle = createInitialCanonicalLifecycle("worker"); + lifecycle.session.state = "working"; + lifecycle.session.reason = "task_in_progress"; + lifecycle.session.startedAt = "2024-12-01T00:00:00.000Z"; + lifecycle.runtime.state = "alive"; + lifecycle.runtime.reason = "process_running"; + writeMetadata(dataDir, sessionId, { + worktree: "/tmp/worktree", + branch: "feat/x", + status: "working", + project: "demo", + }); + writeCanonicalLifecycle(dataDir, sessionId, lifecycle); + + applyAgentReport(dataDir, sessionId, { + state: "needs_input", + now: new Date(), + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + expect(calls.find((c) => c.kind === "api.agent_report.transition_rejected")).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/activity-events-config.test.ts b/packages/core/src/__tests__/activity-events-config.test.ts new file mode 100644 index 000000000..29db59c50 --- /dev/null +++ b/packages/core/src/__tests__/activity-events-config.test.ts @@ -0,0 +1,205 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { loadConfig } from "../config.js"; +import { recordActivityEvent } from "../activity-events.js"; +import { saveGlobalConfig, type GlobalConfig } from "../global-config.js"; + +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); + +function makeGlobalConfig(projects: GlobalConfig["projects"] = {}): GlobalConfig { + return { + port: 3000, + readyThresholdMs: 300_000, + defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + projects, + notifiers: {}, + notificationRouting: {}, + reactions: {}, + }; +} + +describe("activity events: config loading", () => { + let tempRoot: string; + let configPath: string; + let originalHome: string | undefined; + let originalGlobalConfig: string | undefined; + + beforeEach(() => { + tempRoot = join( + tmpdir(), + `ao-config-events-${Date.now()}-${Math.random().toString(16).slice(2)}`, + ); + configPath = join(tempRoot, ".agent-orchestrator", "config.yaml"); + mkdirSync(tempRoot, { recursive: true }); + originalHome = process.env["HOME"]; + originalGlobalConfig = process.env["AO_GLOBAL_CONFIG"]; + process.env["HOME"] = tempRoot; + process.env["AO_GLOBAL_CONFIG"] = configPath; + vi.mocked(recordActivityEvent).mockClear(); + }); + + afterEach(() => { + process.env["HOME"] = originalHome; + if (originalGlobalConfig === undefined) { + delete process.env["AO_GLOBAL_CONFIG"]; + } else { + process.env["AO_GLOBAL_CONFIG"] = originalGlobalConfig; + } + rmSync(tempRoot, { recursive: true, force: true }); + }); + + it("emits config.project_resolve_failed for unresolved projects without a specific config event", () => { + const projectPath = join(tempRoot, "old-format"); + mkdirSync(projectPath, { recursive: true }); + writeFileSync( + join(projectPath, "agent-orchestrator.yaml"), + ["projects:", " old-format:", " path: .", ""].join("\n"), + ); + + saveGlobalConfig( + makeGlobalConfig({ + "old-format": { + projectId: "old-format", + path: projectPath, + displayName: "Old format", + defaultBranch: "main", + sessionPrefix: "old-format", + }, + }), + configPath, + ); + + loadConfig(configPath); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "config", + kind: "config.project_resolve_failed", + projectId: "old-format", + }), + ); + }); + + it("does not emit config.project_resolve_failed for malformed local yaml", () => { + const projectPath = join(tempRoot, "malformed"); + mkdirSync(projectPath, { recursive: true }); + writeFileSync(join(projectPath, "agent-orchestrator.yaml"), "tracker: [\n"); + + saveGlobalConfig( + makeGlobalConfig({ + malformed: { + projectId: "malformed", + path: projectPath, + displayName: "Malformed", + defaultBranch: "main", + sessionPrefix: "malformed", + }, + }), + configPath, + ); + + loadConfig(configPath); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "config", + kind: "config.project_malformed", + projectId: "malformed", + }), + ); + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + expect(calls.find((c) => c.kind === "config.project_resolve_failed")).toBeUndefined(); + }); + + it("does not emit config.project_resolve_failed for healthy projects", () => { + const projectPath = join(tempRoot, "clean"); + mkdirSync(projectPath, { recursive: true }); + writeFileSync( + join(projectPath, "agent-orchestrator.yaml"), + ["agent: codex", "runtime: tmux", "workspace: worktree", ""].join("\n"), + ); + + saveGlobalConfig( + makeGlobalConfig({ + clean: { + projectId: "clean", + path: projectPath, + displayName: "Clean", + defaultBranch: "main", + sessionPrefix: "clean", + }, + }), + configPath, + ); + + loadConfig(configPath); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + expect(calls.find((c) => c.kind === "config.project_resolve_failed")).toBeUndefined(); + }); + + it("emits config.project_malformed for unparseable local yaml", () => { + const projectPath = join(tempRoot, "malformed"); + mkdirSync(projectPath, { recursive: true }); + writeFileSync(join(projectPath, "agent-orchestrator.yaml"), "tracker: [\n"); + + saveGlobalConfig( + makeGlobalConfig({ + malformed: { + projectId: "malformed", + path: projectPath, + displayName: "Malformed", + defaultBranch: "main", + sessionPrefix: "malformed", + }, + }), + configPath, + ); + + loadConfig(configPath); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "config", + kind: "config.project_malformed", + projectId: "malformed", + }), + ); + }); + + it("emits config.project_invalid for schema-invalid local yaml", () => { + const projectPath = join(tempRoot, "invalid"); + mkdirSync(projectPath, { recursive: true }); + // Valid YAML but fails LocalProjectConfigSchema (numeric agent isn't a string) + writeFileSync(join(projectPath, "agent-orchestrator.yaml"), "agent: 123\n"); + + saveGlobalConfig( + makeGlobalConfig({ + invalid: { + projectId: "invalid", + path: projectPath, + displayName: "Invalid", + defaultBranch: "main", + sessionPrefix: "invalid", + }, + }), + configPath, + ); + + loadConfig(configPath); + + expect(recordActivityEvent).toHaveBeenCalledWith( + expect.objectContaining({ + source: "config", + kind: "config.project_invalid", + projectId: "invalid", + }), + ); + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + expect(calls.find((c) => c.kind === "config.project_resolve_failed")).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/activity-events-migration.test.ts b/packages/core/src/__tests__/activity-events-migration.test.ts new file mode 100644 index 000000000..efaf60a94 --- /dev/null +++ b/packages/core/src/__tests__/activity-events-migration.test.ts @@ -0,0 +1,223 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { execSync } from "node:child_process"; +import { migrateStorage } from "../migration/storage-v2.js"; +import { recordActivityEvent } from "../activity-events.js"; + +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); + +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...(actual as Record), + execSync: vi.fn(), + }; +}); + +function createTempDir(): string { + const dir = join( + tmpdir(), + `ao-migration-events-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(dir, { recursive: true }); + return dir; +} + +describe("activity events: storage migration", () => { + let testDir: string; + let aoBaseDir: string; + let configPath: string; + + beforeEach(() => { + testDir = createTempDir(); + aoBaseDir = join(testDir, ".agent-orchestrator"); + configPath = join(aoBaseDir, "config.yaml"); + mkdirSync(aoBaseDir, { recursive: true }); + vi.mocked(recordActivityEvent).mockClear(); + vi.mocked(execSync).mockReturnValue(""); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + }); + + it("emits migration.completed when there is nothing to migrate", async () => { + const result = await migrateStorage({ + aoBaseDir, + globalConfigPath: configPath, + force: true, + log: () => {}, + }); + + expect(result.projects).toBe(0); + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const completed = calls.find((c) => c.kind === "migration.completed"); + expect(completed).toBeDefined(); + expect(completed?.source).toBe("migration"); + expect(completed?.level).toBe("info"); + expect(completed?.data).toMatchObject({ + projectsMigrated: 0, + sessions: 0, + worktrees: 0, + projectErrors: 0, + }); + }); + + it("emits migration.completed with totals when migration succeeds", async () => { + const hashDir = join(aoBaseDir, "aaaaaa000000-myproject"); + mkdirSync(join(hashDir, "sessions"), { recursive: true }); + mkdirSync(join(hashDir, "worktrees", "ao-1"), { recursive: true }); + writeFileSync( + join(hashDir, "sessions", "ao-1"), + [ + "project=myproject", + "agent=claude-code", + "status=working", + "createdAt=2026-04-21T12:00:00.000Z", + "branch=session/ao-1", + `worktree=${join(hashDir, "worktrees", "ao-1")}`, + ].join("\n"), + ); + writeFileSync( + configPath, + [ + "projects:", + " myproject:", + " path: /home/user/myproject", + " storageKey: aaaaaa000000", + " defaultBranch: main", + "", + ].join("\n"), + ); + + await migrateStorage({ + aoBaseDir, + globalConfigPath: configPath, + force: true, + log: () => {}, + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const completed = calls.find((c) => c.kind === "migration.completed"); + expect(completed).toBeDefined(); + const data = (completed?.data ?? {}) as Record; + expect(data["projectsMigrated"]).toBe(1); + expect(data["projectErrors"]).toBe(0); + }); + + it("emits migration.project_failed plus migration.completed when a project fails", async () => { + // Force migrateProject to throw by pre-creating projects/badproj as a FILE + // — mkdirSync with recursive:true tolerates existing directories but NOT existing + // files at the same path, so it throws EEXIST when migrateProject tries to create + // projects/badproj/sessions. + const badDir = join(aoBaseDir, "bbbbbb000000-badproj"); + mkdirSync(join(badDir, "sessions"), { recursive: true }); + writeFileSync( + join(badDir, "sessions", "ao-99"), + [ + "project=badproj", + "agent=claude-code", + "createdAt=2026-04-21T12:00:00.000Z", + "branch=session/ao-99", + `worktree=${join(badDir, "worktrees", "ao-99")}`, + ].join("\n"), + ); + + // Pre-create projects/badproj as a regular file so mkdirSync inside migrateProject + // raises ENOTDIR / EEXIST when it tries to create a subdirectory under it. + mkdirSync(join(aoBaseDir, "projects"), { recursive: true }); + writeFileSync(join(aoBaseDir, "projects", "badproj"), "blocker"); + + writeFileSync( + configPath, + [ + "projects:", + " badproj:", + " path: /home/user/badproj", + " storageKey: bbbbbb000000", + " defaultBranch: main", + "", + ].join("\n"), + ); + + await migrateStorage({ + aoBaseDir, + globalConfigPath: configPath, + force: true, + log: () => {}, + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const projectFailed = calls.filter((c) => c.kind === "migration.project_failed"); + const completed = calls.find((c) => c.kind === "migration.completed"); + + expect(projectFailed.length).toBeGreaterThanOrEqual(1); + expect(projectFailed[0]?.projectId).toBe("badproj"); + expect(completed).toBeDefined(); + const completedData = (completed?.data ?? {}) as Record; + expect(completedData["projectErrors"]).toBeGreaterThanOrEqual(1); + }); + + it("emits migration.blocked when active sessions are detected", async () => { + vi.mocked(execSync).mockReturnValue("ao-1\nabcdef012345-worker-7\nunrelated\n"); + + writeFileSync( + configPath, + "projects:\n myproject:\n path: /tmp/x\n storageKey: aaaaaa000000\n defaultBranch: main\n", + ); + + await expect(migrateStorage({ + aoBaseDir, + globalConfigPath: configPath, + log: () => {}, + })).rejects.toThrow(/Found 2 active AO tmux session/); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const blocked = calls.find((c) => c.kind === "migration.blocked"); + expect(blocked).toBeDefined(); + expect(blocked?.source).toBe("migration"); + expect(blocked?.level).toBe("warn"); + expect(blocked?.summary).toBe("migration blocked by 2 active session(s)"); + expect(blocked?.data).toMatchObject({ + activeSessionCount: 2, + sample: ["ao-1", "abcdef012345-worker-7"], + }); + }); + + it("does not emit migration.blocked when force skips active-session detection", async () => { + vi.mocked(execSync).mockReturnValue("ao-1\n"); + + const hashDir = join(aoBaseDir, "aaaaaa000000-myproject"); + mkdirSync(join(hashDir, "sessions"), { recursive: true }); + writeFileSync( + join(hashDir, "sessions", "ao-1"), + [ + "project=myproject", + "agent=claude-code", + "createdAt=2026-04-21T12:00:00.000Z", + "branch=session/ao-1", + `worktree=${join(hashDir, "worktrees", "ao-1")}`, + ].join("\n"), + ); + mkdirSync(join(hashDir, "worktrees", "ao-1"), { recursive: true }); + writeFileSync( + configPath, + "projects:\n myproject:\n path: /tmp/x\n storageKey: aaaaaa000000\n defaultBranch: main\n", + ); + + // force: true skips the active-session check, so no migration.blocked event. + await migrateStorage({ + aoBaseDir, + globalConfigPath: configPath, + force: true, + log: () => {}, + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + expect(calls.find((c) => c.kind === "migration.blocked")).toBeUndefined(); + }); +}); diff --git a/packages/core/src/__tests__/activity-events-plugin-registry.test.ts b/packages/core/src/__tests__/activity-events-plugin-registry.test.ts new file mode 100644 index 000000000..ab5a3187a --- /dev/null +++ b/packages/core/src/__tests__/activity-events-plugin-registry.test.ts @@ -0,0 +1,129 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createPluginRegistry } from "../plugin-registry.js"; +import { recordActivityEvent } from "../activity-events.js"; +import type { OrchestratorConfig, PluginManifest, PluginModule } from "../types.js"; + +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); + +function makePlugin(slot: PluginManifest["slot"], name: string): PluginModule { + return { + manifest: { + name, + slot, + description: `Test ${slot} plugin: ${name}`, + version: "0.0.1", + }, + create: vi.fn(() => ({ name })), + }; +} + +function makeOrchestratorConfig(overrides?: Partial): OrchestratorConfig { + return { + projects: {}, + ...overrides, + } as OrchestratorConfig; +} + +beforeEach(() => { + vi.mocked(recordActivityEvent).mockClear(); +}); + +describe("activity events: plugin-registry", () => { + it("emits plugin-registry.load_failed when a configured external plugin fails to import", async () => { + const registry = createPluginRegistry(); + const config = makeOrchestratorConfig({ + plugins: [ + { + name: "broken-plugin", + source: "npm", + package: "@example/broken", + enabled: true, + }, + ], + }); + + await registry.loadFromConfig(config, async (pkg: string) => { + // Built-ins all silently skip; external import throws + if (pkg === "@example/broken") { + throw new Error("module not found"); + } + throw new Error(`builtin not installed: ${pkg}`); + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const loadFailed = calls.find( + (c) => + c.kind === "plugin-registry.load_failed" && + c.source === "plugin-registry" && + (c.data as Record | undefined)?.["builtin"] === false, + ); + expect(loadFailed).toBeDefined(); + }); + + it("emits plugin-registry.specifier_failed when a plugin specifier cannot be resolved", async () => { + const registry = createPluginRegistry(); + const config = makeOrchestratorConfig({ + plugins: [ + { + name: "no-specifier", + source: "local", + // No path — resolvePluginSpecifier returns null + enabled: true, + }, + ], + }); + + await registry.loadFromConfig(config, async (pkg: string) => { + throw new Error(`builtin not installed: ${pkg}`); + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const specifierFailed = calls.find( + (c) => c.kind === "plugin-registry.specifier_failed", + ); + expect(specifierFailed).toBeDefined(); + }); + + it("emits plugin-registry.load_failed when a built-in plugin's register() throws", async () => { + const registry = createPluginRegistry(); + // Make a plugin whose create() throws to force registration failure + const fakeRuntime: PluginModule = { + manifest: { + name: "tmux", + slot: "runtime", + description: "throwing test plugin", + version: "0.0.1", + }, + create: () => { + throw new Error("boom"); + }, + }; + + await registry.loadBuiltins(undefined, async (pkg: string) => { + if (pkg === "@aoagents/ao-plugin-runtime-tmux") return fakeRuntime; + throw new Error(`Not found: ${pkg}`); + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const loadFailed = calls.find((c) => c.kind === "plugin-registry.load_failed"); + expect(loadFailed).toBeDefined(); + }); + + it("does not emit any failure events when plugins load cleanly", async () => { + const registry = createPluginRegistry(); + const fakePlugin = makePlugin("runtime", "tmux"); + + await registry.loadBuiltins(undefined, async (pkg: string) => { + if (pkg === "@aoagents/ao-plugin-runtime-tmux") return fakePlugin; + throw new Error(`Not found: ${pkg}`); + }); + + const calls = vi.mocked(recordActivityEvent).mock.calls.map((c) => c[0]); + const failures = calls.filter((c) => + typeof c.kind === "string" && c.kind.startsWith("plugin-registry."), + ); + expect(failures).toEqual([]); + }); +}); diff --git a/packages/core/src/activity-events.ts b/packages/core/src/activity-events.ts index aafcc4f42..87b6c9175 100644 --- a/packages/core/src/activity-events.ts +++ b/packages/core/src/activity-events.ts @@ -25,6 +25,9 @@ export type ActivityEventSource = | "notifier" | "reaction" | "report-watcher" + | "config" + | "plugin-registry" + | "migration" | "recovery"; export type ActivityEventKind = @@ -71,6 +74,19 @@ export type ActivityEventKind = | "detecting.escalated" // Report watcher | "report_watcher.triggered" + // Config/plugin-registry/storage migration + | "config.project_resolve_failed" + | "config.project_malformed" + | "config.project_invalid" + | "config.migrated" + | "plugin-registry.load_failed" + | "plugin-registry.validation_failed" + | "plugin-registry.specifier_failed" + | "migration.blocked" + | "migration.project_failed" + | "migration.rename_failed" + | "migration.completed" + | "migration.rollback_skipped" // Webhook ingress (api source) | "api.webhook_unverified" | "api.webhook_rejected" @@ -88,7 +104,8 @@ export type ActivityEventKind = | "recovery.action_failed" | "metadata.corrupt_detected" | "api.agent_report.session_not_found" - | "api.agent_report.transition_rejected"; + | "api.agent_report.transition_rejected" + | "api.agent_report.apply_failed"; export type ActivityEventLevel = "debug" | "info" | "warn" | "error"; @@ -128,14 +145,12 @@ export function droppedEventCount(): number { } function pruneOldEvents(db: ReturnType, cutoff: number): void { - db - ?.prepare( - `DELETE FROM activity_events + db?.prepare( + `DELETE FROM activity_events WHERE rowid IN ( SELECT rowid FROM activity_events WHERE ts_epoch < ? LIMIT ? )`, - ) - .run(cutoff, PRUNE_BATCH_SIZE); + ).run(cutoff, PRUNE_BATCH_SIZE); } // Patterns that indicate sensitive field names @@ -152,7 +167,10 @@ function redactCredentialUrls(input: string): string { const proto = result.indexOf("://", offset); if (proto === -1) break; // Only match http:// or https:// (case-insensitive, matching old /gi flag) - if (proto < 4) { offset = proto + 3; continue; } + if (proto < 4) { + offset = proto + 3; + continue; + } const schemeEnd = result.slice(Math.max(0, proto - 5), proto).toLowerCase(); if (!schemeEnd.endsWith("http") && !schemeEnd.endsWith("https")) { offset = proto + 3; @@ -163,7 +181,7 @@ function redactCredentialUrls(input: string): string { while (cursor < result.length) { const ch = result.charCodeAt(cursor); // Space/control chars or '/' mean no '@' is coming in userinfo - if (ch <= 0x20 || ch === 0x2F) break; + if (ch <= 0x20 || ch === 0x2f) break; if (ch === 0x40) { // '@' found — redact everything between :// and @ // Lowercase the scheme to match the old /gi regex behavior @@ -176,7 +194,11 @@ function redactCredentialUrls(input: string): string { cursor++; } // No '@' found — not a credential URL, move past this :// - if (cursor >= result.length || result.charCodeAt(cursor) <= 0x20 || result.charCodeAt(cursor) === 0x2F) { + if ( + cursor >= result.length || + result.charCodeAt(cursor) <= 0x20 || + result.charCodeAt(cursor) === 0x2f + ) { offset = proto + 3; } } diff --git a/packages/core/src/agent-report.ts b/packages/core/src/agent-report.ts index 13b1a187a..091f56a3d 100644 --- a/packages/core/src/agent-report.ts +++ b/packages/core/src/agent-report.ts @@ -28,7 +28,12 @@ import type { } from "./types.js"; import { recordActivityEvent } from "./activity-events.js"; import { mutateMetadata, readMetadataRaw } from "./metadata.js"; -import { buildLifecycleMetadataPatch, cloneLifecycle, deriveLegacyStatus, parseCanonicalLifecycle } from "./lifecycle-state.js"; +import { + buildLifecycleMetadataPatch, + cloneLifecycle, + deriveLegacyStatus, + parseCanonicalLifecycle, +} from "./lifecycle-state.js"; import { parsePrFromUrl } from "./utils/pr.js"; import { assertValidSessionIdComponent } from "./utils/session-id.js"; import { validateStatus } from "./utils/validation.js"; @@ -448,111 +453,124 @@ export function applyAgentReport( let legacyStatus: SessionStatus | null = null; let previousLegacyStatus: SessionStatus | null = null; - const nextMetadata = mutateMetadata(dataDir, sessionId, (existing) => { - const current = cloneLifecycle( - parseCanonicalLifecycle(existing, { - sessionId, - status: validateStatus(existing["status"]), - }), - ); - previousLegacyStatus = deriveLegacyStatus(current); - before = buildAuditSnapshot(current, previousLegacyStatus); - const validation = validateAgentReportTransition(current, input.state); - if (!validation.ok) { - const rejectionReason = validation.reason ?? "transition rejected"; - appendAgentReportAuditEntry(dataDir, sessionId, { - timestamp: now, - actor, - source, - reportState: input.state, - note: trimmedNote, - prNumber, - prUrl: trimmedPrUrl, - prIsDraft, - accepted: false, - rejectionReason, - before, - after: before, - }); - recordActivityEvent({ - projectId, - sessionId, - source: "api", - kind: "api.agent_report.transition_rejected", - level: "warn", - summary: `applyAgentReport rejected ${input.state} for ${sessionId}: ${rejectionReason}`, - data: { - reportState: input.state, - rejectionReason, + const nextMetadata = mutateMetadata( + dataDir, + sessionId, + (existing) => { + const current = cloneLifecycle( + parseCanonicalLifecycle(existing, { + sessionId, + status: validateStatus(existing["status"]), + }), + ); + previousLegacyStatus = deriveLegacyStatus(current); + before = buildAuditSnapshot(current, previousLegacyStatus); + const validation = validateAgentReportTransition(current, input.state); + if (!validation.ok) { + const rejectionReason = validation.reason ?? "transition rejected"; + appendAgentReportAuditEntry(dataDir, sessionId, { + timestamp: now, actor, - reportSource: source, - fromState: current.session.state, - fromReason: current.session.reason, - legacyStatus: previousLegacyStatus, - }, - }); - throw new Error(rejectionReason); - } - const mapped = mapAgentReportToLifecycle(input.state); - previousState = current.session.state; - nextState = mapped.sessionState; - current.session.state = mapped.sessionState; - current.session.reason = mapped.sessionReason; - current.session.lastTransitionAt = now; - if (isPRWorkflowReport(input.state)) { - const effectivePrUrl = trimmedPrUrl ?? current.pr.url ?? existingPrUrl; - const effectivePrNumber = - prNumber ?? current.pr.number ?? existingPrNumber ?? parsedPrFromUrl?.number; - const canAdvancePrState = - effectivePrUrl !== undefined || - effectivePrNumber !== undefined || - current.pr.state !== "none"; - if (canAdvancePrState) { - current.pr.state = "open"; - current.pr.reason = - input.state === "ready_for_review" ? "review_pending" : "in_progress"; - current.pr.lastObservedAt = now; + source, + reportState: input.state, + note: trimmedNote, + prNumber, + prUrl: trimmedPrUrl, + prIsDraft, + accepted: false, + rejectionReason, + before, + after: before, + }); + recordActivityEvent({ + projectId, + sessionId, + source: "api", + kind: "api.agent_report.transition_rejected", + level: "warn", + summary: `applyAgentReport rejected ${input.state} for ${sessionId}: ${rejectionReason}`, + data: { + reportState: input.state, + rejectionReason, + actor, + reportSource: source, + fromState: current.session.state, + fromReason: current.session.reason, + legacyStatus: previousLegacyStatus, + }, + }); + throw new Error(rejectionReason); } - if (effectivePrUrl) { - current.pr.url = effectivePrUrl; + const mapped = mapAgentReportToLifecycle(input.state); + previousState = current.session.state; + nextState = mapped.sessionState; + current.session.state = mapped.sessionState; + current.session.reason = mapped.sessionReason; + current.session.lastTransitionAt = now; + if (isPRWorkflowReport(input.state)) { + const effectivePrUrl = trimmedPrUrl ?? current.pr.url ?? existingPrUrl; + const effectivePrNumber = + prNumber ?? current.pr.number ?? existingPrNumber ?? parsedPrFromUrl?.number; + const canAdvancePrState = + effectivePrUrl !== undefined || + effectivePrNumber !== undefined || + current.pr.state !== "none"; + if (canAdvancePrState) { + current.pr.state = "open"; + current.pr.reason = input.state === "ready_for_review" ? "review_pending" : "in_progress"; + current.pr.lastObservedAt = now; + } + if (effectivePrUrl) { + current.pr.url = effectivePrUrl; + } + if (effectivePrNumber !== undefined) { + current.pr.number = effectivePrNumber; + } } - if (effectivePrNumber !== undefined) { - current.pr.number = effectivePrNumber; + if (mapped.sessionState === "working" && current.session.startedAt === null) { + current.session.startedAt = now; } - } - if (mapped.sessionState === "working" && current.session.startedAt === null) { - current.session.startedAt = now; - } - legacyStatus = deriveLegacyStatus(current); - const next = { ...existing }; - Object.assign( - next, - buildLifecycleMetadataPatch(current), - { + legacyStatus = deriveLegacyStatus(current); + const next = { ...existing }; + Object.assign(next, buildLifecycleMetadataPatch(current), { [AGENT_REPORT_METADATA_KEYS.STATE]: input.state, [AGENT_REPORT_METADATA_KEYS.AT]: now, - }, - ); - if (trimmedNote) { - next[AGENT_REPORT_METADATA_KEYS.NOTE] = trimmedNote; - } else { - next[AGENT_REPORT_METADATA_KEYS.NOTE] = ""; - } - if (isPRWorkflowReport(input.state)) { - if (trimmedPrUrl) { - next[AGENT_REPORT_METADATA_KEYS.PR_URL] = trimmedPrUrl; + }); + if (trimmedNote) { + next[AGENT_REPORT_METADATA_KEYS.NOTE] = trimmedNote; + } else { + next[AGENT_REPORT_METADATA_KEYS.NOTE] = ""; } - if (prNumber !== undefined) { - next[AGENT_REPORT_METADATA_KEYS.PR_NUMBER] = String(prNumber); + if (isPRWorkflowReport(input.state)) { + if (trimmedPrUrl) { + next[AGENT_REPORT_METADATA_KEYS.PR_URL] = trimmedPrUrl; + } + if (prNumber !== undefined) { + next[AGENT_REPORT_METADATA_KEYS.PR_NUMBER] = String(prNumber); + } + if (prIsDraft !== undefined) { + next[AGENT_REPORT_METADATA_KEYS.PR_IS_DRAFT] = prIsDraft ? "true" : "false"; + } } - if (prIsDraft !== undefined) { - next[AGENT_REPORT_METADATA_KEYS.PR_IS_DRAFT] = prIsDraft ? "true" : "false"; - } - } - return next; - }, { activityEventSource: "api" }); + return next; + }, + { activityEventSource: "api" }, + ); if (!nextMetadata || !before || !previousState || !nextState || !legacyStatus) { + recordActivityEvent({ + projectId, + sessionId, + source: "api", + kind: "api.agent_report.apply_failed", + level: "error", + summary: `failed to apply agent report ${input.state} for ${sessionId}`, + data: { + reportState: input.state, + actor, + source, + }, + }); throw new Error(`Failed to apply agent report for session ${sessionId}`); } diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 37c59d3ce..0bf552192 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -32,6 +32,7 @@ import { loadGlobalConfig, } from "./global-config.js"; import { loadEffectiveProjectConfig } from "./project-resolver.js"; +import { recordActivityEvent } from "./activity-events.js"; function inferScmPlugin(project: { repo?: string; @@ -876,6 +877,17 @@ function buildEffectiveConfigFromGlobalConfigPath(configPath: string): LoadedCon path: entry.path, resolveError: error.message, }; + if (error.reasonKind === "malformed" || error.reasonKind === "invalid") { + continue; + } + recordActivityEvent({ + projectId, + source: "config", + kind: "config.project_resolve_failed", + level: "error", + summary: `project ${projectId} failed to resolve`, + data: { path: entry.path, error: error.message }, + }); } } diff --git a/packages/core/src/global-config.ts b/packages/core/src/global-config.ts index cda4ca625..6ad925953 100644 --- a/packages/core/src/global-config.ts +++ b/packages/core/src/global-config.ts @@ -7,10 +7,11 @@ import { z } from "zod"; import { atomicWriteFileSync } from "./atomic-write.js"; import { detectScmPlatform } from "./config-generator.js"; import { withFileLockSync } from "./file-lock.js"; -import { ProjectResolveError } from "./types.js"; +import { ProjectResolveError, type ProjectResolveErrorKind } from "./types.js"; import { generateSessionPrefix } from "./paths.js"; import { normalizeOriginUrl } from "./storage-key.js"; import { getDefaultRuntime } from "./platform.js"; +import { recordActivityEvent } from "./activity-events.js"; function globalConfigLockPath(configPath: string): string { return `${configPath}.lock`; @@ -347,6 +348,12 @@ export function loadGlobalConfig( if (migrationSummary) { // eslint-disable-next-line no-console -- required migration visibility for stale shadow stripping console.info(migrationSummary); + recordActivityEvent({ + source: "config", + kind: "config.migrated", + summary: "global config migrated", + data: { migrationSummary }, + }); } const config = GlobalConfigSchema.parse(parsed); @@ -836,6 +843,7 @@ export function resolveProjectIdentity( defaultBranch: string; sessionPrefix: string; resolveError?: string; + resolveErrorKind?: ProjectResolveErrorKind; }) | null { const entry = globalConfig.projects[projectId] as @@ -914,15 +922,48 @@ export function resolveProjectIdentity( }; } + if (localConfigResult.kind === "malformed") { + recordActivityEvent({ + projectId, + source: "config", + kind: "config.project_malformed", + level: "error", + summary: `local config for ${projectId} could not be parsed`, + data: { + path: localConfigResult.path, + error: localConfigResult.error, + }, + }); + } else if (localConfigResult.kind === "invalid") { + recordActivityEvent({ + projectId, + source: "config", + kind: "config.project_invalid", + level: "error", + summary: `local config for ${projectId} failed validation`, + data: { + path: localConfigResult.path, + error: localConfigResult.error, + }, + }); + } + const resolveError = localConfigResult.kind !== "missing" ? (localConfigResult.error ?? "Failed to load local config") : undefined; + const resolveErrorKind: ProjectResolveErrorKind | undefined = + localConfigResult.kind === "malformed" || + localConfigResult.kind === "invalid" || + localConfigResult.kind === "old-format" + ? localConfigResult.kind + : undefined; return { ...(resolveError ? {} : applyBehaviorDefaults({})), ...identityFields, ...(resolveError ? { resolveError } : {}), + ...(resolveErrorKind ? { resolveErrorKind } : {}), }; } diff --git a/packages/core/src/migration/storage-v2.ts b/packages/core/src/migration/storage-v2.ts index 3572e578a..d8df92aac 100644 --- a/packages/core/src/migration/storage-v2.ts +++ b/packages/core/src/migration/storage-v2.ts @@ -30,6 +30,7 @@ import { parseKeyValueContent } from "../key-value.js"; import { generateSessionPrefix } from "../paths.js"; import { atomicWriteFileSync } from "../atomic-write.js"; import { withFileLockSync } from "../file-lock.js"; +import { recordActivityEvent } from "../activity-events.js"; // --------------------------------------------------------------------------- // Constants @@ -1209,6 +1210,16 @@ export async function migrateStorage(options: MigrationOptions = {}): Promise 0) { + recordActivityEvent({ + source: "migration", + kind: "migration.blocked", + level: "warn", + summary: `migration blocked by ${activeSessions.length} active session(s)`, + data: { + activeSessionCount: activeSessions.length, + sample: activeSessions.slice(0, 5), + }, + }); throw new Error( `Found ${activeSessions.length} active AO tmux session(s): ${activeSessions.slice(0, 5).join(", ")}${activeSessions.length > 5 ? "..." : ""}. ` + `Kill active sessions first (ao session kill --all) or use --force to migrate anyway.`, @@ -1228,7 +1239,7 @@ export async function migrateStorage(options: MigrationOptions = {}): Promise 0 ? "warn" : "info", + summary: + projectErrors.length > 0 + ? `migration finished with ${projectErrors.length} error(s)` + : `migration completed: ${totals.projects} project(s), ${totals.sessions} session(s)`, + data: { + dryRun, + projectsMigrated: totals.projects, + sessions: totals.sessions, + worktrees: totals.worktrees, + strayWorktreesMoved: totals.strayWorktreesMoved, + claudeSessionsRelinked: totals.claudeSessionsRelinked, + codexSessionsRewritten: totals.codexSessionsRewritten, + emptyDirsDeleted: totals.emptyDirsDeleted, + projectErrors: projectErrors.length, + }, + }); + return totals; } @@ -1587,6 +1661,16 @@ export async function rollbackStorage(options: RollbackOptions = {}): Promise 0) { log(` Warning: projects/${projectId} has ${postMigrationSessions} session(s) created after migration — skipping deletion.`); log(` These sessions exist only in projects/${projectId}/ and would be lost. Remove manually after verifying.`); + recordActivityEvent({ + projectId, + source: "migration", + kind: "migration.rollback_skipped", + level: "warn", + summary: `rollback skipped projects/${projectId} — ${postMigrationSessions} post-migration session(s)`, + data: { + postMigrationSessions, + }, + }); } else { safeToDeleteProjects.add(projectId); } diff --git a/packages/core/src/plugin-registry.ts b/packages/core/src/plugin-registry.ts index d67a0d32d..a00d1e401 100644 --- a/packages/core/src/plugin-registry.ts +++ b/packages/core/src/plugin-registry.ts @@ -19,6 +19,7 @@ import type { PluginRegistry, OrchestratorConfig, } from "./types.js"; +import { recordActivityEvent } from "./activity-events.js"; /** Map from "slot:name" → plugin instance */ type PluginMap = Map; @@ -461,9 +462,23 @@ export function createPluginRegistry(): PluginRegistry { this.register(mod); } } catch (error) { + const message = error instanceof Error ? error.message : String(error); process.stderr.write( `[plugin-registry] Failed to load built-in plugin "${builtin.name}": ${error}\n`, ); + recordActivityEvent({ + source: "plugin-registry", + kind: "plugin-registry.load_failed", + level: "error", + summary: `built-in plugin ${builtin.name} failed to load`, + data: { + plugin: builtin.name, + slot: builtin.slot, + pkg: builtin.pkg, + builtin: true, + error: message, + }, + }); } } } @@ -486,6 +501,16 @@ export function createPluginRegistry(): PluginRegistry { const specifier = resolvePluginSpecifier(plugin, config); if (!specifier) { process.stderr.write(`[plugin-registry] Could not resolve specifier for plugin "${plugin.name}" (source: ${plugin.source})\n`); + recordActivityEvent({ + source: "plugin-registry", + kind: "plugin-registry.specifier_failed", + level: "error", + summary: `could not resolve specifier for plugin ${plugin.name}`, + data: { + plugin: plugin.name, + source: plugin.source ?? null, + }, + }); continue; } @@ -508,9 +533,27 @@ export function createPluginRegistry(): PluginRegistry { // Log validation errors but don't abort - other projects can still use the plugin. // The misconfigured project will fail later when it tries to use the plugin // with the wrong name, giving a clearer error at point of use. + const message = + validationError instanceof Error + ? validationError.message + : String(validationError); process.stderr.write( `[plugin-registry] Config validation failed for ${externalEntry.source}: ${validationError}\n`, ); + recordActivityEvent({ + source: "plugin-registry", + kind: "plugin-registry.validation_failed", + level: "error", + summary: `plugin manifest validation failed for ${plugin.name}`, + data: { + plugin: plugin.name, + externalSource: externalEntry.source, + specifier, + manifestName: mod.manifest.name, + manifestSlot: mod.manifest.slot, + error: message, + }, + }); } } @@ -520,7 +563,21 @@ export function createPluginRegistry(): PluginRegistry { this.register(mod); } } catch (error) { + const message = error instanceof Error ? error.message : String(error); process.stderr.write(`[plugin-registry] Failed to load plugin "${specifier}": ${error}\n`); + recordActivityEvent({ + source: "plugin-registry", + kind: "plugin-registry.load_failed", + level: "error", + summary: `external plugin ${plugin.name} failed to load`, + data: { + plugin: plugin.name, + specifier, + source: plugin.source ?? null, + builtin: false, + error: message, + }, + }); } } }, diff --git a/packages/core/src/project-resolver.ts b/packages/core/src/project-resolver.ts index bd98d9db3..fd8311f25 100644 --- a/packages/core/src/project-resolver.ts +++ b/packages/core/src/project-resolver.ts @@ -16,7 +16,7 @@ export function loadEffectiveProjectConfig( throw new ProjectResolveError(projectId, `Unknown project: ${projectId}`); } if (typeof resolved.resolveError === "string" && resolved.resolveError.length > 0) { - throw new ProjectResolveError(projectId, resolved.resolveError); + throw new ProjectResolveError(projectId, resolved.resolveError, resolved.resolveErrorKind); } return resolved; } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 1d5af5b3e..b68deaf2a 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -2005,11 +2005,14 @@ export class ConfigNotFoundError extends Error { } } +export type ProjectResolveErrorKind = "malformed" | "invalid" | "old-format"; + /** Thrown when a project cannot be resolved into an effective runtime config. */ export class ProjectResolveError extends Error { constructor( public readonly projectId: string, message: string, + public readonly reasonKind?: ProjectResolveErrorKind, ) { super(message); this.name = "ProjectResolveError"; From 73ffd4ab13904d7c18c64003dc20274bd036a94a Mon Sep 17 00:00:00 2001 From: neversettle <41864816+neversettle17-101@users.noreply.github.com> Date: Mon, 18 May 2026 19:19:39 +0530 Subject: [PATCH 5/6] fix(cli): fall back to local config when global config is missing in project supervisor (#1809) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(npm): suppress prebuild-install deprecation warning The prebuild-install package is no longer maintained, but it's a transitive dependency of better-sqlite3 (optional) and node-pty (optional). Both packages continue to use it reliably for downloading prebuilt binaries across platforms. Suppressing this warning avoids noise in npm install output while we await upstream changes. See https://github.com/ComposioHQ/agent-orchestrator/issues/1752 Co-Authored-By: Claude Haiku 4.5 * fix(cli): warn users when spawning in permissionless mode When agentConfig.permissions is unset, the schema defaults to "permissionless", which passes --dangerously-skip-permissions to Claude Code. Claude then shows a one-time confirmation prompt inside the tmux session; if dismissed, the session exits silently with no user-facing error — making it very hard to debug. Add a post-spawn warning in ao spawn output explaining the mode and how to either accept the prompt or switch to interactive mode. Also improve the config-instruction docs to describe each permissions value and the silent-exit risk. Fixes #1754 Co-Authored-By: Claude Haiku 4.5 * fix(cli): direct users to dashboard terminal for permissionless prompt Co-Authored-By: Claude Haiku 4.5 * removed unwanted changes * fix(cli): fall back to local config when global config is missing in project supervisor There are two separate things reading config: - Dashboard (Next.js web server): loadDashboardConfig() — tries global config, falls back to local. - Project supervisor (CLI process): loadConfig(getGlobalConfigPath()) only — no fallback. When a user runs ao start with a local agent-orchestrator.yaml that has never been registered in ~/.agent-orchestrator/config.yaml, the supervisor silently returned with 0 lifecycle workers, leaving sessions frozen in their last known state and PR status never updating. loadSupervisorConfig() mirrors the dashboard fallback: try global config first, fall back to loadConfig() (local discovery) on ENOENT. ConfigNotFoundError (no config anywhere) is handled by isMissingGlobalConfigError so the supervisor still exits cleanly when AO has never been configured. Co-Authored-By: Claude Sonnet 4.6 * fix(cli): address review feedback and fix failing supervisor tests Review feedback from greptile: - Remove unreachable ConfigNotFoundError branch from loadSupervisorConfig's catch — loadConfig(globalConfigPath) cannot throw it with a non-null path. The error still propagates from the fallback loadConfig() (no args) and is caught by isMissingConfigError at the outer scope. - Rename isMissingGlobalConfigError → isMissingConfigError. After extending it to catch ConfigNotFoundError (the "no config anywhere" case), the old name was misleading. Test fix (CI was failing): - Add ConfigNotFoundError to the vi.mock factory so tests that exercise the fallback path compile. - Add coverage for the new fallback paths: - ENOENT on global config → falls back to loadConfig() (local discovery) - Non-missing-config errors (e.g. invalid yaml) propagate up - No config anywhere → supervisor exits cleanly, no workers attached Co-Authored-By: Claude Sonnet 4.6 * chore(cli): drop unused config-instruction module The getConfigInstruction() helper and its config-help subcommand were the module's only consumers, so remove both. The schema URL referenced via CONFIG_SCHEMA_URL still lives in core and is reachable from the yaml's $schema field for editor-side completion. Co-Authored-By: Claude Sonnet 4.6 * chore(cli): tighten loadSupervisorConfig comment Replace the rationale paragraph with a one-line description of what the function does. The "why" lives in the PR description and commit history. Co-Authored-By: Claude Sonnet 4.6 * fix(cli): scope loadSupervisorConfig ENOENT guard to the global config path Review feedback from greptile: the ENOENT catch was broader than the matching guard in isMissingConfigError. If the global config exists but references a missing nested file, the supervisor would silently fall back to the local config instead of surfacing the configuration error. Mirror the path check from isMissingConfigError so only ENOENT for the global config path itself triggers the fallback. Add a test covering the nested-file case. Co-Authored-By: Claude Sonnet 4.6 * chore: restore trailing newline in .npmrc Review feedback from greptile. The trailing newline was accidentally dropped by an earlier "removed unwanted changes" commit on this branch and is unrelated to the supervisor fix. Co-Authored-By: Claude Sonnet 4.6 * Revert "chore(cli): drop unused config-instruction module" This reverts commit cb58e15d0289718854acdbce865fc00d4a494a18. * revert the unwanted comments * Addressing comments to make the resolved path as a fallback for supervisor * test(cli): assert local fallback configPath propagates distinct from global Reviewer nit: the ENOENT-fallback test was returning makeConfig with the default `/tmp/global-config.yaml` path, so the assertion on ensureLifecycleWorker could pass even if the supervisor accidentally propagated the global path. Use a distinct local path in the fallback fixture so the regression coverage is honest. Co-Authored-By: Claude Haiku 4.5 --------- Co-authored-by: Claude Haiku 4.5 --- .../__tests__/lib/project-supervisor.test.ts | 240 +++++++++++++++++- packages/cli/src/commands/start.ts | 2 +- packages/cli/src/lib/project-supervisor.ts | 104 ++++++-- 3 files changed, 316 insertions(+), 30 deletions(-) diff --git a/packages/cli/__tests__/lib/project-supervisor.test.ts b/packages/cli/__tests__/lib/project-supervisor.test.ts index 5e8cce679..6f9ce0319 100644 --- a/packages/cli/__tests__/lib/project-supervisor.test.ts +++ b/packages/cli/__tests__/lib/project-supervisor.test.ts @@ -9,6 +9,12 @@ const mockSetHealth = vi.fn(); const activeWorkers = new Set(); vi.mock("@aoagents/ao-core", () => ({ + ConfigNotFoundError: class ConfigNotFoundError extends Error { + constructor(message = "No agent-orchestrator.yaml found.") { + super(message); + this.name = "ConfigNotFoundError"; + } + }, createCorrelationId: () => "correlation-id", createProjectObserver: () => ({ setHealth: (...args: unknown[]) => mockSetHealth(...args) }), getGlobalConfigPath: () => "/tmp/global-config.yaml", @@ -67,9 +73,9 @@ import { stopProjectSupervisor, } from "../../src/lib/project-supervisor.js"; -function makeConfig(projectIds: string[]) { +function makeConfig(projectIds: string[], configPath = "/tmp/global-config.yaml") { return { - configPath: "/tmp/global-config.yaml", + configPath, projects: Object.fromEntries(projectIds.map((id) => [id, { name: id, path: `/tmp/${id}` }])), }; } @@ -223,7 +229,7 @@ describe("project-supervisor", () => { }, }); - const startPromise = startProjectSupervisor(1_000); + const startPromise = startProjectSupervisor({ intervalMs: 1_000 }); await vi.waitFor(() => expect(releaseList).toBeDefined()); stopProjectSupervisor(); @@ -242,7 +248,7 @@ describe("project-supervisor", () => { throw new Error("bad config"); }); - await expect(startProjectSupervisor(1_000)).rejects.toThrow("bad config"); + await expect(startProjectSupervisor({ intervalMs: 1_000 })).rejects.toThrow("bad config"); }); it("allows startup when the global config does not exist yet", async () => { @@ -257,7 +263,7 @@ describe("project-supervisor", () => { throw error; }); - const handle = await startProjectSupervisor(1_000); + const handle = await startProjectSupervisor({ intervalMs: 1_000 }); expect(handle).toEqual({ stop: expect.any(Function), @@ -266,10 +272,230 @@ describe("project-supervisor", () => { handle.stop(); }); + it("falls back to local config when the global config is missing (ENOENT)", async () => { + // The local fallback uses a DIFFERENT configPath than the global — + // a real bare `loadConfig()` discovers the local file and sets + // `config.configPath` to that path. Asserting on the local path here + // catches any bug that would propagate the global path through the + // fallback (e.g. accidentally returning the global config object). + const localConfigPath = "/tmp/cwd/agent-orchestrator.yaml"; + sessionsByProject.set("app", [makeSession("app")]); + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") { + throw Object.assign(new Error("ENOENT"), { + code: "ENOENT", + path: "/tmp/global-config.yaml", + }); + } + return makeConfig(["app"], localConfigPath); + }); + + await reconcileProjectSupervisor(); + + expect(mockLoadConfig).toHaveBeenCalledWith("/tmp/global-config.yaml"); + expect(mockLoadConfig).toHaveBeenCalledWith(); + expect(mockEnsureLifecycleWorker).toHaveBeenCalledWith( + expect.objectContaining({ configPath: localConfigPath }), + "app", + undefined, + ); + expect(activeWorkers.has("app")).toBe(true); + }); + + it("uses the caller-provided configPath as the local fallback when global is missing", async () => { + sessionsByProject.set("app", [makeSession("app")]); + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") { + throw Object.assign(new Error("ENOENT"), { + code: "ENOENT", + path: "/tmp/global-config.yaml", + }); + } + if (path === "/some/repo/agent-orchestrator.yaml") { + return makeConfig(["app"]); + } + throw new Error(`unexpected loadConfig path: ${path}`); + }); + + await reconcileProjectSupervisor({ configPath: "/some/repo/agent-orchestrator.yaml" }); + + expect(mockLoadConfig).toHaveBeenCalledWith("/tmp/global-config.yaml"); + expect(mockLoadConfig).toHaveBeenCalledWith("/some/repo/agent-orchestrator.yaml"); + // No bare cwd-walk when the caller resolved a path for us. + expect(mockLoadConfig).not.toHaveBeenCalledWith(); + expect(activeWorkers.has("app")).toBe(true); + }); + + it("ignores the caller-provided configPath when the global config is healthy", async () => { + sessionsByProject.set("app", [makeSession("app")]); + // Both paths would return a valid config — assert we only ever consult + // the global path. The configPath is the fallback, not an override. + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") return makeConfig(["app"]); + if (path === "/repo/agent-orchestrator.yaml") { + throw new Error("supervisor should not consult configPath when global is healthy"); + } + throw new Error(`unexpected loadConfig path: ${path}`); + }); + + await reconcileProjectSupervisor({ configPath: "/repo/agent-orchestrator.yaml" }); + + expect(mockLoadConfig).toHaveBeenCalledWith("/tmp/global-config.yaml"); + expect(mockLoadConfig).not.toHaveBeenCalledWith("/repo/agent-orchestrator.yaml"); + expect(activeWorkers.has("app")).toBe(true); + }); + + it("preserves workers across a global→fallback transition (multi-tick)", async () => { + // Tick 1: global exists with {alpha, beta, gamma} — supervisor attaches + // all three. Tick 2: global has been deleted; the local fallback config + // (passed-in configPath) lists only {alpha}. Without the source-aware + // detach skip, the second tick would kill beta and gamma even though + // they're still running real sessions. + sessionsByProject.set("alpha", [makeSession("alpha")]); + sessionsByProject.set("beta", [makeSession("beta")]); + sessionsByProject.set("gamma", [makeSession("gamma")]); + + // Tick 1: global is the source. + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") return makeConfig(["alpha", "beta", "gamma"]); + throw new Error(`unexpected path on tick 1: ${path}`); + }); + await reconcileProjectSupervisor({ configPath: "/repo/agent-orchestrator.yaml" }); + expect(activeWorkers.has("alpha")).toBe(true); + expect(activeWorkers.has("beta")).toBe(true); + expect(activeWorkers.has("gamma")).toBe(true); + + // Tick 2: global deleted, fallback has a narrower view. + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") { + throw Object.assign(new Error("ENOENT"), { + code: "ENOENT", + path: "/tmp/global-config.yaml", + }); + } + if (path === "/repo/agent-orchestrator.yaml") return makeConfig(["alpha"]); + throw new Error(`unexpected path on tick 2: ${path}`); + }); + await reconcileProjectSupervisor({ configPath: "/repo/agent-orchestrator.yaml" }); + + // All three workers must survive — fallback isn't authoritative for removal. + expect(activeWorkers.has("alpha")).toBe(true); + expect(activeWorkers.has("beta")).toBe(true); + expect(activeWorkers.has("gamma")).toBe(true); + expect(mockRemoveProjectFromRunning).not.toHaveBeenCalledWith("beta"); + expect(mockRemoveProjectFromRunning).not.toHaveBeenCalledWith("gamma"); + }); + + it("does detach when global is restored after a fallback period (symmetric flip)", async () => { + // Documents intentional current behavior: when source flips back to + // "global", the detach pass treats the global config as authoritative, + // so projects not listed there ARE detached — including any that were + // attached during a prior fallback window. The reviewer's guidance was + // scoped to the fallback direction; protecting the symmetric flip would + // require per-worker source tracking and is out of scope here. + sessionsByProject.set("local-only", [makeSession("local-only")]); + sessionsByProject.set("from-global", [makeSession("from-global")]); + + // Tick 1: no global, fallback attaches local-only. + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") { + throw Object.assign(new Error("ENOENT"), { + code: "ENOENT", + path: "/tmp/global-config.yaml", + }); + } + if (path === "/repo/agent-orchestrator.yaml") return makeConfig(["local-only"]); + throw new Error(`unexpected path on tick 1: ${path}`); + }); + await reconcileProjectSupervisor({ configPath: "/repo/agent-orchestrator.yaml" }); + expect(activeWorkers.has("local-only")).toBe(true); + + // Tick 2: global appears (e.g. another `ao start ` wrote it), + // listing only "from-global". Source = "global" → detach pass runs. + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") return makeConfig(["from-global"]); + throw new Error(`unexpected path on tick 2: ${path}`); + }); + await reconcileProjectSupervisor({ configPath: "/repo/agent-orchestrator.yaml" }); + + expect(activeWorkers.has("from-global")).toBe(true); + expect(activeWorkers.has("local-only")).toBe(false); + expect(mockRemoveProjectFromRunning).toHaveBeenCalledWith("local-only"); + }); + + it("does not detach unrelated active workers when operating from local fallback", async () => { + // Simulates: daemon already supervising "other-project" (registered via + // a prior reconcile against a global config that has since been deleted). + // The current reconcile sees only "cwd-project" in the local fallback — + // it must NOT treat "other-project" as removed. + activeWorkers.add("other-project"); + sessionsByProject.set("cwd-project", [makeSession("cwd-project")]); + mockLoadConfig.mockImplementation((path?: string) => { + if (path === "/tmp/global-config.yaml") { + throw Object.assign(new Error("ENOENT"), { + code: "ENOENT", + path: "/tmp/global-config.yaml", + }); + } + return makeConfig(["cwd-project"]); + }); + + await reconcileProjectSupervisor({ configPath: "/repo/agent-orchestrator.yaml" }); + + expect(activeWorkers.has("other-project")).toBe(true); + expect(mockRemoveProjectFromRunning).not.toHaveBeenCalledWith("other-project"); + // Attach pass still runs for the configured cwd project. + expect(activeWorkers.has("cwd-project")).toBe(true); + }); + + it("rethrows ENOENT from a nested file referenced by the global config", async () => { + mockLoadConfig.mockImplementation(() => { + throw Object.assign(new Error("ENOENT"), { + code: "ENOENT", + path: "/tmp/some-referenced-file.yaml", + }); + }); + + await expect(reconcileProjectSupervisor()).rejects.toThrow("ENOENT"); + expect(mockLoadConfig).toHaveBeenCalledTimes(1); + }); + + it("rethrows non-missing-config errors from the global config load", async () => { + mockLoadConfig.mockImplementation(() => { + throw new Error("invalid yaml"); + }); + + await expect(reconcileProjectSupervisor()).rejects.toThrow("invalid yaml"); + expect(mockLoadConfig).toHaveBeenCalledTimes(1); + }); + + it("exits cleanly when neither global nor local config exists", async () => { + const { ConfigNotFoundError } = await import("@aoagents/ao-core"); + mockLoadConfig + .mockImplementationOnce(() => { + throw Object.assign(new Error("ENOENT"), { + code: "ENOENT", + path: "/tmp/global-config.yaml", + }); + }) + .mockImplementationOnce(() => { + throw new ConfigNotFoundError(); + }); + + const handle = await startProjectSupervisor({ intervalMs: 1_000 }); + + expect(handle).toEqual({ + stop: expect.any(Function), + reconcileNow: expect.any(Function), + }); + expect(mockEnsureLifecycleWorker).not.toHaveBeenCalled(); + handle.stop(); + }); + it("forwards the supervisor interval to lifecycle workers it starts", async () => { sessionsByProject.set("app", [makeSession("app")]); - const handle = await startProjectSupervisor(1_234); + const handle = await startProjectSupervisor({ intervalMs: 1_234 }); expect(mockEnsureLifecycleWorker).toHaveBeenCalledWith( expect.objectContaining({ configPath: "/tmp/global-config.yaml" }), @@ -280,7 +506,7 @@ describe("project-supervisor", () => { }); it("reconcileNow waits for a queued reconcile when one is already running", async () => { - const handle = await startProjectSupervisor(1_000); + const handle = await startProjectSupervisor({ intervalMs: 1_000 }); let firstRelease: (() => void) | undefined; let secondRelease: (() => void) | undefined; let listCalls = 0; diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 1b5841a30..7f657c399 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -953,7 +953,7 @@ async function runStartup( if (shouldStartLifecycle) { try { spinner.start("Starting project supervisor"); - await startProjectSupervisor(); + await startProjectSupervisor({ configPath: config.configPath }); spinner.succeed("Lifecycle project supervisor started"); } catch (err) { spinner.fail("Project supervisor failed to start"); diff --git a/packages/cli/src/lib/project-supervisor.ts b/packages/cli/src/lib/project-supervisor.ts index fdfa70a13..2a7747a7e 100644 --- a/packages/cli/src/lib/project-supervisor.ts +++ b/packages/cli/src/lib/project-supervisor.ts @@ -4,6 +4,7 @@ import { isTerminalSession, createCorrelationId, createProjectObserver, + ConfigNotFoundError, type OrchestratorConfig, type ProjectObserver, } from "@aoagents/ao-core"; @@ -24,11 +25,36 @@ interface SupervisorHandle { let activeSupervisor: SupervisorHandle | null = null; -export interface ReconcileProjectSupervisorOptions { - intervalMs?: number; +type SupervisorConfigSource = "global" | "local-fallback"; + +interface LoadedSupervisorConfig { + config: OrchestratorConfig; + source: SupervisorConfigSource; } -function isMissingGlobalConfigError(error: unknown): boolean { +export interface ReconcileProjectSupervisorOptions { + intervalMs?: number; + /** + * Resolved config path from the caller (typically `ao start`). When the + * global config is missing, this is used as the explicit local-fallback + * source. Without it the supervisor would fall back to a cwd-walk via + * bare `loadConfig()`, which misses configs in `ao start ` / + * `ao start ` first-run flows — there the resolved config can + * live under the clone/target path while the daemon's cwd is somewhere + * else. A bare cwd-walk in that case throws ConfigNotFoundError, which + * `run()` silently swallows, leaving `running.projects` empty. + */ + configPath?: string; +} + +export interface StartProjectSupervisorOptions { + intervalMs?: number; + /** See {@link ReconcileProjectSupervisorOptions.configPath}. */ + configPath?: string; +} + +function isMissingConfigError(error: unknown): boolean { + if (error instanceof ConfigNotFoundError) return true; return ( error instanceof Error && "code" in error && @@ -38,6 +64,29 @@ function isMissingGlobalConfigError(error: unknown): boolean { ); } +/** Load the supervisor config: prefer the global registry, fall back to the + * caller-resolved local config path (or cwd discovery when none provided). + * Returns the source so callers can gate authoritative actions (like the + * detach pass) on whether we're looking at the real registry. */ +function loadSupervisorConfig(configPath?: string): LoadedSupervisorConfig { + const globalConfigPath = getGlobalConfigPath(); + try { + return { config: loadConfig(globalConfigPath), source: "global" }; + } catch (error) { + if ( + error instanceof Error && + "code" in error && + error.code === "ENOENT" && + "path" in error && + error.path === globalConfigPath + ) { + const config = configPath ? loadConfig(configPath) : loadConfig(); + return { config, source: "local-fallback" }; + } + throw error; + } +} + function reportProjectSupervisorError( observer: ProjectObserver, projectId: string, @@ -69,23 +118,31 @@ async function projectHasNonTerminalSession( export async function reconcileProjectSupervisor( options: ReconcileProjectSupervisorOptions = {}, ): Promise { - const config = loadConfig(getGlobalConfigPath()); + const { config, source } = loadSupervisorConfig(options.configPath); const observer = createProjectObserver(config, "project-supervisor"); const configuredProjectIds = new Set(Object.keys(config.projects)); - const activeProjectIds = new Set(listLifecycleWorkers()); - for (const projectId of activeProjectIds) { - if (!configuredProjectIds.has(projectId)) { - try { - stopLifecycleWorker(projectId); - await removeProjectFromRunning(projectId); - } catch (error) { - reportProjectSupervisorError( - observer, - projectId, - "Failed to detach lifecycle worker for removed project", - error, - ); + // Only the authoritative global registry can declare a project "removed". + // On a local fallback (e.g. global config was deleted while the daemon is + // already supervising multiple projects) the loaded config likely doesn't + // enumerate every supervised project — running the detach pass would kill + // unrelated lifecycle workers. Pre-fallback behavior was a no-op on + // missing global; preserve that property for the detach pass specifically. + if (source === "global") { + const activeProjectIds = new Set(listLifecycleWorkers()); + for (const projectId of activeProjectIds) { + if (!configuredProjectIds.has(projectId)) { + try { + stopLifecycleWorker(projectId); + await removeProjectFromRunning(projectId); + } catch (error) { + reportProjectSupervisorError( + observer, + projectId, + "Failed to detach lifecycle worker for removed project", + error, + ); + } } } } @@ -117,16 +174,19 @@ export async function reconcileProjectSupervisor( } export async function startProjectSupervisor( - intervalMs: number = DEFAULT_SUPERVISOR_INTERVAL_MS, + options: StartProjectSupervisorOptions = {}, ): Promise { if (activeSupervisor) return activeSupervisor; + const intervalMs = options.intervalMs ?? DEFAULT_SUPERVISOR_INTERVAL_MS; + const configPath = options.configPath; + let reconciling = false; let pending = false; let stopped = false; let waiters: Array<() => void> = []; - const run = async (options: { swallowErrors?: boolean } = {}): Promise => { + const run = async (runOptions: { swallowErrors?: boolean } = {}): Promise => { if (stopped) return; if (reconciling) { pending = true; @@ -140,10 +200,10 @@ export async function startProjectSupervisor( do { pending = false; try { - await reconcileProjectSupervisor({ intervalMs }); + await reconcileProjectSupervisor({ intervalMs, configPath }); } catch (error) { - if (isMissingGlobalConfigError(error)) return; - if (!options.swallowErrors) throw error; + if (isMissingConfigError(error)) return; + if (!runOptions.swallowErrors) throw error; // Best-effort background loop: transient config/state errors should not crash ao start. } } while (pending && !stopped); From c1e43f61dcb612fd84ec8e1481a1af8328ca3212 Mon Sep 17 00:00:00 2001 From: Dhruv Sharma Date: Mon, 18 May 2026 19:53:55 +0530 Subject: [PATCH 6/6] feat(core): round out session-manager activity events (#1657) (#1697) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(core): round out session-manager activity event instrumentation (#1657) PR #1620 wired activity events for the worker-spawn happy path (spawn_started/spawned/spawn_failed/killed). This extends the same pattern across the rest of session-manager's failure surfaces so RCA can answer questions today's logs can't: - Did the orchestrator spawn fail? (orchestrator path had no AE envelope at all; now wrapped like worker spawn) - Did the agent's startup prompt deliver, or did all 3 retries fail? - Did `runtime.destroy` / `workspace.destroy` succeed during kill, or silently leak? - Did `runtime_lost` reconciliation fire for this session? - Did `ao send` actually deliver? New event kinds (added to ActivityEventKind union): session.kill_started, session.prompt_delivery_failed, session.send_failed, session.restore_failed, session.restore_fallback, session.rollback_started, session.rollback_step_failed, session.workspace_hooks_failed, session.cleanup_error, session.orchestrator_conflict, runtime.lost_detected, runtime.lost_persist_failed, runtime.destroy_failed, workspace.destroy_failed, agent.opencode_purge_failed, tracker.issue_fetch_failed, tracker.generate_prompt_failed, metadata.corrupt_detected Coverage (per #1657 spec): - All 8 MUST emits implemented and have a regression test in session-manager-instrumentation.test.ts - 13 SHOULD emits implemented (cleanup loop, orchestrator subpaths, kill silent catches, send/restore failure paths, etc.) - 4 COULD emits implemented - Out-of-scope per-list enrichment paths intentionally NOT instrumented Invariants preserved (extends #1620's B1-B16): - B1: state mutation BEFORE event emission (runtime_lost persist happens before the AE fires; verified by test) - B2: never wrap recordActivityEvent in try/catch - B11: prompt content excluded from prompt_delivery_failed data (verified by test that asserts the prompt string is absent) - B16: failure-only — emit on final retry exhaustion only, not per attempt - B25 (new): cleanup-stack rollbacks emit per-step via the onError callback hook, not in aggregate Co-Authored-By: Claude Opus 4.7 * fix: address session activity event review * fix: address session activity event review * fix(core): avoid duplicate restore failure events * chore(ci): retrigger checks * Fix orchestrator activity event instrumentation --------- Co-authored-by: Claude Opus 4.7 Co-authored-by: whoisasx --- .../session-manager-instrumentation.test.ts | 635 ++++++++++++++++++ packages/core/src/activity-events.ts | 19 +- packages/core/src/metadata.ts | 236 ++++--- packages/core/src/session-manager.ts | 497 ++++++++++++-- 4 files changed, 1241 insertions(+), 146 deletions(-) create mode 100644 packages/core/src/__tests__/session-manager-instrumentation.test.ts diff --git a/packages/core/src/__tests__/session-manager-instrumentation.test.ts b/packages/core/src/__tests__/session-manager-instrumentation.test.ts new file mode 100644 index 000000000..69b82e44c --- /dev/null +++ b/packages/core/src/__tests__/session-manager-instrumentation.test.ts @@ -0,0 +1,635 @@ +/** + * Regression tests for session-manager activity event instrumentation + * (issue #1657 — extends PR #1620 to cover the rest of the failure paths + * inside spawn / kill / restore / send / list). + * + * One test per MUST-class emit. Pattern follows the lifecycle-manager- + * instrumentation tests: mock `recordActivityEvent`, drive the manager into + * the failure path, then assert the right kind/level/data was logged. + * + * Invariants asserted by these tests (PR #1620 B1/B2 plus #1657 B25): + * - state mutation happens BEFORE event emission + * - failure-only emits — no event on a successful send/spawn + * - cleanup-stack rollbacks emit per failed step (not in aggregate) + */ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { writeFileSync, mkdirSync } from "node:fs"; +import { join } from "node:path"; +import { createSessionManager } from "../session-manager.js"; +import { writeMetadata, readMetadataRaw } from "../metadata.js"; +import { recordActivityEvent } from "../activity-events.js"; +import { getProjectWorktreesDir } from "../paths.js"; +import type { OrchestratorConfig, PluginRegistry, Agent } from "../types.js"; +import { + setupTestContext, + teardownTestContext, + makeHandle, + type TestContext, +} from "./test-utils.js"; + +vi.mock("../activity-events.js", () => ({ + recordActivityEvent: vi.fn(), +})); + +let ctx: TestContext; +let sessionsDir: string; +let mockRegistry: PluginRegistry; +let config: OrchestratorConfig; + +beforeEach(() => { + ctx = setupTestContext(); + ({ sessionsDir, mockRegistry, config } = ctx); + vi.mocked(recordActivityEvent).mockClear(); +}); + +afterEach(() => { + teardownTestContext(ctx); + vi.restoreAllMocks(); + vi.useRealTimers(); +}); + +function findEvent(kind: string) { + return vi + .mocked(recordActivityEvent) + .mock.calls.map((c) => c[0]) + .find((e) => e.kind === kind); +} + +function findAllEvents(kind: string) { + return vi + .mocked(recordActivityEvent) + .mock.calls.map((c) => c[0]) + .filter((e) => e.kind === kind); +} + +function writeTerminatedSession( + sessionId: string, + options: { worktree: string; branch?: string }, +): void { + const metadata: Record = { + worktree: options.worktree, + status: "killed", + project: "my-app", + runtimeHandle: makeHandle("rt-old"), + lifecycle: { + version: 2, + session: { + kind: "worker", + state: "terminated", + reason: "manually_killed", + startedAt: "2025-01-01T00:00:00.000Z", + completedAt: null, + terminatedAt: "2025-01-01T00:00:00.000Z", + lastTransitionAt: "2025-01-01T00:00:00.000Z", + }, + pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null }, + runtime: { + state: "missing", + reason: "process_missing", + lastObservedAt: "2025-01-01T00:00:00.000Z", + handle: null, + tmuxName: null, + }, + }, + }; + if (options.branch !== undefined) { + metadata["branch"] = options.branch; + } + writeMetadata(sessionsDir, sessionId, metadata as unknown as Parameters[2]); +} + +describe("session.kill_started (MUST)", () => { + it("emits before runtime.destroy is attempted", async () => { + let destroyCalled = false; + let killStartedEmittedBeforeDestroy = false; + + vi.mocked(ctx.mockRuntime.destroy).mockImplementation(async () => { + destroyCalled = true; + killStartedEmittedBeforeDestroy = !!findEvent("session.kill_started"); + }); + + writeMetadata(sessionsDir, "app-killed", { + worktree: "/tmp/ws", + branch: "main", + status: "working", + project: "my-app", + runtimeHandle: makeHandle("rt-1"), + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await sm.kill("app-killed"); + + expect(destroyCalled).toBe(true); + expect(killStartedEmittedBeforeDestroy).toBe(true); + + const start = findEvent("session.kill_started"); + expect(start).toMatchObject({ + projectId: "my-app", + sessionId: "app-killed", + source: "session-manager", + kind: "session.kill_started", + }); + }); + + it("does not emit kill_started when session is already terminated (idempotent)", async () => { + writeMetadata(sessionsDir, "app-already-killed", { + worktree: "/tmp/ws", + branch: "main", + status: "killed", + project: "my-app", + lifecycle: { + version: 2, + session: { + kind: "worker", + state: "terminated", + reason: "manually_killed", + startedAt: "2025-01-01T00:00:00.000Z", + completedAt: null, + terminatedAt: "2025-01-01T00:00:00.000Z", + lastTransitionAt: "2025-01-01T00:00:00.000Z", + }, + pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null }, + runtime: { + state: "missing", + reason: "manual_kill_requested", + lastObservedAt: "2025-01-01T00:00:00.000Z", + handle: null, + tmuxName: null, + }, + }, + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await sm.kill("app-already-killed"); + + expect(findEvent("session.kill_started")).toBeUndefined(); + }); +}); + +describe("session.spawn_failed — orchestrator path (MUST)", () => { + it("emits session.spawned after a successful orchestrator spawn", async () => { + const sm = createSessionManager({ config, registry: mockRegistry }); + const session = await sm.spawnOrchestrator({ + projectId: "my-app", + systemPrompt: "be helpful", + }); + + const events = findAllEvents("session.spawned"); + const orchestratorSpawned = events.find( + (e) => e.data && (e.data as Record)["role"] === "orchestrator", + ); + + expect(session.id).toBe("app-orchestrator"); + expect(orchestratorSpawned).toMatchObject({ + projectId: "my-app", + sessionId: "app-orchestrator", + source: "session-manager", + kind: "session.spawned", + summary: "spawned: app-orchestrator", + data: { + agent: "mock-agent", + branch: "orchestrator/app-orchestrator", + role: "orchestrator", + }, + }); + }); + + it("does not emit terminal spawn_failed when ensure recovers a fixed reservation conflict", async () => { + let releaseWorkspace: () => void = () => {}; + const blockingWorkspace = new Promise((resolve) => { + releaseWorkspace = resolve; + }); + vi.mocked(ctx.mockWorkspace.create).mockImplementationOnce(async (cfg) => { + await blockingWorkspace; + return { + path: join(ctx.tmpDir, "ws-orchestrator"), + branch: cfg.branch, + sessionId: cfg.sessionId, + projectId: cfg.projectId, + }; + }); + + const firstManager = createSessionManager({ config, registry: mockRegistry }); + const secondManager = createSessionManager({ config, registry: mockRegistry }); + + const firstEnsure = firstManager.ensureOrchestrator({ + projectId: "my-app", + systemPrompt: "be helpful", + }); + await vi.waitFor(() => { + expect(ctx.mockWorkspace.create).toHaveBeenCalledTimes(1); + }); + + const secondEnsure = secondManager.ensureOrchestrator({ + projectId: "my-app", + systemPrompt: "be helpful", + }); + await vi.waitFor(() => { + expect(findEvent("session.orchestrator_conflict")).toBeDefined(); + }); + + releaseWorkspace(); + const [created, recovered] = await Promise.all([firstEnsure, secondEnsure]); + + expect(created.id).toBe("app-orchestrator"); + expect(recovered.id).toBe("app-orchestrator"); + expect(findAllEvents("session.orchestrator_conflict")).toHaveLength(1); + expect( + findAllEvents("session.spawn_failed").filter( + (e) => e.data && (e.data as Record)["role"] === "orchestrator", + ), + ).toHaveLength(0); + }); + + it("emits one terminal failure plus one stage failure when workspace.create throws", async () => { + vi.mocked(ctx.mockWorkspace.create).mockRejectedValue(new Error("disk full")); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect( + sm.spawnOrchestrator({ projectId: "my-app", systemPrompt: "be helpful" }), + ).rejects.toThrow("disk full"); + + const events = findAllEvents("session.spawn_failed"); + expect(events).toHaveLength(1); + const orchestratorFailure = events.find( + (e) => e.data && (e.data as Record)["role"] === "orchestrator", + ); + expect(orchestratorFailure).toBeDefined(); + expect(orchestratorFailure!.level).toBe("error"); + expect(orchestratorFailure!.projectId).toBe("my-app"); + + const stepEvents = findAllEvents("session.spawn_step_failed"); + expect(stepEvents).toHaveLength(1); + expect(stepEvents[0]!.sessionId).toBe("app-orchestrator"); + expect(stepEvents[0]!.data).toMatchObject({ + role: "orchestrator", + stage: "workspace_create", + }); + }); + + it("emits one terminal failure plus one stage failure when runtime.create throws", async () => { + vi.mocked(ctx.mockRuntime.create).mockRejectedValue(new Error("tmux not found")); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect( + sm.spawnOrchestrator({ projectId: "my-app", systemPrompt: "be helpful" }), + ).rejects.toThrow("tmux not found"); + + const events = findAllEvents("session.spawn_failed"); + expect(events).toHaveLength(1); + const orchestratorFailure = events.find( + (e) => e.data && (e.data as Record)["role"] === "orchestrator", + ); + expect(orchestratorFailure).toBeDefined(); + expect(orchestratorFailure!.level).toBe("error"); + + const stepEvents = findAllEvents("session.spawn_step_failed"); + expect(stepEvents).toHaveLength(1); + expect(stepEvents[0]!.sessionId).toBe("app-orchestrator"); + expect(stepEvents[0]!.data).toMatchObject({ + role: "orchestrator", + stage: "runtime_create", + }); + }); +}); + +describe("session.rollback_started/session.rollback_step_failed (MUST)", () => { + it("includes reserved sessionId when worker spawn rolls back after reservation", async () => { + const workspacePath = join(getProjectWorktreesDir("my-app"), "app-1"); + vi.mocked(ctx.mockWorkspace.create).mockResolvedValue({ + path: workspacePath, + branch: "feat/test", + sessionId: "app-1", + projectId: "my-app", + }); + vi.mocked(ctx.mockWorkspace.destroy).mockRejectedValue(new Error("destroy failed")); + vi.mocked(ctx.mockRuntime.create).mockRejectedValue(new Error("runtime failed")); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.spawn({ projectId: "my-app" })).rejects.toThrow("runtime failed"); + + const rollbackStarted = findEvent("session.rollback_started"); + expect(rollbackStarted).toBeDefined(); + expect(rollbackStarted!.sessionId).toBe("app-1"); + + const rollbackStepFailed = findEvent("session.rollback_step_failed"); + expect(rollbackStepFailed).toBeDefined(); + expect(rollbackStepFailed!.sessionId).toBe("app-1"); + expect(rollbackStepFailed!.data).toMatchObject({ reason: "destroy failed" }); + }); +}); + +describe("session.workspace_hooks_failed (MUST)", () => { + it("emits when setupWorkspaceHooks throws during orchestrator spawn", async () => { + const hookFailingAgent: Agent = { + ...ctx.mockAgent, + name: "hook-failing-agent", + setupWorkspaceHooks: vi.fn().mockRejectedValue(new Error("settings.json EACCES")), + }; + const originalGet = mockRegistry.get; + mockRegistry.get = vi.fn().mockImplementation((slot: string, name?: string) => { + if (slot === "agent") return hookFailingAgent; + return (originalGet as any)(slot, name); + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect( + sm.spawnOrchestrator({ projectId: "my-app", systemPrompt: "be helpful" }), + ).rejects.toThrow("settings.json EACCES"); + + const event = findEvent("session.workspace_hooks_failed"); + expect(event).toBeDefined(); + expect(event!.level).toBe("error"); + expect(event!.projectId).toBe("my-app"); + }); +}); + +describe("runtime.lost_detected (MUST)", () => { + it("emits when sm.list() persists runtime_lost for a dead runtime", async () => { + writeMetadata(sessionsDir, "app-dead", { + worktree: "/tmp/ws", + branch: "main", + status: "working", + project: "my-app", + runtimeHandle: makeHandle("rt-dead"), + }); + + // Runtime claims dead, agent process gone + vi.mocked(ctx.mockRuntime.isAlive).mockResolvedValue(false); + vi.mocked(ctx.mockAgent.isProcessRunning).mockResolvedValue(false); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await sm.list(); + + const event = findEvent("runtime.lost_detected"); + expect(event).toBeDefined(); + expect(event!.projectId).toBe("my-app"); + expect(event!.sessionId).toBe("app-dead"); + expect(event!.level).toBe("warn"); + + // B1: state mutation BEFORE event emission — verify metadata was persisted + const persisted = readMetadataRaw(sessionsDir, "app-dead"); + expect(persisted).not.toBeNull(); + const lifecycleStr = persisted!["lifecycle"]; + expect(lifecycleStr).toBeDefined(); + const lc = JSON.parse(lifecycleStr!) as { session: { state: string; reason: string } }; + expect(lc.session.state).toBe("detecting"); + expect(lc.session.reason).toBe("runtime_lost"); + }); +}); + +describe("session.send_failed (MUST)", () => { + it("emits after send retry-with-restore exhausts", async () => { + writeMetadata(sessionsDir, "app-send", { + worktree: "/tmp/ws", + branch: "main", + status: "killed", + project: "my-app", + runtimeHandle: makeHandle("rt-1"), + lifecycle: { + version: 2, + session: { + kind: "worker", + state: "terminated", + reason: "manually_killed", + startedAt: "2025-01-01T00:00:00.000Z", + completedAt: null, + terminatedAt: "2025-01-01T00:00:00.000Z", + lastTransitionAt: "2025-01-01T00:00:00.000Z", + }, + pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null }, + runtime: { + state: "missing", + reason: "process_missing", + lastObservedAt: "2025-01-01T00:00:00.000Z", + handle: null, + tmuxName: null, + }, + }, + }); + + vi.mocked(ctx.mockRuntime.sendMessage).mockRejectedValue(new Error("send broke")); + // restore() throws so retry-with-restore exhausts + vi.mocked(ctx.mockRuntime.create).mockRejectedValue(new Error("restore failed")); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.send("app-send", "hi")).rejects.toThrow(); + + const event = findEvent("session.send_failed"); + expect(event).toBeDefined(); + expect(event!.level).toBe("error"); + expect(event!.sessionId).toBe("app-send"); + // B11: data must not contain message content + expect(JSON.stringify(event!.data ?? {})).not.toContain("hi"); + }); + + it("does not double-emit session.restore_failed when restore fails during send", async () => { + const wsPath = join(ctx.tmpDir, "missing-send-ws"); + writeTerminatedSession("app-send-restore", { worktree: wsPath, branch: "feat/send" }); + + ctx.mockWorkspace.exists = vi.fn().mockResolvedValue(false); + ctx.mockWorkspace.restore = vi.fn().mockRejectedValue(new Error("restore failed")); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.send("app-send-restore", "hi")).rejects.toThrow(); + + const restoreFailed = findAllEvents("session.restore_failed"); + expect(restoreFailed).toHaveLength(1); + expect(restoreFailed[0]!.data).toMatchObject({ stage: "workspace_restore" }); + expect(findEvent("session.send_failed")).toBeDefined(); + }); + + it("tags restore-for-delivery timeout restore_failed with ready_timeout stage", async () => { + vi.useFakeTimers(); + try { + const wsPath = join(ctx.tmpDir, "send-restore-ready-timeout"); + mkdirSync(wsPath, { recursive: true }); + writeTerminatedSession("app-send-timeout", { worktree: wsPath, branch: "feat/send" }); + + vi.mocked(ctx.mockRuntime.isAlive).mockImplementation(async (handle) => { + return handle.id !== "rt-restored"; + }); + vi.mocked(ctx.mockAgent.isProcessRunning).mockImplementation(async (handle) => { + return handle.id !== "rt-restored"; + }); + vi.mocked(ctx.mockRuntime.create).mockResolvedValue(makeHandle("rt-restored")); + vi.mocked(ctx.mockRuntime.getOutput).mockResolvedValue(""); + vi.mocked(ctx.mockAgent.detectActivity).mockReturnValue("idle"); + + const sm = createSessionManager({ config, registry: mockRegistry }); + const sendPromise = sm.send("app-send-timeout", "hi"); + const rejection = expect(sendPromise).rejects.toThrow( + "restored session did not become ready for delivery", + ); + + await vi.runAllTimersAsync(); + await rejection; + + const restoreFailed = findAllEvents("session.restore_failed"); + expect(restoreFailed).toHaveLength(1); + expect(restoreFailed[0]!.data).toMatchObject({ + stage: "ready_timeout", + reason: "restored session did not become ready for delivery", + trigger: "send", + }); + expect(findEvent("session.send_failed")).toBeDefined(); + } finally { + vi.useRealTimers(); + } + }); +}); + +describe("session.restore_failed (MUST)", () => { + it("emits when restore's workspace restore throws", async () => { + const wsPath = join(ctx.tmpDir, "missing-ws"); + writeMetadata(sessionsDir, "app-rest", { + worktree: wsPath, + branch: "feat/x", + status: "killed", + project: "my-app", + runtimeHandle: makeHandle("rt-old"), + lifecycle: { + version: 2, + session: { + kind: "worker", + state: "terminated", + reason: "manually_killed", + startedAt: "2025-01-01T00:00:00.000Z", + completedAt: null, + terminatedAt: "2025-01-01T00:00:00.000Z", + lastTransitionAt: "2025-01-01T00:00:00.000Z", + }, + pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null }, + runtime: { + state: "missing", + reason: "process_missing", + lastObservedAt: "2025-01-01T00:00:00.000Z", + handle: null, + tmuxName: null, + }, + }, + }); + + // workspace doesn't exist + restore throws + vi.mocked(ctx.mockWorkspace.exists ?? (() => false)).mockResolvedValue?.(false); + ctx.mockWorkspace.exists = vi.fn().mockResolvedValue(false); + ctx.mockWorkspace.restore = vi.fn().mockRejectedValue(new Error("clone failed")); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.restore("app-rest")).rejects.toThrow(); + + const event = findEvent("session.restore_failed"); + expect(event).toBeDefined(); + expect(event!.level).toBe("error"); + expect(event!.sessionId).toBe("app-rest"); + }); + + it("emits SessionNotRestorableError when session is not restorable", async () => { + writeMetadata(sessionsDir, "app-not-rest", { + worktree: "/tmp/ws", + branch: "feat/x", + status: "working", + project: "my-app", + runtimeHandle: makeHandle("rt-1"), + lifecycle: { + version: 2, + session: { + kind: "worker", + state: "working", + reason: "task_in_progress", + startedAt: "2025-01-01T00:00:00.000Z", + completedAt: null, + terminatedAt: null, + lastTransitionAt: "2025-01-01T00:00:00.000Z", + }, + pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null }, + runtime: { + state: "alive", + reason: "process_running", + lastObservedAt: "2025-01-01T00:00:00.000Z", + handle: makeHandle("rt-1"), + tmuxName: null, + }, + }, + }); + + // active session — not restorable + vi.mocked(ctx.mockRuntime.isAlive).mockResolvedValue(true); + vi.mocked(ctx.mockAgent.isProcessRunning).mockResolvedValue(true); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.restore("app-not-rest")).rejects.toThrow(); + + const event = findEvent("session.restore_failed"); + expect(event).toBeDefined(); + }); + + it("emits when workspace is missing and the workspace plugin cannot restore", async () => { + const wsPath = join(ctx.tmpDir, "missing-no-restore"); + writeTerminatedSession("app-no-restore", { worktree: wsPath, branch: "feat/no-restore" }); + + ctx.mockWorkspace.exists = vi.fn().mockResolvedValue(false); + ctx.mockWorkspace.restore = undefined; + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.restore("app-no-restore")).rejects.toThrow(); + + const event = findEvent("session.restore_failed"); + expect(event).toBeDefined(); + expect(event!.sessionId).toBe("app-no-restore"); + expect(event!.data).toMatchObject({ + stage: "workspace_restore", + workspacePath: wsPath, + reason: "workspace plugin does not support restore", + }); + }); + + it("emits when workspace is missing and branch metadata is absent", async () => { + const wsPath = join(ctx.tmpDir, "missing-no-branch"); + writeTerminatedSession("app-no-branch", { worktree: wsPath }); + + ctx.mockWorkspace.exists = vi.fn().mockResolvedValue(false); + ctx.mockWorkspace.restore = vi.fn().mockResolvedValue({ + path: wsPath, + branch: "unused", + sessionId: "app-no-branch", + projectId: "my-app", + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + await expect(sm.restore("app-no-branch")).rejects.toThrow(); + + const event = findEvent("session.restore_failed"); + expect(event).toBeDefined(); + expect(event!.sessionId).toBe("app-no-branch"); + expect(event!.data).toMatchObject({ + stage: "workspace_restore", + workspacePath: wsPath, + reason: "branch metadata is missing", + }); + }); +}); + +describe("metadata.corrupt_detected (MUST)", () => { + it("emits when mutateMetadata side-renames a corrupt file", async () => { + // Simulate a corrupt metadata file in the sessions dir + const sessionPath = join(sessionsDir, "app-corrupt.json"); + mkdirSync(sessionsDir, { recursive: true }); + writeFileSync(sessionPath, "{ this is not json", "utf-8"); + + const { mutateMetadata } = await import("../metadata.js"); + mutateMetadata(sessionsDir, "app-corrupt", () => ({ branch: "feat/x", project: "my-app" }), { + createIfMissing: true, + activityEventSource: "api", + }); + + const event = findEvent("metadata.corrupt_detected"); + expect(event).toBeDefined(); + expect(event!.level).toBe("error"); + expect(event!.source).toBe("api"); + expect(event!.sessionId).toBe("app-corrupt"); + const data = event!.data as Record; + expect(data["renameSucceeded"]).toBe(true); + expect(data["renamedTo"]).toMatch(/\.corrupt-\d+$/); + }); +}); diff --git a/packages/core/src/activity-events.ts b/packages/core/src/activity-events.ts index 87b6c9175..ed9d79841 100644 --- a/packages/core/src/activity-events.ts +++ b/packages/core/src/activity-events.ts @@ -34,7 +34,25 @@ export type ActivityEventKind = | "session.spawn_started" | "session.spawned" | "session.spawn_failed" + | "session.spawn_step_failed" | "session.killed" + | "session.kill_started" + | "session.send_failed" + | "session.restore_failed" + | "session.restore_fallback" + | "session.rollback_started" + | "session.rollback_step_failed" + | "session.workspace_hooks_failed" + | "session.cleanup_error" + | "session.orchestrator_conflict" + | "runtime.lost_detected" + | "runtime.lost_persist_failed" + | "runtime.destroy_failed" + | "workspace.destroy_failed" + | "agent.opencode_purge_failed" + | "tracker.issue_fetch_failed" + | "tracker.generate_prompt_failed" + | "metadata.corrupt_detected" | "activity.transition" | "lifecycle.transition" | "ci.failing" @@ -102,7 +120,6 @@ export type ActivityEventKind = // Recovery/forensic instrumentation | "recovery.session_failed" | "recovery.action_failed" - | "metadata.corrupt_detected" | "api.agent_report.session_not_found" | "api.agent_report.transition_rejected" | "api.agent_report.apply_failed"; diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index 451c127b5..c93164f5c 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -23,7 +23,12 @@ import { constants, } from "node:fs"; import { basename, join, dirname } from "node:path"; -import type { CanonicalSessionLifecycle, RuntimeHandle, SessionId, SessionMetadata } from "./types.js"; +import type { + CanonicalSessionLifecycle, + RuntimeHandle, + SessionId, + SessionMetadata, +} from "./types.js"; import { atomicWriteFileSync } from "./atomic-write.js"; import { recordActivityEvent, type ActivityEventSource } from "./activity-events.js"; import { @@ -95,7 +100,9 @@ function parseRuntimeHandleField(value: unknown): RuntimeHandle | undefined { if (typeof parsed["id"] === "string" && typeof parsed["runtimeName"] === "string") { return parsed as unknown as RuntimeHandle; } - } catch { /* not valid JSON */ } + } catch { + /* not valid JSON */ + } } return undefined; } @@ -107,13 +114,16 @@ function parseDashboardField(raw: Record): SessionMetadata["das return { port: typeof d["port"] === "number" ? d["port"] : undefined, terminalWsPort: typeof d["terminalWsPort"] === "number" ? d["terminalWsPort"] : undefined, - directTerminalWsPort: typeof d["directTerminalWsPort"] === "number" ? d["directTerminalWsPort"] : undefined, + directTerminalWsPort: + typeof d["directTerminalWsPort"] === "number" ? d["directTerminalWsPort"] : undefined, }; } // Legacy format: flat fields const port = typeof raw["dashboardPort"] === "number" ? raw["dashboardPort"] : undefined; - const terminalWsPort = typeof raw["terminalWsPort"] === "number" ? raw["terminalWsPort"] : undefined; - const directTerminalWsPort = typeof raw["directTerminalWsPort"] === "number" ? raw["directTerminalWsPort"] : undefined; + const terminalWsPort = + typeof raw["terminalWsPort"] === "number" ? raw["terminalWsPort"] : undefined; + const directTerminalWsPort = + typeof raw["directTerminalWsPort"] === "number" ? raw["directTerminalWsPort"] : undefined; if (port !== undefined || terminalWsPort !== undefined || directTerminalWsPort !== undefined) { return { port, terminalWsPort, directTerminalWsPort }; } @@ -160,8 +170,15 @@ export function readMetadata(dataDir: string, sessionId: SessionId): SessionMeta issueTitle: raw["issueTitle"] as string | undefined, pr: raw["pr"] as string | undefined, prAutoDetect: - raw["prAutoDetect"] === "off" || raw["prAutoDetect"] === "false" || raw["prAutoDetect"] === false ? false : - raw["prAutoDetect"] === "on" || raw["prAutoDetect"] === "true" || raw["prAutoDetect"] === true ? true : undefined, + raw["prAutoDetect"] === "off" || + raw["prAutoDetect"] === "false" || + raw["prAutoDetect"] === false + ? false + : raw["prAutoDetect"] === "on" || + raw["prAutoDetect"] === "true" || + raw["prAutoDetect"] === true + ? true + : undefined, summary: raw["summary"] as string | undefined, project: raw["project"] as string | undefined, agent: raw["agent"] as string | undefined, @@ -221,8 +238,12 @@ export function readMetadataRaw( /** Fields that are stored as JSON objects and should be parsed when unflattening. */ const jsonFields = new Set([ - "runtimeHandle", "lifecycle", "statePayload", "dashboard", - "agentReport", "reportWatcher", + "runtimeHandle", + "lifecycle", + "statePayload", + "dashboard", + "agentReport", + "reportWatcher", ]); /** Unflatten a Record to proper types for JSON storage. */ @@ -234,7 +255,12 @@ function unflattenFromStringRecord(data: Record): Record>, ): void { - mutateMetadata(dataDir, sessionId, (existing) => { - return applyMetadataUpdates(existing, updates); - }, { createIfMissing: true }); + mutateMetadata( + dataDir, + sessionId, + (existing) => { + return applyMetadataUpdates(existing, updates); + }, + { createIfMissing: true }, + ); } export function applyMetadataUpdates( @@ -331,17 +362,17 @@ export function applyMetadataUpdates( return next; } -interface MutateMetadataOptions { - createIfMissing?: boolean; - activityEventSource?: ActivityEventSource; -} - function normalizeMetadataRecord(data: Record): Record { return Object.fromEntries( Object.entries(data).filter(([, value]) => value !== undefined && value !== ""), ); } +export interface MutateMetadataOptions { + createIfMissing?: boolean; + activityEventSource?: ActivityEventSource; +} + export function mutateMetadata( dataDir: string, sessionId: SessionId, @@ -351,77 +382,80 @@ export function mutateMetadata( const path = metadataPath(dataDir, sessionId); const lockPath = `${path}.lock`; - return withFileLockSync(lockPath, () => { - let existing: Record = {}; + return withFileLockSync( + lockPath, + () => { + let existing: Record = {}; - let content: string | undefined; - try { - content = readFileSync(path, "utf-8").trim(); - } catch { - // File doesn't exist - } - - if (content !== undefined) { - if (content) { - const raw = parseMetadataContent(content); - if (raw) { - existing = flattenToStringRecord(raw); - } else { - // Corrupt JSON. Preserve forensic evidence by side-renaming - // the file before we overwrite it with the merged update. - // Without this, the very next mutateMetadata call destroys - // the corrupt bytes permanently and the user has no signal - // that anything was wrong — the file just becomes "not - // corrupt anymore — and missing fields". - const corruptPath = `${path}.corrupt-${Date.now()}`; - let renamed = false; - try { - renameSync(path, corruptPath); - renamed = true; - // eslint-disable-next-line no-console - console.warn( - `[metadata] corrupt JSON at ${path}; preserved as ${corruptPath} before rewriting`, - ); - } catch { - // best effort — proceed even if the rename fails (e.g. EACCES) - } - // Forensic activity event so RCA can find every silent overwrite. - // Truncate the bad-JSON sample to 200 chars (B11 invariant — full file - // could be 16KB+ and would be dropped by the sanitizer cap). - const contentSample = - content.length > 200 ? content.slice(0, 200) : content; - // dataDir is `.../projects/{projectId}/sessions`; recover projectId for filtering. - const inferredProjectId = basename(dirname(dataDir)); - const summary = renamed - ? `Corrupt metadata for session ${sessionId} renamed to ${basename(corruptPath)}` - : `Corrupt metadata detected for session ${sessionId}; failed to rename forensic copy before rewrite`; - recordActivityEvent({ - projectId: inferredProjectId || undefined, - sessionId, - source: options.activityEventSource ?? "session-manager", - kind: "metadata.corrupt_detected", - level: "error", - summary, - data: { - path, - renamedTo: renamed ? corruptPath : null, - renameSucceeded: renamed, - contentSample, - contentLength: content.length, - }, - }); - } + let content: string | undefined; + try { + content = readFileSync(path, "utf-8").trim(); + } catch { + // File doesn't exist } - } else if (!options.createIfMissing) { - return null; - } - const next = normalizeMetadataRecord(updater({ ...existing })); + if (content !== undefined) { + if (content) { + const raw = parseMetadataContent(content); + if (raw) { + existing = flattenToStringRecord(raw); + } else { + // Corrupt JSON. Preserve forensic evidence by side-renaming + // the file before we overwrite it with the merged update. + // Without this, the very next mutateMetadata call destroys + // the corrupt bytes permanently and the user has no signal + // that anything was wrong — the file just becomes "not + // corrupt anymore — and missing fields". + const corruptPath = `${path}.corrupt-${Date.now()}`; + let renamed = false; + try { + renameSync(path, corruptPath); + renamed = true; + // eslint-disable-next-line no-console + console.warn( + `[metadata] corrupt JSON at ${path}; preserved as ${corruptPath} before rewriting`, + ); + } catch { + // best effort — proceed even if the rename fails (e.g. EACCES) + } + // Forensic activity event so RCA can find every silent overwrite. + // Truncate the bad-JSON sample to 200 chars (B11 invariant — full file + // could be 16KB+ and would be dropped by the sanitizer cap). + const contentSample = content.length > 200 ? content.slice(0, 200) : content; + // dataDir is `.../projects/{projectId}/sessions`; recover projectId for filtering. + const inferredProjectId = basename(dirname(dataDir)); + const summary = renamed + ? `Corrupt metadata for session ${sessionId} renamed to ${basename(corruptPath)}` + : `Corrupt metadata detected for session ${sessionId}; failed to rename forensic copy before rewrite`; + recordActivityEvent({ + projectId: inferredProjectId || undefined, + sessionId, + source: options.activityEventSource ?? "session-manager", + kind: "metadata.corrupt_detected", + level: "error", + summary, + data: { + path, + renamedTo: renamed ? corruptPath : null, + renameSucceeded: renamed, + contentSample, + contentLength: content.length, + }, + }); + } + } + } else if (!options.createIfMissing) { + return null; + } - mkdirSync(dirname(path), { recursive: true }); - atomicWriteFileSync(path, serializeMetadata(unflattenFromStringRecord(next))); - return next; - }, { timeoutMs: 5_000, staleMs: 30_000 }); + const next = normalizeMetadataRecord(updater({ ...existing })); + + mkdirSync(dirname(path), { recursive: true }); + atomicWriteFileSync(path, serializeMetadata(unflattenFromStringRecord(next))); + return next; + }, + { timeoutMs: 5_000, staleMs: 30_000 }, + ); } export function readCanonicalLifecycle( @@ -438,11 +472,7 @@ export function writeCanonicalLifecycle( sessionId: SessionId, lifecycle: CanonicalSessionLifecycle, ): void { - updateMetadata( - dataDir, - sessionId, - buildLifecycleMetadataPatch(cloneLifecycle(lifecycle)), - ); + updateMetadata(dataDir, sessionId, buildLifecycleMetadataPatch(cloneLifecycle(lifecycle))); } export function updateCanonicalLifecycle( @@ -483,18 +513,20 @@ export function listMetadata(dataDir: string): SessionId[] { const dir = dataDir; if (!existsSync(dir)) return []; - return readdirSync(dir).filter((name) => { - // Must be a .json file - if (!name.endsWith(JSON_EXTENSION)) return false; - const baseName = name.slice(0, -JSON_EXTENSION.length); - if (!baseName || baseName.startsWith(".")) return false; - if (!SESSION_ID_COMPONENT_PATTERN.test(baseName)) return false; - try { - return statSync(join(dir, name)).isFile(); - } catch { - return false; - } - }).map((name) => name.slice(0, -JSON_EXTENSION.length)); + return readdirSync(dir) + .filter((name) => { + // Must be a .json file + if (!name.endsWith(JSON_EXTENSION)) return false; + const baseName = name.slice(0, -JSON_EXTENSION.length); + if (!baseName || baseName.startsWith(".")) return false; + if (!SESSION_ID_COMPONENT_PATTERN.test(baseName)) return false; + try { + return statSync(join(dir, name)).isFile(); + } catch { + return false; + } + }) + .map((name) => name.slice(0, -JSON_EXTENSION.length)); } /** diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 6f51650bc..30e7dc12e 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -1221,6 +1221,18 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM // Branch will be generated as feat/{issueId} (line 329-331) } else { // Other error (auth, network, etc) - fail fast + recordActivityEvent({ + projectId: spawnConfig.projectId, + source: "session-manager", + kind: "tracker.issue_fetch_failed", + level: "error", + summary: `tracker getIssue failed for ${spawnConfig.issueId}`, + data: { + issueId: spawnConfig.issueId, + tracker: plugins.tracker.name, + reason: err instanceof Error ? err.message : String(err), + }, + }); throw new Error(`Failed to fetch issue ${spawnConfig.issueId}: ${err}`, { cause: err }); } } @@ -1235,10 +1247,13 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM // step now requires pushing one cleanup, with no risk of forgetting prior // ones. const cleanupStack = new CleanupStack(); + let sessionId: string | undefined; try { // Determine session ID — atomically reserve to prevent concurrent collisions - const { sessionId, tmuxName } = await reserveNextSessionIdentity(project, sessionsDir); - cleanupStack.push(() => deleteMetadata(sessionsDir, sessionId)); + let tmuxName: string | undefined; + ({ sessionId, tmuxName } = await reserveNextSessionIdentity(project, sessionsDir)); + const reservedSessionId = sessionId; + cleanupStack.push(() => deleteMetadata(sessionsDir, reservedSessionId)); // Determine branch name — explicit branch always takes priority let branch: string; @@ -1295,9 +1310,23 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM if (spawnConfig.issueId && plugins.tracker && resolvedIssue) { try { issueContext = await plugins.tracker.generatePrompt(spawnConfig.issueId, project); - } catch { - // Non-fatal: continue without detailed issue context - // Silently ignore errors - caller can check if issueContext is undefined + } catch (err) { + // Non-fatal: continue without detailed issue context. Surface the + // failure via AE so RCA can answer "did the agent get an enriched + // prompt or just the bare issue ID?" + recordActivityEvent({ + projectId: spawnConfig.projectId, + sessionId, + source: "session-manager", + kind: "tracker.generate_prompt_failed", + level: "warn", + summary: `tracker generatePrompt failed for ${spawnConfig.issueId}`, + data: { + issueId: spawnConfig.issueId, + tracker: plugins.tracker.name, + reason: err instanceof Error ? err.message : String(err), + }, + }); } } @@ -1514,14 +1543,81 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM // Log cleanup failures so they don't disappear silently. The original // code used /* best effort */ swallows; the stack preserves that // behavior (cleanup errors don't propagate) but surfaces them for debug. + recordActivityEvent({ + projectId: spawnConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.rollback_started", + level: "warn", + summary: "spawn rollback started", + data: { reason: err instanceof Error ? err.message : String(err) }, + }); await cleanupStack.runAll((cleanupErr) => { console.error("[session-manager] spawn rollback step failed:", cleanupErr); + // B25: emit per-step rollback failure so each leaked resource is queryable. + recordActivityEvent({ + projectId: spawnConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.rollback_step_failed", + level: "error", + summary: "spawn rollback step failed", + data: { + reason: cleanupErr instanceof Error ? cleanupErr.message : String(cleanupErr), + }, + }); }); throw err; } } - async function spawnOrchestrator(orchestratorConfig: OrchestratorSpawnConfig): Promise { + function recordOrchestratorSpawnFailed( + orchestratorConfig: OrchestratorSpawnConfig, + err: unknown, + sessionId?: string, + ): void { + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + ...(sessionId ? { sessionId } : {}), + source: "session-manager", + kind: "session.spawn_failed", + level: "error", + summary: "orchestrator spawn failed", + data: { + role: "orchestrator", + reason: err instanceof Error ? err.message : String(err), + }, + }); + } + + async function spawnOrchestrator( + orchestratorConfig: OrchestratorSpawnConfig, + options?: { suppressFixedReservationFailure?: boolean }, + ): Promise { + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + source: "session-manager", + kind: "session.spawn_started", + summary: "orchestrator spawn started", + data: { agent: orchestratorConfig.agent ?? undefined, role: "orchestrator" }, + }); + try { + return await _spawnOrchestratorInner(orchestratorConfig); + } catch (err) { + const project = config.projects[orchestratorConfig.projectId]; + const sessionId = project ? getOrchestratorSessionId(project) : undefined; + const shouldSuppressRecoverableConflict = + options?.suppressFixedReservationFailure === true && + sessionId !== undefined && + isFixedOrchestratorReservationError(err, sessionId); + if (!shouldSuppressRecoverableConflict) { + recordOrchestratorSpawnFailed(orchestratorConfig, err, sessionId); + } + throw err; + } + } + + async function _spawnOrchestratorInner(orchestratorConfig: OrchestratorSpawnConfig): Promise { const project = config.projects[orchestratorConfig.projectId]; if (!project) { throw new Error(`Unknown project: ${orchestratorConfig.projectId}`); @@ -1582,6 +1678,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM workspacePath = wsInfo.path; adoptedManagedWorkspace = adoptedInfo !== undefined && adoptedInfo !== null; } catch (err) { + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.spawn_step_failed", + level: "error", + summary: "orchestrator workspace.create failed", + data: { + role: "orchestrator", + stage: "workspace_create", + reason: err instanceof Error ? err.message : String(err), + }, + }); try { deleteMetadata(sessionsDir, sessionId); } catch { @@ -1627,6 +1736,21 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM await setupPathWrapperWorkspace(workspacePath); } } catch (err) { + // PR tracking and CI fetch hooks are wired here — emit a dedicated AE + // before rolling back so RCA can answer "did the orchestrator launch + // succeed but lose its hook integration?". + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.workspace_hooks_failed", + level: "error", + summary: "orchestrator workspace hooks installation failed", + data: { + agent: plugins.agent.name, + reason: err instanceof Error ? err.message : String(err), + }, + }); await cleanupWorktreeAndMetadata(); throw err; } @@ -1642,6 +1766,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM systemPromptFile = join(projectDir, `orchestrator-prompt-${sessionId}.md`); writeFileSync(systemPromptFile, orchestratorConfig.systemPrompt, "utf-8"); } catch (err) { + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.spawn_step_failed", + level: "error", + summary: "orchestrator systemPrompt write failed", + data: { + role: "orchestrator", + stage: "system_prompt_write", + reason: err instanceof Error ? err.message : String(err), + }, + }); await cleanupWorktreeAndMetadata(systemPromptFile); throw err; } @@ -1651,6 +1788,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM try { writeWorkspaceOpenCodeAgentsMd(workspacePath, systemPromptFile); } catch (err) { + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.spawn_step_failed", + level: "error", + summary: "orchestrator AGENTS.md write failed", + data: { + role: "orchestrator", + stage: "agents_md_write", + reason: err instanceof Error ? err.message : String(err), + }, + }); await cleanupWorktreeAndMetadata(systemPromptFile); throw err; } @@ -1675,6 +1825,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM }); } } catch (err) { + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.spawn_step_failed", + level: "error", + summary: "orchestrator opencode session resolution failed", + data: { + role: "orchestrator", + stage: "opencode_session_reuse", + reason: err instanceof Error ? err.message : String(err), + }, + }); await cleanupWorktreeAndMetadata(systemPromptFile); throw err; } @@ -1732,6 +1895,22 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM }, }); } catch (err) { + // Outer envelope catches and emits session.spawn_failed; this step emit + // tags the runtime.create failure path specifically so RCA can answer + // "did the orchestrator runtime fail to start at all?". + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.spawn_step_failed", + level: "error", + summary: "orchestrator runtime.create failed", + data: { + role: "orchestrator", + stage: "runtime_create", + reason: err instanceof Error ? err.message : String(err), + }, + }); await cleanupWorktreeAndMetadata(systemPromptFile); throw err; } @@ -1819,6 +1998,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM } invalidateCache(); } catch (err) { + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.spawn_step_failed", + level: "error", + summary: "orchestrator post-launch metadata write failed", + data: { + role: "orchestrator", + stage: "post_launch_metadata", + reason: err instanceof Error ? err.message : String(err), + }, + }); // Clean up runtime on post-launch failure try { await plugins.runtime.destroy(handle); @@ -1829,6 +2021,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM throw err; } + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.spawned", + summary: `spawned: ${sessionId}`, + data: { + agent: plugins.agent.name, + branch: session.branch ?? undefined, + role: "orchestrator", + }, + }); + return session; } @@ -1897,14 +2102,27 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM } try { - return await spawnOrchestrator(orchestratorConfig); + return await spawnOrchestrator(orchestratorConfig, { + suppressFixedReservationFailure: true, + }); } catch (err) { if (!isFixedOrchestratorReservationError(err, sessionId)) { throw err; } + recordActivityEvent({ + projectId: orchestratorConfig.projectId, + sessionId, + source: "session-manager", + kind: "session.orchestrator_conflict", + level: "warn", + summary: "concurrent orchestrator reservation conflict", + data: { reason: err instanceof Error ? err.message : String(err) }, + }); + const concurrent = await waitForConcurrentOrchestrator(sessionId); if (concurrent) return concurrent; + recordOrchestratorSpawnFailed(orchestratorConfig, err, sessionId); throw err; } } @@ -2068,20 +2286,44 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM onDiskLifecycle.session.state !== "done" && onDiskLifecycle.session.state !== "detecting" ) { + const runtimeStateBefore = session.lifecycle.runtime.state; + const runtimeReasonBefore = session.lifecycle.runtime.reason; try { const persisted = buildUpdatedLifecycle(sessionName, raw, (next) => { next.session.state = "detecting"; next.session.reason = "runtime_lost"; next.session.lastTransitionAt = new Date().toISOString(); - next.runtime.state = session.lifecycle!.runtime.state; - next.runtime.reason = session.lifecycle!.runtime.reason; + next.runtime.state = runtimeStateBefore; + next.runtime.reason = runtimeReasonBefore; next.runtime.lastObservedAt = new Date().toISOString(); }); + // B1: persist BEFORE emitting the event updateMetadata(sessionsDir, sessionName, lifecycleMetadataUpdates(raw, persisted)); session.lifecycle = persisted; session.status = deriveLegacyStatus(persisted); - } catch { + recordActivityEvent({ + projectId: sessionProjectId, + sessionId: sessionName, + source: "session-manager", + kind: "runtime.lost_detected", + level: "warn", + summary: `runtime lost reconciled: ${sessionName}`, + data: { + runtimeState: runtimeStateBefore, + runtimeReason: runtimeReasonBefore, + }, + }); + } catch (err) { // Persist failed — in-memory state is still correct for this request + recordActivityEvent({ + projectId: sessionProjectId, + sessionId: sessionName, + source: "session-manager", + kind: "runtime.lost_persist_failed", + level: "error", + summary: `runtime_lost persist failed: ${sessionName}`, + data: { reason: err instanceof Error ? err.message : String(err) }, + }); } } @@ -2192,6 +2434,17 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM const killReason: LifecycleKillReason = options?.reason ?? "manually_killed"; const cleanupAgent = resolveSelectionForSession(project, sessionId, raw).agentName; + // Emit kill_started up-front — this is the only signal that the kill + // intent reached the manager (the destroys below are silent on failure). + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.kill_started", + summary: `kill started: ${sessionId}`, + data: { reason: killReason }, + }); + // Destroy runtime — prefer handle.runtimeName to find the correct plugin if (raw["runtimeHandle"]) { const handle = safeJsonParse(raw["runtimeHandle"]); @@ -2204,8 +2457,20 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM if (runtimePlugin) { try { await runtimePlugin.destroy(handle); - } catch { - // Runtime might already be gone + } catch (err) { + // Runtime might already be gone — surface as AE so leaks are queryable. + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "runtime.destroy_failed", + level: "warn", + summary: `runtime.destroy failed during kill: ${sessionId}`, + data: { + runtime: handle.runtimeName ?? null, + reason: err instanceof Error ? err.message : String(err), + }, + }); } } } @@ -2219,8 +2484,21 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM if (workspacePlugin) { try { await workspacePlugin.destroy(worktree); - } catch { - // Workspace might already be gone + } catch (err) { + // Workspace might already be gone — emit AE so abandoned worktrees + // surface for cleanup tooling. + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "workspace.destroy_failed", + level: "warn", + summary: `workspace.destroy failed during kill: ${sessionId}`, + data: { + workspace: workspacePlugin.name, + reason: err instanceof Error ? err.message : String(err), + }, + }); } } } @@ -2238,8 +2516,20 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM try { await deleteOpenCodeSession(mappedOpenCodeSessionId); didPurgeOpenCodeSession = true; - } catch { - void 0; + } catch (err) { + // Dangling opencode session is a real leak — surface for RCA. + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "agent.opencode_purge_failed", + level: "warn", + summary: `opencode session purge failed: ${sessionId}`, + data: { + opencodeSessionId: mappedOpenCodeSessionId, + reason: err instanceof Error ? err.message : String(err), + }, + }); } } } @@ -2372,9 +2662,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM pushSkipped(session.projectId, session.id); } } catch (err) { + const errorMessage = err instanceof Error ? err.message : String(err); result.errors.push({ sessionId: session.id, - error: err instanceof Error ? err.message : String(err), + error: errorMessage, + }); + recordActivityEvent({ + projectId: session.projectId, + sessionId: session.id, + source: "session-manager", + kind: "session.cleanup_error", + level: "warn", + summary: `cleanup error: ${session.id}`, + data: { reason: errorMessage }, }); } } @@ -2416,11 +2716,24 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM ...existing, opencodeSessionId: "", opencodeCleanedAt: new Date().toISOString(), - })); + }), { activityEventSource: "session-manager" }); } catch (err) { + const errorMessage = err instanceof Error ? err.message : String(err); result.errors.push({ sessionId: terminatedId, - error: `Failed to delete OpenCode session ${mappedOpenCodeSessionId}: ${err instanceof Error ? err.message : String(err)}`, + error: `Failed to delete OpenCode session ${mappedOpenCodeSessionId}: ${errorMessage}`, + }); + recordActivityEvent({ + projectId: projectKey, + sessionId: terminatedId, + source: "session-manager", + kind: "agent.opencode_purge_failed", + level: "warn", + summary: `opencode session purge failed during cleanup: ${terminatedId}`, + data: { + opencodeSessionId: mappedOpenCodeSessionId, + reason: errorMessage, + }, }); continue; } @@ -2451,7 +2764,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM } async function send(sessionId: SessionId, message: string): Promise { - const { raw, sessionsDir, project } = requireSessionRecord(sessionId); + const { raw, sessionsDir, project, projectId } = requireSessionRecord(sessionId); const selection = resolveSelectionForSession(project, sessionId, raw); const selectedAgent = selection.agentName; @@ -2603,19 +2916,31 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM throw new Error(`Cannot send to session ${sessionId}: ${reason}`); } + let restored: Session; try { - const restored = await restore(sessionId); - const ready = await waitForRestoredSession(restored); - if (!ready) { - throw new Error("restored session did not become ready for delivery"); - } - return restored; + restored = await restore(sessionId); } catch (err) { const detail = err instanceof Error ? err.message : String(err); throw new Error(`Cannot send to session ${sessionId}: ${reason} (${detail})`, { cause: err, }); } + + const ready = await waitForRestoredSession(restored); + if (!ready) { + const detail = "restored session did not become ready for delivery"; + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.restore_failed", + level: "error", + summary: `restore for delivery failed: ${sessionId}`, + data: { stage: "ready_timeout", reason: detail, trigger: "send" }, + }); + throw new Error(`Cannot send to session ${sessionId}: ${reason} (${detail})`); + } + return restored; }; const prepareSession = async (forceRestore = false): Promise => { @@ -2707,29 +3032,52 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM return; }; - let prepared = await prepareSession(); - + // Top-level try/catch: any final send failure (initial preparation, + // retry-with-restore, etc.) emits a single `session.send_failed` event + // (B16 — failure-only). Stage tag distinguishes which branch failed. + let stage: "prepare" | "initial" | "restore_retry" = "prepare"; try { - await sendWithConfirmation(prepared); - } catch (err) { - const shouldRetryWithRestore = prepared.restoredAt === undefined && isRestorable(prepared); + let prepared = await prepareSession(); - if (!shouldRetryWithRestore) { - if (err instanceof Error) { - throw err; - } - throw new Error(String(err), { cause: err }); - } - - prepared = await prepareSession(true); try { + stage = "initial"; await sendWithConfirmation(prepared); - } catch (retryErr) { - if (retryErr instanceof Error) { - throw retryErr; + } catch (err) { + const shouldRetryWithRestore = + prepared.restoredAt === undefined && isRestorable(prepared); + + if (!shouldRetryWithRestore) { + if (err instanceof Error) { + throw err; + } + throw new Error(String(err), { cause: err }); + } + + stage = "restore_retry"; + prepared = await prepareSession(true); + try { + await sendWithConfirmation(prepared); + } catch (retryErr) { + if (retryErr instanceof Error) { + throw retryErr; + } + throw new Error(String(retryErr), { cause: retryErr }); } - throw new Error(String(retryErr), { cause: retryErr }); } + } catch (err) { + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.send_failed", + level: "error", + summary: `send failed: ${sessionId}`, + data: { + stage, + reason: err instanceof Error ? err.message : String(err), + }, + }); + throw err; } } @@ -2930,6 +3278,20 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM const reason = NON_RESTORABLE_STATUSES.has(session.status) ? `status "${session.status}" is not restorable` : `session is not in a terminal state (status: "${session.status}", activity: "${session.activity}")`; + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.restore_failed", + level: "error", + summary: `restore not allowed: ${sessionId}`, + data: { + stage: "validation", + status: session.status, + activity: session.activity, + reason, + }, + }); throw new SessionNotRestorableError(sessionId, reason); } @@ -2950,9 +3312,35 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM if (!workspaceExists) { // Try to restore workspace if plugin supports it if (!plugins.workspace?.restore) { + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.restore_failed", + level: "error", + summary: `restore workspace failed: ${sessionId}`, + data: { + stage: "workspace_restore", + workspacePath, + reason: "workspace plugin does not support restore", + }, + }); throw new WorkspaceMissingError(workspacePath, "workspace plugin does not support restore"); } if (!session.branch) { + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.restore_failed", + level: "error", + summary: `restore workspace failed: ${sessionId}`, + data: { + stage: "workspace_restore", + workspacePath, + reason: "branch metadata is missing", + }, + }); throw new WorkspaceMissingError(workspacePath, "branch metadata is missing"); } try { @@ -2972,6 +3360,19 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM await plugins.workspace.postCreate(wsInfo, project); } } catch (err) { + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.restore_failed", + level: "error", + summary: `workspace restore failed: ${sessionId}`, + data: { + stage: "workspace_restore", + workspacePath, + reason: err instanceof Error ? err.message : String(err), + }, + }); throw new WorkspaceMissingError( workspacePath, `restore failed: ${err instanceof Error ? err.message : String(err)}`, @@ -3063,6 +3464,16 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM updateMetadata(sessionsDir, sessionId, { restoreFallbackReason: reason, }); + // Surface that AO fell back to a fresh launch instead of native restore. + recordActivityEvent({ + projectId, + sessionId, + source: "session-manager", + kind: "session.restore_fallback", + level: "warn", + summary: `using fresh launch instead of native restore: ${sessionId}`, + data: { agent: plugins.agent.name, reason }, + }); launchCommand = plugins.agent.getLaunchCommand(agentLaunchConfig); } } else {