mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(workspace): refuse silent env reuse when assignee identity disagrees (PAPA-380, PAPA-431)
resolveExecutionWorkspaceEnvironmentId previously returned the persisted
workspace's environmentId as soon as one was present, regardless of who
now owned the issue. That silently dropped a sandbox-bound agent onto a
local workspace (PAPA-380) and, symmetrically, dropped a Manual QA agent
(defaultEnvironmentId: null) onto an engineer's SSH environment when it
inherited a sibling's workspace via inheritExecutionWorkspaceFromIssueId
(PAPA-431).
The resolver now:
- Computes the assignee's intended environment from non-workspace
signals (issue, project, agent default) and treats a null agent
defaultEnvironmentId as a deliberate "local default" rather than
"inherit anything the issue hands me".
- Returns a structured { environmentId, source, conflict } result; when
the reused workspace env disagrees with the assignee's intent it
returns the assignee-intended env and surfaces a conflict signal.
heartbeat.ts consumes the conflict signal to flip shouldReuseExisting
off, forcing a fresh workspace realization on the assignee's env and
logging the refused reuse with both env ids so operators can see why a
new workspace was provisioned. The downstream persistence/idle paths
naturally handle the new workspace and idle the old one.
Resolver tests cover both directions (sandbox agent on local workspace
and null-default agent on SSH workspace) as required by the issue. No
git remote is needed — tests pass with an empty git remote.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
96f0279e08
commit
07266c07b7
3 files changed
+228
-33
No files matched your search
@@ -148,16 +148,93 @@ describe("execution workspace policy helpers", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("prefers persisted environment selection over issue and project defaults", () => {
|
||||
it("reuses persisted workspace environment when it agrees with the assignee's identity", () => {
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: "agent-env" },
|
||||
issueSettings: { environmentId: "agent-env" },
|
||||
workspaceConfig: { environmentId: "agent-env" },
|
||||
agentDefaultEnvironmentId: "agent-env",
|
||||
defaultEnvironmentId: "default-env",
|
||||
}),
|
||||
).toEqual({
|
||||
environmentId: "agent-env",
|
||||
source: "workspace",
|
||||
conflict: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses silent reuse when the persisted workspace env disagrees with the assignee (PAPA-380: sandbox agent on local workspace)", () => {
|
||||
// Claude E2B was assigned to a child issue whose parent had already
|
||||
// realized a `Local` workspace. The persisted workspace env must not
|
||||
// shadow the agent's intended sandbox env.
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: null },
|
||||
issueSettings: { environmentId: "sandbox-env", mode: "shared_workspace" },
|
||||
workspaceConfig: { environmentId: "local-env" },
|
||||
agentDefaultEnvironmentId: "sandbox-env",
|
||||
defaultEnvironmentId: "local-env",
|
||||
}),
|
||||
).toEqual({
|
||||
environmentId: "sandbox-env",
|
||||
source: "issue",
|
||||
conflict: {
|
||||
reason: "reused_workspace_environment_mismatch",
|
||||
workspaceEnvironmentId: "local-env",
|
||||
assigneeIntendedEnvironmentId: "sandbox-env",
|
||||
assigneeIntendedSource: "issue",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses silent reuse when a null-default (local) agent inherits a non-local workspace env (PAPA-431: Manual QA on engineer SSH workspace)", () => {
|
||||
// Manual QA agent has defaultEnvironmentId: null. When a sibling issue's
|
||||
// SSH workspace is inherited via inheritExecutionWorkspaceFromIssueId,
|
||||
// the persisted SSH env must NOT shadow the agent's deliberate local
|
||||
// identity. The inherited issueSettings.environmentId is treated as a
|
||||
// promoted artifact, not an explicit operator choice.
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: null },
|
||||
issueSettings: { environmentId: "ssh-env", mode: "isolated_workspace" },
|
||||
workspaceConfig: { environmentId: "ssh-env" },
|
||||
agentDefaultEnvironmentId: null,
|
||||
defaultEnvironmentId: "local-env",
|
||||
}),
|
||||
).toEqual({
|
||||
environmentId: "local-env",
|
||||
source: "default",
|
||||
conflict: {
|
||||
reason: "reused_workspace_environment_mismatch",
|
||||
workspaceEnvironmentId: "ssh-env",
|
||||
assigneeIntendedEnvironmentId: "local-env",
|
||||
assigneeIntendedSource: "default",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
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.
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: "project-env" },
|
||||
issueSettings: { environmentId: "issue-env" },
|
||||
workspaceConfig: { environmentId: "workspace-env" },
|
||||
agentDefaultEnvironmentId: "agent-env",
|
||||
defaultEnvironmentId: "default-env",
|
||||
workspaceConfig: null,
|
||||
agentDefaultEnvironmentId: null,
|
||||
defaultEnvironmentId: "local-env",
|
||||
}),
|
||||
).toBe("workspace-env");
|
||||
).toEqual({
|
||||
environmentId: "local-env",
|
||||
source: "default",
|
||||
conflict: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("prefers the explicit issue environment over project and agent defaults when no workspace is reused", () => {
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: "project-env" },
|
||||
@@ -166,7 +243,11 @@ describe("execution workspace policy helpers", () => {
|
||||
agentDefaultEnvironmentId: "agent-env",
|
||||
defaultEnvironmentId: "default-env",
|
||||
}),
|
||||
).toBe("issue-env");
|
||||
).toEqual({
|
||||
environmentId: "issue-env",
|
||||
source: "issue",
|
||||
conflict: null,
|
||||
});
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: "project-env" },
|
||||
@@ -175,7 +256,11 @@ describe("execution workspace policy helpers", () => {
|
||||
agentDefaultEnvironmentId: "agent-env",
|
||||
defaultEnvironmentId: "default-env",
|
||||
}),
|
||||
).toBe("project-env");
|
||||
).toEqual({
|
||||
environmentId: "project-env",
|
||||
source: "project",
|
||||
conflict: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the agent default environment before the company default", () => {
|
||||
@@ -187,7 +272,11 @@ describe("execution workspace policy helpers", () => {
|
||||
agentDefaultEnvironmentId: "agent-env",
|
||||
defaultEnvironmentId: "default-env",
|
||||
}),
|
||||
).toBe("agent-env");
|
||||
).toEqual({
|
||||
environmentId: "agent-env",
|
||||
source: "agent",
|
||||
conflict: null,
|
||||
});
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: null },
|
||||
@@ -196,7 +285,11 @@ describe("execution workspace policy helpers", () => {
|
||||
agentDefaultEnvironmentId: "agent-env",
|
||||
defaultEnvironmentId: "default-env",
|
||||
}),
|
||||
).toBe("default-env");
|
||||
).toEqual({
|
||||
environmentId: "default-env",
|
||||
source: "project",
|
||||
conflict: null,
|
||||
});
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: null,
|
||||
@@ -205,7 +298,11 @@ describe("execution workspace policy helpers", () => {
|
||||
agentDefaultEnvironmentId: null,
|
||||
defaultEnvironmentId: "default-env",
|
||||
}),
|
||||
).toBe("default-env");
|
||||
).toEqual({
|
||||
environmentId: "default-env",
|
||||
source: "default",
|
||||
conflict: null,
|
||||
});
|
||||
expect(
|
||||
resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: { enabled: true, environmentId: null },
|
||||
@@ -214,7 +311,11 @@ describe("execution workspace policy helpers", () => {
|
||||
agentDefaultEnvironmentId: null,
|
||||
defaultEnvironmentId: "default-env",
|
||||
}),
|
||||
).toBe("default-env");
|
||||
).toEqual({
|
||||
environmentId: "default-env",
|
||||
source: "default",
|
||||
conflict: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("maps persisted execution workspace modes back to issue settings", () => {
|
||||
|
||||
@@ -119,26 +119,93 @@ export function parseIssueExecutionWorkspaceSettings(raw: unknown): IssueExecuti
|
||||
};
|
||||
}
|
||||
|
||||
export type ExecutionWorkspaceEnvironmentSource =
|
||||
| "workspace"
|
||||
| "issue"
|
||||
| "project"
|
||||
| "agent"
|
||||
| "default";
|
||||
|
||||
export type ExecutionWorkspaceEnvironmentConflict = {
|
||||
reason: "reused_workspace_environment_mismatch";
|
||||
workspaceEnvironmentId: string;
|
||||
assigneeIntendedEnvironmentId: string;
|
||||
assigneeIntendedSource: Exclude<ExecutionWorkspaceEnvironmentSource, "workspace">;
|
||||
};
|
||||
|
||||
export type ExecutionWorkspaceEnvironmentResolution = {
|
||||
environmentId: string;
|
||||
source: ExecutionWorkspaceEnvironmentSource;
|
||||
conflict: ExecutionWorkspaceEnvironmentConflict | null;
|
||||
};
|
||||
|
||||
function resolveAssigneeIntendedExecutionWorkspaceEnvironment(input: {
|
||||
projectPolicy: ProjectExecutionWorkspacePolicy | null;
|
||||
issueSettings: IssueExecutionWorkspaceSettings | null;
|
||||
agentDefaultEnvironmentId: string | null;
|
||||
defaultEnvironmentId: string;
|
||||
}): {
|
||||
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" };
|
||||
}
|
||||
if (input.issueSettings?.environmentId !== undefined) {
|
||||
return {
|
||||
environmentId: input.issueSettings.environmentId ?? input.defaultEnvironmentId,
|
||||
source: "issue",
|
||||
};
|
||||
}
|
||||
if (input.projectPolicy?.environmentId !== undefined) {
|
||||
return {
|
||||
environmentId: input.projectPolicy.environmentId ?? input.defaultEnvironmentId,
|
||||
source: "project",
|
||||
};
|
||||
}
|
||||
return { environmentId: input.agentDefaultEnvironmentId, source: "agent" };
|
||||
}
|
||||
|
||||
export function resolveExecutionWorkspaceEnvironmentId(input: {
|
||||
projectPolicy: ProjectExecutionWorkspacePolicy | null;
|
||||
issueSettings: IssueExecutionWorkspaceSettings | null;
|
||||
workspaceConfig: { environmentId?: string | null } | null;
|
||||
agentDefaultEnvironmentId: string | null;
|
||||
defaultEnvironmentId: string;
|
||||
}) {
|
||||
}): ExecutionWorkspaceEnvironmentResolution {
|
||||
const assigneeIntended = resolveAssigneeIntendedExecutionWorkspaceEnvironment(input);
|
||||
|
||||
if (input.workspaceConfig?.environmentId !== undefined) {
|
||||
return input.workspaceConfig.environmentId ?? input.defaultEnvironmentId;
|
||||
const workspaceEnvironmentId =
|
||||
input.workspaceConfig.environmentId ?? input.defaultEnvironmentId;
|
||||
// PAPA-380 / PAPA-431: a reused workspace's persisted environmentId must
|
||||
// never silently shadow the current assignee's environment identity.
|
||||
// When they disagree, refuse the silent reuse: return the assignee's
|
||||
// intended env and surface a conflict signal so the caller forces a fresh
|
||||
// workspace realization (or otherwise alerts the operator) instead of
|
||||
// running the agent on someone else's environment.
|
||||
if (workspaceEnvironmentId !== assigneeIntended.environmentId) {
|
||||
return {
|
||||
environmentId: assigneeIntended.environmentId,
|
||||
source: assigneeIntended.source,
|
||||
conflict: {
|
||||
reason: "reused_workspace_environment_mismatch",
|
||||
workspaceEnvironmentId,
|
||||
assigneeIntendedEnvironmentId: assigneeIntended.environmentId,
|
||||
assigneeIntendedSource: assigneeIntended.source,
|
||||
},
|
||||
};
|
||||
}
|
||||
return { environmentId: workspaceEnvironmentId, source: "workspace", conflict: null };
|
||||
}
|
||||
if (input.issueSettings?.environmentId !== undefined) {
|
||||
return input.issueSettings.environmentId ?? input.defaultEnvironmentId;
|
||||
}
|
||||
if (input.projectPolicy?.environmentId !== undefined) {
|
||||
return input.projectPolicy.environmentId ?? input.defaultEnvironmentId;
|
||||
}
|
||||
if (input.agentDefaultEnvironmentId !== null) {
|
||||
return input.agentDefaultEnvironmentId;
|
||||
}
|
||||
return input.defaultEnvironmentId;
|
||||
return { environmentId: assigneeIntended.environmentId, source: assigneeIntended.source, conflict: null };
|
||||
}
|
||||
|
||||
export function defaultIssueExecutionWorkspaceSettingsForProject(
|
||||
|
||||
@@ -7106,13 +7106,47 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
||||
}
|
||||
const existingExecutionWorkspace =
|
||||
issueRef?.executionWorkspaceId ? await executionWorkspacesSvc.getById(issueRef.executionWorkspaceId) : null;
|
||||
const shouldReuseExisting =
|
||||
const requestedShouldReuseExisting =
|
||||
issueRef?.executionWorkspacePreference === "reuse_existing" &&
|
||||
existingExecutionWorkspace !== null &&
|
||||
existingExecutionWorkspace.status !== "archived";
|
||||
const reusableExecutionWorkspaceConfig = shouldReuseExisting
|
||||
const requestedReusableExecutionWorkspaceConfig = requestedShouldReuseExisting
|
||||
? existingExecutionWorkspace?.config ?? null
|
||||
: null;
|
||||
const defaultEnvironment = await environmentsSvc.ensureLocalEnvironment(agent.companyId);
|
||||
const environmentResolution = resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: projectExecutionWorkspacePolicy,
|
||||
issueSettings: issueExecutionWorkspaceSettings,
|
||||
workspaceConfig: requestedReusableExecutionWorkspaceConfig,
|
||||
agentDefaultEnvironmentId: agent.defaultEnvironmentId,
|
||||
defaultEnvironmentId: defaultEnvironment.id,
|
||||
});
|
||||
// PAPA-380 / PAPA-431: when the resolver refuses silent reuse of the
|
||||
// persisted workspace environment, also force a fresh workspace
|
||||
// realization on the assignee's intended env. Reusing the on-disk
|
||||
// workspace while swapping the env underneath it would mismatch the cwd's
|
||||
// runtime expectations (e.g. an SSH-targeted worktree running on the
|
||||
// local default driver).
|
||||
if (environmentResolution.conflict) {
|
||||
logger.warn(
|
||||
{
|
||||
runId: run.id,
|
||||
issueId,
|
||||
agentId: agent.id,
|
||||
adapterType: agent.adapterType,
|
||||
existingExecutionWorkspaceId: existingExecutionWorkspace?.id ?? null,
|
||||
workspaceEnvironmentId: environmentResolution.conflict.workspaceEnvironmentId,
|
||||
assigneeIntendedEnvironmentId:
|
||||
environmentResolution.conflict.assigneeIntendedEnvironmentId,
|
||||
assigneeIntendedSource: environmentResolution.conflict.assigneeIntendedSource,
|
||||
},
|
||||
"Refusing silent reuse of execution workspace whose environment does not match the assignee's intended environment; forcing fresh realization",
|
||||
);
|
||||
}
|
||||
const shouldReuseExisting = requestedShouldReuseExisting && !environmentResolution.conflict;
|
||||
const reusableExecutionWorkspaceConfig = shouldReuseExisting
|
||||
? requestedReusableExecutionWorkspaceConfig
|
||||
: null;
|
||||
const persistedExecutionWorkspaceMode = shouldReuseExisting && existingExecutionWorkspace
|
||||
? issueExecutionWorkspaceModeForPersistedWorkspace(existingExecutionWorkspace.mode)
|
||||
: null;
|
||||
@@ -7122,14 +7156,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
|
||||
persistedExecutionWorkspaceMode === "agent_default"
|
||||
? persistedExecutionWorkspaceMode
|
||||
: requestedExecutionWorkspaceMode;
|
||||
const defaultEnvironment = await environmentsSvc.ensureLocalEnvironment(agent.companyId);
|
||||
const selectedEnvironmentId = resolveExecutionWorkspaceEnvironmentId({
|
||||
projectPolicy: projectExecutionWorkspacePolicy,
|
||||
issueSettings: issueExecutionWorkspaceSettings,
|
||||
workspaceConfig: reusableExecutionWorkspaceConfig,
|
||||
agentDefaultEnvironmentId: agent.defaultEnvironmentId,
|
||||
defaultEnvironmentId: defaultEnvironment.id,
|
||||
});
|
||||
const selectedEnvironmentId = environmentResolution.environmentId;
|
||||
const workspaceManagedConfig = shouldReuseExisting
|
||||
? { ...config }
|
||||
: buildExecutionWorkspaceAdapterConfig({
|
||||
|
||||
Reference in new issue
Block a user