From dc097be00dd5c2dca121ccc74e2698f80558b255 Mon Sep 17 00:00:00 2001 From: Sasan Date: Fri, 10 Jul 2026 23:36:02 -0400 Subject: [PATCH] fix(doctor): follow symlinked launcher when locating sandbox setup scripts (#90942) * fix(doctor): follow symlinked launcher when locating sandbox setup scripts * refactor(doctor): reuse shared package-root resolver for sandbox scripts resolveSandboxScript hand-rolled its own cwd/argv1/realpath candidate scan, duplicating openclaw-root discovery. Reuse resolveOpenClawPackageRootSync, which already follows the symlinked launcher via realpath and handles node_modules/.bin and version-manager links, then look for scripts/ under the resolved package root. Resolves the duplicate-discovery review finding. * fix(doctor): keep searching cwd for sandbox scripts when first package root lacks them resolveSandboxScript stopped at the first openclaw package root the shared resolver returned. argv1 candidates resolve before cwd, so an installed/published root (package.json present, scripts/ dropped by the npm files allowlist) shadowed a valid source-checkout cwd and made doctor --fix return null. - add resolveOpenClawPackageRootsSync: all distinct roots in candidate order - resolveSandboxScript returns the first root that actually contains the script - resolveOpenClawPackageRootSync now delegates to the plural (behavior unchanged) - test: source-checkout cwd shadowed by an installed argv1 root without scripts/ --- src/commands/doctor-sandbox.test.ts | 110 ++++++++++++++++++++++++++++ src/commands/doctor-sandbox.ts | 27 ++++--- src/infra/openclaw-root.ts | 47 ++++++++---- 3 files changed, 157 insertions(+), 27 deletions(-) create mode 100644 src/commands/doctor-sandbox.test.ts diff --git a/src/commands/doctor-sandbox.test.ts b/src/commands/doctor-sandbox.test.ts new file mode 100644 index 000000000000..7e5808fe27c3 --- /dev/null +++ b/src/commands/doctor-sandbox.test.ts @@ -0,0 +1,110 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { __testing as openClawRootTesting } from "../infra/openclaw-root.js"; +import { resolveSandboxScript } from "./doctor-sandbox.js"; + +describe("resolveSandboxScript", () => { + const created: string[] = []; + + afterEach(() => { + // The shared resolver memoizes package-root lookups process-wide; reset so per-test temp dirs + // never see a stale (including negative) cache hit. + openClawRootTesting.clearOpenClawPackageRootCaches(); + for (const dir of created.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + function mkTmp(prefix: string): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + created.push(dir); + // Resolve macOS /var → /private/var so expectations match realpath output. + return fs.realpathSync(dir); + } + + const scriptRel = path.join("scripts", "sandbox-setup.sh"); + + // Create a repo checkout that the shared resolver will recognize: it keys off an openclaw + // package.json marker, then resolveSandboxScript looks for scripts/ under that root. + function mkRepo(prefix: string): string { + const repo = mkTmp(prefix); + fs.mkdirSync(path.join(repo, "scripts"), { recursive: true }); + fs.writeFileSync(path.join(repo, scriptRel), "#!/bin/sh\n"); + fs.writeFileSync(path.join(repo, "package.json"), JSON.stringify({ name: "openclaw" })); + return repo; + } + + it("follows a symlinked launcher to find scripts/ in the real repo", () => { + // Repo checkout that actually contains scripts/sandbox-setup.sh ... + const repo = mkRepo("ocsbx-repo-"); + const entry = path.join(repo, "openclaw.mjs"); + fs.writeFileSync(entry, ""); + + // ... reached only via a symlinked launcher in an unrelated bin dir (the npm/pnpm global case). + const binDir = mkTmp("ocsbx-bin-"); + const launcher = path.join(binDir, "openclaw"); + fs.symlinkSync(entry, launcher); + + const result = resolveSandboxScript(scriptRel, { argv1: launcher, cwd: binDir }); + + // Without following the symlink this returns null (the old bug); with realpath it finds the repo. + expect(result).not.toBeNull(); + expect(result?.scriptPath).toBe(path.join(repo, scriptRel)); + expect(result?.cwd).toBe(repo); + }); + + it("still resolves a script relative to a non-symlinked launcher dir", () => { + const repo = mkRepo("ocsbx-direct-"); + const entry = path.join(repo, "openclaw.mjs"); + fs.writeFileSync(entry, ""); + + const result = resolveSandboxScript(scriptRel, { argv1: entry, cwd: os.tmpdir() }); + + expect(result?.scriptPath).toBe(path.join(repo, scriptRel)); + }); + + it("returns null when the script is unreachable from cwd or the launcher", () => { + const binDir = mkTmp("ocsbx-none-"); + const launcher = path.join(binDir, "openclaw"); + fs.writeFileSync(launcher, ""); + + expect( + resolveSandboxScript(scriptRel, { + argv1: launcher, + cwd: binDir, + }), + ).toBeNull(); + }); + + it("falls back to cwd when the launcher path does not resolve to a repo", () => { + const repo = mkRepo("ocsbx-missing-argv1-"); + + const result = resolveSandboxScript(scriptRel, { + argv1: "/nonexistent-ocsbx/bin/openclaw", + cwd: repo, + }); + + expect(result?.scriptPath).toBe(path.join(repo, scriptRel)); + expect(result?.cwd).toBe(repo); + }); + + it("keeps searching cwd when the launcher resolves to a package root without the script", () => { + // Installed/published openclaw package root: it carries the package.json marker but not + // scripts/sandbox-setup.sh, because the npm files allowlist drops scripts/. It resolves from + // argv1 before cwd, so stopping at the first root would miss the source checkout below. + const installed = mkTmp("ocsbx-installed-"); + fs.writeFileSync(path.join(installed, "package.json"), JSON.stringify({ name: "openclaw" })); + const entry = path.join(installed, "openclaw.mjs"); + fs.writeFileSync(entry, ""); + + // Valid source checkout (cwd) that does contain the script. + const repo = mkRepo("ocsbx-source-"); + + const result = resolveSandboxScript(scriptRel, { argv1: entry, cwd: repo }); + + expect(result?.scriptPath).toBe(path.join(repo, scriptRel)); + expect(result?.cwd).toBe(repo); + }); +}); diff --git a/src/commands/doctor-sandbox.ts b/src/commands/doctor-sandbox.ts index 57ecc515fe5c..ae39e1d227bd 100644 --- a/src/commands/doctor-sandbox.ts +++ b/src/commands/doctor-sandbox.ts @@ -18,6 +18,7 @@ import { import { formatCliCommand } from "../cli/command-format.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { HealthFinding, HealthRepairEffect } from "../flows/health-checks.js"; +import { resolveOpenClawPackageRootsSync } from "../infra/openclaw-root.js"; import { runCommandWithTimeout, runExec } from "../process/exec.js"; import type { RuntimeEnv } from "../runtime.js"; import { shortenHomePath } from "../utils.js"; @@ -30,23 +31,25 @@ type SandboxScriptInfo = { cwd: string; }; -function resolveSandboxScript(scriptRel: string): SandboxScriptInfo | null { - const candidates = new Set(); - candidates.add(process.cwd()); - const argv1 = process.argv[1]; - if (argv1) { - const normalized = path.resolve(argv1); - candidates.add(path.resolve(path.dirname(normalized), "..")); - candidates.add(path.resolve(path.dirname(normalized))); - } - - for (const root of candidates) { +export function resolveSandboxScript( + scriptRel: string, + options: { argv1?: string; cwd?: string } = {}, +): SandboxScriptInfo | null { + // Scan every openclaw package root the shared resolver finds (symlinked launcher via realpath, + // then cwd) and return the first that actually holds the script. The resolver follows npm/pnpm + // global bins and version-manager links, but a published package root can resolve first and ship + // without scripts/sandbox-setup.sh (the npm files allowlist drops scripts/); stopping at the + // first root would then skip a valid source-checkout cwd that still has it. + const roots = resolveOpenClawPackageRootsSync({ + cwd: options.cwd ?? process.cwd(), + argv1: options.argv1 ?? process.argv[1], + }); + for (const root of roots) { const scriptPath = path.join(root, scriptRel); if (fs.existsSync(scriptPath)) { return { scriptPath, cwd: root }; } } - return null; } diff --git a/src/infra/openclaw-root.ts b/src/infra/openclaw-root.ts index e5b5d9aa1804..b55fdae34207 100644 --- a/src/infra/openclaw-root.ts +++ b/src/infra/openclaw-root.ts @@ -6,6 +6,7 @@ import { openClawRootFs, openClawRootFsSync } from "./openclaw-root.fs.runtime.j const CORE_PACKAGE_NAMES = new Set(["openclaw"]); const packageNameCache = new Map(); const packageRootCache = new Map(); +const packageRootsCache = new Map(); const argv1CandidateCache = new Map(); function parsePackageName(raw: string): string | null { @@ -139,26 +140,41 @@ export async function resolveOpenClawPackageRoot(opts: { return null; } +// Every distinct OpenClaw package root among the runtime hints, in candidate order (symlinked +// launcher via realpath first, then cwd). Callers that need a specific file under the root must +// pick the first root that actually contains it: an installed package root can resolve first but +// omit files the npm allowlist drops (e.g. scripts/), so stopping at root[0] would skip a valid +// source-checkout cwd that still has them. +export function resolveOpenClawPackageRootsSync(opts: { + cwd?: string; + argv1?: string; + moduleUrl?: string; +}): string[] { + const candidates = buildCandidates(opts); + const cacheKey = createPackageRootCacheKey(candidates); + const cached = packageRootsCache.get(cacheKey); + if (cached) { + return [...cached]; + } + const seen = new Set(); + const roots: string[] = []; + for (const candidate of candidates) { + const found = findPackageRootSync(candidate); + if (found && !seen.has(found)) { + seen.add(found); + roots.push(found); + } + } + packageRootsCache.set(cacheKey, roots); + return [...roots]; +} + export function resolveOpenClawPackageRootSync(opts: { cwd?: string; argv1?: string; moduleUrl?: string; }): string | null { - const candidates = buildCandidates(opts); - const cacheKey = createPackageRootCacheKey(candidates); - if (packageRootCache.has(cacheKey)) { - return packageRootCache.get(cacheKey) ?? null; - } - for (const candidate of candidates) { - const found = findPackageRootSync(candidate); - if (found) { - packageRootCache.set(cacheKey, found); - return found; - } - } - - packageRootCache.set(cacheKey, null); - return null; + return resolveOpenClawPackageRootsSync(opts)[0] ?? null; } function buildCandidates(opts: { cwd?: string; argv1?: string; moduleUrl?: string }): string[] { @@ -203,6 +219,7 @@ export const testing = { clearOpenClawPackageRootCaches(): void { packageNameCache.clear(); packageRootCache.clear(); + packageRootsCache.clear(); argv1CandidateCache.clear(); }, };