From f8bcfb9d73f0c65645dd7f2e511172c2e3aa5760 Mon Sep 17 00:00:00 2001 From: Hung-Che Lo Date: Mon, 16 Mar 2026 22:12:15 +0800 Subject: [PATCH 1/5] feat(skills): preserve all skills in prompt via compact fallback before dropping (#47553) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(skills): add compact format fallback for skill catalog truncation When the full-format skill catalog exceeds the character budget, applySkillsPromptLimits now tries a compact format (name + location only, no description) before binary-searching for the largest fitting prefix. This preserves full model awareness of registered skills in the common overflow case. Three-tier strategy: 1. Full format fits → use as-is 2. Compact format fits → switch to compact, keep all skills 3. Compact still too large → binary search largest compact prefix Other changes: - escapeXml() utility for safe XML attribute values - formatSkillsCompact() emits same XML structure minus - Compact char-budget check reserves 150 chars for the warning line the caller prepends, preventing prompt overflow at the boundary - 13 tests covering all tiers, edge cases, and budget reservation - docs/.generated/config-baseline.json: fix pre-existing oxfmt issue * docs: document compact skill prompt fallback --------- Co-authored-by: Frank Yang --- CHANGELOG.md | 1 + src/agents/skills/compact-format.test.ts | 230 +++++++++++++++++++++++ src/agents/skills/workspace.ts | 104 +++++++--- 3 files changed, 310 insertions(+), 25 deletions(-) create mode 100644 src/agents/skills/compact-format.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 090f6046334..30ca4b0a21c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ Docs: https://docs.openclaw.ai - Docs/Zalo: clarify the Marketplace-bot support matrix and config guidance so the Zalo channel docs match current Bot Creator behavior more closely. (#47552) Thanks @No898. - secrets: harden read-only SecretRef command paths and diagnostics. (#47794) Thanks @joshavant. - Browser/existing-session: support `browser.profiles..userDataDir` so Chrome DevTools MCP can attach to Brave, Edge, and other Chromium-based browsers through their own user data directories. (#48170) thanks @velvet-shark. +- Skills/prompt budget: preserve all registered skills via a compact catalog fallback before dropping entries when the full prompt format exceeds `maxSkillsPromptChars`. (#47553) Thanks @snese. ### Breaking diff --git a/src/agents/skills/compact-format.test.ts b/src/agents/skills/compact-format.test.ts new file mode 100644 index 00000000000..20f3b8a256e --- /dev/null +++ b/src/agents/skills/compact-format.test.ts @@ -0,0 +1,230 @@ +import os from "node:os"; +import { formatSkillsForPrompt, type Skill } from "@mariozechner/pi-coding-agent"; +import { describe, expect, it } from "vitest"; +import type { SkillEntry } from "./types.js"; +import { + formatSkillsCompact, + buildWorkspaceSkillsPrompt, + buildWorkspaceSkillSnapshot, +} from "./workspace.js"; + +function makeSkill(name: string, desc = "A skill", filePath = `/skills/${name}/SKILL.md`): Skill { + return { + name, + description: desc, + filePath, + baseDir: `/skills/${name}`, + source: "workspace", + disableModelInvocation: false, + }; +} + +function makeEntry(skill: Skill): SkillEntry { + return { skill, frontmatter: {} }; +} + +function buildPrompt( + skills: Skill[], + limits: { maxChars?: number; maxCount?: number } = {}, +): string { + return buildWorkspaceSkillsPrompt("/fake", { + entries: skills.map(makeEntry), + config: { + skills: { + limits: { + ...(limits.maxChars !== undefined && { maxSkillsPromptChars: limits.maxChars }), + ...(limits.maxCount !== undefined && { maxSkillsInPrompt: limits.maxCount }), + }, + }, + } as any, + }); +} + +describe("formatSkillsCompact", () => { + it("returns empty string for no skills", () => { + expect(formatSkillsCompact([])).toBe(""); + }); + + it("omits description, keeps name and location", () => { + const out = formatSkillsCompact([makeSkill("weather", "Get weather data")]); + expect(out).toContain("weather"); + expect(out).toContain("/skills/weather/SKILL.md"); + expect(out).not.toContain("Get weather data"); + expect(out).not.toContain(""); + }); + + it("filters out disableModelInvocation skills", () => { + const hidden: Skill = { ...makeSkill("hidden"), disableModelInvocation: true }; + const out = formatSkillsCompact([makeSkill("visible"), hidden]); + expect(out).toContain("visible"); + expect(out).not.toContain("hidden"); + }); + + it("escapes XML special characters", () => { + const out = formatSkillsCompact([makeSkill("a { + const skills = Array.from({ length: 50 }, (_, i) => + makeSkill(`skill-${i}`, "A moderately long description that takes up space in the prompt"), + ); + const compact = formatSkillsCompact(skills); + expect(compact.length).toBeLessThan(6000); + }); +}); + +describe("applySkillsPromptLimits (via buildWorkspaceSkillsPrompt)", () => { + it("tier 1: uses full format when under budget", () => { + const skills = [makeSkill("weather", "Get weather data")]; + const prompt = buildPrompt(skills, { maxChars: 50_000 }); + expect(prompt).toContain(""); + expect(prompt).toContain("Get weather data"); + expect(prompt).not.toContain("⚠️"); + }); + + it("tier 2: compact when full exceeds budget but compact fits", () => { + const skills = Array.from({ length: 20 }, (_, i) => makeSkill(`skill-${i}`, "A".repeat(200))); + const fullLen = formatSkillsForPrompt(skills).length; + const compactLen = formatSkillsCompact(skills).length; + const budget = Math.floor((fullLen + compactLen) / 2); + // Verify preconditions: full exceeds budget, compact fits within overhead-adjusted budget + expect(fullLen).toBeGreaterThan(budget); + expect(compactLen + 150).toBeLessThan(budget); + const prompt = buildPrompt(skills, { maxChars: budget }); + expect(prompt).not.toContain(""); + // All skills preserved — distinct message, no "included X of Y" + expect(prompt).toContain("compact format (descriptions omitted)"); + expect(prompt).not.toContain("included"); + expect(prompt).toContain("skill-0"); + expect(prompt).toContain("skill-19"); + }); + + it("tier 3: compact + binary search when compact also exceeds budget", () => { + const skills = Array.from({ length: 100 }, (_, i) => makeSkill(`skill-${i}`, "description")); + const prompt = buildPrompt(skills, { maxChars: 2000 }); + expect(prompt).toContain("compact format, descriptions omitted"); + expect(prompt).not.toContain(""); + expect(prompt).toContain("skill-0"); + const match = prompt.match(/included (\d+) of (\d+)/); + expect(match).toBeTruthy(); + expect(Number(match![1])).toBeLessThan(Number(match![2])); + expect(Number(match![1])).toBeGreaterThan(0); + }); + + it("compact preserves all skills where full format would drop some", () => { + const skills = Array.from({ length: 50 }, (_, i) => makeSkill(`skill-${i}`, "A".repeat(200))); + const compactLen = formatSkillsCompact(skills).length; + const budget = compactLen + 250; + // Verify precondition: full format must not fit so tier 2 is actually exercised + expect(formatSkillsForPrompt(skills).length).toBeGreaterThan(budget); + const prompt = buildPrompt(skills, { maxChars: budget }); + // All 50 fit in compact — no truncation, just compact notice + expect(prompt).toContain("compact format"); + expect(prompt).not.toContain("included"); + expect(prompt).toContain("skill-0"); + expect(prompt).toContain("skill-49"); + }); + + it("count truncation + compact: shows included X of Y with compact note", () => { + // 30 skills but maxCount=10, and full format of 10 exceeds budget + const skills = Array.from({ length: 30 }, (_, i) => makeSkill(`skill-${i}`, "A".repeat(200))); + const tenSkills = skills.slice(0, 10); + const fullLen = formatSkillsForPrompt(tenSkills).length; + const compactLen = formatSkillsCompact(tenSkills).length; + const budget = compactLen + 200; + // Verify precondition: full format of 10 skills exceeds budget + expect(fullLen).toBeGreaterThan(budget); + const prompt = buildPrompt(skills, { maxChars: budget, maxCount: 10 }); + // Count-truncated (30→10) AND compact (full format of 10 exceeds budget) + expect(prompt).toContain("included 10 of 30"); + expect(prompt).toContain("compact format, descriptions omitted"); + expect(prompt).not.toContain(""); + }); + + it("extreme budget: even a single compact skill overflows", () => { + const skills = [makeSkill("only-one", "desc")]; + // Budget so small that even one compact skill can't fit + const prompt = buildPrompt(skills, { maxChars: 10 }); + expect(prompt).not.toContain("only-one"); + const match = prompt.match(/included (\d+) of (\d+)/); + expect(match).toBeTruthy(); + expect(Number(match![1])).toBe(0); + }); + + it("count truncation only: shows included X of Y without compact note", () => { + const skills = Array.from({ length: 20 }, (_, i) => makeSkill(`skill-${i}`, "short")); + const prompt = buildPrompt(skills, { maxChars: 50_000, maxCount: 5 }); + expect(prompt).toContain("included 5 of 20"); + expect(prompt).not.toContain("compact"); + expect(prompt).toContain(""); + }); + + it("compact budget reserves space for the warning line", () => { + // Build skills whose compact output exactly equals the char budget. + // Without overhead reservation the compact block would fit, but the + // warning line prepended by the caller would push the total over budget. + const skills = Array.from({ length: 50 }, (_, i) => makeSkill(`s-${i}`, "A".repeat(200))); + const compactLen = formatSkillsCompact(skills).length; + // Set budget = compactLen + 50 — less than the 150-char overhead reserve. + // The function should NOT choose compact-only because the warning wouldn't fit. + const prompt = buildPrompt(skills, { maxChars: compactLen + 50 }); + // Should fall through to compact + binary search (some skills dropped) + expect(prompt).toContain("included"); + expect(prompt).not.toContain(""); + }); + + it("budget check uses compacted home-dir paths, not canonical paths", () => { + // Skills with home-dir prefix get compacted (e.g. /home/user/... → ~/...). + // Budget check must use the compacted length, not the longer canonical path. + // If it used canonical paths, it would overestimate and potentially drop + // skills that actually fit after compaction. + const home = os.homedir(); + const skills = Array.from({ length: 30 }, (_, i) => + makeSkill( + `skill-${i}`, + "A".repeat(200), + `${home}/.openclaw/workspace/skills/skill-${i}/SKILL.md`, + ), + ); + // Compute compacted lengths (what the prompt will actually contain) + const compactedSkills = skills.map((s) => ({ + ...s, + filePath: s.filePath.replace(home, "~"), + })); + const compactedCompactLen = formatSkillsCompact(compactedSkills).length; + const canonicalCompactLen = formatSkillsCompact(skills).length; + // Sanity: canonical paths are longer than compacted paths + expect(canonicalCompactLen).toBeGreaterThan(compactedCompactLen); + // Set budget between compacted and canonical lengths — only fits if + // budget check uses compacted paths (correct) not canonical (wrong). + const budget = Math.floor((compactedCompactLen + canonicalCompactLen) / 2) + 150; + const prompt = buildPrompt(skills, { maxChars: budget }); + // All 30 skills should be preserved in compact form (tier 2, no dropping) + expect(prompt).toContain("skill-0"); + expect(prompt).toContain("skill-29"); + expect(prompt).not.toContain("included"); + expect(prompt).toContain("compact format"); + // Verify paths in output are compacted + expect(prompt).toContain("~/"); + expect(prompt).not.toContain(home); + }); + + it("resolvedSkills in snapshot keeps canonical paths, not compacted", () => { + const home = os.homedir(); + const skills = Array.from({ length: 5 }, (_, i) => + makeSkill(`skill-${i}`, "A skill", `${home}/.openclaw/workspace/skills/skill-${i}/SKILL.md`), + ); + const snapshot = buildWorkspaceSkillSnapshot("/fake", { + entries: skills.map(makeEntry), + }); + // Prompt should use compacted paths + expect(snapshot.prompt).toContain("~/"); + // resolvedSkills should preserve canonical (absolute) paths + expect(snapshot.resolvedSkills).toBeDefined(); + for (const skill of snapshot.resolvedSkills!) { + expect(skill.filePath).toContain(home); + expect(skill.filePath).not.toMatch(/^~\//); + } + }); +}); diff --git a/src/agents/skills/workspace.ts b/src/agents/skills/workspace.ts index 84c8ea78df3..80624a30139 100644 --- a/src/agents/skills/workspace.ts +++ b/src/agents/skills/workspace.ts @@ -526,10 +526,47 @@ function loadSkillEntries( return skillEntries; } +function escapeXml(str: string): string { + return str + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} + +/** + * Compact skill catalog: name + location only (no description). + * Used as a fallback when the full format exceeds the char budget, + * preserving awareness of all skills before resorting to dropping. + */ +export function formatSkillsCompact(skills: Skill[]): string { + const visible = skills.filter((s) => !s.disableModelInvocation); + if (visible.length === 0) return ""; + const lines = [ + "\n\nThe following skills provide specialized instructions for specific tasks.", + "Use the read tool to load a skill's file when the task matches its name.", + "When a skill file references a relative path, resolve it against the skill directory (parent of SKILL.md / dirname of the path) and use that absolute path in tool commands.", + "", + "", + ]; + for (const skill of visible) { + lines.push(" "); + lines.push(` ${escapeXml(skill.name)}`); + lines.push(` ${escapeXml(skill.filePath)}`); + lines.push(" "); + } + lines.push(""); + return lines.join("\n"); +} + +// Budget reserved for the compact-mode warning line prepended by the caller. +const COMPACT_WARNING_OVERHEAD = 150; + function applySkillsPromptLimits(params: { skills: Skill[]; config?: OpenClawConfig }): { skillsForPrompt: Skill[]; truncated: boolean; - truncatedReason: "count" | "chars" | null; + compact: boolean; } { const limits = resolveSkillsLimits(params.config); const total = params.skills.length; @@ -537,31 +574,41 @@ function applySkillsPromptLimits(params: { skills: Skill[]; config?: OpenClawCon let skillsForPrompt = byCount; let truncated = total > byCount.length; - let truncatedReason: "count" | "chars" | null = truncated ? "count" : null; + let compact = false; - const fits = (skills: Skill[]): boolean => { - const block = formatSkillsForPrompt(skills); - return block.length <= limits.maxSkillsPromptChars; - }; + const fitsFull = (skills: Skill[]): boolean => + formatSkillsForPrompt(skills).length <= limits.maxSkillsPromptChars; - if (!fits(skillsForPrompt)) { - // Binary search the largest prefix that fits in the char budget. - let lo = 0; - let hi = skillsForPrompt.length; - while (lo < hi) { - const mid = Math.ceil((lo + hi) / 2); - if (fits(skillsForPrompt.slice(0, mid))) { - lo = mid; - } else { - hi = mid - 1; + // Reserve space for the warning line the caller prepends in compact mode. + const compactBudget = limits.maxSkillsPromptChars - COMPACT_WARNING_OVERHEAD; + const fitsCompact = (skills: Skill[]): boolean => + formatSkillsCompact(skills).length <= compactBudget; + + if (!fitsFull(skillsForPrompt)) { + // Full format exceeds budget. Try compact (name + location, no description) + // to preserve awareness of all skills before dropping any. + if (fitsCompact(skillsForPrompt)) { + compact = true; + // No skills dropped — only format downgraded. Preserve existing truncated state. + } else { + // Compact still too large — binary search the largest prefix that fits. + compact = true; + let lo = 0; + let hi = skillsForPrompt.length; + while (lo < hi) { + const mid = Math.ceil((lo + hi) / 2); + if (fitsCompact(skillsForPrompt.slice(0, mid))) { + lo = mid; + } else { + hi = mid - 1; + } } + skillsForPrompt = skillsForPrompt.slice(0, lo); + truncated = true; } - skillsForPrompt = skillsForPrompt.slice(0, lo); - truncated = true; - truncatedReason = "chars"; } - return { skillsForPrompt, truncated, truncatedReason }; + return { skillsForPrompt, truncated, compact }; } export function buildWorkspaceSkillSnapshot( @@ -620,17 +667,24 @@ function resolveWorkspaceSkillPromptState( ); const remoteNote = opts?.eligibility?.remote?.note?.trim(); const resolvedSkills = promptEntries.map((entry) => entry.skill); - const { skillsForPrompt, truncated } = applySkillsPromptLimits({ - skills: resolvedSkills, + // Derive prompt-facing skills with compacted paths (e.g. ~/...) once. + // Budget checks and final render both use this same representation so the + // tier decision is based on the exact strings that end up in the prompt. + // resolvedSkills keeps canonical paths for snapshot / runtime consumers. + const promptSkills = compactSkillPaths(resolvedSkills); + const { skillsForPrompt, truncated, compact } = applySkillsPromptLimits({ + skills: promptSkills, config: opts?.config, }); const truncationNote = truncated - ? `⚠️ Skills truncated: included ${skillsForPrompt.length} of ${resolvedSkills.length}. Run \`openclaw skills check\` to audit.` - : ""; + ? `⚠️ Skills truncated: included ${skillsForPrompt.length} of ${resolvedSkills.length}${compact ? " (compact format, descriptions omitted)" : ""}. Run \`openclaw skills check\` to audit.` + : compact + ? `⚠️ Skills catalog using compact format (descriptions omitted). Run \`openclaw skills check\` to audit.` + : ""; const prompt = [ remoteNote, truncationNote, - formatSkillsForPrompt(compactSkillPaths(skillsForPrompt)), + compact ? formatSkillsCompact(skillsForPrompt) : formatSkillsForPrompt(skillsForPrompt), ] .filter(Boolean) .join("\n"); From 771fbeae792c8c368cc266f1f64c42d53896bbe4 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Mon, 16 Mar 2026 14:31:08 +0000 Subject: [PATCH 2/5] Gateway: simplify startup and stabilize mock responses tests --- .../auth-profiles.readonly-sync.test.ts | 5 +- src/agents/auth-profiles/external-cli-sync.ts | 32 ++- src/agents/auth-profiles/store.ts | 4 +- src/gateway/client.test.ts | 38 ++- src/gateway/client.ts | 91 ++++++- src/gateway/gateway.test.ts | 11 +- src/gateway/server.impl.ts | 123 ++++++--- src/gateway/test-helpers.mocks.ts | 251 +++++++++--------- src/plugins/bundled-dir.test.ts | 53 ++++ src/plugins/bundled-dir.ts | 17 ++ 10 files changed, 435 insertions(+), 190 deletions(-) create mode 100644 src/plugins/bundled-dir.test.ts diff --git a/src/agents/auth-profiles.readonly-sync.test.ts b/src/agents/auth-profiles.readonly-sync.test.ts index 2ef1c40d2f8..30a7b501a95 100644 --- a/src/agents/auth-profiles.readonly-sync.test.ts +++ b/src/agents/auth-profiles.readonly-sync.test.ts @@ -47,7 +47,10 @@ describe("auth profiles read-only external CLI sync", () => { const loaded = loadAuthProfileStoreForRuntime(agentDir, { readOnly: true }); - expect(mocks.syncExternalCliCredentials).toHaveBeenCalled(); + expect(mocks.syncExternalCliCredentials).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ log: false }), + ); expect(loaded.profiles["qwen-portal:default"]).toMatchObject({ type: "oauth", provider: "qwen-portal", diff --git a/src/agents/auth-profiles/external-cli-sync.ts b/src/agents/auth-profiles/external-cli-sync.ts index 2627845ed40..7e490c97c94 100644 --- a/src/agents/auth-profiles/external-cli-sync.ts +++ b/src/agents/auth-profiles/external-cli-sync.ts @@ -14,6 +14,10 @@ import type { AuthProfileCredential, AuthProfileStore, OAuthCredential } from ". const OPENAI_CODEX_DEFAULT_PROFILE_ID = "openai-codex:default"; +type ExternalCliSyncOptions = { + log?: boolean; +}; + function shallowEqualOAuthCredentials(a: OAuthCredential | undefined, b: OAuthCredential): boolean { if (!a) { return false; @@ -60,6 +64,7 @@ function syncExternalCliCredentialsForProvider( provider: string, readCredentials: () => OAuthCredential | null, now: number, + options: ExternalCliSyncOptions, ): boolean { const existing = store.profiles[profileId]; const shouldSync = @@ -78,10 +83,12 @@ function syncExternalCliCredentialsForProvider( if (shouldUpdate && !shallowEqualOAuthCredentials(existingOAuth, creds)) { store.profiles[profileId] = creds; - log.info(`synced ${provider} credentials from external cli`, { - profileId, - expires: new Date(creds.expires).toISOString(), - }); + if (options.log !== false) { + log.info(`synced ${provider} credentials from external cli`, { + profileId, + expires: new Date(creds.expires).toISOString(), + }); + } return true; } @@ -94,7 +101,10 @@ function syncExternalCliCredentialsForProvider( * * Returns true if any credentials were updated. */ -export function syncExternalCliCredentials(store: AuthProfileStore): boolean { +export function syncExternalCliCredentials( + store: AuthProfileStore, + options: ExternalCliSyncOptions = {}, +): boolean { let mutated = false; const now = Date.now(); @@ -119,10 +129,12 @@ export function syncExternalCliCredentials(store: AuthProfileStore): boolean { if (shouldUpdate && !shallowEqualOAuthCredentials(existingOAuth, qwenCreds)) { store.profiles[QWEN_CLI_PROFILE_ID] = qwenCreds; mutated = true; - log.info("synced qwen credentials from qwen cli", { - profileId: QWEN_CLI_PROFILE_ID, - expires: new Date(qwenCreds.expires).toISOString(), - }); + if (options.log !== false) { + log.info("synced qwen credentials from qwen cli", { + profileId: QWEN_CLI_PROFILE_ID, + expires: new Date(qwenCreds.expires).toISOString(), + }); + } } } @@ -134,6 +146,7 @@ export function syncExternalCliCredentials(store: AuthProfileStore): boolean { "minimax-portal", () => readMiniMaxCliCredentialsCached({ ttlMs: EXTERNAL_CLI_SYNC_TTL_MS }), now, + options, ) ) { mutated = true; @@ -145,6 +158,7 @@ export function syncExternalCliCredentials(store: AuthProfileStore): boolean { "openai-codex", () => readCodexCliCredentialsCached({ ttlMs: EXTERNAL_CLI_SYNC_TTL_MS }), now, + options, ) ) { mutated = true; diff --git a/src/agents/auth-profiles/store.ts b/src/agents/auth-profiles/store.ts index 0fa050e55ec..b1362310b7f 100644 --- a/src/agents/auth-profiles/store.ts +++ b/src/agents/auth-profiles/store.ts @@ -381,7 +381,7 @@ function loadAuthProfileStoreForAgent( if (asStore) { // Runtime secret activation must remain read-only: // sync external CLI credentials in-memory, but never persist while readOnly. - const synced = syncExternalCliCredentials(asStore); + const synced = syncExternalCliCredentials(asStore, { log: !readOnly }); if (synced && !readOnly) { saveJsonFile(authPath, asStore); } @@ -413,7 +413,7 @@ function loadAuthProfileStoreForAgent( const mergedOAuth = mergeOAuthFileIntoStore(store); // Keep external CLI credentials visible in runtime even during read-only loads. - const syncedCli = syncExternalCliCredentials(store); + const syncedCli = syncExternalCliCredentials(store, { log: !readOnly }); const forceReadOnly = process.env.OPENCLAW_AUTH_STORE_READONLY === "1"; const shouldWrite = !readOnly && !forceReadOnly && (legacy !== null || mergedOAuth || syncedCli); if (shouldWrite) { diff --git a/src/gateway/client.test.ts b/src/gateway/client.test.ts index d3fdc89c86a..4108ed30e46 100644 --- a/src/gateway/client.test.ts +++ b/src/gateway/client.test.ts @@ -25,6 +25,7 @@ class MockWebSocket { readonly sent: string[] = []; closeCalls = 0; terminateCalls = 0; + autoCloseOnClose = true; constructor(_url: string, _options?: unknown) { wsInstances.push(this); @@ -55,7 +56,9 @@ class MockWebSocket { close(code?: number, reason?: string): void { this.closeCalls += 1; - this.emitClose(code ?? 1000, reason ?? ""); + if (this.autoCloseOnClose) { + this.emitClose(code ?? 1000, reason ?? ""); + } } terminate(): void { @@ -327,6 +330,39 @@ describe("GatewayClient close handling", () => { } }); + it("waits for a lingering socket to terminate in stopAndWait", async () => { + vi.useFakeTimers(); + try { + const client = new GatewayClient({ + url: "ws://127.0.0.1:18789", + }); + + client.start(); + const ws = getLatestWs(); + ws.autoCloseOnClose = false; + + let settled = false; + const stopPromise = client.stopAndWait().then(() => { + settled = true; + }); + + expect(ws.closeCalls).toBe(1); + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(249); + expect(ws.terminateCalls).toBe(0); + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(1); + await stopPromise; + + expect(ws.terminateCalls).toBe(1); + expect(settled).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + it("does not clear persisted device auth when explicit shared token is provided", () => { const onClose = vi.fn(); const identity: DeviceIdentity = { diff --git a/src/gateway/client.ts b/src/gateway/client.ts index 0e30cef34e8..f4e49df1a10 100644 --- a/src/gateway/client.ts +++ b/src/gateway/client.ts @@ -120,6 +120,13 @@ export function describeGatewayCloseCode(code: number): string | undefined { } const FORCE_STOP_TERMINATE_GRACE_MS = 250; +const STOP_AND_WAIT_TIMEOUT_MS = 1_000; + +type PendingStop = { + ws: WebSocket; + promise: Promise; + resolve: () => void; +}; export class GatewayClient { private ws: WebSocket | null = null; @@ -139,6 +146,7 @@ export class GatewayClient { private tickIntervalMs = 30_000; private tickTimer: NodeJS.Timeout | null = null; private readonly requestTimeoutMs: number; + private pendingStop: PendingStop | null = null; constructor(opts: GatewayClientOptions) { this.opts = { @@ -217,9 +225,10 @@ export class GatewayClient { // oxlint-disable-next-line typescript/no-explicit-any }) as any; } - this.ws = new WebSocket(url, wsOptions); + const ws = new WebSocket(url, wsOptions); + this.ws = ws; - this.ws.on("open", () => { + ws.on("open", () => { if (url.startsWith("wss://") && this.opts.tlsFingerprint) { const tlsError = this.validateTlsFingerprint(); if (tlsError) { @@ -230,12 +239,15 @@ export class GatewayClient { } this.queueConnect(); }); - this.ws.on("message", (data) => this.handleMessage(rawDataToString(data))); - this.ws.on("close", (code, reason) => { + ws.on("message", (data) => this.handleMessage(rawDataToString(data))); + ws.on("close", (code, reason) => { const reasonText = rawDataToString(reason); const connectErrorDetailCode = this.pendingConnectErrorDetailCode; this.pendingConnectErrorDetailCode = null; - this.ws = null; + if (this.ws === ws) { + this.ws = null; + } + this.resolvePendingStop(ws); // Clear persisted device auth state only when device-token auth was active. // Shared token/password failures can return the same close reason but should // not erase a valid cached device token. @@ -265,7 +277,7 @@ export class GatewayClient { this.scheduleReconnect(); this.opts.onClose?.(code, reasonText); }); - this.ws.on("error", (err) => { + ws.on("error", (err) => { logDebug(`gateway client error: ${String(err)}`); if (!this.connectSent) { this.opts.onConnectError?.(err instanceof Error ? err : new Error(String(err))); @@ -274,6 +286,39 @@ export class GatewayClient { } stop() { + void this.beginStop(); + } + + async stopAndWait(opts?: { timeoutMs?: number }): Promise { + // Some callers need teardown ordering, not just "close requested". Wait for + // the socket to close or the terminate fallback to fire. + const stopPromise = this.beginStop(); + if (!stopPromise) { + return; + } + const timeoutMs = + typeof opts?.timeoutMs === "number" && Number.isFinite(opts.timeoutMs) + ? Math.max(1, Math.floor(opts.timeoutMs)) + : STOP_AND_WAIT_TIMEOUT_MS; + let timeout: NodeJS.Timeout | null = null; + try { + await Promise.race([ + stopPromise, + new Promise((_, reject) => { + timeout = setTimeout(() => { + reject(new Error(`gateway client stop timed out after ${timeoutMs}ms`)); + }, timeoutMs); + timeout.unref?.(); + }), + ]); + } finally { + if (timeout) { + clearTimeout(timeout); + } + } + } + + private beginStop(): Promise | null { this.closed = true; this.pendingDeviceTokenRetry = false; this.deviceTokenRetryBudgetUsed = false; @@ -282,18 +327,52 @@ export class GatewayClient { clearInterval(this.tickTimer); this.tickTimer = null; } + if (this.connectTimer) { + clearTimeout(this.connectTimer); + this.connectTimer = null; + } + if (this.pendingStop) { + this.flushPendingErrors(new Error("gateway client stopped")); + return this.pendingStop.promise; + } const ws = this.ws; this.ws = null; if (ws) { + const stopPromise = this.createPendingStop(ws); ws.close(); const forceTerminateTimer = setTimeout(() => { try { ws.terminate(); } catch {} + this.resolvePendingStop(ws); }, FORCE_STOP_TERMINATE_GRACE_MS); forceTerminateTimer.unref?.(); + this.flushPendingErrors(new Error("gateway client stopped")); + return stopPromise; } this.flushPendingErrors(new Error("gateway client stopped")); + return null; + } + + private createPendingStop(ws: WebSocket): Promise { + if (this.pendingStop?.ws === ws) { + return this.pendingStop.promise; + } + let resolve!: () => void; + const promise = new Promise((res) => { + resolve = res; + }); + this.pendingStop = { ws, promise, resolve }; + return promise; + } + + private resolvePendingStop(ws: WebSocket): void { + if (this.pendingStop?.ws !== ws) { + return; + } + const { resolve } = this.pendingStop; + this.pendingStop = null; + resolve(); } private sendConnect() { diff --git a/src/gateway/gateway.test.ts b/src/gateway/gateway.test.ts index aea5a816fa7..3625a3c481e 100644 --- a/src/gateway/gateway.test.ts +++ b/src/gateway/gateway.test.ts @@ -18,6 +18,9 @@ let writeConfigFile: typeof import("../config/config.js").writeConfigFile; let resolveConfigPath: typeof import("../config/config.js").resolveConfigPath; const GATEWAY_E2E_TIMEOUT_MS = 30_000; let gatewayTestSeq = 0; +// Keep this off the real "openai" provider id so the runtime stays on the +// mocked HTTP Responses path instead of upgrading to the OpenAI WS transport. +const MOCK_OPENAI_PROVIDER_ID = "mock-openai"; function nextGatewayId(prefix: string): string { return `${prefix}-${process.pid}-${process.env.VITEST_POOL_ID ?? "0"}-${gatewayTestSeq++}`; @@ -73,7 +76,7 @@ describe("gateway e2e", () => { models: { mode: "replace", providers: { - openai: buildOpenAiResponsesProviderConfig(openaiBaseUrl), + [MOCK_OPENAI_PROVIDER_ID]: buildOpenAiResponsesProviderConfig(openaiBaseUrl), }, }, gateway: { auth: { token } }, @@ -91,7 +94,7 @@ describe("gateway e2e", () => { await client.request("sessions.patch", { key: sessionKey, - model: "openai/gpt-5.2", + model: `${MOCK_OPENAI_PROVIDER_ID}/gpt-5.2`, }); const runId = nextGatewayId("run"); @@ -116,7 +119,7 @@ describe("gateway e2e", () => { expect(text).toContain(nonceA); expect(text).toContain(nonceB); } finally { - client.stop(); + await client.stopAndWait(); await server.close({ reason: "mock openai test complete" }); await fs.rm(tempHome, { recursive: true, force: true }); restore(); @@ -216,7 +219,7 @@ describe("gateway e2e", () => { | undefined; expect((token?.auth as { token?: string } | undefined)?.token).toBe(wizardToken); } finally { - client.stop(); + await client.stopAndWait(); await server.close({ reason: "wizard e2e complete" }); } diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index 350172bcee4..cec8f2cb42a 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -10,8 +10,9 @@ import { formatCliCommand } from "../cli/command-format.js"; import { createDefaultDeps } from "../cli/deps.js"; import { isRestartEnabled } from "../config/commands.js"; import { - CONFIG_PATH, + type ConfigFileSnapshot, type OpenClawConfig, + applyConfigOverrides, isNixMode, loadConfig, migrateLegacyConfig, @@ -217,6 +218,73 @@ function applyGatewayAuthOverridesForStartupPreflight( }; } +function assertValidGatewayStartupConfigSnapshot( + snapshot: ConfigFileSnapshot, + options: { includeDoctorHint?: boolean } = {}, +): void { + if (snapshot.valid) { + return; + } + const issues = + snapshot.issues.length > 0 + ? formatConfigIssueLines(snapshot.issues, "", { normalizeRoot: true }).join("\n") + : "Unknown validation issue."; + const doctorHint = options.includeDoctorHint + ? `\nRun "${formatCliCommand("openclaw doctor")}" to repair, then retry.` + : ""; + throw new Error(`Invalid config at ${snapshot.path}.\n${issues}${doctorHint}`); +} + +async function prepareGatewayStartupConfig(params: { + configSnapshot: ConfigFileSnapshot; + // Keep startup auth/runtime behavior aligned with loadConfig(), which applies + // runtime overrides beyond the raw on-disk snapshot. + runtimeConfig: OpenClawConfig; + authOverride?: GatewayServerOptions["auth"]; + tailscaleOverride?: GatewayServerOptions["tailscale"]; + activateRuntimeSecrets: ( + config: OpenClawConfig, + options: { reason: "startup"; activate: boolean }, + ) => Promise<{ config: OpenClawConfig }>; +}): Promise>> { + assertValidGatewayStartupConfigSnapshot(params.configSnapshot); + + // Fail fast before startup auth persists anything if required refs are unresolved. + const startupPreflightConfig = applyGatewayAuthOverridesForStartupPreflight( + params.runtimeConfig, + { + auth: params.authOverride, + tailscale: params.tailscaleOverride, + }, + ); + await params.activateRuntimeSecrets(startupPreflightConfig, { + reason: "startup", + activate: false, + }); + + const authBootstrap = await ensureGatewayStartupAuth({ + cfg: params.runtimeConfig, + env: process.env, + authOverride: params.authOverride, + tailscaleOverride: params.tailscaleOverride, + persist: true, + }); + const runtimeStartupConfig = applyGatewayAuthOverridesForStartupPreflight(authBootstrap.cfg, { + auth: params.authOverride, + tailscale: params.tailscaleOverride, + }); + const activatedConfig = ( + await params.activateRuntimeSecrets(runtimeStartupConfig, { + reason: "startup", + activate: true, + }) + ).config; + return { + ...authBootstrap, + cfg: activatedConfig, + }; +} + export type GatewayServer = { close: (opts?: { reason?: string; restartExpectedMs?: number | null }) => Promise; }; @@ -315,20 +383,16 @@ export async function startGatewayServer( } configSnapshot = await readConfigFileSnapshot(); - if (configSnapshot.exists && !configSnapshot.valid) { - const issues = - configSnapshot.issues.length > 0 - ? formatConfigIssueLines(configSnapshot.issues, "", { normalizeRoot: true }).join("\n") - : "Unknown validation issue."; - throw new Error( - `Invalid config at ${configSnapshot.path}.\n${issues}\nRun "${formatCliCommand("openclaw doctor")}" to repair, then retry.`, - ); + if (configSnapshot.exists) { + assertValidGatewayStartupConfigSnapshot(configSnapshot, { includeDoctorHint: true }); } const autoEnable = applyPluginAutoEnable({ config: configSnapshot.config, env: process.env }); if (autoEnable.changes.length > 0) { try { await writeConfigFile(autoEnable.config); + configSnapshot = await readConfigFileSnapshot(); + assertValidGatewayStartupConfigSnapshot(configSnapshot); log.info( `gateway: auto-enabled plugins:\n${autoEnable.changes .map((entry) => `- ${entry}`) @@ -405,37 +469,14 @@ export async function startGatewayServer( } }); - // Fail fast before startup if required refs are unresolved. let cfgAtStart: OpenClawConfig; - { - const freshSnapshot = await readConfigFileSnapshot(); - if (!freshSnapshot.valid) { - const issues = - freshSnapshot.issues.length > 0 - ? formatConfigIssueLines(freshSnapshot.issues, "", { normalizeRoot: true }).join("\n") - : "Unknown validation issue."; - throw new Error(`Invalid config at ${freshSnapshot.path}.\n${issues}`); - } - const startupPreflightConfig = applyGatewayAuthOverridesForStartupPreflight( - freshSnapshot.config, - { - auth: opts.auth, - tailscale: opts.tailscale, - }, - ); - await activateRuntimeSecrets(startupPreflightConfig, { - reason: "startup", - activate: false, - }); - } - - cfgAtStart = loadConfig(); - const authBootstrap = await ensureGatewayStartupAuth({ - cfg: cfgAtStart, - env: process.env, + const startupRuntimeConfig = applyConfigOverrides(configSnapshot.config); + const authBootstrap = await prepareGatewayStartupConfig({ + configSnapshot, + runtimeConfig: startupRuntimeConfig, authOverride: opts.auth, tailscaleOverride: opts.tailscale, - persist: true, + activateRuntimeSecrets, }); cfgAtStart = authBootstrap.cfg; if (authBootstrap.generatedToken) { @@ -449,12 +490,6 @@ export async function startGatewayServer( ); } } - cfgAtStart = ( - await activateRuntimeSecrets(cfgAtStart, { - reason: "startup", - activate: true, - }) - ).config; const diagnosticsEnabled = isDiagnosticsEnabled(cfgAtStart); if (diagnosticsEnabled) { startDiagnosticHeartbeat(); @@ -1061,7 +1096,7 @@ export async function startGatewayServer( warn: (msg) => logReload.warn(msg), error: (msg) => logReload.error(msg), }, - watchPath: CONFIG_PATH, + watchPath: configSnapshot.path, }); })(); diff --git a/src/gateway/test-helpers.mocks.ts b/src/gateway/test-helpers.mocks.ts index 59ad8a9cedc..4bfb7ef4e4d 100644 --- a/src/gateway/test-helpers.mocks.ts +++ b/src/gateway/test-helpers.mocks.ts @@ -367,6 +367,130 @@ vi.mock("../config/config.js", async () => { } }; + const composeTestConfig = (baseConfig: Record) => { + const fileAgents = + baseConfig.agents && + typeof baseConfig.agents === "object" && + !Array.isArray(baseConfig.agents) + ? (baseConfig.agents as Record) + : {}; + const fileDefaults = + fileAgents.defaults && + typeof fileAgents.defaults === "object" && + !Array.isArray(fileAgents.defaults) + ? (fileAgents.defaults as Record) + : {}; + const defaults = { + model: { primary: "anthropic/claude-opus-4-6" }, + workspace: path.join(os.tmpdir(), "openclaw-gateway-test"), + ...fileDefaults, + ...testState.agentConfig, + }; + const agents = testState.agentsConfig + ? { ...fileAgents, ...testState.agentsConfig, defaults } + : { ...fileAgents, defaults }; + + const fileBindings = Array.isArray(baseConfig.bindings) + ? (baseConfig.bindings as AgentBinding[]) + : undefined; + + const fileChannels = + baseConfig.channels && + typeof baseConfig.channels === "object" && + !Array.isArray(baseConfig.channels) + ? ({ ...(baseConfig.channels as Record) } as Record) + : {}; + const overrideChannels = + testState.channelsConfig && typeof testState.channelsConfig === "object" + ? { ...testState.channelsConfig } + : {}; + const mergedChannels = { ...fileChannels, ...overrideChannels }; + if (testState.allowFrom !== undefined) { + const existing = + mergedChannels.whatsapp && + typeof mergedChannels.whatsapp === "object" && + !Array.isArray(mergedChannels.whatsapp) + ? (mergedChannels.whatsapp as Record) + : {}; + mergedChannels.whatsapp = { + ...existing, + allowFrom: testState.allowFrom, + }; + } + const channels = Object.keys(mergedChannels).length > 0 ? mergedChannels : undefined; + + const fileSession = + baseConfig.session && + typeof baseConfig.session === "object" && + !Array.isArray(baseConfig.session) + ? (baseConfig.session as Record) + : {}; + const session: Record = { + ...fileSession, + mainKey: fileSession.mainKey ?? "main", + }; + if (typeof testState.sessionStorePath === "string") { + session.store = testState.sessionStorePath; + } + if (testState.sessionConfig) { + Object.assign(session, testState.sessionConfig); + } + + const fileGateway = + baseConfig.gateway && + typeof baseConfig.gateway === "object" && + !Array.isArray(baseConfig.gateway) + ? ({ ...(baseConfig.gateway as Record) } as Record) + : {}; + if (testState.gatewayBind) { + fileGateway.bind = testState.gatewayBind; + } + if (testState.gatewayAuth) { + fileGateway.auth = testState.gatewayAuth; + } + if (testState.gatewayControlUi) { + fileGateway.controlUi = testState.gatewayControlUi; + } + const gateway = Object.keys(fileGateway).length > 0 ? fileGateway : undefined; + + const fileCanvasHost = + baseConfig.canvasHost && + typeof baseConfig.canvasHost === "object" && + !Array.isArray(baseConfig.canvasHost) + ? ({ ...(baseConfig.canvasHost as Record) } as Record) + : {}; + if (typeof testState.canvasHostPort === "number") { + fileCanvasHost.port = testState.canvasHostPort; + } + const canvasHost = Object.keys(fileCanvasHost).length > 0 ? fileCanvasHost : undefined; + + const hooks = testState.hooksConfig ?? (baseConfig.hooks as HooksConfig | undefined); + + const fileCron = + baseConfig.cron && typeof baseConfig.cron === "object" && !Array.isArray(baseConfig.cron) + ? ({ ...(baseConfig.cron as Record) } as Record) + : {}; + if (typeof testState.cronEnabled === "boolean") { + fileCron.enabled = testState.cronEnabled; + } + if (typeof testState.cronStorePath === "string") { + fileCron.store = testState.cronStorePath; + } + const cron = Object.keys(fileCron).length > 0 ? fileCron : undefined; + + return { + ...baseConfig, + agents, + bindings: testState.bindingsConfig ?? fileBindings, + channels, + session, + gateway, + canvasHost, + hooks, + cron, + } as OpenClawConfig; + }; + const writeConfigFile = vi.fn(async (cfg: Record) => { const configPath = resolveConfigPath(); await fs.mkdir(path.dirname(configPath), { recursive: true }); @@ -389,6 +513,8 @@ vi.mock("../config/config.js", async () => { config: testState.migrationConfig ?? (raw as Record), changes: testState.migrationChanges, }), + applyConfigOverrides: (cfg: OpenClawConfig) => + composeTestConfig(cfg as Record), loadConfig: () => { const configPath = resolveConfigPath(); let fileConfig: Record = {}; @@ -400,129 +526,8 @@ vi.mock("../config/config.js", async () => { } catch { fileConfig = {}; } - - const fileAgents = - fileConfig.agents && - typeof fileConfig.agents === "object" && - !Array.isArray(fileConfig.agents) - ? (fileConfig.agents as Record) - : {}; - const fileDefaults = - fileAgents.defaults && - typeof fileAgents.defaults === "object" && - !Array.isArray(fileAgents.defaults) - ? (fileAgents.defaults as Record) - : {}; - const defaults = { - model: { primary: "anthropic/claude-opus-4-6" }, - workspace: path.join(os.tmpdir(), "openclaw-gateway-test"), - ...fileDefaults, - ...testState.agentConfig, - }; - const agents = testState.agentsConfig - ? { ...fileAgents, ...testState.agentsConfig, defaults } - : { ...fileAgents, defaults }; - - const fileBindings = Array.isArray(fileConfig.bindings) - ? (fileConfig.bindings as AgentBinding[]) - : undefined; - - const fileChannels = - fileConfig.channels && - typeof fileConfig.channels === "object" && - !Array.isArray(fileConfig.channels) - ? ({ ...(fileConfig.channels as Record) } as Record) - : {}; - const overrideChannels = - testState.channelsConfig && typeof testState.channelsConfig === "object" - ? { ...testState.channelsConfig } - : {}; - const mergedChannels = { ...fileChannels, ...overrideChannels }; - if (testState.allowFrom !== undefined) { - const existing = - mergedChannels.whatsapp && - typeof mergedChannels.whatsapp === "object" && - !Array.isArray(mergedChannels.whatsapp) - ? (mergedChannels.whatsapp as Record) - : {}; - mergedChannels.whatsapp = { - ...existing, - allowFrom: testState.allowFrom, - }; - } - const channels = Object.keys(mergedChannels).length > 0 ? mergedChannels : undefined; - - const fileSession = - fileConfig.session && - typeof fileConfig.session === "object" && - !Array.isArray(fileConfig.session) - ? (fileConfig.session as Record) - : {}; - const session: Record = { - ...fileSession, - mainKey: fileSession.mainKey ?? "main", - }; - if (typeof testState.sessionStorePath === "string") { - session.store = testState.sessionStorePath; - } - if (testState.sessionConfig) { - Object.assign(session, testState.sessionConfig); - } - - const fileGateway = - fileConfig.gateway && - typeof fileConfig.gateway === "object" && - !Array.isArray(fileConfig.gateway) - ? ({ ...(fileConfig.gateway as Record) } as Record) - : {}; - if (testState.gatewayBind) { - fileGateway.bind = testState.gatewayBind; - } - if (testState.gatewayAuth) { - fileGateway.auth = testState.gatewayAuth; - } - if (testState.gatewayControlUi) { - fileGateway.controlUi = testState.gatewayControlUi; - } - const gateway = Object.keys(fileGateway).length > 0 ? fileGateway : undefined; - - const fileCanvasHost = - fileConfig.canvasHost && - typeof fileConfig.canvasHost === "object" && - !Array.isArray(fileConfig.canvasHost) - ? ({ ...(fileConfig.canvasHost as Record) } as Record) - : {}; - if (typeof testState.canvasHostPort === "number") { - fileCanvasHost.port = testState.canvasHostPort; - } - const canvasHost = Object.keys(fileCanvasHost).length > 0 ? fileCanvasHost : undefined; - - const hooks = testState.hooksConfig ?? (fileConfig.hooks as HooksConfig | undefined); - - const fileCron = - fileConfig.cron && typeof fileConfig.cron === "object" && !Array.isArray(fileConfig.cron) - ? ({ ...(fileConfig.cron as Record) } as Record) - : {}; - if (typeof testState.cronEnabled === "boolean") { - fileCron.enabled = testState.cronEnabled; - } - if (typeof testState.cronStorePath === "string") { - fileCron.store = testState.cronStorePath; - } - const cron = Object.keys(fileCron).length > 0 ? fileCron : undefined; - - const config = { - ...fileConfig, - agents, - bindings: testState.bindingsConfig ?? fileBindings, - channels, - session, - gateway, - canvasHost, - hooks, - cron, - }; - return applyPluginAutoEnable({ config, env: process.env }).config; + return applyPluginAutoEnable({ config: composeTestConfig(fileConfig), env: process.env }) + .config; }, parseConfigJson5: (raw: string) => { try { diff --git a/src/plugins/bundled-dir.test.ts b/src/plugins/bundled-dir.test.ts new file mode 100644 index 00000000000..8bc1fc9cf76 --- /dev/null +++ b/src/plugins/bundled-dir.test.ts @@ -0,0 +1,53 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { resolveBundledPluginsDir } from "./bundled-dir.js"; + +const tempDirs: string[] = []; +const originalCwd = process.cwd(); +const originalBundledDir = process.env.OPENCLAW_BUNDLED_PLUGINS_DIR; +const originalWatchMode = process.env.OPENCLAW_WATCH_MODE; + +function makeRepoRoot(prefix: string): string { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.push(repoRoot); + return repoRoot; +} + +afterEach(() => { + process.chdir(originalCwd); + if (originalBundledDir === undefined) { + delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR; + } else { + process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = originalBundledDir; + } + if (originalWatchMode === undefined) { + delete process.env.OPENCLAW_WATCH_MODE; + } else { + process.env.OPENCLAW_WATCH_MODE = originalWatchMode; + } + for (const dir of tempDirs.splice(0, tempDirs.length)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("resolveBundledPluginsDir", () => { + it("prefers source extensions from the package root in watch mode", () => { + const repoRoot = makeRepoRoot("openclaw-bundled-dir-watch-"); + fs.mkdirSync(path.join(repoRoot, "extensions"), { recursive: true }); + fs.mkdirSync(path.join(repoRoot, "dist", "extensions"), { recursive: true }); + fs.writeFileSync( + path.join(repoRoot, "package.json"), + `${JSON.stringify({ name: "openclaw" }, null, 2)}\n`, + "utf8", + ); + + process.chdir(repoRoot); + process.env.OPENCLAW_WATCH_MODE = "1"; + + expect(fs.realpathSync(resolveBundledPluginsDir() ?? "")).toBe( + fs.realpathSync(path.join(repoRoot, "extensions")), + ); + }); +}); diff --git a/src/plugins/bundled-dir.ts b/src/plugins/bundled-dir.ts index 89d43444640..7fa25092f42 100644 --- a/src/plugins/bundled-dir.ts +++ b/src/plugins/bundled-dir.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { resolveOpenClawPackageRootSync } from "../infra/openclaw-root.js"; import { resolveUserPath } from "../utils.js"; export function resolveBundledPluginsDir(env: NodeJS.ProcessEnv = process.env): string | undefined { @@ -9,6 +10,22 @@ export function resolveBundledPluginsDir(env: NodeJS.ProcessEnv = process.env): return resolveUserPath(override, env); } + if (env.OPENCLAW_WATCH_MODE === "1") { + try { + const packageRoot = resolveOpenClawPackageRootSync({ cwd: process.cwd() }); + if (packageRoot) { + // In watch mode, prefer source plugin roots so plugin-local runtime deps + // resolve from extensions//node_modules instead of stripped dist copies. + const sourceExtensionsDir = path.join(packageRoot, "extensions"); + if (fs.existsSync(sourceExtensionsDir)) { + return sourceExtensionsDir; + } + } + } catch { + // ignore + } + } + // bun --compile: ship a sibling `extensions/` next to the executable. try { const execDir = path.dirname(process.execPath); From ce1d95454fbdbb4caf3a2835afcd51761818252b Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Mon, 16 Mar 2026 20:04:21 +0530 Subject: [PATCH 3/5] test: fix stale web search and boot-md contracts --- ...andler.gateway-startup.integration.test.ts | 13 +++++++--- src/hooks/bundled/boot-md/handler.test.ts | 13 +++++++--- src/plugins/contracts/loader.contract.test.ts | 25 ++++++------------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/hooks/bundled/boot-md/handler.gateway-startup.integration.test.ts b/src/hooks/bundled/boot-md/handler.gateway-startup.integration.test.ts index 7875bd04a1d..d4d441fa687 100644 --- a/src/hooks/bundled/boot-md/handler.gateway-startup.integration.test.ts +++ b/src/hooks/bundled/boot-md/handler.gateway-startup.integration.test.ts @@ -5,12 +5,17 @@ import type { OpenClawConfig } from "../../../config/config.js"; const runBootOnce = vi.fn(); -vi.mock("../../../gateway/boot.js", () => ({ runBootOnce })); -vi.mock("../../../logging/subsystem.js", () => ({ - createSubsystemLogger: () => ({ +function createMockLogger() { + return { warn: vi.fn(), debug: vi.fn(), - }), + child: vi.fn(() => createMockLogger()), + }; +} + +vi.mock("../../../gateway/boot.js", () => ({ runBootOnce })); +vi.mock("../../../logging/subsystem.js", () => ({ + createSubsystemLogger: () => createMockLogger(), })); const { default: runBootChecklist } = await import("./handler.js"); diff --git a/src/hooks/bundled/boot-md/handler.test.ts b/src/hooks/bundled/boot-md/handler.test.ts index b212842fbae..de2abd6475f 100644 --- a/src/hooks/bundled/boot-md/handler.test.ts +++ b/src/hooks/bundled/boot-md/handler.test.ts @@ -10,16 +10,21 @@ const logDebug = vi.fn(); const MAIN_WORKSPACE_DIR = path.join(path.sep, "ws", "main"); const OPS_WORKSPACE_DIR = path.join(path.sep, "ws", "ops"); +function createMockLogger() { + return { + warn: logWarn, + debug: logDebug, + child: vi.fn(() => createMockLogger()), + }; +} + vi.mock("../../../gateway/boot.js", () => ({ runBootOnce })); vi.mock("../../../agents/agent-scope.js", () => ({ listAgentIds, resolveAgentWorkspaceDir, })); vi.mock("../../../logging/subsystem.js", () => ({ - createSubsystemLogger: () => ({ - warn: logWarn, - debug: logDebug, - }), + createSubsystemLogger: () => createMockLogger(), })); const { default: runBootChecklist } = await import("./handler.js"); diff --git a/src/plugins/contracts/loader.contract.test.ts b/src/plugins/contracts/loader.contract.test.ts index a1f82f56fda..a42c24712ec 100644 --- a/src/plugins/contracts/loader.contract.test.ts +++ b/src/plugins/contracts/loader.contract.test.ts @@ -75,15 +75,12 @@ describe("plugin loader contract", () => { webSearchProviderContractRegistry.map((entry) => entry.pluginId), ); - resolvePluginWebSearchProviders({}); + const providers = resolvePluginWebSearchProviders({}); - expect(loadOpenClawPluginsMock).toHaveBeenCalledWith( - expect.objectContaining({ - onlyPluginIds: webSearchPluginIds, - activate: false, - cache: false, - }), + expect(uniqueSortedPluginIds(providers.map((provider) => provider.pluginId))).toEqual( + webSearchPluginIds, ); + expect(loadOpenClawPluginsMock).not.toHaveBeenCalled(); }); it("keeps bundled web search allowlist compatibility wired to the web search registry", () => { @@ -91,7 +88,7 @@ describe("plugin loader contract", () => { webSearchProviderContractRegistry.map((entry) => entry.pluginId), ); - resolvePluginWebSearchProviders({ + const providers = resolvePluginWebSearchProviders({ bundledAllowlistCompat: true, config: { plugins: { @@ -100,15 +97,9 @@ describe("plugin loader contract", () => { }, }); - expect(loadOpenClawPluginsMock).toHaveBeenCalledWith( - expect.objectContaining({ - config: expect.objectContaining({ - plugins: expect.objectContaining({ - allow: expect.arrayContaining(webSearchPluginIds), - }), - }), - onlyPluginIds: webSearchPluginIds, - }), + expect(uniqueSortedPluginIds(providers.map((provider) => provider.pluginId))).toEqual( + webSearchPluginIds, ); + expect(loadOpenClawPluginsMock).not.toHaveBeenCalled(); }); }); From d352be8e99f61d1d8e19f3d6ae58d57f6d86c4d2 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Mon, 16 Mar 2026 14:35:20 +0000 Subject: [PATCH 4/5] Gateway tests: centralize mock responses provider setup --- src/gateway/gateway.test.ts | 10 ++++------ src/gateway/test-openai-responses-model.ts | 11 +++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/gateway/gateway.test.ts b/src/gateway/gateway.test.ts index 3625a3c481e..31c2f3d1f7e 100644 --- a/src/gateway/gateway.test.ts +++ b/src/gateway/gateway.test.ts @@ -12,15 +12,12 @@ import { startGatewayWithClient, } from "./test-helpers.e2e.js"; import { installOpenAiResponsesMock } from "./test-helpers.openai-mock.js"; -import { buildOpenAiResponsesProviderConfig } from "./test-openai-responses-model.js"; +import { buildMockOpenAiResponsesProvider } from "./test-openai-responses-model.js"; let writeConfigFile: typeof import("../config/config.js").writeConfigFile; let resolveConfigPath: typeof import("../config/config.js").resolveConfigPath; const GATEWAY_E2E_TIMEOUT_MS = 30_000; let gatewayTestSeq = 0; -// Keep this off the real "openai" provider id so the runtime stays on the -// mocked HTTP Responses path instead of upgrading to the OpenAI WS transport. -const MOCK_OPENAI_PROVIDER_ID = "mock-openai"; function nextGatewayId(prefix: string): string { return `${prefix}-${process.pid}-${process.env.VITEST_POOL_ID ?? "0"}-${gatewayTestSeq++}`; @@ -70,13 +67,14 @@ describe("gateway e2e", () => { const configDir = path.join(tempHome, ".openclaw"); await fs.mkdir(configDir, { recursive: true }); const configPath = path.join(configDir, "openclaw.json"); + const mockProvider = buildMockOpenAiResponsesProvider(openaiBaseUrl); const cfg = { agents: { defaults: { workspace: workspaceDir } }, models: { mode: "replace", providers: { - [MOCK_OPENAI_PROVIDER_ID]: buildOpenAiResponsesProviderConfig(openaiBaseUrl), + [mockProvider.providerId]: mockProvider.config, }, }, gateway: { auth: { token } }, @@ -94,7 +92,7 @@ describe("gateway e2e", () => { await client.request("sessions.patch", { key: sessionKey, - model: `${MOCK_OPENAI_PROVIDER_ID}/gpt-5.2`, + model: mockProvider.modelRef, }); const runId = nextGatewayId("run"); diff --git a/src/gateway/test-openai-responses-model.ts b/src/gateway/test-openai-responses-model.ts index 8d9cac2242d..77e32d1a6e8 100644 --- a/src/gateway/test-openai-responses-model.ts +++ b/src/gateway/test-openai-responses-model.ts @@ -1,3 +1,5 @@ +export const MOCK_OPENAI_RESPONSES_PROVIDER_ID = "mock-openai"; + export function buildOpenAiResponsesTestModel(id = "gpt-5.2") { return { id, @@ -19,3 +21,12 @@ export function buildOpenAiResponsesProviderConfig(baseUrl: string, modelId = "g models: [buildOpenAiResponsesTestModel(modelId)], } as const; } + +export function buildMockOpenAiResponsesProvider(baseUrl: string, modelId = "gpt-5.2") { + return { + providerId: MOCK_OPENAI_RESPONSES_PROVIDER_ID, + modelId, + modelRef: `${MOCK_OPENAI_RESPONSES_PROVIDER_ID}/${modelId}`, + config: buildOpenAiResponsesProviderConfig(baseUrl, modelId), + } as const; +} From 13894ec5aacfa9ba880c4b7f0196212914f51f9b Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Mon, 16 Mar 2026 14:35:52 +0000 Subject: [PATCH 5/5] Gateway tests: share ordered client teardown helper --- src/gateway/gateway.test.ts | 5 +++-- src/gateway/test-helpers.e2e.ts | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/gateway/gateway.test.ts b/src/gateway/gateway.test.ts index 31c2f3d1f7e..5277673d408 100644 --- a/src/gateway/gateway.test.ts +++ b/src/gateway/gateway.test.ts @@ -7,6 +7,7 @@ import { startGatewayServer } from "./server.js"; import { extractPayloadText } from "./test-helpers.agent-results.js"; import { connectDeviceAuthReq, + disconnectGatewayClient, connectGatewayClient, getFreeGatewayPort, startGatewayWithClient, @@ -117,7 +118,7 @@ describe("gateway e2e", () => { expect(text).toContain(nonceA); expect(text).toContain(nonceB); } finally { - await client.stopAndWait(); + await disconnectGatewayClient(client); await server.close({ reason: "mock openai test complete" }); await fs.rm(tempHome, { recursive: true, force: true }); restore(); @@ -217,7 +218,7 @@ describe("gateway e2e", () => { | undefined; expect((token?.auth as { token?: string } | undefined)?.token).toBe(wizardToken); } finally { - await client.stopAndWait(); + await disconnectGatewayClient(client); await server.close({ reason: "wizard e2e complete" }); } diff --git a/src/gateway/test-helpers.e2e.ts b/src/gateway/test-helpers.e2e.ts index 34afd6614a8..eb0452c219e 100644 --- a/src/gateway/test-helpers.e2e.ts +++ b/src/gateway/test-helpers.e2e.ts @@ -104,6 +104,10 @@ export async function connectGatewayClient(params: { }); } +export async function disconnectGatewayClient(client: GatewayClient): Promise { + await client.stopAndWait(); +} + export async function connectDeviceAuthReq(params: { url: string; token?: string }) { const ws = new WebSocket(params.url); const connectNoncePromise = new Promise((resolve, reject) => {