mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-08 00:54:38 +02:00
fix(grok-local): do not pin empty GROK_HOME over host login (#13570)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Local adapters such as `grok_local` invoke a host CLI (`grok`) for each heartbeat > - Grok authenticates from `GROK_HOME/auth.json` when that env var is set, otherwise from `~/.grok` > - Recent work (#12469, #12618) isolated subscription credentials into a company-scoped Grok home filled only by sandbox device login > - Local_trusted instances have only a Local environment, so that login never runs, the company home stays empty, and execute still sets `GROK_HOME` to it > - This pull request stops pinning `GROK_HOME` on local subscription runs unless the company home already has usable auth, a managed AI connection supplied a home, or the run is remote/sandbox > - The benefit is that `grok login` on the host works again for local Grok agents, without leaking host credentials into sandboxes ## Linked Issues or Issue Description Fixes: #13568 Related PRs (predecessors, not duplicates): - Refs #12469 - Refs #12618 - Refs #12696 I searched GitHub for `GROK_HOME`, `grok login`, `not signed in`, and `device-code`. No existing PR restores host-login fallback for local `grok_local` runs. ## What Changed - Local subscription execute no longer sets `GROK_HOME` when the company Grok home has no usable `auth.json` - Remote/sandbox runs and managed AI connections still pin `GROK_HOME` so they cannot fall through to the host login - Local runs still pin `GROK_HOME` once a company home has a usable credential (completed device login) - Adapter configuration notes document the host-login vs company-home split - Subscription detection respects an explicit empty `XAI_API_KEY` that clears an inherited host key. This keeps a valid company login selected. - Tests cover a real child process reading fixture host credentials, custom host homes, malformed company credentials, API-key overrides, managed connections, and empty remote homes. - Original fix by @hawikk. The follow-up preserves the contributor commit and adds independent regression coverage. ## Verification - `pnpm exec vitest run packages/adapters/grok-local`: 131 tests pass in 12 files. - `pnpm --filter @paperclipai/adapter-grok-local typecheck`: passes. - The new subprocess host-login regression fails against `master` and passes with this fix. It uses disposable fixture credentials and makes no provider request. - The explicit-empty-key regression fails against the contributed commit and passes with the follow-up. - `pnpm -r typecheck` and `pnpm build`: pass locally. - `pnpm test:run`: attempted locally, then stopped after embedded PostgreSQL startup failures. A focused retry of `ai-legacy-compatibility.test.ts` reproduced the same startup failure after five attempts. - All CI checks pass on `f4a380fec`: 54 successful checks and two intentional Storybook skips. This includes all general tests, serialized server suites, runner checks, browser shards, typecheck, build, and canary packaging. - Greptile reviewed `f4a380fec` at 5/5 with no actionable findings. GitHub reports no merge conflicts. - Grok CLI 1.0.13 is installed on the verification host, but it has no signed-in account. A live authenticated inference run was not performed. ## Risks - Low. Behavior changes only local subscription runs whose company Grok home has no usable `auth.json`. - Remote/sandbox isolation is unchanged: those runs still pin `GROK_HOME` and never use host `~/.grok`. - Managed AI connections still pin even with an empty home (fail closed rather than using the host account). - Operators who previously copied `auth.json` into the company home keep the pinned-home path. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Provider: xAI Grok - Model: Grok 4.6 (`grok-4.6`) - Tool use: yes (repository search, local tests, GitHub issue/PR) - Human-authored: no — AI-assisted implementation - Follow-up review, code, and tests: OpenAI GPT-6 in Codex, with reasoning, repository tools, and code execution. Exact backend model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Dotta <bippadotta@protonmail.com>
This commit is contained in:
3 files changed
+203
-25
No files matched your search
@@ -42,4 +42,6 @@ Notes:
|
||||
- Sessions resume with \`--resume <sessionId>\` when the saved session cwd matches the current cwd.
|
||||
- Paperclip stages desired runtime skills into \`.claude/skills\` inside the execution workspace so Grok discovers them as project skills.
|
||||
- Use \`grok models\` to inspect authentication and available models on the host.
|
||||
- Local subscription runs use the host \`grok login\` (\`~/.grok\`) until the company Grok home has a usable \`auth.json\` (sandbox device login). \`XAI_API_KEY\` authenticates without a home. Remote/sandbox runs never fall back to the host login.
|
||||
- Without a usable company login, local runs preserve an inherited or configured \`GROK_HOME\`. Managed AI connections keep their selected home. An explicit empty \`XAI_API_KEY\` clears an inherited key and selects subscription authentication.
|
||||
`;
|
||||
@@ -1,6 +1,7 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { AdapterExecutionContext } from "@paperclipai/adapter-utils";
|
||||
|
||||
@@ -305,29 +306,167 @@ describe("grok_local execute", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("sets GROK_HOME to the company home in subscription mode, and leaves it unset when XAI_API_KEY exists", async () => {
|
||||
let seenEnv: Record<string, string> = {};
|
||||
runProcessMock.mockImplementation(async (_runId, _target, _command, _args, options) => {
|
||||
seenEnv = options.env;
|
||||
return makeSuccessfulRunResult();
|
||||
describe("local lane GROK_HOME", () => {
|
||||
let previousApiKey: string | undefined;
|
||||
let previousPaperclipHome: string | undefined;
|
||||
let previousGrokHome: string | undefined;
|
||||
|
||||
beforeEach(async () => {
|
||||
previousApiKey = process.env.XAI_API_KEY;
|
||||
previousPaperclipHome = process.env.PAPERCLIP_HOME;
|
||||
previousGrokHome = process.env.GROK_HOME;
|
||||
process.env.PAPERCLIP_HOME = await makeTempRoot();
|
||||
delete process.env.XAI_API_KEY;
|
||||
delete process.env.GROK_HOME;
|
||||
});
|
||||
|
||||
const previousApiKey = process.env.XAI_API_KEY;
|
||||
try {
|
||||
delete process.env.XAI_API_KEY;
|
||||
await execute(await makeCtx("run-subscription-home", await makeTempRoot()));
|
||||
expect(seenEnv.GROK_HOME).toBe(resolveManagedGrokHomeDir(process.env, "company-1"));
|
||||
afterEach(() => {
|
||||
if (previousApiKey === undefined) delete process.env.XAI_API_KEY;
|
||||
else process.env.XAI_API_KEY = previousApiKey;
|
||||
if (previousPaperclipHome === undefined) delete process.env.PAPERCLIP_HOME;
|
||||
else process.env.PAPERCLIP_HOME = previousPaperclipHome;
|
||||
if (previousGrokHome === undefined) delete process.env.GROK_HOME;
|
||||
else process.env.GROK_HOME = previousGrokHome;
|
||||
});
|
||||
|
||||
it("leaves GROK_HOME unset when the company home has no usable auth", async () => {
|
||||
let seenEnv: Record<string, string> = {};
|
||||
runProcessMock.mockImplementation(async (_runId, _target, _command, _args, options) => {
|
||||
seenEnv = options.env;
|
||||
return makeSuccessfulRunResult();
|
||||
});
|
||||
|
||||
await execute(await makeCtx("run-subscription-home-empty", await makeTempRoot()));
|
||||
expect(seenEnv.GROK_HOME).toBeUndefined();
|
||||
});
|
||||
|
||||
it("lets a local child read the host login when the company home is empty", async () => {
|
||||
const hostRoot = await makeTempRoot();
|
||||
const hostHome = path.join(hostRoot, ".grok");
|
||||
await fs.mkdir(hostHome);
|
||||
const auth = grokAuth({ key: "fixture-host-key", expiresAt: NEWER_EXPIRY });
|
||||
await fs.writeFile(path.join(hostHome, "auth.json"), auth);
|
||||
const companyHome = resolveManagedGrokHomeDir(process.env, "company-1");
|
||||
await fs.mkdir(companyHome, { recursive: true });
|
||||
const ctx = await makeCtx("run-host-login-child", await makeTempRoot());
|
||||
ctx.config.env = { HOME: hostRoot };
|
||||
runProcessMock.mockImplementation(async (_runId, _target, _command, _args, options) => {
|
||||
// A real subprocess with Grok's home lookup contract, using only
|
||||
// disposable fixture credentials. No provider request is made.
|
||||
const stdout = execFileSync(process.execPath, ["-e", `
|
||||
const fs = require("node:fs");
|
||||
const path = require("node:path");
|
||||
const home = process.env.GROK_HOME || path.join(process.env.HOME, ".grok");
|
||||
const auth = JSON.parse(fs.readFileSync(path.join(home, "auth.json"), "utf8"));
|
||||
if (Object.values(auth)[0].key !== "fixture-host-key") process.exit(1);
|
||||
console.log(JSON.stringify({ type: "end", stopReason: "EndTurn", sessionId: "host-login" }));
|
||||
`], { env: { ...process.env, ...options.env }, encoding: "utf8" });
|
||||
return { ...makeSuccessfulRunResult(), stdout };
|
||||
});
|
||||
|
||||
const result = await execute(ctx);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.sessionId).toBe("host-login");
|
||||
expect(await fs.readdir(companyHome)).toEqual([]);
|
||||
expect(await fs.readFile(path.join(hostHome, "auth.json"), "utf8")).toBe(auth);
|
||||
});
|
||||
|
||||
it("pins GROK_HOME to the company home when that home has usable auth", async () => {
|
||||
const companyHome = resolveManagedGrokHomeDir(process.env, "company-1");
|
||||
await fs.mkdir(companyHome, { recursive: true });
|
||||
await fs.writeFile(
|
||||
path.join(companyHome, "auth.json"),
|
||||
grokAuth({ key: "local-key", expiresAt: NEWER_EXPIRY }),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
let seenEnv: Record<string, string> = {};
|
||||
runProcessMock.mockImplementation(async (_runId, _target, _command, _args, options) => {
|
||||
seenEnv = options.env;
|
||||
return makeSuccessfulRunResult();
|
||||
});
|
||||
|
||||
await execute(await makeCtx("run-subscription-home-seeded", await makeTempRoot()));
|
||||
expect(seenEnv.GROK_HOME).toBe(companyHome);
|
||||
});
|
||||
|
||||
it.each(["{invalid", "{}", JSON.stringify({ [GROK_IDENTITY]: { key: "incomplete" } })])(
|
||||
"uses host login when company auth is unusable (%s)",
|
||||
async (contents) => {
|
||||
const companyHome = resolveManagedGrokHomeDir(process.env, "company-1");
|
||||
await fs.mkdir(companyHome, { recursive: true });
|
||||
await fs.writeFile(path.join(companyHome, "auth.json"), contents);
|
||||
runProcessMock.mockResolvedValue(makeSuccessfulRunResult());
|
||||
|
||||
await execute(await makeCtx("run-unusable-company-auth", await makeTempRoot()));
|
||||
|
||||
expect(runProcessMock.mock.calls[0][4].env.GROK_HOME).toBeUndefined();
|
||||
expect(await fs.readFile(path.join(companyHome, "auth.json"), "utf8")).toBe(contents);
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["inherited", "configured"])("preserves the %s host GROK_HOME fallback", async (source) => {
|
||||
const hostHome = await makeTempRoot();
|
||||
const ctx = await makeCtx("run-custom-host-home", await makeTempRoot());
|
||||
if (source === "inherited") process.env.GROK_HOME = hostHome;
|
||||
else ctx.config.env = { GROK_HOME: hostHome };
|
||||
runProcessMock.mockResolvedValue(makeSuccessfulRunResult());
|
||||
|
||||
await execute(ctx);
|
||||
|
||||
// Command resolution receives the merged child environment, including
|
||||
// inherited values that are absent from the explicit spawn overrides.
|
||||
const commandCall = ensureCommandMock.mock.calls[0] as unknown as [unknown, unknown, unknown, Record<string, string>];
|
||||
expect(commandCall[3].GROK_HOME).toBe(hostHome);
|
||||
});
|
||||
|
||||
it("uses company login when an explicit empty API key overrides an inherited key", async () => {
|
||||
process.env.XAI_API_KEY = "host-api-key";
|
||||
process.env.GROK_HOME = await makeTempRoot();
|
||||
const companyHome = resolveManagedGrokHomeDir(process.env, "company-1");
|
||||
await fs.mkdir(companyHome, { recursive: true });
|
||||
await fs.writeFile(path.join(companyHome, "auth.json"), grokAuth({ key: "company-key", expiresAt: NEWER_EXPIRY }));
|
||||
const ctx = await makeCtx("run-cleared-host-api-key", await makeTempRoot());
|
||||
ctx.config.env = { XAI_API_KEY: "" };
|
||||
runProcessMock.mockResolvedValue(makeSuccessfulRunResult());
|
||||
|
||||
const result = await execute(ctx);
|
||||
|
||||
expect(runProcessMock.mock.calls[0][4].env.GROK_HOME).toBe(companyHome);
|
||||
expect(result.billingType).toBe("subscription");
|
||||
});
|
||||
|
||||
it("leaves GROK_HOME unset when XAI_API_KEY exists", async () => {
|
||||
let seenEnv: Record<string, string> = {};
|
||||
runProcessMock.mockImplementation(async (_runId, _target, _command, _args, options) => {
|
||||
seenEnv = options.env;
|
||||
return makeSuccessfulRunResult();
|
||||
});
|
||||
|
||||
// The XAI_API_KEY path stays unchanged: no GROK_HOME is set when the key
|
||||
// exists, because the CLI authenticates via the environment variable
|
||||
// directly, not from the company Grok home's auth.json.
|
||||
process.env.XAI_API_KEY = "test-key";
|
||||
await execute(await makeCtx("run-api-home", await makeTempRoot()));
|
||||
expect(seenEnv.GROK_HOME).toBeUndefined();
|
||||
} finally {
|
||||
if (previousApiKey === undefined) delete process.env.XAI_API_KEY;
|
||||
else process.env.XAI_API_KEY = previousApiKey;
|
||||
}
|
||||
});
|
||||
|
||||
it("pins GROK_HOME for a managed AI connection even when the home has no usable auth", async () => {
|
||||
process.env.GROK_HOME = await makeTempRoot();
|
||||
process.env.XAI_API_KEY = "inherited-host-key";
|
||||
let seenEnv: Record<string, string> = {};
|
||||
runProcessMock.mockImplementation(async (_runId, _target, _command, _args, options) => {
|
||||
seenEnv = options.env;
|
||||
return makeSuccessfulRunResult();
|
||||
});
|
||||
|
||||
const ctx = await makeCtx("run-connection-home", await makeTempRoot());
|
||||
ctx.config = {
|
||||
...ctx.config,
|
||||
managedAiConnection: true,
|
||||
env: { GROK_HOME: "/connection/grok-home" },
|
||||
};
|
||||
await execute(ctx);
|
||||
expect(seenEnv.GROK_HOME).toBe("/connection/grok-home");
|
||||
});
|
||||
});
|
||||
|
||||
it("passes an explicitly configured permissionMode through to the CLI", async () => {
|
||||
@@ -492,6 +631,30 @@ describe("grok_local execute", () => {
|
||||
expect(seenEnv.GROK_HOME).toBe("/remote/workspace/.paperclip-runtime/grok/home");
|
||||
});
|
||||
|
||||
it("stages an empty company home instead of a configured host login for remote runs", async () => {
|
||||
delete process.env.XAI_API_KEY;
|
||||
mocks.state.isRemote = true;
|
||||
const hostHome = await makeTempRoot();
|
||||
await fs.writeFile(path.join(hostHome, "auth.json"), grokAuth({ key: "host-only", expiresAt: NEWER_EXPIRY }));
|
||||
const ctx = await makeCtx("run-remote-empty-company", await makeTempRoot());
|
||||
ctx.config.env = { GROK_HOME: hostHome };
|
||||
let stagedEntries: string[] | undefined;
|
||||
prepareRuntimeMock.mockImplementationOnce(async (input) => {
|
||||
stagedEntries = await fs.readdir(input.assets![0].localDir);
|
||||
return {
|
||||
workspaceRemoteDir: "/remote/workspace",
|
||||
assetDirs: { home: "/remote/workspace/.paperclip-runtime/grok/home" },
|
||||
restoreWorkspace: async () => {},
|
||||
};
|
||||
});
|
||||
runProcessMock.mockResolvedValue(makeSuccessfulRunResult());
|
||||
|
||||
await execute(ctx);
|
||||
|
||||
expect(stagedEntries).toEqual([]);
|
||||
expect(runProcessMock.mock.calls[0][4].env.GROK_HOME).toBe("/remote/workspace/.paperclip-runtime/grok/home");
|
||||
});
|
||||
|
||||
it("uses the fallback remote path when assetDirs.home is absent", async () => {
|
||||
delete process.env.XAI_API_KEY;
|
||||
mocks.state.isRemote = true;
|
||||
|
||||
@@ -44,7 +44,7 @@ import {
|
||||
} from "@paperclipai/adapter-utils/server-utils";
|
||||
import { DEFAULT_GROK_LOCAL_MODEL } from "../index.js";
|
||||
import { copyBackGrokAuth } from "./grok-auth-copyback.js";
|
||||
import { resolveManagedGrokHomeDir, stageGrokHomeForSync } from "./grok-home.js";
|
||||
import { grokHomeHasUsableAuth, resolveManagedGrokHomeDir, stageGrokHomeForSync } from "./grok-home.js";
|
||||
import { isGrokUnknownSessionError, parseGrokJsonl } from "./parse.js";
|
||||
|
||||
const __moduleDir = path.dirname(fileURLToPath(import.meta.url));
|
||||
@@ -58,7 +58,7 @@ function firstNonEmptyLine(text: string): string {
|
||||
);
|
||||
}
|
||||
|
||||
function hasNonEmptyEnvValue(env: Record<string, string>, key: string): boolean {
|
||||
function hasNonEmptyEnvValue(env: Record<string, string | undefined>, key: string): boolean {
|
||||
const raw = env[key];
|
||||
return typeof raw === "string" && raw.trim().length > 0;
|
||||
}
|
||||
@@ -314,13 +314,26 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
// Held before the remote block below, so the remote lane can stage this
|
||||
// same host home into the sandbox without re-resolving it.
|
||||
const hostGrokHome = config.managedAiConnection ? asString(env.GROK_HOME, "") : resolveManagedGrokHomeDir(process.env, agent.companyId);
|
||||
// Subscription mode (no XAI_API_KEY): point the run at the company-scoped
|
||||
// Grok home a completed device login wrote. Leaves the API-key path below
|
||||
// (`resolveBillingType`) unchanged when the key exists.
|
||||
const isGrokSubscriptionMode =
|
||||
!hasNonEmptyEnvValue(env, "XAI_API_KEY") && (Boolean(config.managedAiConnection) || !hasNonEmptyEnvValue(process.env as Record<string, string>, "XAI_API_KEY"));
|
||||
// Subscription mode (no XAI_API_KEY): pin GROK_HOME to the company-scoped
|
||||
// home a completed device login wrote. Do not pin an empty local home —
|
||||
// that shadows the host `~/.grok` login and fails with "Not signed in"
|
||||
// (#13568). Remote/sandbox runs and managed AI connections still pin so
|
||||
// they cannot fall through to the host credential. The API-key path below
|
||||
// (`resolveBillingType`) stays unchanged when the key exists.
|
||||
// Explicit empty overrides clear inherited API keys in the child process.
|
||||
// Use the same precedence here when selecting its credential home.
|
||||
const isGrokSubscriptionMode = !hasNonEmptyEnvValue(
|
||||
config.managedAiConnection ? env : { ...process.env, ...env },
|
||||
"XAI_API_KEY",
|
||||
);
|
||||
if (isGrokSubscriptionMode) {
|
||||
env.GROK_HOME = hostGrokHome;
|
||||
const pinManagedHome =
|
||||
executionTargetIsRemote ||
|
||||
Boolean(config.managedAiConnection) ||
|
||||
(hostGrokHome.length > 0 && await grokHomeHasUsableAuth(hostGrokHome));
|
||||
if (pinManagedHome) {
|
||||
env.GROK_HOME = hostGrokHome;
|
||||
}
|
||||
}
|
||||
|
||||
const timeoutSec = resolveAdapterExecutionTargetTimeoutSec(
|
||||
|
||||
Reference in new issue
Block a user