From 4a1061c8b61bbf8f49810cc1e5a99116a6db2967 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 23 Sep 2026 07:07:29 -0700 Subject: [PATCH] fix(grok): retire superseded task sessions and clean actual credential homes Close the previous idle task owner before a reset or plan acceptance starts a new native session. Fence concurrent starts and fail closed on incomplete retirement. Read back and remove Grok credentials from the actual ACPX runtime paths. Align package policy and runner assertions with the new agent contract. Co-Authored-By: Paperclip --- packages/grok-acp/package.json | 3 + .../src/cli/acpx-runtime-sidecar.test.ts | 2 +- .../src/live/live-session.test.ts | 2 +- .../native-session-executor.test.ts | 152 ++++++++++++++++++ .../native-runtime/native-session-executor.ts | 65 +++++++- 5 files changed, 220 insertions(+), 4 deletions(-) diff --git a/packages/grok-acp/package.json b/packages/grok-acp/package.json index 87de39f380d..79d5522de79 100644 --- a/packages/grok-acp/package.json +++ b/packages/grok-acp/package.json @@ -17,5 +17,8 @@ }, "exports": { "./package.json": "./package.json" + }, + "engines": { + "node": ">=24.11.0" } } diff --git a/packages/paperclip-runner/src/cli/acpx-runtime-sidecar.test.ts b/packages/paperclip-runner/src/cli/acpx-runtime-sidecar.test.ts index 489178cdc7c..927631ba91c 100644 --- a/packages/paperclip-runner/src/cli/acpx-runtime-sidecar.test.ts +++ b/packages/paperclip-runner/src/cli/acpx-runtime-sidecar.test.ts @@ -545,7 +545,7 @@ describe("qualified ACPX runtime sidecar", () => { ok: false, error: { code: "acpx_sidecar_command_failed", - message: "ACPX agent must be claude or codex", + message: "ACPX agent must be claude, codex, or grok", retryable: false, }, }); diff --git a/packages/paperclip-runner/src/live/live-session.test.ts b/packages/paperclip-runner/src/live/live-session.test.ts index 00bdc571f24..4f5ab051ded 100644 --- a/packages/paperclip-runner/src/live/live-session.test.ts +++ b/packages/paperclip-runner/src/live/live-session.test.ts @@ -895,7 +895,7 @@ describe("Capability live runnerd and Codex session", () => { (request) => request.method === "thread/start", )?.params.baseInstructions, ).toBe( - "Native instructions\n\nRead-only instruction sibling root: /runtime/instructions", + 'Native instructions\n\nRead-only instruction sibling root: /runtime/instructions\nFor native paperclip_finish/paperclip_block reports, completionContract={"revision":"paperclip-capability-live-v1","criterionIds":["objective"]}. This report does not change mock task state; use the exposed semantic tools for task changes.', ); await service.shutdown(session.id); }); diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 1b68d31c054..fdb5082c886 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -5,6 +5,7 @@ import { mkdir, mkdtemp, readdir, + realpath, readFile, rename, rm, @@ -82,6 +83,7 @@ type BackendFactoryOptions = { }; type RunnerTransportOptions = { + acpxRuntimeDirectory?: string; adoptExistingRunner?: { pid: number; isAlive: () => Promise | boolean }; stateDirectory?: string; runnerBinary?: string; @@ -178,6 +180,12 @@ const state = vi.hoisted(() => ({ release: null as null | (() => void), })); +const grokCopyBack = vi.hoisted(() => vi.fn(async (_input: { readSandboxAuth: () => Promise; hostHomeDir: string }) => undefined)); +vi.mock("@paperclipai/adapter-grok-local/server", async importOriginal => ({ + ...await importOriginal(), + copyBackGrokAuth: grokCopyBack, +})); + vi.mock("../../vendor/paperclip-runner/index.js", async (importOriginal) => { const original = await importOriginal< typeof import("../../vendor/paperclip-runner/index.js") @@ -5729,6 +5737,110 @@ describe("native session same-turn steering", () => { }); describe("native warm session supervision", () => { + describe("warm session identity transitions", () => { + let previousHome: string | undefined; + let isolatedHome: string; + beforeEach(async () => { + previousHome = process.env.PAPERCLIP_HOME; + isolatedHome = await mkdtemp(join(tmpdir(), "native-identity-transition-")); + process.env.PAPERCLIP_HOME = isolatedHome; + }); + afterEach(async () => { + await closeIdleWarmNativeSessionsForRestart(); + if (previousHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = previousHome; + await rm(isolatedHome, { recursive: true, force: true }); + }); + const result = { + result: { summary: "completed" }, terminal: { runTerminalState: "succeeded" }, + turnId: "turn", normalizedSessionId: "old", providerSessionId: "provider", + driverKind: "test", driverVersion: "1", nativeEventCount: 1, + highestContiguousSourceSeq: 1, usage: null, + }; + function fixture(name: string) { + const base = { + ...execution, + binding: { ...execution.binding, runId: `${name}-first`, executionWorkspaceId: name }, + session: { ...execution.session, normalizedSessionId: `${name}-old`, + lifecyclePolicy: { mode: "warm" as const, idleTimeoutMs: 60_000 } }, + } as NativeExecutionInputV1; + const next = { ...base, binding: { ...base.binding, runId: `${name}-second` }, + session: { ...base.session, normalizedSessionId: `${name}-new` } }; + const run = (input: NativeExecutionInputV1) => executePaperclipNativeSession({ + db: leaseDb(input), execution: input, runnerInstanceId: "runner", + }); + return { base, next, run }; + } + + it("awaits prior idle ownership retirement before launching the accepted-plan session", async () => { + const { base, next, run } = fixture("identity-handoff"); + let finishClose!: () => void; + const close = vi.fn(() => new Promise(resolve => { finishClose = resolve; })); + state.execute.mockReset().mockImplementationOnce(async options => { + options.onSession?.({ close }); return result; + }).mockImplementationOnce(async options => { + expect(close).toHaveBeenCalledOnce(); + expect(options.existingSession).toBeUndefined(); + return result; + }); + await run(base); + const replacement = run(next); + await vi.waitFor(() => expect(close).toHaveBeenCalledOnce()); + expect(state.execute).toHaveBeenCalledTimes(1); + await expect(run({ ...next, binding: { ...next.binding, runId: "racing-new-session" } })) + .rejects.toThrow("native_session_supervisor_busy"); + finishClose(); + await replacement; + expect(close).toHaveBeenCalledWith({ reason: "warm native session identity changed" }); + expect(state.execute).toHaveBeenCalledTimes(2); + }); + + it("does not retire an active turn when a new session identity arrives", async () => { + const { base, next, run } = fixture("identity-active"); + let finish!: () => void; + const close = vi.fn(async () => undefined); + state.execute.mockReset().mockImplementationOnce(async options => { + options.onSession?.({ close }); + await new Promise(resolve => { finish = resolve; }); + return result; + }); + const active = run(base); + await vi.waitFor(() => expect(finish).toBeTypeOf("function")); + await expect(run(next)).rejects.toThrow("native_session_supervisor_busy"); + expect(close).not.toHaveBeenCalled(); + finish(); await active; + await closeIdleWarmNativeSessionsForRestart(); + }); + + it("retains failed retirement for retry and never launches over an uncontained owner", async () => { + const { base, next, run } = fixture("identity-close-failure"); + const close = vi.fn().mockRejectedValueOnce(new Error("containment unavailable")).mockResolvedValue(undefined); + state.execute.mockReset().mockImplementationOnce(async options => { + options.onSession?.({ close }); return result; + }).mockResolvedValue(result); + await run(base); + await expect(run(next)).rejects.toThrow("containment unavailable"); + expect(state.execute).toHaveBeenCalledTimes(1); + await run(next); + expect(close).toHaveBeenCalledTimes(2); + expect(state.execute).toHaveBeenCalledTimes(2); + }); + + it.each(["companyId", "agentId", "issueId", "executionWorkspaceId"] as const)( + "does not retire an unrelated %s owner", async field => { + const { base, next, run } = fixture(`identity-isolation-${field}`); + const close = vi.fn(async () => undefined); + state.execute.mockReset().mockImplementationOnce(async options => { + options.onSession?.({ close }); return result; + }).mockResolvedValue(result); + await run(base); + await run({ ...next, binding: { ...next.binding, [field]: `different-${field}` } }); + expect(close).not.toHaveBeenCalled(); + await closeIdleWarmNativeSessionsForRestart(); + }, + ); + }); + it.each([true, false])( "uses provider turn completion without a semantic-result cutoff: chat=%s", async (conversationMode) => { @@ -7657,6 +7769,46 @@ describe("runnerd provider runtime wiring", () => { await expect(readFile(authPath, "utf8")).resolves.toBe(auth); }); + it("copies back and removes Grok credentials from the exact ACPX launch home", async () => { + const priorHome = process.env.PAPERCLIP_HOME; + const privateRoot = await realpath(isolatedStateDirectory); + process.env.PAPERCLIP_HOME = privateRoot; + const managedHome = join(privateRoot, "company-login"); + await mkdir(managedHome, { mode: 0o700 }); + await writeFile(join(managedHome, "auth.json"), "fixture-old-login", { mode: 0o600 }); + const grokExecution = { ...execution, + provider: { kind: "acpx", agent: "grok", model: "grok-4.7", permissionMode: "deny-all" }, + session: { ...execution.session, driverKind: "acpx_runtime" }, + } as unknown as NativeExecutionInputV1; + state.createBackend.mockReturnValueOnce({ + kind: "test", openSession: async () => ({ close: vi.fn(async () => undefined) }), + } as never); + try { + const backend = await createRunnerdBackend({ db: leaseDb(grokExecution), execution: grokExecution, + runnerInstanceId: "grok-cleanup", managedAiCredentialHome: managedHome }); + state.createBackend.mock.calls.at(-1)![1].codexTransportFactory!(); + const runtimeDirectory = state.createTransport.mock.calls.at(-1)![0].acpxRuntimeDirectory!; + const agentHome = join(runtimeDirectory, acpxRuntimeSessionDirectoryName(execution.session.normalizedSessionId!), "grok-home"); + await mkdir(agentHome, { recursive: true, mode: 0o700 }); + for (const name of ["auth.json", "auth-refresh.json", "auth-refresh.json.tmp"]) { + await writeFile(join(agentHome, name), "fixture-refreshed-login", { mode: 0o600 }); + } + grokCopyBack.mockReset().mockImplementationOnce(async input => { + expect(input.hostHomeDir).toBe(managedHome); + expect((await input.readSandboxAuth()).toString()).toBe("fixture-refreshed-login"); + }); + const session = await backend.openSession({} as never); + await session.close({ reason: "completed" }); + expect(grokCopyBack).toHaveBeenCalledOnce(); + for (const name of ["auth.json", "auth-refresh.json", "auth-refresh.json.tmp"]) { + await expect(access(join(agentHome, name))).rejects.toMatchObject({ code: "ENOENT" }); + } + } finally { + if (priorHome === undefined) delete process.env.PAPERCLIP_HOME; + else process.env.PAPERCLIP_HOME = priorHome; + } + }); + it("still cleans up managed Codex credentials after an owned session closes", async () => { const close = vi.fn(async () => undefined); state.copyBackCodexAuth.mockClear(); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index cfc8a360e8c..e15e6ce883c 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -389,6 +389,7 @@ type WarmNativeSession = { session: NativeSession; ownerToken: symbol; configDigest: string; + ownerScope: string; companyId: string; environmentId: string | null; busy: boolean; @@ -1446,6 +1447,7 @@ class SessionToolAuthorityEpoch { const sessionToolAuthorityEpochs = new Map(); const initializingSessionToolAuthorities = new Set(); const executingRunnerdSessionScopes = new Map(); +const executingNativeOwnerScopes = new Map(); function nativeSessionKey(execution: NativeExecutionInput): string { return ( @@ -1517,6 +1519,45 @@ function nativeSessionScopeKey(execution: NativeExecutionInput): string { }); } +// A plan acceptance or explicit reset can rotate the provider/session identity +// while retaining the same task sandbox and its fixed ingress port. +function nativeSessionOwnerScope( + execution: NativeExecutionInput, + environmentId: string | null, +): string { + return canonicalJson({ + companyId: execution.binding.companyId, + agentId: execution.binding.agentId, + issueId: execution.binding.issueId, + workspace: nativeSessionWorkspaceScope(execution), + environmentId, + }); +} + +async function retireSupersededWarmNativeSessions( + ownerScope: string, + nextSessionScope: string, +): Promise { + for (const [scope, entry] of warmNativeSessions) { + if (scope === nextSessionScope || entry.ownerScope !== ownerScope) continue; + if (entry.busy || executingRunnerdSessionScopes.has(scope)) { + throw new Error("native_session_supervisor_busy"); + } + if (entry.idleTimer !== null) clearTimeout(entry.idleTimer); + entry.idleTimer = null; + entry.busy = true; + entry.closeOnReleaseReason = "warm native session identity changed"; + try { + await closeWarmNativeSession(entry, entry.closeOnReleaseReason); + if (warmNativeSessions.get(scope) === entry) warmNativeSessions.delete(scope); + } finally { + // A failed close retains the owner and prevents launch. A later attempt + // must retry retirement, never adopt this partially closed transport. + entry.busy = false; + } + } +} + function legacyCompanyNativeSessionScopeKey( execution: NativeExecutionInput, ): string { @@ -7165,7 +7206,20 @@ export async function executePaperclipNativeSession(input: { let sessionScopeId: string | null = null; let ownsSessionScope = false; let executionFailure: unknown; + const ownerScope = nativeSessionOwnerScope( + input.execution, + input.runnerExecutionTarget?.environmentId ?? null, + ); + const ownerToken = Symbol("native execution owner"); try { + // Reserve across session identities before retiring or staging anything. + if (executingNativeOwnerScopes.has(ownerScope)) { + throw new Error("native_session_supervisor_busy"); + } + executingNativeOwnerScopes.set(ownerScope, ownerToken); + await retireSupersededWarmNativeSessions( + ownerScope, nativeSessionScopeKey(input.execution), + ); if (!input.useRunnerd) { return await executePaperclipNativeSessionWithinScope(input); } @@ -7235,6 +7289,9 @@ export async function executePaperclipNativeSession(input: { executionFailure = error; throw error; } finally { + if (executingNativeOwnerScopes.get(ownerScope) === ownerToken) { + executingNativeOwnerScopes.delete(ownerScope); + } startup.resolve(null); if (nativeSessionStartups.get(runId) === startup) { nativeSessionStartups.delete(runId); @@ -7973,6 +8030,7 @@ async function executePaperclipNativeSessionWithinScope( (hasBrokerCapability && entry.credentialRunId !== input.execution.binding.runId); if ( + entry.closeOnReleaseReason !== undefined || entry.configDigest !== warmConfigDigest || entry.managedAiCredentialIdentity !== input.managedAiCredentialIdentity || credentialRunChanged || @@ -8337,6 +8395,9 @@ async function executePaperclipNativeSessionWithinScope( session, ownerToken: warmSessionOwnerToken, configDigest: warmConfigDigest, + ownerScope: nativeSessionOwnerScope( + input.execution, input.runnerExecutionTarget?.environmentId ?? null, + ), companyId: input.execution.binding.companyId, environmentId: input.runnerExecutionTarget?.environmentId ?? null, @@ -12598,8 +12659,8 @@ async function createRunnerdBackendWithinSessionClaim( if (isGrok && grokCredential?.home && !boundManagedSessions.has(session)) { boundManagedSessions.add(session); const relativeHome = `acpx/${acpxRuntimeSessionDirectoryName(nativeSessionKey(input.execution))}/grok-home`; - const localHome = join(root, "acpx", relativeHome); - const remoteHome = remoteRunnerFilesystemRoot ? posix.join(remoteRunnerFilesystemRoot, "acpx", relativeHome) : null; + const localHome = resolve(resolvePaperclipInstanceRoot(), "runtime", "paperclip-runner", relativeHome); + const remoteHome = remoteRunnerFilesystemRoot ? posix.join(remoteRunnerFilesystemRoot, relativeHome) : null; const readAuth = async (name: string): Promise => { if (!remoteHome || !remoteCommandRunner) return Buffer.from(await readLocalAiCredentialFile(join(localHome, name))); const script = `const fs=require('node:fs'),path=require('node:path');let fd;try{const file=process.argv[1];let parent=path.dirname(file);while(true){if(!fs.lstatSync(parent).isDirectory())throw Error('directory');const next=path.dirname(parent);if(next===parent)break;parent=next;}fd=fs.openSync(file,fs.constants.O_RDONLY|fs.constants.O_NOFOLLOW);const st=fs.fstatSync(fd);if(!st.isFile()||st.uid!==process.getuid()||(st.mode&511)!==384||st.size>65536)throw Error('credential');const b=Buffer.alloc(65537);let n=0;while(n65536)throw Error('size');process.stdout.write(b.subarray(0,n).toString('base64'));b.fill(0);}catch(e){process.exitCode=e.code==='ENOENT'?66:1;}finally{if(fd!==undefined)fs.closeSync(fd);}`;