From 0dc8d80eea0f69ec4ca8894fcccc87cc53a8d205 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 30 Sep 2026 18:35:33 -0700 Subject: [PATCH] Clarify worktree seed source setup failures (#14795) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Managed worktrees can prepare an isolated Paperclip development instance. > - The built-in provisioner requires a canonical registered seed source config. > - A missing source currently has the same message as a rejected symlink or non-regular file. > - This pull request separates those messages and names the supported setup choices. > - Operators can choose the intended setup without changing the source-validation guards. ## Linked Issues or Issue Description **What happened?** A plain repository checkout can select the control-plane instance as its seed source. If that instance runs with environment-only configuration, the source config file can be unavailable. The provisioner stops with a message that also covers noncanonical files and gives no repair guidance. **Expected behavior** The error should identify the selected source and distinguish an unavailable prerequisite from a rejected file. It should explain that a seeded development instance needs a canonical registered source. It should describe the explicit no-op only for a checkout-only worktree. **Steps to reproduce** 1. Use a plain base checkout with no repository-local config. 2. Leave the control-plane instance config file absent. 3. Run the built-in worktree provisioner against an isolated checkout. **Paperclip version or commit** Base commit `c8f874311c`. **Deployment mode** Managed local worktrees, including servers configured only through environment variables. Related: #11733 adds deeper source-readiness checks. #11735 changes runtime and seed lifecycle handling. This change only improves the existing shell guard's diagnostics. ## What Changed - Distinguish unavailable source configs from symlinks and non-regular files. - Identify whether the selected source belongs to the base workspace or control-plane instance. - Explain seeded-instance prerequisites and the explicit checkout-only setup choice. - Verify failure still precedes target-state creation and CLI invocation. - Document the setup choice and its runtime-readiness limit. ## Verification - `node --test scripts/__tests__/provision-worktree-self-heal.test.mjs`: 21 passed; one platform-gated test skipped because macOS lacks `flock`. - `bash -n scripts/provision-worktree.sh` and `git diff --check`: passed. - `pnpm -r typecheck`: passed. - `pnpm exec vitest run server/src/__tests__/ai-connections.test.ts`: 50 passed after running the installed PostgreSQL package's own symlink hydration script in this worktree. - `pnpm test:run`: attempted, then stopped after unrelated database suites failed at startup. The offline install had omitted PostgreSQL native library symlinks. The focused database rerun above verifies the local repair; the complete suite is delegated to CI. - `pnpm build`: passed. - [Required PR CI](https://github.com/paperclipai/paperclip/actions/runs/36797650741) passed on `0a3ba63e12`: 50 successful checks and two intentional Storybook skips. Greptile scored that exact commit 5/5; there are zero unresolved review threads and no merge conflicts. ## Risks - Diagnostics only. This does not supply a source config or repair an existing blocked task. - The failure predicates and exit status stay unchanged. Symlink and non-regular-file errors do not recommend skipping setup. - The checkout-only no-op requires an explicit policy choice. It does not grant runtime or seed readiness. - No schema, migration, tenant policy, deployment, or Sentry reporting change. ## Model Used OpenAI GPT-6-based Codex, with reasoning, shell tools, and code execution. The exact serving model ID and context-window size are not exposed by 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 `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [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 --- doc/DEVELOPING.md | 1 + .../provision-worktree-self-heal.test.mjs | 44 +++++++++++++++++-- scripts/provision-worktree.sh | 11 ++++- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 7fdd364f4a..5d37d441c8 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -824,6 +824,7 @@ The default `worktree init` still seeds eagerly. A lean worktree (created withou - `pnpm paperclipai worktree ensure-seeded` performs the deferred seed **exactly once**. It is lock-guarded and idempotent: only a complete `verified` manifest short-circuits it, so it is safe to call repeatedly and from concurrent processes. Managed workspaces derive the source from the control-plane-provided base project workspace when it carries its own `.paperclip/config.json`, and otherwise from the control plane's own registered instance config; either way the workspace's manifest never selects it. Manual worktrees must pass `--from-config`. - `paperclipai run` calls `ensureWorktreeSeeded` automatically before doctor/boot. Managed runs transparently seed a lean worktree from their registered base workspace; an unmanaged lean worktree must first run `worktree ensure-seeded --from-config `. - Managed Paperclip git worktrees default to the repository's `scripts/provision-worktree.sh` when the strategy omits `provisionCommand`, so the isolated config and pending manifest cannot be silently skipped. Runtime startup also runs `scripts/provision-worktree-runtime.sh` automatically when no explicit runtime provision command is configured and the manifest is not verified. Explicitly configured provision commands still take precedence. +- If the built-in provisioner reports an unavailable seed source config, decide whether the task needs a seeded development instance or only an isolated checkout. A seeded instance needs a canonical config from the registered base workspace or control-plane instance; environment-only server configuration does not provide that file. For a checkout-only worktree, explicitly set the `git_worktree` strategy's `provisionCommand` to `"true"`. This skips setup and does not establish runtime or seed readiness. Repair rejected symlinks or non-regular source files instead of treating those validation failures as a missing prerequisite. - The built-in deferred seed is recorded as its own terminal `workspace_seed` operation. A zero exit code is not enough for success: the operation succeeds only when `.paperclip/seed-manifest.json` contains complete verified evidence; failed, missing, or malformed manifests produce a failed operation with the seed phase in metadata. - Worktrees created before lazy seeding shipped may have neither marker. Paperclip adopts them only after their configured database proves a compatible migration journal and the core Paperclip schema; otherwise managed startup creates a pending manifest and performs the normal verified seed. Manual markerless worktrees must provide `--from-config` so the source remains explicit. diff --git a/scripts/__tests__/provision-worktree-self-heal.test.mjs b/scripts/__tests__/provision-worktree-self-heal.test.mjs index 943a15f48b..bff638cdc4 100644 --- a/scripts/__tests__/provision-worktree-self-heal.test.mjs +++ b/scripts/__tests__/provision-worktree-self-heal.test.mjs @@ -102,11 +102,12 @@ process.exit(0); return baseCwd; } -function runProvision(baseCwd, { pathPrefix, setupWorktree, existingWorktree } = {}) { +function runProvision(baseCwd, { pathPrefix, setupWorktree, setupInstance, existingWorktree } = {}) { const worktreeCwd = existingWorktree ?? makeTempDir("paperclip-provision-worktree-"); setupWorktree?.(worktreeCwd); const worktreesHome = makeTempDir("paperclip-provision-home-"); const paperclipHome = makeInstanceHome(); + setupInstance?.(paperclipHome); const result = spawnSync("bash", [script], { cwd: worktreeCwd, encoding: "utf8", @@ -182,15 +183,52 @@ test("uses the base CLI when its import graph boots", () => { ); }); +test("explains unavailable instance source config without creating target state", () => { + const baseCwd = makeBaseWorkspace({ helpExit: 0, initExit: 0 }); + const { result, worktreeCwd } = runProvision(baseCwd, { + setupInstance: (home) => fs.rmSync(path.join(home, "instances", "default", "config.json")), + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /config is unavailable \(control-plane instance\)/); + assert.match(result.stderr, /For a seeded development instance, configure a canonical config/); + assert.match(result.stderr, /Only for a checkout-only worktree/); + assert.match(result.stderr, /workspaceStrategy\.provisionCommand to "true"/); + assert.match(result.stderr, /does not prepare a development runtime/); + assert.equal(fs.existsSync(path.join(worktreeCwd, ".paperclip")), false); + assert.deepEqual(readCliInvocations(baseCwd), []); +}); + test("rejects a dangling base workspace config symlink instead of falling back", () => { const baseCwd = makeBaseWorkspace({ helpExit: 0, initExit: 0 }); fs.mkdirSync(path.join(baseCwd, ".paperclip"), { recursive: true }); fs.symlinkSync(path.join(baseCwd, "absent.json"), path.join(baseCwd, ".paperclip", "config.json")); - const { result } = runProvision(baseCwd); + const { result, worktreeCwd } = runProvision(baseCwd); assert.notEqual(result.status, 0); - assert.match(result.stderr, /is missing or is not a canonical file/); + assert.match(result.stderr, /is not a canonical file \(base project workspace\)/); + assert.doesNotMatch(result.stderr, /checkout-only|provisionCommand/); + assert.equal(fs.existsSync(path.join(worktreeCwd, ".paperclip")), false); + assert.deepEqual(readCliInvocations(baseCwd), []); +}); + +test("rejects a non-regular instance config without suggesting a setup bypass", () => { + const baseCwd = makeBaseWorkspace({ helpExit: 0, initExit: 0 }); + const { result, worktreeCwd } = runProvision(baseCwd, { + setupInstance: (home) => { + const configPath = path.join(home, "instances", "default", "config.json"); + fs.rmSync(configPath); + fs.mkdirSync(configPath); + }, + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /is not a canonical file \(control-plane instance\)/); + assert.match(result.stderr, /Repair the registered source path/); + assert.doesNotMatch(result.stderr, /checkout-only|provisionCommand/); + assert.equal(fs.existsSync(path.join(worktreeCwd, ".paperclip")), false); + assert.deepEqual(readCliInvocations(baseCwd), []); }); test("rejects a dangling base workspace .paperclip symlink instead of falling back", () => { diff --git a/scripts/provision-worktree.sh b/scripts/provision-worktree.sh index af7c61b1a8..2418fa3bb0 100644 --- a/scripts/provision-worktree.sh +++ b/scripts/provision-worktree.sh @@ -48,14 +48,23 @@ if [[ -L "$canonical_base_cwd/.paperclip" && ! -d "$canonical_base_cwd/.papercli exit 1 fi source_config_path="$canonical_base_cwd/.paperclip/config.json" +source_config_origin="base project workspace" if [[ ! -e "$source_config_path" && ! -L "$source_config_path" ]]; then # A base workspace that is a plain checkout carries no instance config of its own. # Fall back to the control plane's own registered instance config, which is process # state this workspace cannot rewrite. source_config_path="${PAPERCLIP_CONFIG:-$paperclip_home/instances/$paperclip_instance_id/config.json}" + source_config_origin="control-plane instance" fi if [[ ! -f "$source_config_path" || -L "$source_config_path" ]]; then - echo "Registered Paperclip seed source config is missing or is not a canonical file: $source_config_path" >&2 + if [[ ! -e "$source_config_path" && ! -L "$source_config_path" ]]; then + echo "Registered Paperclip seed source config is unavailable ($source_config_origin): $source_config_path" >&2 + echo "For a seeded development instance, configure a canonical config for that registered source before retrying." >&2 + echo 'Only for a checkout-only worktree, explicitly set workspaceStrategy.provisionCommand to "true". This skips setup; it does not prepare a development runtime.' >&2 + else + echo "Registered Paperclip seed source config is not a canonical file ($source_config_origin): $source_config_path" >&2 + echo "Repair the registered source path; symlinks and non-regular files are not accepted." >&2 + fi exit 1 fi canonical_source_dir="$(cd "$(dirname "$source_config_path")" && pwd -P)"