From 02926e730a59c80ca1298fc6b58e9f937d6c1bd8 Mon Sep 17 00:00:00 2001 From: Hussain Phalasiya Date: Sun, 18 Oct 2026 14:25:47 +0300 Subject: [PATCH] VC-807: confirm a minted harness launch before it moves the resume seed The wrapper announced and minted for every invocation, so 'claude --version', 'claude mcp list' or 'codex --help' repointed the running harness and overwrote the resume seed, and Resume session ran a phantom id or the wrong binary. - Wrapper: skip the announce/mint for adapter-declared non-session argv (--version/--help and housekeeping subcommands), and run a harness nested inside a running one of its kind (its own session markers set) exactly as outside Volli. - Host: hold a minted id as pending; the harness and its seed move together only when that harness's startup hook, or a hook naming the id, arrives. - Host: an announce that changes harness without a mint clears the foreign seed, so resume falls back to the new harness's latest. --- apps/desktop/e2e/harness-wrapper-smoke.mjs | 6 +- apps/desktop/e2e/interrupt-resume-smoke.mjs | 11 +- apps/desktop/e2e/lib/fake-harness.mjs | 20 +- apps/desktop/src/main/harness-runtime.test.ts | 109 +++++++++++ packages/host-core/src/agent-commands.test.ts | 179 +++++++++++++++--- packages/host-core/src/agent-commands.ts | 15 +- .../host-core/src/agent-dispatch/context.ts | 17 ++ .../src/agent-dispatch/harness-verbs.ts | 163 ++++++++++++---- packages/shared/src/harness/claude-code.ts | 17 ++ packages/shared/src/harness/codex.ts | 23 +++ packages/shared/src/harness/cursor.ts | 21 ++ packages/shared/src/harness/opencode.ts | 23 +++ packages/shared/src/harness/types.ts | 21 ++ packages/shared/src/harness/wrapper.test.ts | 66 ++++++- packages/shared/src/harness/wrapper.ts | 72 ++++++- packages/shared/src/host-events.ts | 3 + 16 files changed, 691 insertions(+), 75 deletions(-) diff --git a/apps/desktop/e2e/harness-wrapper-smoke.mjs b/apps/desktop/e2e/harness-wrapper-smoke.mjs index 1e9ae1568..8efbcd402 100644 --- a/apps/desktop/e2e/harness-wrapper-smoke.mjs +++ b/apps/desktop/e2e/harness-wrapper-smoke.mjs @@ -347,8 +347,10 @@ async function main() { UUID_V4.test(first ?? "") && UUID_V4.test(second ?? "") && first !== second && - // The newest launch is what a resume of this session must find. - seed === second; + // Neither launch fired a SessionStart (the fake binary has no hooks), so + // neither proved itself, and neither id may become the seed (VC-807). + seed !== first && + seed !== second; return { ok, detail: `first=${first} second=${second} seed=${seed}` }; }); diff --git a/apps/desktop/e2e/interrupt-resume-smoke.mjs b/apps/desktop/e2e/interrupt-resume-smoke.mjs index f2be1a6a6..a52c24b04 100644 --- a/apps/desktop/e2e/interrupt-resume-smoke.mjs +++ b/apps/desktop/e2e/interrupt-resume-smoke.mjs @@ -24,8 +24,10 @@ * `sessionId: { kind: "reported" | "none" }` (opencode, codex), since * an `argv`-tier one always has a minted seed; * – the harness's own wrapper minted one for THAT LAUNCH (`volli session - * harness --mint`, `sessionId: { kind: "argv" }`) → check 5, - * which asserts the resume comes back with that exact id. + * harness --mint`, `sessionId: { kind: "argv" }`) and the + * launch's own SessionStart confirmed it (the fake `claude` fires one, + * as a real one does at boot) → check 5, which asserts the resume comes + * back with that exact id. * Resume opens a new attachment on the same durable Volli Session; it does * not mint a second Session. Ticket history names the successful reattachment * and its user origin, while process lifecycle evidence stays in the ledger. @@ -719,8 +721,9 @@ async function main() { // // The other half of what check 4 stopped covering. An `argv`-tier harness // gets a fresh UUIDv4 from its own wrapper's synchronous `volli session - // harness --mint` call on every launch, written to `harness_session_id`, - // and a resume must come back with THAT id — not merely some uuid, and + // harness --mint` call on every launch, written to `harness_session_id` + // once the launch's SessionStart names it (VC-807: the fake fires one, as + // a real claude does at boot), and a resume must come back with THAT id — not merely some uuid, and // emphatically not `VOLLI_SESSION` (reusing the PTY's own id per launch is // the collision this path replaced). Check 3 proves the `volli session // link` seed; this proves the minted one, which nothing else here reaches. diff --git a/apps/desktop/e2e/lib/fake-harness.mjs b/apps/desktop/e2e/lib/fake-harness.mjs index fe654385f..279cb7e82 100644 --- a/apps/desktop/e2e/lib/fake-harness.mjs +++ b/apps/desktop/e2e/lib/fake-harness.mjs @@ -98,6 +98,24 @@ export async function buildFakeHarness(scratchDir, probePath, opts = {}) { // only — see the module header), the fake then puts the pty in raw mode and // blocks forever copying stdin verbatim into a per-session log instead of // exiting; otherwise (every other smoke) it exits 0 right after recording. + // In interactive mode the fake `claude` also does what a real one does at + // boot: fire SessionStart naming the id it was launched with. The app + // holds a wrapper's minted id as pending until that event proves the + // launch (VC-807), so a fake that stayed silent would never be resumable + // by id. Synchronous, so it has landed before the capture loop opens. + const startupHook = + name === "claude" + ? ` fake_sid='' + fake_prev='' + for arg in "$@"; do + if [ "$fake_prev" = "--session-id" ]; then fake_sid=$arg; fi + fake_prev=$arg + done + if [ -n "$fake_sid" ]; then + volli hook claude-code session.started "{\\"session_id\\":\\"$fake_sid\\"}" /dev/null 2>&1 || true + fi +` + : ""; const script = `#!/bin/sh # Fake "${name}" harness for e2e kickoff / interrupt-resume probes. Records # argv, then either exits 0 (default) or blocks copying raw stdin (interactive @@ -110,7 +128,7 @@ probe="\${VOLLI_FAKE_HARNESS_PROBE:-${probe}}" done } >> "$probe" if [ -n "$VOLLI_FAKE_HARNESS_INTERACTIVE_DIR" ] && [ -n "$VOLLI_SESSION" ]; then - stty raw -echo 2>/dev/null +${startupHook} stty raw -echo 2>/dev/null exec cat >> "$VOLLI_FAKE_HARNESS_INTERACTIVE_DIR/$VOLLI_SESSION.stdin" fi exit 0 diff --git a/apps/desktop/src/main/harness-runtime.test.ts b/apps/desktop/src/main/harness-runtime.test.ts index 5354f3a6a..b26062eb2 100644 --- a/apps/desktop/src/main/harness-runtime.test.ts +++ b/apps/desktop/src/main/harness-runtime.test.ts @@ -600,6 +600,10 @@ describe("ensureHarnessRuntime", () => { * live PTY's real environment; this runs it in milliseconds, in CI. */ /** Drops a stand-in `volli` shim where the wrapper will look for the real one. */ +function pause(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + async function installFakeVolli(shimPath: string, script: string): Promise { await mkdir(join(shimPath, ".."), { recursive: true }); await writeFile(shimPath, script, { mode: 0o755 }); @@ -881,6 +885,111 @@ describe("the generated wrapper, run", () => { }); expect(elsewhere.config).toBe("unset"); }); + + /** A built-in harness's real wrapper over a fake binary, and a `volli` that logs every call. */ + async function builtInRuntime(id: string): Promise<{ + wrapperPath: string; + session: Record; + }> { + const paths = await scratch(); + const adapter = adapterFor(id); + const binaryPath = await fakeBinary(adapter.command); + const runtime = await ensureHarnessRuntime({ + ...paths, + socketPath: "/tmp/volli.sock", + adapters: [adapter], + adapterCensus: "complete", + resolveCommand: () => Promise.resolve(binaryPath), + }); + const wrapperPath = runtime.wrapperPaths.get(id as HarnessId); + if (wrapperPath === undefined) throw new Error("no wrapper was written"); + await installFakeVolli(paths.shimPath, FAKE_VOLLI); + return { wrapperPath, session: mintingSession({ ...runtime.env, PATH: "/nonexistent" }) }; + } + + /** Every call the wrappers made to `volli`, in order. */ + async function volliCalls(): Promise { + const log = await readFile(join(scratchRoot, "mint-log"), "utf8").catch(() => ""); + return log.split("\n").filter(Boolean); + } + + /** + * The calls once `expected` of them have landed, and a moment more for any + * straggler. The announce is detached by design, so a launch that makes one + * is waited for rather than read once. + */ + async function volliCallsSettled(expected: number): Promise { + for (let attempt = 0; attempt < 100; attempt += 1) { + if ((await volliCalls()).length >= expected) break; + await pause(20); + } + await pause(250); + return volliCalls(); + } + + // VC-807. Each of these went through the wrapper and was minted for as a + // launch: the phantom id overwrote the seed of the conversation the user + // really had, and Resume session ran `claude --resume `. + it.each([ + [["--version"]], + [["-v"]], + [["--help"]], + [["mcp", "list"]], + [["doctor"]], + [["update"]], + [["install"]], + ])("asks the app for nothing when claude is run as `claude %j`", async (words) => { + const { wrapperPath, session } = await builtInRuntime("claude-code"); + + const run = await runWrapper(wrapperPath, words, session); + const launch = await runWrapper(wrapperPath, ["hello"], session); + + expect(run.code).toBe(0); + expect(run.args).not.toContain("--session-id"); + expect(run.args.slice(-words.length)).toEqual(words); + // The real launch after it still mints, and is the only call made. + expect(launch.args).toContain("--session-id"); + expect(await volliCalls()).toEqual(["session harness claude-code --mint"]); + }); + + // VC-807. The announce for `codex --help` repointed the running harness at + // codex while keeping claude's id, so resume built `codex resume `. + it.each([ + ["codex", ["--help"]], + ["codex", ["login"]], + ["opencode", ["--version"]], + ])("announces nothing when %s is run with %j", async (id, words) => { + const { wrapperPath, session } = await builtInRuntime(id); + + const run = await runWrapper(wrapperPath, words, session); + await runWrapper(wrapperPath, ["hello"], session); + + expect(run.code).toBe(0); + expect(run.args.slice(-words.length)).toEqual(words); + expect(await volliCallsSettled(1)).toEqual([`session harness ${id}`]); + }); + + // VC-807. A `claude -p` fired by a live claude's own Bash tool inherits PATH + // and `VOLLI_SESSION`. It must not mint over the parent's seed, nor carry the + // injected hooks that would report its turns as the parent's. + it.each([ + ["CLAUDECODE", "1"], + ["CLAUDE_CODE_ENTRYPOINT", "cli"], + ])( + "runs a claude nested inside a running claude (%s set) exactly as outside Volli", + async (marker, value) => { + const { wrapperPath, session } = await builtInRuntime("claude-code"); + + const run = await runWrapper(wrapperPath, ["-p", "hello"], { + ...session, + [marker]: value, + }); + + expect(run.code).toBe(0); + expect(run.args).toEqual(["-p", "hello"]); + expect(await volliCalls()).toEqual([]); + }, + ); }); describe("harnessLaunchArgv", () => { diff --git a/packages/host-core/src/agent-commands.test.ts b/packages/host-core/src/agent-commands.test.ts index 99bfeb4ba..355bbb909 100644 --- a/packages/host-core/src/agent-commands.test.ts +++ b/packages/host-core/src/agent-commands.test.ts @@ -6,7 +6,10 @@ import { afterEach, describe, expect, it, vi } from "vite-plus/test"; import { ACTIVITY_METADATA_KEY, + buildHarnessResumeCommand, DEFAULT_CODE_MODE_LIMITS, + effectiveHarnessId, + getHarnessAdapter, sanitizeMcpToolDefinition, makeAgentError, MUTATION_PLAN_CONTRACT, @@ -104,6 +107,14 @@ const ACTING_ENV: AgentRequest["ctx"]["env"] = { token: ACTING_TOKEN, }; +/** The minted id a mint response carries back to the wrapper. */ +function mintedId(response: AgentResponse): string { + if (!response.ok) throw new Error("the mint was refused"); + const id = (response.data as { harnessSessionId?: unknown }).harnessSessionId; + if (typeof id !== "string") throw new Error("the mint returned no id"); + return id; +} + /** * An env the door will authenticate as `sessionId`. * @@ -4047,7 +4058,28 @@ describe("agent command service", () => { args: { id, mint: true }, ctx: { cwd: "/repo/volli", env: session ? asSession(session) : {} }, }); - return { announce, mint, notices }; + // The same service, so a mint and the hook that answers it share the + // host's memory of what is pending — exactly as one running app does. + const hook = (args: Record) => + service.execute({ + v: 1, + cmd: "hook", + args, + ctx: { cwd: "/repo/volli", env: asSession(sessionId) }, + }); + return { announce, mint, hook, notices }; + } + + /** What **Resume session** would run for this record — `pty/scope.ts`'s inputs. */ + function resumeLine(): string | null { + const record = getSession(ctx.db, sessionId); + if (record === undefined) return null; + return buildHarnessResumeCommand( + effectiveHarnessId(record), + record.harnessSessionId, + null, + getHarnessAdapter, + ); } // THE BUG. The terminal was opened by opencode; the user quit it and ran @@ -4189,41 +4221,142 @@ describe("agent command service", () => { // reused it handed the second launch in one terminal a byte-identical id — // which cursor, mkdir-ing a directory named after it, refuses with EEXIST. // Every launch asks, and every ask is answered with a new one. - it("mints a fresh v4 id per launch, overwriting the previous launch's seed", async () => { - const { mint } = announceService("cursor"); + it("mints a fresh v4 id per launch", async () => { + const { mint, hook } = announceService("cursor"); const v4 = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; - const first = await mint("cursor"); - const firstId = getSession(ctx.db, sessionId)?.harnessSessionId; - const second = await mint("cursor"); - const secondId = getSession(ctx.db, sessionId)?.harnessSessionId; + const firstId = mintedId(await mint("cursor")); + const secondId = mintedId(await mint("cursor")); - expect(first).toMatchObject({ ok: true }); - expect(second).toMatchObject({ ok: true }); // Cursor validates exactly this shape and rejects a v7. expect(firstId).toMatch(v4); expect(secondId).toMatch(v4); expect(secondId).not.toBe(firstId); - // What the wrapper reads back is what was recorded, or the harness would - // launch under an id no future resume could find. - expect(first).toMatchObject({ ok: true, data: { harnessSessionId: firstId } }); - expect(second).toMatchObject({ ok: true, data: { harnessSessionId: secondId } }); + // The launch that boots names the id it was handed, and that id is what + // a future resume finds. + await hook({ harness: "cursor", event: "session.started", harnessSessionId: secondId }); + expect(getSession(ctx.db, sessionId)?.harnessSessionId).toBe(secondId); }); - it("does not report a minted id when its native write loses the live attachment", async () => { + // VC-807. `claude --version`, `claude mcp list` or any other invocation + // that exits without starting a session still went through the wrapper, + // which minted for it. The mint overwrote the seed of the conversation the + // user actually had, so Resume session ran `claude --resume `. + it("keeps the confirmed seed when a minted launch never proves itself", async () => { + const { mint, hook } = announceService("claude-code"); + await hook({ harness: "claude-code", event: "session.started", harnessSessionId: "seed-a" }); + + const phantom = mintedId(await mint("claude-code")); + + expect(phantom).not.toBe("seed-a"); + expect(getSession(ctx.db, sessionId)?.harnessSessionId).toBe("seed-a"); + expect(resumeLine()).toBe("claude --resume 'seed-a'"); + }); + + // The existing behaviour, kept: a real launch fires SessionStart at boot + // naming the id it was launched with, and that is when it becomes the seed. + it("makes a minted id the seed once the launch's own SessionStart names it", async () => { + const { mint, hook } = announceService("claude-code"); + await hook({ harness: "claude-code", event: "session.started", harnessSessionId: "seed-a" }); + + const minted = mintedId(await mint("claude-code")); + await hook({ harness: "claude-code", event: "session.started", harnessSessionId: minted }); + + expect(getSession(ctx.db, sessionId)?.harnessSessionId).toBe(minted); + expect(resumeLine()).toBe(`claude --resume '${minted}'`); + }); + + // A minted launch of a DIFFERENT harness moves nothing until it proves + // itself — not the seed, and not the harness it is paired with — and then + // moves both together, telling the renderer the record moved. + it("switches the running harness only when the minted launch proves itself", async () => { + const { mint, hook, notices } = announceService("opencode"); + await hook({ harness: "opencode", event: "session.started", harnessSessionId: "ses_open" }); + + const minted = mintedId(await mint("claude-code")); + + expect(getSession(ctx.db, sessionId)?.activeHarnessId).toBeNull(); + expect(getSession(ctx.db, sessionId)?.harnessSessionId).toBe("ses_open"); + expect(resumeLine()).toBe("opencode --session 'ses_open'"); + // Still a launch: the renderer's grace window is anchored to it. + expect(notices).toEqual([ + expect.objectContaining({ harnessId: "claude-code", changed: false }), + ]); + + await hook({ harness: "claude-code", event: "session.started", harnessSessionId: minted }); + + expect(getSession(ctx.db, sessionId)?.activeHarnessId).toBe("claude-code"); + expect(resumeLine()).toBe(`claude --resume '${minted}'`); + expect(notices).toEqual([ + expect.objectContaining({ harnessId: "claude-code", changed: false }), + expect.objectContaining({ harnessId: "claude-code", changed: true }), + ]); + }); + + // A startup event that carries no id still proves the launch; the id it was + // handed on argv is then the one it is running under. + it("accepts a startup event that names no id as proof of the minted launch", async () => { + const { mint, hook } = announceService("cursor"); + + const minted = mintedId(await mint("cursor")); + await hook({ harness: "cursor", event: "session.started" }); + + expect(getSession(ctx.db, sessionId)?.harnessSessionId).toBe(minted); + }); + + // A later launch supersedes a pending one, so a straggling hook from the + // launch that never proved itself cannot repoint the harness. + it("forgets a pending mint once another launch is announced", async () => { + const { mint, announce, hook } = announceService("claude-code"); + + await mint("cursor"); + await announce("codex"); + await hook({ harness: "cursor", event: "session.started" }); + + expect(getSession(ctx.db, sessionId)?.activeHarnessId).toBe("codex"); + }); + + // VC-807. `codex --help` (or a real codex closed before its first turn, + // which reports nothing) after a claude session repointed the running + // harness and kept claude's id, so Resume session built + // `codex resume ` — the wrong binary with the wrong id. + it("drops a foreign seed when an announce changes harness without a mint", async () => { + const { announce, hook } = announceService("claude-code"); + await hook({ harness: "claude-code", event: "session.started", harnessSessionId: "seed-a" }); + + await announce("codex"); + + expect(getSession(ctx.db, sessionId)?.activeHarnessId).toBe("codex"); + expect(getSession(ctx.db, sessionId)?.harnessSessionId).toBeNull(); + expect(resumeLine()).toBe("codex resume --last"); + }); + + it("keeps the seed when an announce names the harness already running", async () => { + const { announce, hook } = announceService("codex"); + await hook({ harness: "codex", event: "turn.completed", harnessSessionId: "thread-c" }); + + await announce("codex"); + + expect(resumeLine()).toBe("codex resume 'thread-c'"); + }); + + it("does not hand out an id when the attachment closes under the mint", async () => { announceService("cursor"); const sessionEngine = createTestSessionEngine(ctx.db); - const observe = sessionEngine.observe.bind(sessionEngine); - let closeAfterHarnessWrite = true; - sessionEngine.observe = async (observation) => { - const event = await observe(observation); - if (closeAfterHarnessWrite && observation.kind === "attachment.native_referenced") { - closeAfterHarnessWrite = false; - await observe({ + const getSessionProjection = sessionEngine.getSession.bind(sessionEngine); + let closeAfterFirstRead = true; + // The verb's own read sees the terminal open; it closes before the + // native read-modify-write that the mint is decided under. + sessionEngine.getSession = async (query) => { + const projection = await getSessionProjection(query); + const attachment = projection?.attachments.at(-1); + if (closeAfterFirstRead && attachment !== undefined) { + closeAfterFirstRead = false; + await sessionEngine.observe({ id: "close-before-mint", kind: "attachment.closed", sessionId, - attachmentId: observation.attachmentId, + attachmentId: attachment.id, occurredAt: 5000, provenance: { source: { kind: "system", id: "test", detail: null }, @@ -4232,7 +4365,7 @@ describe("agent command service", () => { outcome: "interrupted", }); } - return event; + return projection; }; const service = createAgentCommandService({ db: ctx.db, diff --git a/packages/host-core/src/agent-commands.ts b/packages/host-core/src/agent-commands.ts index bba036568..593d1d2a7 100644 --- a/packages/host-core/src/agent-commands.ts +++ b/packages/host-core/src/agent-commands.ts @@ -31,7 +31,11 @@ import type { import { AGENT_VERB_TABLE } from "./agent-dispatch/table"; import { coordinationRefusal, socketHandlerPolicy } from "./agent-dispatch/admission"; import { isHostHandlerMap } from "./handlers/handler-map"; -import type { AgentCommandContext, EnvSessionIdentity } from "./agent-dispatch/context"; +import type { + AgentCommandContext, + EnvSessionIdentity, + PendingHarnessLaunch, +} from "./agent-dispatch/context"; import { agentCommandPreflight } from "./agent-dispatch/preview"; import { doorActor, requestActor } from "./agent-dispatch/resolution"; import type { DoorActor } from "./agent-dispatch/resolution"; @@ -85,6 +89,14 @@ export function createAgentCommandService( const worktreeExists = options.worktreeExists ?? existsSync; const sessionEngine = options.sessionEngine; const terminalUpdateLocks = new Map>(); + /** + * The launch each Session's wrapper minted an id for that has not yet proved + * itself (VC-807). In memory, like the watermarks: a pending launch is worth + * something for the second or so between a wrapper's mint and the harness's + * own startup hook, and a restart in between costs that one launch its + * promotion — its hooks still write the seed they report. + */ + const pendingLaunches = new Map(); /** * How this door decides whether `VOLLI_SESSION` is a claim or an * authentication (VC-163). @@ -244,6 +256,7 @@ export function createAgentCommandService( sessionEngine, watermarks, terminalUpdateLocks, + pendingLaunches, projects, loadProjections, loadSessions, diff --git a/packages/host-core/src/agent-dispatch/context.ts b/packages/host-core/src/agent-dispatch/context.ts index bd123a79b..03d28df32 100644 --- a/packages/host-core/src/agent-dispatch/context.ts +++ b/packages/host-core/src/agent-dispatch/context.ts @@ -29,6 +29,7 @@ import type { AgentResponse, AuthorityPolicy, DoctorFacts, + HarnessId, ModelAccessSnapshot, Project, PromptResource, @@ -330,6 +331,16 @@ export interface EnvSessionIdentity { role: SessionRole; } +/** + * A launch a wrapper minted a harness session id for, held until the harness + * proves the launch happened (VC-807). Until then it is a promise, not a fact: + * `claude --version` mints too, and never starts a session. + */ +export interface PendingHarnessLaunch { + readonly harnessId: HarnessId; + readonly harnessSessionId: string; +} + /** * The service state and request-scoped snapshot one verb handler runs against. * @@ -370,6 +381,12 @@ export interface AgentCommandContext { * Session are exactly what it exists to serialize. */ readonly terminalUpdateLocks: Map>; + /** + * The launch each Session's wrapper last minted an id for and that has not + * yet proved itself — see `sessionHarnessVerb`. Service-lived: the mint and + * the startup hook that confirms it arrive on separate requests. + */ + readonly pendingLaunches: Map; /** Every registered project, listed once per request. */ readonly projects: readonly Project[]; /** diff --git a/packages/host-core/src/agent-dispatch/harness-verbs.ts b/packages/host-core/src/agent-dispatch/harness-verbs.ts index 010830af4..9eba3ba2e 100644 --- a/packages/host-core/src/agent-dispatch/harness-verbs.ts +++ b/packages/host-core/src/agent-dispatch/harness-verbs.ts @@ -46,7 +46,7 @@ import { terminalSessionRecord, } from "../session-control"; import { failure } from "./context"; -import type { AgentCommandContext } from "./context"; +import type { AgentCommandContext, PendingHarnessLaunch } from "./context"; import { dryRunResponse } from "./preview"; /** @@ -221,6 +221,24 @@ function rememberFiredAt( } } +/** + * Holds `launch` as the one this session is waiting on, evicting the stalest + * once the map is full — the same bound, and the same reason, as + * {@link rememberFiredAt}. + */ +function rememberPendingLaunch( + pending: Map, + sessionId: string, + launch: PendingHarnessLaunch, +): void { + pending.delete(sessionId); + pending.set(sessionId, launch); + if (pending.size > HARNESS_WATERMARK_LIMIT) { + const stalest = pending.keys().next(); + if (!stalest.done) pending.delete(stalest.value); + } +} + /** `volli session link` — the harness's own resume id for this Volli session. */ export async function sessionLinkVerb( context: AgentCommandContext, @@ -276,7 +294,7 @@ export async function sessionHarnessVerb( context: AgentCommandContext, request: AgentRequest, ): Promise { - const { options, sessionEngine, terminalUpdateLocks, now } = context; + const { options, sessionEngine, terminalUpdateLocks, pendingLaunches, now } = context; // The second involuntary channel: a harness's own PATH-shim wrapper, // one step before it execs, saying what is about to run and — when that // harness takes its session id on argv — asking for one. Addressed by @@ -299,10 +317,12 @@ export async function sessionHarnessVerb( // id is only worth anything once it is written down — `harness_session_id` // is what every future resume of this session reads — so a shell that // minted its own would still have to call in to report it. One call - // does both. It is minted per LAUNCH, not per session: `VOLLI_SESSION` - // is stamped once per PTY, and a harness that treats its session id as - // single-use per workspace (cursor mkdirs a directory named after it) - // fails outright on the second launch in one terminal. + // does both — and the host holds the id as pending until the launch's own + // hook names it, which is when it is written. It is minted per LAUNCH, + // not per session: `VOLLI_SESSION` is stamped once per PTY, and a harness + // that treats its session id as single-use per workspace (cursor mkdirs a + // directory named after it) fails outright on the second launch in one + // terminal. const sessionId = request.ctx.env.session; if (!sessionId) { return failure("CONTEXT_REQUIRED", "session harness requires VOLLI_SESSION context."); @@ -353,13 +373,39 @@ export async function sessionHarnessVerb( // `delivered` channel, and could never be judged silent. `changed` // stays ON the notice — the renderer still needs it to decide whether // to repoint the durable record's labels — but it stops being the gate. + // + // Only when the caller asked for an id is one minted. The wrapper asks + // exactly when the adapter it was rendered from takes an id on argv, and + // that is the only place the answer is known — a registered manifest's + // adapter never reaches main's own table, so main cannot re-derive it + // here. For a `reported` or `none` harness a minted id would overwrite the + // resume seed the harness's own events are about to write with one it + // never heard of. + const mintRequested = request.args["mint"] === true; + // Whatever this launch is, it supersedes one still waiting to prove + // itself: a straggling hook from that one may not promote it now. + pendingLaunches.delete(session.id); let changed = false; const nativeUpdate = await updateTerminalNative( terminalUpdateLocks, sessionEngine, session, (detail) => { + // A MINTED launch moves nothing yet (VC-807). The wrapper mints for + // every invocation it cannot tell apart from a launch, and one that + // never starts a session (`claude --version` before the wrapper knew + // better, a subcommand its adapter does not list) used to overwrite + // the seed of the conversation the user really had, and repoint the + // running harness at it. The id is held as pending below and lands, + // harness and seed together, when the launch's own hook names it. + if (mintRequested) return detail; changed = harnessId !== (detail.activeHarnessId ?? detail.harnessId); + // A DIFFERENT harness with no minted id of its own (codex, opencode, a + // claude driving its own resume) takes over the record, and the seed + // it inherits is the previous harness's: `codex resume ` is the wrong binary handed the wrong id. Cleared, so resume + // falls back to the new harness's own `latest` until it reports one. + if (changed) return { ...detail, activeHarnessId: harnessId, harnessSessionId: null }; return detail.activeHarnessId === harnessId ? detail : { ...detail, activeHarnessId: harnessId }; @@ -368,6 +414,14 @@ export async function sessionHarnessVerb( ); if (nativeUpdate === null) return failure("SESSION_ENDED", `Session ${shortSessionId(session.id)} has ended.`); + // `randomUUID` directly, not the injectable `newId` seam: the FORMAT is + // the contract, not an implementation detail a test may vary. Cursor + // validates strict v4 (`4` in the third group, `[89ab]` in the fourth) + // and refuses a v7. Minted per launch, still, and never reused. + const minted = mintRequested ? randomUUID() : null; + if (minted !== null) { + rememberPendingLaunch(pendingLaunches, session.id, { harnessId, harnessSessionId: minted }); + } options.onSessionHarness?.({ sessionId: session.id, projectId: session.projectId, @@ -376,32 +430,6 @@ export async function sessionHarnessVerb( changed, at: now(), }); - // Only when the caller asked. The wrapper asks exactly when the adapter - // it was rendered from takes an id on argv, and that is the only place - // the answer is known — a registered manifest's adapter never reaches - // main's own table, so main cannot re-derive it here. For a `reported` - // or `none` harness a minted id would overwrite the resume seed the - // harness's own events are about to write with one it never heard of. - // - // `randomUUID` directly, not the injectable `newId` seam: the FORMAT is - // the contract, not an implementation detail a test may vary. Cursor - // validates strict v4 (`4` in the third group, `[89ab]` in the fourth) - // and refuses a v7. - const minted = request.args["mint"] === true ? randomUUID() : null; - if (minted !== null) { - // Overwrites, always. The seed belongs to the launch that is starting - // now; the previous one describes an agent this terminal has quit. - const mintedUpdate = await updateTerminalNative( - terminalUpdateLocks, - sessionEngine, - nativeUpdate, - (detail) => ({ ...detail, harnessSessionId: minted }), - now(), - ); - if (mintedUpdate === null) { - return failure("SESSION_ENDED", `Session ${shortSessionId(session.id)} has ended.`); - } - } return { v: 1, ok: true, @@ -419,7 +447,15 @@ export async function hookVerb( context: AgentCommandContext, request: AgentRequest, ): Promise { - const { options, projects, sessionEngine, terminalUpdateLocks, watermarks, now } = context; + const { + options, + projects, + sessionEngine, + terminalUpdateLocks, + watermarks, + pendingLaunches, + now, + } = context; // The involuntary channel (harness-events): a hook the wrapper // configured, not something the agent chose to run. `VOLLI_SESSION` is // both the addressing and the harness's own session id, so an event @@ -481,12 +517,57 @@ export async function hookVerb( const firedAt = harnessEventOrder(request.args["firedAt"]); const superseded = supersededHarnessEvent(watermarks.get(session.id) ?? null, firedAt); if (firedAt !== null && !superseded) rememberFiredAt(watermarks, session.id, firedAt); - // A reported id is what replaces `session link` on the critical path. - // Idempotent overwrite, exactly as `session.link` is — a harness that - // rotates its id mid-session leaves the newest resume seed behind. Which - // is precisely why a superseded one may not be written: newest by - // arrival is not newest, and this row seeds every future resume. - if (harnessSessionId !== null && !superseded) { + // THE PROOF A MINTED LAUNCH WAS WAITING FOR (VC-807). The wrapper's mint + // is held pending in `sessionHarnessVerb`; the launch is real once the + // harness it was minted for fires its startup event, or a hook naming the + // id it was handed. Then, and only then, the record moves — the running + // harness and its seed together, so neither is ever paired with the + // other's. The id the event reports wins over the one minted for it: it is + // what the harness says it is running under. + const pending = pendingLaunches.get(session.id); + const proved = + pending !== undefined && + !superseded && + firing.harnessId === pending.harnessId && + (harnessSessionId === pending.harnessSessionId || event === "session.started"); + let current = session; + if (proved) { + const launch = pending; + pendingLaunches.delete(session.id); + let moved = false; + const promoted = await updateTerminalNative( + terminalUpdateLocks, + sessionEngine, + session, + (detail) => { + moved = launch.harnessId !== (detail.activeHarnessId ?? detail.harnessId); + return { + ...detail, + activeHarnessId: launch.harnessId, + harnessSessionId: harnessSessionId ?? launch.harnessSessionId, + }; + }, + now(), + ); + if (promoted !== null) current = promoted; + // The mint's own notice said `changed: false`, because nothing durable + // had moved. Now it has, and the renderer's caches repoint on this. + if (promoted !== null && moved) { + options.onSessionHarness?.({ + sessionId: session.id, + projectId: session.projectId, + ticketId: session.ticketId, + harnessId: launch.harnessId, + changed: true, + at: now(), + }); + } + } else if (harnessSessionId !== null && !superseded) { + // A reported id is what replaces `session link` on the critical path. + // Idempotent overwrite, exactly as `session.link` is — a harness that + // rotates its id mid-session leaves the newest resume seed behind. Which + // is precisely why a superseded one may not be written: newest by + // arrival is not newest, and this row seeds every future resume. await updateTerminalNative( terminalUpdateLocks, sessionEngine, @@ -546,7 +627,7 @@ export async function hookVerb( event === "input.needed" && status === "verified" && declaresInputNeeded(firing.harnessId) && - firing.harnessId === effectiveHarnessId(session) && + firing.harnessId === effectiveHarnessId(current) && !superseded ) { const ticket = session.ticketId ? getTicket(options.db, session.ticketId) : undefined; @@ -562,7 +643,7 @@ export async function hookVerb( options.notify?.({ producer: "harness-input-needed", title: `${subject} needs you`, - body: `${harnessLabel(effectiveHarnessId(session))} is waiting on a human`, + body: `${harnessLabel(effectiveHarnessId(current))} is waiting on a human`, target: { kind: "session", projectId: session.projectId, diff --git a/packages/shared/src/harness/claude-code.ts b/packages/shared/src/harness/claude-code.ts index 34ff9536c..80e2e8902 100644 --- a/packages/shared/src/harness/claude-code.ts +++ b/packages/shared/src/harness/claude-code.ts @@ -68,4 +68,21 @@ export const claudeCodeAdapter: HarnessAdapter = { "CLAUDE_CODE_EXECPATH", "CLAUDE_PID", ], + // Read off `claude --help`, 2.1.x. Housekeeping only: the background-session + // verbs (`attach`, `agents`, `stop`, ...) are left out, and so is `config`, + // which this version no longer has (it would be read as a prompt). + nonSessionArgv: { + flags: ["--version", "-v", "--help", "-h"], + subcommands: [ + "mcp", + "doctor", + "update", + "upgrade", + "install", + "auth", + "setup-token", + "plugin", + "plugins", + ], + }, }; diff --git a/packages/shared/src/harness/codex.ts b/packages/shared/src/harness/codex.ts index de354f770..71ef53f0c 100644 --- a/packages/shared/src/harness/codex.ts +++ b/packages/shared/src/harness/codex.ts @@ -122,4 +122,27 @@ export const codexAdapter: HarnessAdapter = { // Empty until a marker is observed on a real codex session — a guessed name is // a variable deleted from the user's environment for no reason. sessionMarkers: [], + // Read off `codex --help`. Its version flag is `-V`; `-v` is not a codex flag. + // `exec`, `review`, `resume`, `fork` and the servers run sessions, so they are + // launches and stay off this list. + nonSessionArgv: { + flags: ["--version", "-V", "--help", "-h"], + subcommands: [ + "login", + "logout", + "mcp", + "plugin", + "completion", + "update", + "doctor", + "sandbox", + "debug", + "apply", + "archive", + "delete", + "unarchive", + "features", + "help", + ], + }, }; diff --git a/packages/shared/src/harness/cursor.ts b/packages/shared/src/harness/cursor.ts index 9c7ec0ce2..fe0c89d84 100644 --- a/packages/shared/src/harness/cursor.ts +++ b/packages/shared/src/harness/cursor.ts @@ -71,4 +71,25 @@ export const cursorAdapter: HarnessAdapter = { // Empty until a marker is observed on a real cursor-agent session — a guessed // name is a variable deleted from the user's environment for no reason. sessionMarkers: [], + // Read off `cursor-agent --help`. `agent`, `ls` and `resume` start or reopen + // a chat, so they are launches and stay off this list. + nonSessionArgv: { + flags: ["--version", "-v", "--help", "-h"], + subcommands: [ + "install-shell-integration", + "uninstall-shell-integration", + "login", + "logout", + "mcp", + "plugin", + "status", + "whoami", + "models", + "bedrock", + "about", + "update", + "create-chat", + "help", + ], + }, }; diff --git a/packages/shared/src/harness/opencode.ts b/packages/shared/src/harness/opencode.ts index bc00038d2..121e4bbb4 100644 --- a/packages/shared/src/harness/opencode.ts +++ b/packages/shared/src/harness/opencode.ts @@ -57,4 +57,27 @@ export const opencodeAdapter: HarnessAdapter = { // Empty until a marker is observed on a real opencode session — a guessed name // is a variable deleted from the user's environment for no reason. sessionMarkers: [], + // Read off `opencode --help`. `run`, `serve`, `web`, `acp`, `attach` and `pr` + // run sessions, so they are launches and stay off this list. + nonSessionArgv: { + flags: ["--version", "-v", "--help", "-h"], + subcommands: [ + "completion", + "mcp", + "debug", + "providers", + "auth", + "agent", + "upgrade", + "uninstall", + "models", + "stats", + "export", + "import", + "session", + "plugin", + "plug", + "db", + ], + }, }; diff --git a/packages/shared/src/harness/types.ts b/packages/shared/src/harness/types.ts index 7913bf6c4..b5fe5f60d 100644 --- a/packages/shared/src/harness/types.ts +++ b/packages/shared/src/harness/types.ts @@ -345,6 +345,27 @@ export interface HarnessAdapter { * name too many costs the user a terminal that cannot authenticate. */ readonly sessionMarkers: readonly string[]; + /** + * The invocations that only ask the harness about itself and exit without + * starting a session — `claude --version`, `codex --help`, `claude mcp list`. + * The wrapper still runs for them, so without this it announced each one as a + * launch: the running harness moved, a mint was spent on a launch that never + * happened, and the grace window started for a channel nothing would use. + * + * `flags` match any word the user typed, because a CLI answers `--help` and + * `--version` wherever they appear. `subcommands` match only the FIRST word, + * which is the only place a CLI reads one. + * + * List only what the installed CLI really treats this way. A word that is not + * one of its subcommands is a PROMPT, and a launch whose prompt matched here + * would start a real session that was never announced. Absent means the + * harness declares none, which is what a registered manifest gets: Volli + * cannot know another CLI's argv, and `-v` is not `--version` everywhere. + */ + readonly nonSessionArgv?: { + readonly flags: readonly string[]; + readonly subcommands: readonly string[]; + }; } /** diff --git a/packages/shared/src/harness/wrapper.test.ts b/packages/shared/src/harness/wrapper.test.ts index e6c88ab24..c4328370e 100644 --- a/packages/shared/src/harness/wrapper.test.ts +++ b/packages/shared/src/harness/wrapper.test.ts @@ -207,7 +207,7 @@ describe("renderWrapperScript", () => { }); it("gates both calls on there being an app to ask", () => { - expect(claude).toContain('if [ -n "${VOLLI_SOCKET:-}" ]; then'); + expect(claude).toContain('&& [ -n "${VOLLI_SOCKET:-}" ]; then'); }); // A harness TUI is about to own this terminal: neither call may print into @@ -248,6 +248,70 @@ describe("renderWrapperScript", () => { ); }); + // VC-807. `claude --version`, `claude mcp list` and `codex --help` went + // through the wrapper and were announced as launches: the running harness + // moved and a mint overwrote the resume seed for a session that never began. + it("tells Volli nothing about an invocation that only asks the harness about itself", () => { + expect(claude).toContain( + " case $volli_arg in\n '--version'|'-v'|'--help'|'-h')\n volli_launch=0\n", + ); + expect(claude).toContain( + `case "\${1:-}" in\n 'mcp'|'doctor'|'update'|'upgrade'|'install'|'auth'|'setup-token'|'plugin'|'plugins')\n volli_launch=0\n`, + ); + expect(claude).toContain('if [ "$volli_launch" = 1 ] && [ -n "${VOLLI_SOCKET:-}" ]; then'); + const codex = renderWrapperScript(adapterFor("codex"), BARE); + expect(codex).toContain("'--version'|'-V'|'--help'|'-h')"); + expect(codex).toContain('if [ "$volli_launch" = 1 ] && [ -n "${VOLLI_SOCKET:-}" ]; then'); + const opencode = renderWrapperScript(adapterFor("opencode"), BARE); + expect(opencode).toContain("'--version'|'-v'|'--help'|'-h')"); + }); + + it("scans nothing for a harness that declares no such invocation", () => { + const bare = renderWrapperScript(bareAdapter(), BARE); + expect(bare).not.toContain("volli_launch"); + expect(bare).toContain('if [ -n "${VOLLI_SOCKET:-}" ]; then'); + }); + + it("renders only the half of the scan a harness declares", () => { + const flagsOnly = renderWrapperScript( + bareAdapter({ nonSessionArgv: { flags: ["--version"], subcommands: [] } }), + BARE, + ); + expect(flagsOnly).toContain("'--version')"); + expect(flagsOnly).not.toContain('case "${1:-}" in'); + const subcommandsOnly = renderWrapperScript( + bareAdapter({ nonSessionArgv: { flags: [], subcommands: ["login"] } }), + BARE, + ); + expect(subcommandsOnly).toContain(" 'login')"); + expect(subcommandsOnly).not.toContain('for volli_arg in "$@"; do'); + expect(subcommandsOnly).toContain('if [ "$volli_launch" = 1 ]'); + }); + + // VC-807. A `claude -p` or `claude mcp list` run by a live claude's own Bash + // tool inherits `VOLLI_SESSION`. It must not announce, mint, or carry the + // hooks that would report its turns as the parent's. + it("hands a run nested inside a running harness straight to the real binary", () => { + const lines = claude.split("\n"); + const gate = lines.findIndex((line) => line.includes('[ -n "${CLAUDECODE:-}" ]')); + expect(lines[gate]).toContain('[ -n "${CLAUDE_CODE_ENTRYPOINT:-}" ] ||'); + expect(lines.slice(gate + 1, gate + 3)).toEqual([' exec "$volli_real" "$@"', "fi"]); + // After the outside-Volli passthrough, before anything Volli adds. + expect(gate).toBeGreaterThan(lines.indexOf('if [ -z "${VOLLI_SESSION:-}" ]; then')); + expect(gate).toBeLessThan(lines.findIndex((line) => line.includes("session harness"))); + expect(gate).toBeLessThan(lines.findIndex((line) => line.includes("VOLLI_HARNESS_ARGV_"))); + }); + + it("skips a marker that is not a shell name rather than render a broken wrapper", () => { + const script = renderWrapperScript( + bareAdapter({ sessionMarkers: ["NOT-A-NAME", "MY_HARNESS_RUN"] }), + BARE, + ); + expect(script).toContain('if [ -n "${MY_HARNESS_RUN:-}" ]; then'); + expect(script).not.toContain("NOT-A-NAME"); + expect(renderWrapperScript(bareAdapter(), BARE)).not.toContain("Nested inside"); + }); + it("quotes a slug and a cli path so neither can become shell", () => { const odd = renderWrapperScript(bareAdapter(), { ...BARE, diff --git a/packages/shared/src/harness/wrapper.ts b/packages/shared/src/harness/wrapper.ts index b294c7ba4..9a4f420f6 100644 --- a/packages/shared/src/harness/wrapper.ts +++ b/packages/shared/src/harness/wrapper.ts @@ -78,6 +78,9 @@ function resumePattern(token: string): string { return `${shellSingleQuote(token)}|${shellSingleQuote(`${token}=`)}*`; } +/** A name the shell can expand as `${NAME:-}`. */ +const SHELL_NAME = /^[A-Za-z_][A-Za-z0-9_]*$/; + /** * The wrapper for `adapter`, to be written into `binDir`. * @@ -96,7 +99,10 @@ function resumePattern(token: string): string { * the id has to be WRITTEN somewhere to be worth anything — `harness_session_id` * is what every future resume reads — and a `/bin/sh` that minted its own would * still have to make a second call to report it, in a script whose one job is - * to get out of the way of the agent it is about to exec. + * to get out of the way of the agent it is about to exec. The app holds the id + * as PENDING until the launch proves itself with its own startup hook, so an + * invocation that never becomes a session cannot overwrite the seed of one that + * did. * * Failure degrades to no id at all: the session flag is omitted and the harness * launches unpinned (no resume seed for that launch), because the alternative — @@ -170,6 +176,29 @@ export function renderWrapperScript(adapter: HarnessAdapter, input: WrapperInput "", ); + // A run NESTED inside a running harness of this kind — `claude -p` or + // `claude mcp list` fired by a live claude's own Bash tool. It inherits PATH + // and `VOLLI_SESSION`, so it lands here, but the Volli session belongs to the + // run that spawned it: announcing would repoint that session, minting would + // spend its resume seed, and the injected hooks would report the nested run's + // turns as the parent's. So it gets exactly what a harness outside Volli gets. + // + // The harness's own markers are the evidence. Volli scrubs them from every + // terminal it spawns (`scrubInheritedSessionEnv`), so here they can only have + // come from a running harness above this process. A name that is not a shell + // name is skipped rather than rendered: `${…}` around it would be a syntax + // error, and a wrapper that cannot parse is a harness that cannot launch. + const markers = adapter.sessionMarkers.filter((name) => SHELL_NAME.test(name)); + if (markers.length > 0) { + lines.push( + `# Nested inside a running ${adapter.label}: this session is that run's, not ours.`, + `if ${markers.map((name) => `[ -n "\${${name}:-}" ]`).join(" || ")}; then`, + ' exec "$volli_real" "$@"', + "fi", + "", + ); + } + const envEntries = Object.entries(input.env); if (envEntries.length > 0) { lines.push( @@ -239,8 +268,47 @@ export function renderWrapperScript(adapter: HarnessAdapter, input: WrapperInput // its own wait on the socket, so a wedged app costs a moment, not the agent. const mint = `volli_harness_session=$(${verb} --mint /dev/null) || volli_harness_session=''`; + // An invocation that only asks the harness about itself — `--version`, + // `--help`, `mcp list` — is not a launch, and neither call may treat it as + // one: the announce would repoint the running harness, and the mint would + // hand a launch that never happens the resume seed. See + // {@link HarnessAdapter.nonSessionArgv}. Everything else about the + // invocation is untouched; only Volli stops being told about it. + const nonSessionFlags = adapter.nonSessionArgv?.flags ?? []; + const nonSessionSubcommands = adapter.nonSessionArgv?.subcommands ?? []; + const scansForNonSession = nonSessionFlags.length > 0 || nonSessionSubcommands.length > 0; + if (scansForNonSession) { + lines.push("# Asking the harness about itself is not a launch.", "volli_launch=1"); + if (nonSessionFlags.length > 0) { + lines.push( + 'for volli_arg in "$@"; do', + " case $volli_arg in", + ` ${nonSessionFlags.map((flag) => shellSingleQuote(flag)).join("|")})`, + " volli_launch=0", + " break", + " ;;", + " esac", + "done", + ); + } + if (nonSessionSubcommands.length > 0) { + lines.push( + 'case "${1:-}" in', + ` ${nonSessionSubcommands.map((word) => shellSingleQuote(word)).join("|")})`, + " volli_launch=0", + " ;;", + "esac", + ); + } + lines.push(""); + } + if (sessionIdFlag !== null) lines.push("volli_harness_session=''"); - lines.push('if [ -n "${VOLLI_SOCKET:-}" ]; then'); + lines.push( + scansForNonSession + ? 'if [ "$volli_launch" = 1 ] && [ -n "${VOLLI_SOCKET:-}" ]; then' + : 'if [ -n "${VOLLI_SOCKET:-}" ]; then', + ); if (sessionIdFlag === null) { lines.push(` ${announce}`); } else if (scansForUserResume) { diff --git a/packages/shared/src/host-events.ts b/packages/shared/src/host-events.ts index 43a5f33b8..7f672727b 100644 --- a/packages/shared/src/host-events.ts +++ b/packages/shared/src/host-events.ts @@ -177,6 +177,9 @@ export interface SessionHarnessNotice { * Whether this announce named a DIFFERENT harness than the session was * already believed to be running. Not a gate on the notice — every launch is * broadcast — but the durable record only has to be repointed when it moved. + * A launch the wrapper minted an id for moves nothing until its own hook + * proves it, so it is announced with `false` and announced again, with + * `true`, when that proof lands and the record moves (VC-807). */ changed: boolean; /** Epoch ms the announce was ingested (main's clock). */