mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-15 18:30:39 +09:00
Honor reuse-existing preference and assignee default environment in issue runs (#5139)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Agents run inside execution workspaces (a per-issue cwd + env), and an issue > can prefer to reuse an existing workspace or get a fresh one each time > - The heartbeat service was reading the existing workspace's config to derive > environment selection regardless of whether the issue actually wanted to reuse > it. So fresh-run issues were inheriting stale config from a workspace that was > about to be discarded > - Separately, when an issue is assigned to an agent, the issue's execution > workspace settings weren't picking up the agent's `defaultEnvironmentId`, > even though the agent's choice is the natural default for that issue > - This PR makes both selection paths honor the obvious source of truth: > workspace config flows only when the issue actually wants `reuse_existing`, > and the assignee agent's default environment is applied at assignment time if > nothing else is set on the issue > - The benefit is that re-running a flaky issue picks up the right environment > instead of inheriting the previous run's config, and assigning an agent to an > issue does the obvious thing without operator intervention ## What Changed - `server/src/services/heartbeat.ts`: introduce `reusableExecutionWorkspaceConfig` that is non-null only when `shouldReuseExisting` is true. Both `resolveExecutionWorkspaceEnvironmentId(...)` and `applyPersistedExecutionWorkspaceConfig(...)` now read from it instead of unconditionally consulting `existingExecutionWorkspace?.config`. Fresh-run issues no longer inherit stale environment config from an in-flight workspace about to be discarded. - `server/src/services/issues.ts`: when an issue update sets a new `assigneeAgentId` and isolated workspaces are enabled, populate `executionWorkspaceSettings.environmentId` from the assignee agent's `defaultEnvironmentId` if the issue doesn't have an explicit `environmentId` set yet. - Tests added in `heartbeat-plugin-environment.test.ts` (~216 lines) and `issues-service.test.ts` (~85 lines) covering both paths. ## Verification - `pnpm --filter @paperclipai/server test -- heartbeat-plugin-environment issues-service` - Manual QA: assign an issue to an agent that has a non-default `defaultEnvironmentId`, confirm the issue's workspace settings now include that environment id without operator intervention. Trigger a rerun on an issue whose existing workspace points at a stale environment, confirm the rerun uses the freshly-resolved environment. ## Risks - Behavioural shift on assignment: previously assigning an agent didn't propagate the agent's default environment to the issue. Now it does. Callers that explicitly want the issue to keep its existing/null environment must set `executionWorkspaceSettings.environmentId` themselves; the new logic only fires when no explicit value is set. - Behavioural shift on rerun: stale workspace config is no longer applied to fresh runs. Operators who relied on this implicit inheritance may see different environment selection on the first rerun after deploy. Mitigation: the explicit isssue settings and project policy are still honored as before. ## Model Used - OpenAI GPT-5.4 (reasoning effort: high) via Codex CLI - Provider: OpenAI - Used to author the code changes in this PR ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots — N/A (no UI changes) - [ ] I have updated relevant documentation to reflect my changes — N/A - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
GitHub
parent
09eceb952a
commit
0e51fa2b0d
4 changed files with 700 additions and 14 deletions
|
|
@ -35,6 +35,7 @@ import type {
|
|||
} from "@paperclipai/shared";
|
||||
import { clampIssueRequestDepth, extractAgentMentionIds, extractProjectMentionIds, isUuidLike } from "@paperclipai/shared";
|
||||
import { conflict, notFound, unprocessable } from "../errors.js";
|
||||
import { parseObject } from "../adapters/utils.js";
|
||||
import {
|
||||
defaultIssueExecutionWorkspaceSettingsForProject,
|
||||
gateProjectExecutionWorkspacePolicy,
|
||||
|
|
@ -2733,24 +2734,68 @@ export function issueService(db: Db) {
|
|||
}
|
||||
}
|
||||
}
|
||||
// Cache the project policy lookup for this insert. Both the
|
||||
// default-settings block and the assignee-environment-promotion block
|
||||
// need the same row; without caching they'd issue two round-trips.
|
||||
let projectPolicyCached: ReturnType<typeof parseProjectExecutionWorkspacePolicy> | null = null;
|
||||
let projectPolicyLoaded = false;
|
||||
const loadProjectPolicyOnce = async () => {
|
||||
if (projectPolicyLoaded) return projectPolicyCached;
|
||||
projectPolicyLoaded = true;
|
||||
if (!issueData.projectId) return null;
|
||||
const projectRow = await tx
|
||||
.select({ executionWorkspacePolicy: projects.executionWorkspacePolicy })
|
||||
.from(projects)
|
||||
.where(and(eq(projects.id, issueData.projectId), eq(projects.companyId, companyId)))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
projectPolicyCached = parseProjectExecutionWorkspacePolicy(projectRow?.executionWorkspacePolicy);
|
||||
return projectPolicyCached;
|
||||
};
|
||||
|
||||
if (
|
||||
executionWorkspaceSettings == null &&
|
||||
executionWorkspaceId == null &&
|
||||
issueData.projectId
|
||||
) {
|
||||
const project = await tx
|
||||
.select({ executionWorkspacePolicy: projects.executionWorkspacePolicy })
|
||||
.from(projects)
|
||||
.where(and(eq(projects.id, issueData.projectId), eq(projects.companyId, companyId)))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
executionWorkspaceSettings =
|
||||
defaultIssueExecutionWorkspaceSettingsForProject(
|
||||
gateProjectExecutionWorkspacePolicy(
|
||||
parseProjectExecutionWorkspacePolicy(project?.executionWorkspacePolicy),
|
||||
await loadProjectPolicyOnce(),
|
||||
isolatedWorkspacesEnabled,
|
||||
),
|
||||
) as Record<string, unknown> | null;
|
||||
}
|
||||
if (data.assigneeAgentId && isolatedWorkspacesEnabled) {
|
||||
const currentWorkspaceSettings = executionWorkspaceSettings == null
|
||||
? {}
|
||||
: parseObject(executionWorkspaceSettings);
|
||||
const issueHasEnvironmentSelection =
|
||||
Object.prototype.hasOwnProperty.call(currentWorkspaceSettings, "environmentId");
|
||||
// Don't promote the assignee agent's defaultEnvironmentId if either
|
||||
// the issue or the project policy already specifies an environment.
|
||||
// resolveExecutionWorkspaceEnvironmentId treats issue settings as
|
||||
// higher priority than project policy, so promoting the agent's
|
||||
// default to issue settings would invert the documented priority
|
||||
// (project policy must win over agent default when explicitly set).
|
||||
let projectHasEnvironmentSelection = false;
|
||||
if (!issueHasEnvironmentSelection && issueData.projectId) {
|
||||
const projectPolicy = await loadProjectPolicyOnce();
|
||||
projectHasEnvironmentSelection = projectPolicy?.environmentId !== undefined;
|
||||
}
|
||||
if (!issueHasEnvironmentSelection && !projectHasEnvironmentSelection) {
|
||||
const assigneeAgent = await tx
|
||||
.select({ defaultEnvironmentId: agents.defaultEnvironmentId })
|
||||
.from(agents)
|
||||
.where(and(eq(agents.id, data.assigneeAgentId), eq(agents.companyId, companyId)))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (typeof assigneeAgent?.defaultEnvironmentId === "string" && assigneeAgent.defaultEnvironmentId.length > 0) {
|
||||
executionWorkspaceSettings = {
|
||||
...currentWorkspaceSettings,
|
||||
environmentId: assigneeAgent.defaultEnvironmentId,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!projectWorkspaceId && issueData.projectId) {
|
||||
const project = await tx
|
||||
.select({
|
||||
|
|
@ -2978,6 +3023,94 @@ export function issueService(db: Db) {
|
|||
issueData.projectId !== undefined ? issueData.projectId : existing.projectId,
|
||||
),
|
||||
]);
|
||||
|
||||
// Mirror the create() path: when the assignee changes to a non-null
|
||||
// agent, default the issue's executionWorkspaceSettings.environmentId
|
||||
// to the new agent's defaultEnvironmentId. Skip when:
|
||||
// - this update explicitly sets executionWorkspaceSettings.environmentId
|
||||
// (caller is making a deliberate override; respect it), OR
|
||||
// - the project policy already specifies an environmentId (project
|
||||
// policy must win over agent default per the documented priority
|
||||
// order in resolveExecutionWorkspaceEnvironmentId), OR
|
||||
// - the issue already has an environmentId that was *not* the prior
|
||||
// assignee's default (i.e., the operator set it explicitly in an
|
||||
// earlier update; preserve their choice). When the existing
|
||||
// environmentId matches the prior assignee's default, treat it as
|
||||
// auto-promoted and refresh it to the new assignee's default.
|
||||
const assigneeChanged =
|
||||
issueData.assigneeAgentId !== undefined &&
|
||||
issueData.assigneeAgentId !== null &&
|
||||
issueData.assigneeAgentId !== existing.assigneeAgentId;
|
||||
const explicitEnvInThisUpdate =
|
||||
issueData.executionWorkspaceSettings !== undefined &&
|
||||
Object.prototype.hasOwnProperty.call(
|
||||
parseObject(issueData.executionWorkspaceSettings),
|
||||
"environmentId",
|
||||
);
|
||||
if (assigneeChanged && isolatedWorkspacesEnabled && !explicitEnvInThisUpdate) {
|
||||
let projectHasEnvironmentSelection = false;
|
||||
if (nextProjectId) {
|
||||
const projectRow = await tx
|
||||
.select({ executionWorkspacePolicy: projects.executionWorkspacePolicy })
|
||||
.from(projects)
|
||||
.where(and(eq(projects.id, nextProjectId), eq(projects.companyId, existing.companyId)))
|
||||
.then((rows: Array<{ executionWorkspacePolicy: unknown }>) => rows[0] ?? null);
|
||||
const projectPolicy = parseProjectExecutionWorkspacePolicy(projectRow?.executionWorkspacePolicy);
|
||||
projectHasEnvironmentSelection = projectPolicy?.environmentId !== undefined;
|
||||
}
|
||||
if (!projectHasEnvironmentSelection) {
|
||||
const baseSettings = nextExecutionWorkspaceSettings == null
|
||||
? {}
|
||||
: parseObject(nextExecutionWorkspaceSettings);
|
||||
const existingEnvId = typeof baseSettings.environmentId === "string"
|
||||
? baseSettings.environmentId
|
||||
: null;
|
||||
|
||||
// Look up both the prior assignee (to detect auto-promoted env)
|
||||
// and the new assignee in a single query.
|
||||
type AgentRow = { id: string; defaultEnvironmentId: string | null };
|
||||
const agentRows: AgentRow[] = await tx
|
||||
.select({ id: agents.id, defaultEnvironmentId: agents.defaultEnvironmentId })
|
||||
.from(agents)
|
||||
.where(
|
||||
and(
|
||||
eq(agents.companyId, existing.companyId),
|
||||
inArray(
|
||||
agents.id,
|
||||
[issueData.assigneeAgentId!, existing.assigneeAgentId].filter(
|
||||
(value): value is string => typeof value === "string",
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
const newAssignee = agentRows.find((row: AgentRow) => row.id === issueData.assigneeAgentId);
|
||||
const previousAssignee = existing.assigneeAgentId
|
||||
? agentRows.find((row: AgentRow) => row.id === existing.assigneeAgentId)
|
||||
: null;
|
||||
|
||||
const newDefaultEnvId =
|
||||
typeof newAssignee?.defaultEnvironmentId === "string" && newAssignee.defaultEnvironmentId.length > 0
|
||||
? newAssignee.defaultEnvironmentId
|
||||
: null;
|
||||
const previousDefaultEnvId =
|
||||
typeof previousAssignee?.defaultEnvironmentId === "string" && previousAssignee.defaultEnvironmentId.length > 0
|
||||
? previousAssignee.defaultEnvironmentId
|
||||
: null;
|
||||
|
||||
const existingEnvWasAutoPromoted =
|
||||
existingEnvId === null ||
|
||||
(previousDefaultEnvId !== null && existingEnvId === previousDefaultEnvId);
|
||||
|
||||
if (newDefaultEnvId && existingEnvWasAutoPromoted) {
|
||||
patch.executionWorkspaceSettings = {
|
||||
...baseSettings,
|
||||
environmentId: newDefaultEnvId,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
patch.goalId = resolveNextIssueGoalId({
|
||||
currentProjectId: existing.projectId,
|
||||
currentGoalId: existing.goalId,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue