fix(heartbeat): resolve repo-relative project workspace cwd inside its checkout (BLO-25415)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agents run against project workspaces; before a run starts, the heartbeat resolver must "realize" the workspace the issue targets into a real directory on disk
• A git_repo workspace may declare a repo-relative cwd such as packages/iwa, meaning "this subdirectory of my repo"
• The resolver consumed workspace.cwd raw and passed it to fs.stat(), which resolves a relative path against the API process's own working directory — so the stat could never succeed, and the non-empty cwd also caused the managed-clone branch to be skipped, so the repo was never checked out either
• Every run targeting such a workspace died preferred_workspace_unrealizable before any adapter started, stranding 6+ non-terminal issues across 2 workspaces
• The refusal names a path rather than a cause, so it reads as "missing checkout" — BLO-24751 hand-cloned the repo, closed done, and the next run failed byte-identically 71 seconds later
• This pull request makes a relative cwd mean what it evidently intended: realize the managed checkout first, then join the subpath inside it, refusing traversal that escapes the checkout root
• The benefit is that this class of stranding is fixed for every affected workspace with no manual provisioning step, including one that has no checkout on disk at all

Linked Issues or Issue Description

Refs BLO-25415 (Paperclip issue tracker), BLO-24751, BLO-18767.

Problem: A git_repo project workspace can declare a repo-relative cwd such as packages/iwa. The workspace resolver in heartbeat.ts consumed workspace.cwd raw and handed it to fs.stat(), which resolves a relative path against the API process's own working directory. That never matches, so the run failed preferred_workspace_unrealizable.

Expected behavior: A repo-relative cwd resolves to <managed checkout>/<cwd>, cloning the repo if it is absent.

Steps to reproduce: Point an issue at a non-primary git_repo workspace whose cwd is a relative path (e.g. packages/iwa) and trigger a run. The run is refused before any adapter starts with preferred_workspace_unrealizable, quoting the bare relative path.

Deployment mode: on-prem k8s (paperclip namespace), CephFS-backed shared PVC.

Two things made this expensive to diagnose:

1. The repo was never cloned. The managed-checkout branch only ran when cwd was empty or the repo-only sentinel, so a workspace with a relative cwd skipped provisioning entirely.
2. The refusal names a path, not a cause — Selected project workspace path "packages/iwa" is not available yet. That reads as a missing checkout, so BLO-24751 hand-cloned the repo, closed the issue done, and the next run failed byte-identically 71 seconds later. The clone was real and correctly placed; it was simply never consulted.

What Changed

• resolveRepoRelativeWorkspaceCwd() — decides whether a workspace's cwd is a repo-relative subpath that must be joined onto its managed checkout. Only repo-backed workspaces qualify: repoUrl must be present, and explicitly non-repo sourceTypes (local_path, non_git_path, remote_managed) keep their prior meaning.
• resolveContainedWorkspaceSubpath() — joins the subpath onto the realized checkout and refuses anything that escapes the checkout root. Containment is enforced twice: lexically (against .. traversal and sibling-prefix paths) and again after fs.realpath, because the checkout's contents are repo-controlled and a repo carrying packages/iwa -> /etc would otherwise pass the lexical check and then be followed by fs.stat().
• The workspace realization loop now realizes the managed checkout before joining a relative subpath, instead of stat'ing the raw relative string.
• Absolute-cwd, empty-cwd, and repo-only-sentinel behaviour is unchanged.

Verification

Run locally against this branch rebased on master:

• heartbeat-workspace-session.test.ts + heartbeat-preferred-workspace-fail-loud.test.ts — 230 passed (230), 2 test files.
• npx tsc --noEmit -p server/tsconfig.json — exit 0, 0 errors.
• node ./scripts/check-commit-author-attribution.mjs --base <master> --head <head> — passes (✓ No commits in range carry the shared allyblockcast[bot] App identity).

New regression coverage in server/src/__tests__/heartbeat-workspace-session.test.ts: the join and normalization, checkout root, lexical traversal escape, sibling-prefix (/managed/repo-evil vs /managed/repo), escaping symlink, in-repo symlink, symlinked checkout root, relative local_path / non_git_path / remote_managed, the out-of-enum "git" source_type spelling, and repoUrl: null.

End-to-end signal: run 0e91c938-564f-4a34-b6e7-ff05a8b4236c against workspace fcd6d149 passed the workspace gate and produced a real run log (3103 bytes, adapter invoked), where the pre-fix run a791e306 failed preferred_workspace_unrealizable with no log at all.

Risks

Low-to-moderate, and narrowly scoped. The new behaviour only engages for a workspace whose cwd is non-absolute and which is repo-backed (repoUrl present, sourceType not one of the non-repo kinds). Fleet-wide that is 1 remaining row (4dac485f), audited after the repoUrl gate was added — every other workspace has a null or absolute cwd and takes an unchanged code path.

• Behavioural shift: a relative cwd previously always failed the realization stat. It now resolves. Any workspace relying on that failure would change behaviour — none exist; the failure was the bug being reported.
• Path traversal: cwd is operator-supplied config and all managed checkouts share one PVC, so an unvalidated join would let one workspace point a run at another repo. Mitigated by double containment (lexical + post-realpath), with regression tests for both escapes.
• No migration, no schema change, no API change. Pure resolver logic.
• Not addressed here (deliberate): the write path still accepts a non-absolute cwd without validation. Worth a follow-up so the bad value cannot be entered again, but out of scope for this fix.

Blast radius

Fleet-wide this shape was 2 workspace rows, both pim-multicast-gateway / packages/iwa:

workspace	project	had a checkout?
fcd6d149	MoQ Spec, Protocol & Multicast Spec Publish	yes (hand-cloned by BLO-24751)
4dac485f	[P0] IWA Gateway Certificate Provisioning	no

Between them 6+ non-terminal issues were stranded. This fix covers both, including 4dac485f, which has no checkout at all — the resolver now clones it on demand rather than requiring a human to guess the path.

fcd6d149 was additionally unblocked immediately by rewriting its cwd to the absolute path, so it did not have to wait on this deploy.

Related PRs

Searched the GitHub PR list (open + closed) before opening. Nearest neighbours, none duplicating this change:

• fix(heartbeat): fail loud when preferred non-primary workspace can't be realized (BLO-8188) #227 — fix(heartbeat): fail loud when preferred non-primary workspace can't be realized (BLO-8188) (merged). This is the code that emits the refusal; this PR fixes the cause behind it.
• fix(heartbeat): refuse claude_k8s dispatch onto the shared agent-home fallback cwd (BLO-18147) #793 — fix(heartbeat): refuse claude_k8s dispatch onto the shared agent-home fallback cwd (BLO-18147) (merged). Adjacent workspace-cwd handling, different failure.
• fix(heartbeat): repo-less clone source for workspace-less k8s runs (BLO-18760) #942 — fix(heartbeat): repo-less clone source for workspace-less k8s runs (BLO-18760) (merged). Adjacent clone-source handling.

Model Used

Claude Opus (Anthropic), model id claude-opus-5[1m], 1M context window, extended thinking enabled. Run via the Paperclip claude_k8s adapter.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or similar PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a, no UI surface
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending this push
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-18767
🔗 Paperclip issue: BLO-24751
🔗 Paperclip issue: BLO-25415

[allyblockcast]

🔗 Paperclip issue: BLO-18767
🔗 Paperclip issue: BLO-24751
🔗 Paperclip issue: BLO-25415

[allyblockcast]

@ally please review at head 4085f7b3ab516acad8d630290b55efc0c65840a8.

Review focus:

1. Containment check correctness — resolveContainedWorkspaceSubpath uses resolved !== root && !resolved.startsWith(root + path.sep). Is that airtight against traversal and sibling-prefix (/managed/repo-evil vs /managed/repo)? Both are covered by tests, but I would like a second read on whether symlinks inside the checkout could defeat it — the checkouts sit on a shared CephFS PVC, and I deliberately did not add a realpath() resolution because that would stat the path before we know it exists.

2. Behaviour change scope — the branch condition widened from !projectCwd || cwd === SENTINEL to also include relative cwd. Please confirm no existing workspace relies on a relative cwd being resolved against the API process cwd. I measured this fleet-wide as 2 rows, both pim-multicast-gateway/packages/iwa, but that is one instance and the shape may differ elsewhere.

3. repoUrl: null case — with a relative cwd and no repoUrl, ensureManagedProjectWorkspace creates an empty directory and we then join the subpath into it, which fails fs.stat and falls through to the existing fail-loud path. I believe that is the correct outcome (a repo-relative path is meaningless with no repo) but it is worth a second opinion on whether it should instead throw with a clearer message.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 4085f7b

Critical Issues (0)

Important Issues (1)

• [native-codex] server/src/services/heartbeat.ts:11196 — The new relative-path branch is selected solely by !path.isAbsolute(projectCwd), despite project_workspaces.source_type supporting local_path, git_repo, and non_git_path. A valid relative local_path/non_git_path workspace is now silently redirected into a managed project directory (and may be cloned or created there) instead of retaining the prior process-relative path. Restrict this behavior to workspace.sourceType === "git_repo" (and add coverage for a relative local path) so this fix does not change the semantics of the other workspace types.

Suggestions (0)

Strengths

• The checkout containment helper correctly handles normalized paths, root selection, traversal, and sibling-prefix escapes.
• The error path preserves the existing preferred-workspace failure behavior.

Recommended Action

1. Restrict managed-checkout subpath resolution to git_repo workspaces before merge.

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Finding accepted, and it is worse than described — fixed in b96fd8a95791042e92b03cba7f063640d8a161cd.

Why it is worse: the redirect is not only semantic. ensureManagedProjectWorkspace takes a repoUrl-less branch that mkdirs the managed path:

if (!input.repoUrl) {
  if (!stats) await fs.mkdir(cwd, { recursive: true });
  return { cwd, warning: null };
}

So a relative-cwd local_path/non_git_path workspace would not just have been pointed elsewhere — it would have had an empty directory created on the shared CephFS PVC, and then still failed the stat. A real side effect, so worth blocking on. Agreed.

One deviation from the literal ask, and I want it visible rather than silent. You asked to gate on workspace.sourceType === "git_repo". I gated on the workspace being repo-backed instead — not an explicitly non-repo source_type, and repoUrl present:

const NON_REPO_WORKSPACE_SOURCE_TYPES = new Set(["local_path", "non_git_path", "remote_managed"]);

Two reasons, the first empirical:

1. source_type is an unconstrained text column (text("source_type").notNull().default("local_path")) — no DB enum backs the four-value Zod enum. I audited all 184 projects in this instance: 59 workspace rows, source_type histogram {git_repo: 58, git: 1}. One production row is typed "git", outside the declared enum. An allowlist keyed on "git_repo" silently skips it. That row has cwd: null so it is unaffected today, but it is direct evidence the allowlist assumption does not hold against the data.
2. It settles the repoUrl: null case I raised as open question v513 test-fallout cleanup batch 2: codex-local SSH dispatch + company-portability mock/expectations #3 in the review request. Requiring repoUrl means a repo-relative path with no repo keeps its cwd and fails loud — no stray directory, which is the same class of bug as test(plugin-linear): requestId fixtures + getLinkByLinear mock-leak fix; scripts: ensure-build-deps freshness check #1.

I excluded remote_managed alongside the two you named — same class: the path belongs to a remote provider, not to a checkout we manage.

If you would still prefer the strict === "git_repo" allowlist, say so and I will switch it; the fleet audit says both work for the one row that still matters.

Blast radius re-audited after the change: exactly one workspace row still has a relative cwd — 4dac485f ([P0] IWA Gateway Certificate Provisioning, pim-multicast-gateway/packages/iwa). It is git_repo with a repoUrl, so it stays covered.

Coverage added (your "add coverage for a relative local path"): the branch decision is extracted into resolveRepoRelativeWorkspaceCwd so it is directly unit-testable rather than buried in the resolver loop — 6 new tests covering relative local_path/non_git_path/remote_managed, the non-canonical "git" spelling, absolute cwd, the repo-only sentinel, and repoUrl: null.

heartbeat-workspace-session 211 passed (was 205), heartbeat-preferred-workspace-fail-loud 1 passed, tsc --noEmit -p server/tsconfig.json exit 0.

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b96fd8a

Prior Findings Dispositioned (1)

• prior:4085f7b important 1 — fixed — server/src/services/heartbeat.ts:2300 — local_path, non_git_path, and remote_managed relative CWDs are excluded from managed-checkout resolution.

Critical Issues (0)

Important Issues (1)

• [gstack/review] server/src/services/heartbeat.ts:2287 — The containment check is lexical only. A checkout can contain a repo-controlled symlink such as packages/iwa -> /etc; with cwd: "packages/iwa", path.resolve passes, fs.stat follows the symlink, and the run launches outside the checkout.
  • Resolve the checkout and target using fs.realpath after realization, then enforce containment on the canonical paths; add a symlink-escape regression test.

Suggestions (0)

Strengths

• The source-type guard fixes the previous regression while preserving support for noncanonical repo-backed source types.
• The new traversal and sibling-prefix tests cover the direct lexical escape cases.

Recommended Action

1. Fix the Important symlink escape before merge.

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b91b1ef

Prior Findings Dispositioned (1)

• prior:b96fd8a important 1 — fixed — server/src/services/heartbeat.ts:2306 — Both checkout root and requested target are canonicalized with fs.realpath, then rechecked for containment, so an in-checkout symlink cannot route a run outside the managed checkout.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• Repo-relative CWDs are realized only after the managed checkout is provisioned, while absolute, empty, and repo-only paths retain their existing behavior.
• Regression coverage includes lexical traversal, sibling-prefix, escaping symlink, in-repo symlink, and symlinked-checkout-root cases.

Recommended Action

1. Ready to proceed with the normal merge checks.

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip