Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions docs/superpowers/specs/2026-08-16-cross-project-spawn-guard-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
# Cross-project spawn guard

Issue #109, reframed. Filed as "let a thread act in another project"; the real
finding is that a thread *already* acts in another project, by accident, with
no guard rail. The user's ruling: a run must never write to a second repo.

## The incident

Shipnest-v2 thread, 2026-08-16 12:48–13:10. An agent working in
`/Users/willem/code/Shipnest-v2` called `thread_fork({threadId: "260be2c1"})`
— a thread belonging to **Solenta**. Two grok workers spawned on git worktrees
of the wrong repo. Told off, it repeated the mistake four minutes later with
`9fafc689`, guessing from the title "changelog and website". Cleanup cost two
deleted branches and a removed worktree.

The agent's own diagnosis:

> "I have no way to see which project a thread belongs to — `threads_list`
> returns id, title, provider, status, nothing about the workspace — so every
> fork I make is a guess, and guessing is exactly what caused this."

Three leaked worker threads survive in the store, archived.

## Root cause

`thread_fork(threadId)` takes its source thread as an **argument**. There is no
caller identity anywhere in `electron/orchServer.js`: one workspace-wide bearer
token, stateless HTTP (`sessionIdGenerator: undefined`). So the server cannot
know who is calling, and the caller must name a thread.

Naming one is necessarily a guess, because:

- the agent is never told its **own** thread id, and
- `threads_list` does not say which **project** any thread belongs to.

Everything downstream is already correct when the caller passes its own thread
id: the fork lands in the caller's project, the digest is the caller's own
context, the wake-up notice returns to the caller. The entire bug is that the
agent cannot know its own id.

`thread_send` has the same hole — it starts a run on any threadId with no check.

## Design

Three changes. No new feature, no UI change, no schema change. Because each
thread stays bound to exactly one project, the diff/commit/PR/worktree views
are untouched, which answers all three of issue #109's open questions with
"nothing changes".

### 1. `threads_list` reports the project

`electron/orchServer.js` — each row gains `projectId` and `projectName`. This
is the field whose absence the agent named as the reason it guessed.

### 2. Every run learns its own identity

`electron/services.js` gains `selfIdNoteFor(thread, project)`, a sibling of the
existing `planboardNoteFor`: a short CLI-only note, never stored in the
transcript, appended at the single prompt finalization point
(`electron/runner.js`, `dispatchPrompt`).

```
[Thread] You are thread <id> in project "<name>" (projectId <id>), checked out
at <cwd>. Pass this threadId and projectId to coder-threads tools; never guess
another thread's id.
```

It rides every dispatch, like the planboard note, so it survives context
compaction and resumed sessions rather than being a first-turn-only fact.

### 3. `thread_fork` and `thread_send` require a stated `projectId`

Both gain a required `projectId` param. The server compares it to the target
thread's `projectId` and throws, naming both projects, when they differ.

Change 2 makes stating it free; the assert converts a wrong guess from two grok
workers on the wrong repo into a loud error. A same-project fork of a sibling
thread still works — the assert only blocks crossing a project boundary.

`thread_send` gets the same guard even though the incident used `thread_fork`:
one guard, both callers.

## Known ceiling

The assert is a **self-declared claim, not proof**. A caller that states a
projectId matching the thread it names passes, whatever project it actually
runs in.

Real caller identity would need a per-run bearer token mapped to a threadId.
Rejected as too expensive for this failure: claude and codex take per-run MCP
config, but grok and kimi register the server globally
(`grok mcp add … -s user`, `~/.kimi-code/mcp.json`), so a per-run token cannot
reach them without racy global config rewrites — and the incident's workers
were grok.

This failure mode is a confused agent, not a malicious one, and confusion is
exactly what a stated assertion catches. Upgrade to per-run tokens only if an
adversarial caller ever matters. Marked with a `ponytail:` comment in the code.

## Check

`electron/test/orch-server.test.js` already drives the handlers spawn-free via
the exported `createToolHandlers`. Added: a fork with a foreign `projectId`
throws and starts no run; a fork with the matching one succeeds.

`thread_status` is deliberately left unguarded: it is read-only and cannot
start anything, and `threads_list` already exposes every thread's title across
projects. A required param there would cost callers something and buy no safety.

The note is emitted only while `coder-threads` is registered — with no thread
tools in the run there is nothing to pass the ids to. This keeps it out of
every prompt in test and orchestrator-down runs, matching `planboardNoteFor`'s
GitHub-origin gate.

## Out of scope

- Cross-project **writes** in any form (user: "It should not ever do that").
- Read-only reach into another project ("go look at how X does this"). Deferred
until the leak fix has been lived with.
- Purging the three leaked worker threads — cosmetic, user's call.
84 changes: 74 additions & 10 deletions electron/orchServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ const MAX_BODY_BYTES = 1024 * 1024;

const INSTRUCTIONS =
"Solenta thread orchestrator: drive other agent threads in this Solenta workspace. " +
"threads_list shows every thread with id, title, provider, status and handoffFrom. " +
"The workspace holds SEVERAL projects and this server sees all of them, but you may " +
"only drive threads in your own. Your thread id and project id are stated at the end " +
"of your prompt; pass them, never guess an id from a title. " +
"threads_list shows every thread with id, title, provider, status, handoffFrom, " +
"projectId and projectName — filter it by your own projectId first. " +
"thread_fork and thread_send reject a thread outside the projectId you pass. " +
"thread_fork starts a new thread (optionally on a different provider) on your " +
"prompt; the worker does NOT inherit the source session, only a truncated digest " +
"of its last messages, so write a self-contained prompt with every fact the worker " +
Expand Down Expand Up @@ -197,21 +202,69 @@ async function loadOrCreateConfig(file) {
function createToolHandlers(deps) {
const { store, runner, forkThread, getProvider } = deps;

/**
* The project a thread belongs to, or null when it has been deleted.
* @param {{ projectId?: string } | null | undefined} thread
*/
function projectOf(thread) {
if (!thread || !thread.projectId) return null;
if (typeof store.getProject !== "function") return null;
return store.getProject(thread.projectId);
}

/**
* Guard against driving a thread in ANOTHER project (issue #109). The
* server has no caller identity, so the caller must STATE the project it
* believes it is acting in and we reject a mismatch.
*
* ponytail: a self-declared claim, not proof — it catches a confused
* caller, not a malicious one. Upgrade to per-run bearer tokens mapped to
* threadIds if forgery ever matters (costly: grok/kimi register MCP
* globally at user scope, so a per-run token cannot reach them).
*
* @param {{ id: string, projectId?: string }} thread - the target thread
* @param {unknown} claimed - projectId the caller says it is working in
*/
function assertSameProject(thread, claimed) {
const want = String(claimed || "");
const got = String(thread.projectId || "");
if (want === got) return;
const name = (p) => (p && p.name ? `"${p.name}"` : "unknown project");
const target = name(projectOf(thread));
const caller = name(
typeof store.getProject === "function" ? store.getProject(want) : null,
);
throw new Error(
`Thread ${thread.id} belongs to ${target} (projectId ${got}), not to ` +
`${caller} (projectId ${want}). A thread can only drive threads in ` +
`its own project. Use your own projectId and a threadId from ` +
`threads_list with that same projectId.`,
);
}

async function threads_list() {
return store.getThreads().map((t) => ({
id: t.id,
title: t.title ?? null,
provider: t.provider ?? null,
status: t.status ?? null,
handoffFrom: t.handoffFrom ?? null,
}));
return store.getThreads().map((t) => {
const project = projectOf(t);
return {
id: t.id,
title: t.title ?? null,
provider: t.provider ?? null,
status: t.status ?? null,
handoffFrom: t.handoffFrom ?? null,
// Without these, picking a thread is a guess — which is exactly how
// one project's agent spawned workers on another project's repo.
projectId: t.projectId ?? null,
projectName: project ? (project.name ?? null) : null,
};
});
}

async function thread_fork(args) {
const source = store.getThread(args.threadId);
if (!source) {
throw new Error(`Unknown thread: ${args.threadId}`);
}
assertSameProject(source, args.projectId);
/** @type {{ threadId: string, provider?: string }} */
const input = { threadId: args.threadId };
if (args.provider != null) {
Expand Down Expand Up @@ -249,6 +302,7 @@ function createToolHandlers(deps) {
if (!thread) {
throw new Error(`Unknown thread: ${args.threadId}`);
}
assertSameProject(thread, args.projectId);
// Re-dispatched worker: unarchive so the new run is visible again.
if (thread.orchWorker && thread.archived) {
store.updateThread(args.threadId, { archived: false });
Expand Down Expand Up @@ -325,7 +379,9 @@ function buildMcpServer(sdk, handlers) {
"threads_list",
{
description:
"List every thread: id, title, provider, status, handoffFrom.",
"List every thread: id, title, provider, status, handoffFrom, " +
"projectId, projectName. Threads from EVERY project are listed — " +
"match projectId against your own before touching one.",
inputSchema: {},
},
async () => json(await handlers.threads_list()),
Expand All @@ -336,12 +392,17 @@ function buildMcpServer(sdk, handlers) {
{
description:
"Fork a thread into a new one (optionally on another provider) and start it on prompt. " +
"threadId is normally YOUR OWN thread id, and projectId YOUR OWN project id — both are " +
"stated at the end of your prompt. The fork lands in the source thread's project, so " +
"forking a thread from another project spawns workers on the WRONG repo: projectId must " +
"match the source thread's project or the call is rejected. " +
"The fork carries only a truncated digest of the source thread's last messages, not the " +
"whole conversation — the prompt must be self-contained. " +
"The worker gets its own git worktree so parallel workers never edit the same files; " +
"pass worktree:false to run it in the project checkout instead. Returns the new threadId.",
inputSchema: {
threadId: z.string().min(1),
projectId: z.string().min(1),
provider: z.string().min(1).optional(),
prompt: z.string().min(1),
worktree: z.boolean().optional(),
Expand All @@ -354,9 +415,12 @@ function buildMcpServer(sdk, handlers) {
"thread_send",
{
description:
"Start a run with prompt on an existing thread. Rejects unknown threads.",
"Start a run with prompt on an existing thread. projectId is YOUR OWN project id " +
"(stated at the end of your prompt); the thread must belong to it. Rejects unknown " +
"threads and threads in another project.",
inputSchema: {
threadId: z.string().min(1),
projectId: z.string().min(1),
prompt: z.string().min(1),
},
},
Expand Down
12 changes: 11 additions & 1 deletion electron/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -3483,11 +3483,21 @@ function createRunner(opts) {
// Single finalization point for every provider path: prefix only goes to
// the CLI (buildArgs / runAgent), never into the stored user message.
// Live-session follow-ups (handle.send) receive this same string.
// Re-read: materializePendingWorktree may have just set worktreePath, and
// the self-id note quotes the cwd the CLI actually gets.
const dispatchThread = store.getThread(threadId) || thread;
const dispatchPrompt =
services.buildHandoffPrefix(thread, (id) => store.getMessages(id)) +
String(prompt ?? "") +
attachmentPromptSection(attachments) +
services.planboardNoteFor(projectForGate && projectForGate.path);
services.planboardNoteFor(projectForGate && projectForGate.path) +
services.selfIdNoteFor(
dispatchThread,
projectForGate,
dispatchThread.worktreePath ||
(projectForGate && projectForGate.path) ||
null,
);

const name = workflowNameFromThreadId(threadId);

Expand Down
41 changes: 41 additions & 0 deletions electron/services.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,46 @@ function planboardNoteFor(projectPath) {
}
}

/**
* Standing note appended to every dispatched prompt (CLI-only, never stored
* in the transcript) telling the agent WHICH thread and project it is, so the
* coder-threads tools can be called with real ids instead of a guess.
*
* The orchestrator server has no caller identity (one workspace-wide token,
* stateless HTTP), so this note is the only channel by which an agent learns
* its own id. Without it an agent picked thread ids off threads_list by title
* and spawned workers on another project's repo (issue #109).
*
* Rides every dispatch rather than only the first turn: context compaction
* and resumed sessions would otherwise lose it.
*
* Emitted only when the coder-threads server is actually registered: with no
* thread tools in the run there is nothing to pass these ids to, and the note
* would just be noise. Same rule as planboardNoteFor's GitHub-origin gate.
*
* @param {{ id?: string, projectId?: string } | null | undefined} thread
* @param {{ name?: string } | null | undefined} project
* @param {string | null | undefined} cwd - worktree path, else project path
* @returns {string}
*/
function selfIdNoteFor(thread, project, cwd) {
if (!thread || !thread.id || !thread.projectId) return "";
try {
const { activeServers } = require("./memory-sup.js");
if (!activeServers().some((s) => s.name === "coder-threads")) return "";
} catch {
return "";
}
const name = project && project.name ? String(project.name) : "this project";
const where = cwd ? `, checked out at ${cwd}` : "";
return (
`\n\n[Thread] You are thread ${thread.id} in project "${name}" ` +
`(projectId ${thread.projectId})${where}. Pass these ids to the ` +
`coder-threads tools; never guess another thread's id from its title. ` +
`Threads in other projects are off limits.`
);
}

/** Keep a persisted plan bounded: it rides every threads:changed push. */
const PLAN_STEP_MAX = 200;
const PLAN_STEPS_MAX = 50;
Expand Down Expand Up @@ -1777,6 +1817,7 @@ module.exports = {
HANDOFF_MESSAGE_COUNT,
PLANBOARD_NOTE,
planboardNoteFor,
selfIdNoteFor,
planStepsFrom,
setArchived,
setSettled,
Expand Down
Loading
Loading