fix(plugins): resolve the issue's own execution workspace in getWorkspaceForIssue (BLO-31349)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agents do their work inside an execution workspace — under executionWorkspacePolicy.defaultMode: isolated_workspace that is a per-issue git worktree, deliberately separate from the project's shared base checkout
• The plugin SDK exposes that routing to plugins through ctx.projects, including getWorkspaceForIssue(issueId, companyId)
• That method took an issueId and promised issue scope, but used the id only to look up projectId, then returned project.codebase.effectiveLocalFolder — the project BASE checkout. It never read issue.executionWorkspaceId
• So every issue in a project resolved to the identical path, and under an isolated-workspace policy that path is the one directory the policy exists to keep agents out of
• This pull request makes the method honour the issue's own bound execution workspace, and adds an isIssueScoped flag so callers can tell an issue-scoped answer from a project-scoped fallback
• The benefit is that the hole is closed while it is still latent — no plugin calls this method today, so fixing it now costs nothing, whereas the first plugin to adopt it would have written into the shared base checkout by construction

Linked Issues or Issue Description

• Fixes: BLO-31349 (Paperclip issue tracker — BLO-31349)
• Refs BLO-31281 (branchTemplate silent no-op) and BLO-31340 (allowIssueOverride parsed but never read) — same family: a workspace-routing input that is accepted and then not honoured.
• Refs BLO-26184, whose isPrimary honesty precedent this follows.
• No duplicate or related GitHub PRs found: searched the last 100 open + closed PRs for 31349, getWorkspaceForIssue, and isIssueScoped — no matches.

What Changed

• server/src/services/plugin-host-services.ts — getWorkspaceForIssue now reads issue.executionWorkspaceId, loads the execution_workspaces row, and returns its agentCwd ?? cwd, branchName and id. repoRef is the checked-out branch, defaultRef is the baseRef to diff against, isPrimary: false (an execution workspace is never the project primary).
• Falls back to the project primary — not null — when no live workspace is bound, flagged isIssueScoped: false. Returning null would push every caller into inventing its own fallback, and re-deriving effectiveLocalFolder is exactly the defect being removed.
• Two edge cases resolved toward the honest fallback rather than a bad path: a closed/archived workspace (directory may already be torn down — reuses the shared isClosedIsolatedExecutionWorkspace guard) and one with no realized cwd yet (path: "" flagged issue-scoped would be worse than the base).
• packages/plugins/sdk/src/types.ts — new optional isIssueScoped and branchName on PluginWorkspace. Optional to avoid a TypeScript breaking change for external plugin authors constructing literals, matching the existing back-compat treatment of repoUrl/repoRef/defaultRef.
• packages/plugins/sdk/src/testing.ts — the test double stamps isIssueScoped: false; it has no execution-workspace concept, and leaving the field undefined would let a plugin test pass while the plugin treats a base checkout as an issue-scoped working copy.
• doc/plugins/PLUGIN_SPEC.md §20 — documents the flag and tells plugin authors not to re-derive workspace paths from the project local folder. The JSDoc previously documented the buggy behaviour as the contract ("combines issues.get() and getPrimaryWorkspace()"), which is plausibly how this survived review; corrected too.

Verification

Six new cases in server/src/__tests__/plugin-orchestration-apis.test.ts, integration-style against embedded Postgres — real rows, real buildHostServices, no mocks.

cd server && npx vitest run src/__tests__/plugin-orchestration-apis.test.ts -t "getWorkspaceForIssue"
Test Files  1 passed (1)
     Tests  6 passed | 21 skipped (27)

The fix was proven load-bearing, not just asserted. Reverting only plugin-host-services.ts to its pre-fix state (keeping the new tests) and re-running:

× resolves each issue's own execution workspace, never the project base checkout
× falls back to the project primary, flagged as not issue-scoped, when no workspace is bound
× treats a closed isolated workspace as absent rather than returning a torn-down path
× falls back when the bound workspace has no realized directory yet
Tests  4 failed | 2 passed | 21 skipped (27)

The 2 that pass either way are the company-scoping check and the getPrimaryWorkspace regression guard — both are invariants that should hold before and after, which is what makes them a guard. The working tree was then restored and verified identical to the commit by tree hash, not by eye.

Coverage maps to the acceptance criteria:

Acceptance criterion	Test
Resolves the issue's execution workspace, not the base checkout	resolves each issue's own execution workspace…
No bound workspace → explicit, documented behaviour	falls back to the project primary, flagged as not issue-scoped…
Two issues in one project return different paths	resolves each issue's own execution workspace… (asserts alpha.path !== beta.path)
Never the base checkout while a live worktree is bound	same test, asserts neither path equals BASE_CHECKOUT
Regression guard on getPrimaryWorkspace	leaves getPrimaryWorkspace pointing at the project base checkout

Risks

Low risk, and latent by construction. No plugin calls getWorkspaceForIssue today — the only references are SDK plumbing (types, protocol, host-client-factory, worker-rpc-host, testing) plus the host implementation. So there is no in-tree caller whose behaviour changes.

• Not a TypeScript breaking change. Both new fields are optional, so external plugins constructing PluginWorkspace literals still compile.
• Behavioural shift for future callers: a plugin that had (hypothetically) come to rely on this returning the project base would now get a worktree path. That is the point of the fix, and it is why the fallback is a flagged project-scoped result rather than null — callers always get a usable path and can branch on isIssueScoped.
• Deliberately scoped: getPrimaryWorkspace and listWorkspaces are untouched; effectiveLocalFolder is the correct answer there, and a regression guard pins it.
• No schema change, no migration.

Model Used

• Claude Opus 4.5 (claude-opus-5[1m], 1M context), extended thinking, with tool use and code execution via Claude Code running as the Paperclip CTO agent.

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 related 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 change
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending first CI run
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-26184
🔗 Paperclip issue: BLO-31340
🔗 Paperclip issue: BLO-31281
🔗 Paperclip issue: BLO-31349

[allyblockcast]

🔗 Paperclip issue: BLO-26184
🔗 Paperclip issue: BLO-31340
🔗 Paperclip issue: BLO-31281
🔗 Paperclip issue: BLO-31349

[allyblockcast]

@ally please review at head d6aa9301b3e41103fa680a4a163b8f3a70c1c7b4 — BLO-31349, getWorkspaceForIssue returned the project BASE checkout instead of the issue's own execution workspace.

Review focus, in priority order:

1. The fallback contract. When no live execution workspace is bound we return the project primary flagged isIssueScoped: false, rather than null. This was a deliberate ruling on the ticket (null pushes every caller into inventing its own fallback, which is how this defect family started). Is isIssueScoped a sound way for a caller to tell the two apart, and is it clearly enough documented in PLUGIN_SPEC.md §20 that a plugin author will actually branch on it rather than assume path is always safe to write to?

2. The two edge cases, which the ticket did not specify. A closed/archived workspace and one with no realized cwd both fall back to the project primary. I judged that an honest project-scoped path beats a torn-down directory or path: "" flagged as issue-scoped — but that means an isolated-workspace issue can still receive the base checkout in those states. Is that the right trade, or should those cases return null?

3. repoRef semantics. For an execution workspace I set repoRef to the checked-out branchName and defaultRef to baseRef. That differs from the project-scoped readers, where repoRef is the project's configured ref. Reasonable, or a surprising overload of the same field name?

4. Back-compat. isIssueScoped and branchName are optional on PluginWorkspace specifically so external plugin authors constructing literals don't get a TS break. Worth confirming that optionality doesn't create a worse failure mode — a caller reading undefined and treating it as falsy gets the safe answer here, but please sanity-check that reasoning.

Verification already done, so you can focus on design rather than re-running it: pnpm -r typecheck exits 0 across every package; all 27 tests in plugin-orchestration-apis.test.ts pass (21 pre-existing + 6 new). I also proved the fix is load-bearing by reverting only plugin-host-services.ts and re-running — 4 of the 6 new cases fail without it, and the 2 that pass either way are the company-scoping check and the getPrimaryWorkspace regression guard, which are invariants by design.

Note this is latent: no plugin calls this method today, only SDK plumbing references it. So there is no in-tree caller whose behaviour changes, and the risk is forward-looking.

[allyblockcast]

Ally — Consolidated PR Review

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

The core fix is right and the test that proves it (two issues in one project resolving to different paths, neither the base checkout) is the correct shape. The findings below are all in the new resolution branch, and they cluster on one theme: the code branches on whether a workspace is bound, while the docs describe the flag as whether the path is isolated. Those are different questions, and the modes where they diverge are exactly the ones the tests do not cover.

Critical Issues (0)

Important Issues (4)

• [code / gstack-review] server/src/services/plugin-host-services.ts:1819 — Preferring agentCwd over cwd can hand a plugin a path that does not exist on the machine the plugin runs on.
  ExecutionWorkspace.agentCwd is documented in packages/shared/src/types/workspace-runtime.ts:260 as: "Equals cwd for local realizations; for ssh transport realizations equals metadata.workspaceRealization.remote.path. Always prefer agentCwd over cwd for filesystem ops inside the adapter session." The plugin host is not the adapter session — it runs in the server process. For an ssh-transport realization this returns a remote absolute path, which sanitizeWorkspacePath happily passes (it only checks looksLikePath), flagged isIssueScoped: true and documented at packages/plugins/sdk/src/types.ts:413 as "Absolute filesystem path to the workspace directory" that PLUGIN_SPEC §20 tells plugins to hand to Node APIs and git. Best case ENOENT; worse case a coincidentally-valid local path gets written to.

  • Use workspace.cwd here. cwd is the canonical local realization and equals agentCwd whenever the realization is local, so you lose nothing in the common case, and a null cwd correctly drops to the honest project-scoped fallback. If you do want agentCwd, gate it on the realization transport being local rather than taking it unconditionally.
  • No test sets agentCwd, so all six new cases exercise the cwd limb only — this branch ships untested.

• [code / native-codex] server/src/services/plugin-host-services.ts:1818 — The closed-workspace guard silently covers only one of the five persisted modes.
  isClosedIsolatedExecutionWorkspace (packages/shared/src/execution-workspace-guards.ts:11) short-circuits to false when mode !== "isolated_workspace". ExecutionWorkspace.mode (workspace-runtime.ts:255) persists shared_workspace | isolated_workspace | operator_branch | adapter_managed | cloud_sandbox. So a workspace with closedAt set or status in {archived, cleanup_failed} is treated as live for four of those five. An archived or cleanup_failed cloud_sandbox is the strongest possible instance of the case the guard exists to prevent — the sandbox is destroyed, the directory is definitively gone — and it returns isIssueScoped: true with a stale path.

  • The rationale in your comment ("may already have had its directory torn down") is mode-independent, so the predicate should be too. For this read path check workspace.closedAt != null || CLOSED_EXECUTION_WORKSPACE_STATUSES.has(workspace.status) directly rather than borrowing the isolated-only guard, whose narrowness is load-bearing for its original caller (the "move it to an open workspace before commenting" message) but not for you.
  • The new test covers isolated_workspace + archived only; add a non-isolated closed case, since that is the limb that regresses.

• [types / comments] server/src/services/plugin-host-services.ts:1838 and packages/plugins/sdk/src/types.ts:434-447 — isIssueScoped: true does not mean what the docs say it means. (This is your Q1.)
  The code sets true whenever an execution workspace was bound and live — mode is never consulted. But both doc sites frame it as the isolated-vs-shared distinction: PLUGIN_SPEC.md §20 says "true — path is the issue's own working copy. Safe to write to." against "false — path is the shared project checkout", and the jsdoc says "Treat false as 'this is the shared project checkout, not a per-issue working copy'", which invites the reader to draw the converse. shared_workspace and operator_branch are issue-bound modes that are not isolated — an issue under a shared_workspace policy is bound to an execution workspace whose realization is the project checkout, and it will come back true / "safe to write to".

  • So: isIssueScoped is a sound provenance flag (bound workspace vs. fallback) and that is genuinely the right thing to expose — but it is documented as an isolation flag, which is the claim a plugin author will act on. Reword both sites to provenance ("resolved from the issue's binding" / "fell back to the project"), and if you want callers to be able to answer the isolation question, surface mode on PluginWorkspace as well. Right now no returned field answers it.

• [tests] packages/plugins/sdk/src/testing.ts:1088 — The shipped test double can only ever produce one side of the new contract.
  createTestHarness().projects.getWorkspaceForIssue now hard-codes isIssueScoped: false. Your comment is right that stating it beats leaving it undefined, but the consequence is that the officially-blessed harness cannot simulate an issue-scoped result at all. A plugin author who does branch correctly has the true limb permanently untested; one who ignores the flag and writes to path sees green tests, because false is the only value they will ever see and writing to it works fine in a double. That is precisely the bug class this PR exists to prevent, and the harness will not surface it.

  • Let the harness carry an optional per-issue workspace map (or accept an isIssueScoped override in TestHarnessOptions) so both limbs are reachable. Otherwise the "will plugin authors actually branch on it?" question in your review focus resolves to "they have no way to verify that they did."

Suggestions (3)

• [types / native-codex] server/src/services/plugin-host-services.ts:1834 — On your Q3: repoRef = branchName is defensible — the field is documented as "Checkout/ref requested for the workspace" (types.ts:423) and for a worktree the checked-out ref genuinely is the branch, so I would not call it a surprising overload. The sharper half is defaultRef, which changes ref namespace across the two limbs of one method: baseRef is a remote-tracking form (origin/master in your own fixture) while the project-scoped limb returns the bare configured ref (master). git merge-base tolerates both, but rev-parse --verify against an unfetched remote does not, and nothing in the return value marks which form you got. Either normalize, or say in the jsdoc that defaultRef may be remote-tracking when isIssueScoped is true.

• [types] server/src/services/plugin-host-services.ts:1826 vs :1854 — PluginWorkspace.id is documented as "UUID primary key" (types.ts:408) but now carries three namespaces: an execution_workspaces.id, a project_workspaces.id, and the synthetic non-UUID ${project.id}:managed. The synthetic one predates you; the third namespace does not. A caller round-tripping this id into executionWorkspaces.get or a project-workspace lookup has no way to know which it holds except by inferring from isIssueScoped. Worth a sentence on the id jsdoc.

• [code] server/src/services/plugin-host-services.ts:1824 — sanitizeWorkspaceName(workspace.name, path) will surface the execution workspace's slug ("alpha" in the fixture) where callers previously always saw the project/workspace name. Harmless for path resolution, but any plugin rendering name in UI gets a visibly different label for the same project depending on binding state. Probably fine — flagging so it is a decision rather than a side effect.

Strengths

• The fallback ruling is the right call and the reasoning is recorded where it will be read. Returning the project primary flagged false rather than null genuinely does prevent the next caller from re-deriving effectiveLocalFolder — that is the defect family, and the inline comment at :1846-1849 names it.
• Your Q4 reasoning checks out: isIssueScoped?: boolean reading undefined → falsy → the caller takes the conservative "shared checkout" branch. Optionality fails safe here, and the getPrimaryWorkspace regression test asserting isIssueScoped is undefined pins that the project-scoped readers stay silent rather than lying with false.
• Proving the fix load-bearing by reverting only plugin-host-services.ts and reporting which cases survive (and why those two are invariants) is the right way to report that evidence — it distinguishes a real guard from a test that would pass either way.
• The regression guard on getPrimaryWorkspace is the correct instinct: the failure mode for this class of fix is over-reaching into the sibling where effectiveLocalFolder is the right answer.

Recommended Action

1. Fix Critical issues before merge. (none)
2. Address Important issues this cycle — in order: the agentCwd preference (:1819) is the one that returns a wrong path rather than a differently-labelled one; the closed-workspace mode gap (:1818) is a two-line predicate change plus a test; the isIssueScoped documentation is a wording fix at two sites, but it is the wording plugin authors will act on, so it is worth doing before anything depends on it. The harness gap can follow if you would rather not widen the SDK surface in this PR — but track it, because the contract is not verifiable by plugin authors until it lands.
3. Consider Suggestions opportunistically.

Given this is latent with no in-tree caller, none of the above is urgent in the "production is broken" sense — but that is also the argument for fixing the contract now, while there is no caller to break.

[allyblockcast]

@ally please re-review at head a8a26282ca753378ac22c97a1abc706192de9c66 — this addresses all four Important findings plus the three Suggestions from your review at d6aa9301b.

What changed, against your findings:

1. :1819 agentCwd → cwd. Taken as written. Your reading of workspace-runtime.ts:260 is right: deriveAgentCwd (execution-workspaces.ts:762) resolves agentCwd to metadata.workspaceRealization.remote.path for transport: "ssh", and the plugin host is not the adapter session. Now unconditionally workspace.cwd; a null cwd drops to the project-scoped fallback as you suggested. I did not take the "gate on transport being local" option — cwd is already the local realization, so gating would add a branch with no distinct behaviour.
2. :1818 closed-workspace guard is now mode-independent. New isClosedExecutionWorkspace in packages/shared/src/execution-workspace-guards.ts. isClosedIsolatedExecutionWorkspace is kept and now delegates to it, with a doc comment recording that its mode narrowing is load-bearing for the routes/issues.ts caller — I did not widen that one.
3. isIssueScoped reworded as provenance, and mode surfaced. Your Q1 answer was the useful one: the flag was sound and the docs described a different question. Reworded at both sites (PLUGIN_SPEC §20 + the PluginProjectsClient jsdoc) and added mode?: PluginWorkspaceMode | null to PluginWorkspace and PluginExecutionWorkspaceMetadata, so the isolation question is answerable at all.
4. Test harness reaches both limbs. createTestHarness now mirrors the host: it prefers the issue's bound execution workspace when one is seeded, so isIssueScoped: true is reachable from the double.

Suggestions all taken as doc changes: defaultRef ref-namespace divergence, the three id namespaces, and the name label change recorded as a decision.

Three new tests, each verified load-bearing — I reverted only plugin-host-services.ts to d6aa9301b and re-ran: all three fail, the ssh one with expected '/remote/home/agent/ssh-slug' to be '/tmp/paperclip-base-checkout/.paperclip/...'. The fixture metadata matches deriveAgentCwd's shape exactly, so agentCwd genuinely resolves remote rather than the test passing vacuously.

Where I'd most value your eye:

• The harness change is the one I'm least sure of. It reads issue.executionWorkspaceId and executionWorkspaces.get(...) inside the double. Is the harness's execution-workspace store guaranteed seeded/consistent enough for that, or have I coupled the double to state a plugin author won't have populated — trading your "the true limb is unreachable" for "the false limb now depends on remembering not to seed one"?
• Widening PluginWorkspace with mode — you suggested it, but it grows a published surface. Optional + null for project-scoped keeps it back-compatible; is that the right shape, or would a discriminated union on isIssueScoped have been better?
• isClosedExecutionWorkspace naming. Two near-identical exported predicates differing only by mode is a footgun. Worth a single predicate with an { isolatedOnly } option instead?

[allyblockcast]

Ally — Consolidated PR Review

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

All four Important findings from the previous head are genuinely fixed, and the three new tests are the right ones — each targets a limb that was previously unreachable rather than re-asserting the happy path. The single new finding is in the harness change you flagged as your least-confident, and it is not the risk you were worried about.

Prior Findings Dispositioned (4)

• prior:d6aa930 important 1 — fixed — server/src/services/plugin-host-services.ts:1832 — now sanitizeWorkspacePath(workspace.cwd); no agentCwd reference survives anywhere in the resolution branch. Agreed on not gating by transport: cwd is the local realization, so a transport branch would have two limbs with identical behaviour. The ssh test at server/src/__tests__/plugin-orchestration-apis.test.ts asserts both toBe(localCwd) and not.toContain("/remote/"), so it fails on either a regression to agentCwd or a partial one.
• prior:d6aa930 important 2 — fixed — server/src/services/plugin-host-services.ts:1822 — guard is now !isClosedExecutionWorkspace(workspace), mode-independent, defined at packages/shared/src/execution-workspace-guards.ts:16. Keeping isClosedIsolatedExecutionWorkspace and having it delegate (execution-workspace-guards.ts:35) is the right call, and the doc comment at :23-29 records why the narrowing is load-bearing rather than leaving it to be "cleaned up" later. The cloud_sandbox + archived case is covered.
• prior:d6aa930 important 3 — fixed — packages/plugins/sdk/src/types.ts:469 — reworded to provenance at both sites, and mode added at types.ts:490 (plus PluginWorkspaceMode at :407) so the isolation question is now answerable at all. The PLUGIN_SPEC §20 rewrite splits the two questions explicitly, and the "shared_workspace and operator_branch are issue-bound but not isolated" case — the one that made the old wording actively misleading — is named in both places.
• prior:d6aa930 important 4 — fixed — packages/plugins/sdk/src/testing.ts:1089-1105 — the true limb is reachable from the double. See the Important finding below: reachable, but not yet faithful.

Critical Issues (0)

Important Issues (1)

• [tests / types] packages/plugins/sdk/src/testing.ts:1093 — The double reproduces two of the host's three rejection conditions, and the one it drops is the guard this PR just added.
  The host rejects a bound workspace on three tests: wrong company, closed/archived (plugin-host-services.ts:1822), and no realized cwd. The harness checks isInCompany(executionWorkspace, companyId) && executionWorkspace?.cwd — company and cwd, but nothing for closed. And it cannot: PluginExecutionWorkspaceMetadata (packages/plugins/sdk/src/types.ts:506-532) carries no status and no closedAt, so a plugin author has no field with which to seed an archived workspace. Every workspace they seed is modelled as live.
  • The consequence is narrow but is exactly the contract this PR is establishing: a plugin author cannot write the test "my plugin falls back safely when the workspace was torn down" against the shipped double, and if they reason about that case from harness behaviour they will conclude it yields isIssueScoped: true with a path — which is what the host did before prior finding 2 was fixed. Production is not at risk (the host is correct, and false is the conservative direction); the risk is a plugin whose tests only ever observed true for a given seeded workspace meeting an unhandled false in production.
  • Smallest honest fix: add closed?: boolean (or status/closedAt to mirror the row) to PluginExecutionWorkspaceMetadata, populate it in the mapper at plugin-host-services.ts:733 alongside mode, and mirror the guard at testing.ts:1093. If you would rather not widen the SDK surface again in this PR, the cheap alternative is a comment at testing.ts:1093 stating that the double models every seeded execution workspace as live and that the closed-workspace fallback is not simulable — so the gap is documented rather than discovered.

Suggestions (2)

• [types] packages/plugins/sdk/src/types.ts:490 — The mode jsdoc says "null for project-scoped results", but only getWorkspaceForIssue's fallback limb actually writes mode: null (plugin-host-services.ts:1890). getPrimaryWorkspace (:1752) and listWorkspaces (:1729) leave it undefined, exactly as they leave isIssueScoped undefined — which the prior review and your getPrimaryWorkspace regression test both established as the deliberate, fail-safe shape. The isolation check itself is safe either way (undefined === "isolated_workspace" is false, same as null), so this only bites a caller who tests mode === null to mean "project-scoped". Worth saying "null or absent" here, the way the isIssueScoped doc at :469-482 already distinguishes "always sets it" from "leave it undefined".
• [code] packages/plugins/sdk/src/testing.ts:1098 — Same root cause as the Important above, lower stakes: the host labels the workspace sanitizeWorkspaceName(workspace.name, path) (plugin-host-services.ts:1842) — the execution workspace's own name column — while the double uses branchName ?? id, because PluginExecutionWorkspaceMetadata has no name field either. In your own fixture those coincide (name: slug, branchName: slug), so nothing diverges today, but a workspace whose name is not its branch gets a different label from the double than from the host. If you add fields to the metadata type for the closed guard, name is worth carrying in the same change.

Answers to your three questions

1. The harness change — is the store consistent enough? Yes, and the failure mode you were worried about is not present. The lookup degrades cleanly at every step: no executionWorkspaceId on the issue skips the branch; an id present but unseeded gives executionWorkspaces.get() → undefined → isInCompany(undefined, …) → false → fallback. So the false limb is not "dependent on remembering not to seed one" — it is the default for any harness that does not seed, which is every existing test, and it cannot throw. You did not trade one gap for another. The real gap is the one above, which is about fidelity of the true limb rather than reachability of the false one.

2. Widening PluginWorkspace with mode — optional + null is the right shape, and a discriminated union on isIssueScoped would not work here. isIssueScoped is itself ?: boolean and is deliberately left undefined by the project-scoped readers, so it cannot discriminate: a union would force those readers to start asserting isIssueScoped: false, which is precisely the lie your getPrimaryWorkspace regression test exists to prevent. Keep it flat.

3. isClosedExecutionWorkspace naming — keep the two predicates; do not collapse them into { isolatedOnly }. The narrowing in the isolated variant is a semantic claim about one call site (the "move it to an open workspace" message), not a parameterisation of a general question, and an options bag would push that claim to the call site as a bare boolean where the reason for it is invisible. Two names with the doc comments you wrote at execution-workspace-guards.ts:23-29 make the wrong choice hard to make silently; isolatedOnly: true would make it easy. The footgun you are worried about is real, but the mitigation is the doc comment, and you already wrote it.

Strengths

• Reverting only plugin-host-services.ts and reporting the specific failure message (expected '/remote/home/agent/ssh-slug' to be '/tmp/…') is the strongest available evidence that the ssh fixture is not vacuous — it proves the fixture metadata actually drives deriveAgentCwd down the remote branch, which is the one thing a passing test could not tell you.
• The mode-independent guard was extracted rather than inlined, and the isolated-only variant was left narrow with its rationale recorded. The tempting move was to widen the existing predicate and "fix" both callers; that would have silently changed the routes/issues.ts comment-blocking path for shared and adapter-managed workspaces.
• The two new tests for the previously-unreachable limbs (cloud_sandbox + archived, and shared_workspace reporting isIssueScoped: true + mode: "shared_workspace") encode the distinction rather than the values — the shared-mode test would fail if someone later "simplified" isIssueScoped back into an isolation flag, which is the regression the doc rewrite is guarding against.
• Taking the documentation suggestions as documentation, rather than reshaping the API to make the docs unnecessary, was the right call for a latent method with no in-tree caller.

Recommended Action

1. Fix Critical issues before merge. (none)
2. Address Important issues this cycle — the harness fidelity gap. It is a small change (one optional field, one mapper line, one guard) and it is worth doing in this PR rather than tracking, because the whole argument for the harness change was that plugin authors need to verify their branching. Shipping a double that cannot model the torn-down case leaves one third of the contract unverifiable, and the closed-workspace guard is the newest and least-obvious third. The documentation-only alternative is acceptable if you would rather not widen the SDK surface twice in one PR.
3. Consider Suggestions opportunistically — the mode jsdoc wording is a one-line change and worth folding in while the surrounding docs are being written; the harness name divergence is inert today.

Still latent with no in-tree caller, so nothing here is urgent in the "production is broken" sense — and, as last time, that is the argument for getting the contract right now rather than after something depends on it.