diff --git a/.changeset/harden-worker-branch-refresh.md b/.changeset/harden-worker-branch-refresh.md new file mode 100644 index 000000000..f65c22467 --- /dev/null +++ b/.changeset/harden-worker-branch-refresh.md @@ -0,0 +1,5 @@ +--- +"@aoagents/ao-core": patch +--- + +Harden worker branch refresh during lifecycle polling by preserving branch metadata through transient detached Git states, skipping orchestrators and active open PRs, and preventing duplicate branch adoption within a single poll cycle. diff --git a/packages/core/src/__tests__/lifecycle-manager.test.ts b/packages/core/src/__tests__/lifecycle-manager.test.ts index b01794be6..514b8b9d1 100644 --- a/packages/core/src/__tests__/lifecycle-manager.test.ts +++ b/packages/core/src/__tests__/lifecycle-manager.test.ts @@ -1,4 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { mkdirSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; import { createLifecycleManager } from "../lifecycle-manager.js"; import { DEFAULT_BUGBOT_COMMENTS_MESSAGE } from "../config.js"; import { @@ -813,6 +815,7 @@ describe("check (single session)", () => { status: "working", branch: "feat/test", pr: null, + workspacePath: null, metadata: { agent: "mock-agent" }, }), metaOverrides: { branch: "feat/test", agent: "mock-agent" }, @@ -827,6 +830,466 @@ describe("check (single session)", () => { expect(lm.getStates().get("app-1")).toBe("stuck"); }); + it("refreshes worker branch metadata from the current worktree HEAD before PR detection", async () => { + const workspacePath = join(env.tmpDir, "worker-ws"); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-1"); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + writeFileSync(join(gitDir, "HEAD"), "ref: refs/heads/fix-login-v2\n"); + + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "working", + branch: "fix-login", + workspacePath, + pr: null, + metadata: { agent: "mock-agent" }, + }), + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).toHaveBeenCalledWith( + expect.objectContaining({ branch: "fix-login-v2" }), + expect.anything(), + ); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBe("fix-login-v2"); + }); + + it("refreshes worker branch metadata for clone-style repos with a .git directory", async () => { + const workspacePath = join(env.tmpDir, "worker-clone"); + const gitDir = join(workspacePath, ".git"); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(gitDir, "HEAD"), "ref: refs/heads/fix-login-v2\n"); + + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "working", + branch: "fix-login", + workspacePath, + pr: null, + metadata: { agent: "mock-agent" }, + }), + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).toHaveBeenCalledWith( + expect.objectContaining({ branch: "fix-login-v2" }), + expect.anything(), + ); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBe("fix-login-v2"); + }); + + it("does not overwrite an attached PR branch from a workspace checkout change", async () => { + const workspacePath = join(env.tmpDir, "worker-ws-pr"); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-1-pr"); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + writeFileSync(join(gitDir, "HEAD"), "ref: refs/heads/fix-login-v2\n"); + + const pr = makePR({ branch: "fix-login" }); + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "pr_open", + branch: "fix-login", + workspacePath, + pr, + metadata: { agent: "mock-agent" }, + }), + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + pr: pr.url, + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).not.toHaveBeenCalled(); + expect(pr.branch).toBe("fix-login"); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBe("fix-login"); + }); + + it("refreshes branch metadata again after a closed PR when the worker switches branches", async () => { + const workspacePath = join(env.tmpDir, "worker-ws-closed-pr"); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-1-closed-pr"); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + writeFileSync(join(gitDir, "HEAD"), "ref: refs/heads/follow-up-fix\n"); + + const closedPR = makePR({ branch: "fix-login", url: "https://github.com/org/repo/pull/42" }); + const followUpPR = makePR({ + number: 43, + branch: "follow-up-fix", + url: "https://github.com/org/repo/pull/43", + title: "Follow up fix", + }); + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(followUpPR) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const session = makeSession({ + status: "idle", + branch: "fix-login", + workspacePath, + pr: closedPR, + metadata: { agent: "mock-agent" }, + }); + session.lifecycle.pr.state = "closed"; + session.lifecycle.pr.reason = "closed_unmerged"; + session.lifecycle.pr.number = closedPR.number; + session.lifecycle.pr.url = closedPR.url; + session.lifecycle.pr.lastObservedAt = new Date().toISOString(); + session.lifecycle.session.state = "idle"; + session.lifecycle.session.reason = "pr_closed_waiting_decision"; + + const lm = setupCheck("app-1", { + session, + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + pr: closedPR.url, + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).toHaveBeenCalledWith( + expect.objectContaining({ branch: "follow-up-fix" }), + expect.anything(), + ); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBe("follow-up-fix"); + expect(meta?.["pr"]).toBe(followUpPR.url); + }); + + it("clears stale worker branch metadata when the current worktree HEAD is detached", async () => { + const workspacePath = join(env.tmpDir, "worker-ws-detached"); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-1-detached"); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + writeFileSync(join(gitDir, "HEAD"), "6f1d2c3b4a5e67890123456789abcdef01234567\n"); + + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "working", + branch: "fix-login", + workspacePath, + pr: null, + metadata: { agent: "mock-agent" }, + }), + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).not.toHaveBeenCalled(); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBeUndefined(); + }); + + for (const marker of [ + "rebase-merge", + "rebase-apply", + "CHERRY_PICK_HEAD", + "BISECT_LOG", + ] as const) { + it(`keeps the previous branch during transient detached git state: ${marker}`, async () => { + const workspacePath = join(env.tmpDir, `worker-ws-${marker}`); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", `app-1-${marker}`); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + writeFileSync(join(gitDir, "HEAD"), "6f1d2c3b4a5e67890123456789abcdef01234567\n"); + if (marker.includes("/")) { + mkdirSync(join(gitDir, marker), { recursive: true }); + } else { + if (marker === "rebase-merge" || marker === "rebase-apply") { + mkdirSync(join(gitDir, marker), { recursive: true }); + } else { + writeFileSync(join(gitDir, marker), "in-progress\n"); + } + } + + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "working", + branch: "fix-login", + workspacePath, + pr: null, + metadata: { agent: "mock-agent" }, + }), + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).toHaveBeenCalledWith( + expect.objectContaining({ branch: "fix-login" }), + expect.anything(), + ); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBe("fix-login"); + }); + } + + it("keeps existing branch metadata when the current worktree HEAD cannot be read", async () => { + const workspacePath = join(env.tmpDir, "worker-ws-missing-head"); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-1-missing-head"); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const lm = setupCheck("app-1", { + session: makeSession({ + status: "working", + branch: "fix-login", + workspacePath, + pr: null, + metadata: { agent: "mock-agent" }, + }), + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).toHaveBeenCalledWith( + expect.objectContaining({ branch: "fix-login" }), + expect.anything(), + ); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBe("fix-login"); + }); + + it("does not adopt a branch already tracked by another active worker", async () => { + const workspacePath = join(env.tmpDir, "worker-ws-conflict"); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-1-conflict"); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + writeFileSync(join(gitDir, "HEAD"), "ref: refs/heads/fix-login-v2\n"); + + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const session = makeSession({ + id: "app-1", + status: "working", + branch: "fix-login", + workspacePath, + pr: null, + metadata: { agent: "mock-agent" }, + }); + const sibling = makeSession({ + id: "app-2", + status: "working", + branch: "fix-login-v2", + workspacePath: null, + pr: null, + metadata: { agent: "mock-agent" }, + }); + + const lm = setupCheck("app-1", { + session, + metaOverrides: { + worktree: workspacePath, + branch: "fix-login", + agent: "mock-agent", + }, + registry, + }); + vi.mocked(mockSessionManager.list).mockResolvedValue([session, sibling]); + + await lm.check("app-1"); + + expect(mockSCM.detectPR).toHaveBeenCalledWith( + expect.objectContaining({ branch: "fix-login" }), + expect.anything(), + ); + const meta = readMetadataRaw(env.sessionsDir, "app-1"); + expect(meta?.["branch"]).toBe("fix-login"); + }); + + it("serializes competing branch adoption within one poll cycle without extra session list calls", async () => { + const workspacePathA = join(env.tmpDir, "worker-ws-race-a"); + const workspacePathB = join(env.tmpDir, "worker-ws-race-b"); + const gitDirA = join(env.tmpDir, "repo", ".git", "worktrees", "app-1-race"); + const gitDirB = join(env.tmpDir, "repo", ".git", "worktrees", "app-2-race"); + mkdirSync(workspacePathA, { recursive: true }); + mkdirSync(workspacePathB, { recursive: true }); + mkdirSync(gitDirA, { recursive: true }); + mkdirSync(gitDirB, { recursive: true }); + writeFileSync(join(workspacePathA, ".git"), `gitdir: ${gitDirA}\n`); + writeFileSync(join(workspacePathB, ".git"), `gitdir: ${gitDirB}\n`); + writeFileSync(join(gitDirA, "HEAD"), "ref: refs/heads/shared-branch\n"); + writeFileSync(join(gitDirB, "HEAD"), "ref: refs/heads/shared-branch\n"); + + const sessionA = makeSession({ + id: "app-1", + status: "working", + branch: "old-a", + workspacePath: workspacePathA, + pr: null, + metadata: { agent: "mock-agent" }, + }); + const sessionB = makeSession({ + id: "app-2", + status: "working", + branch: "old-b", + workspacePath: workspacePathB, + pr: null, + metadata: { agent: "mock-agent" }, + }); + vi.mocked(mockSessionManager.list).mockResolvedValue([sessionA, sessionB]); + + const lm = createLifecycleManager({ + config, + registry: mockRegistry, + sessionManager: mockSessionManager, + }); + + try { + lm.start(60_000); + await new Promise((resolve) => setTimeout(resolve, 25)); + + const adoptedCount = [sessionA.branch, sessionB.branch].filter( + (branch) => branch === "shared-branch", + ).length; + expect(adoptedCount).toBe(1); + expect(mockSessionManager.list).toHaveBeenCalledTimes(1); + } finally { + lm.stop(); + } + }); + + it("skips branch refresh for orchestrator sessions", async () => { + const workspacePath = join(env.tmpDir, "orchestrator-ws"); + const gitDir = join(env.tmpDir, "repo", ".git", "worktrees", "app-orchestrator-1"); + mkdirSync(workspacePath, { recursive: true }); + mkdirSync(gitDir, { recursive: true }); + writeFileSync(join(workspacePath, ".git"), `gitdir: ${gitDir}\n`); + writeFileSync(join(gitDir, "HEAD"), "ref: refs/heads/orchestrator-new\n"); + + const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(null) }); + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + const lm = setupCheck("app-orchestrator-1", { + session: makeSession({ + id: "app-orchestrator-1", + status: "working", + branch: "orchestrator-old", + workspacePath, + pr: null, + metadata: { agent: "mock-agent", role: "orchestrator" }, + }), + metaOverrides: { + worktree: workspacePath, + branch: "orchestrator-old", + role: "orchestrator", + agent: "mock-agent", + }, + registry, + }); + + await lm.check("app-orchestrator-1"); + + expect(mockSCM.detectPR).not.toHaveBeenCalled(); + const meta = readMetadataRaw(env.sessionsDir, "app-orchestrator-1"); + expect(meta?.["branch"]).toBe("orchestrator-old"); + }); + it("preserves stuck state when getActivityState throws", async () => { vi.mocked(plugins.agent.getActivityState).mockRejectedValue(new Error("probe failed")); @@ -1295,6 +1758,7 @@ describe("reactions", () => { const staleSession = makeSession({ id: "app-1", status: "working", + workspacePath: null, createdAt: new Date("2025-01-01T11:40:00.000Z"), metadata: { createdAt: "2025-01-01T11:40:00.000Z", @@ -1445,19 +1909,22 @@ describe("reactions", () => { }; const mockSCM = createMockSCM({ - getReviewThreads: vi.fn().mockResolvedValue({ threads: [ - { - id: "c1", - author: "reviewer", - body: "Please rename this helper", - path: "src/app.ts", - line: 12, - isResolved: false, - createdAt: new Date(), - url: "https://example.com/comment/1", - isBot: false, - }, - ], reviews: [] }), + getReviewThreads: vi.fn().mockResolvedValue({ + threads: [ + { + id: "c1", + author: "reviewer", + body: "Please rename this helper", + path: "src/app.ts", + line: 12, + isResolved: false, + createdAt: new Date(), + url: "https://example.com/comment/1", + isBot: false, + }, + ], + reviews: [], + }), }); const registry = createMockRegistry({ runtime: plugins.runtime, @@ -1502,24 +1969,30 @@ describe("reactions", () => { const result = new Map(); for (const pr of prs) { result.set(`${pr.owner}/${pr.repo}#${pr.number}`, { - state: "open", ciStatus: "passing", reviewDecision: "changes_requested", mergeable: false, + state: "open", + ciStatus: "passing", + reviewDecision: "changes_requested", + mergeable: false, }); } return result; }), - getReviewThreads: vi.fn().mockResolvedValue({ threads: [ - { - id: "c1", - author: "reviewer", - body: "Please add validation", - path: "src/route.ts", - line: 44, - isResolved: false, - createdAt: new Date(), - url: "https://example.com/comment/2", - isBot: false, - }, - ], reviews: [] }), + getReviewThreads: vi.fn().mockResolvedValue({ + threads: [ + { + id: "c1", + author: "reviewer", + body: "Please add validation", + path: "src/route.ts", + line: 44, + isResolved: false, + createdAt: new Date(), + url: "https://example.com/comment/2", + isBot: false, + }, + ], + reviews: [], + }), }); const registry = createMockRegistry({ runtime: plugins.runtime, @@ -1554,19 +2027,22 @@ describe("reactions", () => { }; const mockSCM = createMockSCM({ - getReviewThreads: vi.fn().mockResolvedValue({ threads: [ - { - id: "bot-1", - author: "cursor[bot]", - body: "Potential issue detected", - path: "src/worker.ts", - line: 9, - isResolved: false, - createdAt: new Date(), - url: "https://example.com/comment/3", - isBot: true, - }, - ], reviews: [] }), + getReviewThreads: vi.fn().mockResolvedValue({ + threads: [ + { + id: "bot-1", + author: "cursor[bot]", + body: "Potential issue detected", + path: "src/worker.ts", + line: 9, + isResolved: false, + createdAt: new Date(), + url: "https://example.com/comment/3", + isBot: true, + }, + ], + reviews: [], + }), }); const registry = createMockRegistry({ runtime: plugins.runtime, @@ -1609,19 +2085,22 @@ describe("reactions", () => { }; const mockSCM = createMockSCM({ - getReviewThreads: vi.fn().mockResolvedValue({ threads: [ - { - id: "bot-1", - author: "cursor[bot]", - body: "Potential issue detected", - path: "src/worker.ts", - line: 9, - isResolved: false, - createdAt: new Date(), - url: "https://example.com/comment/3", - isBot: true, - }, - ], reviews: [] }), + getReviewThreads: vi.fn().mockResolvedValue({ + threads: [ + { + id: "bot-1", + author: "cursor[bot]", + body: "Potential issue detected", + path: "src/worker.ts", + line: 9, + isResolved: false, + createdAt: new Date(), + url: "https://example.com/comment/3", + isBot: true, + }, + ], + reviews: [], + }), }); const registry = createMockRegistry({ runtime: plugins.runtime, @@ -2293,19 +2772,25 @@ describe("pollAll terminal status accounting", () => { await vi.advanceTimersByTimeAsync(0); // all-complete should NOT have fired — "working" is still active - const allCompleteNotifications = vi.mocked(notifier.notify).mock.calls.filter( - (call: unknown[]) => { + const allCompleteNotifications = vi + .mocked(notifier.notify) + .mock.calls.filter((call: unknown[]) => { const event = call[0] as Record | undefined; const data = event?.data as Record | undefined; return event?.type === "reaction.triggered" && data?.reactionKey === "all-complete"; - }, - ); + }); expect(allCompleteNotifications).toHaveLength(0); lm.stop(); }); it("skips polling sessions in terminal statuses like done or errored", async () => { + const isolatedPlugins = createMockPlugins(); + const isolatedRegistry = createMockRegistry({ + runtime: isolatedPlugins.runtime, + agent: isolatedPlugins.agent, + }); + // Sessions in "done" / "errored" should not be polled const sessions = [ makeSession({ id: "s-done", status: "done" as SessionStatus }), @@ -2316,11 +2801,11 @@ describe("pollAll terminal status accounting", () => { // If these sessions were polled, determineStatus would call runtime.isAlive. // Reset call count and verify it's not called. - vi.mocked(plugins.runtime.isAlive).mockClear(); + vi.mocked(isolatedPlugins.runtime.isAlive).mockClear(); const lm = createLifecycleManager({ config, - registry: mockRegistry, + registry: isolatedRegistry, sessionManager: mockSessionManager, }); @@ -2328,7 +2813,7 @@ describe("pollAll terminal status accounting", () => { await vi.advanceTimersByTimeAsync(0); // Terminal sessions should not be polled — runtime.isAlive should not be called - expect(plugins.runtime.isAlive).not.toHaveBeenCalled(); + expect(isolatedPlugins.runtime.isAlive).not.toHaveBeenCalled(); lm.stop(); }); @@ -2401,7 +2886,7 @@ describe("rate limiting optimizations", () => { scm: mockSCM, }); - const session = makeSession({ id: "s-1", status: "pr_open", pr }); + const session = makeSession({ id: "s-1", status: "pr_open", pr, workspacePath: null }); vi.mocked(mockSessionManager.list).mockResolvedValue([session]); vi.mocked(mockSessionManager.send).mockResolvedValue(undefined); @@ -2416,6 +2901,77 @@ describe("rate limiting optimizations", () => { expect(mockSessionManager.send).toHaveBeenCalledWith("s-1", "Resolve conflicts."); }); + it("skips getCIChecks() when batch enrichment has ciChecks data", async () => { + config.reactions = { + "ci-failed": { + auto: true, + action: "send-to-agent", + message: "CI failing.", + retries: 3, + escalateAfter: 3, + }, + }; + + const pr = makeMatchingPR(); + const getCIChecksMock = vi.fn(); + const mockSCM = createMockSCM({ + getCIChecks: getCIChecksMock, + getCISummary: vi.fn().mockResolvedValue("failing"), + enrichSessionsPRBatch: vi.fn().mockResolvedValue( + new Map([ + [ + `${pr.owner}/${pr.repo}#${pr.number}`, + { + state: "open" as const, + ciStatus: "failing" as const, + reviewDecision: "none" as const, + mergeable: false, + hasConflicts: false, + ciChecks: [ + { + name: "lint", + status: "failed" as const, + conclusion: "FAILURE", + url: "https://example.com/lint", + }, + { name: "test", status: "passed" as const, conclusion: "SUCCESS" }, + ], + }, + ], + ]), + ), + }); + + const registry = createMockRegistry({ + runtime: plugins.runtime, + agent: plugins.agent, + scm: mockSCM, + }); + + // Start with pr_open state so that ci_failed transition happens on first poll + const session = makeSession({ id: "s-2", status: "pr_open", pr, workspacePath: null }); + vi.mocked(mockSessionManager.list).mockResolvedValue([session]); + vi.mocked(mockSessionManager.send).mockResolvedValue(undefined); + + const lm = createLifecycleManager({ config, registry, sessionManager: mockSessionManager }); + lm.start(60_000); + // First poll: transitions to ci_failed and sends the enriched reaction message. + await vi.advanceTimersByTimeAsync(0); + + // getCIChecks() should NOT be called — batch enrichment has ciChecks + expect(getCIChecksMock).not.toHaveBeenCalled(); + // Detailed message with lint check name/URL should be sent + const calls = vi.mocked(mockSessionManager.send).mock.calls; + const sentMessages = calls.map((c) => c[1] as string); + const detailMessage = sentMessages.find((m) => m.includes("lint")); + expect(detailMessage).toBeDefined(); + expect(detailMessage).toContain("https://example.com/lint"); + // Passing check should not be included + expect(detailMessage).not.toContain("test"); + + lm.stop(); + }); + it("throttles review backlog API calls to at most once per 2 minutes", async () => { config.reactions = { "changes-requested": { @@ -2425,19 +2981,22 @@ describe("rate limiting optimizations", () => { }, }; - const getReviewThreadsMock = vi.fn().mockResolvedValue({ threads: [ - { - id: "c1", - author: "reviewer", - body: "Please fix this", - path: "src/index.ts", - line: 10, - isResolved: false, - createdAt: new Date(), - url: "https://example.com/comment/1", - isBot: false, - }, - ], reviews: [] }); + const getReviewThreadsMock = vi.fn().mockResolvedValue({ + threads: [ + { + id: "c1", + author: "reviewer", + body: "Please fix this", + path: "src/index.ts", + line: 10, + isResolved: false, + createdAt: new Date(), + url: "https://example.com/comment/1", + isBot: false, + }, + ], + reviews: [], + }); const mockSCM = createMockSCM({ getReviewThreads: getReviewThreadsMock, }); diff --git a/packages/core/src/__tests__/plugin-registry.test.ts b/packages/core/src/__tests__/plugin-registry.test.ts index 6cb9168dc..690a7bbeb 100644 --- a/packages/core/src/__tests__/plugin-registry.test.ts +++ b/packages/core/src/__tests__/plugin-registry.test.ts @@ -139,9 +139,11 @@ describe("list", () => { describe("loadBuiltins", () => { it("silently skips unavailable packages", async () => { const registry = createPluginRegistry(); - // loadBuiltins tries to import all built-in packages. - // In the test environment, most are not resolvable — should not throw. - await expect(registry.loadBuiltins()).resolves.toBeUndefined(); + const importUnavailable = async (pkg: string): Promise => { + throw new Error(`Not found: ${pkg}`); + }; + + await expect(registry.loadBuiltins(undefined, importUnavailable)).resolves.toBeUndefined(); }); it("registers multiple agent plugins from importFn", async () => { @@ -263,7 +265,9 @@ describe("loadBuiltins", () => { }); expect(fakeSlackNotifier.create).toHaveBeenCalledTimes(2); - expect(registry.get<{ _config: Record }>("notifier", "alerts")?._config).toEqual({ + expect( + registry.get<{ _config: Record }>("notifier", "alerts")?._config, + ).toEqual({ webhookUrl: "https://hooks.slack.com/services/alerts", channel: "#alerts", configPath: "/test/config.yaml", @@ -304,12 +308,16 @@ describe("loadBuiltins", () => { throw new Error(`Not found: ${pkg}`); }); - expect(registry.get<{ _config: Record }>("notifier", "slack")?._config).toEqual({ + expect( + registry.get<{ _config: Record }>("notifier", "slack")?._config, + ).toEqual({ webhookUrl: "https://hooks.slack.com/services/default", channel: "#general", configPath: "/test/config.yaml", }); - expect(registry.get<{ _config: Record }>("notifier", "alerts")?._config).toEqual({ + expect( + registry.get<{ _config: Record }>("notifier", "alerts")?._config, + ).toEqual({ webhookUrl: "https://hooks.slack.com/services/alerts", channel: "#alerts", configPath: "/test/config.yaml", @@ -390,7 +398,9 @@ describe("loadBuiltins", () => { return null; }); - expect(stderrSpy).toHaveBeenCalledWith(expect.stringContaining('"path" field conflicts with reserved')); + expect(stderrSpy).toHaveBeenCalledWith( + expect.stringContaining('"path" field conflicts with reserved'), + ); stderrSpy.mockRestore(); // Plugin should not be registered due to config error @@ -482,10 +492,11 @@ describe("loadFromConfig", () => { it("does not throw when no plugins are importable", async () => { const registry = createPluginRegistry(); const config = makeOrchestratorConfig({}); + const importUnavailable = async (pkg: string): Promise => { + throw new Error(`Not found: ${pkg}`); + }; - // loadFromConfig calls loadBuiltins internally, which may fail to - // import packages in the test env — should still succeed gracefully - await expect(registry.loadFromConfig(config)).resolves.toBeUndefined(); + await expect(registry.loadFromConfig(config, importUnavailable)).resolves.toBeUndefined(); }); it("should pass importFn through loadFromConfig to loadBuiltins", async () => { @@ -596,7 +607,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "jira", slot: "tracker" as const, version: "1.0.0", description: "Jira tracker" }, + manifest: { + name: "jira", + slot: "tracker" as const, + version: "1.0.0", + description: "Jira tracker", + }, create: vi.fn(() => ({})), }; @@ -637,7 +653,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "jira-enterprise", slot: "tracker" as const, version: "1.0.0", description: "Jira Enterprise" }, + manifest: { + name: "jira-enterprise", + slot: "tracker" as const, + version: "1.0.0", + description: "Jira Enterprise", + }, create: vi.fn(() => ({})), }; @@ -687,7 +708,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "jira", slot: "tracker" as const, version: "1.0.0", description: "Jira tracker" }, + manifest: { + name: "jira", + slot: "tracker" as const, + version: "1.0.0", + description: "Jira tracker", + }, create: vi.fn(() => ({})), }; @@ -732,7 +758,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "ms-teams", slot: "notifier" as const, version: "1.0.0", description: "Teams notifier" }, + manifest: { + name: "ms-teams", + slot: "notifier" as const, + version: "1.0.0", + description: "Teams notifier", + }, create: vi.fn(() => ({})), }; @@ -778,7 +809,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "ms-teams", slot: "notifier" as const, version: "1.0.0", description: "Teams notifier" }, + manifest: { + name: "ms-teams", + slot: "notifier" as const, + version: "1.0.0", + description: "Teams notifier", + }, create: vi.fn(() => ({})), }; @@ -838,7 +874,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "ms-teams", slot: "notifier" as const, version: "1.0.0", description: "Teams notifier" }, + manifest: { + name: "ms-teams", + slot: "notifier" as const, + version: "1.0.0", + description: "Teams notifier", + }, create: vi.fn((pluginConfig?: Record) => ({ name: "ms-teams", _config: pluginConfig, @@ -885,7 +926,9 @@ describe("External plugin manifest validation", () => { expect(config.notifiers?.alerts?.plugin).toBe("ms-teams"); expect(config.notifiers?.ops?.plugin).toBe("ms-teams"); - expect(registry.get<{ _config: Record }>("notifier", "alerts")?._config).toEqual({ + expect( + registry.get<{ _config: Record }>("notifier", "alerts")?._config, + ).toEqual({ webhookUrl: "https://teams.example/alerts", configPath: "/test/config.yaml", }); @@ -900,7 +943,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "jira", slot: "notifier" as const, version: "1.0.0", description: "Wrong slot!" }, + manifest: { + name: "jira", + slot: "notifier" as const, + version: "1.0.0", + description: "Wrong slot!", + }, create: vi.fn(() => ({})), }; @@ -935,7 +983,7 @@ describe("External plugin manifest validation", () => { await registry.loadFromConfig(config, importFn); expect(stderrSpy).toHaveBeenCalledWith( - expect.stringContaining("has slot \"notifier\" but was configured as \"tracker\""), + expect.stringContaining('has slot "notifier" but was configured as "tracker"'), ); stderrSpy.mockRestore(); }); @@ -944,7 +992,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "jira-cloud", slot: "tracker" as const, version: "1.0.0", description: "Jira Cloud" }, + manifest: { + name: "jira-cloud", + slot: "tracker" as const, + version: "1.0.0", + description: "Jira Cloud", + }, create: vi.fn(() => ({})), }; @@ -1005,7 +1058,12 @@ describe("External plugin manifest validation", () => { const registry = createPluginRegistry(); const mockPlugin = { - manifest: { name: "jira-cloud", slot: "tracker" as const, version: "1.0.0", description: "Jira Cloud" }, + manifest: { + name: "jira-cloud", + slot: "tracker" as const, + version: "1.0.0", + description: "Jira Cloud", + }, create: vi.fn(() => ({})), }; diff --git a/packages/core/src/__tests__/test-utils.ts b/packages/core/src/__tests__/test-utils.ts index f3fb79dfd..bc3a2a8a1 100644 --- a/packages/core/src/__tests__/test-utils.ts +++ b/packages/core/src/__tests__/test-utils.ts @@ -43,11 +43,18 @@ export function makeSession(overrides: Partial = {}): Session { lifecycle.session.startedAt = lifecycle.session.lastTransitionAt; break; case "stuck": - case "errored": lifecycle.session.state = "stuck"; - lifecycle.session.reason = requestedStatus === "errored" ? "error_in_process" : "probe_failure"; + lifecycle.session.reason = "probe_failure"; lifecycle.session.startedAt = lifecycle.session.lastTransitionAt; break; + case "errored": + lifecycle.session.state = "terminated"; + lifecycle.session.reason = "error_in_process"; + lifecycle.session.startedAt = lifecycle.session.lastTransitionAt; + lifecycle.session.terminatedAt = lifecycle.session.lastTransitionAt; + lifecycle.runtime.state = "missing"; + lifecycle.runtime.reason = "process_missing"; + break; case "merged": lifecycle.session.state = "idle"; lifecycle.session.reason = "merged_waiting_decision"; @@ -231,7 +238,10 @@ export interface RegistryPlugins { notifier?: Notifier; } -export function createMockRegistry(plugins: RegistryPlugins, options: { strict?: boolean } = {}): PluginRegistry { +export function createMockRegistry( + plugins: RegistryPlugins, + options: { strict?: boolean } = {}, +): PluginRegistry { return { register: vi.fn(), get: vi.fn().mockImplementation((slot: string, name?: string) => { @@ -257,8 +267,6 @@ export function createMockRegistry(plugins: RegistryPlugins, options: { strict?: if (!options.strict) return plugins.notifier; } - - return null; }), list: vi.fn().mockReturnValue([]), @@ -370,7 +378,11 @@ export function setupTestContext(): TestContext { const storageKey = createHash("sha256").update(join(tmpDir, "my-app")).digest("hex").slice(0, 12); const { runtime: mockRuntime, agent: mockAgent, workspace: mockWorkspace } = createMockPlugins(); - const mockRegistry = createMockRegistry({ runtime: mockRuntime, agent: mockAgent, workspace: mockWorkspace }); + const mockRegistry = createMockRegistry({ + runtime: mockRuntime, + agent: mockAgent, + workspace: mockWorkspace, + }); const config: OrchestratorConfig = { configPath, @@ -444,8 +456,16 @@ export function teardownTestContext(ctx: TestContext): void { export function createMockSessionManager(): OpenCodeSessionManager { return { spawn: vi.fn().mockResolvedValue(makeSession()), - spawnOrchestrator: vi.fn().mockResolvedValue(makeSession({ id: "app-orchestrator", metadata: { role: "orchestrator" } })), - ensureOrchestrator: vi.fn().mockResolvedValue(makeSession({ id: "app-orchestrator", metadata: { role: "orchestrator" } })), + spawnOrchestrator: vi + .fn() + .mockResolvedValue( + makeSession({ id: "app-orchestrator", metadata: { role: "orchestrator" } }), + ), + ensureOrchestrator: vi + .fn() + .mockResolvedValue( + makeSession({ id: "app-orchestrator", metadata: { role: "orchestrator" } }), + ), restore: vi.fn().mockResolvedValue(makeSession()), list: vi.fn().mockResolvedValue([]), listCached: vi.fn().mockResolvedValue([]), diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts index b42fda5a2..0731f2e39 100644 --- a/packages/core/src/lifecycle-manager.ts +++ b/packages/core/src/lifecycle-manager.ts @@ -11,6 +11,8 @@ */ import { randomUUID } from "node:crypto"; +import { readFile, stat } from "node:fs/promises"; +import { dirname, join, resolve } from "node:path"; import { ACTIVITY_STATE, SESSION_STATUS, @@ -38,7 +40,11 @@ import { type ReviewComment, type ReviewSummary, } from "./types.js"; -import { buildLifecycleMetadataPatch, cloneLifecycle, deriveLegacyStatus } from "./lifecycle-state.js"; +import { + buildLifecycleMetadataPatch, + cloneLifecycle, + deriveLegacyStatus, +} from "./lifecycle-state.js"; import { updateMetadata } from "./metadata.js"; import { getSessionsDir } from "./paths.js"; import { applyDecisionToLifecycle as commitLifecycleDecisionInPlace } from "./lifecycle-transition.js"; @@ -49,11 +55,7 @@ import { hasPositiveIdleEvidence, isWeakActivityEvidence, } from "./activity-signal.js"; -import { - isAgentReportFresh, - mapAgentReportToLifecycle, - readAgentReport, -} from "./agent-report.js"; +import { isAgentReportFresh, mapAgentReportToLifecycle, readAgentReport } from "./agent-report.js"; import { auditAgentReports, getReactionKeyForTrigger, @@ -90,6 +92,84 @@ function parseDuration(str: string): number { } } +type WorkspaceBranchProbe = + | { kind: "branch"; branch: string } + | { kind: "detached" } + | { kind: "unavailable" }; + +const TRANSIENT_DETACHED_GIT_MARKERS = [ + "rebase-merge", + "rebase-apply", + "CHERRY_PICK_HEAD", + "BISECT_LOG", +] as const; + +function isErrnoException(error: unknown): error is NodeJS.ErrnoException { + return typeof error === "object" && error !== null && "code" in error; +} + +async function pathExists(path: string): Promise { + try { + await stat(path); + return true; + } catch (error) { + if (isErrnoException(error) && error.code === "ENOENT") { + return false; + } + throw error; + } +} + +async function hasTransientDetachedGitState(gitDir: string): Promise { + const checks = await Promise.all( + TRANSIENT_DETACHED_GIT_MARKERS.map((marker) => pathExists(join(gitDir, marker))), + ); + return checks.some(Boolean); +} + +async function resolveGitDir(workspacePath: string): Promise { + const dotGitPath = join(workspacePath, ".git"); + const dotGitStats = await stat(dotGitPath); + if (dotGitStats.isDirectory()) return dotGitPath; + + const dotGitContent = (await readFile(dotGitPath, "utf8")).trim(); + const gitDirMatch = dotGitContent.match(/^gitdir:\s*(.+)$/i); + if (!gitDirMatch) { + throw new Error(`Invalid .git pointer in workspace: ${workspacePath}`); + } + + return resolve(dirname(dotGitPath), gitDirMatch[1].trim()); +} + +async function readWorkspaceBranch(workspacePath: string): Promise { + let gitDir: string; + try { + gitDir = await resolveGitDir(workspacePath); + } catch { + return { kind: "unavailable" }; + } + + try { + const head = (await readFile(join(gitDir, "HEAD"), "utf8")).trim(); + const prefix = "ref: refs/heads/"; + if (!head.startsWith(prefix)) { + return (await hasTransientDetachedGitState(gitDir)) + ? { kind: "unavailable" } + : { kind: "detached" }; + } + + const branch = head.slice(prefix.length).trim(); + if (branch.length > 0) { + return { kind: "branch", branch }; + } + return (await hasTransientDetachedGitState(gitDir)) + ? { kind: "unavailable" } + : { kind: "detached" }; + } catch { + return { kind: "unavailable" }; + } +} + /** Infer a reasonable priority from event type. */ function inferPriority(type: EventType): EventPriority { if (type.includes("stuck") || type.includes("needs_input") || type.includes("errored")) { @@ -317,6 +397,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan let pollTimer: ReturnType | null = null; let polling = false; // re-entrancy guard let allCompleteEmitted = false; // guard against repeated all_complete + const branchAdoptionReservations = new Map(); /** * Cache for PR enrichment data within a single poll cycle. @@ -469,10 +550,16 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan // Only run detectPR when Guard 1 returned 200 (repo's PR list changed). // When Guard 1 returned 304, the repo is in prListUnchangedRepos — no new PRs exist. for (const session of sessions) { - if (session.pr) continue; if (!session.branch) continue; if (session.metadata["prAutoDetect"] === "off") continue; - if (session.metadata["role"] === "orchestrator" || session.id.endsWith("-orchestrator")) continue; + if (session.metadata["role"] === "orchestrator" || session.id.endsWith("-orchestrator")) + continue; + if ( + session.pr && + !(session.lifecycle.pr.state === "closed" && session.pr.branch !== session.branch) + ) { + continue; + } const project = config.projects[session.projectId]; if (!project?.repo || !project.scm?.plugin) continue; @@ -558,6 +645,103 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan return idleMs > stuckThresholdMs; } + function isBranchOwnedByAnotherActiveWorker( + session: Session, + branch: string, + siblingSessions: Session[], + allSessionPrefixes: string[], + ): boolean { + return siblingSessions.some((other) => { + if (other.id === session.id) return false; + if (other.projectId !== session.projectId) return false; + if (TERMINAL_STATUSES.has(other.status)) return false; + + const otherProject = config.projects[other.projectId]; + if (!otherProject) return false; + + const otherRole = resolveSessionRole( + other.id, + other.metadata, + otherProject.sessionPrefix, + allSessionPrefixes, + ); + return otherRole === "worker" && other.branch === branch; + }); + } + + function acquireBranchAdoptionReservation(session: Session, branch: string): string | null { + const reservationKey = `${session.projectId}:${branch}`; + const existingOwner = branchAdoptionReservations.get(reservationKey); + if (existingOwner && existingOwner !== session.id) { + return null; + } + branchAdoptionReservations.set(reservationKey, session.id); + return reservationKey; + } + + function releaseBranchAdoptionReservation(reservationKey: string, sessionId: SessionId): void { + if (branchAdoptionReservations.get(reservationKey) === sessionId) { + branchAdoptionReservations.delete(reservationKey); + } + } + + async function refreshTrackedBranch( + session: Session, + siblingSessions?: Session[], + ): Promise { + const project = config.projects[session.projectId]; + if (!project) return; + + const allSessionPrefixes = Object.values(config.projects).map((p) => p.sessionPrefix); + const sessionRole = resolveSessionRole( + session.id, + session.metadata, + project.sessionPrefix, + allSessionPrefixes, + ); + const workspacePath = session.workspacePath; + const canRefreshTrackedBranch = + sessionRole === "worker" && + workspacePath !== null && + (!session.pr || session.lifecycle.pr.state === "closed"); + + if (!canRefreshTrackedBranch) return; + + const branchProbe = await readWorkspaceBranch(workspacePath); + if (branchProbe.kind === "detached") { + if (session.branch !== null) { + session.branch = null; + updateSessionMetadata(session, { branch: "" }); + } + return; + } + + if (branchProbe.kind !== "branch" || branchProbe.branch === session.branch) { + return; + } + + const reservationKey = acquireBranchAdoptionReservation(session, branchProbe.branch); + if (!reservationKey) return; + + try { + const sessionsForConflictCheck = + siblingSessions ?? (await sessionManager.list(session.projectId)); + if ( + !isBranchOwnedByAnotherActiveWorker( + session, + branchProbe.branch, + sessionsForConflictCheck, + allSessionPrefixes, + ) + ) { + session.branch = branchProbe.branch; + updateSessionMetadata(session, { branch: branchProbe.branch }); + } + } finally { + releaseBranchAdoptionReservation(reservationKey, session.id); + } + } + /** Determine current status for a session by polling plugins. */ async function determineStatus(session: Session): Promise { const project = config.projects[session.projectId]; @@ -571,20 +755,21 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan const lifecycle = cloneLifecycle(session.lifecycle); const nowIso = new Date().toISOString(); + const allSessionPrefixes = Object.values(config.projects).map((p) => p.sessionPrefix); + const sessionRole = resolveSessionRole( + session.id, + session.metadata, + project.sessionPrefix, + allSessionPrefixes, + ); const agentName = resolveAgentSelection({ - role: resolveSessionRole( - session.id, - session.metadata, - project.sessionPrefix, - Object.values(config.projects).map((p) => p.sessionPrefix), - ), + role: sessionRole, project, defaults: config.defaults, persistedAgent: session.metadata["agent"], }).agentName; const agent = registry.get("agent", agentName); const scm = project.scm?.plugin ? registry.get("scm", project.scm.plugin) : null; - let detectedIdleTimestamp: Date | null = null; let idleWasBlocked = false; const canProbeRuntimeIdentity = session.status !== SESSION_STATUS.SPAWNING; @@ -650,8 +835,13 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan canProbeRuntimeIdentity ) { try { - const runtime = registry.get("runtime", project.runtime ?? config.defaults.runtime); - const terminalOutput = runtime ? await runtime.getOutput(session.runtimeHandle, 10) : ""; + const runtime = registry.get( + "runtime", + project.runtime ?? config.defaults.runtime, + ); + const terminalOutput = runtime + ? await runtime.getOutput(session.runtimeHandle, 10) + : ""; if (terminalOutput) { await agent.recordActivity(session, terminalOutput); } @@ -700,7 +890,10 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan } else if (session.runtimeHandle && canProbeRuntimeIdentity) { activitySignal = createActivitySignal("null", { source: "native" }); activityEvidence = formatActivitySignalEvidence(activitySignal); - const runtime = registry.get("runtime", project.runtime ?? config.defaults.runtime); + const runtime = registry.get( + "runtime", + project.runtime ?? config.defaults.runtime, + ); const terminalOutput = runtime ? await runtime.getOutput(session.runtimeHandle, 10) : ""; if (terminalOutput) { const activity = agent.detectActivity(terminalOutput); @@ -1100,10 +1293,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan return reactionConfig ? (reactionConfig as ReactionConfig) : null; } - function updateSessionMetadata( - session: Session, - updates: Partial>, - ): void { + function updateSessionMetadata(session: Session, updates: Partial>): void { const project = config.projects[session.projectId]; if (!project) return; @@ -1225,12 +1415,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan } } - const pendingComments = allThreads - ? allThreads.filter((c) => !c.isBot) - : null; - const automatedComments = allThreads - ? allThreads.filter((c) => c.isBot) - : null; + const pendingComments = allThreads ? allThreads.filter((c) => !c.isBot) : null; + const automatedComments = allThreads ? allThreads.filter((c) => c.isBot) : null; // --- Pending (human) review comments --- // null = SCM fetch failed; skip processing to preserve existing metadata. @@ -1300,12 +1486,9 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan // --- Automated (bot) review comments --- if (automatedComments !== null) { - const automatedFingerprint = makeFingerprint( - automatedComments.map((comment) => comment.id), - ); + const automatedFingerprint = makeFingerprint(automatedComments.map((comment) => comment.id)); const lastAutomatedFingerprint = session.metadata["lastAutomatedReviewFingerprint"] ?? ""; - const lastAutomatedDispatchHash = - session.metadata["lastAutomatedReviewDispatchHash"] ?? ""; + const lastAutomatedDispatchHash = session.metadata["lastAutomatedReviewDispatchHash"] ?? ""; if (automatedFingerprint !== lastAutomatedFingerprint) { clearReactionTracker(session.id, automatedReactionKey); @@ -1382,7 +1565,10 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan if (c.url) lines.push(` ${c.url}`); if (c.threadId) lines.push(` Thread ID: ${c.threadId}`); } - lines.push("", "Address each comment, push fixes. Use the thread ID to resolve each thread directly after pushing. You should not need to re-fetch review data unless you need additional context beyond what is provided here."); + lines.push( + "", + "Address each comment, push fixes. Use the thread ID to resolve each thread directly after pushing. You should not need to re-fetch review data unless you need additional context beyond what is provided here.", + ); return lines.join("\n"); } @@ -1391,19 +1577,13 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan * Includes check names, statuses, and links for debugging. */ function formatCIFailureMessage(failedChecks: CICheck[]): string { - const lines = [ - "CI checks are failing on your PR. Here are the failed checks:", - "", - ]; + const lines = ["CI checks are failing on your PR. Here are the failed checks:", ""]; for (const check of failedChecks) { const status = check.conclusion ?? check.status; const link = check.url ? ` — ${check.url}` : ""; lines.push(`- **${check.name}**: ${status}${link}`); } - lines.push( - "", - "Investigate the failures, fix the issues, and push again.", - ); + lines.push("", "Investigate the failures, fix the issues, and push again."); return lines.join("\n"); } @@ -1642,7 +1822,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan (assessment.detectingAttempts > DETECTING_MAX_ATTEMPTS || isDetectingTimedOut(nextDetectingStartedAt)); const nextDetectingEscalatedAt = isDetectingEscalated - ? (session.metadata["detectingEscalatedAt"] || new Date().toISOString()) + ? session.metadata["detectingEscalatedAt"] || new Date().toISOString() : ""; const metadataUpdates: Record = {}; @@ -1720,7 +1900,10 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan if (cachedData?.ciChecks) { const failedChecks = cachedData.ciChecks.filter((c) => c.status === "failed"); if (failedChecks.length > 0) { - reactionConfig = { ...reactionConfig, message: formatCIFailureMessage(failedChecks) }; + reactionConfig = { + ...reactionConfig, + message: formatCIFailureMessage(failedChecks), + }; } } } @@ -1955,6 +2138,10 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan return tracked !== undefined && tracked !== s.status; }); + await Promise.allSettled( + sessionsToCheck.map((session) => refreshTrackedBranch(session, sessions)), + ); + // Prime the per-poll PR enrichment cache before session checks so // downstream status/reaction logic can reuse batch GraphQL data. await populatePREnrichmentCache(sessionsToCheck); @@ -2072,6 +2259,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan async check(sessionId: SessionId): Promise { const session = await sessionManager.get(sessionId); if (!session) throw new Error(`Session ${sessionId} not found`); + await refreshTrackedBranch(session); // Populate batch enrichment cache for this session's PR so // checkSession can read from cache (no individual REST fallback). await populatePREnrichmentCache([session]);