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/.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__/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/__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/__tests__/session-manager-instrumentation.test.ts b/packages/core/src/__tests__/session-manager-instrumentation.test.ts index d0206f121..69b82e44c 100644 --- a/packages/core/src/__tests__/session-manager-instrumentation.test.ts +++ b/packages/core/src/__tests__/session-manager-instrumentation.test.ts @@ -20,7 +20,12 @@ 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"; +import { + setupTestContext, + teardownTestContext, + makeHandle, + type TestContext, +} from "./test-utils.js"; vi.mock("../activity-events.js", () => ({ recordActivityEvent: vi.fn(), @@ -90,11 +95,7 @@ function writeTerminatedSession( if (options.branch !== undefined) { metadata["branch"] = options.branch; } - writeMetadata( - sessionsDir, - sessionId, - metadata as unknown as Parameters[2], - ); + writeMetadata(sessionsDir, sessionId, metadata as unknown as Parameters[2]); } describe("session.kill_started (MUST)", () => { @@ -250,7 +251,9 @@ describe("session.spawn_failed — orchestrator path (MUST)", () => { const events = findAllEvents("session.spawn_failed"); expect(events).toHaveLength(1); - const orchestratorFailure = events.find((e) => e.data && (e.data as Record)["role"] === "orchestrator"); + 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"); @@ -274,7 +277,9 @@ describe("session.spawn_failed — orchestrator path (MUST)", () => { const events = findAllEvents("session.spawn_failed"); expect(events).toHaveLength(1); - const orchestratorFailure = events.find((e) => e.data && (e.data as Record)["role"] === "orchestrator"); + const orchestratorFailure = events.find( + (e) => e.data && (e.data as Record)["role"] === "orchestrator", + ); expect(orchestratorFailure).toBeDefined(); expect(orchestratorFailure!.level).toBe("error"); @@ -613,20 +618,18 @@ describe("metadata.corrupt_detected (MUST)", () => { 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: "agent-report" }, - ); + 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("warn"); - expect(event!.source).toBe("agent-report"); + expect(event!.level).toBe("error"); + expect(event!.source).toBe("api"); expect(event!.sessionId).toBe("app-corrupt"); const data = event!.data as Record; - expect(data["renamed"]).toBe(true); - expect(data["corruptPath"]).toMatch(/\.corrupt-\d+$/); + 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 5c21d7a46..ed9d79841 100644 --- a/packages/core/src/activity-events.ts +++ b/packages/core/src/activity-events.ts @@ -20,8 +20,15 @@ export type ActivityEventSource = | "scm" | "runtime" | "agent" + | "tracker" + | "workspace" + | "notifier" | "reaction" - | "report-watcher"; + | "report-watcher" + | "config" + | "plugin-registry" + | "migration" + | "recovery"; export type ActivityEventKind = | "session.spawn_started" @@ -59,6 +66,20 @@ export type ActivityEventKind = | "runtime.probe_failed" | "agent.process_probe_failed" | "agent.activity_probe_failed" + // Plugin-internal failure shapes (issue #1659) + | "scm.gh_unavailable" + | "scm.batch_enrich_pr_failed" + | "scm.ci_summary_failclosed" + | "workspace.post_create_failed" + | "workspace.branch_collision" + | "workspace.destroy_fell_back" + | "workspace.corrupt_clone_skipped" + | "tracker.dep_missing" + | "tracker.api_timeout" + | "notifier.auth_failed" + | "notifier.unreachable" + | "notifier.rate_limited" + | "notifier.dep_missing" // Reaction lifecycle | "reaction.escalated" | "reaction.send_to_agent_failed" @@ -70,7 +91,38 @@ export type ActivityEventKind = | "lifecycle.poll_failed" | "detecting.escalated" // Report watcher - | "report_watcher.triggered"; + | "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" + | "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" + | "api.agent_report.session_not_found" + | "api.agent_report.transition_rejected" + | "api.agent_report.apply_failed"; export type ActivityEventLevel = "debug" | "info" | "warn" | "error"; @@ -110,14 +162,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 @@ -134,7 +184,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; @@ -145,7 +198,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 @@ -158,7 +211,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 275305e10..091f56a3d 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,8 +26,14 @@ 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 { + 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"; @@ -254,6 +260,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 +394,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}`); } @@ -428,93 +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) { - appendAgentReportAuditEntry(dataDir, sessionId, { - timestamp: now, - actor, - source, - reportState: input.state, - note: trimmedNote, - prNumber, - prUrl: trimmedPrUrl, - prIsDraft, - accepted: false, - rejectionReason: validation.reason ?? "transition rejected", - before, - after: before, - }); - throw new Error(validation.reason ?? "transition rejected"); - } - 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; + 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, + 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: "agent-report" }); + 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/metadata.ts b/packages/core/src/metadata.ts index 3a430636d..c93164f5c 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -22,9 +22,15 @@ import { closeSync, constants, } from "node:fs"; -import { join, dirname } from "node:path"; -import type { CanonicalSessionLifecycle, RuntimeHandle, SessionId, SessionMetadata } from "./types.js"; +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, @@ -35,7 +41,6 @@ import { assertValidSessionIdComponent, SESSION_ID_COMPONENT_PATTERN } from "./u import { flattenToStringRecord } from "./utils/metadata-flatten.js"; import { validateStatus } from "./utils/validation.js"; import { withFileLockSync } from "./file-lock.js"; -import { recordActivityEvent } from "./activity-events.js"; const JSON_EXTENSION = ".json"; @@ -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( @@ -339,7 +370,7 @@ function normalizeMetadataRecord(data: Record): Record { - 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) - } - // Surface forensically — the original `console.warn` is invisible - // to anyone not tailing the orchestrator log. AE makes corrupt - // detection queryable. - recordActivityEvent({ - sessionId, - source: options.activityEventSource ?? "metadata", - kind: "metadata.corrupt_detected", - level: "warn", - summary: `corrupt metadata JSON detected: ${sessionId}`, - data: { - path, - corruptPath: renamed ? corruptPath : null, - renamed, - bytes: 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( @@ -429,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( @@ -474,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/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/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 message.includes("MODULE_NOT_FOUND") || code === "ERR_MODULE_NOT_FOUND" ) { + // User-actionable. Emit once per process so RCA can answer + // "why is the composio notifier silent?" without spamming on every notify call. + if (!depMissingEmitted) { + depMissingEmitted = true; + recordActivityEvent({ + source: "notifier", + kind: "notifier.dep_missing", + level: "error", + summary: "Composio SDK (composio-core) is not installed", + data: { + plugin: "notifier-composio", + package: "composio-core", + installHint: "pnpm add composio-core", + }, + }); + } return null; } throw err; diff --git a/packages/plugins/notifier-discord/src/index.ts b/packages/plugins/notifier-discord/src/index.ts index 7134fa2e2..00191239e 100644 --- a/packages/plugins/notifier-discord/src/index.ts +++ b/packages/plugins/notifier-discord/src/index.ts @@ -1,4 +1,5 @@ import { + recordActivityEvent, validateUrl, type PluginModule, type Notifier, @@ -129,6 +130,17 @@ async function postWithRetry( // Rate-limit budget exhausted — fail immediately rather than falling through // to the error retry path (which would compound the two counters). const body = await response.text().catch(() => ""); + recordActivityEvent({ + source: "notifier", + kind: "notifier.rate_limited", + level: "warn", + summary: `Discord webhook rate-limit retry budget exhausted`, + data: { + plugin: "notifier-discord", + status: 429, + rateLimitRetries, + }, + }); lastError = new Error(`Discord webhook rate-limited (HTTP 429)${body ? `: ${body.trim()}` : ""}`); throw lastError; } diff --git a/packages/plugins/notifier-openclaw/src/activity-events.test.ts b/packages/plugins/notifier-openclaw/src/activity-events.test.ts new file mode 100644 index 000000000..da89f5e2f --- /dev/null +++ b/packages/plugins/notifier-openclaw/src/activity-events.test.ts @@ -0,0 +1,174 @@ +/** + * Regression tests for plugin-internal activity events (issue #1659). + * + * Covers notifier.auth_failed (MUST) and notifier.unreachable (SHOULD) — + * the two failure shapes RCA needs to distinguish. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { OrchestratorEvent } from "@aoagents/ao-core"; + +const { recordActivityEventMock } = vi.hoisted(() => ({ + recordActivityEventMock: vi.fn(), +})); + +vi.mock("@aoagents/ao-core", async () => { + const actual = (await vi.importActual("@aoagents/ao-core")) as Record; + return { + ...actual, + recordActivityEvent: recordActivityEventMock, + }; +}); + +import { create } from "./index.js"; + +function makeEvent(overrides: Partial = {}): OrchestratorEvent { + return { + id: "evt-1", + type: "reaction.escalated", + priority: "urgent", + sessionId: "ao-5", + projectId: "ao", + timestamp: new Date("2026-03-08T12:00:00Z"), + message: "Reaction escalated", + data: {}, + ...overrides, + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); + delete process.env.OPENCLAW_HOOKS_TOKEN; +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("notifier.auth_failed (MUST emit)", () => { + it("emits on 401 (distinct from notifier.unreachable on ECONNREFUSED)", async () => { + const fetchMock = vi + .fn() + .mockResolvedValue({ ok: false, status: 401, text: () => Promise.resolve("unauthorized") }); + vi.stubGlobal("fetch", fetchMock); + + const notifier = create({ token: "tok", retries: 0 }); + await expect(notifier.notify(makeEvent())).rejects.toThrow(/OpenClaw rejected the auth token/); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "notifier", + kind: "notifier.auth_failed", + level: "error", + sessionId: "ao-5", + data: expect.objectContaining({ + plugin: "notifier-openclaw", + status: 401, + }), + }), + ); + }); + + it("emits on 403", async () => { + const fetchMock = vi + .fn() + .mockResolvedValue({ ok: false, status: 403, text: () => Promise.resolve("forbidden") }); + vi.stubGlobal("fetch", fetchMock); + + const notifier = create({ token: "tok", retries: 0 }); + await expect(notifier.notify(makeEvent())).rejects.toThrow(); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + kind: "notifier.auth_failed", + data: expect.objectContaining({ status: 403 }), + }), + ); + }); +}); + +describe("notifier.unreachable (SHOULD emit)", () => { + it.each(["ECONNREFUSED", "ETIMEDOUT", "ENOTFOUND", "ENETUNREACH"])( + "emits on %s (distinct from notifier.auth_failed)", + async (code) => { + const fetchMock = vi.fn().mockRejectedValue(new Error(`fetch failed: ${code}`)); + vi.stubGlobal("fetch", fetchMock); + + const notifier = create({ token: "tok", retries: 0 }); + await expect(notifier.notify(makeEvent())).rejects.toThrow(/Can't reach OpenClaw gateway/); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "notifier", + kind: "notifier.unreachable", + level: "warn", + sessionId: "ao-5", + data: expect.objectContaining({ + plugin: "notifier-openclaw", + errorMessage: expect.stringContaining(code), + }), + }), + ); + + // Critically: should NOT also fire auth_failed — distinct shapes. + const authFailedCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "notifier.auth_failed", + ); + expect(authFailedCalls).toHaveLength(0); + }, + ); + + it.each(["ETIMEDOUT", "ENOTFOUND", "ENETUNREACH"])( + "does not emit on transient %s when a retry succeeds", + async (code) => { + const fetchMock = vi + .fn() + .mockRejectedValueOnce(new Error(`fetch failed: ${code}`)) + .mockResolvedValueOnce({ ok: true }); + vi.stubGlobal("fetch", fetchMock); + + const notifier = create({ token: "tok", retries: 1, retryDelayMs: 0 }); + await notifier.notify(makeEvent()); + + expect(fetchMock).toHaveBeenCalledTimes(2); + const unreachableCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "notifier.unreachable", + ); + expect(unreachableCalls).toHaveLength(0); + }, + ); + + it.each(["ETIMEDOUT", "ENOTFOUND", "ENETUNREACH"])( + "emits on transient %s only after retry budget is exhausted", + async (code) => { + const fetchMock = vi.fn().mockRejectedValue(new Error(`fetch failed: ${code}`)); + vi.stubGlobal("fetch", fetchMock); + + const notifier = create({ token: "tok", retries: 1, retryDelayMs: 0 }); + await expect(notifier.notify(makeEvent())).rejects.toThrow(/Can't reach OpenClaw gateway/); + + expect(fetchMock).toHaveBeenCalledTimes(2); + const unreachableCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "notifier.unreachable", + ); + expect(unreachableCalls).toHaveLength(1); + expect(unreachableCalls[0]?.[0].data).toMatchObject({ + errorMessage: expect.stringContaining(code), + }); + }, + ); + + it("does not emit unreachable for unrelated network errors", async () => { + const fetchMock = vi.fn().mockRejectedValue(new Error("fetch failed: certificate expired")); + vi.stubGlobal("fetch", fetchMock); + + const notifier = create({ token: "tok", retries: 0 }); + await expect(notifier.notify(makeEvent())).rejects.toThrow(/fetch failed: certificate expired/); + + const unreachableCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "notifier.unreachable", + ); + expect(unreachableCalls).toHaveLength(0); + }); +}); diff --git a/packages/plugins/notifier-openclaw/src/index.ts b/packages/plugins/notifier-openclaw/src/index.ts index fcd5ae6cf..ced31e7cf 100644 --- a/packages/plugins/notifier-openclaw/src/index.ts +++ b/packages/plugins/notifier-openclaw/src/index.ts @@ -9,6 +9,7 @@ import { type OrchestratorEvent, type PluginModule, getObservabilityBaseDir, + recordActivityEvent, } from "@aoagents/ao-core"; import { isRetryableHttpStatus, normalizeRetryConfig, validateUrl } from "@aoagents/ao-core/utils"; @@ -39,6 +40,13 @@ export const manifest = { }; const DEFAULT_TIMEOUT_MS = 10_000; +const UNREACHABLE_NETWORK_ERROR_CODES = [ + "ECONNREFUSED", + "ETIMEDOUT", + "ENOTFOUND", + "ENETUNREACH", +] as const; +type UnreachableNetworkErrorCode = (typeof UNREACHABLE_NETWORK_ERROR_CODES)[number]; type WakeMode = "now" | "next-heartbeat"; @@ -117,6 +125,10 @@ function recordHealthFailure(path: string | null, error: unknown): void { writeHealthSummary(path, summary); } +function getUnreachableNetworkErrorCode(error: Error): UnreachableNetworkErrorCode | undefined { + return UNREACHABLE_NETWORK_ERROR_CODES.find((code) => error.message.includes(code)); +} + async function postWithRetry( url: string, payload: OpenClawWebhookPayload, @@ -130,6 +142,7 @@ async function postWithRetry( for (let attempt = 0; attempt <= retries; attempt++) { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), DEFAULT_TIMEOUT_MS); + let shouldRethrowResponseError = false; try { const response = await fetch(url, { method: "POST", @@ -143,17 +156,33 @@ async function postWithRetry( const body = await response.text(); if (response.status === 401 || response.status === 403) { + // User-actionable: distinct from generic 5xx — token expired or wrong. + recordActivityEvent({ + sessionId: context.sessionId, + source: "notifier", + kind: "notifier.auth_failed", + level: "error", + summary: `OpenClaw rejected auth token (HTTP ${response.status})`, + data: { + plugin: "notifier-openclaw", + status: response.status, + url, + fixHint: "ao setup openclaw", + }, + }); lastError = new Error( `OpenClaw rejected the auth token (HTTP ${response.status}).\n` + ` Check that hooks.token in your OpenClaw config matches the token configured for AO.\n` + ` Reconfigure: ao setup openclaw`, ); + shouldRethrowResponseError = true; throw lastError; } lastError = new Error(`OpenClaw webhook failed (${response.status}): ${body}`); if (!isRetryableHttpStatus(response.status)) { + shouldRethrowResponseError = true; throw lastError; } @@ -163,10 +192,24 @@ async function postWithRetry( ); } } catch (err) { - if (err === lastError) throw err; + if (shouldRethrowResponseError && err === lastError) throw err; lastError = err instanceof Error ? err : new Error(String(err)); - if (lastError.message.includes("ECONNREFUSED")) { + const unreachableCode = getUnreachableNetworkErrorCode(lastError); + if (unreachableCode && (unreachableCode === "ECONNREFUSED" || attempt >= retries)) { + recordActivityEvent({ + sessionId: context.sessionId, + source: "notifier", + kind: "notifier.unreachable", + level: "warn", + summary: `OpenClaw gateway unreachable at ${url}`, + data: { + plugin: "notifier-openclaw", + url, + errorMessage: lastError.message, + fixHint: "openclaw status", + }, + }); throw new Error( `Can't reach OpenClaw gateway at ${url}.\n` + ` Is OpenClaw running? Check: openclaw status\n` + diff --git a/packages/plugins/scm-github/src/graphql-batch.ts b/packages/plugins/scm-github/src/graphql-batch.ts index 108d47168..cf92add29 100644 --- a/packages/plugins/scm-github/src/graphql-batch.ts +++ b/packages/plugins/scm-github/src/graphql-batch.ts @@ -9,6 +9,7 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { execGhObserved, + recordActivityEvent, type BatchObserver, type CICheck, type CIStatus, @@ -56,10 +57,10 @@ export function setExecGhAsync( * Configuration constants for cache sizing. * LRU cache automatically evicts oldest entries when these limits are reached. */ -const MAX_PR_LIST_ETAGS = 100; // Number of repos to cache -const MAX_COMMIT_STATUS_ETAGS = 500; // Number of commits to cache -const MAX_REVIEW_COMMENTS_ETAGS = 500; // Number of PRs to cache review ETags -const MAX_PR_METADATA = 200; // Number of PRs to cache full data +const MAX_PR_LIST_ETAGS = 100; // Number of repos to cache +const MAX_COMMIT_STATUS_ETAGS = 500; // Number of commits to cache +const MAX_REVIEW_COMMENTS_ETAGS = 500; // Number of PRs to cache review ETags +const MAX_PR_METADATA = 200; // Number of PRs to cache full data /** * ETag cache for REST API endpoints. @@ -125,11 +126,7 @@ export function getPRListETag(owner: string, repo: string): string | undefined { /** * Get commit status ETag for a specific commit. */ -export function getCommitStatusETag( - owner: string, - repo: string, - sha: string, -): string | undefined { +export function getCommitStatusETag(owner: string, repo: string, sha: string): string | undefined { return etagCache.commitStatus.get(`${owner}/${repo}#${sha}`); } @@ -145,12 +142,7 @@ export function setPRListETag(owner: string, repo: string, etag: string): void { * Set commit status ETag for a specific commit. * Exported for testing. */ -export function setCommitStatusETag( - owner: string, - repo: string, - sha: string, - etag: string, -): void { +export function setCommitStatusETag(owner: string, repo: string, sha: string, etag: string): void { etagCache.commitStatus.set(`${owner}/${repo}#${sha}`, etag); } @@ -161,10 +153,9 @@ export function setCommitStatusETag( * * Uses LRU eviction to ensure bounded memory usage. */ -const prMetadataCache = new LRUCache< - string, - { headSha: string | null; ciStatus: CIStatus } ->(MAX_PR_METADATA); +const prMetadataCache = new LRUCache( + MAX_PR_METADATA, +); /** * Cache for full PR enrichment data. @@ -241,7 +232,11 @@ export async function shouldRefreshPREnrichment( } if (repos.size === 0) { - return { shouldRefresh: false, details: ["No repos to check"], prListUnchangedRepos: new Set() }; + return { + shouldRefresh: false, + details: ["No repos to check"], + prListUnchangedRepos: new Set(), + }; } // Guard 1: Check PR list ETag for each repository @@ -297,9 +292,7 @@ export async function shouldRefreshPREnrichment( ); if (statusChanged) { shouldRefresh = true; - details.push( - `CI status changed for ${pr.owner}/${pr.repo}#${pr.number} (Guard 2)`, - ); + details.push(`CI status changed for ${pr.owner}/${pr.repo}#${pr.number} (Guard 2)`); } } } @@ -310,10 +303,7 @@ export async function shouldRefreshPREnrichment( /** * Get cached PR metadata for testing. */ -export function getPRMetadataCache(): Map< - string, - { headSha: string | null; ciStatus: CIStatus } -> { +export function getPRMetadataCache(): Map { return prMetadataCache.toMap(); } @@ -350,6 +340,11 @@ interface ErrorWithCause extends Error { cause?: unknown; } +// Module-level guard so we only emit gh_unavailable once per process. +// The error is system-wide (gh missing globally), not session-specific. +let ghUnavailableEmitted = false; +const batchEnrichPRFailedEmitted = new Set(); + /** * Pre-flight check to verify gh CLI is available and authenticated. * This prevents silent failures during GraphQL batch queries. @@ -357,7 +352,21 @@ interface ErrorWithCause extends Error { async function verifyGhCLI(): Promise { try { await execFileAsync("gh", ["--version"], { timeout: 5000 }); - } catch { + } catch (err) { + if (!ghUnavailableEmitted) { + ghUnavailableEmitted = true; + const errorMessage = err instanceof Error ? err.message : String(err); + recordActivityEvent({ + source: "scm", + kind: "scm.gh_unavailable", + level: "error", + summary: "gh CLI not available or not authenticated", + data: { + plugin: "scm-github", + errorMessage, + }, + }); + } const error = new Error( "gh CLI not available or not authenticated. GraphQL batch enrichment requires gh CLI to be installed and configured.", ) as ErrorWithCause; @@ -366,6 +375,16 @@ async function verifyGhCLI(): Promise { } } +/** Test-only: reset the once-per-process gh_unavailable guard. */ +export function _resetGhUnavailableEmittedForTesting(): void { + ghUnavailableEmitted = false; +} + +/** Test-only: reset the once-per-PR batch extraction failure guard. */ +export function _resetBatchEnrichPRFailedEmittedForTesting(): void { + batchEnrichPRFailedEmitted.clear(); +} + /** * Maximum number of PRs to query in a single GraphQL batch. * GitHub has limits on query complexity and we stay well under this limit. @@ -533,7 +552,10 @@ async function checkCommitStatusETag( if (is304(errorMsg)) { return false; } - observer?.log("warn", `[ETag Guard 2] Commit status check failed for ${commitKey}: ${errorMsg}`); + observer?.log( + "warn", + `[ETag Guard 2] Commit status check failed for ${commitKey}: ${errorMsg}`, + ); return true; // Assume changed to be safe } } @@ -596,7 +618,10 @@ export async function checkReviewCommentsETag( if (is304(errorMsg)) { return false; } - observer?.log("warn", `[ETag Guard 3] Review comments check failed for ${cacheKey}: ${errorMsg}`); + observer?.log( + "warn", + `[ETag Guard 3] Review comments check failed for ${cacheKey}: ${errorMsg}`, + ); return true; // Assume changed to be safe } } @@ -704,9 +729,7 @@ export function generateBatchQuery(prs: PRInfo[]): { * * @throws Error if the query fails with GraphQL errors or parsing issues. */ -async function executeBatchQuery( - prs: PRInfo[], -): Promise> { +async function executeBatchQuery(prs: PRInfo[]): Promise> { const { query, variables } = generateBatchQuery(prs); // Handle empty array - no query needed @@ -866,9 +889,7 @@ function parseCheckContexts(contexts: unknown): CICheck[] { * Uses only the top-level aggregate state to determine overall CI status. * Individual check details are parsed separately via parseCheckContexts(). */ -function parseCIState( - statusCheckRollup: unknown, -): CIStatus { +function parseCIState(statusCheckRollup: unknown): CIStatus { if (!statusCheckRollup || typeof statusCheckRollup !== "object") { return "none"; } @@ -885,8 +906,7 @@ function parseCIState( if (state === "PENDING" || state === "EXPECTED") return "pending"; if (state === "TIMED_OUT" || state === "CANCELLED" || state === "ACTION_REQUIRED") return "failing"; - if (state === "QUEUED" || state === "IN_PROGRESS" || state === "WAITING") - return "pending"; + if (state === "QUEUED" || state === "IN_PROGRESS" || state === "WAITING") return "pending"; return "none"; } @@ -927,11 +947,7 @@ function extractPREnrichment( const pr = pullRequest as Record; // Check for at least one required field to validate this is a valid PR object - if ( - pr["state"] === undefined && - pr["title"] === undefined && - pr["commits"] === undefined - ) { + if (pr["state"] === undefined && pr["title"] === undefined && pr["commits"] === undefined) { return null; } @@ -954,9 +970,7 @@ function extractPREnrichment( // Extract merge info const mergeable = pr["mergeable"]; const mergeStateStatus = - typeof pr["mergeStateStatus"] === "string" - ? pr["mergeStateStatus"].toUpperCase() - : ""; + typeof pr["mergeStateStatus"] === "string" ? pr["mergeStateStatus"].toUpperCase() : ""; const hasConflicts = mergeable === "CONFLICTING"; const isBehind = mergeStateStatus === "BEHIND"; @@ -974,9 +988,7 @@ function extractPREnrichment( // contexts(first: 20) silently truncates PRs with >20 checks — when truncated, // the failing check may be missing, so we set ciChecks to undefined to force // the getCIChecks() REST fallback in maybeDispatchCIFailureDetails. - const contextsField = statusCheckRollup?.["contexts"] as - | Record - | undefined; + const contextsField = statusCheckRollup?.["contexts"] as Record | undefined; const pageInfo = contextsField?.["pageInfo"]; const contextsHasNextPage = pageInfo !== null && @@ -984,15 +996,12 @@ function extractPREnrichment( typeof pageInfo === "object" && (pageInfo as Record)["hasNextPage"] === true; const ciChecks = - contextsField && !contextsHasNextPage - ? parseCheckContexts(contextsField) - : undefined; + contextsField && !contextsHasNextPage ? parseCheckContexts(contextsField) : undefined; // Build blockers list const blockers: string[] = []; if (ciStatus === "failing") blockers.push("CI is failing"); - if (reviewDecision === "changes_requested") - blockers.push("Changes requested in review"); + if (reviewDecision === "changes_requested") blockers.push("Changes requested in review"); if (reviewDecision === "pending") blockers.push("Review required"); if (hasConflicts) blockers.push("Merge conflicts"); if (isBehind) blockers.push("Branch is behind base branch"); @@ -1126,6 +1135,25 @@ export async function enrichSessionsPRBatch( result.set(prKey, enrichment); // Update PR metadata cache for future ETag checks updatePRMetadataCache(prKey, enrichment, headSha); + } else { + // GraphQL returned a PR object but extractPREnrichment couldn't + // parse it (missing fields, schema drift). Distinct from the + // whole-batch failure D02 catches further down. + if (!batchEnrichPRFailedEmitted.has(prKey)) { + batchEnrichPRFailedEmitted.add(prKey); + recordActivityEvent({ + source: "scm", + kind: "scm.batch_enrich_pr_failed", + level: "warn", + summary: `batch enrich extraction failed for PR #${pr.number}`, + data: { + plugin: "scm-github", + prNumber: pr.number, + prOwner: pr.owner, + prRepo: pr.repo, + }, + }); + } } } else { // PR not found (deleted/closed/permission issue) @@ -1147,7 +1175,10 @@ export async function enrichSessionsPRBatch( durationMs: batchDuration, }; observer?.recordSuccess(successData); - observer?.log("info", `[GraphQL Batch Success] Batch ${batchIndex + 1}/${batches.length} succeeded: added ${prCountAfter - prCountBefore} PRs to cache (${batchDuration}ms)`); + observer?.log( + "info", + `[GraphQL Batch Success] Batch ${batchIndex + 1}/${batches.length} succeeded: added ${prCountAfter - prCountBefore} PRs to cache (${batchDuration}ms)`, + ); } } catch (err) { // Calculate duration even on failure diff --git a/packages/plugins/scm-github/src/index.ts b/packages/plugins/scm-github/src/index.ts index 8d065db01..fa59cecdc 100644 --- a/packages/plugins/scm-github/src/index.ts +++ b/packages/plugins/scm-github/src/index.ts @@ -11,6 +11,7 @@ import { CI_STATUS, execGhObserved, memoizeAsync, + recordActivityEvent, type PluginModule, type PreflightContext, type SCM, @@ -62,6 +63,12 @@ const BOT_AUTHORS = new Set([ ]); const CI_FAILURE_LOG_TAIL_LINES = 120; +const ciSummaryFailClosedEmitted = new Set(); + +/** Test-only: reset once-per-PR activity event guards. */ +export function _resetGitHubActivityEventDedupeForTesting(): void { + ciSummaryFailClosedEmitted.clear(); +} // --------------------------------------------------------------------------- // Helpers @@ -237,10 +244,7 @@ async function getFailedJobLog( ]); } catch (err) { if (!runReference.jobId) throw err; - return gh([ - "api", - `repos/${pr.owner}/${pr.repo}/actions/jobs/${runReference.jobId}/logs`, - ]); + return gh(["api", `repos/${pr.owner}/${pr.repo}/actions/jobs/${runReference.jobId}/logs`]); } } @@ -522,6 +526,10 @@ function repoFlag(pr: PRInfo): string { return `${pr.owner}/${pr.repo}`; } +function prEventKey(pr: PRInfo): string { + return `${repoFlag(pr)}#${pr.number}`; +} + function parseDate(val: string | undefined | null): Date { if (!val) return new Date(0); const d = new Date(val); @@ -932,7 +940,7 @@ function createGitHubSCM(): SCM { let checks: CICheck[]; try { checks = await this.getCIChecks(pr); - } catch { + } catch (err) { // Before fail-closing, check if the PR is merged/closed — // GitHub may not return check data for those, and reporting // "failing" for a merged PR is wrong. @@ -943,7 +951,26 @@ function createGitHubSCM(): SCM { // Can't determine state either; fall through to fail-closed. } // Fail closed for open PRs: report as failing rather than - // "none" (which getMergeability treats as passing). + // "none" (which getMergeability treats as passing). Emit so RCA + // can distinguish "really failing" from "we couldn't tell". + const eventKey = prEventKey(pr); + if (!ciSummaryFailClosedEmitted.has(eventKey)) { + ciSummaryFailClosedEmitted.add(eventKey); + const errorMessage = err instanceof Error ? err.message : String(err); + recordActivityEvent({ + source: "scm", + kind: "scm.ci_summary_failclosed", + level: "warn", + summary: `getCISummary failed-closed for PR #${pr.number}`, + data: { + plugin: "scm-github", + prNumber: pr.number, + prOwner: pr.owner, + prRepo: pr.repo, + errorMessage, + }, + }); + } return "failing"; } if (checks.length === 0) return "none"; @@ -1035,16 +1062,16 @@ function createGitHubSCM(): SCM { try { // Use GraphQL with variables to get review threads with actual isResolved status const raw = await gh([ - "api", - "graphql", - "-f", - `owner=${pr.owner}`, - "-f", - `name=${pr.repo}`, - "-F", - `number=${pr.number}`, - "-f", - `query=query($owner: String!, $name: String!, $number: Int!) { + "api", + "graphql", + "-f", + `owner=${pr.owner}`, + "-f", + `name=${pr.repo}`, + "-F", + `number=${pr.number}`, + "-f", + `query=query($owner: String!, $name: String!, $number: Int!) { repository(owner: $owner, name: $name) { pullRequest(number: $number) { reviewThreads(first: 100) { @@ -1067,58 +1094,58 @@ function createGitHubSCM(): SCM { } } }`, - ]); + ]); - const data: { - data: { - repository: { - pullRequest: { - reviewThreads: { - nodes: Array<{ - id: string; - isResolved: boolean; - comments: { - nodes: Array<{ - id: string; - author: { login: string } | null; - body: string; - path: string | null; - line: number | null; - url: string; - createdAt: string; - }>; - }; - }>; + const data: { + data: { + repository: { + pullRequest: { + reviewThreads: { + nodes: Array<{ + id: string; + isResolved: boolean; + comments: { + nodes: Array<{ + id: string; + author: { login: string } | null; + body: string; + path: string | null; + line: number | null; + url: string; + createdAt: string; + }>; + }; + }>; + }; }; }; }; - }; - } = JSON.parse(raw); + } = JSON.parse(raw); - const threads = data.data.repository.pullRequest.reviewThreads.nodes; + const threads = data.data.repository.pullRequest.reviewThreads.nodes; - return threads - .filter((t) => { - if (t.isResolved) return false; // only pending (unresolved) threads - const c = t.comments.nodes[0]; - if (!c) return false; // skip threads with no comments - const author = c.author?.login ?? ""; - return !BOT_AUTHORS.has(author); - }) - .map((t) => { - const c = t.comments.nodes[0]; - return { - id: c.id, - threadId: t.id, - author: c.author?.login ?? "unknown", - body: c.body, - path: c.path || undefined, - line: c.line ?? undefined, - isResolved: t.isResolved, - createdAt: parseDate(c.createdAt), - url: c.url, - }; - }); + return threads + .filter((t) => { + if (t.isResolved) return false; // only pending (unresolved) threads + const c = t.comments.nodes[0]; + if (!c) return false; // skip threads with no comments + const author = c.author?.login ?? ""; + return !BOT_AUTHORS.has(author); + }) + .map((t) => { + const c = t.comments.nodes[0]; + return { + id: c.id, + threadId: t.id, + author: c.author?.login ?? "unknown", + body: c.body, + path: c.path || undefined, + line: c.line ?? undefined, + isResolved: t.isResolved, + createdAt: parseDate(c.createdAt), + url: c.url, + }; + }); } catch (err) { throw new Error("Failed to fetch pending comments", { cause: err }); } @@ -1129,7 +1156,12 @@ function createGitHubSCM(): SCM { const cacheKey = `${pr.owner}/${pr.repo}#${pr.number}`; // Guard 3: check if review comments changed via REST ETag - const reviewsChanged = await checkReviewCommentsETag(pr.owner, pr.repo, pr.number, instanceObserver); + const reviewsChanged = await checkReviewCommentsETag( + pr.owner, + pr.repo, + pr.number, + instanceObserver, + ); if (!reviewsChanged) { const cached = reviewThreadsCache.get(cacheKey); if (cached) return cached; diff --git a/packages/plugins/scm-github/test/activity-events.test.ts b/packages/plugins/scm-github/test/activity-events.test.ts new file mode 100644 index 000000000..ad54976df --- /dev/null +++ b/packages/plugins/scm-github/test/activity-events.test.ts @@ -0,0 +1,152 @@ +/** + * Regression tests for plugin-internal activity events (issue #1659). + * + * Covers scm.gh_unavailable (MUST emit, deduped once-per-process). + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const { recordActivityEventMock } = vi.hoisted(() => ({ + recordActivityEventMock: vi.fn(), +})); + +vi.mock("@aoagents/ao-core", async () => { + const actual = (await vi.importActual("@aoagents/ao-core")) as Record; + return { + ...actual, + recordActivityEvent: recordActivityEventMock, + }; +}); + +import { + enrichSessionsPRBatch, + setExecFileAsync, + setExecGhAsync, + clearETagCache, + clearPRMetadataCache, + _resetGhUnavailableEmittedForTesting, + _resetBatchEnrichPRFailedEmittedForTesting, +} from "../src/graphql-batch.js"; +import type { PRInfo } from "@aoagents/ao-core"; + +const samplePRs: PRInfo[] = [ + { + owner: "octocat", + repo: "hello-world", + number: 42, + url: "https://github.com/octocat/hello-world/pull/42", + title: "Add new feature", + branch: "feature/new", + baseBranch: "main", + isDraft: false, + }, +]; + +beforeEach(() => { + vi.clearAllMocks(); + clearETagCache(); + clearPRMetadataCache(); + _resetGhUnavailableEmittedForTesting(); + _resetBatchEnrichPRFailedEmittedForTesting(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("scm.gh_unavailable (MUST emit)", () => { + it("emits when verifyGhCLI fails because gh is missing/unauthenticated", async () => { + const execFileMock = vi.fn().mockImplementation((file: string) => { + if (file === "gh") { + const err = new Error("spawn gh ENOENT") as Error & { code?: string }; + err.code = "ENOENT"; + return Promise.reject(err); + } + return Promise.resolve({ stdout: "", stderr: "" }); + }); + setExecFileAsync(execFileMock as unknown as Parameters[0]); + + // The batch-level try/catch swallows verifyGhCLI's throw — but the event + // fires before the throw, which is what we care about for RCA. + const result = await enrichSessionsPRBatch(samplePRs); + expect(result.enrichment.size).toBe(0); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "scm", + kind: "scm.gh_unavailable", + level: "error", + data: expect.objectContaining({ + plugin: "scm-github", + errorMessage: expect.any(String), + }), + }), + ); + }); + + it("emits exactly once across multiple gh-missing failures (deduped per-process)", async () => { + const execFileMock = vi.fn().mockImplementation((file: string) => { + if (file === "gh") { + return Promise.reject(new Error("gh ENOENT")); + } + return Promise.resolve({ stdout: "", stderr: "" }); + }); + setExecFileAsync(execFileMock as unknown as Parameters[0]); + + await enrichSessionsPRBatch(samplePRs); + await enrichSessionsPRBatch(samplePRs); + await enrichSessionsPRBatch(samplePRs); + + const ghUnavailableCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "scm.gh_unavailable", + ); + expect(ghUnavailableCalls).toHaveLength(1); + }); +}); + +describe("scm.batch_enrich_pr_failed (poll-path emit)", () => { + it("emits exactly once per PR across repeated extraction failures", async () => { + const execFileMock = vi.fn().mockResolvedValue({ stdout: "gh version", stderr: "" }); + setExecFileAsync(execFileMock as unknown as Parameters[0]); + + const execGhMock = vi.fn( + async (_args: string[], _timeout: number, operation: string): Promise => { + if (operation === "gh.api.guard-pr-list") { + return 'HTTP/2 200 OK\netag: W/"pr-list"\n\n[]'; + } + if (operation === "gh.api.graphql-batch") { + return `HTTP/2 200 OK\n\n${JSON.stringify({ + data: { + pr0: { + pullRequest: { unexpectedShape: true }, + }, + }, + })}`; + } + throw new Error(`Unexpected gh operation: ${operation}`); + }, + ); + setExecGhAsync(execGhMock); + + await enrichSessionsPRBatch(samplePRs); + await enrichSessionsPRBatch(samplePRs); + + const extractionFailureCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "scm.batch_enrich_pr_failed", + ); + expect(extractionFailureCalls).toHaveLength(1); + expect(extractionFailureCalls[0]?.[0]).toEqual( + expect.objectContaining({ + source: "scm", + kind: "scm.batch_enrich_pr_failed", + level: "warn", + data: expect.objectContaining({ + plugin: "scm-github", + prNumber: 42, + prOwner: "octocat", + prRepo: "hello-world", + }), + }), + ); + }); +}); diff --git a/packages/plugins/scm-github/test/index.test.ts b/packages/plugins/scm-github/test/index.test.ts index 501ff4e09..7fee69578 100644 --- a/packages/plugins/scm-github/test/index.test.ts +++ b/packages/plugins/scm-github/test/index.test.ts @@ -4,7 +4,10 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; // Mock node:child_process — gh CLI calls go through execFileAsync = promisify(execFile) // vi.hoisted ensures the mock fn is available when vi.mock factory runs (hoisted above imports) // --------------------------------------------------------------------------- -const { ghMock } = vi.hoisted(() => ({ ghMock: vi.fn() })); +const { ghMock, recordActivityEventMock } = vi.hoisted(() => ({ + ghMock: vi.fn(), + recordActivityEventMock: vi.fn(), +})); vi.mock("node:child_process", () => { // Attach the custom promisify symbol so `promisify(execFile)` returns ghMock @@ -14,8 +17,24 @@ vi.mock("node:child_process", () => { return { execFile }; }); -import { create, manifest } from "../src/index.js"; -import { _clearProcessCacheForTests, createActivitySignal, type PreflightContext, type PRInfo, type SCMWebhookRequest, type Session, type ProjectConfig } from "@aoagents/ao-core"; +vi.mock("@aoagents/ao-core", async () => { + const actual = (await vi.importActual("@aoagents/ao-core")) as Record; + return { + ...actual, + recordActivityEvent: recordActivityEventMock, + }; +}); + +import { create, manifest, _resetGitHubActivityEventDedupeForTesting } from "../src/index.js"; +import { + _clearProcessCacheForTests, + createActivitySignal, + type PreflightContext, + type PRInfo, + type SCMWebhookRequest, + type Session, + type ProjectConfig, +} from "@aoagents/ao-core"; // --------------------------------------------------------------------------- // Fixtures @@ -101,6 +120,7 @@ describe("scm-github plugin", () => { beforeEach(() => { vi.clearAllMocks(); + _resetGitHubActivityEventDedupeForTesting(); scm = create(); delete process.env["GITHUB_WEBHOOK_SECRET"]; }); @@ -665,13 +685,10 @@ describe("scm-github plugin", () => { url: "https://github.com/acme/repo/actions/runs/124/job/457", }, ]; - const logLines = Array.from( - { length: 125 }, - (_, index) => { - const step = index < 100 ? "Install dependencies" : "Run pnpm test"; - return `build\t${step}\t2026-05-12T00:00:00Z line ${index + 1}`; - }, - ); + const logLines = Array.from({ length: 125 }, (_, index) => { + const step = index < 100 ? "Install dependencies" : "Run pnpm test"; + return `build\t${step}\t2026-05-12T00:00:00Z line ${index + 1}`; + }); mockGhRaw(logLines.join("\n")); const summary = await scm.getCIFailureSummary?.(pr, checks); @@ -774,6 +791,34 @@ describe("scm-github plugin", () => { expect(await scm.getCISummary(pr)).toBe("failing"); }); + it("dedupes fail-closed activity events per PR", async () => { + mockGhError("checks failed"); + mockGhError("state failed"); + mockGhError("checks failed again"); + mockGhError("state failed again"); + + expect(await scm.getCISummary(pr)).toBe("failing"); + expect(await scm.getCISummary(pr)).toBe("failing"); + + const failClosedCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "scm.ci_summary_failclosed", + ); + expect(failClosedCalls).toHaveLength(1); + expect(failClosedCalls[0]?.[0]).toEqual( + expect.objectContaining({ + source: "scm", + kind: "scm.ci_summary_failclosed", + level: "warn", + data: expect.objectContaining({ + plugin: "scm-github", + prNumber: 42, + prOwner: "acme", + prRepo: "repo", + }), + }), + ); + }); + it('returns "none" when all checks are skipped', async () => { mockGh([ { name: "a", state: "SKIPPED" }, diff --git a/packages/plugins/scm-gitlab/src/index.ts b/packages/plugins/scm-gitlab/src/index.ts index df9511244..31402653a 100644 --- a/packages/plugins/scm-gitlab/src/index.ts +++ b/packages/plugins/scm-gitlab/src/index.ts @@ -7,6 +7,7 @@ import { createHash, timingSafeEqual } from "node:crypto"; import { CI_STATUS, + recordActivityEvent, type PluginModule, type SCM, type SCMWebhookEvent, @@ -45,6 +46,14 @@ const BOT_AUTHORS = new Set([ "sonarcloud[bot]", "snyk-bot", ]); +const ciSummaryFailClosedEmitted = new Set(); +const reviewFetchFailedEmitted = new Set(); + +/** Test-only: reset once-per-PR activity event guards. */ +export function _resetGitLabActivityEventDedupeForTesting(): void { + ciSummaryFailClosedEmitted.clear(); + reviewFetchFailedEmitted.clear(); +} function isBot(username: string): boolean { return ( @@ -60,6 +69,10 @@ function repoFlag(pr: PRInfo): string { return `${pr.owner}/${pr.repo}`; } +function prEventKey(pr: PRInfo): string { + return `${repoFlag(pr)}#${pr.number}`; +} + function parseDate(val: string | undefined | null): Date { if (!val) return new Date(0); const d = new Date(val); @@ -106,7 +119,6 @@ function mapPRState(state: string): PRState { return "open"; } - function getGitLabWebhookConfig(project: ProjectConfig) { const webhook = project.scm?.webhook; return { @@ -585,6 +597,24 @@ function createGitLabSCM(config?: Record): SCM { `getCISummary: PR state fallback also failed for MR !${pr.number}: ${(innerErr as Error).message}`, ); } + const eventKey = prEventKey(pr); + if (!ciSummaryFailClosedEmitted.has(eventKey)) { + ciSummaryFailClosedEmitted.add(eventKey); + const errorMessage = err instanceof Error ? err.message : String(err); + recordActivityEvent({ + source: "scm", + kind: "scm.ci_summary_failclosed", + level: "warn", + summary: `getCISummary failed-closed for MR !${pr.number}`, + data: { + plugin: "scm-gitlab", + prNumber: pr.number, + prOwner: pr.owner, + prRepo: pr.repo, + errorMessage, + }, + }); + } return "failing"; } if (checks.length === 0) return "none"; @@ -642,6 +672,24 @@ function createGitLabSCM(config?: Record): SCM { console.warn( `getReviews: discussions fetch failed for MR !${pr.number}: ${(err as Error).message}`, ); + const eventKey = prEventKey(pr); + if (!reviewFetchFailedEmitted.has(eventKey)) { + reviewFetchFailedEmitted.add(eventKey); + const errorMessage = err instanceof Error ? err.message : String(err); + recordActivityEvent({ + source: "scm", + kind: "scm.review_fetch_failed", + level: "warn", + summary: `getReviews discussions fetch failed for MR !${pr.number}`, + data: { + plugin: "scm-gitlab", + prNumber: pr.number, + prOwner: pr.owner, + prRepo: pr.repo, + errorMessage, + }, + }); + } } return reviews; diff --git a/packages/plugins/scm-gitlab/test/index.test.ts b/packages/plugins/scm-gitlab/test/index.test.ts index 9dad4d0eb..c72d93ee0 100644 --- a/packages/plugins/scm-gitlab/test/index.test.ts +++ b/packages/plugins/scm-gitlab/test/index.test.ts @@ -3,7 +3,10 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; // --------------------------------------------------------------------------- // Mock node:child_process — glab CLI calls go through execFileAsync // --------------------------------------------------------------------------- -const { glabMock } = vi.hoisted(() => ({ glabMock: vi.fn() })); +const { glabMock, recordActivityEventMock } = vi.hoisted(() => ({ + glabMock: vi.fn(), + recordActivityEventMock: vi.fn(), +})); vi.mock("node:child_process", () => { const execFile = Object.assign(vi.fn(), { @@ -12,8 +15,22 @@ vi.mock("node:child_process", () => { return { execFile }; }); -import { create, manifest } from "../src/index.js"; -import { createActivitySignal, type PRInfo, type Session, type ProjectConfig, type SCMWebhookRequest } from "@aoagents/ao-core"; +vi.mock("@aoagents/ao-core", async () => { + const actual = (await vi.importActual("@aoagents/ao-core")) as Record; + return { + ...actual, + recordActivityEvent: recordActivityEventMock, + }; +}); + +import { create, manifest, _resetGitLabActivityEventDedupeForTesting } from "../src/index.js"; +import { + createActivitySignal, + type PRInfo, + type Session, + type ProjectConfig, + type SCMWebhookRequest, +} from "@aoagents/ao-core"; // --------------------------------------------------------------------------- // Fixtures @@ -104,6 +121,7 @@ describe("scm-gitlab plugin", () => { beforeEach(() => { vi.clearAllMocks(); + _resetGitLabActivityEventDedupeForTesting(); warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); scm = create(); delete process.env["GITLAB_WEBHOOK_SECRET"]; @@ -701,6 +719,34 @@ describe("scm-gitlab plugin", () => { expect(await scm.getCISummary(pr)).toBe("failing"); }); + it("dedupes fail-closed activity events per MR", async () => { + mockGlabError("pipeline error"); + mockGlabError("network error"); + mockGlabError("pipeline error again"); + mockGlabError("network error again"); + + expect(await scm.getCISummary(pr)).toBe("failing"); + expect(await scm.getCISummary(pr)).toBe("failing"); + + const failClosedCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "scm.ci_summary_failclosed", + ); + expect(failClosedCalls).toHaveLength(1); + expect(failClosedCalls[0]?.[0]).toEqual( + expect.objectContaining({ + source: "scm", + kind: "scm.ci_summary_failclosed", + level: "warn", + data: expect.objectContaining({ + plugin: "scm-gitlab", + prNumber: 42, + prOwner: "acme", + prRepo: "repo", + }), + }), + ); + }); + it('returns "none" when all jobs are skipped', async () => { mockGlab([{ id: 1 }]); mockGlab([ @@ -807,6 +853,34 @@ describe("scm-gitlab plugin", () => { ); }); + it("dedupes review fetch failed activity events per MR", async () => { + mockGlab({ approved_by: [] }); + mockGlabError("discussions fetch failed"); + mockGlab({ approved_by: [] }); + mockGlabError("discussions fetch failed again"); + + expect(await scm.getReviews(pr)).toEqual([]); + expect(await scm.getReviews(pr)).toEqual([]); + + const reviewFetchCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "scm.review_fetch_failed", + ); + expect(reviewFetchCalls).toHaveLength(1); + expect(reviewFetchCalls[0]?.[0]).toEqual( + expect.objectContaining({ + source: "scm", + kind: "scm.review_fetch_failed", + level: "warn", + data: expect.objectContaining({ + plugin: "scm-gitlab", + prNumber: 42, + prOwner: "acme", + prRepo: "repo", + }), + }), + ); + }); + it("filters bot authors from discussions", async () => { mockGlab({ approved_by: [] }); mockGlab([ diff --git a/packages/plugins/tracker-linear/src/activity-events.test.ts b/packages/plugins/tracker-linear/src/activity-events.test.ts new file mode 100644 index 000000000..06d552ee4 --- /dev/null +++ b/packages/plugins/tracker-linear/src/activity-events.test.ts @@ -0,0 +1,143 @@ +/** + * Regression tests for plugin-internal activity events (issue #1659). + * + * Covers tracker.dep_missing (MUST emit, deduped once-per-process). + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const { recordActivityEventMock, requestMock } = vi.hoisted(() => ({ + recordActivityEventMock: vi.fn(), + requestMock: vi.fn(), +})); + +vi.mock("node:https", () => ({ + request: requestMock, +})); + +vi.mock("@aoagents/ao-core", async () => { + const actual = (await vi.importActual("@aoagents/ao-core")) as Record; + return { + ...actual, + recordActivityEvent: recordActivityEventMock, + }; +}); + +// @composio/core is intentionally not installed — the real dynamic import +// will fail with ERR_MODULE_NOT_FOUND, which is exactly the dep_missing +// shape we want to exercise. + +import { create, _resetDepMissingEmittedForTesting } from "./index.js"; +import type { ProjectConfig } from "@aoagents/ao-core"; + +beforeEach(() => { + vi.clearAllMocks(); + recordActivityEventMock.mockReset(); + requestMock.mockReset(); + _resetDepMissingEmittedForTesting(); + process.env.COMPOSIO_API_KEY = "test-key"; + process.env.COMPOSIO_ENTITY_ID = "test-entity"; + delete process.env.LINEAR_API_KEY; +}); + +afterEach(() => { + delete process.env.COMPOSIO_API_KEY; + delete process.env.COMPOSIO_ENTITY_ID; + delete process.env.LINEAR_API_KEY; + vi.useRealTimers(); +}); + +function makeProject(): ProjectConfig { + return { + name: "test-project", + repo: "test/repo", + path: "/repo/path", + defaultBranch: "main", + sessionPrefix: "test", + tracker: { teamId: "TEAM-1" }, + }; +} + +describe("tracker.dep_missing (MUST emit)", () => { + it("emits when Composio SDK is not installed", async () => { + const tracker = create(); + + // Any tracker call routes through the composio transport, which will + // fail to load the missing SDK on first use. + await expect(tracker.getIssue("TEST-1", makeProject())).rejects.toThrow( + /Composio SDK.*not installed/, + ); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "tracker", + kind: "tracker.dep_missing", + level: "error", + data: expect.objectContaining({ + plugin: "tracker-linear", + package: "@composio/core", + }), + }), + ); + }); + + it("emits exactly once across multiple calls (deduped per-process)", async () => { + const tracker = create(); + + await expect(tracker.getIssue("TEST-1", makeProject())).rejects.toThrow(); + await expect(tracker.getIssue("TEST-2", makeProject())).rejects.toThrow(); + await expect(tracker.getIssue("TEST-3", makeProject())).rejects.toThrow(); + + const depMissingCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "tracker.dep_missing", + ); + expect(depMissingCalls).toHaveLength(1); + }); +}); + +describe("tracker.api_timeout", () => { + it("rejects direct transport timeouts even when activity logging throws", async () => { + delete process.env.COMPOSIO_API_KEY; + delete process.env.COMPOSIO_ENTITY_ID; + process.env.LINEAR_API_KEY = "linear-key"; + vi.useFakeTimers(); + + const req = { + setTimeout: vi.fn(), + on: vi.fn(), + write: vi.fn(), + end: vi.fn(), + destroy: vi.fn(), + }; + req.setTimeout.mockImplementation((_timeoutMs: number, cb: () => void) => { + setTimeout(cb, 0); + return req; + }); + req.on.mockReturnValue(req); + requestMock.mockReturnValue(req); + recordActivityEventMock.mockImplementationOnce(() => { + throw new Error("activity sink failed"); + }); + + const tracker = create(); + const timeoutExpectation = expect(tracker.getIssue("TEST-1", makeProject())).rejects.toThrow( + "Linear API request timed out after 30s", + ); + + await vi.runAllTimersAsync(); + await timeoutExpectation; + expect(req.destroy).toHaveBeenCalled(); + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "tracker", + kind: "tracker.api_timeout", + level: "warn", + data: expect.objectContaining({ + plugin: "tracker-linear", + transport: "direct", + timeoutMs: 30_000, + }), + }), + ); + }); +}); diff --git a/packages/plugins/tracker-linear/src/index.ts b/packages/plugins/tracker-linear/src/index.ts index fd93e3eb8..6101ac1c3 100644 --- a/packages/plugins/tracker-linear/src/index.ts +++ b/packages/plugins/tracker-linear/src/index.ts @@ -9,17 +9,35 @@ */ import { request } from "node:https"; -import type { - PluginModule, - Tracker, - Issue, - IssueFilters, - IssueUpdate, - CreateIssueInput, - ProjectConfig, +import { + recordActivityEvent, + type CreateIssueInput, + type Issue, + type IssueFilters, + type IssueUpdate, + type PluginModule, + type ProjectConfig, + type Tracker, } from "@aoagents/ao-core"; import type { Composio } from "@composio/core"; +// Module-level guard so we only emit tracker.dep_missing once per process +// even if multiple sessions trigger the missing-SDK path. +let depMissingEmitted = false; + +/** Test-only: reset the once-per-process dep_missing guard. */ +export function _resetDepMissingEmittedForTesting(): void { + depMissingEmitted = false; +} + +function recordTransportActivityEvent(event: Parameters[0]): void { + try { + recordActivityEvent(event); + } catch { + // Activity logging must never prevent timeout promises from settling. + } +} + // --------------------------------------------------------------------------- // Transport abstraction // --------------------------------------------------------------------------- @@ -111,6 +129,17 @@ function createDirectTransport(): GraphQLTransport { req.setTimeout(30_000, () => { settle(() => { req.destroy(); + recordTransportActivityEvent({ + source: "tracker", + kind: "tracker.api_timeout", + level: "warn", + summary: "Linear API request timed out after 30s", + data: { + plugin: "tracker-linear", + transport: "direct", + timeoutMs: 30_000, + }, + }); reject(new Error("Linear API request timed out after 30s")); }); }); @@ -147,6 +176,21 @@ function createComposioTransport(apiKey: string, entityId: string): GraphQLTrans msg.includes("Cannot find package") || msg.includes("ERR_MODULE_NOT_FOUND") ) { + // User-actionable, system-wide. Emit once per process. + if (!depMissingEmitted) { + depMissingEmitted = true; + recordActivityEvent({ + source: "tracker", + kind: "tracker.dep_missing", + level: "error", + summary: "Composio SDK (@composio/core) is not installed", + data: { + plugin: "tracker-linear", + package: "@composio/core", + installHint: "pnpm add @composio/core", + }, + }); + } throw new Error( "Composio SDK (@composio/core) is not installed. " + "Install it with: pnpm add @composio/core", @@ -175,6 +219,17 @@ function createComposioTransport(apiKey: string, entityId: string): GraphQLTrans let timer: ReturnType | undefined; const timeoutPromise = new Promise((_resolve, reject) => { timer = setTimeout(() => { + recordTransportActivityEvent({ + source: "tracker", + kind: "tracker.api_timeout", + level: "warn", + summary: "Composio Linear API request timed out after 30s", + data: { + plugin: "tracker-linear", + transport: "composio", + timeoutMs: 30_000, + }, + }); reject(new Error("Composio Linear API request timed out after 30s")); }, 30_000); }); diff --git a/packages/plugins/workspace-clone/src/__tests__/index.test.ts b/packages/plugins/workspace-clone/src/__tests__/index.test.ts index 2b194ccd3..0c2b1da45 100644 --- a/packages/plugins/workspace-clone/src/__tests__/index.test.ts +++ b/packages/plugins/workspace-clone/src/__tests__/index.test.ts @@ -3,6 +3,20 @@ import * as childProcess from "node:child_process"; import * as fs from "node:fs"; import type { ProjectConfig } from "@aoagents/ao-core"; +const { getShellMock, recordActivityEventMock } = vi.hoisted(() => ({ + getShellMock: vi.fn(() => ({ cmd: "sh", args: (c: string) => ["-c", c] })), + recordActivityEventMock: vi.fn(), +})); + +vi.mock("@aoagents/ao-core", async () => { + const actual = (await vi.importActual("@aoagents/ao-core")) as Record; + return { + ...actual, + getShell: getShellMock, + recordActivityEvent: recordActivityEventMock, + }; +}); + // Mock node:child_process with custom promisify support vi.mock("node:child_process", () => { const mockExecFile = vi.fn(); @@ -10,10 +24,6 @@ vi.mock("node:child_process", () => { return { execFile: mockExecFile }; }); -vi.mock("@aoagents/ao-core", () => ({ - getShell: vi.fn(() => ({ cmd: "sh", args: (c: string) => ["-c", c] })), -})); - // Mock node:fs vi.mock("node:fs", () => ({ existsSync: vi.fn(), @@ -267,9 +277,48 @@ describe("workspace.create()", () => { cwd: "/mock-home/.ao-clones/proj/sess", }); + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "workspace", + kind: "workspace.branch_collision", + level: "warn", + projectId: "proj", + sessionId: "sess", + data: expect.objectContaining({ + plugin: "workspace-clone", + branch: "feat/existing", + errorMessage: expect.stringContaining("already exists"), + }), + }), + ); expect(info.branch).toBe("feat/existing"); }); + it("does not emit branch_collision when checkout -b fails for a non-collision reason", async () => { + const workspace = create(); + + mockGitSuccess("https://github.com/test/repo.git"); + (fs.existsSync as ReturnType).mockReturnValue(false); + mockGitSuccess(""); + // git checkout -b fails for a non-collision reason + mockGitError("fatal: cannot lock ref 'refs/heads/feat/locked': Permission denied"); + // git checkout (plain) succeeds + mockGitSuccess(""); + + const info = await workspace.create({ + projectId: "proj", + sessionId: "sess", + branch: "feat/locked", + project: makeProject(), + }); + + expect(info.branch).toBe("feat/locked"); + const branchCollisionCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "workspace.branch_collision", + ); + expect(branchCollisionCalls).toHaveLength(0); + }); + it("cleans up partial clone on clone failure", async () => { const workspace = create(); @@ -588,6 +637,41 @@ describe("workspace.list()", () => { warnSpy.mockRestore(); }); + it("emits corrupt_clone_skipped only once per clone path", async () => { + const workspace = create(); + (fs.existsSync as ReturnType).mockReturnValue(true); + + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + (fs.readdirSync as ReturnType).mockReturnValue([ + { name: "corrupt-repeat", isDirectory: () => true }, + ]); + + mockGitError("fatal: not a git repository"); + mockGitError("fatal: not a git repository"); + + await expect(workspace.list("myproject")).resolves.toEqual([]); + await expect(workspace.list("myproject")).resolves.toEqual([]); + + const corruptCloneCalls = recordActivityEventMock.mock.calls.filter( + ([event]) => event.kind === "workspace.corrupt_clone_skipped", + ); + expect(corruptCloneCalls).toHaveLength(1); + expect(corruptCloneCalls[0][0]).toEqual( + expect.objectContaining({ + projectId: "myproject", + sessionId: "corrupt-repeat", + source: "workspace", + kind: "workspace.corrupt_clone_skipped", + data: expect.objectContaining({ + clonePath: "/mock-home/.ao-clones/myproject/corrupt-repeat", + }), + }), + ); + + warnSpy.mockRestore(); + }); + it("rejects invalid projectId with special characters", async () => { const workspace = create(); diff --git a/packages/plugins/workspace-clone/src/index.ts b/packages/plugins/workspace-clone/src/index.ts index 075fc3119..71100644a 100644 --- a/packages/plugins/workspace-clone/src/index.ts +++ b/packages/plugins/workspace-clone/src/index.ts @@ -3,7 +3,15 @@ import { promisify } from "node:util"; import { existsSync, rmSync, mkdirSync, readdirSync } from "node:fs"; import { join } from "node:path"; import { homedir } from "node:os"; -import { getShell, type PluginModule, type Workspace, type WorkspaceCreateConfig, type WorkspaceInfo, type ProjectConfig } from "@aoagents/ao-core"; +import { + getShell, + recordActivityEvent, + type PluginModule, + type ProjectConfig, + type Workspace, + type WorkspaceCreateConfig, + type WorkspaceInfo, +} from "@aoagents/ao-core"; const execFileAsync = promisify(execFile); @@ -37,6 +45,16 @@ function expandPath(p: string): string { return p; } +function getErrorMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +function isBranchAlreadyExistsError(err: unknown): boolean { + return getErrorMessage(err).toLowerCase().includes("already exists"); +} + +const emittedCorruptClonePaths = new Set(); + export function create(config?: Record): Workspace { const cloneBaseDir = config?.cloneDir ? expandPath(config.cloneDir as string) @@ -97,8 +115,24 @@ export function create(config?: Record): Workspace { // Create and checkout the feature branch try { await git(clonePath, "checkout", "-b", cfg.branch); - } catch { - // Branch may exist on remote — try plain checkout + } catch (branchErr: unknown) { + // Branch may exist on remote — try plain checkout, but only label it + // as a branch_collision when git reported the distinct collision shape. + if (isBranchAlreadyExistsError(branchErr)) { + recordActivityEvent({ + projectId: cfg.projectId, + sessionId: cfg.sessionId, + source: "workspace", + kind: "workspace.branch_collision", + level: "warn", + summary: `branch "${cfg.branch}" already exists; falling back to checkout`, + data: { + plugin: "workspace-clone", + branch: cfg.branch, + errorMessage: getErrorMessage(branchErr), + }, + }); + } try { await git(clonePath, "checkout", cfg.branch); } catch (checkoutErr: unknown) { @@ -142,10 +176,27 @@ export function create(config?: Record): Workspace { try { branch = await git(clonePath, "branch", "--show-current"); } catch (err: unknown) { - // Warn about corrupted clones instead of silently skipping + // Warn about corrupted clones instead of silently skipping. + // RCA: "session shows up on disk but isn't returned by list()". const msg = err instanceof Error ? err.message : String(err); // eslint-disable-next-line no-console -- expected diagnostic for corrupted clones console.warn(`[workspace-clone] Skipping "${entry.name}": not a valid git repo (${msg})`); + if (!emittedCorruptClonePaths.has(clonePath)) { + emittedCorruptClonePaths.add(clonePath); + recordActivityEvent({ + projectId, + sessionId: entry.name, + source: "workspace", + kind: "workspace.corrupt_clone_skipped", + level: "warn", + summary: `skipped corrupt clone "${entry.name}"`, + data: { + plugin: "workspace-clone", + clonePath, + errorMessage: msg, + }, + }); + } continue; } diff --git a/packages/plugins/workspace-worktree/src/__tests__/activity-events.test.ts b/packages/plugins/workspace-worktree/src/__tests__/activity-events.test.ts new file mode 100644 index 000000000..3a2f6039c --- /dev/null +++ b/packages/plugins/workspace-worktree/src/__tests__/activity-events.test.ts @@ -0,0 +1,192 @@ +/** + * Regression tests for plugin-internal activity events (issue #1659). + * + * Covers the MUST emit: workspace.post_create_failed, plus the SHOULDs + * workspace.branch_collision and workspace.destroy_fell_back. + */ + +import { describe, it, expect, beforeEach, vi } from "vitest"; + +// --------------------------------------------------------------------------- +// Mocks — declared before any import that uses the mocked modules +// --------------------------------------------------------------------------- + +const { recordActivityEventMock } = vi.hoisted(() => ({ + recordActivityEventMock: vi.fn(), +})); + +vi.mock("@aoagents/ao-core", async () => { + const actual = (await vi.importActual("@aoagents/ao-core")) as Record; + return { + ...actual, + recordActivityEvent: recordActivityEventMock, + }; +}); + +vi.mock("node:child_process", () => { + const mockExecFile = vi.fn(); + (mockExecFile as unknown as Record)[Symbol.for("nodejs.util.promisify.custom")] = + vi.fn(); + return { execFile: mockExecFile }; +}); + +vi.mock("node:fs", () => ({ + cpSync: vi.fn(), + existsSync: vi.fn(() => false), + linkSync: vi.fn(), + lstatSync: vi.fn(), + statSync: vi.fn(), + symlinkSync: vi.fn(), + rmSync: vi.fn(), + mkdirSync: vi.fn(), + readdirSync: vi.fn(), +})); + +vi.mock("node:os", () => ({ homedir: () => "/mock-home" })); + +import * as childProcess from "node:child_process"; +import { create } from "../index.js"; +import type { ProjectConfig, WorkspaceCreateConfig, WorkspaceInfo } from "@aoagents/ao-core/types"; + +const mockExecFileAsync = (childProcess.execFile as unknown as Record)[ + Symbol.for("nodejs.util.promisify.custom") +] as ReturnType; + +function makeProject(overrides?: Partial): ProjectConfig { + return { + name: "test-project", + repo: "test/repo", + path: "/repo/path", + defaultBranch: "main", + sessionPrefix: "test", + ...overrides, + }; +} + +function makeCreateConfig(overrides?: Partial): WorkspaceCreateConfig { + return { + projectId: "myproject", + project: makeProject(), + sessionId: "session-1", + branch: "feat/TEST-1", + ...overrides, + }; +} + +function makeWorkspaceInfo(): WorkspaceInfo { + return { + path: "/mock-home/.worktrees/myproject/session-1", + branch: "feat/TEST-1", + sessionId: "session-1", + projectId: "myproject", + }; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("workspace.post_create_failed (MUST emit)", () => { + it("emits when a postCreate command fails, then rethrows", async () => { + const ws = create(); + const project = makeProject({ postCreate: ["pnpm install"] }); + + mockExecFileAsync.mockRejectedValueOnce(new Error("Command failed: exit 127")); + + await expect(ws.postCreate!(makeWorkspaceInfo(), project)).rejects.toThrow( + "Command failed: exit 127", + ); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "workspace", + kind: "workspace.post_create_failed", + level: "error", + sessionId: "session-1", + projectId: "myproject", + data: expect.objectContaining({ + plugin: "workspace-worktree", + command: "pnpm install", + errorMessage: expect.stringContaining("exit 127"), + }), + }), + ); + }); + + it("does NOT emit when postCreate command succeeds", async () => { + const ws = create(); + const project = makeProject({ postCreate: ["echo hi"] }); + + mockExecFileAsync.mockResolvedValueOnce({ stdout: "hi\n", stderr: "" }); + + await ws.postCreate!(makeWorkspaceInfo(), project); + + expect(recordActivityEventMock).not.toHaveBeenCalled(); + }); +}); + +describe("workspace.branch_collision (SHOULD emit)", () => { + it("emits when worktree add -b fails because branch already exists", async () => { + const ws = create(); + + // git remote get-url origin + mockExecFileAsync.mockResolvedValueOnce({ stdout: "origin\n", stderr: "" }); + // git fetch origin --quiet + mockExecFileAsync.mockResolvedValueOnce({ stdout: "", stderr: "" }); + // resolveBaseRef -> rev-parse origin/main + mockExecFileAsync.mockResolvedValueOnce({ stdout: "abc\n", stderr: "" }); + // git worktree add -b ... → already exists + mockExecFileAsync.mockRejectedValueOnce( + new Error("fatal: a branch named 'feat/TEST-1' already exists"), + ); + // rev-parse baseRef for stale-branch comparison + mockExecFileAsync.mockResolvedValueOnce({ stdout: "abc\n", stderr: "" }); + // refExists(branchRef) -> true + mockExecFileAsync.mockResolvedValueOnce({ stdout: "refs/heads/feat/TEST-1\n", stderr: "" }); + // rev-parse existing branch -> same as base, so reuse it + mockExecFileAsync.mockResolvedValueOnce({ stdout: "abc\n", stderr: "" }); + // git worktree add (without -b) — succeeds + mockExecFileAsync.mockResolvedValueOnce({ stdout: "", stderr: "" }); + + await ws.create(makeCreateConfig()); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "workspace", + kind: "workspace.branch_collision", + level: "warn", + projectId: "myproject", + sessionId: "session-1", + data: expect.objectContaining({ + plugin: "workspace-worktree", + branch: "feat/TEST-1", + errorMessage: expect.stringContaining("already exists"), + }), + }), + ); + }); +}); + +describe("workspace.destroy_fell_back (SHOULD emit)", () => { + it("emits when destroy() falls back to rmSync after git failure", async () => { + const ws = create(); + + // git rev-parse --git-common-dir → fails + mockExecFileAsync.mockRejectedValueOnce(new Error("not a git repository")); + + await ws.destroy("/some/stale/path"); + + expect(recordActivityEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + source: "workspace", + kind: "workspace.destroy_fell_back", + level: "warn", + data: expect.objectContaining({ + plugin: "workspace-worktree", + workspacePath: "/some/stale/path", + errorMessage: expect.stringContaining("not a git repository"), + }), + }), + ); + }); +}); diff --git a/packages/plugins/workspace-worktree/src/__tests__/index.test.ts b/packages/plugins/workspace-worktree/src/__tests__/index.test.ts index f4837fd62..c39afd6d9 100644 --- a/packages/plugins/workspace-worktree/src/__tests__/index.test.ts +++ b/packages/plugins/workspace-worktree/src/__tests__/index.test.ts @@ -5,6 +5,10 @@ import type { ProjectConfig, WorkspaceCreateConfig, WorkspaceInfo } from "@aoage // Mocks — must be declared before any import that uses the mocked modules // --------------------------------------------------------------------------- +const { recordActivityEventMock } = vi.hoisted(() => ({ + recordActivityEventMock: vi.fn(), +})); + vi.mock("node:child_process", () => { const mockExecFile = vi.fn(); // Set custom promisify so `promisify(execFile)` returns { stdout, stderr } @@ -27,6 +31,7 @@ vi.mock("node:fs", () => ({ vi.mock("@aoagents/ao-core", () => ({ getShell: vi.fn(() => ({ cmd: "sh", args: (c: string) => ["-c", c] })), isWindows: vi.fn(() => false), + recordActivityEvent: recordActivityEventMock, })); vi.mock("node:os", () => ({ diff --git a/packages/plugins/workspace-worktree/src/index.ts b/packages/plugins/workspace-worktree/src/index.ts index 7e0b6f61e..1f5c32840 100644 --- a/packages/plugins/workspace-worktree/src/index.ts +++ b/packages/plugins/workspace-worktree/src/index.ts @@ -16,6 +16,7 @@ import { homedir } from "node:os"; import { getShell, isWindows, + recordActivityEvent, type PluginModule, type Workspace, type WorkspaceCreateConfig, @@ -361,7 +362,21 @@ export function create(config?: Record): Workspace { // Branch already exists. It may be a stale session branch left behind // from an earlier spawn, so compare it with the freshly-resolved base - // before reusing it. + // before reusing it. Surface the collision shape for RCA before the + // recovery path decides whether to reuse or reset the local branch. + recordActivityEvent({ + projectId: cfg.projectId, + sessionId: cfg.sessionId, + source: "workspace", + kind: "workspace.branch_collision", + level: "warn", + summary: `branch "${cfg.branch}" already exists; falling back to worktree recovery`, + data: { + plugin: "workspace-worktree", + branch: cfg.branch, + errorMessage: msg, + }, + }); const baseSha = await git(repoPath, "rev-parse", baseRef); const branchRef = `refs/heads/${cfg.branch}`; const existingBranchSha = (await refExists(repoPath, branchRef)) @@ -453,8 +468,24 @@ export function create(config?: Record): Workspace { // pre-existing local branches unrelated to this workspace (any branch // containing "/" would have been deleted). Stale branches can be // cleaned up separately via `git branch --merged` or similar. - } catch { + } catch (err) { // If git commands fail, try to clean up the directory. + // The worktree metadata may be left stale in `git worktree list` + // because we couldn't run `worktree remove`. Surface so RCA can + // explain why a path was deleted but `git worktree list` still + // references it. + const errorMessage = err instanceof Error ? err.message : String(err); + recordActivityEvent({ + source: "workspace", + kind: "workspace.destroy_fell_back", + level: "warn", + summary: "destroy fell back to rmSync; git worktree metadata may be stale", + data: { + plugin: "workspace-worktree", + workspacePath, + errorMessage, + }, + }); // On Windows, retry with backoff for the file-handle drain race // (just-killed pty-host children still hold handles inside the worktree). if (existsSync(workspacePath)) { @@ -661,10 +692,31 @@ export function create(config?: Record): Workspace { if (project.postCreate) { const shell = getShell(); for (const command of project.postCreate) { - await execFileAsync(shell.cmd, shell.args(command), { - cwd: info.path, - windowsHide: true, - }); + try { + await execFileAsync(shell.cmd, shell.args(command), { + cwd: info.path, + windowsHide: true, + }); + } catch (err) { + // Surface which postCreate command failed. Lifecycle records + // a generic spawn_failed but loses the specific command and + // its sanitized error output. + const errorMessage = err instanceof Error ? err.message : String(err); + recordActivityEvent({ + projectId: info.projectId, + sessionId: info.sessionId, + source: "workspace", + kind: "workspace.post_create_failed", + level: "error", + summary: `postCreate command failed for session ${info.sessionId}`, + data: { + plugin: "workspace-worktree", + command, + errorMessage, + }, + }); + throw err; + } } } }, 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__/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/__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/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 }); } } 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 }]; }); }