fix(prWorkItems): preserve workItemId across racing pipeline writers

src/db/repositories/prWorkItemsRepository.ts

@@ -8,6 +8,7 @@ import {
 	isNull,
 	max,
 	type SQL,
+	sql,
 	sum,
 } from 'drizzle-orm';
 import { getDb } from '../client.js';
@@ -80,10 +81,20 @@ export async function createWorkItem(
  * Upsert a PR ↔ work item link.
  *
  * Two-step logic:
- * 1. If a work-item-only row exists for (projectId, workItemId), UPDATE it with PR data.
+ * 1. If `workItemId` is provided, drop any racing orphan `(projectId, NULL workItemId,
+ *    prNumber)` row, then UPDATE the existing work-item-only row with PR data.
  * 2. Otherwise INSERT a new row, using onConflictDoUpdate on (projectId, prNumber).
  *
- * workItemId is optional to support "orphan" PRs (PRs created without a linked work item).
+ * `workItemId` is optional to support "orphan" PRs (PRs created without a linked
+ * work item).
+ *
+ * **Link-preservation invariant** — the `work_item_id` column is one-way set: once
+ * an earlier writer landed a non-null value, a later writer that happens to lack
+ * the workItemId must NOT unset it. Step 2's ON CONFLICT clause uses
+ * `COALESCE(EXCLUDED.work_item_id, pr_work_items.work_item_id)` to enforce this.
+ * Without it, a stale review-pipeline writeback (workItemId captured at PR-opened
+ * time, before the implementation linked the PR) would clobber the correct value
+ * and break downstream lookups (notably the `pr-ready-to-merge` auto-merge trigger).
  */
 export async function linkPRToWorkItem(
 	projectId: string,
@@ -96,8 +107,23 @@ export async function linkPRToWorkItem(
 	const now = new Date();
 	const { workItemUrl, workItemTitle, prUrl, prTitle } = options;
 
-	// Step 1: If workItemId is provided, try to update the existing work-item-only row
+	// Step 1: If workItemId is provided, try to update the existing work-item-only row.
 	if (workItemId) {
+		// First, drop any racing orphan row (projectId, NULL workItemId, prNumber). It
+		// only exists because no link existed when an earlier writer (e.g. the review
+		// pipeline's pre-execution pass on a freshly opened PR) inserted it. Promoting
+		// our work-item-only row to the same prNumber would otherwise hit
+		// uq_pr_work_items_project_pr.
+		await db
+			.delete(prWorkItems)
+			.where(
+				and(
+					eq(prWorkItems.projectId, projectId),
+					eq(prWorkItems.prNumber, prNumber),
+					isNull(prWorkItems.workItemId),
+				),
+			);
+
 		const updated = await db
 			.update(prWorkItems)
 			.set({
@@ -142,7 +168,10 @@ export async function linkPRToWorkItem(
 			target: [prWorkItems.projectId, prWorkItems.prNumber],
 			targetWhere: isNotNull(prWorkItems.prNumber),
 			set: {
-				workItemId,
+				// COALESCE: never erase a known workItemId with null. The link is
+				// one-way set — once an earlier writer landed it, later writers that
+				// happen to lack the workItemId must not unset it.
+				workItemId: sql`COALESCE(${workItemId}, ${prWorkItems.workItemId})`,
 				repoFullName,
 				workItemUrl,
 				workItemTitle,

src/triggers/shared/agent-execution.ts

@@ -395,7 +395,21 @@ export async function runAgentExecutionPipeline(
 
 	const { skipPrepareForAgent = false, onSuccess, onFailure, logLabel = 'Agent' } = executionConfig;
 
-	const workItemId = result.workItemId;
+	// Re-resolve workItemId at run time. The trigger handler (e.g. PROpenedTrigger)
+	// captures workItemId synchronously at webhook arrival, before any other
+	// pipeline has had time to link the PR. By the time we run, the DB may have
+	// caught up — preferring the live value avoids carrying a stale `undefined`
+	// into runAgent (and therefore agent_runs.work_item_id) and into the
+	// post-execution linkPRToWorkItem write.
+	const workItemId = await resolveWorkItemId(result.workItemId, project.id, result.prNumber);
+
+	// If we recovered a workItemId the trigger didn't have, patch agentInput so
+	// the corrected value flows into runAgent and into the agent_runs row that
+	// tryCreateRun (src/agents/shared/runTracking.ts) writes.
+	const agentInput =
+		workItemId && workItemId !== result.workItemId
+			? { ...result.agentInput, workItemId }
+			: result.agentInput;
 
 	let remainingBudgetUsd: number | undefined;
 	if (workItemId) {
@@ -448,7 +462,7 @@ export async function runAgentExecutionPipeline(
 	}
 
 	const agentResult = await runAgent(agentType, {
-		...result.agentInput,
+		...agentInput,
 		remainingBudgetUsd,
 		project,
 		config,

tests/integration/db/prWorkItemsRepository.test.ts

@@ -2,6 +2,7 @@ import { and, eq } from 'drizzle-orm';
 import { beforeEach, describe, expect, it } from 'vitest';
 import { getDb } from '../../../src/db/client.js';
 import {
+	createWorkItem,
 	linkPRToWorkItem,
 	listPRsForProject,
 	listPRsForWorkItem,
@@ -105,6 +106,62 @@ describe('prWorkItemsRepository (integration)', () => {
 		});
 	});
 
+	// =========================================================================
+	// Link-preservation invariant — workItemId is one-way set
+	// =========================================================================
+
+	describe('linkPRToWorkItem — link-preservation invariant', () => {
+		it('preserves an existing non-null workItemId when called with workItemId=null', async () => {
+			// First writer (e.g. implementation pipeline post-execution): correct link.
+			await linkPRToWorkItem('test-project', 'owner/repo', 568, 'MNG-93', {
+				workItemUrl: 'https://linear.app/mongrel/issue/MNG-93/...',
+				prUrl: 'https://github.com/owner/repo/pull/568',
+				prTitle: 'feat: add unit tests',
+			});
+
+			// Second writer (e.g. review pipeline post-execution) carries no workItemId.
+			await linkPRToWorkItem('test-project', 'owner/repo', 568, null, {
+				prUrl: 'https://github.com/owner/repo/pull/568',
+				prTitle: 'feat: add unit tests',
+			});
+
+			// The known link must NOT be erased.
+			const workItemId = await lookupWorkItemForPR('test-project', 568);
+			expect(workItemId).toBe('MNG-93');
+		});
+
+		it('removes a stale orphan (projectId, NULL, prNumber) row when promoting a work-item-only row', async () => {
+			// Step 1: review pipeline pre-execution insert an orphan row.
+			await linkPRToWorkItem('test-project', 'owner/repo', 568, null, {
+				prUrl: 'https://github.com/owner/repo/pull/568',
+				prTitle: 'feat: add unit tests',
+			});
+			// Step 2: implementation pipeline createWorkItem inserted earlier
+			// (we do it here in a different order to simulate the race).
+			await createWorkItem('test-project', 'MNG-93', {
+				workItemUrl: 'https://linear.app/mongrel/issue/MNG-93/...',
+				workItemTitle: 'add unit tests',
+			});
+
+			// Step 3: implementation completes — promote work-item-only row.
+			// Without orphan cleanup this throws on uq_pr_work_items_project_pr.
+			await linkPRToWorkItem('test-project', 'owner/repo', 568, 'MNG-93', {
+				workItemUrl: 'https://linear.app/mongrel/issue/MNG-93/...',
+				prUrl: 'https://github.com/owner/repo/pull/568',
+				prTitle: 'feat: add unit tests',
+			});
+
+			const db = getDb();
+			const rows = await db
+				.select()
+				.from(prWorkItems)
+				.where(and(eq(prWorkItems.projectId, 'test-project'), eq(prWorkItems.prNumber, 568)));
+			expect(rows).toHaveLength(1);
+			expect(rows[0].workItemId).toBe('MNG-93');
+			expect(rows[0].workItemUrl).toBe('https://linear.app/mongrel/issue/MNG-93/...');
+		});
+	});
+
 	describe('lookupWorkItemForPR', () => {
 		it('returns null for non-existent link', async () => {
 			const result = await lookupWorkItemForPR('test-project', 999);

tests/unit/db/repositories/prWorkItemsRepository.test.ts

@@ -120,6 +120,7 @@ function makeQueryChainWithRows(rows: unknown[]): ReturnType<typeof createQueryM
 
 describe('prWorkItemsRepository', () => {
 	let chain: ReturnType<typeof createMockChain>;
+	let deleteWhere: ReturnType<typeof vi.fn>;
 	let mockDb: {
 		select: ReturnType<typeof vi.fn>;
 		insert: ReturnType<typeof vi.fn>;
@@ -131,12 +132,14 @@ describe('prWorkItemsRepository', () => {
 		// linkPRToWorkItem's two-step logic: first update (returns []), then insert.
 		// Default: update returns [] (no existing work-item row), insert proceeds.
 		chain = createMockChain([]);
+		// Step 1's orphan-cleanup uses db.delete(table).where(condition).
+		deleteWhere = vi.fn().mockResolvedValue(undefined);
 
 		mockDb = {
 			select: vi.fn().mockReturnValue({ from: chain.from }),
 			insert: vi.fn().mockReturnValue({ values: chain.values }),
 			update: vi.fn().mockReturnValue({ set: chain.set }),
-			delete: vi.fn(),
+			delete: vi.fn().mockReturnValue({ where: deleteWhere }),
 		};
 		mockGetDb.mockReturnValue(mockDb as never);
 	});
@@ -269,12 +272,26 @@ describe('prWorkItemsRepository', () => {
 			await linkPRToWorkItem('proj-1', 'owner/repo', 42, 'wi-abc');
 
 			expect(chain.onConflictDoUpdate).toHaveBeenCalledTimes(1);
-			expect(chain.onConflictDoUpdate).toHaveBeenCalledWith(
-				expect.objectContaining({
-					target: expect.arrayContaining([expect.anything(), expect.anything()]),
-					set: expect.objectContaining({ workItemId: 'wi-abc', repoFullName: 'owner/repo' }),
-				}),
-			);
+			// workItemId in `set` is a COALESCE SQL expression (one-way set semantics),
+			// not the literal string. Other fields are passed through unchanged.
+			const conflictArg = chain.onConflictDoUpdate.mock.calls[0][0];
+			expect(conflictArg.target).toHaveLength(2);
+			expect(conflictArg.set.repoFullName).toBe('owner/repo');
+			expect(conflictArg.set.workItemId).toBeDefined();
+			expect(conflictArg.set.workItemId).not.toBe('wi-abc'); // wrapped in SQL
+		});
+
+		it('deletes any racing orphan (NULL workItemId, prNumber) row before Step 1 UPDATE', async () => {
+			await linkPRToWorkItem('proj-1', 'owner/repo', 42, 'wi-abc');
+
+			expect(mockDb.delete).toHaveBeenCalledTimes(1);
+			expect(deleteWhere).toHaveBeenCalledTimes(1);
+		});
+
+		it('does NOT call delete when workItemId is null (no Step 1 path)', async () => {
+			await linkPRToWorkItem('proj-1', 'owner/repo', 42, null);
+
+			expect(mockDb.delete).not.toHaveBeenCalled();
 		});
 
 		it('persists optional display fields when provided', async () => {

tests/unit/integrations/pm-conformance.test.ts

@@ -43,7 +43,7 @@ describe('PM provider conformance (every registered provider)', () => {
 			expect(manifest.category).toBe('pm');
 		});
 
-		it('webhookRoute matches the /${id}/webhook convention', () => {
+		it('webhookRoute matches the /<id>/webhook convention', () => {
 			expect(manifest.webhookRoute).toBe(`/${id}/webhook`);
 		});
 

tests/unit/triggers/shared/agent-execution.test.ts

@@ -819,3 +819,97 @@ describe('pre-execution PR linking (via runAgentExecutionPipeline)', () => {
 		);
 	});
 });
+
+// ---------------------------------------------------------------------------
+// workItemId staleness recovery (via runAgentExecutionPipeline)
+// ---------------------------------------------------------------------------
+
+describe('workItemId staleness recovery (via runAgentExecutionPipeline)', () => {
+	beforeEach(() => {
+		mockCreatePMProvider.mockReturnValue({});
+		mockResolveProjectPMConfig.mockReturnValue(PM_CONFIG);
+		mockValidateIntegrations.mockResolvedValue({ valid: true, errors: [] });
+		mockCheckBudgetExceeded.mockResolvedValue(null);
+		mockHandleAgentResultArtifacts.mockResolvedValue(undefined);
+		mockShouldTriggerDebug.mockResolvedValue(null);
+		mockGetSessionState.mockReturnValue({});
+		mockRunAgent.mockResolvedValue({ success: true, output: '', runId: 'run-1' });
+	});
+
+	it('re-resolves workItemId from DB when result.workItemId is undefined but PR is already linked', async () => {
+		// Implementation has already linked PR #42 to card-from-db
+		mockLookupWorkItemForPR.mockResolvedValueOnce('card-from-db');
+
+		// PROpenedTrigger-style result captured before the link existed
+		await runAgentExecutionPipeline(
+			{
+				agentType: 'review',
+				agentInput: { prNumber: 42 },
+				prNumber: 42,
+				prUrl: 'https://github.com/acme/myapp/pull/42',
+				prTitle: 'Test PR',
+			},
+			PROJECT,
+			CONFIG,
+		);
+
+		// runAgent receives the resolved workItemId in agentInput
+		expect(mockRunAgent).toHaveBeenCalledWith(
+			'review',
+			expect.objectContaining({ workItemId: 'card-from-db' }),
+		);
+
+		// linkPRToWorkItem is called with the resolved workItemId, not null
+		expect(vi.mocked(linkPRToWorkItem)).toHaveBeenCalledWith(
+			'project-1',
+			'acme/myapp',
+			42,
+			'card-from-db',
+			expect.anything(),
+		);
+	});
+
+	it('preserves trigger-supplied workItemId when DB lookup is unnecessary', async () => {
+		// Trigger already carries a workItemId — no DB lookup expected
+		await runAgentExecutionPipeline(
+			{
+				agentType: 'review',
+				agentInput: { prNumber: 42, workItemId: 'card-from-trigger' },
+				prNumber: 42,
+				workItemId: 'card-from-trigger',
+				prUrl: 'https://github.com/acme/myapp/pull/42',
+			},
+			PROJECT,
+			CONFIG,
+		);
+
+		expect(mockLookupWorkItemForPR).not.toHaveBeenCalled();
+		expect(mockRunAgent).toHaveBeenCalledWith(
+			'review',
+			expect.objectContaining({ workItemId: 'card-from-trigger' }),
+		);
+	});
+
+	it('leaves workItemId undefined when neither trigger nor DB has one', async () => {
+		// Default mockLookupWorkItemForPR returns null
+		await runAgentExecutionPipeline(
+			{
+				agentType: 'review',
+				agentInput: { prNumber: 42 },
+				prNumber: 42,
+				prUrl: 'https://github.com/acme/myapp/pull/42',
+			},
+			PROJECT,
+			CONFIG,
+		);
+
+		expect(mockLookupWorkItemForPR).toHaveBeenCalledWith('project-1', 42);
+		expect(vi.mocked(linkPRToWorkItem)).toHaveBeenCalledWith(
+			'project-1',
+			'acme/myapp',
+			42,
+			null,
+			expect.anything(),
+		);
+	});
+});

web/src/components/projects/pm-wizard.tsx

@@ -11,7 +11,6 @@ import './pm-providers/jira/index.js';
 import './pm-providers/linear/index.js';
 import { ManifestProviderWizardSection } from './pm-providers/manifest-section.js';
 import { getProviderWizard } from './pm-providers/registry.js';
-import { renderManifestStep } from './pm-providers/render.js';
 import { SaveStep, WebhookStep } from './pm-wizard-common-steps.js';
 import {
 	useLinearWebhookInfo,