diff --git a/src/core/workflow.ts b/src/core/workflow.ts index db32f1eb..54c02615 100644 --- a/src/core/workflow.ts +++ b/src/core/workflow.ts @@ -1268,7 +1268,7 @@ async function handlePrCreatedStage( await linear.applyStageLabel(state.issue.id, "reviewing"); } -async function handleReviewTestingStage( +export async function handleReviewTestingStage( config: ResolvedProjectConfig, agent: AgentAdapter, notifications: ResolvedNotificationConfig, @@ -1345,10 +1345,12 @@ async function handleReviewTestingStage( await readyPullRequestAfterPassingReview(config, state.pullRequest, true); Object.assign(state, transitionStage(state, "done")); await saveRunState(config.workspacePath, state); - await linear.markStage(state.issue.id, "done"); - await linear.clearWorkflowStageLabels(state.issue.id); - await linear.comment(state.issue.id, "Review/testing passed. Marked done."); - await safeNotifyTaskOutcome(notifications, state, "done"); + await linear.markStage(state.issue.id, "reviewing"); + await linear.applyStageLabel(state.issue.id, "reviewing"); + await linear.comment( + state.issue.id, + "Review/testing passed. PR is ready and issue remains in review until merge.", + ); logger.info( buildIssueJobLogFields(state, "testing"), "Review/testing completed", @@ -1391,12 +1393,24 @@ async function handleDoneReviewMergeStage( return; } - state.pullRequestApprovedAt = new Date().toISOString(); - await saveRunState(config.workspacePath, state); + await finalizeIssueAfterReviewMerge(config, notifications, linear, state); +} + +export async function finalizeIssueAfterReviewMerge( + config: ResolvedProjectConfig, + notifications: ResolvedNotificationConfig, + linear: LinearClient, + state: RunState, +): Promise { + await linear.markStage(state.issue.id, "done"); + await linear.clearWorkflowStageLabels(state.issue.id); await linear.comment( state.issue.id, "PR squash-merged after completed review.", ); + state.pullRequestApprovedAt = new Date().toISOString(); + await saveRunState(config.workspacePath, state); + await safeNotifyTaskOutcome(notifications, state, "done"); } export function normalizeFailedReviewBugs( diff --git a/tests/config.test.ts b/tests/config.test.ts index 78c8da63..4b680ad5 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -272,17 +272,30 @@ describe("loadConfig", () => { }); it("loads notification settings from RESEND env vars", async () => { + const tempDir = await mkdtemp( + path.join(process.cwd(), ".tmp-config-test-"), + ); process.env.RESEND_API_KEY = "re_test_key"; process.env.RESEND_FROM = "ADHD.ai "; process.env.RESEND_TO = "a@example.com,b@example.com"; - const config = await loadConfig(process.cwd()); - expect(config.notifications.email.enabled).toBe(true); - expect(config.notifications.email.resendApiKey).toBe("re_test_key"); - expect(config.notifications.email.from).toBe("ADHD.ai "); - expect(config.notifications.email.to).toEqual([ - "a@example.com", - "b@example.com", - ]); + await writeFile( + path.join(tempDir, "adhd-ai.config.ts"), + ["export default {", " projects: [{ id: 'default' }]", "};", ""].join( + "\n", + ), + ); + try { + const config = await loadConfig(tempDir); + expect(config.notifications.email.enabled).toBe(true); + expect(config.notifications.email.resendApiKey).toBe("re_test_key"); + expect(config.notifications.email.from).toBe("ADHD.ai "); + expect(config.notifications.email.to).toEqual([ + "a@example.com", + "b@example.com", + ]); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } }); it("supports disabling notifications even with RESEND_API_KEY", async () => { @@ -314,12 +327,25 @@ describe("loadConfig", () => { }); it("rejects missing sender when notifications are enabled", async () => { + const tempDir = await mkdtemp( + path.join(process.cwd(), ".tmp-config-test-"), + ); process.env.RESEND_API_KEY = "re_test_key"; process.env.RESEND_FROM = ""; process.env.RESEND_TO = "a@example.com"; - await expect(loadConfig(process.cwd())).rejects.toThrow( - "notifications.email.from (or RESEND_FROM) is required when email notifications are enabled", + await writeFile( + path.join(tempDir, "adhd-ai.config.ts"), + ["export default {", " projects: [{ id: 'default' }]", "};", ""].join( + "\n", + ), ); + try { + await expect(loadConfig(tempDir)).rejects.toThrow( + "notifications.email.from (or RESEND_FROM) is required when email notifications are enabled", + ); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } }); it("rejects project-level notification overrides", async () => { diff --git a/tests/workflow.test.ts b/tests/workflow.test.ts index bc909815..acc678cc 100644 --- a/tests/workflow.test.ts +++ b/tests/workflow.test.ts @@ -20,7 +20,9 @@ import { buildPrioritizedIssueQueue, buildReviewOnlyIssueQueue, buildRunLeaseOwnerId, + finalizeIssueAfterReviewMerge, fixedBugsForImplementationComment, + handleReviewTestingStage, isReviewOnlyEligibleRunState, isReviewOnlyExecutableStage, isRunStateStaleForRetry, @@ -827,6 +829,153 @@ describe("readyPullRequestAfterPassingReview", () => { }); }); +describe("review pass stage transitions", () => { + it("keeps Linear in reviewing after a passing review result", async () => { + const workspace = await mkdtemp( + path.join(os.tmpdir(), "adhd-review-pass-"), + ); + const state = createRunState("ENG-100", "reviewing", Date.now()); + state.pullRequest = { + branch: "codex/eng-100", + title: "ENG-100", + url: "https://github.com/acme/repo/pull/100", + }; + const config = { + ...createProject("default"), + workspacePath: workspace, + dryRun: true, + }; + const notifications = { + email: { enabled: false, to: [] }, + }; + const agent = { + runPlan: async () => { + throw new Error("unused"); + }, + resume: async () => { + throw new Error("unused"); + }, + runReview: async () => ({ + finalMessage: "RESULT: PASS\nSUMMARY: good\nBUGS_JSON: []", + stdout: "", + usage: { inputTokens: 1, outputTokens: 1, totalTokens: 2 }, + }), + }; + const markStage = mock(async () => {}); + const applyStageLabel = mock(async () => {}); + const comment = mock(async () => {}); + const linear = { + markStage, + applyStageLabel, + comment, + }; + + await handleReviewTestingStage( + config, + agent, + notifications, + linear as never, + state, + ); + + expect(state.stage).toBe("done"); + expect(markStage).toHaveBeenCalledWith("lin_ENG-100", "testing"); + expect(markStage).toHaveBeenCalledWith("lin_ENG-100", "reviewing"); + expect(markStage).not.toHaveBeenCalledWith("lin_ENG-100", "done"); + expect(applyStageLabel).toHaveBeenCalledWith("lin_ENG-100", "testing"); + expect(applyStageLabel).toHaveBeenCalledWith("lin_ENG-100", "reviewing"); + expect(comment).toHaveBeenCalledWith( + "lin_ENG-100", + "Review/testing passed. PR is ready and issue remains in review until merge.", + ); + }); +}); + +describe("review-only done-stage merge finalization", () => { + it("moves Linear to done only after merge finalization", async () => { + const workspace = await mkdtemp( + path.join(os.tmpdir(), "adhd-review-merge-"), + ); + const state = createRunState("ENG-101", "done", Date.now()); + state.pullRequest = { + branch: "codex/eng-101", + title: "ENG-101", + url: "https://github.com/acme/repo/pull/101", + }; + const config = { + ...createProject("default"), + workspacePath: workspace, + }; + const notifications = { + email: { enabled: false, to: [] }, + }; + const markStage = mock(async () => {}); + const clearWorkflowStageLabels = mock(async () => {}); + const comment = mock(async () => {}); + const linear = { + markStage, + clearWorkflowStageLabels, + comment, + }; + + await finalizeIssueAfterReviewMerge( + config, + notifications, + linear as never, + state, + ); + + expect(state.pullRequestApprovedAt).toBeDefined(); + expect(markStage).toHaveBeenCalledWith("lin_ENG-101", "done"); + expect(clearWorkflowStageLabels).toHaveBeenCalledWith("lin_ENG-101"); + expect(comment).toHaveBeenCalledWith( + "lin_ENG-101", + "PR squash-merged after completed review.", + ); + }); + + it("does not persist pullRequestApprovedAt when Linear finalization fails", async () => { + const workspace = await mkdtemp( + path.join(os.tmpdir(), "adhd-review-merge-fail-"), + ); + const state = createRunState("ENG-102", "done", Date.now()); + state.pullRequest = { + branch: "codex/eng-102", + title: "ENG-102", + url: "https://github.com/acme/repo/pull/102", + }; + const config = { + ...createProject("default"), + workspacePath: workspace, + }; + const notifications = { + email: { enabled: false, to: [] }, + }; + const markStage = mock(async () => { + throw new Error("Linear unavailable"); + }); + const clearWorkflowStageLabels = mock(async () => {}); + const comment = mock(async () => {}); + const linear = { + markStage, + clearWorkflowStageLabels, + comment, + }; + + await expect( + finalizeIssueAfterReviewMerge( + config, + notifications, + linear as never, + state, + ), + ).rejects.toThrow("Linear unavailable"); + expect(state.pullRequestApprovedAt).toBeUndefined(); + expect(clearWorkflowStageLabels).not.toHaveBeenCalled(); + expect(comment).not.toHaveBeenCalled(); + }); +}); + describe("normalizeFailedReviewBugs", () => { it("returns empty list when review passed", () => { expect(