diff --git a/server/src/__tests__/execution-workspace-policy.test.ts b/server/src/__tests__/execution-workspace-policy.test.ts index 507db5551c3..e7b840f7c0e 100644 --- a/server/src/__tests__/execution-workspace-policy.test.ts +++ b/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({ diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index 22af3ec9595..7bf9dfed3ad 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/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"); diff --git a/server/src/services/execution-workspace-policy.ts b/server/src/services/execution-workspace-policy.ts index de97fe9a893..6da16b42899 100644 --- a/server/src/services/execution-workspace-policy.ts +++ b/server/src/services/execution-workspace-policy.ts @@ -148,22 +148,27 @@ function resolveAssigneeIntendedExecutionWorkspaceEnvironment(input: { environmentId: string; source: Exclude; } { - // 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 = diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 283e3bb850c..f5e683b7c0b 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -7814,24 +7814,20 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) metadata?: Record, ) => { 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>; @@ -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 diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 774c23a7a7c..73999e009be 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -771,11 +771,14 @@ export function issueThreadInteractionService(db: Db) { ): Promise => { 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,