fix: refresh worker branch metadata after branch changes (#1368)
* fix: refresh worker branch metadata * test: make plugin registry imports deterministic * fix: make lifecycle branch probe async * fix: clear stale worker branch metadata * fix: harden branch refresh probing * test: fix terminal status lifecycle fixtures * fix: harden worker branch refresh
This commit is contained in:
yyovil
GitHub
parent
dcfb6fe8fd
commit
62353eb5a1
|
|
@ -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.
|
||||
|
|
@ -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<string, unknown> | undefined;
|
||||
const data = event?.data as Record<string, unknown> | 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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<unknown> => {
|
||||
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<string, unknown> }>("notifier", "alerts")?._config).toEqual({
|
||||
expect(
|
||||
registry.get<{ _config: Record<string, unknown> }>("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<string, unknown> }>("notifier", "slack")?._config).toEqual({
|
||||
expect(
|
||||
registry.get<{ _config: Record<string, unknown> }>("notifier", "slack")?._config,
|
||||
).toEqual({
|
||||
webhookUrl: "https://hooks.slack.com/services/default",
|
||||
channel: "#general",
|
||||
configPath: "/test/config.yaml",
|
||||
});
|
||||
expect(registry.get<{ _config: Record<string, unknown> }>("notifier", "alerts")?._config).toEqual({
|
||||
expect(
|
||||
registry.get<{ _config: Record<string, unknown> }>("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<unknown> => {
|
||||
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<string, unknown>) => ({
|
||||
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<string, unknown> }>("notifier", "alerts")?._config).toEqual({
|
||||
expect(
|
||||
registry.get<{ _config: Record<string, unknown> }>("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(() => ({})),
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -43,11 +43,18 @@ export function makeSession(overrides: Partial<Session> = {}): 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([]),
|
||||
|
|
|
|||
|
|
@ -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<boolean> {
|
||||
try {
|
||||
await stat(path);
|
||||
return true;
|
||||
} catch (error) {
|
||||
if (isErrnoException(error) && error.code === "ENOENT") {
|
||||
return false;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
async function hasTransientDetachedGitState(gitDir: string): Promise<boolean> {
|
||||
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<string> {
|
||||
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<WorkspaceBranchProbe> {
|
||||
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<typeof setInterval> | null = null;
|
||||
let polling = false; // re-entrancy guard
|
||||
let allCompleteEmitted = false; // guard against repeated all_complete
|
||||
const branchAdoptionReservations = new Map<string, SessionId>();
|
||||
|
||||
/**
|
||||
* 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<void> {
|
||||
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<DeterminedStatus> {
|
||||
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>("agent", agentName);
|
||||
const scm = project.scm?.plugin ? registry.get<SCM>("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>("runtime", project.runtime ?? config.defaults.runtime);
|
||||
const terminalOutput = runtime ? await runtime.getOutput(session.runtimeHandle, 10) : "";
|
||||
const runtime = registry.get<Runtime>(
|
||||
"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>("runtime", project.runtime ?? config.defaults.runtime);
|
||||
const runtime = registry.get<Runtime>(
|
||||
"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<Record<string, string>>,
|
||||
): void {
|
||||
function updateSessionMetadata(session: Session, updates: Partial<Record<string, string>>): 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<string, string> = {};
|
||||
|
|
@ -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<void> {
|
||||
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]);
|
||||
|
|
|
|||
Loading…
Reference in New Issue