mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-02 02:07:25 +08:00
fix: retain project defaults in partial workspace overrides (#14502)
## Thinking Path > - Paperclip manages AI agents and their work. > - Project workspace policies define how isolated worktrees are set up. > - Tasks can override a branch without providing every setup field. > - The resolver currently replaces the entire project strategy with that partial override. > - Losing an explicit setup command can run the repository fallback script and block the task. > - This pull request keeps enabled project defaults when the task uses the same strategy type. ## Linked Issues or Issue Description **What happened?** A project uses `git_worktree` with `provisionCommand: "true"`. A task overrides only `baseRef`. The resolver drops the command. Worktree creation then invokes `scripts/provision-worktree.sh`, which can fail because its required setup is absent. **Expected behavior** A branch override keeps the project's provision, runtime provision, and teardown commands unless the task explicitly overrides them. A different strategy type must not inherit those commands. **Steps to reproduce** Configure the project with an enabled `git_worktree` strategy and `provisionCommand: "true"`. Give the task an isolated workspace with a `git_worktree` strategy and a different `baseRef`. Add a failing repository fallback provisioner. Before this change, worktree creation invokes that script. After this change, it uses the project's explicit command and succeeds. Related: #4968 concerns agent strategy and working-directory fallback. #13903 concerns gated API fields and reusable-workspace updates. #11091 concerns provision hooks on workspace reuse. None fixes partial task overrides discarding project defaults. ## What Changed - Merge a partial task strategy over the enabled project's strategy only when their types match. - Preserve explicit null values when parsing nullable strategy fields, so they can clear project values. - Keep explicit empty-string overrides and agent fallback behavior. - Exclude disabled project strategies and avoid an inherited branch template when a task pins an existing branch. - Add policy regression coverage and a real Git worktree test with a failing fallback script. - Document inheritance, explicit clearing, and no-op provisioning in the development guide. ## Verification - Policy regression: eight failures before the fix; all 41 policy tests pass after it. - Real worktree regression: passes and creates a worktree using the task's base branch without invoking the failing fallback provisioner. - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - `pnpm test:run`: general-server phase completed with 13,873 passed, 86 skipped, and 14 failures in unchanged macOS skills-cache and Git long-path tests. The same failures reproduce on unmodified base code. The command stops at that phase, so no full local pass is claimed. - CI initially failed the existing Telegram subscription recovery test on a 15-second timeout. The separate fix and investigation are in #14501. A serialized job also lost its runner; GitHub reported lost communication, and that job was rerun without source changes. All 52 final-commit checks pass, with two intentional skips. Greptile is 5/5, with no unresolved comments or merge conflicts. The chat shard passed on one unchanged rerun. The timeout cause remains unproven; #14501 adds phase diagnostics for a recurrence. ## Risks Tasks that specify a partial strategy now retain the project's omitted fields, including setup and teardown hooks. This is the intended behavior change. Inheritance requires an enabled project policy and matching strategy types. Explicit task values still win. Null and empty commands restore existing runtime defaults; they do not guarantee that no script runs. Use `"true"` for an explicit no-op provision command. No migration, live configuration change, or task replay is included. ## Model Used OpenAI GPT-6 (Codex), with reasoning, terminal tools, and code execution. The context window size is not exposed in this session. ## 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 searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes:` / `Closes` / `Refs` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub references) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run focused tests locally and they pass; full-suite status is recorded above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin Foley
@@ -992,6 +992,8 @@ eval "$(npx paperclipai worktree env)"
|
||||
|
||||
For project execution worktrees, Paperclip can also run a project-defined provision command after it creates or reuses an isolated git worktree. Configure this on the project's execution workspace policy (`workspaceStrategy.provisionCommand`). The command runs inside the derived worktree and receives `PAPERCLIP_WORKSPACE_*`, `PAPERCLIP_PROJECT_ID`, `PAPERCLIP_AGENT_ID`, and `PAPERCLIP_ISSUE_*` environment variables so each repo can bootstrap itself however it wants.
|
||||
|
||||
An issue's partial `workspaceStrategy` inherits omitted fields from the enabled project's strategy when both use the same type. For example, an issue can override `baseRef` without losing the project's provision, runtime provision, or teardown commands. An explicit value, including `null` or an empty string, replaces the project value. Clearing a command restores the runtime's usual default behavior; use `provisionCommand: "true"` for an explicit no-op. A different strategy type or a disabled project policy does not supply these defaults. An issue's `existingBranch` pin also excludes the project's `branchTemplate`.
|
||||
|
||||
An issue can pin its isolated worktree to an exact pre-existing branch instead of a template-derived one — the contract PR-preparation tasks use. Set the issue's `executionWorkspaceSettings` to `{ "mode": "isolated_workspace", "workspaceStrategy": { "type": "git_worktree", "existingBranch": "<branch>" } }`. The validator requires isolated mode plus a `git_worktree` strategy and rejects `branchTemplate` alongside `existingBranch`. At dispatch the runtime attaches (never creates, renames, fast-forwards, or resets) that branch: it reuses a registered worktree that already has the branch checked out (including legacy `.worktrees/` paths), otherwise it attaches the branch under the managed worktree parent. A missing branch, an occupied worktree path on another branch, or a non-worktree strategy fails closed with a `workspace_validation_failed` error instead of falling back to the shared checkout or a derived branch, and an inherited `reuse_existing` workspace binding on a different branch is ignored in favor of realizing the pinned branch.
|
||||
|
||||
Heavier setup that is only needed by a managed runtime service can use `workspaceStrategy.runtimeProvisionCommand`. Paperclip runs this command lazily before spawning the first service in a start batch, serializes concurrent provisioning for the same workspace, and records the attempt as `workspace_runtime_provision`. The command receives the same workspace environment as `provisionCommand` and should be idempotent because later service-start batches invoke it again.
|
||||
|
||||
@@ -300,6 +300,107 @@ describe("execution workspace policy helpers", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("partial issue workspace strategies", () => {
|
||||
const projectStrategy = {
|
||||
type: "git_worktree" as const,
|
||||
baseRef: "origin/main",
|
||||
branchTemplate: "{{issue.identifier}}-{{slug}}",
|
||||
worktreeParentDir: ".paperclip/worktrees",
|
||||
provisionCommand: "true",
|
||||
runtimeProvisionCommand: "npm run setup:runtime",
|
||||
teardownCommand: "npm run teardown",
|
||||
};
|
||||
|
||||
function resolveStrategy(
|
||||
strategy: Record<string, unknown>,
|
||||
enabled = true,
|
||||
) {
|
||||
return buildExecutionWorkspaceAdapterConfig({
|
||||
agentConfig: { workspaceStrategy: { type: "git_worktree", provisionCommand: "agent-setup" } },
|
||||
projectPolicy: parseProjectExecutionWorkspacePolicy({
|
||||
enabled,
|
||||
defaultMode: "isolated_workspace",
|
||||
workspaceStrategy: projectStrategy,
|
||||
}),
|
||||
issueSettings: parseIssueExecutionWorkspaceSettings({
|
||||
mode: "isolated_workspace",
|
||||
workspaceStrategy: strategy,
|
||||
}),
|
||||
mode: "isolated_workspace",
|
||||
legacyUseProjectWorkspace: null,
|
||||
}).workspaceStrategy;
|
||||
}
|
||||
|
||||
it("retains project hooks when an issue changes only its base branch", () => {
|
||||
expect(resolveStrategy({ type: "git_worktree", baseRef: "origin/release" })).toEqual({
|
||||
...projectStrategy,
|
||||
baseRef: "origin/release",
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["npm run issue-setup", "", null])("honors an explicit provisioning override of %j", (provisionCommand) => {
|
||||
expect(resolveStrategy({ type: "git_worktree", provisionCommand })).toEqual({
|
||||
...projectStrategy,
|
||||
provisionCommand,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves explicit null clears through persisted JSON parsing", () => {
|
||||
const strategy = {
|
||||
type: "git_worktree",
|
||||
baseRef: null,
|
||||
branchTemplate: null,
|
||||
worktreeParentDir: null,
|
||||
provisionCommand: null,
|
||||
runtimeProvisionCommand: null,
|
||||
teardownCommand: null,
|
||||
};
|
||||
expect(resolveStrategy(strategy)).toEqual(strategy);
|
||||
});
|
||||
|
||||
it.each(["cloud_sandbox", "adapter_managed", "project_primary"])("does not carry project hooks into %s", (type) => {
|
||||
expect(resolveStrategy({ type })).toEqual({ type });
|
||||
});
|
||||
|
||||
it("does not inherit a disabled project strategy", () => {
|
||||
expect(resolveStrategy({ type: "git_worktree", baseRef: "origin/release" }, false)).toEqual({
|
||||
type: "git_worktree",
|
||||
baseRef: "origin/release",
|
||||
});
|
||||
expect(resolveStrategy({}, false)).toEqual({
|
||||
type: "git_worktree",
|
||||
provisionCommand: "agent-setup",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps project hooks for an exact branch pin without inheriting a branch template", () => {
|
||||
const resolved = resolveStrategy({ type: "git_worktree", existingBranch: "fix/existing" });
|
||||
expect(resolved).toEqual({
|
||||
...projectStrategy,
|
||||
branchTemplate: undefined,
|
||||
existingBranch: "fix/existing",
|
||||
});
|
||||
expect(issueExecutionWorkspaceSettingsSchema.safeParse({
|
||||
mode: "isolated_workspace",
|
||||
workspaceStrategy: resolved,
|
||||
}).success).toBe(true);
|
||||
});
|
||||
|
||||
it("does not mutate the project or issue strategy", () => {
|
||||
const issueStrategy = { type: "git_worktree" as const, baseRef: "origin/release" };
|
||||
const result = buildExecutionWorkspaceAdapterConfig({
|
||||
agentConfig: {},
|
||||
projectPolicy: { enabled: true, workspaceStrategy: Object.freeze({ ...projectStrategy }) },
|
||||
issueSettings: { workspaceStrategy: Object.freeze(issueStrategy) },
|
||||
mode: "isolated_workspace",
|
||||
legacyUseProjectWorkspace: null,
|
||||
});
|
||||
expect(result.workspaceStrategy).not.toBe(issueStrategy);
|
||||
expect(issueStrategy).toEqual({ type: "git_worktree", baseRef: "origin/release" });
|
||||
expect(projectStrategy.baseRef).toBe("origin/main");
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves project authorization policy for trust-preset resolution", () => {
|
||||
expect(parseProjectExecutionWorkspacePolicy({
|
||||
enabled: true,
|
||||
|
||||
@@ -24,6 +24,11 @@ import {
|
||||
workspaceRuntimeServices,
|
||||
} from "@paperclipai/db";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
buildExecutionWorkspaceAdapterConfig,
|
||||
parseIssueExecutionWorkspaceSettings,
|
||||
parseProjectExecutionWorkspacePolicy,
|
||||
} from "../services/execution-workspace-policy.ts";
|
||||
import {
|
||||
buildWorkspaceRuntimeDesiredStatePatch,
|
||||
cleanupExecutionWorkspaceArtifacts,
|
||||
@@ -869,6 +874,52 @@ describe("realizeExecutionWorkspace", () => {
|
||||
expect(second.branchName).toBe(first.branchName);
|
||||
});
|
||||
|
||||
it("retains the project provision command when an issue overrides its base branch", async () => {
|
||||
const repoRoot = await createTempRepo();
|
||||
await fs.mkdir(path.join(repoRoot, "scripts"));
|
||||
await fs.writeFile(
|
||||
path.join(repoRoot, "scripts", "provision-worktree.sh"),
|
||||
"#!/usr/bin/env bash\necho 'Unexpected repository provision fallback' >&2\nexit 1\n",
|
||||
);
|
||||
await runGit(repoRoot, ["add", "scripts/provision-worktree.sh"]);
|
||||
await runGit(repoRoot, ["commit", "-m", "Add fallback provisioner"]);
|
||||
await runGit(repoRoot, ["branch", "release"]);
|
||||
const config = buildExecutionWorkspaceAdapterConfig({
|
||||
agentConfig: {},
|
||||
projectPolicy: parseProjectExecutionWorkspacePolicy({
|
||||
enabled: true,
|
||||
defaultMode: "isolated_workspace",
|
||||
workspaceStrategy: { type: "git_worktree", baseRef: "main", provisionCommand: "true" },
|
||||
}),
|
||||
issueSettings: parseIssueExecutionWorkspaceSettings({
|
||||
mode: "isolated_workspace",
|
||||
workspaceStrategy: { type: "git_worktree", baseRef: "release" },
|
||||
}),
|
||||
mode: "isolated_workspace",
|
||||
legacyUseProjectWorkspace: null,
|
||||
});
|
||||
try {
|
||||
const workspace = await realizeExecutionWorkspace({
|
||||
base: {
|
||||
baseCwd: repoRoot,
|
||||
source: "project_primary",
|
||||
projectId: "project-1",
|
||||
workspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
repoRef: "HEAD",
|
||||
},
|
||||
config,
|
||||
issue: { id: "issue-1", identifier: "TEST-1", title: "Keep project setup" },
|
||||
agent: { id: "agent-1", name: "Test agent", companyId: "company-1" },
|
||||
});
|
||||
expect(workspace.created).toBe(true);
|
||||
expect(workspace.baseRefSha).toBe(await readGit(repoRoot, ["rev-parse", "release"]));
|
||||
await expect(fs.stat(path.join(workspace.cwd, ".git"))).resolves.toBeTruthy();
|
||||
} finally {
|
||||
await fs.rm(repoRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("defaults the repo-provided worktree provisioner for git worktree strategies", async () => {
|
||||
const repoRoot = await createTempRepo();
|
||||
await fs.mkdir(path.join(repoRoot, "scripts"), { recursive: true });
|
||||
|
||||
@@ -38,17 +38,17 @@ function parseExecutionWorkspaceStrategy(raw: unknown): ExecutionWorkspaceStrate
|
||||
}
|
||||
return {
|
||||
type,
|
||||
...(typeof parsed.baseRef === "string" ? { baseRef: parsed.baseRef } : {}),
|
||||
...(typeof parsed.branchTemplate === "string" ? { branchTemplate: parsed.branchTemplate } : {}),
|
||||
...(typeof parsed.baseRef === "string" || parsed.baseRef === null ? { baseRef: parsed.baseRef } : {}),
|
||||
...(typeof parsed.branchTemplate === "string" || parsed.branchTemplate === null ? { branchTemplate: parsed.branchTemplate } : {}),
|
||||
...(typeof parsed.existingBranch === "string" && parsed.existingBranch.trim().length > 0
|
||||
? { existingBranch: parsed.existingBranch.trim() }
|
||||
: {}),
|
||||
...(typeof parsed.worktreeParentDir === "string" ? { worktreeParentDir: parsed.worktreeParentDir } : {}),
|
||||
...(typeof parsed.provisionCommand === "string" ? { provisionCommand: parsed.provisionCommand } : {}),
|
||||
...(typeof parsed.runtimeProvisionCommand === "string"
|
||||
...(typeof parsed.worktreeParentDir === "string" || parsed.worktreeParentDir === null ? { worktreeParentDir: parsed.worktreeParentDir } : {}),
|
||||
...(typeof parsed.provisionCommand === "string" || parsed.provisionCommand === null ? { provisionCommand: parsed.provisionCommand } : {}),
|
||||
...(typeof parsed.runtimeProvisionCommand === "string" || parsed.runtimeProvisionCommand === null
|
||||
? { runtimeProvisionCommand: parsed.runtimeProvisionCommand }
|
||||
: {}),
|
||||
...(typeof parsed.teardownCommand === "string" ? { teardownCommand: parsed.teardownCommand } : {}),
|
||||
...(typeof parsed.teardownCommand === "string" || parsed.teardownCommand === null ? { teardownCommand: parsed.teardownCommand } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -418,11 +418,18 @@ export function buildExecutionWorkspaceAdapterConfig(input: {
|
||||
|
||||
if (hasWorkspaceControl) {
|
||||
if (input.mode === "isolated_workspace") {
|
||||
const strategy =
|
||||
input.issueSettings?.workspaceStrategy ??
|
||||
input.projectPolicy?.workspaceStrategy ??
|
||||
const projectStrategy = projectHasPolicy ? input.projectPolicy?.workspaceStrategy : undefined;
|
||||
const issueStrategy = input.issueSettings?.workspaceStrategy;
|
||||
// An issue that changes its branch still needs the project's setup hooks.
|
||||
// Do not carry those defaults into a different execution strategy.
|
||||
const strategy = issueStrategy && projectStrategy?.type === issueStrategy.type
|
||||
? { ...projectStrategy, ...issueStrategy }
|
||||
: issueStrategy ?? projectStrategy ??
|
||||
parseExecutionWorkspaceStrategy(nextConfig.workspaceStrategy) ??
|
||||
({ type: "git_worktree" } satisfies ExecutionWorkspaceStrategy);
|
||||
if (issueStrategy?.existingBranch && issueStrategy.branchTemplate === undefined && strategy !== issueStrategy) {
|
||||
delete strategy.branchTemplate;
|
||||
}
|
||||
nextConfig.workspaceStrategy = strategy as unknown as Record<string, unknown>;
|
||||
} else {
|
||||
delete nextConfig.workspaceStrategy;
|
||||
|
||||
Reference in New Issue
Block a user