fix(workspaces): address Cody review on PAPA-430 (env resolver, accept gate, finalize record)

Three follow-ups to the PAPA-431/432/440 fixes flagged in Cody's review:

1. execution-workspace-policy: honor an explicit issue-level environmentId
   for null-default agents. The previous resolver returned the local default
   the moment `agentDefaultEnvironmentId === null`, before considering an
   operator-set `executionWorkspaceSettings.environmentId` on the issue. This
   conflicted with the issues-service contract (issues-service.test.ts:1924).
   The inheritance protection is now scoped to the actual inheritance signal:
   the issue env is stripped only when it exactly matches a reused workspace
   env (the shape produced by `inheritExecutionWorkspaceFromIssueId`).

2. issue-thread-interactions: scope the workspace_finalize accept gate to
   request_confirmation only. suggest_tasks acceptance just creates follow-up
   issues, so gating it on workspace_finalize was returning unnecessary 409s
   to the board.

3. heartbeat: surface workspace_finalize record-write failures on the
   succeeded path instead of swallowing them. A logged-and-suppressed write
   failure left the run successful while dependents remained gated behind a
   missing finalize row. The catch path still best-effort records
   finalize=failed before re-throwing.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/__tests__/execution-workspace-policy.test.ts

@@ -214,11 +214,11 @@ describe("execution workspace policy helpers", () => {
     });
   });
 
-  it("ignores issue/project env promotion for null-default agents even when no workspace is being reused", () => {
-    // A null-default (local-only) agent must not silently inherit an env that
-    // could have been auto-promoted from a different assignee. With no
-    // reused workspace there is no conflict to surface, but the resolved env
-    // is still the local default rather than the issue setting.
+  it("honors an explicit issue env override for null-default agents when no workspace is being reused", () => {
+    // Operator explicitly chose an env on this issue via PATCH (see the
+    // issues-service contract at issues-service.test.ts:1924). For null-default
+    // agents, this is a deliberate choice — only inherited issue env (which
+    // matches a reused workspace env) should be discarded.
     expect(
       resolveExecutionWorkspaceEnvironmentId({
         projectPolicy: { enabled: true, environmentId: "project-env" },
@@ -228,12 +228,36 @@ describe("execution workspace policy helpers", () => {
         defaultEnvironmentId: "local-env",
       }),
     ).toEqual({
-      environmentId: "local-env",
-      source: "default",
+      environmentId: "issue-env",
+      source: "issue",
       conflict: null,
     });
   });
 
+  it("honors an explicit issue env override for null-default agents even against a disagreeing reused workspace", () => {
+    // Operator picked sandbox-env explicitly while the previously-realized
+    // workspace was on local-env. The mismatch is genuine — surface a conflict
+    // so the heartbeat forces a fresh realization on the operator's chosen env.
+    expect(
+      resolveExecutionWorkspaceEnvironmentId({
+        projectPolicy: { enabled: true, environmentId: null },
+        issueSettings: { environmentId: "sandbox-env", mode: "shared_workspace" },
+        workspaceConfig: { environmentId: "local-env" },
+        agentDefaultEnvironmentId: null,
+        defaultEnvironmentId: "local-env",
+      }),
+    ).toEqual({
+      environmentId: "sandbox-env",
+      source: "issue",
+      conflict: {
+        reason: "reused_workspace_environment_mismatch",
+        workspaceEnvironmentId: "local-env",
+        assigneeIntendedEnvironmentId: "sandbox-env",
+        assigneeIntendedSource: "issue",
+      },
+    });
+  });
+
   it("prefers the explicit issue environment over project and agent defaults when no workspace is reused", () => {
     expect(
       resolveExecutionWorkspaceEnvironmentId({

server/src/__tests__/issue-thread-interactions-service.test.ts

@@ -1322,6 +1322,55 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
       });
     });
 
+    it("allows accept of suggest_tasks even when no successful workspace_finalize has landed", async () => {
+      // suggest_tasks acceptance only creates follow-up issues; it does not
+      // approve code state or move the source workspace forward, so the
+      // workspace_finalize gate (PAPA-440) must not apply here. Without this
+      // carve-out the board cannot triage suggested tasks on an issue whose
+      // latest workspace op is still worktree_prepare.
+      const { companyId, executionWorkspaceId, issueId, goalId } = await seedAcceptGateFixture();
+
+      await db.insert(workspaceOperations).values({
+        companyId,
+        executionWorkspaceId,
+        phase: "worktree_prepare",
+        status: "succeeded",
+        startedAt: new Date("2026-05-28T22:00:00.000Z"),
+      });
+
+      const created = await interactionsSvc.create({
+        id: issueId,
+        companyId,
+      }, {
+        kind: "suggest_tasks",
+        continuationPolicy: "wake_assignee",
+        payload: {
+          version: 1,
+          tasks: [
+            {
+              clientKey: "follow-up",
+              title: "Created from suggest_tasks accept under prepare-only workspace",
+            },
+          ],
+        },
+      }, {
+        userId: "local-board",
+      });
+
+      const accepted = await interactionsSvc.acceptInteraction(
+        { id: issueId, companyId, goalId, projectId: null },
+        created.id,
+        {},
+        { userId: "local-board" },
+      );
+
+      expect(accepted.interaction).toMatchObject({
+        id: created.id,
+        kind: "suggest_tasks",
+        status: "accepted",
+      });
+    });
+
     it("allows accept when the issue has no execution workspace attached", async () => {
       const { companyId, issueId } = await seedConfirmationIssue("No execution workspace accept");
 

server/src/services/execution-workspace-policy.ts

@@ -148,22 +148,27 @@ function resolveAssigneeIntendedExecutionWorkspaceEnvironment(input: {
   environmentId: string;
   source: Exclude<ExecutionWorkspaceEnvironmentSource, "workspace">;
 } {
-  // Companion bug to PAPA-380 / PAPA-431: when the assignee has no explicit
-  // defaultEnvironmentId, the agent is deliberately scoped to the local
-  // default. Such agents (e.g. Manual QA today) must NOT silently inherit an
-  // issue/project environment that may have been promoted from a different
-  // agent's identity via inheritExecutionWorkspaceFromIssueId or sibling
-  // assignment. Treat null as a positive "local default" signal rather than
-  // "inherit anything."
-  if (input.agentDefaultEnvironmentId === null) {
-    return { environmentId: input.defaultEnvironmentId, source: "default" };
-  }
+  // Explicit issue-level env override always wins, even for null-default
+  // (local-only) agents. An operator who deliberately set
+  // `executionWorkspaceSettings.environmentId` on this specific issue (see the
+  // issues-service contract preserved in issues.ts:4243) chose that env for
+  // this assignment and should not be silently downgraded to the local default
+  // (PAPA-430 review fix). Inherited issue envs from
+  // `inheritExecutionWorkspaceFromIssueId` are stripped before this point in
+  // `resolveExecutionWorkspaceEnvironmentId`.
   if (input.issueSettings?.environmentId !== undefined) {
     return {
       environmentId: input.issueSettings.environmentId ?? input.defaultEnvironmentId,
       source: "issue",
     };
   }
+  // A null defaultEnvironmentId on the agent means it is deliberately scoped to
+  // the local default (e.g. Manual QA today). Project policy must not promote
+  // such an agent off of local — only an explicit issue-level override above
+  // can move the assignee away from the local default.
+  if (input.agentDefaultEnvironmentId === null) {
+    return { environmentId: input.defaultEnvironmentId, source: "default" };
+  }
   if (input.projectPolicy?.environmentId !== undefined) {
     return {
       environmentId: input.projectPolicy.environmentId ?? input.defaultEnvironmentId,
@@ -180,7 +185,35 @@ export function resolveExecutionWorkspaceEnvironmentId(input: {
   agentDefaultEnvironmentId: string | null;
   defaultEnvironmentId: string;
 }): ExecutionWorkspaceEnvironmentResolution {
-  const assigneeIntended = resolveAssigneeIntendedExecutionWorkspaceEnvironment(input);
+  // PAPA-431 companion: when the assignee has no explicit defaultEnvironmentId
+  // (deliberately local-only, e.g. Manual QA) AND the issue settings env exactly
+  // matches the reused workspace env, treat the issue env as a promoted artifact
+  // from `inheritExecutionWorkspaceFromIssueId` rather than a deliberate
+  // operator choice. Strip it so the resolver falls back to the local default
+  // and the workspace-vs-intended conflict check forces a fresh realization.
+  // A genuine operator override (via PATCH on the issue) reaches this code path
+  // either with no reused workspace (workspaceConfig === null) or against a
+  // workspace whose persisted env does not match the new override; both keep
+  // the issue setting in place.
+  const inheritedIssueEnvOnNullDefaultAssignee =
+    input.agentDefaultEnvironmentId === null &&
+    input.workspaceConfig?.environmentId !== undefined &&
+    input.workspaceConfig?.environmentId !== null &&
+    input.issueSettings?.environmentId !== undefined &&
+    input.issueSettings.environmentId === input.workspaceConfig.environmentId;
+  let issueSettingsForResolution = input.issueSettings;
+  if (inheritedIssueEnvOnNullDefaultAssignee && input.issueSettings) {
+    const { environmentId: _droppedInheritedEnv, ...rest } = input.issueSettings;
+    void _droppedInheritedEnv;
+    issueSettingsForResolution = rest as IssueExecutionWorkspaceSettings;
+  }
+
+  const assigneeIntended = resolveAssigneeIntendedExecutionWorkspaceEnvironment({
+    projectPolicy: input.projectPolicy,
+    issueSettings: issueSettingsForResolution,
+    agentDefaultEnvironmentId: input.agentDefaultEnvironmentId,
+    defaultEnvironmentId: input.defaultEnvironmentId,
+  });
 
   if (input.workspaceConfig?.environmentId !== undefined) {
     const workspaceEnvironmentId =

server/src/services/heartbeat.ts

@@ -7814,24 +7814,20 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
         metadata?: Record<string, unknown>,
       ) => {
         if (adapterFinalizeOutcome) return;
+        await workspaceOperationRecorder.recordOperation({
+          phase: "workspace_finalize",
+          cwd: executionWorkspace.cwd,
+          metadata: {
+            adapterType: agent.adapterType,
+            executionTargetKind: executionTarget?.kind ?? "local",
+            ...metadata,
+          },
+          run: async () => ({ status }),
+        });
+        // Only mark the outcome after the row landed, so a transient write
+        // failure on the succeeded path can still be recovered by recording
+        // finalize=failed from the catch path below.
         adapterFinalizeOutcome = status;
-        try {
-          await workspaceOperationRecorder.recordOperation({
-            phase: "workspace_finalize",
-            cwd: executionWorkspace.cwd,
-            metadata: {
-              adapterType: agent.adapterType,
-              executionTargetKind: executionTarget?.kind ?? "local",
-              ...metadata,
-            },
-            run: async () => ({ status }),
-          });
-        } catch (recordErr) {
-          logger.warn(
-            { err: recordErr, runId: run.id, executionWorkspaceId: persistedExecutionWorkspace?.id ?? null },
-            "failed to record workspace_finalize operation",
-          );
-        }
       };
 
       let adapterResult: Awaited<ReturnType<typeof adapter.execute>>;
@@ -7864,14 +7860,26 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
         // Adapter returned cleanly, which means its workspace-restore finally
         // block also ran without throwing. Record the workspace_finalize
         // barrier so dependents that share this executionWorkspace can wake.
+        // If recording the barrier itself fails, propagate as a run failure
+        // rather than silently leaving dependents stranded behind a missing
+        // finalize row.
         await recordWorkspaceFinalize("succeeded");
       } catch (adapterErr) {
-        // Adapter (or its restore finally) threw. The workspace may be in a
-        // partial state — record finalize=failed so the dependent readiness
-        // check keeps the gate closed instead of waking on stale local state.
-        await recordWorkspaceFinalize("failed", {
-          errorMessage: adapterErr instanceof Error ? adapterErr.message : String(adapterErr),
-        });
+        // Adapter (or its restore finally) threw — or the finalize record
+        // write itself threw. Either way the workspace may be in a partial
+        // state. Best-effort record finalize=failed so the dependent readiness
+        // check keeps the gate closed instead of waking on stale local state,
+        // and surface the original error to the caller.
+        try {
+          await recordWorkspaceFinalize("failed", {
+            errorMessage: adapterErr instanceof Error ? adapterErr.message : String(adapterErr),
+          });
+        } catch (recordErr) {
+          logger.warn(
+            { err: recordErr, runId: run.id, executionWorkspaceId: persistedExecutionWorkspace?.id ?? null },
+            "failed to record workspace_finalize=failed operation; dependents may remain gated",
+          );
+        }
         throw adapterErr;
       }
       const adapterManagedRuntimeServices = adapterResult.runtimeServices

server/src/services/issue-thread-interactions.ts

@@ -771,11 +771,14 @@ export function issueThreadInteractionService(db: Db) {
     ): Promise<ResolvedInteractionResult> => {
       const data = acceptIssueThreadInteractionSchema.parse(input);
       const current = await getPendingInteractionForResolution({ issue, interactionId });
-      await assertIssueWorkspaceFinalizedForAccept({ db, issue });
       switch (current.kind) {
         case "suggest_tasks":
+          // Accepting suggest_tasks only creates follow-up issues; it does not
+          // approve code state or move the source workspace forward, so the
+          // workspace_finalize gate (PAPA-440) does not apply here.
           return issueThreadInteractionService(db).acceptSuggestedTasks(issue, interactionId, data, actor);
         case "request_confirmation": {
+          await assertIssueWorkspaceFinalizedForAccept({ db, issue });
           const accepted = await acceptRequestConfirmation({
             issue,
             current,