Merge pull request #118 from harshitsinghbhandari/stage4-pr-policy
fix: decouple PR truth from worker session policy
This commit is contained in:
commit
ffdfd869ff
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
"@aoagents/ao-core": patch
|
||||
"@aoagents/ao-web": patch
|
||||
---
|
||||
|
||||
Decouple canonical session state from PR state so workers stay idle while waiting on reviews or merged/closed PR decisions, stop cleanup from auto-killing merged PR sessions, and make the dashboard/rendered labels follow canonical PR truth instead of inferring it from legacy lifecycle aliases.
|
||||
|
|
@ -375,7 +375,7 @@ describe("check (single session)", () => {
|
|||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
});
|
||||
vi.mocked(registryWithoutAgent.get).mockImplementation((slot: string, name?: string) => {
|
||||
vi.mocked(registryWithoutAgent.get).mockImplementation((slot: string, _name?: string) => {
|
||||
if (slot === "runtime") return plugins.runtime;
|
||||
if (slot === "agent") return null;
|
||||
return null;
|
||||
|
|
@ -584,6 +584,38 @@ describe("check (single session)", () => {
|
|||
expect(lm.getStates().get("app-1")).toBe("ci_failed");
|
||||
});
|
||||
|
||||
it("keeps canonical session state idle while waiting on external review", async () => {
|
||||
const mockSCM = createMockSCM({ getReviewDecision: vi.fn().mockResolvedValue("pending") });
|
||||
const registry = createMockRegistry({
|
||||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
scm: mockSCM,
|
||||
});
|
||||
const session = makeSession({ status: "pr_open", pr: makePR() });
|
||||
vi.mocked(mockSessionManager.get).mockResolvedValue(session);
|
||||
|
||||
writeMetadata(env.sessionsDir, "app-1", {
|
||||
worktree: "/tmp",
|
||||
branch: session.branch ?? "main",
|
||||
status: session.status,
|
||||
project: "my-app",
|
||||
pr: session.pr?.url,
|
||||
runtimeHandle: session.runtimeHandle ? JSON.stringify(session.runtimeHandle) : undefined,
|
||||
});
|
||||
|
||||
const lm = createLifecycleManager({
|
||||
config,
|
||||
registry,
|
||||
sessionManager: mockSessionManager,
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
|
||||
expect(lm.getStates().get("app-1")).toBe("review_pending");
|
||||
expect(session.lifecycle.session.state).toBe("idle");
|
||||
expect(session.lifecycle.session.reason).toBe("awaiting_external_review");
|
||||
});
|
||||
|
||||
it("skips PR auto-detection when metadata disables it", async () => {
|
||||
const mockSCM = createMockSCM({ detectPR: vi.fn().mockResolvedValue(makePR()) });
|
||||
const registry = createMockRegistry({
|
||||
|
|
@ -702,23 +734,77 @@ describe("check (single session)", () => {
|
|||
expect(lm.getStates().get("app-1")).toBe("merged");
|
||||
});
|
||||
|
||||
it("treats closed PRs as done when the canonical lifecycle marks them complete", async () => {
|
||||
it("keeps closed PR sessions idle and emits a PR-closed notification", async () => {
|
||||
const mockSCM = createMockSCM({ getPRState: vi.fn().mockResolvedValue("closed") });
|
||||
const notifier = createMockNotifier();
|
||||
const registry = createMockRegistry({
|
||||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
scm: mockSCM,
|
||||
notifier,
|
||||
});
|
||||
|
||||
const session = makeSession({ status: "pr_open", pr: makePR() });
|
||||
const lm = setupCheck("app-1", {
|
||||
session,
|
||||
registry,
|
||||
configOverride: {
|
||||
...config,
|
||||
notificationRouting: {
|
||||
...config.notificationRouting,
|
||||
info: ["desktop"],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
expect(lm.getStates().get("app-1")).toBe("idle");
|
||||
const meta = readMetadataRaw(env.sessionsDir, "app-1");
|
||||
expect(meta?.["status"]).toBe("idle");
|
||||
expect(meta?.["statePayload"]).toContain('"state":"closed"');
|
||||
expect(meta?.["statePayload"]).toContain('"reason":"pr_closed_waiting_decision"');
|
||||
expect(notifier.notify).toHaveBeenCalledWith(expect.objectContaining({ type: "pr.closed" }));
|
||||
});
|
||||
|
||||
it("routes closed PR transitions through the pr-closed reaction key", async () => {
|
||||
const notifier = createMockNotifier();
|
||||
const mockSCM = createMockSCM({ getPRState: vi.fn().mockResolvedValue("closed") });
|
||||
const registry = createMockRegistry({
|
||||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
scm: mockSCM,
|
||||
notifier,
|
||||
});
|
||||
|
||||
const session = makeSession({ status: "pr_open", pr: makePR() });
|
||||
const lm = setupCheck("app-1", {
|
||||
session: makeSession({ status: "pr_open", pr: makePR() }),
|
||||
session,
|
||||
registry,
|
||||
configOverride: {
|
||||
...config,
|
||||
reactions: {
|
||||
...config.reactions,
|
||||
"pr-closed": {
|
||||
auto: true,
|
||||
action: "notify",
|
||||
priority: "action",
|
||||
},
|
||||
},
|
||||
notificationRouting: {
|
||||
...config.notificationRouting,
|
||||
action: ["desktop"],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
expect(lm.getStates().get("app-1")).toBe("done");
|
||||
const meta = readMetadataRaw(env.sessionsDir, "app-1");
|
||||
expect(meta?.["status"]).toBe("done");
|
||||
|
||||
expect(notifier.notify).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: "reaction.triggered",
|
||||
data: expect.objectContaining({ reactionKey: "pr-closed" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("detects mergeable when approved + CI green", async () => {
|
||||
|
|
@ -1563,6 +1649,42 @@ describe("reactions", () => {
|
|||
expect(metadata?.["lastMergeConflictDispatched"]).toBeFalsy();
|
||||
});
|
||||
|
||||
it("clears merge conflict tracking when PR is closed", async () => {
|
||||
config.reactions = {
|
||||
"merge-conflicts": {
|
||||
auto: true,
|
||||
action: "send-to-agent",
|
||||
message: "Resolve merge conflicts.",
|
||||
},
|
||||
};
|
||||
|
||||
const getMergeability = vi.fn();
|
||||
const mockSCM = createMockSCM({
|
||||
getPRState: vi.fn().mockResolvedValue("closed"),
|
||||
getMergeability,
|
||||
});
|
||||
const registry = createMockRegistry({
|
||||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
scm: mockSCM,
|
||||
});
|
||||
|
||||
const lm = setupCheck("app-1", {
|
||||
session: makeSession({
|
||||
status: "pr_open",
|
||||
pr: makePR(),
|
||||
metadata: { lastMergeConflictDispatched: "true" },
|
||||
}),
|
||||
registry,
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
|
||||
const metadata = readMetadataRaw(env.sessionsDir, "app-1");
|
||||
expect(metadata?.["lastMergeConflictDispatched"]).toBeFalsy();
|
||||
expect(getMergeability).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("notifies humans on significant transitions without reaction config", async () => {
|
||||
const notifier = createMockNotifier();
|
||||
const mockSCM = createMockSCM({ getPRState: vi.fn().mockResolvedValue("merged") });
|
||||
|
|
@ -2068,6 +2190,49 @@ describe("rate limiting optimizations", () => {
|
|||
await lm.check("app-1");
|
||||
expect(getPendingMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("clears review backlog tracking when PR is closed", async () => {
|
||||
const getPendingMock = vi.fn();
|
||||
const getAutomatedMock = vi.fn();
|
||||
const mockSCM = createMockSCM({
|
||||
getPRState: vi.fn().mockResolvedValue("closed"),
|
||||
getPendingComments: getPendingMock,
|
||||
getAutomatedComments: getAutomatedMock,
|
||||
});
|
||||
const registry = createMockRegistry({
|
||||
runtime: plugins.runtime,
|
||||
agent: plugins.agent,
|
||||
scm: mockSCM,
|
||||
});
|
||||
|
||||
const lm = setupCheck("app-1", {
|
||||
session: makeSession({
|
||||
status: "pr_open",
|
||||
pr: makePR(),
|
||||
metadata: {
|
||||
lastPendingReviewFingerprint: "fingerprint",
|
||||
lastPendingReviewDispatchHash: "dispatch",
|
||||
lastPendingReviewDispatchAt: "2025-01-01T00:00:00.000Z",
|
||||
lastAutomatedReviewFingerprint: "auto-fingerprint",
|
||||
lastAutomatedReviewDispatchHash: "auto-dispatch",
|
||||
lastAutomatedReviewDispatchAt: "2025-01-01T00:00:00.000Z",
|
||||
},
|
||||
}),
|
||||
registry,
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
|
||||
const metadata = readMetadataRaw(env.sessionsDir, "app-1");
|
||||
expect(metadata?.["lastPendingReviewFingerprint"]).toBeFalsy();
|
||||
expect(metadata?.["lastPendingReviewDispatchHash"]).toBeFalsy();
|
||||
expect(metadata?.["lastPendingReviewDispatchAt"]).toBeFalsy();
|
||||
expect(metadata?.["lastAutomatedReviewFingerprint"]).toBeFalsy();
|
||||
expect(metadata?.["lastAutomatedReviewDispatchHash"]).toBeFalsy();
|
||||
expect(metadata?.["lastAutomatedReviewDispatchAt"]).toBeFalsy();
|
||||
expect(getPendingMock).not.toHaveBeenCalled();
|
||||
expect(getAutomatedMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
describe("summary pinning", () => {
|
||||
it("pins first quality summary when pinnedSummary not set", async () => {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,59 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { createInitialCanonicalLifecycle, deriveLegacyStatus } from "../lifecycle-state.js";
|
||||
|
||||
function createOpenPRLifecycle() {
|
||||
const lifecycle = createInitialCanonicalLifecycle("worker", new Date("2025-01-01T00:00:00Z"));
|
||||
lifecycle.session.startedAt = lifecycle.session.lastTransitionAt;
|
||||
lifecycle.pr.state = "open";
|
||||
lifecycle.pr.reason = "review_pending";
|
||||
lifecycle.pr.number = 42;
|
||||
lifecycle.pr.url = "https://github.com/org/repo/pull/42";
|
||||
lifecycle.pr.lastObservedAt = lifecycle.session.lastTransitionAt;
|
||||
lifecycle.runtime.state = "alive";
|
||||
lifecycle.runtime.reason = "process_running";
|
||||
return lifecycle;
|
||||
}
|
||||
|
||||
describe("deriveLegacyStatus", () => {
|
||||
it("preserves urgent session states ahead of open PR aliases", () => {
|
||||
const needsInput = createOpenPRLifecycle();
|
||||
needsInput.session.state = "needs_input";
|
||||
needsInput.session.reason = "awaiting_user_input";
|
||||
|
||||
const stuck = createOpenPRLifecycle();
|
||||
stuck.session.state = "stuck";
|
||||
stuck.session.reason = "probe_failure";
|
||||
|
||||
const terminated = createOpenPRLifecycle();
|
||||
terminated.session.state = "terminated";
|
||||
terminated.session.reason = "manually_killed";
|
||||
|
||||
expect(deriveLegacyStatus(needsInput)).toBe("needs_input");
|
||||
expect(deriveLegacyStatus(stuck)).toBe("stuck");
|
||||
expect(deriveLegacyStatus(terminated)).toBe("terminated");
|
||||
});
|
||||
|
||||
it("preserves prior terminal legacy statuses for terminated sessions", () => {
|
||||
const terminated = createOpenPRLifecycle();
|
||||
terminated.session.state = "terminated";
|
||||
terminated.session.reason = "manually_killed";
|
||||
|
||||
expect(deriveLegacyStatus(terminated, "killed")).toBe("killed");
|
||||
expect(deriveLegacyStatus(terminated, "cleanup")).toBe("cleanup");
|
||||
expect(deriveLegacyStatus(terminated, "errored")).toBe("errored");
|
||||
});
|
||||
|
||||
it("keeps PR-oriented aliases for idle workers with open PRs", () => {
|
||||
const reviewPending = createOpenPRLifecycle();
|
||||
reviewPending.session.state = "idle";
|
||||
reviewPending.session.reason = "awaiting_external_review";
|
||||
|
||||
const mergeReady = createOpenPRLifecycle();
|
||||
mergeReady.session.state = "idle";
|
||||
mergeReady.session.reason = "awaiting_external_review";
|
||||
mergeReady.pr.reason = "merge_ready";
|
||||
|
||||
expect(deriveLegacyStatus(reviewPending)).toBe("review_pending");
|
||||
expect(deriveLegacyStatus(mergeReady)).toBe("mergeable");
|
||||
});
|
||||
});
|
||||
|
|
@ -392,7 +392,7 @@ describe("plugin integration", () => {
|
|||
|
||||
// -------------------------------------------------------------------------
|
||||
describe("SessionManager + SCM", () => {
|
||||
it("cleanup() calls scm-github getPRState() and kills merged PR sessions", async () => {
|
||||
it("cleanup() calls scm-github getPRState() but keeps merged PR sessions alive", async () => {
|
||||
const registry = createTestRegistry();
|
||||
const sm = createSessionManager({ config, registry });
|
||||
|
||||
|
|
@ -412,7 +412,7 @@ describe("plugin integration", () => {
|
|||
|
||||
const result = await sm.cleanup("my-app");
|
||||
|
||||
expect(result.killed).toContain("app-1");
|
||||
expect(result.skipped).toContain("app-1");
|
||||
// Verify gh CLI was called for PR state check
|
||||
expect(ghMock).toHaveBeenCalledWith(
|
||||
"gh",
|
||||
|
|
|
|||
|
|
@ -229,11 +229,11 @@ describe("kill", () => {
|
|||
});
|
||||
|
||||
describe("cleanup", () => {
|
||||
it("kills sessions with merged PRs", async () => {
|
||||
it("kills sessions with closed PRs", async () => {
|
||||
const mockSCM: SCM = {
|
||||
name: "mock-scm",
|
||||
detectPR: vi.fn(),
|
||||
getPRState: vi.fn().mockResolvedValue("merged"),
|
||||
getPRState: vi.fn().mockResolvedValue("closed"),
|
||||
mergePR: vi.fn(),
|
||||
closePR: vi.fn(),
|
||||
getCIChecks: vi.fn(),
|
||||
|
|
@ -272,7 +272,7 @@ describe("cleanup", () => {
|
|||
expect(result.skipped).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("deletes mapped OpenCode session during cleanup", async () => {
|
||||
it("deletes mapped OpenCode session during cleanup for closed PRs", async () => {
|
||||
const deleteLogPath = join(tmpDir, "opencode-delete.log");
|
||||
const mockBin = installMockOpencode(tmpDir, "[]", deleteLogPath);
|
||||
process.env.PATH = `${mockBin}:${originalPath ?? ""}`;
|
||||
|
|
@ -280,7 +280,7 @@ describe("cleanup", () => {
|
|||
const mockSCM: SCM = {
|
||||
name: "mock-scm",
|
||||
detectPR: vi.fn(),
|
||||
getPRState: vi.fn().mockResolvedValue("merged"),
|
||||
getPRState: vi.fn().mockResolvedValue("closed"),
|
||||
mergePR: vi.fn(),
|
||||
closePR: vi.fn(),
|
||||
getCIChecks: vi.fn(),
|
||||
|
|
@ -322,14 +322,14 @@ describe("cleanup", () => {
|
|||
expect(deleteLog).toContain("session delete ses_cleanup");
|
||||
});
|
||||
|
||||
it("treats missing mapped OpenCode session as already cleaned", async () => {
|
||||
it("treats missing mapped OpenCode session as already cleaned for closed PRs", async () => {
|
||||
const mockBin = installMockOpencodeWithNotFoundDelete(tmpDir, "[]");
|
||||
process.env.PATH = `${mockBin}:${originalPath ?? ""}`;
|
||||
|
||||
const mockSCM: SCM = {
|
||||
name: "mock-scm",
|
||||
detectPR: vi.fn(),
|
||||
getPRState: vi.fn().mockResolvedValue("merged"),
|
||||
getPRState: vi.fn().mockResolvedValue("closed"),
|
||||
mergePR: vi.fn(),
|
||||
closePR: vi.fn(),
|
||||
getCIChecks: vi.fn(),
|
||||
|
|
@ -392,7 +392,7 @@ describe("cleanup", () => {
|
|||
expect(result.killed).toContain("app-6");
|
||||
const deleteLog = readFileSync(deleteLogPath, "utf-8");
|
||||
expect(deleteLog).toContain("session delete ses_archived");
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("does not skip archived cleanup for matching session IDs in other projects", async () => {
|
||||
const deleteLogPath = join(tmpDir, "opencode-delete-archived-cross-project.log");
|
||||
|
|
|
|||
|
|
@ -576,7 +576,7 @@ describe("spawn", () => {
|
|||
|
||||
const metadata = readMetadataRaw(sessionsDir, session.id);
|
||||
expect(metadata?.["opencodeSessionId"]).toBeUndefined();
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("throws for unknown project", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
|
@ -1865,7 +1865,7 @@ describe("spawn", () => {
|
|||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("throws for unknown project", async () => {
|
||||
const sm = createSessionManager({ config, registry: mockRegistry });
|
||||
|
|
|
|||
|
|
@ -548,6 +548,13 @@ function validateProjectUniqueness(config: OrchestratorConfig): void {
|
|||
/** Apply default reactions */
|
||||
function applyDefaultReactions(config: OrchestratorConfig): OrchestratorConfig {
|
||||
const defaults: Record<string, (typeof config.reactions)[string]> = {
|
||||
"pr-closed": {
|
||||
auto: true,
|
||||
action: "notify",
|
||||
priority: "action",
|
||||
message:
|
||||
"A PR was closed without merging. Decide whether to learn from the closure, resume the work, or terminate the session.",
|
||||
},
|
||||
"ci-failed": {
|
||||
auto: true,
|
||||
action: "send-to-agent",
|
||||
|
|
|
|||
|
|
@ -142,9 +142,24 @@ function statusToEventType(_from: SessionStatus | undefined, to: SessionStatus):
|
|||
}
|
||||
}
|
||||
|
||||
function prStateToEventType(
|
||||
from: Session["lifecycle"]["pr"]["state"],
|
||||
to: Session["lifecycle"]["pr"]["state"],
|
||||
): EventType | null {
|
||||
if (from === to) return null;
|
||||
switch (to) {
|
||||
case "closed":
|
||||
return "pr.closed";
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/** Map event type to reaction config key. */
|
||||
function eventToReactionKey(eventType: EventType): string | null {
|
||||
switch (eventType) {
|
||||
case "pr.closed":
|
||||
return "pr-closed";
|
||||
case "ci.failing":
|
||||
return "ci-failed";
|
||||
case "review.changes_requested":
|
||||
|
|
@ -683,8 +698,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
if (cachedData.state === PR_STATE.CLOSED) {
|
||||
lifecycle.pr.state = "closed";
|
||||
lifecycle.pr.reason = "closed_unmerged";
|
||||
setSessionState("done", "research_complete");
|
||||
return commit(SESSION_STATUS.DONE, "pr_closed", 0);
|
||||
setSessionState("idle", "pr_closed_waiting_decision");
|
||||
return commit(SESSION_STATUS.IDLE, "pr_closed", 0);
|
||||
}
|
||||
|
||||
lifecycle.pr.state = "open";
|
||||
|
|
@ -701,18 +716,18 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
if (cachedData.reviewDecision === "approved" || cachedData.reviewDecision === "none") {
|
||||
if (cachedData.mergeable) {
|
||||
lifecycle.pr.reason = "merge_ready";
|
||||
setSessionState("working", "awaiting_external_review");
|
||||
setSessionState("idle", "awaiting_external_review");
|
||||
return commit(SESSION_STATUS.MERGEABLE, "merge_ready", 0);
|
||||
}
|
||||
if (cachedData.reviewDecision === "approved") {
|
||||
lifecycle.pr.reason = "approved";
|
||||
setSessionState("working", "awaiting_external_review");
|
||||
setSessionState("idle", "awaiting_external_review");
|
||||
return commit(SESSION_STATUS.APPROVED, "review_approved", 0);
|
||||
}
|
||||
}
|
||||
if (cachedData.reviewDecision === "pending") {
|
||||
lifecycle.pr.reason = "review_pending";
|
||||
setSessionState("working", "awaiting_external_review");
|
||||
setSessionState("idle", "awaiting_external_review");
|
||||
return commit(SESSION_STATUS.REVIEW_PENDING, "review_pending", 0);
|
||||
}
|
||||
|
||||
|
|
@ -723,7 +738,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
}
|
||||
|
||||
lifecycle.pr.reason = "in_progress";
|
||||
setSessionState("working", "pr_created");
|
||||
setSessionState("idle", "pr_created");
|
||||
return commit(SESSION_STATUS.PR_OPEN, "pr_open", 0);
|
||||
}
|
||||
|
||||
|
|
@ -737,8 +752,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
if (prState === PR_STATE.CLOSED) {
|
||||
lifecycle.pr.state = "closed";
|
||||
lifecycle.pr.reason = "closed_unmerged";
|
||||
setSessionState("done", "research_complete");
|
||||
return commit(SESSION_STATUS.DONE, "pr_closed", 0);
|
||||
setSessionState("idle", "pr_closed_waiting_decision");
|
||||
return commit(SESSION_STATUS.IDLE, "pr_closed", 0);
|
||||
}
|
||||
|
||||
lifecycle.pr.state = "open";
|
||||
|
|
@ -759,18 +774,18 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
const mergeReady = await scm.getMergeability(session.pr);
|
||||
if (mergeReady.mergeable) {
|
||||
lifecycle.pr.reason = "merge_ready";
|
||||
setSessionState("working", "awaiting_external_review");
|
||||
setSessionState("idle", "awaiting_external_review");
|
||||
return commit(SESSION_STATUS.MERGEABLE, "merge_ready", 0);
|
||||
}
|
||||
if (reviewDecision === "approved") {
|
||||
lifecycle.pr.reason = "approved";
|
||||
setSessionState("working", "awaiting_external_review");
|
||||
setSessionState("idle", "awaiting_external_review");
|
||||
return commit(SESSION_STATUS.APPROVED, "review_approved", 0);
|
||||
}
|
||||
}
|
||||
if (reviewDecision === "pending") {
|
||||
lifecycle.pr.reason = "review_pending";
|
||||
setSessionState("working", "awaiting_external_review");
|
||||
setSessionState("idle", "awaiting_external_review");
|
||||
return commit(SESSION_STATUS.REVIEW_PENDING, "review_pending", 0);
|
||||
}
|
||||
|
||||
|
|
@ -781,7 +796,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
}
|
||||
|
||||
lifecycle.pr.reason = "in_progress";
|
||||
setSessionState("working", "pr_created");
|
||||
setSessionState("idle", "pr_created");
|
||||
return commit(SESSION_STATUS.PR_OPEN, "pr_open", 0);
|
||||
} catch {
|
||||
// Keep current status on SCM failure.
|
||||
|
|
@ -1018,7 +1033,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
const humanReactionKey = "changes-requested";
|
||||
const automatedReactionKey = "bugbot-comments";
|
||||
|
||||
if (TERMINAL_STATUSES.has(newStatus)) {
|
||||
if (TERMINAL_STATUSES.has(newStatus) || session.lifecycle.pr.state !== "open") {
|
||||
clearReactionTracker(session.id, humanReactionKey);
|
||||
clearReactionTracker(session.id, automatedReactionKey);
|
||||
lastReviewBacklogCheckAt.delete(session.id);
|
||||
|
|
@ -1350,8 +1365,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
|
||||
const conflictReactionKey = "merge-conflicts";
|
||||
|
||||
// Clear tracking when PR is closed/merged
|
||||
if (newStatus === "merged" || newStatus === "killed") {
|
||||
// Clear tracking when PR is no longer open.
|
||||
if (session.lifecycle.pr.state !== "open" || newStatus === "killed") {
|
||||
clearReactionTracker(session.id, conflictReactionKey);
|
||||
updateSessionMetadata(session, {
|
||||
lastMergeConflictDispatched: "",
|
||||
|
|
@ -1463,6 +1478,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
const tracked = states.get(session.id);
|
||||
const oldStatus =
|
||||
tracked ?? ((session.metadata?.["status"] as SessionStatus | undefined) || session.status);
|
||||
const previousPRState = session.lifecycle.pr.state;
|
||||
const assessment = await determineStatus(session);
|
||||
const newStatus = assessment.status;
|
||||
const lifecycleChanged = session.metadata["statePayload"] !== JSON.stringify(session.lifecycle);
|
||||
|
|
@ -1572,6 +1588,37 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
}
|
||||
}
|
||||
|
||||
const prEventType = prStateToEventType(previousPRState, session.lifecycle.pr.state);
|
||||
if (prEventType) {
|
||||
let reactionHandledNotify = false;
|
||||
const reactionKey = eventToReactionKey(prEventType);
|
||||
|
||||
if (reactionKey) {
|
||||
const reactionConfig = getReactionConfigForSession(session, reactionKey);
|
||||
if (reactionConfig && reactionConfig.action) {
|
||||
if (reactionConfig.auto !== false || reactionConfig.action === "notify") {
|
||||
await executeReaction(session.id, session.projectId, reactionKey, reactionConfig);
|
||||
reactionHandledNotify = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!reactionHandledNotify) {
|
||||
const prEvent = createEvent(prEventType, {
|
||||
sessionId: session.id,
|
||||
projectId: session.projectId,
|
||||
message: `${session.id}: PR ${previousPRState} → ${session.lifecycle.pr.state}`,
|
||||
data: {
|
||||
oldPRState: previousPRState,
|
||||
newPRState: session.lifecycle.pr.state,
|
||||
prNumber: session.lifecycle.pr.number,
|
||||
prUrl: session.lifecycle.pr.url,
|
||||
},
|
||||
});
|
||||
await notifyHuman(prEvent, inferPriority(prEventType));
|
||||
}
|
||||
}
|
||||
|
||||
// Pin first quality summary for title stability
|
||||
if (
|
||||
session.agentInfo?.summary &&
|
||||
|
|
|
|||
|
|
@ -252,6 +252,16 @@ export function deriveLegacyStatus(
|
|||
lifecycle: CanonicalSessionLifecycle,
|
||||
previousStatus: SessionStatus = "working",
|
||||
): SessionStatus {
|
||||
if (
|
||||
lifecycle.session.state === "terminated" &&
|
||||
(previousStatus === "cleanup" ||
|
||||
previousStatus === "errored" ||
|
||||
previousStatus === "killed" ||
|
||||
previousStatus === "terminated")
|
||||
) {
|
||||
return previousStatus;
|
||||
}
|
||||
|
||||
switch (lifecycle.session.state) {
|
||||
case "not_started":
|
||||
return "spawning";
|
||||
|
|
@ -263,17 +273,28 @@ export function deriveLegacyStatus(
|
|||
return "done";
|
||||
case "terminated":
|
||||
return "terminated";
|
||||
case "idle":
|
||||
return lifecycle.pr.state === "merged" ? "merged" : "idle";
|
||||
case "detecting":
|
||||
return "detecting";
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
if (lifecycle.pr.state === "merged") {
|
||||
return "merged";
|
||||
}
|
||||
if (lifecycle.pr.state === "open") {
|
||||
if (lifecycle.pr.reason === "ci_failing") return "ci_failed";
|
||||
if (lifecycle.pr.reason === "changes_requested") return "changes_requested";
|
||||
if (lifecycle.pr.reason === "review_pending") return "review_pending";
|
||||
if (lifecycle.pr.reason === "approved") return "approved";
|
||||
if (lifecycle.pr.reason === "merge_ready") return "mergeable";
|
||||
return "pr_open";
|
||||
}
|
||||
|
||||
switch (lifecycle.session.state) {
|
||||
case "idle":
|
||||
return "idle";
|
||||
case "working":
|
||||
if (lifecycle.pr.reason === "ci_failing") return "ci_failed";
|
||||
if (lifecycle.pr.reason === "changes_requested") return "changes_requested";
|
||||
if (lifecycle.pr.reason === "review_pending") return "review_pending";
|
||||
if (lifecycle.pr.reason === "approved") return "approved";
|
||||
if (lifecycle.pr.reason === "merge_ready") return "mergeable";
|
||||
if (lifecycle.pr.state === "open") return "pr_open";
|
||||
return "working";
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1872,11 +1872,11 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
|
|||
const plugins = resolvePlugins(project);
|
||||
let shouldKill = false;
|
||||
|
||||
// Check if PR is merged
|
||||
// Check if PR was closed without merging.
|
||||
if (session.pr && plugins.scm) {
|
||||
try {
|
||||
const prState = await plugins.scm.getPRState(session.pr);
|
||||
if (prState === PR_STATE.MERGED || prState === PR_STATE.CLOSED) {
|
||||
if (prState === PR_STATE.CLOSED) {
|
||||
shouldKill = true;
|
||||
}
|
||||
} catch {
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ export type CanonicalSessionReason =
|
|||
| "agent_acknowledged"
|
||||
| "task_in_progress"
|
||||
| "pr_created"
|
||||
| "pr_closed_waiting_decision"
|
||||
| "fixing_ci"
|
||||
| "resolving_review_comments"
|
||||
| "awaiting_user_input"
|
||||
|
|
|
|||
|
|
@ -53,6 +53,25 @@ function getDoneStatusInfo(session: DashboardSession): {
|
|||
};
|
||||
}
|
||||
|
||||
if (prState === "closed") {
|
||||
return {
|
||||
label: "closed",
|
||||
pillClass: "done-status-pill--exited",
|
||||
icon: (
|
||||
<svg
|
||||
fill="none"
|
||||
stroke="currentColor"
|
||||
strokeWidth="2"
|
||||
viewBox="0 0 24 24"
|
||||
className="h-3 w-3"
|
||||
>
|
||||
<circle cx="12" cy="12" r="9" />
|
||||
<path d="M9 12h6" />
|
||||
</svg>
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
if (status === "killed" || status === "terminated") {
|
||||
return {
|
||||
label: status,
|
||||
|
|
|
|||
|
|
@ -60,15 +60,15 @@ describe("getDashboardPageData fast path", () => {
|
|||
hoisted.enrichSessionsMetadataFastMock.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it("runs fast enrichment, uses cache-only PR hydration, and infers merged/closed state for terminal cache misses even without SCM", async () => {
|
||||
it("runs fast enrichment, uses cache-only PR hydration, and preserves canonical PR state on cache misses even without SCM", async () => {
|
||||
const noPrCore = { id: "session-no-pr", status: "working", pr: null };
|
||||
const closedCore = { id: "session-closed", status: "killed", pr: { number: 2 } };
|
||||
const mergedCore = { id: "session-merged", status: "merged", pr: { number: 3 } };
|
||||
const closedCore = { id: "session-closed", status: "idle", pr: { number: 2 } };
|
||||
const mergedCore = { id: "session-merged", status: "idle", pr: { number: 3 } };
|
||||
const allSessions = [noPrCore, closedCore, mergedCore];
|
||||
|
||||
const dashboardNoPr = { id: "session-no-pr", pr: null };
|
||||
const dashboardClosed = { id: "session-closed", pr: { state: "open", enriched: false } };
|
||||
const dashboardMerged = { id: "session-merged", pr: { state: "open", enriched: false } };
|
||||
const dashboardClosed = { id: "session-closed", pr: { state: "closed", enriched: false } };
|
||||
const dashboardMerged = { id: "session-merged", pr: { state: "merged", enriched: false } };
|
||||
|
||||
hoisted.getServicesMock.mockResolvedValue({
|
||||
config: { projects: { docs: { id: "docs" } } },
|
||||
|
|
|
|||
|
|
@ -149,6 +149,27 @@ describe("sessionToDashboard", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("should seed dashboard PR state from canonical lifecycle truth", () => {
|
||||
const lifecycle = createInitialCanonicalLifecycle("worker", new Date("2025-01-01T00:00:00Z"));
|
||||
lifecycle.session.state = "idle";
|
||||
lifecycle.session.reason = "merged_waiting_decision";
|
||||
lifecycle.session.startedAt = lifecycle.session.lastTransitionAt;
|
||||
lifecycle.pr.state = "merged";
|
||||
lifecycle.pr.reason = "merged";
|
||||
lifecycle.runtime.state = "alive";
|
||||
lifecycle.runtime.reason = "process_running";
|
||||
|
||||
const coreSession = createCoreSession({
|
||||
status: "idle",
|
||||
lifecycle,
|
||||
pr: createPRInfo(),
|
||||
});
|
||||
|
||||
const dashboard = sessionToDashboard(coreSession);
|
||||
|
||||
expect(dashboard.pr?.state).toBe("merged");
|
||||
});
|
||||
|
||||
it("should use agentInfo summary with summaryIsFallback false", () => {
|
||||
const coreSession = createCoreSession({
|
||||
agentInfo: {
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import "server-only";
|
||||
|
||||
import { cache } from "react";
|
||||
import { TERMINAL_STATUSES, type DashboardSession, type DashboardOrchestratorLink } from "@/lib/types";
|
||||
import { type DashboardSession, type DashboardOrchestratorLink } from "@/lib/types";
|
||||
import { getServices, getSCM } from "@/lib/services";
|
||||
import {
|
||||
sessionToDashboard,
|
||||
|
|
@ -73,8 +73,7 @@ export const getDashboardPageData = cache(async function getDashboardPageData(pr
|
|||
FAST_METADATA_ENRICH_TIMEOUT_MS,
|
||||
);
|
||||
|
||||
// PR cache hits only (in-memory lookup, no SCM API calls)
|
||||
// TERMINAL_STATUSES includes merged, killed, cleanup, done, terminated, errored
|
||||
// PR cache hits only (in-memory lookup, no SCM API calls).
|
||||
for (let i = 0; i < coreSessions.length; i++) {
|
||||
const core = coreSessions[i];
|
||||
if (!core.pr) continue;
|
||||
|
|
@ -83,17 +82,6 @@ export const getDashboardPageData = cache(async function getDashboardPageData(pr
|
|||
if (scm) {
|
||||
await enrichSessionPR(pageData.sessions[i], scm, core.pr, { cacheOnly: true });
|
||||
}
|
||||
|
||||
// For cache-miss PRs, infer terminal PR state from lifecycle status
|
||||
// to avoid showing merged/closed PRs as "open" until client refresh.
|
||||
const sessionPR = pageData.sessions[i].pr;
|
||||
if (sessionPR && !sessionPR.enriched && TERMINAL_STATUSES.has(core.status)) {
|
||||
if (core.status === "merged") {
|
||||
sessionPR.state = "merged";
|
||||
} else if (core.status === "killed") {
|
||||
sessionPR.state = "closed";
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
pageData.sessions = [];
|
||||
|
|
|
|||
|
|
@ -75,7 +75,12 @@ export function sessionToDashboard(session: Session): DashboardSession {
|
|||
summaryIsFallback: agentSummary ? (session.agentInfo?.summaryIsFallback ?? false) : false,
|
||||
createdAt: session.createdAt.toISOString(),
|
||||
lastActivityAt: session.lastActivityAt.toISOString(),
|
||||
pr: session.pr ? basicPRToDashboard(session.pr) : null,
|
||||
pr: session.pr
|
||||
? {
|
||||
...basicPRToDashboard(session.pr),
|
||||
state: normalizeDashboardPRState(session.lifecycle.pr.state),
|
||||
}
|
||||
: null,
|
||||
metadata: session.metadata,
|
||||
};
|
||||
}
|
||||
|
|
@ -137,6 +142,17 @@ function basicPRToDashboard(pr: PRInfo): DashboardPR {
|
|||
};
|
||||
}
|
||||
|
||||
function normalizeDashboardPRState(state: Session["lifecycle"]["pr"]["state"]): DashboardPR["state"] {
|
||||
switch (state) {
|
||||
case "merged":
|
||||
return "merged";
|
||||
case "closed":
|
||||
return "closed";
|
||||
default:
|
||||
return "open";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Enrich a DashboardSession's PR with live data from the SCM plugin.
|
||||
* Uses cache to reduce API calls and handles rate limit errors gracefully.
|
||||
|
|
|
|||
|
|
@ -129,7 +129,9 @@ async function labelIssuesForVerification(
|
|||
): Promise<void> {
|
||||
const mergedSessions = sessions.filter(
|
||||
(s) =>
|
||||
s.status === "merged" && s.issueId && !processedIssues.has(`${s.projectId}:${s.issueId}`),
|
||||
s.lifecycle.pr.state === "merged" &&
|
||||
s.issueId &&
|
||||
!processedIssues.has(`${s.projectId}:${s.issueId}`),
|
||||
);
|
||||
|
||||
for (const session of mergedSessions) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue