From a0635eaa2ec16344e25f77f41916637f9cddc4c3 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 04:02:12 +0900 Subject: [PATCH 1/2] fix(claude): gate the shell hook on an installed Claude Code CLI The .zshrc hook was appended whenever system-env injection succeeded, so a user who never installed Claude Code still got an OpenCodex-owned block sourcing a file for a CLI they do not have, and a stale block survived uninstalling it. Write the hook only when an executable claude is discoverable on PATH, and reconcile rather than append: when Claude Code or system-env integration is gone, remove the OpenCodex-owned block and leave every user line untouched. Empty PATH segments are ignored so a workspace-local file is never mistaken for a durable installation, and a failed removal is reported as failed instead of reported as absent. Carries @lilinxiong's #2105 implementation and tests. Added a note on the intended failure direction: "installed" is answered from the PATH of the calling process, so a service context with a stripped PATH can remove a hook an interactive shell would have kept. That is reversible on the next foreground ocx start, whereas a hook pointing at an uninstalled CLI is the stale state this reconciliation exists to clear. Closes #2105 --- .../src/content/docs/fr/guides/claude-code.md | 5 +- .../src/content/docs/guides/claude-code.md | 5 +- .../src/content/docs/ja/guides/claude-code.md | 4 +- .../src/content/docs/ko/guides/claude-code.md | 4 +- .../src/content/docs/ru/guides/claude-code.md | 5 +- .../src/content/docs/tr/guides/claude-code.md | 8 +- .../content/docs/zh-cn/guides/claude-code.md | 4 +- .../content/docs/zh-tw/guides/claude-code.md | 4 +- src/cli/index.ts | 13 +- src/server/system-env.ts | 67 ++++++++- tests/claude-shell-hook.test.ts | 135 ++++++++++++++++++ 11 files changed, 235 insertions(+), 19 deletions(-) create mode 100644 tests/claude-shell-hook.test.ts diff --git a/docs-site/src/content/docs/fr/guides/claude-code.md b/docs-site/src/content/docs/fr/guides/claude-code.md index 9579f84a9a..8a5ccf79b2 100644 --- a/docs-site/src/content/docs/fr/guides/claude-code.md +++ b/docs-site/src/content/docs/fr/guides/claude-code.md @@ -142,7 +142,10 @@ par le proxy sans nécessiter le wrapper `ocx claude`. Les shells déjà ouverts `ocx stop` et l'arrêt du proxy **suppriment les variables injectées** (ils ne restaurent pas les valeurs précédentes — seules les clés injectées par opencodex sont supprimées). Le proxy écrit également `~/.opencodex/claude-env.sh` ; -`ocx start` installe un hook source `.zshrc` qui le charge automatiquement. +`ocx start` installe le hook source `.zshrc` uniquement lorsqu’un exécutable Claude Code CLI est +présent dans le `PATH`. Au démarrage et avec `ocx ensure`, le hook appartenant à OpenCodex est supprimé +si aucun exécutable du CLI Claude Code n’est trouvé dans le `PATH` ou si l’intégration de l’environnement +système est inactive. Claude Desktop utilise son propre profil et ne déclenche pas l’installation du hook shell. Désactivez cette intégration avec `claudeCode.systemEnv: false` dans la configuration ou avec le commutateur de l'interface. La fonctionnalité est réservée à macOS ; sur les autres plateformes, utilisez `ocx claude`. diff --git a/docs-site/src/content/docs/guides/claude-code.md b/docs-site/src/content/docs/guides/claude-code.md index 8f5a305a44..7c3a86dea3 100644 --- a/docs-site/src/content/docs/guides/claude-code.md +++ b/docs-site/src/content/docs/guides/claude-code.md @@ -142,7 +142,10 @@ requiring the `ocx claude` wrapper. Already-open shells are unaffected and must `ocx stop` and proxy shutdown **unset the injected keys** (it does not restore previous values — only the keys opencodex injected are removed). The proxy also writes `~/.opencodex/claude-env.sh`; -`ocx start` installs a `.zshrc` source hook that loads it automatically. +`ocx start` installs a `.zshrc` source hook that loads it automatically only when an executable +Claude Code CLI is present on `PATH`. Startup and `ocx ensure` remove the OpenCodex-owned hook when +Claude Code is absent or system environment integration is inactive. Claude Desktop uses its +separate profile and does not cause shell-hook installation. Disable with `claudeCode.systemEnv: false` in the configuration or with the GUI toggle. This feature is macOS-only; on other platforms, use `ocx claude`. diff --git a/docs-site/src/content/docs/ja/guides/claude-code.md b/docs-site/src/content/docs/ja/guides/claude-code.md index 4254e9a906..532419e23f 100644 --- a/docs-site/src/content/docs/ja/guides/claude-code.md +++ b/docs-site/src/content/docs/ja/guides/claude-code.md @@ -37,7 +37,9 @@ ocx claude `ocx stop` とプロキシ終了は**注入されたキーを解除します**。以前の値を復元せず、opencodex が 注入したキーのみ削除します。プロキシは `~/.opencodex/claude-env.sh` も書き出し、`ocx start` はこのファイルを -自動で読み込む `.zshrc` source hook をインストールします。 +自動で読み込む `.zshrc` source hook を、実行可能な Claude Code CLI が `PATH` にある場合にのみインストールします。 +Claude Code が存在しない場合、またはシステム環境連携が無効な場合、起動処理と `ocx ensure` は OpenCodex が追加した +hook を削除します。Claude Desktop は独立した profile を使用し、shell hook のインストールを引き起こしません。 設定で `claudeCode.systemEnv: false` に指定するか GUI トグルでオフにできます。この機能は macOS 専用で、他のプラットフォームでは `ocx claude` を使ってください。 diff --git a/docs-site/src/content/docs/ko/guides/claude-code.md b/docs-site/src/content/docs/ko/guides/claude-code.md index a3bab95851..c5aae96d36 100644 --- a/docs-site/src/content/docs/ko/guides/claude-code.md +++ b/docs-site/src/content/docs/ko/guides/claude-code.md @@ -62,7 +62,9 @@ macOS의 자동 연결(`claudeCode.systemEnv`)도 같은 방식으로 판단하 `ocx stop`과 프록시 종료는 **주입된 키를 해제해요**. 이전 값을 복원하지는 않고 opencodex가 주입한 키만 제거해요. 프록시는 `~/.opencodex/claude-env.sh`도 작성하고, `ocx start`는 이 파일을 -자동으로 불러오는 `.zshrc` source hook을 설치해요. +자동으로 불러오는 `.zshrc` source hook을 실행 가능한 Claude Code CLI가 `PATH`에 있을 때만 설치해요. +Claude Code가 없거나 시스템 환경 연동이 비활성화되어 있으면 시작 과정과 `ocx ensure`가 OpenCodex가 추가한 +hook을 제거해요. Claude Desktop은 별도 profile을 사용하며 shell hook 설치를 유발하지 않아요. 설정에서 `claudeCode.systemEnv: false`로 지정하거나 GUI 토글로 끌 수 있어요. 이 기능은 macOS 전용이며, 다른 플랫폼에서는 `ocx claude`를 사용하세요. diff --git a/docs-site/src/content/docs/ru/guides/claude-code.md b/docs-site/src/content/docs/ru/guides/claude-code.md index c8e88c1947..46bc060c40 100644 --- a/docs-site/src/content/docs/ru/guides/claude-code.md +++ b/docs-site/src/content/docs/ru/guides/claude-code.md @@ -39,7 +39,10 @@ ocx claude `ocx stop` и остановка прокси **снимают внедрённые ключи** (прежние значения не восстанавливаются — удаляются только ключи, внедрённые opencodex). Прокси также записывает `~/.opencodex/claude-env.sh`; -`ocx start` устанавливает source-хук в `.zshrc`, который загружает этот файл автоматически. +`ocx start` устанавливает source-хук в `.zshrc` только когда исполняемый Claude Code CLI найден в +`PATH`. При запуске и выполнении `ocx ensure` хук, принадлежащий OpenCodex, удаляется, если файл Claude +Code CLI не найден в `PATH`, не имеет права на выполнение или системная интеграция окружения выключена. +Claude Desktop использует отдельный профиль и не вызывает установку shell-хука. Отключить можно параметром `claudeCode.systemEnv: false` в конфигурации или переключателем в GUI. Функция доступна только на macOS; на других платформах используйте `ocx claude`. diff --git a/docs-site/src/content/docs/tr/guides/claude-code.md b/docs-site/src/content/docs/tr/guides/claude-code.md index 918ccaba62..e335bea33e 100644 --- a/docs-site/src/content/docs/tr/guides/claude-code.md +++ b/docs-site/src/content/docs/tr/guides/claude-code.md @@ -172,8 +172,11 @@ olan kabuklar etkilenmez ve yeniden açılmalıdır. `ocx stop` ve proxy'nin kapatılması **enjekte edilen anahtarları kaldırır** (önceki değerleri geri yüklemez — yalnızca opencodex'in enjekte ettiği anahtarlar kaldırılır). Proxy ayrıca `~/.opencodex/claude-env.sh` dosyasını -yazar; `ocx start`, bunu otomatik olarak yükleyen bir `.zshrc` kaynak kancası -kurar. +yazar; `ocx start`, bunu otomatik olarak yükleyen `.zshrc` kaynak kancasını +yalnızca çalıştırılabilir Claude Code CLI `PATH` içinde bulunduğunda kurar. +Claude Code yoksa veya sistem ortamı entegrasyonu etkin değilse başlangıç işlemi +ve `ocx ensure`, OpenCodex tarafından eklenen kancayı kaldırır. Claude Desktop ayrı +bir profil kullanır ve shell kancasının kurulmasını tetiklemez. Yapılandırmada `claudeCode.systemEnv: false` ile veya GUI anahtarıyla devre dışı bırakın. Bu özellik yalnızca macOS içindir; diğer platformlarda `ocx claude` @@ -605,4 +608,3 @@ aracının `model` argümanını değil, `` yönergelerin kullanır. Yönergenin hedeflenen rotayla eşleştiğinden emin olun. Model yer tutucusu olarak `"haiku"` iletin. - diff --git a/docs-site/src/content/docs/zh-cn/guides/claude-code.md b/docs-site/src/content/docs/zh-cn/guides/claude-code.md index 1bb5ef6acf..08fb965f27 100644 --- a/docs-site/src/content/docs/zh-cn/guides/claude-code.md +++ b/docs-site/src/content/docs/zh-cn/guides/claude-code.md @@ -37,7 +37,9 @@ shell 不受影响，必须重新打开。 `ocx stop` 和代理关闭操作会**取消设置已注入的键**（不会恢复之前的值——只会移除 opencodex 注入的键）。代理还会写入 `~/.opencodex/claude-env.sh`；`ocx start` 会安装一个 `.zshrc` -source hook，以自动加载该文件。 +source hook，以自动加载该文件，但仅限 `PATH` 中存在可执行的 Claude Code CLI。Claude Code +不存在或系统环境集成未启用时，启动过程和 `ocx ensure` 会移除 OpenCodex 自己写入的 hook。 +Claude Desktop 使用独立 profile，不会触发 shell hook 安装。 可以在配置中设置 `claudeCode.systemEnv: false`，或使用 GUI 开关来禁用。此功能仅适用于 macOS；在其他平台上，请使用 `ocx claude`。 diff --git a/docs-site/src/content/docs/zh-tw/guides/claude-code.md b/docs-site/src/content/docs/zh-tw/guides/claude-code.md index bc21c430f1..be06746fba 100644 --- a/docs-site/src/content/docs/zh-tw/guides/claude-code.md +++ b/docs-site/src/content/docs/zh-tw/guides/claude-code.md @@ -113,7 +113,9 @@ shell 不受影響，必須重新開啟。 `ocx stop` 和代理關閉操作會**取消設定已注入的鍵**（不會恢復之前的值——只會移除 opencodex 注入的鍵）。代理還會寫入 `~/.opencodex/claude-env.sh`；`ocx start` 會安裝一個 `.zshrc` -source hook，以自動載入該檔案。 +source hook，以自動載入該檔案，但僅限 `PATH` 中存在可執行的 Claude Code CLI。Claude Code +不存在或系統環境整合未啟用時，啟動程序和 `ocx ensure` 會移除 OpenCodex 自己寫入的 hook。 +Claude Desktop 使用獨立 profile，不會觸發 shell hook 安裝。 可以在設定中設定 `claudeCode.systemEnv: false`，或使用 GUI 開關來停用。此功能僅適用於 macOS；在其他平臺上，請使用 `ocx claude`。 diff --git a/src/cli/index.ts b/src/cli/index.ts index bbb265d2b5..eda778b16b 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -46,9 +46,8 @@ import { loadServiceTokenFromFile } from "../lib/service-secrets"; import { diagnoseService, isServiceOwnershipError, serviceCommand, serviceEnvironmentOwnedHere, serviceStartableFromTray, serviceStatusSummary, stopServiceIfInstalled, uninstallServiceIfInstalled } from "../service"; import { startupHealthSummary } from "../codex/autostart-health"; import { drainAndShutdown, isRecyclingForExit, startServer } from "../server"; -import { injectSystemEnv, revertSystemEnv } from "../server/system-env"; +import { injectSystemEnv, reconcileShellHook, revertSystemEnv, uninstallShellHook } from "../server/system-env"; import { buildDesktop3pRegistry } from "../claude/desktop-3p"; -import { installShellHook, uninstallShellHook } from "../server/system-env"; import { startTokenGuardian } from "../oauth/token-guardian"; import { startHistoryMigrationGuardian } from "../codex/history-migration-guardian"; import { maybeShowStarPrompt } from "./star-prompt"; @@ -366,9 +365,10 @@ async function handleStart(options: { block?: boolean } = {}) { // System-wide env injection AFTER signal handlers are registered (crash safety: // syncCleanup reverts even if injection itself or subsequent startup steps fail). - await injectSystemEnv(port, config).catch(() => {}); - // Auto-install .zshrc hook (idempotent — skips if already present). - installShellHook(); + const systemEnv = await injectSystemEnv(port, config).catch(() => ({ injected: false })); + // The hook is useful only for an installed Claude Code CLI. Reconcile instead of + // appending unconditionally so stale OpenCodex-owned hooks are removed as well. + reconcileShellHook(systemEnv.injected); await maybeShowStarPrompt(); // once-only Yes/No GitHub-star prompt on first interactive start // Post-startup sync drives the readiness gate AND the #1046 stale app-server @@ -455,7 +455,8 @@ async function handleEnsure(options: { existingIsSuccess?: boolean } = {}): Prom }); if (synced?.status === "skipped") console.log(" Codex integration OFF; startup left Codex native."); // Ensure env file exists for already-running proxy (may have been deleted or pre-dates this feature). - await injectSystemEnv(live.port, config).catch(() => {}); + const systemEnv = await injectSystemEnv(live.port, config).catch(() => ({ injected: false })); + reconcileShellHook(systemEnv.injected); // Refresh the Grok Build fence too (same contract as start). live.hostname is the // hostname the running proxy actually bound — config.hostname may have drifted. try { diff --git a/src/server/system-env.ts b/src/server/system-env.ts index 0ae251605c..927fc48682 100644 --- a/src/server/system-env.ts +++ b/src/server/system-env.ts @@ -1,6 +1,6 @@ import { execFileSync } from "node:child_process"; -import { readFileSync, writeFileSync, unlinkSync, mkdirSync } from "node:fs"; -import { join } from "node:path"; +import { accessSync, constants, readFileSync, writeFileSync, unlinkSync, mkdirSync, statSync } from "node:fs"; +import { delimiter, join } from "node:path"; import { getConfigDir } from "../config"; import { resolveAutoContext, type AutoContextMode } from "../claude/context-windows"; import { PROXY_MARKER, defaultAuthDetectDeps, detectClaudeAuth, ownAdmissionTokens } from "../claude/auth-detect"; @@ -122,11 +122,72 @@ export function uninstallShellHook(): { removed: boolean; reason?: string } { const cleaned = content.replace(/\n?# opencodex claude-env hook\n\[.*claude-env\.sh.*\n?/g, "\n"); writeFileSync(zshrcPath, cleaned, { encoding: "utf8", mode: 0o644 }); return { removed: true }; - } catch { + } catch (error) { + if (error && typeof error === "object" && (error as { code?: unknown }).code === "ENOENT") { + return { removed: false, reason: "not installed" }; + } return { removed: false, reason: "read/write failed" }; } } +/** Whether a real `claude` executable is discoverable from this process's PATH. */ +export function claudeCodeCliInstalled(pathValue = process.env.PATH): boolean { + if (!pathValue) return false; + for (const directory of pathValue.split(delimiter)) { + // An empty PATH segment means the current directory. Do not let the proxy treat a + // workspace-local file as a durable user installation. + if (!directory) continue; + const candidate = join(directory, "claude"); + try { + if (!statSync(candidate).isFile()) continue; + accessSync(candidate, constants.X_OK); + return true; + } catch { + // Keep scanning PATH after missing, non-file, and non-executable entries. + } + } + return false; +} + +/** + * Keep the shell hook aligned with the integration that can actually consume it. + * Claude Desktop uses its own profile and does not source `.zshrc`; this hook exists + * only for plain Claude Code CLI launches. + * + * Reconciliation is PATH-sensitive by construction: "Claude Code is installed" is answered + * from the PATH of whichever process calls this. A launchd/service context with a stripped + * PATH can therefore fail to see a `claude` the user's interactive shell finds, and this will + * remove the hook. That is the intended failure direction — removing an OpenCodex-owned block + * is reversible on the next foreground `ocx start`, whereas leaving a hook pointing at an + * uninstalled CLI is the stale state this reconciliation exists to clear. Only the block + * carrying our own marker is ever touched; user lines are preserved. + */ +export function reconcileShellHook(systemEnvInjected: boolean): { + changed: boolean; + state: "installed" | "absent" | "failed"; + reason?: string; +} { + if (process.platform !== "darwin") return { changed: false, state: "absent", reason: "not macOS" }; + if (systemEnvInjected && claudeCodeCliInstalled()) { + const result = installShellHook(); + if (result.installed) return { changed: true, state: "installed" }; + if (result.reason === "already installed") { + return { changed: false, state: "installed", reason: result.reason }; + } + return { changed: false, state: "failed", reason: result.reason ?? "install failed" }; + } + + const result = uninstallShellHook(); + if (!result.removed && result.reason !== "not installed") { + return { changed: false, state: "failed", reason: result.reason ?? "remove failed" }; + } + return { + changed: result.removed, + state: "absent", + reason: systemEnvInjected ? "Claude Code not installed" : "system environment inactive", + }; +} + const SYSTEM_ENV_NAMES = [ "ANTHROPIC_BASE_URL", "CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY", diff --git a/tests/claude-shell-hook.test.ts b/tests/claude-shell-hook.test.ts new file mode 100644 index 0000000000..2b02b84d26 --- /dev/null +++ b/tests/claude-shell-hook.test.ts @@ -0,0 +1,135 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { delimiter, join } from "node:path"; +import { claudeCodeCliInstalled, reconcileShellHook } from "../src/server/system-env"; + +const originalPlatform = process.platform; +let originalHome: string | undefined; +let originalPath: string | undefined; +let root = ""; +let binDir = ""; +let zshrcPath = ""; + +function setPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { configurable: true, value: platform }); +} + +/** Create the smallest executable that represents a Claude Code CLI on PATH. */ +function installClaudeCli(): void { + const executable = join(binDir, "claude"); + writeFileSync(executable, "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + chmodSync(executable, 0o755); +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "ocx-claude-hook-")); + binDir = join(root, "bin"); + zshrcPath = join(root, ".zshrc"); + mkdirSync(binDir); + originalHome = process.env.HOME; + originalPath = process.env.PATH; + process.env.HOME = root; + process.env.PATH = binDir; + setPlatform("darwin"); +}); + +afterEach(() => { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + setPlatform(originalPlatform); + rmSync(root, { recursive: true, force: true }); +}); + +describe("Claude Code shell-hook reconciliation", () => { + test("does not create .zshrc when Claude Code is absent", () => { + expect(claudeCodeCliInstalled()).toBe(false); + expect(reconcileShellHook(true)).toMatchObject({ changed: false, state: "absent" }); + expect(existsSync(zshrcPath)).toBe(false); + }); + + test("removes only the stale OpenCodex hook when Claude Code is absent", () => { + writeFileSync(zshrcPath, [ + "export USER_SETTING=1", + "# opencodex claude-env hook", + "[ -f ~/.opencodex/claude-env.sh ] && source ~/.opencodex/claude-env.sh", + "alias keep-me='yes'", + "", + ].join("\n")); + + expect(reconcileShellHook(true)).toEqual({ + changed: true, + state: "absent", + reason: "Claude Code not installed", + }); + const content = readFileSync(zshrcPath, "utf8"); + expect(content).toContain("export USER_SETTING=1"); + expect(content).toContain("alias keep-me='yes'"); + expect(content).not.toContain("opencodex claude-env hook"); + expect(content).not.toContain("claude-env.sh"); + }); + + test("installs the hook only for an executable Claude Code CLI and stays idempotent", () => { + installClaudeCli(); + expect(claudeCodeCliInstalled()).toBe(true); + expect(reconcileShellHook(true)).toEqual({ changed: true, state: "installed" }); + expect(reconcileShellHook(true)).toEqual({ + changed: false, + state: "installed", + reason: "already installed", + }); + expect(readFileSync(zshrcPath, "utf8").match(/opencodex claude-env hook/g)).toHaveLength(1); + }); + + test("does not treat a non-executable claude file as an installed CLI", () => { + writeFileSync(join(binDir, "claude"), "#!/bin/sh\nexit 0\n", { mode: 0o644 }); + + expect(claudeCodeCliInstalled()).toBe(false); + expect(reconcileShellHook(true)).toMatchObject({ changed: false, state: "absent" }); + expect(existsSync(zshrcPath)).toBe(false); + }); + + test("removes the hook when system environment integration is inactive", () => { + installClaudeCli(); + reconcileShellHook(true); + + expect(reconcileShellHook(false)).toEqual({ + changed: true, + state: "absent", + reason: "system environment inactive", + }); + expect(readFileSync(zshrcPath, "utf8")).not.toContain("opencodex claude-env hook"); + }); + + test("ignores empty PATH segments instead of trusting a workspace-local claude file", () => { + writeFileSync(join(root, "claude"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + const previousCwd = process.cwd(); + process.chdir(root); + process.env.PATH = `${delimiter}${binDir}`; + + try { + expect(claudeCodeCliInstalled()).toBe(false); + } finally { + process.chdir(previousCwd); + } + }); + + test("reports hook-removal failures instead of claiming the hook is absent", () => { + mkdirSync(zshrcPath); + + expect(reconcileShellHook(false)).toEqual({ + changed: false, + state: "failed", + reason: "read/write failed", + }); + }); + + test("start and ensure reconcile the hook from the actual injection result", async () => { + const source = await Bun.file(new URL("../src/cli/index.ts", import.meta.url)).text(); + + expect(source).not.toMatch(/\n\s*installShellHook\(\);/); + expect(source.match(/reconcileShellHook\(systemEnv\.injected\)/g)).toHaveLength(2); + }); +}); From b394b035bfac14b0aac2b9ca439cd7f51b75dea2 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Thu, 20 Aug 2026 11:25:53 +0900 Subject: [PATCH 2/2] fix(claude): remove a CRLF hook block instead of reporting a false success Two defects CodeRabbit found on #2144. uninstallShellHook matched the owned block with LF-only newlines. A .zshrc with CRLF endings -- ordinary on a home directory an editor or another OS has touched -- did not match, so the file was rewritten unchanged while the caller was told the hook was removed. The hook kept sourcing on every new interactive shell and the reported state said it did not. The pattern now accepts CR?LF and verifies the marker is gone before claiming removal; a block shaped differently is reported failed. Both cli/index.ts call sites discarded the result, so state: "failed" -- the one outcome the user has to act on -- was never surfaced. They now print the reason and where to look. --- src/cli/index.ts | 16 +++++++++++-- src/server/system-env.ts | 12 ++++++++-- tests/claude-shell-hook.test.ts | 42 +++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index eda778b16b..e723db0e56 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -56,6 +56,18 @@ import { maybeShowUpdatePrompt } from "../update/notify"; import { syncModelsToCodex } from "../codex/sync"; import { setIntegrationEnabled, shouldSyncCodexOnStart, shouldSyncGrokOnStart, syncCodexOnStartIfEnabled } from "../codex/desired-state"; +/** + * A failed shell-hook reconcile is not cosmetic: a stale hook keeps sourcing + * `claude-env.sh` from every new interactive shell, pointing at a proxy or a CLI that may no + * longer exist. `reconcileShellHook` already reports `state: "failed"`, but both call sites + * discarded it, so the one outcome the user has to act on was the one they never saw. + */ +function reportShellHookFailure(result: { state: "installed" | "absent" | "failed"; reason?: string }): void { + if (result.state !== "failed") return; + console.warn(` Claude shell hook not reconciled${result.reason ? `: ${result.reason}` : ""}`); + console.warn(" Check ~/.zshrc for the '# opencodex claude-env hook' block."); +} + import { removeOwnedConfigState } from "../lib/config-ownership"; import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; @@ -368,7 +380,7 @@ async function handleStart(options: { block?: boolean } = {}) { const systemEnv = await injectSystemEnv(port, config).catch(() => ({ injected: false })); // The hook is useful only for an installed Claude Code CLI. Reconcile instead of // appending unconditionally so stale OpenCodex-owned hooks are removed as well. - reconcileShellHook(systemEnv.injected); + reportShellHookFailure(reconcileShellHook(systemEnv.injected)); await maybeShowStarPrompt(); // once-only Yes/No GitHub-star prompt on first interactive start // Post-startup sync drives the readiness gate AND the #1046 stale app-server @@ -456,7 +468,7 @@ async function handleEnsure(options: { existingIsSuccess?: boolean } = {}): Prom if (synced?.status === "skipped") console.log(" Codex integration OFF; startup left Codex native."); // Ensure env file exists for already-running proxy (may have been deleted or pre-dates this feature). const systemEnv = await injectSystemEnv(live.port, config).catch(() => ({ injected: false })); - reconcileShellHook(systemEnv.injected); + reportShellHookFailure(reconcileShellHook(systemEnv.injected)); // Refresh the Grok Build fence too (same contract as start). live.hostname is the // hostname the running proxy actually bound — config.hostname may have drifted. try { diff --git a/src/server/system-env.ts b/src/server/system-env.ts index 927fc48682..f23a33531c 100644 --- a/src/server/system-env.ts +++ b/src/server/system-env.ts @@ -118,8 +118,16 @@ export function uninstallShellHook(): { removed: boolean; reason?: string } { try { const content = readFileSync(zshrcPath, "utf8"); if (!content.includes(SHELL_HOOK_MARKER)) return { removed: false, reason: "not installed" }; - // Remove the hook block (marker line + source line + surrounding newlines) - const cleaned = content.replace(/\n?# opencodex claude-env hook\n\[.*claude-env\.sh.*\n?/g, "\n"); + // Match CR?LF, not LF alone. A .zshrc with CRLF line endings — ordinary on a home + // directory an editor or another OS has touched — did not match, so the file was + // rewritten unchanged and the caller was told the hook was removed. Reporting success + // while the hook still sources on every new shell is the worse of the two failures. + const cleaned = content.replace(/\r?\n?# opencodex claude-env hook\r?\n\[.*claude-env\.sh.*(?:\r?\n)?/g, "\n"); + // Verify instead of assuming: if the marker survives, the block is shaped in a way this + // pattern does not own, and the honest answer is failure rather than a silent no-op. + if (cleaned.includes(SHELL_HOOK_MARKER)) { + return { removed: false, reason: "hook block present but not in the expected shape; remove it manually" }; + } writeFileSync(zshrcPath, cleaned, { encoding: "utf8", mode: 0o644 }); return { removed: true }; } catch (error) { diff --git a/tests/claude-shell-hook.test.ts b/tests/claude-shell-hook.test.ts index 2b02b84d26..9a2b4cbd36 100644 --- a/tests/claude-shell-hook.test.ts +++ b/tests/claude-shell-hook.test.ts @@ -103,6 +103,48 @@ describe("Claude Code shell-hook reconciliation", () => { expect(readFileSync(zshrcPath, "utf8")).not.toContain("opencodex claude-env hook"); }); + // A .zshrc with CRLF endings is ordinary on a home directory an editor or another OS has + // touched. The removal pattern matched LF only, so the file was rewritten unchanged while + // the caller was told the hook was gone — the worse outcome, because the hook keeps + // sourcing on every new shell and the reported state says it does not. + test("removes a CRLF-terminated hook block instead of reporting a false success", () => { + installClaudeCli(); + reconcileShellHook(true); + const installed = readFileSync(zshrcPath, "utf8"); + writeFileSync(zshrcPath, installed.replace(/\n/g, "\r\n"), "utf8"); + + const result = reconcileShellHook(false); + + expect(readFileSync(zshrcPath, "utf8")).not.toContain("opencodex claude-env hook"); + expect(result.state).toBe("absent"); + }); + + test("unrelated CRLF lines survive the removal", () => { + installClaudeCli(); + writeFileSync(zshrcPath, "export FOO=1\n", "utf8"); + reconcileShellHook(true); + const installed = readFileSync(zshrcPath, "utf8"); + writeFileSync(zshrcPath, installed.replace(/\n/g, "\r\n"), "utf8"); + + reconcileShellHook(false); + const after = readFileSync(zshrcPath, "utf8"); + + expect(after).toContain("export FOO=1"); + expect(after).not.toContain("claude-env.sh"); + }); + + test("a marker block this pattern does not own is reported failed, not removed", () => { + installClaudeCli(); + // The marker is there but the next line is not the block we wrote, so it is not ours to + // delete. Answering "removed" here would be a claim the user acts on and it would be false. + writeFileSync(zshrcPath, "# opencodex claude-env hook\n# hand-edited by the user\n", "utf8"); + + const result = reconcileShellHook(false); + + expect(result.state).toBe("failed"); + expect(readFileSync(zshrcPath, "utf8")).toContain("hand-edited by the user"); + }); + test("ignores empty PATH segments instead of trusting a workspace-local claude file", () => { writeFileSync(join(root, "claude"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); const previousCwd = process.cwd();