From e42c4f45134cd4f7325296e0234daae3611d3f56 Mon Sep 17 00:00:00 2001 From: Shadow Date: Mon, 9 Mar 2026 22:43:51 -0500 Subject: [PATCH 001/126] docs: harden PR review gates against unsubstantiated fixes --- .pi/prompts/reviewpr.md | 46 ++++++++++++++++++++++++++++++++++------- AGENTS.md | 12 +++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/.pi/prompts/reviewpr.md b/.pi/prompts/reviewpr.md index 835be806dd5..e3ebc0dd9c6 100644 --- a/.pi/prompts/reviewpr.md +++ b/.pi/prompts/reviewpr.md @@ -9,7 +9,20 @@ Input - If ambiguous: ask. Do (review-only) -Goal: produce a thorough review and a clear recommendation (READY for /landpr vs NEEDS WORK). Do NOT merge, do NOT push, do NOT make changes in the repo as part of this command. +Goal: produce a thorough review and a clear recommendation (READY FOR /landpr vs NEEDS WORK vs INVALID CLAIM). Do NOT merge, do NOT push, do NOT make changes in the repo as part of this command. + +0. Truthfulness + reality gate (required for bug-fix claims) + + - Do not trust the issue text or PR summary by default; verify in code and evidence. + - If the PR claims to fix a bug linked to an issue, confirm the bug exists now (repro steps, logs, failing test, or clear code-path proof). + - Prove root cause with exact location (`path/file.ts:line` + explanation of why behavior is wrong). + - Verify fix targets the same code path as the root cause. + - Require a regression test when feasible (fails before fix, passes after fix). If not feasible, require explicit justification + manual verification evidence. + - Hallucination/BS red flags (treat as BLOCKER until disproven): + - claimed behavior not present in repo, + - issue/PR says "fixes #..." but changed files do not touch implicated path, + - only docs/comments changed for a runtime bug claim, + - vague AI-generated rationale without concrete evidence. 1. Identify PR meta + context @@ -56,6 +69,7 @@ Goal: produce a thorough review and a clear recommendation (READY for /landpr vs - Any deprecations, docs, types, or lint rules we should adjust? 8. Key questions to answer explicitly + - Is the core claim substantiated by evidence, or is it likely invalid/hallucinated? - Can we fix everything ourselves in a follow-up, or does the contributor need to update this PR? - Any blocking concerns (must-fix before merge)? - Is this PR ready to land, or does it need work? @@ -65,18 +79,32 @@ Goal: produce a thorough review and a clear recommendation (READY for /landpr vs A) TL;DR recommendation -- One of: READY FOR /landpr | NEEDS WORK | NEEDS DISCUSSION +- One of: READY FOR /landpr | NEEDS WORK | INVALID CLAIM (issue/bug not substantiated) | NEEDS DISCUSSION - 1–3 sentence rationale. -B) What changed +B) Claim verification matrix (required) + +- Fill this table: + + | Field | Evidence | + |---|---| + | Claimed problem | ... | + | Evidence observed (repro/log/test/code) | ... | + | Root cause location (`path:line`) | ... | + | Why this fix addresses that root cause | ... | + | Regression coverage (test name or manual proof) | ... | + +- If any row is missing/weak, default to `NEEDS WORK` or `INVALID CLAIM`. + +C) What changed - Brief bullet summary of the diff/behavioral changes. -C) What's good +D) What's good - Bullets: correctness, simplicity, tests, docs, ergonomics, etc. -D) Concerns / questions (actionable) +E) Concerns / questions (actionable) - Numbered list. - Mark each item as: @@ -84,17 +112,19 @@ D) Concerns / questions (actionable) - IMPORTANT (should fix before merge) - NIT (optional) - For each: point to the file/area and propose a concrete fix or alternative. +- If evidence for the core bug claim is missing, add a `BLOCKER` explicitly. -E) Tests +F) Tests - What exists. - What's missing (specific scenarios). +- State clearly whether there is a regression test for the claimed bug. -F) Follow-ups (optional) +G) Follow-ups (optional) - Non-blocking refactors/tickets to open later. -G) Suggested PR comment (optional) +H) Suggested PR comment (optional) - Offer: "Want me to draft a PR comment to the author?" - If yes, provide a ready-to-paste comment summarizing the above, with clear asks. diff --git a/AGENTS.md b/AGENTS.md index 1516f2e4f58..80443603c87 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,6 +27,18 @@ - `invalid`: close invalid items (issues are closed as `not_planned`; PRs are closed). - `dirty`: close PRs with too many unrelated/unexpected changes (PR-only label). +## PR truthfulness and bug-fix validation + +- Never merge a bug-fix PR based only on issue text, PR text, or AI rationale. +- Before `/landpr`, run `/reviewpr` and require explicit evidence for bug-fix claims. +- Minimum merge gate for bug-fix PRs: + 1. symptom evidence (repro/log/failing test), + 2. verified root cause in code with file/line, + 3. fix touches the implicated code path, + 4. regression test (fail before/pass after) when feasible; if not feasible, include manual verification proof and why no test was added. +- If claim is unsubstantiated or likely hallucinated/BS: do not merge. Request evidence/changes, or close with `invalid` when appropriate. +- If linked issue appears wrong/outdated, correct triage first; do not merge speculative fixes. + ## Project Structure & Module Organization - Source code: `src/` (CLI wiring in `src/cli`, commands in `src/commands`, web provider in `src/provider-web.ts`, infra in `src/infra`, media pipeline in `src/media`). From 93c44e3dad3ef0f4bcfe1f44872cac197a0baae3 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Tue, 10 Mar 2026 09:14:57 +0530 Subject: [PATCH 002/126] ci: drop gha cache from docker release (#41692) --- .github/workflows/docker-release.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/docker-release.yml b/.github/workflows/docker-release.yml index f991b7f8653..2cc29748c91 100644 --- a/.github/workflows/docker-release.yml +++ b/.github/workflows/docker-release.yml @@ -109,8 +109,6 @@ jobs: labels: ${{ steps.labels.outputs.value }} provenance: false push: true - cache-from: type=gha,scope=docker-release-amd64 - cache-to: type=gha,mode=max,scope=docker-release-amd64 - name: Build and push amd64 slim image id: build-slim @@ -124,8 +122,6 @@ jobs: labels: ${{ steps.labels.outputs.value }} provenance: false push: true - cache-from: type=gha,scope=docker-release-amd64 - cache-to: type=gha,mode=max,scope=docker-release-amd64 # Build arm64 images (default + slim share the build stage cache) build-arm64: @@ -214,8 +210,6 @@ jobs: labels: ${{ steps.labels.outputs.value }} provenance: false push: true - cache-from: type=gha,scope=docker-release-arm64 - cache-to: type=gha,mode=max,scope=docker-release-arm64 - name: Build and push arm64 slim image id: build-slim @@ -229,8 +223,6 @@ jobs: labels: ${{ steps.labels.outputs.value }} provenance: false push: true - cache-from: type=gha,scope=docker-release-arm64 - cache-to: type=gha,mode=max,scope=docker-release-arm64 # Create multi-platform manifests create-manifest: From f0eb67923cd74b9278b408e868b80b0db40a23e9 Mon Sep 17 00:00:00 2001 From: Josh Avant <830519+joshavant@users.noreply.github.com> Date: Mon, 9 Mar 2026 22:57:03 -0500 Subject: [PATCH 003/126] fix(secrets): resolve web tool SecretRefs atomically at runtime --- CHANGELOG.md | 1 + docs/gateway/secrets.md | 3 +- docs/help/faq.md | 15 +- docs/perplexity.md | 3 + docs/reference/api-usage-costs.md | 8 +- .../reference/secretref-credential-surface.md | 4 +- ...tref-user-supplied-credentials-matrix.json | 7 + docs/tools/firecrawl.md | 3 +- docs/tools/web.md | 32 +- src/agents/openclaw-tools.ts | 4 + src/agents/openclaw-tools.web-runtime.test.ts | 135 ++++ .../tools/web-fetch.cf-markdown.test.ts | 41 + src/agents/tools/web-fetch.ts | 27 +- src/agents/tools/web-search.ts | 62 +- .../tools/web-tools.enabled-defaults.test.ts | 140 +++- src/cli/command-secret-gateway.test.ts | 113 +++ src/cli/command-secret-gateway.ts | 60 +- src/cli/command-secret-targets.test.ts | 1 + src/cli/command-secret-targets.ts | 1 + src/config/types.tools.ts | 2 +- src/gateway/server.reload.test.ts | 93 +++ src/secrets/runtime-config-collectors-core.ts | 62 -- src/secrets/runtime-shared.ts | 7 +- src/secrets/runtime-web-tools.test.ts | 451 +++++++++++ src/secrets/runtime-web-tools.ts | 705 ++++++++++++++++++ src/secrets/runtime.test.ts | 164 +++- src/secrets/runtime.ts | 16 +- src/secrets/target-registry-data.ts | 11 + 28 files changed, 2059 insertions(+), 112 deletions(-) create mode 100644 src/agents/openclaw-tools.web-runtime.test.ts create mode 100644 src/secrets/runtime-web-tools.test.ts create mode 100644 src/secrets/runtime-web-tools.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 95f3ab600cb..c19a5c2eda7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Resolve web tool SecretRefs atomically at runtime. (#41599) Thanks @joshavant. - Feishu/local image auto-convert: pass `mediaLocalRoots` through the `sendText` local-image shim so allowed local image paths upload as Feishu images again instead of falling back to raw path text. (#40623) Thanks @ayanesakura. - macOS/LaunchAgent install: tighten LaunchAgent directory and plist permissions during install so launchd bootstrap does not fail when the target home path or generated plist inherited group/world-writable modes. - Gateway/Control UI: keep dashboard auth tokens in session-scoped browser storage so same-tab refreshes preserve remote token auth without restoring long-lived localStorage token persistence, while scoping tokens to the selected gateway URL and fragment-only bootstrap flow. (#40892) thanks @velvet-shark. diff --git a/docs/gateway/secrets.md b/docs/gateway/secrets.md index 3ef08267618..e9d75343147 100644 --- a/docs/gateway/secrets.md +++ b/docs/gateway/secrets.md @@ -38,7 +38,8 @@ Examples of inactive surfaces: - Top-level channel credentials that no enabled account inherits. - Disabled tool/feature surfaces. - Web search provider-specific keys that are not selected by `tools.web.search.provider`. - In auto mode (provider unset), provider-specific keys are also active for provider auto-detection. + In auto mode (provider unset), keys are consulted by precedence for provider auto-detection until one resolves. + After selection, non-selected provider keys are treated as inactive until selected. - `gateway.remote.token` / `gateway.remote.password` SecretRefs are active (when `gateway.remote.enabled` is not `false`) if one of these is true: - `gateway.mode=remote` - `gateway.remote.url` is configured diff --git a/docs/help/faq.md b/docs/help/faq.md index 7dad0548fd4..a43e91f4396 100644 --- a/docs/help/faq.md +++ b/docs/help/faq.md @@ -1489,10 +1489,16 @@ Set `cli.banner.taglineMode` in config: ### How do I enable web search and web fetch -`web_fetch` works without an API key. `web_search` requires a Brave Search API -key. **Recommended:** run `openclaw configure --section web` to store it in -`tools.web.search.apiKey`. Environment alternative: set `BRAVE_API_KEY` for the -Gateway process. +`web_fetch` works without an API key. `web_search` requires a key for your +selected provider (Brave, Gemini, Grok, Kimi, or Perplexity). +**Recommended:** run `openclaw configure --section web` and choose a provider. +Environment alternatives: + +- Brave: `BRAVE_API_KEY` +- Gemini: `GEMINI_API_KEY` +- Grok: `XAI_API_KEY` +- Kimi: `KIMI_API_KEY` or `MOONSHOT_API_KEY` +- Perplexity: `PERPLEXITY_API_KEY` or `OPENROUTER_API_KEY` ```json5 { @@ -1500,6 +1506,7 @@ Gateway process. web: { search: { enabled: true, + provider: "brave", apiKey: "BRAVE_API_KEY_HERE", maxResults: 5, }, diff --git a/docs/perplexity.md b/docs/perplexity.md index bb1acef49c8..f7eccc9453e 100644 --- a/docs/perplexity.md +++ b/docs/perplexity.md @@ -71,11 +71,14 @@ Optional legacy controls: **Via config:** run `openclaw configure --section web`. It stores the key in `~/.openclaw/openclaw.json` under `tools.web.search.perplexity.apiKey`. +That field also accepts SecretRef objects. **Via environment:** set `PERPLEXITY_API_KEY` or `OPENROUTER_API_KEY` in the Gateway process environment. For a gateway install, put it in `~/.openclaw/.env` (or your service environment). See [Env vars](/help/faq#how-does-openclaw-load-environment-variables). +If `provider: "perplexity"` is configured and the Perplexity key SecretRef is unresolved with no env fallback, startup/reload fails fast. + ## Tool parameters These parameters apply to the native Perplexity Search API path. diff --git a/docs/reference/api-usage-costs.md b/docs/reference/api-usage-costs.md index dba017aacc1..baf4302ac0d 100644 --- a/docs/reference/api-usage-costs.md +++ b/docs/reference/api-usage-costs.md @@ -80,10 +80,10 @@ See [Memory](/concepts/memory). `web_search` uses API keys and may incur usage charges depending on your provider: - **Brave Search API**: `BRAVE_API_KEY` or `tools.web.search.apiKey` -- **Gemini (Google Search)**: `GEMINI_API_KEY` -- **Grok (xAI)**: `XAI_API_KEY` -- **Kimi (Moonshot)**: `KIMI_API_KEY` or `MOONSHOT_API_KEY` -- **Perplexity Search API**: `PERPLEXITY_API_KEY` +- **Gemini (Google Search)**: `GEMINI_API_KEY` or `tools.web.search.gemini.apiKey` +- **Grok (xAI)**: `XAI_API_KEY` or `tools.web.search.grok.apiKey` +- **Kimi (Moonshot)**: `KIMI_API_KEY`, `MOONSHOT_API_KEY`, or `tools.web.search.kimi.apiKey` +- **Perplexity Search API**: `PERPLEXITY_API_KEY`, `OPENROUTER_API_KEY`, or `tools.web.search.perplexity.apiKey` **Brave Search free credit:** Each Brave plan includes $5/month in renewing free credit. The Search plan costs $5 per 1,000 requests, so the credit covers diff --git a/docs/reference/secretref-credential-surface.md b/docs/reference/secretref-credential-surface.md index dd1b5f1fd2f..2a5fc5a66ac 100644 --- a/docs/reference/secretref-credential-surface.md +++ b/docs/reference/secretref-credential-surface.md @@ -31,6 +31,7 @@ Scope intent: - `talk.providers.*.apiKey` - `messages.tts.elevenlabs.apiKey` - `messages.tts.openai.apiKey` +- `tools.web.fetch.firecrawl.apiKey` - `tools.web.search.apiKey` - `tools.web.search.gemini.apiKey` - `tools.web.search.grok.apiKey` @@ -102,7 +103,8 @@ Notes: - For SecretRef-managed model providers, generated `agents/*/agent/models.json` entries persist non-secret markers (not resolved secret values) for `apiKey`/header surfaces. - For web search: - In explicit provider mode (`tools.web.search.provider` set), only the selected provider key is active. - - In auto mode (`tools.web.search.provider` unset), `tools.web.search.apiKey` and provider-specific keys are active. + - In auto mode (`tools.web.search.provider` unset), only the first provider key that resolves by precedence is active. + - In auto mode, non-selected provider refs are treated as inactive until selected. ## Unsupported credentials diff --git a/docs/reference/secretref-user-supplied-credentials-matrix.json b/docs/reference/secretref-user-supplied-credentials-matrix.json index 773ef8ab162..6d4b05d2822 100644 --- a/docs/reference/secretref-user-supplied-credentials-matrix.json +++ b/docs/reference/secretref-user-supplied-credentials-matrix.json @@ -454,6 +454,13 @@ "secretShape": "secret_input", "optIn": true }, + { + "id": "tools.web.fetch.firecrawl.apiKey", + "configFile": "openclaw.json", + "path": "tools.web.fetch.firecrawl.apiKey", + "secretShape": "secret_input", + "optIn": true + }, { "id": "tools.web.search.apiKey", "configFile": "openclaw.json", diff --git a/docs/tools/firecrawl.md b/docs/tools/firecrawl.md index e859eb2dcb1..2cd90a06bf5 100644 --- a/docs/tools/firecrawl.md +++ b/docs/tools/firecrawl.md @@ -40,7 +40,8 @@ with JS-heavy sites or pages that block plain HTTP fetches. Notes: -- `firecrawl.enabled` defaults to true when an API key is present. +- `firecrawl.enabled` defaults to `true` unless explicitly set to `false`. +- Firecrawl fallback attempts run only when an API key is available (`tools.web.fetch.firecrawl.apiKey` or `FIRECRAWL_API_KEY`). - `maxAgeMs` controls how old cached results can be (ms). Default is 2 days. ## Stealth / bot circumvention diff --git a/docs/tools/web.md b/docs/tools/web.md index 1eeb4eba7db..e77d046ce5b 100644 --- a/docs/tools/web.md +++ b/docs/tools/web.md @@ -2,7 +2,7 @@ summary: "Web search + fetch tools (Brave, Gemini, Grok, Kimi, and Perplexity providers)" read_when: - You want to enable web_search or web_fetch - - You need Brave or Perplexity Search API key setup + - You need provider API key setup - You want to use Gemini with Google Search grounding title: "Web Tools" --- @@ -49,6 +49,12 @@ The table above is alphabetical. If no `provider` is explicitly set, runtime aut If no keys are found, it falls back to Brave (you'll get a missing-key error prompting you to configure one). +Runtime SecretRef behavior: + +- Web tool SecretRefs are resolved atomically at gateway startup/reload. +- In auto-detect mode, OpenClaw resolves only the selected provider key. Non-selected provider SecretRefs stay inactive until selected. +- If the selected provider SecretRef is unresolved and no provider env fallback exists, startup/reload fails fast. + ## Setting up web search Use `openclaw configure --section web` to set up your API key and choose a provider. @@ -77,9 +83,25 @@ See [Perplexity Search API Docs](https://docs.perplexity.ai/guides/search-quicks ### Where to store the key -**Via config:** run `openclaw configure --section web`. It stores the key under `tools.web.search.apiKey` or `tools.web.search.perplexity.apiKey`, depending on provider. +**Via config:** run `openclaw configure --section web`. It stores the key under the provider-specific config path: -**Via environment:** set `PERPLEXITY_API_KEY`, `OPENROUTER_API_KEY`, or `BRAVE_API_KEY` in the Gateway process environment. For a gateway install, put it in `~/.openclaw/.env` (or your service environment). See [Env vars](/help/faq#how-does-openclaw-load-environment-variables). +- Brave: `tools.web.search.apiKey` +- Gemini: `tools.web.search.gemini.apiKey` +- Grok: `tools.web.search.grok.apiKey` +- Kimi: `tools.web.search.kimi.apiKey` +- Perplexity: `tools.web.search.perplexity.apiKey` + +All of these fields also support SecretRef objects. + +**Via environment:** set provider env vars in the Gateway process environment: + +- Brave: `BRAVE_API_KEY` +- Gemini: `GEMINI_API_KEY` +- Grok: `XAI_API_KEY` +- Kimi: `KIMI_API_KEY` or `MOONSHOT_API_KEY` +- Perplexity: `PERPLEXITY_API_KEY` or `OPENROUTER_API_KEY` + +For a gateway install, put these in `~/.openclaw/.env` (or your service environment). See [Env vars](/help/faq#how-does-openclaw-load-environment-variables). ### Config examples @@ -216,6 +238,7 @@ Search the web using your configured provider. - **Grok**: `XAI_API_KEY` or `tools.web.search.grok.apiKey` - **Kimi**: `KIMI_API_KEY`, `MOONSHOT_API_KEY`, or `tools.web.search.kimi.apiKey` - **Perplexity**: `PERPLEXITY_API_KEY`, `OPENROUTER_API_KEY`, or `tools.web.search.perplexity.apiKey` +- All provider key fields above support SecretRef objects. ### Config @@ -310,6 +333,7 @@ Fetch a URL and extract readable content. - `tools.web.fetch.enabled` must not be `false` (default: enabled) - Optional Firecrawl fallback: set `tools.web.fetch.firecrawl.apiKey` or `FIRECRAWL_API_KEY`. +- `tools.web.fetch.firecrawl.apiKey` supports SecretRef objects. ### web_fetch config @@ -351,6 +375,8 @@ Notes: - `web_fetch` uses Readability (main-content extraction) first, then Firecrawl (if configured). If both fail, the tool returns an error. - Firecrawl requests use bot-circumvention mode and cache results by default. +- Firecrawl SecretRefs are resolved only when Firecrawl is active (`tools.web.fetch.enabled !== false` and `tools.web.fetch.firecrawl.enabled !== false`). +- If Firecrawl is active and its SecretRef is unresolved with no `FIRECRAWL_API_KEY` fallback, startup/reload fails fast. - `web_fetch` sends a Chrome-like User-Agent and `Accept-Language` by default; override `userAgent` if needed. - `web_fetch` blocks private/internal hostnames and re-checks redirects (limit with `maxRedirects`). - `maxChars` is clamped to `tools.web.fetch.maxCharsCap`. diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index 17f8e6dadb4..56d0801d13c 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -1,5 +1,6 @@ import type { OpenClawConfig } from "../config/config.js"; import { resolvePluginTools } from "../plugins/tools.js"; +import { getActiveRuntimeWebToolsMetadata } from "../secrets/runtime.js"; import type { GatewayMessageChannel } from "../utils/message-channel.js"; import { resolveSessionAgentId } from "./agent-scope.js"; import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; @@ -72,6 +73,7 @@ export function createOpenClawTools( } & SpawnedToolContext, ): AnyAgentTool[] { const workspaceDir = resolveWorkspaceRoot(options?.workspaceDir); + const runtimeWebTools = getActiveRuntimeWebToolsMetadata(); const imageTool = options?.agentDir?.trim() ? createImageTool({ config: options?.config, @@ -100,10 +102,12 @@ export function createOpenClawTools( const webSearchTool = createWebSearchTool({ config: options?.config, sandboxed: options?.sandboxed, + runtimeWebSearch: runtimeWebTools?.search, }); const webFetchTool = createWebFetchTool({ config: options?.config, sandboxed: options?.sandboxed, + runtimeFirecrawl: runtimeWebTools?.fetch.firecrawl, }); const messageTool = options?.disableMessageTool ? null diff --git a/src/agents/openclaw-tools.web-runtime.test.ts b/src/agents/openclaw-tools.web-runtime.test.ts new file mode 100644 index 00000000000..94478930cf1 --- /dev/null +++ b/src/agents/openclaw-tools.web-runtime.test.ts @@ -0,0 +1,135 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import { + activateSecretsRuntimeSnapshot, + clearSecretsRuntimeSnapshot, + prepareSecretsRuntimeSnapshot, +} from "../secrets/runtime.js"; +import { withFetchPreconnect } from "../test-utils/fetch-mock.js"; +import { createOpenClawTools } from "./openclaw-tools.js"; + +vi.mock("../plugins/tools.js", () => ({ + resolvePluginTools: () => [], +})); + +function asConfig(value: unknown): OpenClawConfig { + return value as OpenClawConfig; +} + +function findTool(name: string, config: OpenClawConfig) { + const allTools = createOpenClawTools({ config, sandboxed: true }); + const tool = allTools.find((candidate) => candidate.name === name); + expect(tool).toBeDefined(); + if (!tool) { + throw new Error(`missing ${name} tool`); + } + return tool; +} + +function makeHeaders(map: Record): { get: (key: string) => string | null } { + return { + get: (key) => map[key.toLowerCase()] ?? null, + }; +} + +async function prepareAndActivate(params: { config: OpenClawConfig; env?: NodeJS.ProcessEnv }) { + const snapshot = await prepareSecretsRuntimeSnapshot({ + config: params.config, + env: params.env, + agentDirs: ["/tmp/openclaw-agent-main"], + loadAuthStore: () => ({ version: 1, profiles: {} }), + }); + activateSecretsRuntimeSnapshot(snapshot); + return snapshot; +} + +describe("openclaw tools runtime web metadata wiring", () => { + const priorFetch = global.fetch; + + afterEach(() => { + global.fetch = priorFetch; + clearSecretsRuntimeSnapshot(); + }); + + it("uses runtime-selected provider when higher-precedence provider ref is unresolved", async () => { + const snapshot = await prepareAndActivate({ + config: asConfig({ + tools: { + web: { + search: { + apiKey: { source: "env", provider: "default", id: "MISSING_BRAVE_KEY_REF" }, + gemini: { + apiKey: { source: "env", provider: "default", id: "GEMINI_WEB_KEY_REF" }, + }, + }, + }, + }, + }), + env: { + GEMINI_WEB_KEY_REF: "gemini-runtime-key", + }, + }); + + expect(snapshot.webTools.search.selectedProvider).toBe("gemini"); + + const mockFetch = vi.fn((_input?: unknown, _init?: unknown) => + Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + candidates: [ + { + content: { parts: [{ text: "runtime gemini ok" }] }, + groundingMetadata: { groundingChunks: [] }, + }, + ], + }), + } as Response), + ); + global.fetch = withFetchPreconnect(mockFetch); + + const webSearch = findTool("web_search", snapshot.config); + const result = await webSearch.execute("call-runtime-search", { query: "runtime search" }); + + expect(mockFetch).toHaveBeenCalled(); + expect(String(mockFetch.mock.calls[0]?.[0])).toContain("generativelanguage.googleapis.com"); + expect((result.details as { provider?: string }).provider).toBe("gemini"); + }); + + it("skips Firecrawl key resolution when runtime marks Firecrawl inactive", async () => { + const snapshot = await prepareAndActivate({ + config: asConfig({ + tools: { + web: { + fetch: { + firecrawl: { + enabled: false, + apiKey: { source: "env", provider: "default", id: "MISSING_FIRECRAWL_KEY_REF" }, + }, + }, + }, + }, + }), + }); + + const mockFetch = vi.fn((_input?: unknown, _init?: unknown) => + Promise.resolve({ + ok: true, + status: 200, + headers: makeHeaders({ "content-type": "text/html; charset=utf-8" }), + text: () => + Promise.resolve( + "

Runtime Off

Use direct fetch.

", + ), + } as Response), + ); + global.fetch = withFetchPreconnect(mockFetch); + + const webFetch = findTool("web_fetch", snapshot.config); + await webFetch.execute("call-runtime-fetch", { url: "https://example.com/runtime-off" }); + + expect(mockFetch).toHaveBeenCalled(); + expect(mockFetch.mock.calls[0]?.[0]).toBe("https://example.com/runtime-off"); + expect(String(mockFetch.mock.calls[0]?.[0])).not.toContain("api.firecrawl.dev"); + }); +}); diff --git a/src/agents/tools/web-fetch.cf-markdown.test.ts b/src/agents/tools/web-fetch.cf-markdown.test.ts index 6e7768fc43a..e235177a309 100644 --- a/src/agents/tools/web-fetch.cf-markdown.test.ts +++ b/src/agents/tools/web-fetch.cf-markdown.test.ts @@ -84,6 +84,47 @@ describe("web_fetch Cloudflare Markdown for Agents", () => { expect(details?.contentType).toBe("text/html"); }); + it("bypasses Firecrawl when runtime metadata marks Firecrawl inactive", async () => { + const fetchSpy = vi + .fn() + .mockResolvedValue( + htmlResponse( + "

Runtime Off

Use direct fetch.

", + ), + ); + global.fetch = withFetchPreconnect(fetchSpy); + + const tool = createWebFetchTool({ + config: { + tools: { + web: { + fetch: { + firecrawl: { + enabled: true, + apiKey: { + source: "env", + provider: "default", + id: "MISSING_FIRECRAWL_KEY_REF", + }, + }, + }, + }, + }, + }, + sandboxed: false, + runtimeFirecrawl: { + active: false, + apiKeySource: "secretRef", + diagnostics: [], + }, + }); + + await tool?.execute?.("call", { url: "https://example.com/runtime-firecrawl-off" }); + + expect(fetchSpy).toHaveBeenCalled(); + expect(fetchSpy.mock.calls[0]?.[0]).toBe("https://example.com/runtime-firecrawl-off"); + }); + it("logs x-markdown-tokens when header is present", async () => { const logSpy = vi.spyOn(logger, "logDebug").mockImplementation(() => {}); const fetchSpy = vi diff --git a/src/agents/tools/web-fetch.ts b/src/agents/tools/web-fetch.ts index 4ac7a1d7bfd..f4cc88e2d83 100644 --- a/src/agents/tools/web-fetch.ts +++ b/src/agents/tools/web-fetch.ts @@ -1,7 +1,9 @@ import { Type } from "@sinclair/typebox"; import type { OpenClawConfig } from "../../config/config.js"; +import { normalizeResolvedSecretInputString } from "../../config/types.secrets.js"; import { SsrFBlockedError } from "../../infra/net/ssrf.js"; import { logDebug } from "../../logger.js"; +import type { RuntimeWebFetchFirecrawlMetadata } from "../../secrets/runtime-web-tools.js"; import { wrapExternalContent, wrapWebContent } from "../../security/external-content.js"; import { normalizeSecretInput } from "../../utils/normalize-secret-input.js"; import { stringEnum } from "../schema/typebox.js"; @@ -71,7 +73,7 @@ type WebFetchConfig = NonNullable["web"] extends infer type FirecrawlFetchConfig = | { enabled?: boolean; - apiKey?: string; + apiKey?: unknown; baseUrl?: string; onlyMainContent?: boolean; maxAgeMs?: number; @@ -136,10 +138,14 @@ function resolveFirecrawlConfig(fetch?: WebFetchConfig): FirecrawlFetchConfig { } function resolveFirecrawlApiKey(firecrawl?: FirecrawlFetchConfig): string | undefined { - const fromConfig = - firecrawl && "apiKey" in firecrawl && typeof firecrawl.apiKey === "string" - ? normalizeSecretInput(firecrawl.apiKey) - : ""; + const fromConfigRaw = + firecrawl && "apiKey" in firecrawl + ? normalizeResolvedSecretInputString({ + value: firecrawl.apiKey, + path: "tools.web.fetch.firecrawl.apiKey", + }) + : undefined; + const fromConfig = normalizeSecretInput(fromConfigRaw); const fromEnv = normalizeSecretInput(process.env.FIRECRAWL_API_KEY); return fromConfig || fromEnv || undefined; } @@ -712,6 +718,7 @@ function resolveFirecrawlEndpoint(baseUrl: string): string { export function createWebFetchTool(options?: { config?: OpenClawConfig; sandboxed?: boolean; + runtimeFirecrawl?: RuntimeWebFetchFirecrawlMetadata; }): AnyAgentTool | null { const fetch = resolveFetchConfig(options?.config); if (!resolveFetchEnabled({ fetch, sandboxed: options?.sandboxed })) { @@ -719,8 +726,14 @@ export function createWebFetchTool(options?: { } const readabilityEnabled = resolveFetchReadabilityEnabled(fetch); const firecrawl = resolveFirecrawlConfig(fetch); - const firecrawlApiKey = resolveFirecrawlApiKey(firecrawl); - const firecrawlEnabled = resolveFirecrawlEnabled({ firecrawl, apiKey: firecrawlApiKey }); + const runtimeFirecrawlActive = options?.runtimeFirecrawl?.active; + const shouldResolveFirecrawlApiKey = + runtimeFirecrawlActive === undefined ? firecrawl?.enabled !== false : runtimeFirecrawlActive; + const firecrawlApiKey = shouldResolveFirecrawlApiKey + ? resolveFirecrawlApiKey(firecrawl) + : undefined; + const firecrawlEnabled = + runtimeFirecrawlActive ?? resolveFirecrawlEnabled({ firecrawl, apiKey: firecrawlApiKey }); const firecrawlBaseUrl = resolveFirecrawlBaseUrl(firecrawl); const firecrawlOnlyMainContent = resolveFirecrawlOnlyMainContent(firecrawl); const firecrawlMaxAgeMs = resolveFirecrawlMaxAgeMsOrDefault(firecrawl); diff --git a/src/agents/tools/web-search.ts b/src/agents/tools/web-search.ts index d4f88caea61..4fbbfa95e43 100644 --- a/src/agents/tools/web-search.ts +++ b/src/agents/tools/web-search.ts @@ -3,6 +3,7 @@ import { formatCliCommand } from "../../cli/command-format.js"; import type { OpenClawConfig } from "../../config/config.js"; import { normalizeResolvedSecretInputString } from "../../config/types.secrets.js"; import { logVerbose } from "../../globals.js"; +import type { RuntimeWebSearchMetadata } from "../../secrets/runtime-web-tools.js"; import { wrapWebContent } from "../../security/external-content.js"; import { normalizeSecretInput } from "../../utils/normalize-secret-input.js"; import type { AnyAgentTool } from "./common.js"; @@ -193,6 +194,33 @@ function createWebSearchSchema(params: { ), } as const; + const perplexityStructuredFilterSchema = { + country: Type.Optional( + Type.String({ + description: + "Native Perplexity Search API only. 2-letter country code for region-specific results (e.g., 'DE', 'US', 'ALL'). Default: 'US'.", + }), + ), + language: Type.Optional( + Type.String({ + description: + "Native Perplexity Search API only. ISO 639-1 language code for results (e.g., 'en', 'de', 'fr').", + }), + ), + date_after: Type.Optional( + Type.String({ + description: + "Native Perplexity Search API only. Only results published after this date (YYYY-MM-DD).", + }), + ), + date_before: Type.Optional( + Type.String({ + description: + "Native Perplexity Search API only. Only results published before this date (YYYY-MM-DD).", + }), + ), + } as const; + if (params.provider === "brave") { return Type.Object({ ...querySchema, @@ -221,7 +249,8 @@ function createWebSearchSchema(params: { } return Type.Object({ ...querySchema, - ...filterSchema, + freshness: filterSchema.freshness, + ...perplexityStructuredFilterSchema, domain_filter: Type.Optional( Type.Array(Type.String(), { description: @@ -742,6 +771,16 @@ function resolvePerplexityTransport(perplexity?: PerplexityConfig): { }; } +function resolvePerplexitySchemaTransportHint( + perplexity?: PerplexityConfig, +): PerplexityTransport | undefined { + const hasLegacyOverride = Boolean( + (perplexity?.baseUrl && perplexity.baseUrl.trim()) || + (perplexity?.model && perplexity.model.trim()), + ); + return hasLegacyOverride ? "chat_completions" : undefined; +} + function resolveGrokConfig(search?: WebSearchConfig): GrokConfig { if (!search || typeof search !== "object") { return {}; @@ -1809,15 +1848,21 @@ async function runWebSearch(params: { export function createWebSearchTool(options?: { config?: OpenClawConfig; sandboxed?: boolean; + runtimeWebSearch?: RuntimeWebSearchMetadata; }): AnyAgentTool | null { const search = resolveSearchConfig(options?.config); if (!resolveSearchEnabled({ search, sandboxed: options?.sandboxed })) { return null; } - const provider = resolveSearchProvider(search); + const provider = + options?.runtimeWebSearch?.selectedProvider ?? + options?.runtimeWebSearch?.providerConfigured ?? + resolveSearchProvider(search); const perplexityConfig = resolvePerplexityConfig(search); - const perplexityTransport = resolvePerplexityTransport(perplexityConfig); + const perplexitySchemaTransportHint = + options?.runtimeWebSearch?.perplexityTransport ?? + resolvePerplexitySchemaTransportHint(perplexityConfig); const grokConfig = resolveGrokConfig(search); const geminiConfig = resolveGeminiConfig(search); const kimiConfig = resolveKimiConfig(search); @@ -1826,9 +1871,9 @@ export function createWebSearchTool(options?: { const description = provider === "perplexity" - ? perplexityTransport.transport === "chat_completions" + ? perplexitySchemaTransportHint === "chat_completions" ? "Search the web using Perplexity Sonar via Perplexity/OpenRouter chat completions. Returns AI-synthesized answers with citations from web-grounded search." - : "Search the web using the Perplexity Search API. Returns structured results (title, URL, snippet) for fast research. Supports domain, region, language, and freshness filtering." + : "Search the web using Perplexity. Runtime routing decides between native Search API and Sonar chat-completions compatibility. Structured filters are available on the native Search API path." : provider === "grok" ? "Search the web using xAI Grok. Returns AI-synthesized answers with citations from real-time web search." : provider === "kimi" @@ -1845,10 +1890,13 @@ export function createWebSearchTool(options?: { description, parameters: createWebSearchSchema({ provider, - perplexityTransport: provider === "perplexity" ? perplexityTransport.transport : undefined, + perplexityTransport: provider === "perplexity" ? perplexitySchemaTransportHint : undefined, }), execute: async (_toolCallId, args) => { - const perplexityRuntime = provider === "perplexity" ? perplexityTransport : undefined; + // Resolve Perplexity auth/transport lazily at execution time so unrelated providers + // do not touch Perplexity-only credential surfaces during tool construction. + const perplexityRuntime = + provider === "perplexity" ? resolvePerplexityTransport(perplexityConfig) : undefined; const apiKey = provider === "perplexity" ? perplexityRuntime?.apiKey diff --git a/src/agents/tools/web-tools.enabled-defaults.test.ts b/src/agents/tools/web-tools.enabled-defaults.test.ts index 80dcd6a025d..4951f1c6b5a 100644 --- a/src/agents/tools/web-tools.enabled-defaults.test.ts +++ b/src/agents/tools/web-tools.enabled-defaults.test.ts @@ -166,6 +166,39 @@ describe("web tools defaults", () => { const tool = createWebSearchTool({ config: {}, sandboxed: false }); expect(tool?.name).toBe("web_search"); }); + + it("prefers runtime-selected web_search provider over local provider config", async () => { + const mockFetch = installMockFetch(createProviderSuccessPayload("gemini")); + const tool = createWebSearchTool({ + config: { + tools: { + web: { + search: { + provider: "brave", + apiKey: "brave-config-test", // pragma: allowlist secret + gemini: { + apiKey: "gemini-config-test", // pragma: allowlist secret + }, + }, + }, + }, + }, + sandboxed: true, + runtimeWebSearch: { + providerConfigured: "brave", + providerSource: "auto-detect", + selectedProvider: "gemini", + selectedProviderKeySource: "secretRef", + diagnostics: [], + }, + }); + + const result = await tool?.execute?.("call-runtime-provider", { query: "runtime override" }); + + expect(mockFetch).toHaveBeenCalled(); + expect(String(mockFetch.mock.calls[0]?.[0])).toContain("generativelanguage.googleapis.com"); + expect((result?.details as { provider?: string } | undefined)?.provider).toBe("gemini"); + }); }); describe("web_search country and language parameters", () => { @@ -489,20 +522,56 @@ describe("web_search perplexity OpenRouter compatibility", () => { expect(result?.details).toMatchObject({ error: "unsupported_domain_filter" }); }); - it("hides Search API-only schema params on the compatibility path", () => { + it("keeps Search API schema params visible before runtime auth routing", () => { vi.stubEnv("OPENROUTER_API_KEY", "sk-or-v1-test"); // pragma: allowlist secret const tool = createPerplexitySearchTool(); const properties = (tool?.parameters as { properties?: Record } | undefined) ?.properties; expect(properties?.freshness).toBeDefined(); - expect(properties?.country).toBeUndefined(); - expect(properties?.language).toBeUndefined(); - expect(properties?.date_after).toBeUndefined(); - expect(properties?.date_before).toBeUndefined(); - expect(properties?.domain_filter).toBeUndefined(); - expect(properties?.max_tokens).toBeUndefined(); - expect(properties?.max_tokens_per_page).toBeUndefined(); + expect(properties?.country).toBeDefined(); + expect(properties?.language).toBeDefined(); + expect(properties?.date_after).toBeDefined(); + expect(properties?.date_before).toBeDefined(); + expect(properties?.domain_filter).toBeDefined(); + expect(properties?.max_tokens).toBeDefined(); + expect(properties?.max_tokens_per_page).toBeDefined(); + expect( + ( + properties?.country as + | { + description?: string; + } + | undefined + )?.description, + ).toContain("Native Perplexity Search API only."); + expect( + ( + properties?.language as + | { + description?: string; + } + | undefined + )?.description, + ).toContain("Native Perplexity Search API only."); + expect( + ( + properties?.date_after as + | { + description?: string; + } + | undefined + )?.description, + ).toContain("Native Perplexity Search API only."); + expect( + ( + properties?.date_before as + | { + description?: string; + } + | undefined + )?.description, + ).toContain("Native Perplexity Search API only."); }); it("keeps structured schema params on the native Search API path", () => { @@ -522,6 +591,61 @@ describe("web_search perplexity OpenRouter compatibility", () => { }); }); +describe("web_search Perplexity lazy resolution", () => { + const priorFetch = global.fetch; + + afterEach(() => { + vi.unstubAllEnvs(); + global.fetch = priorFetch; + }); + + it("does not read Perplexity credentials while creating non-Perplexity tools", () => { + const perplexityConfig: Record = {}; + Object.defineProperty(perplexityConfig, "apiKey", { + enumerable: true, + get() { + throw new Error("perplexity-apiKey-getter-called"); + }, + }); + + const tool = createWebSearchTool({ + config: { + tools: { + web: { + search: { + provider: "gemini", + gemini: { apiKey: "gemini-config-test" }, + perplexity: perplexityConfig as { apiKey?: string; baseUrl?: string; model?: string }, + }, + }, + }, + }, + sandboxed: true, + }); + + expect(tool?.name).toBe("web_search"); + }); + + it("defers Perplexity credential reads until execute", async () => { + const perplexityConfig: Record = {}; + Object.defineProperty(perplexityConfig, "apiKey", { + enumerable: true, + get() { + throw new Error("perplexity-apiKey-getter-called"); + }, + }); + + const tool = createPerplexitySearchTool( + perplexityConfig as { apiKey?: string; baseUrl?: string; model?: string }, + ); + + expect(tool?.name).toBe("web_search"); + await expect(tool?.execute?.("call-1", { query: "test" })).rejects.toThrow( + /perplexity-apiKey-getter-called/, + ); + }); +}); + describe("web_search kimi provider", () => { const priorFetch = global.fetch; diff --git a/src/cli/command-secret-gateway.test.ts b/src/cli/command-secret-gateway.test.ts index 7929cdbdafc..6d0f89f6349 100644 --- a/src/cli/command-secret-gateway.test.ts +++ b/src/cli/command-secret-gateway.test.ts @@ -206,6 +206,119 @@ describe("resolveCommandSecretRefsViaGateway", () => { } }); + it("falls back to local resolution for web search SecretRefs when gateway is unavailable", async () => { + const envKey = "WEB_SEARCH_GEMINI_API_KEY_LOCAL_FALLBACK"; + const priorValue = process.env[envKey]; + process.env[envKey] = "gemini-local-fallback-key"; + callGateway.mockRejectedValueOnce(new Error("gateway closed")); + + try { + const result = await resolveCommandSecretRefsViaGateway({ + config: { + tools: { + web: { + search: { + provider: "gemini", + gemini: { + apiKey: { source: "env", provider: "default", id: envKey }, + }, + }, + }, + }, + } as OpenClawConfig, + commandName: "agent", + targetIds: new Set(["tools.web.search.gemini.apiKey"]), + }); + + expect(result.resolvedConfig.tools?.web?.search?.gemini?.apiKey).toBe( + "gemini-local-fallback-key", + ); + expect(result.targetStatesByPath["tools.web.search.gemini.apiKey"]).toBe("resolved_local"); + expect( + result.diagnostics.some((entry) => entry.includes("gateway secrets.resolve unavailable")), + ).toBe(true); + expect( + result.diagnostics.some((entry) => entry.includes("resolved command secrets locally")), + ).toBe(true); + } finally { + if (priorValue === undefined) { + delete process.env[envKey]; + } else { + process.env[envKey] = priorValue; + } + } + }); + + it("falls back to local resolution for Firecrawl SecretRefs when gateway is unavailable", async () => { + const envKey = "WEB_FETCH_FIRECRAWL_API_KEY_LOCAL_FALLBACK"; + const priorValue = process.env[envKey]; + process.env[envKey] = "firecrawl-local-fallback-key"; + callGateway.mockRejectedValueOnce(new Error("gateway closed")); + + try { + const result = await resolveCommandSecretRefsViaGateway({ + config: { + tools: { + web: { + fetch: { + firecrawl: { + apiKey: { source: "env", provider: "default", id: envKey }, + }, + }, + }, + }, + } as OpenClawConfig, + commandName: "agent", + targetIds: new Set(["tools.web.fetch.firecrawl.apiKey"]), + }); + + expect(result.resolvedConfig.tools?.web?.fetch?.firecrawl?.apiKey).toBe( + "firecrawl-local-fallback-key", + ); + expect(result.targetStatesByPath["tools.web.fetch.firecrawl.apiKey"]).toBe("resolved_local"); + expect( + result.diagnostics.some((entry) => entry.includes("gateway secrets.resolve unavailable")), + ).toBe(true); + expect( + result.diagnostics.some((entry) => entry.includes("resolved command secrets locally")), + ).toBe(true); + } finally { + if (priorValue === undefined) { + delete process.env[envKey]; + } else { + process.env[envKey] = priorValue; + } + } + }); + + it("marks web SecretRefs inactive when the web surface is disabled during local fallback", async () => { + callGateway.mockRejectedValueOnce(new Error("gateway closed")); + const result = await resolveCommandSecretRefsViaGateway({ + config: { + tools: { + web: { + search: { + enabled: false, + gemini: { + apiKey: { source: "env", provider: "default", id: "WEB_SEARCH_DISABLED_KEY" }, + }, + }, + }, + }, + } as OpenClawConfig, + commandName: "agent", + targetIds: new Set(["tools.web.search.gemini.apiKey"]), + }); + + expect(result.hadUnresolvedTargets).toBe(false); + expect(result.targetStatesByPath["tools.web.search.gemini.apiKey"]).toBe("inactive_surface"); + expect( + result.diagnostics.some((entry) => + entry.includes("tools.web.search.gemini.apiKey: tools.web.search is disabled."), + ), + ).toBe(true); + }); + it("returns a version-skew hint when gateway does not support secrets.resolve", async () => { const envKey = "TALK_API_KEY_UNSUPPORTED"; callGateway.mockRejectedValueOnce(new Error("unknown method: secrets.resolve")); diff --git a/src/cli/command-secret-gateway.ts b/src/cli/command-secret-gateway.ts index 89b8c78a3e3..03e578b642c 100644 --- a/src/cli/command-secret-gateway.ts +++ b/src/cli/command-secret-gateway.ts @@ -10,6 +10,7 @@ import { getPath, setPathExistingStrict } from "../secrets/path-utils.js"; import { resolveSecretRefValue } from "../secrets/resolve.js"; import { collectConfigAssignments } from "../secrets/runtime-config-collectors.js"; import { createResolverContext } from "../secrets/runtime-shared.js"; +import { resolveRuntimeWebTools } from "../secrets/runtime-web-tools.js"; import { assertExpectedResolvedSecretValue } from "../secrets/secret-value.js"; import { describeUnknownError } from "../secrets/shared.js"; import { @@ -44,6 +45,15 @@ type GatewaySecretsResolveResult = { inactiveRefPaths?: string[]; }; +const WEB_RUNTIME_SECRET_TARGET_ID_PREFIXES = [ + "tools.web.search", + "tools.web.fetch.firecrawl", +] as const; +const WEB_RUNTIME_SECRET_PATH_PREFIXES = [ + "tools.web.search.", + "tools.web.fetch.firecrawl.", +] as const; + function dedupeDiagnostics(entries: readonly string[]): string[] { const seen = new Set(); const ordered: string[] = []; @@ -58,6 +68,30 @@ function dedupeDiagnostics(entries: readonly string[]): string[] { return ordered; } +function targetsRuntimeWebPath(path: string): boolean { + return WEB_RUNTIME_SECRET_PATH_PREFIXES.some((prefix) => path.startsWith(prefix)); +} + +function targetsRuntimeWebResolution(params: { + targetIds: ReadonlySet; + allowedPaths?: ReadonlySet; +}): boolean { + if (params.allowedPaths) { + for (const path of params.allowedPaths) { + if (targetsRuntimeWebPath(path)) { + return true; + } + } + return false; + } + for (const targetId of params.targetIds) { + if (WEB_RUNTIME_SECRET_TARGET_ID_PREFIXES.some((prefix) => targetId.startsWith(prefix))) { + return true; + } + } + return false; +} + function collectConfiguredTargetRefPaths(params: { config: OpenClawConfig; targetIds: Set; @@ -193,17 +227,40 @@ async function resolveCommandSecretRefsLocally(params: { sourceConfig, env: process.env, }); + const localResolutionDiagnostics: string[] = []; collectConfigAssignments({ config: structuredClone(params.config), context, }); + if ( + targetsRuntimeWebResolution({ targetIds: params.targetIds, allowedPaths: params.allowedPaths }) + ) { + try { + await resolveRuntimeWebTools({ + sourceConfig, + resolvedConfig, + context, + }); + } catch (error) { + if (params.mode === "strict") { + throw error; + } + localResolutionDiagnostics.push( + `${params.commandName}: failed to resolve web tool secrets locally (${describeUnknownError(error)}).`, + ); + } + } const inactiveRefPaths = new Set( context.warnings .filter((warning) => warning.code === "SECRETS_REF_IGNORED_INACTIVE_SURFACE") + .filter((warning) => !params.allowedPaths || params.allowedPaths.has(warning.path)) .map((warning) => warning.path), ); + const inactiveWarningDiagnostics = context.warnings + .filter((warning) => warning.code === "SECRETS_REF_IGNORED_INACTIVE_SURFACE") + .filter((warning) => !params.allowedPaths || params.allowedPaths.has(warning.path)) + .map((warning) => warning.message); const activePaths = new Set(context.assignments.map((assignment) => assignment.path)); - const localResolutionDiagnostics: string[] = []; for (const target of discoverConfigSecretTargetsByIds(sourceConfig, params.targetIds)) { if (params.allowedPaths && !params.allowedPaths.has(target.path)) { continue; @@ -244,6 +301,7 @@ async function resolveCommandSecretRefsLocally(params: { resolvedConfig, diagnostics: dedupeDiagnostics([ ...params.preflightDiagnostics, + ...inactiveWarningDiagnostics, ...filterInactiveSurfaceDiagnostics({ diagnostics: analyzed.diagnostics, inactiveRefPaths, diff --git a/src/cli/command-secret-targets.test.ts b/src/cli/command-secret-targets.test.ts index 3a7de543a02..a71ac5e00c4 100644 --- a/src/cli/command-secret-targets.test.ts +++ b/src/cli/command-secret-targets.test.ts @@ -9,6 +9,7 @@ describe("command secret target ids", () => { const ids = getAgentRuntimeCommandSecretTargetIds(); expect(ids.has("agents.defaults.memorySearch.remote.apiKey")).toBe(true); expect(ids.has("agents.list[].memorySearch.remote.apiKey")).toBe(true); + expect(ids.has("tools.web.fetch.firecrawl.apiKey")).toBe(true); }); it("keeps memory command target set focused on memorySearch remote credentials", () => { diff --git a/src/cli/command-secret-targets.ts b/src/cli/command-secret-targets.ts index c4a4fb5ea4a..e1c2c49e0ae 100644 --- a/src/cli/command-secret-targets.ts +++ b/src/cli/command-secret-targets.ts @@ -23,6 +23,7 @@ const COMMAND_SECRET_TARGETS = { "skills.entries.", "messages.tts.", "tools.web.search", + "tools.web.fetch.firecrawl.", ]), status: idsByPrefix([ "channels.", diff --git a/src/config/types.tools.ts b/src/config/types.tools.ts index 89775758411..e352f858c39 100644 --- a/src/config/types.tools.ts +++ b/src/config/types.tools.ts @@ -512,7 +512,7 @@ export type ToolsConfig = { /** Enable Firecrawl fallback (default: true when apiKey is set). */ enabled?: boolean; /** Firecrawl API key (optional; defaults to FIRECRAWL_API_KEY env var). */ - apiKey?: string; + apiKey?: SecretInput; /** Firecrawl base URL (default: https://api.firecrawl.dev). */ baseUrl?: string; /** Whether to keep only main content (default: true). */ diff --git a/src/gateway/server.reload.test.ts b/src/gateway/server.reload.test.ts index e691256d70f..b3a603fa287 100644 --- a/src/gateway/server.reload.test.ts +++ b/src/gateway/server.reload.test.ts @@ -175,12 +175,14 @@ describe("gateway hot reload", () => { let prevSkipGmail: string | undefined; let prevSkipProviders: string | undefined; let prevOpenAiApiKey: string | undefined; + let prevGeminiApiKey: string | undefined; beforeEach(() => { prevSkipChannels = process.env.OPENCLAW_SKIP_CHANNELS; prevSkipGmail = process.env.OPENCLAW_SKIP_GMAIL_WATCHER; prevSkipProviders = process.env.OPENCLAW_SKIP_PROVIDERS; prevOpenAiApiKey = process.env.OPENAI_API_KEY; + prevGeminiApiKey = process.env.GEMINI_API_KEY; process.env.OPENCLAW_SKIP_CHANNELS = "0"; delete process.env.OPENCLAW_SKIP_GMAIL_WATCHER; delete process.env.OPENCLAW_SKIP_PROVIDERS; @@ -207,6 +209,11 @@ describe("gateway hot reload", () => { } else { process.env.OPENAI_API_KEY = prevOpenAiApiKey; } + if (prevGeminiApiKey === undefined) { + delete process.env.GEMINI_API_KEY; + } else { + process.env.GEMINI_API_KEY = prevGeminiApiKey; + } }); async function writeEnvRefConfig() { @@ -328,6 +335,34 @@ describe("gateway hot reload", () => { ); } + async function writeWebSearchGeminiRefConfig() { + const configPath = process.env.OPENCLAW_CONFIG_PATH; + if (!configPath) { + throw new Error("OPENCLAW_CONFIG_PATH is not set"); + } + await fs.writeFile( + configPath, + `${JSON.stringify( + { + tools: { + web: { + search: { + enabled: true, + provider: "gemini", + gemini: { + apiKey: { source: "env", provider: "default", id: "GEMINI_API_KEY" }, + }, + }, + }, + }, + }, + null, + 2, + )}\n`, + "utf8", + ); + } + async function removeMainAuthProfileStore() { const stateDir = process.env.OPENCLAW_STATE_DIR; if (!stateDir) { @@ -540,6 +575,64 @@ describe("gateway hot reload", () => { }); }); + it("emits one-shot degraded and recovered system events for web search secret reload transitions", async () => { + await writeWebSearchGeminiRefConfig(); + process.env.GEMINI_API_KEY = "gemini-startup-key"; // pragma: allowlist secret + + await withGatewayServer(async () => { + const onHotReload = hoisted.getOnHotReload(); + expect(onHotReload).toBeTypeOf("function"); + const sessionKey = resolveMainSessionKeyFromConfig(); + const plan = { + changedPaths: ["tools.web.search.gemini.apiKey"], + restartGateway: false, + restartReasons: [], + hotReasons: ["tools.web.search.gemini.apiKey"], + reloadHooks: false, + restartGmailWatcher: false, + restartBrowserControl: false, + restartCron: false, + restartHeartbeat: false, + restartChannels: new Set(), + noopPaths: [], + }; + const nextConfig = { + tools: { + web: { + search: { + enabled: true, + provider: "gemini", + gemini: { + apiKey: { source: "env", provider: "default", id: "GEMINI_API_KEY" }, + }, + }, + }, + }, + }; + + delete process.env.GEMINI_API_KEY; + await expect(onHotReload?.(plan, nextConfig)).rejects.toThrow( + "[WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK]", + ); + const degradedEvents = drainSystemEvents(sessionKey); + expect(degradedEvents.some((event) => event.includes("[SECRETS_RELOADER_DEGRADED]"))).toBe( + true, + ); + + await expect(onHotReload?.(plan, nextConfig)).rejects.toThrow( + "[WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK]", + ); + expect(drainSystemEvents(sessionKey)).toEqual([]); + + process.env.GEMINI_API_KEY = "gemini-recovered-key"; // pragma: allowlist secret + await expect(onHotReload?.(plan, nextConfig)).resolves.toBeUndefined(); + const recoveredEvents = drainSystemEvents(sessionKey); + expect(recoveredEvents.some((event) => event.includes("[SECRETS_RELOADER_RECOVERED]"))).toBe( + true, + ); + }); + }); + it("serves secrets.reload immediately after startup without race failures", async () => { await writeEnvRefConfig(); process.env.OPENAI_API_KEY = "sk-startup"; // pragma: allowlist secret diff --git a/src/secrets/runtime-config-collectors-core.ts b/src/secrets/runtime-config-collectors-core.ts index 504331f0a96..99668371ad1 100644 --- a/src/secrets/runtime-config-collectors-core.ts +++ b/src/secrets/runtime-config-collectors-core.ts @@ -292,67 +292,6 @@ function collectMessagesTtsAssignments(params: { }); } -function collectToolsWebSearchAssignments(params: { - config: OpenClawConfig; - defaults: SecretDefaults | undefined; - context: ResolverContext; -}): void { - const tools = params.config.tools as Record | undefined; - if (!isRecord(tools) || !isRecord(tools.web) || !isRecord(tools.web.search)) { - return; - } - const search = tools.web.search; - const searchEnabled = search.enabled !== false; - const rawProvider = - typeof search.provider === "string" ? search.provider.trim().toLowerCase() : ""; - const selectedProvider = - rawProvider === "brave" || - rawProvider === "gemini" || - rawProvider === "grok" || - rawProvider === "kimi" || - rawProvider === "perplexity" - ? rawProvider - : undefined; - const paths = [ - "apiKey", - "gemini.apiKey", - "grok.apiKey", - "kimi.apiKey", - "perplexity.apiKey", - ] as const; - for (const path of paths) { - const [scope, field] = path.includes(".") ? path.split(".", 2) : [undefined, path]; - const target = scope ? search[scope] : search; - if (!isRecord(target)) { - continue; - } - const active = scope - ? searchEnabled && (selectedProvider === undefined || selectedProvider === scope) - : searchEnabled && (selectedProvider === undefined || selectedProvider === "brave"); - const inactiveReason = !searchEnabled - ? "tools.web.search is disabled." - : scope - ? selectedProvider === undefined - ? undefined - : `tools.web.search.provider is "${selectedProvider}".` - : selectedProvider === undefined - ? undefined - : `tools.web.search.provider is "${selectedProvider}".`; - collectSecretInputAssignment({ - value: target[field], - path: `tools.web.search.${path}`, - expected: "string", - defaults: params.defaults, - context: params.context, - active, - inactiveReason, - apply: (value) => { - target[field] = value; - }, - }); - } -} - function collectCronAssignments(params: { config: OpenClawConfig; defaults: SecretDefaults | undefined; @@ -401,6 +340,5 @@ export function collectCoreConfigAssignments(params: { collectTalkAssignments(params); collectGatewayAssignments(params); collectMessagesTtsAssignments(params); - collectToolsWebSearchAssignments(params); collectCronAssignments(params); } diff --git a/src/secrets/runtime-shared.ts b/src/secrets/runtime-shared.ts index 8374f642de8..77dcb3c051c 100644 --- a/src/secrets/runtime-shared.ts +++ b/src/secrets/runtime-shared.ts @@ -7,7 +7,12 @@ import { isRecord } from "./shared.js"; export type SecretResolverWarningCode = | "SECRETS_REF_OVERRIDES_PLAINTEXT" - | "SECRETS_REF_IGNORED_INACTIVE_SURFACE"; + | "SECRETS_REF_IGNORED_INACTIVE_SURFACE" + | "WEB_SEARCH_PROVIDER_INVALID_AUTODETECT" + | "WEB_SEARCH_KEY_UNRESOLVED_FALLBACK_USED" + | "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK" + | "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_FALLBACK_USED" + | "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_NO_FALLBACK"; export type SecretResolverWarning = { code: SecretResolverWarningCode; diff --git a/src/secrets/runtime-web-tools.test.ts b/src/secrets/runtime-web-tools.test.ts new file mode 100644 index 00000000000..b8c1e679ba6 --- /dev/null +++ b/src/secrets/runtime-web-tools.test.ts @@ -0,0 +1,451 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import * as secretResolve from "./resolve.js"; +import { createResolverContext } from "./runtime-shared.js"; +import { resolveRuntimeWebTools } from "./runtime-web-tools.js"; + +type ProviderUnderTest = "brave" | "gemini" | "grok" | "kimi" | "perplexity"; + +function asConfig(value: unknown): OpenClawConfig { + return value as OpenClawConfig; +} + +async function runRuntimeWebTools(params: { config: OpenClawConfig; env?: NodeJS.ProcessEnv }) { + const sourceConfig = structuredClone(params.config); + const resolvedConfig = structuredClone(params.config); + const context = createResolverContext({ + sourceConfig, + env: params.env ?? {}, + }); + const metadata = await resolveRuntimeWebTools({ + sourceConfig, + resolvedConfig, + context, + }); + return { metadata, resolvedConfig, context }; +} + +function createProviderSecretRefConfig( + provider: ProviderUnderTest, + envRefId: string, +): OpenClawConfig { + const search: Record = { + enabled: true, + provider, + }; + if (provider === "brave") { + search.apiKey = { source: "env", provider: "default", id: envRefId }; + } else { + search[provider] = { + apiKey: { source: "env", provider: "default", id: envRefId }, + }; + } + return asConfig({ + tools: { + web: { + search, + }, + }, + }); +} + +function readProviderKey(config: OpenClawConfig, provider: ProviderUnderTest): unknown { + if (provider === "brave") { + return config.tools?.web?.search?.apiKey; + } + if (provider === "gemini") { + return config.tools?.web?.search?.gemini?.apiKey; + } + if (provider === "grok") { + return config.tools?.web?.search?.grok?.apiKey; + } + if (provider === "kimi") { + return config.tools?.web?.search?.kimi?.apiKey; + } + return config.tools?.web?.search?.perplexity?.apiKey; +} + +describe("runtime web tools resolution", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it.each([ + { + provider: "brave" as const, + envRefId: "BRAVE_PROVIDER_REF", + resolvedKey: "brave-provider-key", + }, + { + provider: "gemini" as const, + envRefId: "GEMINI_PROVIDER_REF", + resolvedKey: "gemini-provider-key", + }, + { + provider: "grok" as const, + envRefId: "GROK_PROVIDER_REF", + resolvedKey: "grok-provider-key", + }, + { + provider: "kimi" as const, + envRefId: "KIMI_PROVIDER_REF", + resolvedKey: "kimi-provider-key", + }, + { + provider: "perplexity" as const, + envRefId: "PERPLEXITY_PROVIDER_REF", + resolvedKey: "pplx-provider-key", + }, + ])( + "resolves configured provider SecretRef for $provider", + async ({ provider, envRefId, resolvedKey }) => { + const { metadata, resolvedConfig, context } = await runRuntimeWebTools({ + config: createProviderSecretRefConfig(provider, envRefId), + env: { + [envRefId]: resolvedKey, + }, + }); + + expect(metadata.search.providerConfigured).toBe(provider); + expect(metadata.search.providerSource).toBe("configured"); + expect(metadata.search.selectedProvider).toBe(provider); + expect(metadata.search.selectedProviderKeySource).toBe("secretRef"); + expect(readProviderKey(resolvedConfig, provider)).toBe(resolvedKey); + expect(context.warnings.map((warning) => warning.code)).not.toContain( + "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + ); + if (provider === "perplexity") { + expect(metadata.search.perplexityTransport).toBe("search_api"); + } + }, + ); + + it("auto-detects provider precedence across all configured providers", async () => { + const { metadata, resolvedConfig, context } = await runRuntimeWebTools({ + config: asConfig({ + tools: { + web: { + search: { + apiKey: { source: "env", provider: "default", id: "BRAVE_REF" }, + gemini: { + apiKey: { source: "env", provider: "default", id: "GEMINI_REF" }, + }, + grok: { + apiKey: { source: "env", provider: "default", id: "GROK_REF" }, + }, + kimi: { + apiKey: { source: "env", provider: "default", id: "KIMI_REF" }, + }, + perplexity: { + apiKey: { source: "env", provider: "default", id: "PERPLEXITY_REF" }, + }, + }, + }, + }, + }), + env: { + BRAVE_REF: "brave-precedence-key", + GEMINI_REF: "gemini-precedence-key", + GROK_REF: "grok-precedence-key", + KIMI_REF: "kimi-precedence-key", + PERPLEXITY_REF: "pplx-precedence-key", + }, + }); + + expect(metadata.search.providerSource).toBe("auto-detect"); + expect(metadata.search.selectedProvider).toBe("brave"); + expect(resolvedConfig.tools?.web?.search?.apiKey).toBe("brave-precedence-key"); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ path: "tools.web.search.gemini.apiKey" }), + expect.objectContaining({ path: "tools.web.search.grok.apiKey" }), + expect.objectContaining({ path: "tools.web.search.kimi.apiKey" }), + expect.objectContaining({ path: "tools.web.search.perplexity.apiKey" }), + ]), + ); + }); + + it("auto-detects first available provider and keeps lower-priority refs inactive", async () => { + const { metadata, resolvedConfig, context } = await runRuntimeWebTools({ + config: asConfig({ + tools: { + web: { + search: { + apiKey: { source: "env", provider: "default", id: "BRAVE_API_KEY_REF" }, + gemini: { + apiKey: { + source: "env", + provider: "default", + id: "MISSING_GEMINI_API_KEY_REF", + }, + }, + }, + }, + }, + }), + env: { + BRAVE_API_KEY_REF: "brave-runtime-key", + }, + }); + + expect(metadata.search.providerSource).toBe("auto-detect"); + expect(metadata.search.selectedProvider).toBe("brave"); + expect(metadata.search.selectedProviderKeySource).toBe("secretRef"); + expect(resolvedConfig.tools?.web?.search?.apiKey).toBe("brave-runtime-key"); + expect(resolvedConfig.tools?.web?.search?.gemini?.apiKey).toEqual({ + source: "env", + provider: "default", + id: "MISSING_GEMINI_API_KEY_REF", + }); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "SECRETS_REF_IGNORED_INACTIVE_SURFACE", + path: "tools.web.search.gemini.apiKey", + }), + ]), + ); + expect(context.warnings.map((warning) => warning.code)).not.toContain( + "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + ); + }); + + it("auto-detects the next provider when a higher-priority ref is unresolved", async () => { + const { metadata, resolvedConfig, context } = await runRuntimeWebTools({ + config: asConfig({ + tools: { + web: { + search: { + apiKey: { source: "env", provider: "default", id: "MISSING_BRAVE_API_KEY_REF" }, + gemini: { + apiKey: { source: "env", provider: "default", id: "GEMINI_API_KEY_REF" }, + }, + }, + }, + }, + }), + env: { + GEMINI_API_KEY_REF: "gemini-runtime-key", + }, + }); + + expect(metadata.search.providerSource).toBe("auto-detect"); + expect(metadata.search.selectedProvider).toBe("gemini"); + expect(resolvedConfig.tools?.web?.search?.gemini?.apiKey).toBe("gemini-runtime-key"); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "SECRETS_REF_IGNORED_INACTIVE_SURFACE", + path: "tools.web.search.apiKey", + }), + ]), + ); + expect(context.warnings.map((warning) => warning.code)).not.toContain( + "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + ); + }); + + it("warns when provider is invalid and falls back to auto-detect", async () => { + const { metadata, resolvedConfig, context } = await runRuntimeWebTools({ + config: asConfig({ + tools: { + web: { + search: { + provider: "invalid-provider", + gemini: { + apiKey: { source: "env", provider: "default", id: "GEMINI_API_KEY_REF" }, + }, + }, + }, + }, + }), + env: { + GEMINI_API_KEY_REF: "gemini-runtime-key", + }, + }); + + expect(metadata.search.providerConfigured).toBeUndefined(); + expect(metadata.search.providerSource).toBe("auto-detect"); + expect(metadata.search.selectedProvider).toBe("gemini"); + expect(resolvedConfig.tools?.web?.search?.gemini?.apiKey).toBe("gemini-runtime-key"); + expect(metadata.search.diagnostics).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "WEB_SEARCH_PROVIDER_INVALID_AUTODETECT", + path: "tools.web.search.provider", + }), + ]), + ); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "WEB_SEARCH_PROVIDER_INVALID_AUTODETECT", + path: "tools.web.search.provider", + }), + ]), + ); + }); + + it("fails fast when configured provider ref is unresolved with no fallback", async () => { + const sourceConfig = asConfig({ + tools: { + web: { + search: { + provider: "gemini", + gemini: { + apiKey: { source: "env", provider: "default", id: "MISSING_GEMINI_API_KEY_REF" }, + }, + }, + }, + }, + }); + const resolvedConfig = structuredClone(sourceConfig); + const context = createResolverContext({ + sourceConfig, + env: {}, + }); + + await expect( + resolveRuntimeWebTools({ + sourceConfig, + resolvedConfig, + context, + }), + ).rejects.toThrow("[WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK]"); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + path: "tools.web.search.gemini.apiKey", + }), + ]), + ); + }); + + it("does not resolve Firecrawl SecretRef when Firecrawl is inactive", async () => { + const resolveSpy = vi.spyOn(secretResolve, "resolveSecretRefValues"); + const { metadata, context } = await runRuntimeWebTools({ + config: asConfig({ + tools: { + web: { + fetch: { + enabled: false, + firecrawl: { + apiKey: { source: "env", provider: "default", id: "MISSING_FIRECRAWL_REF" }, + }, + }, + }, + }, + }), + }); + + expect(resolveSpy).not.toHaveBeenCalled(); + expect(metadata.fetch.firecrawl.active).toBe(false); + expect(metadata.fetch.firecrawl.apiKeySource).toBe("secretRef"); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "SECRETS_REF_IGNORED_INACTIVE_SURFACE", + path: "tools.web.fetch.firecrawl.apiKey", + }), + ]), + ); + }); + + it("does not resolve Firecrawl SecretRef when Firecrawl is disabled", async () => { + const resolveSpy = vi.spyOn(secretResolve, "resolveSecretRefValues"); + const { metadata, context } = await runRuntimeWebTools({ + config: asConfig({ + tools: { + web: { + fetch: { + enabled: true, + firecrawl: { + enabled: false, + apiKey: { source: "env", provider: "default", id: "MISSING_FIRECRAWL_REF" }, + }, + }, + }, + }, + }), + }); + + expect(resolveSpy).not.toHaveBeenCalled(); + expect(metadata.fetch.firecrawl.active).toBe(false); + expect(metadata.fetch.firecrawl.apiKeySource).toBe("secretRef"); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "SECRETS_REF_IGNORED_INACTIVE_SURFACE", + path: "tools.web.fetch.firecrawl.apiKey", + }), + ]), + ); + }); + + it("uses env fallback for unresolved Firecrawl SecretRef when active", async () => { + const { metadata, resolvedConfig, context } = await runRuntimeWebTools({ + config: asConfig({ + tools: { + web: { + fetch: { + firecrawl: { + apiKey: { source: "env", provider: "default", id: "MISSING_FIRECRAWL_REF" }, + }, + }, + }, + }, + }), + env: { + FIRECRAWL_API_KEY: "firecrawl-fallback-key", + }, + }); + + expect(metadata.fetch.firecrawl.active).toBe(true); + expect(metadata.fetch.firecrawl.apiKeySource).toBe("env"); + expect(resolvedConfig.tools?.web?.fetch?.firecrawl?.apiKey).toBe("firecrawl-fallback-key"); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_FALLBACK_USED", + path: "tools.web.fetch.firecrawl.apiKey", + }), + ]), + ); + }); + + it("fails fast when active Firecrawl SecretRef is unresolved with no fallback", async () => { + const sourceConfig = asConfig({ + tools: { + web: { + fetch: { + firecrawl: { + apiKey: { source: "env", provider: "default", id: "MISSING_FIRECRAWL_REF" }, + }, + }, + }, + }, + }); + const resolvedConfig = structuredClone(sourceConfig); + const context = createResolverContext({ + sourceConfig, + env: {}, + }); + + await expect( + resolveRuntimeWebTools({ + sourceConfig, + resolvedConfig, + context, + }), + ).rejects.toThrow("[WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_NO_FALLBACK]"); + expect(context.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_NO_FALLBACK", + path: "tools.web.fetch.firecrawl.apiKey", + }), + ]), + ); + }); +}); diff --git a/src/secrets/runtime-web-tools.ts b/src/secrets/runtime-web-tools.ts new file mode 100644 index 00000000000..004af2bdfe2 --- /dev/null +++ b/src/secrets/runtime-web-tools.ts @@ -0,0 +1,705 @@ +import type { OpenClawConfig } from "../config/config.js"; +import { resolveSecretInputRef } from "../config/types.secrets.js"; +import { normalizeSecretInput } from "../utils/normalize-secret-input.js"; +import { secretRefKey } from "./ref-contract.js"; +import { resolveSecretRefValues } from "./resolve.js"; +import { + pushInactiveSurfaceWarning, + pushWarning, + type ResolverContext, + type SecretDefaults, +} from "./runtime-shared.js"; + +const WEB_SEARCH_PROVIDERS = ["brave", "gemini", "grok", "kimi", "perplexity"] as const; +const PERPLEXITY_DIRECT_BASE_URL = "https://api.perplexity.ai"; +const DEFAULT_PERPLEXITY_BASE_URL = "https://openrouter.ai/api/v1"; +const PERPLEXITY_KEY_PREFIXES = ["pplx-"]; +const OPENROUTER_KEY_PREFIXES = ["sk-or-"]; + +type WebSearchProvider = (typeof WEB_SEARCH_PROVIDERS)[number]; + +type SecretResolutionSource = "config" | "secretRef" | "env" | "missing"; +type RuntimeWebProviderSource = "configured" | "auto-detect" | "none"; + +export type RuntimeWebDiagnosticCode = + | "WEB_SEARCH_PROVIDER_INVALID_AUTODETECT" + | "WEB_SEARCH_AUTODETECT_SELECTED" + | "WEB_SEARCH_KEY_UNRESOLVED_FALLBACK_USED" + | "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK" + | "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_FALLBACK_USED" + | "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_NO_FALLBACK"; + +export type RuntimeWebDiagnostic = { + code: RuntimeWebDiagnosticCode; + message: string; + path?: string; +}; + +export type RuntimeWebSearchMetadata = { + providerConfigured?: WebSearchProvider; + providerSource: RuntimeWebProviderSource; + selectedProvider?: WebSearchProvider; + selectedProviderKeySource?: SecretResolutionSource; + perplexityTransport?: "search_api" | "chat_completions"; + diagnostics: RuntimeWebDiagnostic[]; +}; + +export type RuntimeWebFetchFirecrawlMetadata = { + active: boolean; + apiKeySource: SecretResolutionSource; + diagnostics: RuntimeWebDiagnostic[]; +}; + +export type RuntimeWebToolsMetadata = { + search: RuntimeWebSearchMetadata; + fetch: { + firecrawl: RuntimeWebFetchFirecrawlMetadata; + }; + diagnostics: RuntimeWebDiagnostic[]; +}; + +type FetchConfig = NonNullable["web"] extends infer Web + ? Web extends { fetch?: infer Fetch } + ? Fetch + : undefined + : undefined; + +type SecretResolutionResult = { + value?: string; + source: SecretResolutionSource; + secretRefConfigured: boolean; + unresolvedRefReason?: string; + fallbackEnvVar?: string; + fallbackUsedAfterRefFailure: boolean; +}; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function normalizeProvider(value: unknown): WebSearchProvider | undefined { + if (typeof value !== "string") { + return undefined; + } + const normalized = value.trim().toLowerCase(); + if ( + normalized === "brave" || + normalized === "gemini" || + normalized === "grok" || + normalized === "kimi" || + normalized === "perplexity" + ) { + return normalized; + } + return undefined; +} + +function readNonEmptyEnvValue( + env: NodeJS.ProcessEnv, + names: string[], +): { value?: string; envVar?: string } { + for (const envVar of names) { + const value = normalizeSecretInput(env[envVar]); + if (value) { + return { value, envVar }; + } + } + return {}; +} + +function buildUnresolvedReason(params: { + path: string; + kind: "unresolved" | "non-string" | "empty"; + refLabel: string; +}): string { + if (params.kind === "non-string") { + return `${params.path} SecretRef resolved to a non-string value.`; + } + if (params.kind === "empty") { + return `${params.path} SecretRef resolved to an empty value.`; + } + return `${params.path} SecretRef is unresolved (${params.refLabel}).`; +} + +async function resolveSecretInputWithEnvFallback(params: { + sourceConfig: OpenClawConfig; + context: ResolverContext; + defaults: SecretDefaults | undefined; + value: unknown; + path: string; + envVars: string[]; +}): Promise { + const { ref } = resolveSecretInputRef({ + value: params.value, + defaults: params.defaults, + }); + + if (!ref) { + const configValue = normalizeSecretInput(params.value); + if (configValue) { + return { + value: configValue, + source: "config", + secretRefConfigured: false, + fallbackUsedAfterRefFailure: false, + }; + } + const fallback = readNonEmptyEnvValue(params.context.env, params.envVars); + if (fallback.value) { + return { + value: fallback.value, + source: "env", + fallbackEnvVar: fallback.envVar, + secretRefConfigured: false, + fallbackUsedAfterRefFailure: false, + }; + } + return { + source: "missing", + secretRefConfigured: false, + fallbackUsedAfterRefFailure: false, + }; + } + + const refLabel = `${ref.source}:${ref.provider}:${ref.id}`; + let resolvedFromRef: string | undefined; + let unresolvedRefReason: string | undefined; + + try { + const resolved = await resolveSecretRefValues([ref], { + config: params.sourceConfig, + env: params.context.env, + cache: params.context.cache, + }); + const resolvedValue = resolved.get(secretRefKey(ref)); + if (typeof resolvedValue !== "string") { + unresolvedRefReason = buildUnresolvedReason({ + path: params.path, + kind: "non-string", + refLabel, + }); + } else { + resolvedFromRef = normalizeSecretInput(resolvedValue); + if (!resolvedFromRef) { + unresolvedRefReason = buildUnresolvedReason({ + path: params.path, + kind: "empty", + refLabel, + }); + } + } + } catch { + unresolvedRefReason = buildUnresolvedReason({ + path: params.path, + kind: "unresolved", + refLabel, + }); + } + + if (resolvedFromRef) { + return { + value: resolvedFromRef, + source: "secretRef", + secretRefConfigured: true, + fallbackUsedAfterRefFailure: false, + }; + } + + const fallback = readNonEmptyEnvValue(params.context.env, params.envVars); + if (fallback.value) { + return { + value: fallback.value, + source: "env", + fallbackEnvVar: fallback.envVar, + unresolvedRefReason, + secretRefConfigured: true, + fallbackUsedAfterRefFailure: true, + }; + } + + return { + source: "missing", + unresolvedRefReason, + secretRefConfigured: true, + fallbackUsedAfterRefFailure: false, + }; +} + +function inferPerplexityBaseUrlFromApiKey(apiKey?: string): "direct" | "openrouter" | undefined { + if (!apiKey) { + return undefined; + } + const normalized = apiKey.toLowerCase(); + if (PERPLEXITY_KEY_PREFIXES.some((prefix) => normalized.startsWith(prefix))) { + return "direct"; + } + if (OPENROUTER_KEY_PREFIXES.some((prefix) => normalized.startsWith(prefix))) { + return "openrouter"; + } + return undefined; +} + +function resolvePerplexityRuntimeTransport(params: { + keyValue?: string; + keySource: SecretResolutionSource; + fallbackEnvVar?: string; + configValue: unknown; +}): "search_api" | "chat_completions" | undefined { + const config = isRecord(params.configValue) ? params.configValue : undefined; + const configuredBaseUrl = typeof config?.baseUrl === "string" ? config.baseUrl.trim() : ""; + const configuredModel = typeof config?.model === "string" ? config.model.trim() : ""; + + const baseUrl = (() => { + if (configuredBaseUrl) { + return configuredBaseUrl; + } + if (params.keySource === "env") { + if (params.fallbackEnvVar === "PERPLEXITY_API_KEY") { + return PERPLEXITY_DIRECT_BASE_URL; + } + if (params.fallbackEnvVar === "OPENROUTER_API_KEY") { + return DEFAULT_PERPLEXITY_BASE_URL; + } + } + if ((params.keySource === "config" || params.keySource === "secretRef") && params.keyValue) { + const inferred = inferPerplexityBaseUrlFromApiKey(params.keyValue); + return inferred === "openrouter" ? DEFAULT_PERPLEXITY_BASE_URL : PERPLEXITY_DIRECT_BASE_URL; + } + return DEFAULT_PERPLEXITY_BASE_URL; + })(); + + const hasLegacyOverride = Boolean(configuredBaseUrl || configuredModel); + const direct = (() => { + try { + return new URL(baseUrl).hostname.toLowerCase() === "api.perplexity.ai"; + } catch { + return false; + } + })(); + return hasLegacyOverride || !direct ? "chat_completions" : "search_api"; +} + +function ensureObject(target: Record, key: string): Record { + const current = target[key]; + if (isRecord(current)) { + return current; + } + const next: Record = {}; + target[key] = next; + return next; +} + +function setResolvedWebSearchApiKey(params: { + resolvedConfig: OpenClawConfig; + provider: WebSearchProvider; + value: string; +}): void { + const tools = ensureObject(params.resolvedConfig as Record, "tools"); + const web = ensureObject(tools, "web"); + const search = ensureObject(web, "search"); + if (params.provider === "brave") { + search.apiKey = params.value; + return; + } + const providerConfig = ensureObject(search, params.provider); + providerConfig.apiKey = params.value; +} + +function setResolvedFirecrawlApiKey(params: { + resolvedConfig: OpenClawConfig; + value: string; +}): void { + const tools = ensureObject(params.resolvedConfig as Record, "tools"); + const web = ensureObject(tools, "web"); + const fetch = ensureObject(web, "fetch"); + const firecrawl = ensureObject(fetch, "firecrawl"); + firecrawl.apiKey = params.value; +} + +function envVarsForProvider(provider: WebSearchProvider): string[] { + if (provider === "brave") { + return ["BRAVE_API_KEY"]; + } + if (provider === "gemini") { + return ["GEMINI_API_KEY"]; + } + if (provider === "grok") { + return ["XAI_API_KEY"]; + } + if (provider === "kimi") { + return ["KIMI_API_KEY", "MOONSHOT_API_KEY"]; + } + return ["PERPLEXITY_API_KEY", "OPENROUTER_API_KEY"]; +} + +function resolveProviderKeyValue( + search: Record, + provider: WebSearchProvider, +): unknown { + if (provider === "brave") { + return search.apiKey; + } + const scoped = search[provider]; + if (!isRecord(scoped)) { + return undefined; + } + return scoped.apiKey; +} + +function hasConfiguredSecretRef(value: unknown, defaults: SecretDefaults | undefined): boolean { + return Boolean( + resolveSecretInputRef({ + value, + defaults, + }).ref, + ); +} + +export async function resolveRuntimeWebTools(params: { + sourceConfig: OpenClawConfig; + resolvedConfig: OpenClawConfig; + context: ResolverContext; +}): Promise { + const defaults = params.sourceConfig.secrets?.defaults; + const diagnostics: RuntimeWebDiagnostic[] = []; + + const tools = isRecord(params.sourceConfig.tools) ? params.sourceConfig.tools : undefined; + const web = isRecord(tools?.web) ? tools.web : undefined; + const search = isRecord(web?.search) ? web.search : undefined; + + const searchMetadata: RuntimeWebSearchMetadata = { + providerSource: "none", + diagnostics: [], + }; + + const searchEnabled = search?.enabled !== false; + const rawProvider = + typeof search?.provider === "string" ? search.provider.trim().toLowerCase() : ""; + const configuredProvider = normalizeProvider(rawProvider); + + if (rawProvider && !configuredProvider) { + const diagnostic: RuntimeWebDiagnostic = { + code: "WEB_SEARCH_PROVIDER_INVALID_AUTODETECT", + message: `tools.web.search.provider is "${rawProvider}". Falling back to auto-detect precedence.`, + path: "tools.web.search.provider", + }; + diagnostics.push(diagnostic); + searchMetadata.diagnostics.push(diagnostic); + pushWarning(params.context, { + code: "WEB_SEARCH_PROVIDER_INVALID_AUTODETECT", + path: "tools.web.search.provider", + message: diagnostic.message, + }); + } + + if (configuredProvider) { + searchMetadata.providerConfigured = configuredProvider; + searchMetadata.providerSource = "configured"; + } + + if (searchEnabled && search) { + const candidates = configuredProvider ? [configuredProvider] : [...WEB_SEARCH_PROVIDERS]; + const unresolvedWithoutFallback: Array<{ + provider: WebSearchProvider; + path: string; + reason: string; + }> = []; + + let selectedProvider: WebSearchProvider | undefined; + let selectedResolution: SecretResolutionResult | undefined; + + for (const provider of candidates) { + const path = + provider === "brave" ? "tools.web.search.apiKey" : `tools.web.search.${provider}.apiKey`; + const value = resolveProviderKeyValue(search, provider); + const resolution = await resolveSecretInputWithEnvFallback({ + sourceConfig: params.sourceConfig, + context: params.context, + defaults, + value, + path, + envVars: envVarsForProvider(provider), + }); + + if (resolution.secretRefConfigured && resolution.fallbackUsedAfterRefFailure) { + const diagnostic: RuntimeWebDiagnostic = { + code: "WEB_SEARCH_KEY_UNRESOLVED_FALLBACK_USED", + message: + `${path} SecretRef could not be resolved; using ${resolution.fallbackEnvVar ?? "env fallback"}. ` + + (resolution.unresolvedRefReason ?? "").trim(), + path, + }; + diagnostics.push(diagnostic); + searchMetadata.diagnostics.push(diagnostic); + pushWarning(params.context, { + code: "WEB_SEARCH_KEY_UNRESOLVED_FALLBACK_USED", + path, + message: diagnostic.message, + }); + } + + if (resolution.secretRefConfigured && !resolution.value && resolution.unresolvedRefReason) { + unresolvedWithoutFallback.push({ + provider, + path, + reason: resolution.unresolvedRefReason, + }); + } + + if (configuredProvider) { + selectedProvider = provider; + selectedResolution = resolution; + if (resolution.value) { + setResolvedWebSearchApiKey({ + resolvedConfig: params.resolvedConfig, + provider, + value: resolution.value, + }); + } + break; + } + + if (resolution.value) { + selectedProvider = provider; + selectedResolution = resolution; + setResolvedWebSearchApiKey({ + resolvedConfig: params.resolvedConfig, + provider, + value: resolution.value, + }); + break; + } + } + + if (configuredProvider) { + const unresolved = unresolvedWithoutFallback[0]; + if (unresolved) { + const diagnostic: RuntimeWebDiagnostic = { + code: "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + message: unresolved.reason, + path: unresolved.path, + }; + diagnostics.push(diagnostic); + searchMetadata.diagnostics.push(diagnostic); + pushWarning(params.context, { + code: "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + path: unresolved.path, + message: unresolved.reason, + }); + throw new Error(`[WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK] ${unresolved.reason}`); + } + } else { + if (!selectedProvider && unresolvedWithoutFallback.length > 0) { + const unresolved = unresolvedWithoutFallback[0]; + const diagnostic: RuntimeWebDiagnostic = { + code: "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + message: unresolved.reason, + path: unresolved.path, + }; + diagnostics.push(diagnostic); + searchMetadata.diagnostics.push(diagnostic); + pushWarning(params.context, { + code: "WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK", + path: unresolved.path, + message: unresolved.reason, + }); + throw new Error(`[WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK] ${unresolved.reason}`); + } + + if (selectedProvider) { + const diagnostic: RuntimeWebDiagnostic = { + code: "WEB_SEARCH_AUTODETECT_SELECTED", + message: `tools.web.search auto-detected provider "${selectedProvider}" from available credentials.`, + path: "tools.web.search.provider", + }; + diagnostics.push(diagnostic); + searchMetadata.diagnostics.push(diagnostic); + } + } + + if (selectedProvider) { + searchMetadata.selectedProvider = selectedProvider; + searchMetadata.selectedProviderKeySource = selectedResolution?.source; + if (!configuredProvider) { + searchMetadata.providerSource = "auto-detect"; + } + if (selectedProvider === "perplexity") { + searchMetadata.perplexityTransport = resolvePerplexityRuntimeTransport({ + keyValue: selectedResolution?.value, + keySource: selectedResolution?.source ?? "missing", + fallbackEnvVar: selectedResolution?.fallbackEnvVar, + configValue: search.perplexity, + }); + } + } + } + + if (searchEnabled && search && !configuredProvider && searchMetadata.selectedProvider) { + for (const provider of WEB_SEARCH_PROVIDERS) { + if (provider === searchMetadata.selectedProvider) { + continue; + } + const path = + provider === "brave" ? "tools.web.search.apiKey" : `tools.web.search.${provider}.apiKey`; + const value = resolveProviderKeyValue(search, provider); + if (!hasConfiguredSecretRef(value, defaults)) { + continue; + } + pushInactiveSurfaceWarning({ + context: params.context, + path, + details: `tools.web.search auto-detected provider is "${searchMetadata.selectedProvider}".`, + }); + } + } else if (search && !searchEnabled) { + for (const provider of WEB_SEARCH_PROVIDERS) { + const path = + provider === "brave" ? "tools.web.search.apiKey" : `tools.web.search.${provider}.apiKey`; + const value = resolveProviderKeyValue(search, provider); + if (!hasConfiguredSecretRef(value, defaults)) { + continue; + } + pushInactiveSurfaceWarning({ + context: params.context, + path, + details: "tools.web.search is disabled.", + }); + } + } + + if (searchEnabled && search && configuredProvider) { + for (const provider of WEB_SEARCH_PROVIDERS) { + if (provider === configuredProvider) { + continue; + } + const path = + provider === "brave" ? "tools.web.search.apiKey" : `tools.web.search.${provider}.apiKey`; + const value = resolveProviderKeyValue(search, provider); + if (!hasConfiguredSecretRef(value, defaults)) { + continue; + } + pushInactiveSurfaceWarning({ + context: params.context, + path, + details: `tools.web.search.provider is "${configuredProvider}".`, + }); + } + } + + const fetch = isRecord(web?.fetch) ? (web.fetch as FetchConfig) : undefined; + const firecrawl = isRecord(fetch?.firecrawl) ? fetch.firecrawl : undefined; + const fetchEnabled = fetch?.enabled !== false; + const firecrawlEnabled = firecrawl?.enabled !== false; + const firecrawlActive = Boolean(fetchEnabled && firecrawlEnabled); + const firecrawlPath = "tools.web.fetch.firecrawl.apiKey"; + let firecrawlResolution: SecretResolutionResult = { + source: "missing", + secretRefConfigured: false, + fallbackUsedAfterRefFailure: false, + }; + + const firecrawlDiagnostics: RuntimeWebDiagnostic[] = []; + + if (firecrawlActive) { + firecrawlResolution = await resolveSecretInputWithEnvFallback({ + sourceConfig: params.sourceConfig, + context: params.context, + defaults, + value: firecrawl?.apiKey, + path: firecrawlPath, + envVars: ["FIRECRAWL_API_KEY"], + }); + + if (firecrawlResolution.value) { + setResolvedFirecrawlApiKey({ + resolvedConfig: params.resolvedConfig, + value: firecrawlResolution.value, + }); + } + + if (firecrawlResolution.secretRefConfigured) { + if (firecrawlResolution.fallbackUsedAfterRefFailure) { + const diagnostic: RuntimeWebDiagnostic = { + code: "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_FALLBACK_USED", + message: + `${firecrawlPath} SecretRef could not be resolved; using ${firecrawlResolution.fallbackEnvVar ?? "env fallback"}. ` + + (firecrawlResolution.unresolvedRefReason ?? "").trim(), + path: firecrawlPath, + }; + diagnostics.push(diagnostic); + firecrawlDiagnostics.push(diagnostic); + pushWarning(params.context, { + code: "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_FALLBACK_USED", + path: firecrawlPath, + message: diagnostic.message, + }); + } + + if (!firecrawlResolution.value && firecrawlResolution.unresolvedRefReason) { + const diagnostic: RuntimeWebDiagnostic = { + code: "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_NO_FALLBACK", + message: firecrawlResolution.unresolvedRefReason, + path: firecrawlPath, + }; + diagnostics.push(diagnostic); + firecrawlDiagnostics.push(diagnostic); + pushWarning(params.context, { + code: "WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_NO_FALLBACK", + path: firecrawlPath, + message: firecrawlResolution.unresolvedRefReason, + }); + throw new Error( + `[WEB_FETCH_FIRECRAWL_KEY_UNRESOLVED_NO_FALLBACK] ${firecrawlResolution.unresolvedRefReason}`, + ); + } + } + } else { + if (hasConfiguredSecretRef(firecrawl?.apiKey, defaults)) { + pushInactiveSurfaceWarning({ + context: params.context, + path: firecrawlPath, + details: !fetchEnabled + ? "tools.web.fetch is disabled." + : "tools.web.fetch.firecrawl.enabled is false.", + }); + firecrawlResolution = { + source: "secretRef", + secretRefConfigured: true, + fallbackUsedAfterRefFailure: false, + }; + } else { + const configuredInlineValue = normalizeSecretInput(firecrawl?.apiKey); + if (configuredInlineValue) { + firecrawlResolution = { + value: configuredInlineValue, + source: "config", + secretRefConfigured: false, + fallbackUsedAfterRefFailure: false, + }; + } else { + const envFallback = readNonEmptyEnvValue(params.context.env, ["FIRECRAWL_API_KEY"]); + if (envFallback.value) { + firecrawlResolution = { + value: envFallback.value, + source: "env", + fallbackEnvVar: envFallback.envVar, + secretRefConfigured: false, + fallbackUsedAfterRefFailure: false, + }; + } + } + } + } + + return { + search: searchMetadata, + fetch: { + firecrawl: { + active: firecrawlActive, + apiKeySource: firecrawlResolution.source, + diagnostics: firecrawlDiagnostics, + }, + }, + diagnostics, + }; +} diff --git a/src/secrets/runtime.test.ts b/src/secrets/runtime.test.ts index 463914bf899..f03ce73da3e 100644 --- a/src/secrets/runtime.test.ts +++ b/src/secrets/runtime.test.ts @@ -8,6 +8,7 @@ import { withTempHome } from "../config/home-env.test-harness.js"; import { activateSecretsRuntimeSnapshot, clearSecretsRuntimeSnapshot, + getActiveRuntimeWebToolsMetadata, getActiveSecretsRuntimeSnapshot, prepareSecretsRuntimeSnapshot, } from "./runtime.js"; @@ -342,7 +343,7 @@ describe("secrets runtime snapshot", () => { ); }); - it("resolves provider-specific refs in web search auto mode", async () => { + it("keeps non-selected provider refs inactive in web search auto mode", async () => { const snapshot = await prepareSecretsRuntimeSnapshot({ config: asConfig({ tools: { @@ -366,9 +367,19 @@ describe("secrets runtime snapshot", () => { }); expect(snapshot.config.tools?.web?.search?.apiKey).toBe("web-search-ref"); - expect(snapshot.config.tools?.web?.search?.gemini?.apiKey).toBe("web-search-gemini-ref"); - expect(snapshot.warnings.map((warning) => warning.path)).not.toContain( - "tools.web.search.gemini.apiKey", + expect(snapshot.config.tools?.web?.search?.gemini?.apiKey).toEqual({ + source: "env", + provider: "default", + id: "WEB_SEARCH_GEMINI_API_KEY", + }); + expect(snapshot.webTools.search.selectedProvider).toBe("brave"); + expect(snapshot.warnings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + code: "SECRETS_REF_IGNORED_INACTIVE_SURFACE", + path: "tools.web.search.gemini.apiKey", + }), + ]), ); }); @@ -401,6 +412,71 @@ describe("secrets runtime snapshot", () => { ); }); + it("fails fast at startup when selected web search provider ref is unresolved", async () => { + await expect( + prepareSecretsRuntimeSnapshot({ + config: asConfig({ + tools: { + web: { + search: { + enabled: true, + provider: "gemini", + gemini: { + apiKey: { + source: "env", + provider: "default", + id: "MISSING_WEB_SEARCH_GEMINI_API_KEY", + }, + }, + }, + }, + }, + }), + env: {}, + agentDirs: ["/tmp/openclaw-agent-main"], + loadAuthStore: () => ({ version: 1, profiles: {} }), + }), + ).rejects.toThrow("[WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK]"); + }); + + it("exposes active runtime web tool metadata as a defensive clone", async () => { + const snapshot = await prepareSecretsRuntimeSnapshot({ + config: asConfig({ + tools: { + web: { + search: { + provider: "gemini", + gemini: { + apiKey: { source: "env", provider: "default", id: "WEB_SEARCH_GEMINI_API_KEY" }, + }, + }, + }, + }, + }), + env: { + WEB_SEARCH_GEMINI_API_KEY: "web-search-gemini-ref", // pragma: allowlist secret + }, + agentDirs: ["/tmp/openclaw-agent-main"], + loadAuthStore: () => ({ version: 1, profiles: {} }), + }); + + activateSecretsRuntimeSnapshot(snapshot); + + const first = getActiveRuntimeWebToolsMetadata(); + expect(first?.search.providerConfigured).toBe("gemini"); + expect(first?.search.selectedProvider).toBe("gemini"); + expect(first?.search.selectedProviderKeySource).toBe("secretRef"); + if (!first) { + throw new Error("missing runtime web tools metadata"); + } + first.search.providerConfigured = "brave"; + first.search.selectedProvider = "brave"; + + const second = getActiveRuntimeWebToolsMetadata(); + expect(second?.search.providerConfigured).toBe("gemini"); + expect(second?.search.selectedProvider).toBe("gemini"); + }); + it("resolves file refs via configured file provider", async () => { if (process.platform === "win32") { return; @@ -615,7 +691,7 @@ describe("secrets runtime snapshot", () => { }); }); - it("clears active secrets runtime state and throws when refresh fails after a write", async () => { + it("keeps last-known-good runtime snapshot active when refresh fails after a write", async () => { if (os.platform() === "win32") { return; } @@ -704,9 +780,11 @@ describe("secrets runtime snapshot", () => { /runtime snapshot refresh failed: simulated secrets runtime refresh failure/i, ); - expect(getActiveSecretsRuntimeSnapshot()).toBeNull(); - expect(loadConfig().gateway?.auth).toEqual({ mode: "token" }); - expect(loadConfig().models?.providers?.openai?.apiKey).toEqual({ + const activeAfterFailure = getActiveSecretsRuntimeSnapshot(); + expect(activeAfterFailure).not.toBeNull(); + expect(loadConfig().gateway?.auth).toBeUndefined(); + expect(loadConfig().models?.providers?.openai?.apiKey).toBe("sk-file-runtime"); + expect(activeAfterFailure?.sourceConfig.models?.providers?.openai?.apiKey).toEqual({ source: "file", provider: "default", id: "/providers/openai/apiKey", @@ -715,9 +793,75 @@ describe("secrets runtime snapshot", () => { const persistedStore = ensureAuthProfileStore(agentDir).profiles["openai:default"]; expect(persistedStore).toMatchObject({ type: "api_key", - keyRef: { source: "file", provider: "default", id: "/providers/openai/apiKey" }, + key: "sk-file-runtime", + }); + }); + }); + + it("keeps last-known-good web runtime snapshot when reload introduces unresolved active web refs", async () => { + await withTempHome("openclaw-secrets-runtime-web-reload-lkg-", async (home) => { + const prepared = await prepareSecretsRuntimeSnapshot({ + config: asConfig({ + tools: { + web: { + search: { + provider: "gemini", + gemini: { + apiKey: { source: "env", provider: "default", id: "WEB_SEARCH_GEMINI_API_KEY" }, + }, + }, + }, + }, + }), + env: { + WEB_SEARCH_GEMINI_API_KEY: "web-search-gemini-runtime-key", // pragma: allowlist secret + }, + agentDirs: ["/tmp/openclaw-agent-main"], + loadAuthStore: () => ({ version: 1, profiles: {} }), + }); + + activateSecretsRuntimeSnapshot(prepared); + + await expect( + writeConfigFile({ + ...loadConfig(), + tools: { + web: { + search: { + provider: "gemini", + gemini: { + apiKey: { + source: "env", + provider: "default", + id: "MISSING_WEB_SEARCH_GEMINI_API_KEY", + }, + }, + }, + }, + }, + }), + ).rejects.toThrow( + /runtime snapshot refresh failed: .*WEB_SEARCH_KEY_UNRESOLVED_NO_FALLBACK/i, + ); + + const activeAfterFailure = getActiveSecretsRuntimeSnapshot(); + expect(activeAfterFailure).not.toBeNull(); + expect(loadConfig().tools?.web?.search?.gemini?.apiKey).toBe("web-search-gemini-runtime-key"); + expect(activeAfterFailure?.sourceConfig.tools?.web?.search?.gemini?.apiKey).toEqual({ + source: "env", + provider: "default", + id: "WEB_SEARCH_GEMINI_API_KEY", + }); + expect(getActiveRuntimeWebToolsMetadata()?.search.selectedProvider).toBe("gemini"); + + const persistedConfig = JSON.parse( + await fs.readFile(path.join(home, ".openclaw", "openclaw.json"), "utf8"), + ) as OpenClawConfig; + expect(persistedConfig.tools?.web?.search?.gemini?.apiKey).toEqual({ + source: "env", + provider: "default", + id: "MISSING_WEB_SEARCH_GEMINI_API_KEY", }); - expect("key" in persistedStore ? persistedStore.key : undefined).toBeUndefined(); }); }); diff --git a/src/secrets/runtime.ts b/src/secrets/runtime.ts index 9e69ffa60ad..903fe5a6d24 100644 --- a/src/secrets/runtime.ts +++ b/src/secrets/runtime.ts @@ -25,6 +25,7 @@ import { createResolverContext, type SecretResolverWarning, } from "./runtime-shared.js"; +import { resolveRuntimeWebTools, type RuntimeWebToolsMetadata } from "./runtime-web-tools.js"; export type { SecretResolverWarning } from "./runtime-shared.js"; @@ -33,6 +34,7 @@ export type PreparedSecretsRuntimeSnapshot = { config: OpenClawConfig; authStores: Array<{ agentDir: string; store: AuthProfileStore }>; warnings: SecretResolverWarning[]; + webTools: RuntimeWebToolsMetadata; }; type SecretsRuntimeRefreshContext = { @@ -57,6 +59,7 @@ function cloneSnapshot(snapshot: PreparedSecretsRuntimeSnapshot): PreparedSecret store: structuredClone(entry.store), })), warnings: snapshot.warnings.map((warning) => ({ ...warning })), + webTools: structuredClone(snapshot.webTools), }; } @@ -148,6 +151,11 @@ export async function prepareSecretsRuntimeSnapshot(params: { config: resolvedConfig, authStores, warnings: context.warnings, + webTools: await resolveRuntimeWebTools({ + sourceConfig, + resolvedConfig, + context, + }), }; preparedSnapshotRefreshContext.set(snapshot, { env: { ...(params.env ?? process.env) } as Record, @@ -185,7 +193,6 @@ export function activateSecretsRuntimeSnapshot(snapshot: PreparedSecretsRuntimeS activateSecretsRuntimeSnapshot(refreshed); return true; }, - clearOnRefreshFailure: clearActiveSecretsRuntimeState, }); } @@ -200,6 +207,13 @@ export function getActiveSecretsRuntimeSnapshot(): PreparedSecretsRuntimeSnapsho return snapshot; } +export function getActiveRuntimeWebToolsMetadata(): RuntimeWebToolsMetadata | null { + if (!activeSnapshot) { + return null; + } + return structuredClone(activeSnapshot.webTools); +} + export function resolveCommandSecretsFromActiveRuntimeSnapshot(params: { commandName: string; targetIds: ReadonlySet; diff --git a/src/secrets/target-registry-data.ts b/src/secrets/target-registry-data.ts index 3be4992d28f..f085c9981ab 100644 --- a/src/secrets/target-registry-data.ts +++ b/src/secrets/target-registry-data.ts @@ -689,6 +689,17 @@ const SECRET_TARGET_REGISTRY: SecretTargetRegistryEntry[] = [ includeInConfigure: true, includeInAudit: true, }, + { + id: "tools.web.fetch.firecrawl.apiKey", + targetType: "tools.web.fetch.firecrawl.apiKey", + configFile: "openclaw.json", + pathPattern: "tools.web.fetch.firecrawl.apiKey", + secretShape: SECRET_INPUT_SHAPE, + expectedResolvedValue: "string", + includeInPlan: true, + includeInConfigure: true, + includeInAudit: true, + }, { id: "tools.web.search.apiKey", targetType: "tools.web.search.apiKey", From 705c6a422dfc75463cedc2f51d1a46cd2384d8b7 Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Mon, 9 Mar 2026 23:01:55 -0500 Subject: [PATCH 004/126] Add provider routing details to bug report form (#41712) --- .github/ISSUE_TEMPLATE/bug_report.yml | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index c45885b48b6..3be43c6740a 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -76,6 +76,37 @@ body: label: Install method description: How OpenClaw was installed or launched. placeholder: npm global / pnpm dev / docker / mac app + - type: input + id: model + attributes: + label: Model + description: Effective model under test. + placeholder: minimax/text-01 / openrouter/anthropic/claude-opus-4.1 / anthropic/claude-sonnet-4.5 + validations: + required: true + - type: input + id: provider_chain + attributes: + label: Provider / routing chain + description: Effective request path through gateways, proxies, providers, or model routers. + placeholder: openclaw -> cloudflare-ai-gateway -> minimax + validations: + required: true + - type: input + id: config_location + attributes: + label: Config file / key location + description: Optional. Relevant config source or key path if this bug depends on overrides or custom provider setup. Redact secrets. + placeholder: ~/.openclaw/openclaw.json ; models.providers.cloudflare-ai-gateway.baseUrl ; ~/.openclaw/agents//agent/models.json + - type: textarea + id: provider_setup_details + attributes: + label: Additional provider/model setup details + description: Optional. Include redacted routing details, per-agent overrides, auth-profile interactions, env/config context, or anything else needed to explain the effective provider/model setup. Do not include API keys, tokens, or passwords. + placeholder: | + Default route is openclaw -> cloudflare-ai-gateway -> minimax. + Previous setup was openclaw -> cloudflare-ai-gateway -> openrouter -> minimax. + Relevant config lives in ~/.openclaw/openclaw.json under models.providers.minimax and models.providers.cloudflare-ai-gateway. - type: textarea id: logs attributes: From 989ee21b2414a574164d9871215cf32089edf7a7 Mon Sep 17 00:00:00 2001 From: Benji Peng <11394934+benjipeng@users.noreply.github.com> Date: Tue, 10 Mar 2026 00:14:07 -0400 Subject: [PATCH 005/126] ui: fix sessions table collapse on narrow widths (#12175) Merged via squash. Prepared head SHA: b1fcfba868fcfb7b9ee3496725921f3f38f58ac4 Co-authored-by: benjipeng <11394934+benjipeng@users.noreply.github.com> Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com> Reviewed-by: @BunsDev --- .pi/prompts/reviewpr.md | 15 +++++++-------- CHANGELOG.md | 1 + src/node-host/runner.credentials.test.ts | 3 +++ ui/src/styles/components.css | 15 +++++++++++++++ 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/.pi/prompts/reviewpr.md b/.pi/prompts/reviewpr.md index e3ebc0dd9c6..1b8a20dda90 100644 --- a/.pi/prompts/reviewpr.md +++ b/.pi/prompts/reviewpr.md @@ -12,7 +12,6 @@ Do (review-only) Goal: produce a thorough review and a clear recommendation (READY FOR /landpr vs NEEDS WORK vs INVALID CLAIM). Do NOT merge, do NOT push, do NOT make changes in the repo as part of this command. 0. Truthfulness + reality gate (required for bug-fix claims) - - Do not trust the issue text or PR summary by default; verify in code and evidence. - If the PR claims to fix a bug linked to an issue, confirm the bug exists now (repro steps, logs, failing test, or clear code-path proof). - Prove root cause with exact location (`path/file.ts:line` + explanation of why behavior is wrong). @@ -86,13 +85,13 @@ B) Claim verification matrix (required) - Fill this table: - | Field | Evidence | - |---|---| - | Claimed problem | ... | - | Evidence observed (repro/log/test/code) | ... | - | Root cause location (`path:line`) | ... | - | Why this fix addresses that root cause | ... | - | Regression coverage (test name or manual proof) | ... | + | Field | Evidence | + | ----------------------------------------------- | -------- | + | Claimed problem | ... | + | Evidence observed (repro/log/test/code) | ... | + | Root cause location (`path:line`) | ... | + | Why this fix addresses that root cause | ... | + | Regression coverage (test name or manual proof) | ... | - If any row is missing/weak, default to `NEEDS WORK` or `INVALID CLAIM`. diff --git a/CHANGELOG.md b/CHANGELOG.md index c19a5c2eda7..a786e384dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai - CLI/memory teardown: close cached memory search/index managers in the one-shot CLI shutdown path so watcher-backed memory caches no longer keep completed CLI runs alive after output finishes. (#40389) thanks @Julbarth. - Tools/web search: treat Brave `llm-context` grounding snippets as plain strings so `web_search` no longer returns empty snippet arrays in LLM Context mode. (#41387) thanks @zheliu2. - Telegram/exec approvals: reject `/approve` commands aimed at other bots, keep deterministic approval prompts visible when tool-result delivery fails, and stop resolved exact IDs from matching other pending approvals by prefix. (#37233) Thanks @huntharo. +- Control UI/Sessions: restore single-column session table collapse on narrow viewport or container widths by moving the responsive table override next to the base grid rule and enabling inline-size container queries. (#12175) Thanks @benjipeng. ## 2026.3.8 diff --git a/src/node-host/runner.credentials.test.ts b/src/node-host/runner.credentials.test.ts index 9c17c605421..6138a6b954e 100644 --- a/src/node-host/runner.credentials.test.ts +++ b/src/node-host/runner.credentials.test.ts @@ -76,6 +76,7 @@ describe("resolveNodeHostGatewayCredentials", () => { await withEnvAsync( { OPENCLAW_GATEWAY_TOKEN: undefined, + OPENCLAW_GATEWAY_PASSWORD: undefined, REMOTE_GATEWAY_TOKEN: "token-from-ref", }, async () => { @@ -91,6 +92,7 @@ describe("resolveNodeHostGatewayCredentials", () => { await withEnvAsync( { OPENCLAW_GATEWAY_TOKEN: "token-from-env", + OPENCLAW_GATEWAY_PASSWORD: undefined, REMOTE_GATEWAY_TOKEN: "token-from-ref", }, async () => { @@ -106,6 +108,7 @@ describe("resolveNodeHostGatewayCredentials", () => { await withEnvAsync( { OPENCLAW_GATEWAY_TOKEN: undefined, + OPENCLAW_GATEWAY_PASSWORD: undefined, MISSING_REMOTE_GATEWAY_TOKEN: undefined, }, async () => { diff --git a/ui/src/styles/components.css b/ui/src/styles/components.css index c7a6a425dc7..126972ca003 100644 --- a/ui/src/styles/components.css +++ b/ui/src/styles/components.css @@ -1425,6 +1425,7 @@ .table { display: grid; + container-type: inline-size; gap: 6px; } @@ -1455,6 +1456,20 @@ border-color: var(--border-strong); } +@media (max-width: 1100px) { + .table-head, + .table-row { + grid-template-columns: 1fr; + } +} + +@container (max-width: 1100px) { + .table-head, + .table-row { + grid-template-columns: 1fr; + } +} + .session-link { text-decoration: none; color: var(--accent); From 96e4975922de172ddac985fcd3bfdeaf13cc16ae Mon Sep 17 00:00:00 2001 From: Frank Yang Date: Tue, 10 Mar 2026 12:44:33 +0800 Subject: [PATCH 006/126] fix: protect bootstrap files during memory flush (#38574) Merged via squash. Prepared head SHA: a0b9a02e2ef1a6f5480621ccb799a8b35a10ce48 Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com> Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com> Reviewed-by: @frankekn --- CHANGELOG.md | 1 + src/agents/pi-embedded-runner/run/attempt.ts | 2 + src/agents/pi-embedded-runner/run/params.ts | 2 + src/agents/pi-tools.read.ts | 156 ++++++++++++++++++ src/agents/pi-tools.ts | 40 ++++- .../pi-tools.workspace-only-false.test.ts | 54 +++++- src/auto-reply/reply/agent-runner-memory.ts | 8 + .../agent-runner.runreplyagent.e2e.test.ts | 22 ++- src/auto-reply/reply/memory-flush.test.ts | 15 +- src/auto-reply/reply/memory-flush.ts | 57 ++++++- src/auto-reply/reply/reply-state.test.ts | 8 + src/infra/fs-safe.test.ts | 57 +++++++ src/infra/fs-safe.ts | 61 ++++++- 13 files changed, 468 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a786e384dc4..f017b834209 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -468,6 +468,7 @@ Docs: https://docs.openclaw.ai - Control UI/Telegram sender labels: preserve inbound sender labels in sanitized chat history so dashboard user-message groups split correctly and show real group-member names instead of `You`. (#39414) Thanks @obviyus. - Agents/failover 402 recovery: keep temporary spend-limit `402` payloads retryable, preserve explicit insufficient-credit billing detection even in long provider payloads, and allow throttled billing-cooldown probes so single-provider setups can recover instead of staying locked out. (#38533) Thanks @xialonglee. - Browser/config schema: accept `browser.profiles.*.driver: "openclaw"` while preserving legacy `"clawd"` compatibility in validated config. (#39374; based on #35621) Thanks @gambletan and @ingyukoh. +- Memory flush/bootstrap file protection: restrict memory-flush runs to append-only `read`/`write` tools and route host-side memory appends through root-enforced safe file handles so flush turns cannot overwrite bootstrap files via `exec` or unsafe raw rewrites. (#38574) Thanks @frankekn. ## 2026.3.2 diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index 25f13c666c7..f6f18801497 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -870,6 +870,8 @@ export async function runEmbeddedAttempt( agentDir, workspaceDir: effectiveWorkspace, config: params.config, + trigger: params.trigger, + memoryFlushWritePath: params.memoryFlushWritePath, abortSignal: runAbortController.signal, modelProvider: params.model.provider, modelId: params.modelId, diff --git a/src/agents/pi-embedded-runner/run/params.ts b/src/agents/pi-embedded-runner/run/params.ts index ee743d7a0c1..bf65515ce46 100644 --- a/src/agents/pi-embedded-runner/run/params.ts +++ b/src/agents/pi-embedded-runner/run/params.ts @@ -29,6 +29,8 @@ export type RunEmbeddedPiAgentParams = { agentAccountId?: string; /** What initiated this agent run: "user", "heartbeat", "cron", or "memory". */ trigger?: string; + /** Relative workspace path that memory-triggered writes are allowed to append to. */ + memoryFlushWritePath?: string; /** Delivery target (e.g. telegram:group:123:topic:456) for topic/thread routing. */ messageTo?: string; /** Thread/topic identifier for routing replies to the originating thread. */ diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index b01c7adff03..5ea48b01fa1 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -4,6 +4,7 @@ import { fileURLToPath } from "node:url"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { createEditTool, createReadTool, createWriteTool } from "@mariozechner/pi-coding-agent"; import { + appendFileWithinRoot, SafeOpenError, openFileWithinRoot, readFileWithinRoot, @@ -406,6 +407,161 @@ function mapContainerPathToWorkspaceRoot(params: { return path.resolve(params.root, ...relative.split("/").filter(Boolean)); } +export function resolveToolPathAgainstWorkspaceRoot(params: { + filePath: string; + root: string; + containerWorkdir?: string; +}): string { + const mapped = mapContainerPathToWorkspaceRoot(params); + const candidate = mapped.startsWith("@") ? mapped.slice(1) : mapped; + return path.isAbsolute(candidate) + ? path.resolve(candidate) + : path.resolve(params.root, candidate || "."); +} + +type MemoryFlushAppendOnlyWriteOptions = { + root: string; + relativePath: string; + containerWorkdir?: string; + sandbox?: { + root: string; + bridge: SandboxFsBridge; + }; +}; + +async function readOptionalUtf8File(params: { + absolutePath: string; + relativePath: string; + sandbox?: MemoryFlushAppendOnlyWriteOptions["sandbox"]; + signal?: AbortSignal; +}): Promise { + try { + if (params.sandbox) { + const stat = await params.sandbox.bridge.stat({ + filePath: params.relativePath, + cwd: params.sandbox.root, + signal: params.signal, + }); + if (!stat) { + return ""; + } + const buffer = await params.sandbox.bridge.readFile({ + filePath: params.relativePath, + cwd: params.sandbox.root, + signal: params.signal, + }); + return buffer.toString("utf-8"); + } + return await fs.readFile(params.absolutePath, "utf-8"); + } catch (error) { + if ((error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") { + return ""; + } + throw error; + } +} + +async function appendMemoryFlushContent(params: { + absolutePath: string; + root: string; + relativePath: string; + content: string; + sandbox?: MemoryFlushAppendOnlyWriteOptions["sandbox"]; + signal?: AbortSignal; +}) { + if (!params.sandbox) { + await appendFileWithinRoot({ + rootDir: params.root, + relativePath: params.relativePath, + data: params.content, + mkdir: true, + prependNewlineIfNeeded: true, + }); + return; + } + + const existing = await readOptionalUtf8File({ + absolutePath: params.absolutePath, + relativePath: params.relativePath, + sandbox: params.sandbox, + signal: params.signal, + }); + const separator = + existing.length > 0 && !existing.endsWith("\n") && !params.content.startsWith("\n") ? "\n" : ""; + const next = `${existing}${separator}${params.content}`; + if (params.sandbox) { + const parent = path.posix.dirname(params.relativePath); + if (parent && parent !== ".") { + await params.sandbox.bridge.mkdirp({ + filePath: parent, + cwd: params.sandbox.root, + signal: params.signal, + }); + } + await params.sandbox.bridge.writeFile({ + filePath: params.relativePath, + cwd: params.sandbox.root, + data: next, + mkdir: true, + signal: params.signal, + }); + return; + } + await fs.mkdir(path.dirname(params.absolutePath), { recursive: true }); + await fs.writeFile(params.absolutePath, next, "utf-8"); +} + +export function wrapToolMemoryFlushAppendOnlyWrite( + tool: AnyAgentTool, + options: MemoryFlushAppendOnlyWriteOptions, +): AnyAgentTool { + const allowedAbsolutePath = path.resolve(options.root, options.relativePath); + return { + ...tool, + description: `${tool.description} During memory flush, this tool may only append to ${options.relativePath}.`, + execute: async (toolCallId, args, signal, onUpdate) => { + const normalized = normalizeToolParams(args); + const record = + normalized ?? + (args && typeof args === "object" ? (args as Record) : undefined); + assertRequiredParams(record, CLAUDE_PARAM_GROUPS.write, tool.name); + const filePath = + typeof record?.path === "string" && record.path.trim() ? record.path : undefined; + const content = typeof record?.content === "string" ? record.content : undefined; + if (!filePath || content === undefined) { + return tool.execute(toolCallId, normalized ?? args, signal, onUpdate); + } + + const resolvedPath = resolveToolPathAgainstWorkspaceRoot({ + filePath, + root: options.root, + containerWorkdir: options.containerWorkdir, + }); + if (resolvedPath !== allowedAbsolutePath) { + throw new Error( + `Memory flush writes are restricted to ${options.relativePath}; use that path only.`, + ); + } + + await appendMemoryFlushContent({ + absolutePath: allowedAbsolutePath, + root: options.root, + relativePath: options.relativePath, + content, + sandbox: options.sandbox, + signal, + }); + return { + content: [{ type: "text", text: `Appended content to ${options.relativePath}.` }], + details: { + path: options.relativePath, + appendOnly: true, + }, + }; + }, + }; +} + export function wrapToolWorkspaceRootGuardWithOptions( tool: AnyAgentTool, root: string, diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 543a163ab0c..14418bbd362 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -36,6 +36,7 @@ import { createSandboxedWriteTool, normalizeToolParams, patchToolSchemaForClaudeCompatibility, + wrapToolMemoryFlushAppendOnlyWrite, wrapToolWorkspaceRootGuard, wrapToolWorkspaceRootGuardWithOptions, wrapToolParamNormalization, @@ -67,6 +68,7 @@ const TOOL_DENY_BY_MESSAGE_PROVIDER: Readonly> voice: ["tts"], }; const TOOL_DENY_FOR_XAI_PROVIDERS = new Set(["web_search"]); +const MEMORY_FLUSH_ALLOWED_TOOL_NAMES = new Set(["read", "write"]); function normalizeMessageProvider(messageProvider?: string): string | undefined { const normalized = messageProvider?.trim().toLowerCase(); @@ -207,6 +209,10 @@ export function createOpenClawCodingTools(options?: { sessionId?: string; /** Stable run identifier for this agent invocation. */ runId?: string; + /** What initiated this run (for trigger-specific tool restrictions). */ + trigger?: string; + /** Relative workspace path that memory-triggered writes may append to. */ + memoryFlushWritePath?: string; agentDir?: string; workspaceDir?: string; config?: OpenClawConfig; @@ -258,6 +264,11 @@ export function createOpenClawCodingTools(options?: { }): AnyAgentTool[] { const execToolName = "exec"; const sandbox = options?.sandbox?.enabled ? options.sandbox : undefined; + const isMemoryFlushRun = options?.trigger === "memory"; + if (isMemoryFlushRun && !options?.memoryFlushWritePath) { + throw new Error("memoryFlushWritePath required for memory-triggered tool runs"); + } + const memoryFlushWritePath = isMemoryFlushRun ? options.memoryFlushWritePath : undefined; const { agentId, globalPolicy, @@ -322,7 +333,7 @@ export function createOpenClawCodingTools(options?: { const execConfig = resolveExecConfig({ cfg: options?.config, agentId }); const fsConfig = resolveToolFsConfig({ cfg: options?.config, agentId }); const fsPolicy = createToolFsPolicy({ - workspaceOnly: fsConfig.workspaceOnly, + workspaceOnly: isMemoryFlushRun || fsConfig.workspaceOnly, }); const sandboxRoot = sandbox?.workspaceDir; const sandboxFsBridge = sandbox?.fsBridge; @@ -515,7 +526,32 @@ export function createOpenClawCodingTools(options?: { sessionId: options?.sessionId, }), ]; - const toolsForMessageProvider = applyMessageProviderToolPolicy(tools, options?.messageProvider); + const toolsForMemoryFlush = + isMemoryFlushRun && memoryFlushWritePath + ? tools.flatMap((tool) => { + if (!MEMORY_FLUSH_ALLOWED_TOOL_NAMES.has(tool.name)) { + return []; + } + if (tool.name === "write") { + return [ + wrapToolMemoryFlushAppendOnlyWrite(tool, { + root: sandboxRoot ?? workspaceRoot, + relativePath: memoryFlushWritePath, + containerWorkdir: sandbox?.containerWorkdir, + sandbox: + sandboxRoot && sandboxFsBridge + ? { root: sandboxRoot, bridge: sandboxFsBridge } + : undefined, + }), + ]; + } + return [tool]; + }) + : tools; + const toolsForMessageProvider = applyMessageProviderToolPolicy( + toolsForMemoryFlush, + options?.messageProvider, + ); const toolsForModelProvider = applyModelProviderToolPolicy(toolsForMessageProvider, { modelProvider: options?.modelProvider, modelId: options?.modelId, diff --git a/src/agents/pi-tools.workspace-only-false.test.ts b/src/agents/pi-tools.workspace-only-false.test.ts index 713315de899..fb18260db09 100644 --- a/src/agents/pi-tools.workspace-only-false.test.ts +++ b/src/agents/pi-tools.workspace-only-false.test.ts @@ -1,7 +1,13 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@mariozechner/pi-ai/oauth", () => ({ + getOAuthApiKey: () => undefined, + getOAuthProviders: () => [], +})); + import { createOpenClawCodingTools } from "./pi-tools.js"; describe("FS tools with workspaceOnly=false", () => { @@ -181,4 +187,50 @@ describe("FS tools with workspaceOnly=false", () => { }), ).rejects.toThrow(/Path escapes (workspace|sandbox) root/); }); + + it("restricts memory-triggered writes to append-only canonical memory files", async () => { + const allowedRelativePath = "memory/2026-03-07.md"; + const allowedAbsolutePath = path.join(workspaceDir, allowedRelativePath); + await fs.mkdir(path.dirname(allowedAbsolutePath), { recursive: true }); + await fs.writeFile(allowedAbsolutePath, "seed"); + + const tools = createOpenClawCodingTools({ + workspaceDir, + trigger: "memory", + memoryFlushWritePath: allowedRelativePath, + config: { + tools: { + exec: { + applyPatch: { + enabled: true, + }, + }, + }, + }, + modelProvider: "openai", + modelId: "gpt-5", + }); + + const writeTool = tools.find((tool) => tool.name === "write"); + expect(writeTool).toBeDefined(); + expect(tools.map((tool) => tool.name).toSorted()).toEqual(["read", "write"]); + + await expect( + writeTool!.execute("test-call-memory-deny", { + path: outsideFile, + content: "should not write here", + }), + ).rejects.toThrow(/Memory flush writes are restricted to memory\/2026-03-07\.md/); + + const result = await writeTool!.execute("test-call-memory-append", { + path: allowedRelativePath, + content: "new note", + }); + expect(hasToolError(result)).toBe(false); + expect(result.content).toContainEqual({ + type: "text", + text: "Appended content to memory/2026-03-07.md.", + }); + await expect(fs.readFile(allowedAbsolutePath, "utf-8")).resolves.toBe("seed\nnew note"); + }); }); diff --git a/src/auto-reply/reply/agent-runner-memory.ts b/src/auto-reply/reply/agent-runner-memory.ts index 643611d35a2..623bb9c1490 100644 --- a/src/auto-reply/reply/agent-runner-memory.ts +++ b/src/auto-reply/reply/agent-runner-memory.ts @@ -34,6 +34,7 @@ import { import { hasAlreadyFlushedForCurrentCompaction, resolveMemoryFlushContextWindowTokens, + resolveMemoryFlushRelativePathForRun, resolveMemoryFlushPromptForRun, resolveMemoryFlushSettings, shouldRunMemoryFlush, @@ -465,6 +466,11 @@ export async function runMemoryFlushIfNeeded(params: { }); } let memoryCompactionCompleted = false; + const memoryFlushNowMs = Date.now(); + const memoryFlushWritePath = resolveMemoryFlushRelativePathForRun({ + cfg: params.cfg, + nowMs: memoryFlushNowMs, + }); const flushSystemPrompt = [ params.followupRun.run.extraSystemPrompt, memoryFlushSettings.systemPrompt, @@ -495,9 +501,11 @@ export async function runMemoryFlushIfNeeded(params: { ...senderContext, ...runBaseParams, trigger: "memory", + memoryFlushWritePath, prompt: resolveMemoryFlushPromptForRun({ prompt: memoryFlushSettings.prompt, cfg: params.cfg, + nowMs: memoryFlushNowMs, }), extraSystemPrompt: flushSystemPrompt, bootstrapPromptWarningSignaturesSeen, diff --git a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts index db034ac03a6..599a8fd6a48 100644 --- a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts +++ b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts @@ -28,6 +28,7 @@ type AgentRunParams = { type EmbeddedRunParams = { prompt?: string; extraSystemPrompt?: string; + memoryFlushWritePath?: string; bootstrapPromptWarningSignaturesSeen?: string[]; bootstrapPromptWarningSignature?: string; onAgentEvent?: (evt: { stream?: string; data?: { phase?: string; willRetry?: boolean } }) => void; @@ -1611,9 +1612,14 @@ describe("runReplyAgent memory flush", () => { const flushCall = calls[0]; expect(flushCall?.prompt).toContain("Write notes."); expect(flushCall?.prompt).toContain("NO_REPLY"); + expect(flushCall?.prompt).toMatch(/memory\/\d{4}-\d{2}-\d{2}\.md/); + expect(flushCall?.prompt).toContain("MEMORY.md"); + expect(flushCall?.memoryFlushWritePath).toMatch(/^memory\/\d{4}-\d{2}-\d{2}\.md$/); expect(flushCall?.extraSystemPrompt).toContain("extra system"); expect(flushCall?.extraSystemPrompt).toContain("Flush memory now."); expect(flushCall?.extraSystemPrompt).toContain("NO_REPLY"); + expect(flushCall?.extraSystemPrompt).toContain("memory/YYYY-MM-DD.md"); + expect(flushCall?.extraSystemPrompt).toContain("MEMORY.md"); expect(calls[1]?.prompt).toBe("hello"); }); }); @@ -1701,9 +1707,17 @@ describe("runReplyAgent memory flush", () => { await seedSessionStore({ storePath, sessionKey, entry: sessionEntry }); - const calls: Array<{ prompt?: string }> = []; + const calls: Array<{ + prompt?: string; + extraSystemPrompt?: string; + memoryFlushWritePath?: string; + }> = []; state.runEmbeddedPiAgentMock.mockImplementation(async (params: EmbeddedRunParams) => { - calls.push({ prompt: params.prompt }); + calls.push({ + prompt: params.prompt, + extraSystemPrompt: params.extraSystemPrompt, + memoryFlushWritePath: params.memoryFlushWritePath, + }); if (params.prompt?.includes("Pre-compaction memory flush.")) { return { payloads: [], meta: {} }; } @@ -1730,6 +1744,10 @@ describe("runReplyAgent memory flush", () => { expect(calls[0]?.prompt).toContain("Pre-compaction memory flush."); expect(calls[0]?.prompt).toContain("Current time:"); expect(calls[0]?.prompt).toMatch(/memory\/\d{4}-\d{2}-\d{2}\.md/); + expect(calls[0]?.prompt).toContain("MEMORY.md"); + expect(calls[0]?.memoryFlushWritePath).toMatch(/^memory\/\d{4}-\d{2}-\d{2}\.md$/); + expect(calls[0]?.extraSystemPrompt).toContain("memory/YYYY-MM-DD.md"); + expect(calls[0]?.extraSystemPrompt).toContain("MEMORY.md"); expect(calls[1]?.prompt).toBe("hello"); const stored = JSON.parse(await fs.readFile(storePath, "utf-8")); diff --git a/src/auto-reply/reply/memory-flush.test.ts b/src/auto-reply/reply/memory-flush.test.ts index 0e04e7e0ea3..079c5578676 100644 --- a/src/auto-reply/reply/memory-flush.test.ts +++ b/src/auto-reply/reply/memory-flush.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../config/config.js"; -import { DEFAULT_MEMORY_FLUSH_PROMPT, resolveMemoryFlushPromptForRun } from "./memory-flush.js"; +import { + DEFAULT_MEMORY_FLUSH_PROMPT, + resolveMemoryFlushPromptForRun, + resolveMemoryFlushRelativePathForRun, +} from "./memory-flush.js"; describe("resolveMemoryFlushPromptForRun", () => { const cfg = { @@ -35,6 +39,15 @@ describe("resolveMemoryFlushPromptForRun", () => { expect(prompt).toContain("Current time: already present"); expect((prompt.match(/Current time:/g) ?? []).length).toBe(1); }); + + it("resolves the canonical relative memory path using user timezone", () => { + const relativePath = resolveMemoryFlushRelativePathForRun({ + cfg, + nowMs: Date.UTC(2026, 1, 16, 15, 0, 0), + }); + + expect(relativePath).toBe("memory/2026-02-16.md"); + }); }); describe("DEFAULT_MEMORY_FLUSH_PROMPT", () => { diff --git a/src/auto-reply/reply/memory-flush.ts b/src/auto-reply/reply/memory-flush.ts index c02fad5eca0..95f6dbaa053 100644 --- a/src/auto-reply/reply/memory-flush.ts +++ b/src/auto-reply/reply/memory-flush.ts @@ -10,10 +10,23 @@ import { SILENT_REPLY_TOKEN } from "../tokens.js"; export const DEFAULT_MEMORY_FLUSH_SOFT_TOKENS = 4000; export const DEFAULT_MEMORY_FLUSH_FORCE_TRANSCRIPT_BYTES = 2 * 1024 * 1024; +const MEMORY_FLUSH_TARGET_HINT = + "Store durable memories only in memory/YYYY-MM-DD.md (create memory/ if needed)."; +const MEMORY_FLUSH_APPEND_ONLY_HINT = + "If memory/YYYY-MM-DD.md already exists, APPEND new content only and do not overwrite existing entries."; +const MEMORY_FLUSH_READ_ONLY_HINT = + "Treat workspace bootstrap/reference files such as MEMORY.md, SOUL.md, TOOLS.md, and AGENTS.md as read-only during this flush; never overwrite, replace, or edit them."; +const MEMORY_FLUSH_REQUIRED_HINTS = [ + MEMORY_FLUSH_TARGET_HINT, + MEMORY_FLUSH_APPEND_ONLY_HINT, + MEMORY_FLUSH_READ_ONLY_HINT, +]; + export const DEFAULT_MEMORY_FLUSH_PROMPT = [ "Pre-compaction memory flush.", - "Store durable memories now (use memory/YYYY-MM-DD.md; create memory/ if needed).", - "IMPORTANT: If the file already exists, APPEND new content only — do not overwrite existing entries.", + MEMORY_FLUSH_TARGET_HINT, + MEMORY_FLUSH_READ_ONLY_HINT, + MEMORY_FLUSH_APPEND_ONLY_HINT, "Do NOT create timestamped variant files (e.g., YYYY-MM-DD-HHMM.md); always use the canonical YYYY-MM-DD.md filename.", `If nothing to store, reply with ${SILENT_REPLY_TOKEN}.`, ].join(" "); @@ -21,6 +34,9 @@ export const DEFAULT_MEMORY_FLUSH_PROMPT = [ export const DEFAULT_MEMORY_FLUSH_SYSTEM_PROMPT = [ "Pre-compaction memory flush turn.", "The session is near auto-compaction; capture durable memories to disk.", + MEMORY_FLUSH_TARGET_HINT, + MEMORY_FLUSH_READ_ONLY_HINT, + MEMORY_FLUSH_APPEND_ONLY_HINT, `You may reply, but usually ${SILENT_REPLY_TOKEN} is correct.`, ].join(" "); @@ -40,14 +56,29 @@ function formatDateStampInTimezone(nowMs: number, timezone: string): string { return new Date(nowMs).toISOString().slice(0, 10); } +export function resolveMemoryFlushRelativePathForRun(params: { + cfg?: OpenClawConfig; + nowMs?: number; +}): string { + const nowMs = Number.isFinite(params.nowMs) ? (params.nowMs as number) : Date.now(); + const { userTimezone } = resolveCronStyleNow(params.cfg ?? {}, nowMs); + const dateStamp = formatDateStampInTimezone(nowMs, userTimezone); + return `memory/${dateStamp}.md`; +} + export function resolveMemoryFlushPromptForRun(params: { prompt: string; cfg?: OpenClawConfig; nowMs?: number; }): string { const nowMs = Number.isFinite(params.nowMs) ? (params.nowMs as number) : Date.now(); - const { userTimezone, timeLine } = resolveCronStyleNow(params.cfg ?? {}, nowMs); - const dateStamp = formatDateStampInTimezone(nowMs, userTimezone); + const { timeLine } = resolveCronStyleNow(params.cfg ?? {}, nowMs); + const dateStamp = resolveMemoryFlushRelativePathForRun({ + cfg: params.cfg, + nowMs, + }) + .replace(/^memory\//, "") + .replace(/\.md$/, ""); const withDate = params.prompt.replaceAll("YYYY-MM-DD", dateStamp).trimEnd(); if (!withDate) { return timeLine; @@ -90,8 +121,12 @@ export function resolveMemoryFlushSettings(cfg?: OpenClawConfig): MemoryFlushSet const forceFlushTranscriptBytes = parseNonNegativeByteSize(defaults?.forceFlushTranscriptBytes) ?? DEFAULT_MEMORY_FLUSH_FORCE_TRANSCRIPT_BYTES; - const prompt = defaults?.prompt?.trim() || DEFAULT_MEMORY_FLUSH_PROMPT; - const systemPrompt = defaults?.systemPrompt?.trim() || DEFAULT_MEMORY_FLUSH_SYSTEM_PROMPT; + const prompt = ensureMemoryFlushSafetyHints( + defaults?.prompt?.trim() || DEFAULT_MEMORY_FLUSH_PROMPT, + ); + const systemPrompt = ensureMemoryFlushSafetyHints( + defaults?.systemPrompt?.trim() || DEFAULT_MEMORY_FLUSH_SYSTEM_PROMPT, + ); const reserveTokensFloor = normalizeNonNegativeInt(cfg?.agents?.defaults?.compaction?.reserveTokensFloor) ?? DEFAULT_PI_COMPACTION_RESERVE_TOKENS_FLOOR; @@ -113,6 +148,16 @@ function ensureNoReplyHint(text: string): string { return `${text}\n\nIf no user-visible reply is needed, start with ${SILENT_REPLY_TOKEN}.`; } +function ensureMemoryFlushSafetyHints(text: string): string { + let next = text.trim(); + for (const hint of MEMORY_FLUSH_REQUIRED_HINTS) { + if (!next.includes(hint)) { + next = next ? `${next}\n\n${hint}` : hint; + } + } + return next; +} + export function resolveMemoryFlushContextWindowTokens(params: { modelId?: string; agentCfgContextTokens?: number; diff --git a/src/auto-reply/reply/reply-state.test.ts b/src/auto-reply/reply/reply-state.test.ts index 56623fe6cfa..69dbad531e7 100644 --- a/src/auto-reply/reply/reply-state.test.ts +++ b/src/auto-reply/reply/reply-state.test.ts @@ -203,6 +203,10 @@ describe("memory flush settings", () => { expect(settings?.forceFlushTranscriptBytes).toBe(DEFAULT_MEMORY_FLUSH_FORCE_TRANSCRIPT_BYTES); expect(settings?.prompt.length).toBeGreaterThan(0); expect(settings?.systemPrompt.length).toBeGreaterThan(0); + expect(settings?.prompt).toContain("memory/YYYY-MM-DD.md"); + expect(settings?.prompt).toContain("MEMORY.md"); + expect(settings?.systemPrompt).toContain("memory/YYYY-MM-DD.md"); + expect(settings?.systemPrompt).toContain("MEMORY.md"); }); it("respects disable flag", () => { @@ -230,6 +234,10 @@ describe("memory flush settings", () => { }); expect(settings?.prompt).toContain("NO_REPLY"); expect(settings?.systemPrompt).toContain("NO_REPLY"); + expect(settings?.prompt).toContain("memory/YYYY-MM-DD.md"); + expect(settings?.prompt).toContain("MEMORY.md"); + expect(settings?.systemPrompt).toContain("memory/YYYY-MM-DD.md"); + expect(settings?.systemPrompt).toContain("MEMORY.md"); }); it("falls back to defaults when numeric values are invalid", () => { diff --git a/src/infra/fs-safe.test.ts b/src/infra/fs-safe.test.ts index a8372a86c70..ba4c13dfc7c 100644 --- a/src/infra/fs-safe.test.ts +++ b/src/infra/fs-safe.test.ts @@ -7,6 +7,7 @@ import { } from "../test-utils/symlink-rebind-race.js"; import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; import { + appendFileWithinRoot, copyFileWithinRoot, createRootScopedReadFile, SafeOpenError, @@ -246,6 +247,22 @@ describe("fs-safe", () => { await expect(fs.readFile(path.join(root, "nested", "out.txt"), "utf8")).resolves.toBe("hello"); }); + it("appends to a file within root safely", async () => { + const root = await tempDirs.make("openclaw-fs-safe-root-"); + const targetPath = path.join(root, "nested", "out.txt"); + await fs.mkdir(path.dirname(targetPath), { recursive: true }); + await fs.writeFile(targetPath, "seed"); + + await appendFileWithinRoot({ + rootDir: root, + relativePath: "nested/out.txt", + data: "next", + prependNewlineIfNeeded: true, + }); + + await expect(fs.readFile(targetPath, "utf8")).resolves.toBe("seed\nnext"); + }); + it("does not truncate existing target when atomic rename fails", async () => { const root = await tempDirs.make("openclaw-fs-safe-root-"); const targetPath = path.join(root, "nested", "out.txt"); @@ -439,6 +456,25 @@ describe("fs-safe", () => { }); }); + it.runIf(process.platform !== "win32")("rejects appending through hardlink aliases", async () => { + const root = await tempDirs.make("openclaw-fs-safe-root-"); + const hardlinkPath = path.join(root, "alias.txt"); + await withOutsideHardlinkAlias({ + aliasPath: hardlinkPath, + run: async (outsideFile) => { + await expect( + appendFileWithinRoot({ + rootDir: root, + relativePath: "alias.txt", + data: "pwned", + prependNewlineIfNeeded: true, + }), + ).rejects.toMatchObject({ code: "invalid-path" }); + await expect(fs.readFile(outsideFile, "utf8")).resolves.toBe("outside"); + }, + }); + }); + it("does not truncate out-of-root file when symlink retarget races write open", async () => { const { root, outside, slot, outsideTarget } = await setupSymlinkWriteRaceFixture({ seedInsideTarget: true, @@ -459,6 +495,27 @@ describe("fs-safe", () => { await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); }); + it("does not clobber out-of-root file when symlink retarget races append open", async () => { + const { root, outside, slot, outsideTarget } = await setupSymlinkWriteRaceFixture({ + seedInsideTarget: true, + }); + + await expectSymlinkWriteRaceRejectsOutside({ + slotPath: slot, + outsideDir: outside, + runWrite: async (relativePath) => + await appendFileWithinRoot({ + rootDir: root, + relativePath, + data: "new-content", + mkdir: false, + prependNewlineIfNeeded: true, + }), + }); + + await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); + }); + it("does not clobber out-of-root file when symlink retarget races write-from-path open", async () => { const { root, outside, slot, outsideTarget } = await setupSymlinkWriteRaceFixture(); const sourceDir = await tempDirs.make("openclaw-fs-safe-source-"); diff --git a/src/infra/fs-safe.ts b/src/infra/fs-safe.ts index 3a0f28ddd2c..77754437528 100644 --- a/src/infra/fs-safe.ts +++ b/src/infra/fs-safe.ts @@ -57,6 +57,14 @@ const OPEN_WRITE_CREATE_FLAGS = fsConstants.O_CREAT | fsConstants.O_EXCL | (SUPPORTS_NOFOLLOW ? fsConstants.O_NOFOLLOW : 0); +const OPEN_APPEND_EXISTING_FLAGS = + fsConstants.O_RDWR | fsConstants.O_APPEND | (SUPPORTS_NOFOLLOW ? fsConstants.O_NOFOLLOW : 0); +const OPEN_APPEND_CREATE_FLAGS = + fsConstants.O_RDWR | + fsConstants.O_APPEND | + fsConstants.O_CREAT | + fsConstants.O_EXCL | + (SUPPORTS_NOFOLLOW ? fsConstants.O_NOFOLLOW : 0); const ensureTrailingSep = (value: string) => (value.endsWith(path.sep) ? value : value + path.sep); @@ -375,6 +383,7 @@ export async function openWritableFileWithinRoot(params: { mkdir?: boolean; mode?: number; truncateExisting?: boolean; + append?: boolean; }): Promise { const { rootReal, rootWithSep, resolved } = await resolvePathWithinRoot(params); try { @@ -410,14 +419,16 @@ export async function openWritableFileWithinRoot(params: { let handle: FileHandle; let createdForWrite = false; + const existingFlags = params.append ? OPEN_APPEND_EXISTING_FLAGS : OPEN_WRITE_EXISTING_FLAGS; + const createFlags = params.append ? OPEN_APPEND_CREATE_FLAGS : OPEN_WRITE_CREATE_FLAGS; try { try { - handle = await fs.open(ioPath, OPEN_WRITE_EXISTING_FLAGS, fileMode); + handle = await fs.open(ioPath, existingFlags, fileMode); } catch (err) { if (!isNotFoundPathError(err)) { throw err; } - handle = await fs.open(ioPath, OPEN_WRITE_CREATE_FLAGS, fileMode); + handle = await fs.open(ioPath, createFlags, fileMode); createdForWrite = true; } } catch (err) { @@ -469,7 +480,7 @@ export async function openWritableFileWithinRoot(params: { // Truncate only after boundary and identity checks complete. This avoids // irreversible side effects if a symlink target changes before validation. - if (params.truncateExisting !== false && !createdForWrite) { + if (params.append !== true && params.truncateExisting !== false && !createdForWrite) { await handle.truncate(0); } return { @@ -489,6 +500,50 @@ export async function openWritableFileWithinRoot(params: { } } +export async function appendFileWithinRoot(params: { + rootDir: string; + relativePath: string; + data: string | Buffer; + encoding?: BufferEncoding; + mkdir?: boolean; + prependNewlineIfNeeded?: boolean; +}): Promise { + const target = await openWritableFileWithinRoot({ + rootDir: params.rootDir, + relativePath: params.relativePath, + mkdir: params.mkdir, + truncateExisting: false, + append: true, + }); + try { + let prefix = ""; + if ( + params.prependNewlineIfNeeded === true && + !target.createdForWrite && + target.openedStat.size > 0 && + ((typeof params.data === "string" && !params.data.startsWith("\n")) || + (Buffer.isBuffer(params.data) && params.data.length > 0 && params.data[0] !== 0x0a)) + ) { + const lastByte = Buffer.alloc(1); + const { bytesRead } = await target.handle.read(lastByte, 0, 1, target.openedStat.size - 1); + if (bytesRead === 1 && lastByte[0] !== 0x0a) { + prefix = "\n"; + } + } + + if (typeof params.data === "string") { + await target.handle.appendFile(`${prefix}${params.data}`, params.encoding ?? "utf8"); + return; + } + + const payload = + prefix.length > 0 ? Buffer.concat([Buffer.from(prefix, "utf8"), params.data]) : params.data; + await target.handle.appendFile(payload); + } finally { + await target.handle.close().catch(() => {}); + } +} + export async function writeFileWithinRoot(params: { rootDir: string; relativePath: string; From da4fec664121b8ca443a3d72d19a6a1c9200204f Mon Sep 17 00:00:00 2001 From: Wayne <105773686+hougangdev@users.noreply.github.com> Date: Tue, 10 Mar 2026 12:47:39 +0800 Subject: [PATCH 007/126] fix(telegram): prevent duplicate messages when preview edit times out (#41662) Merged via squash. Prepared head SHA: 2780e62d070d7b4c4d7447e966ca172e33e44ad4 Co-authored-by: hougangdev <105773686+hougangdev@users.noreply.github.com> Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com> Reviewed-by: @obviyus --- CHANGELOG.md | 1 + src/telegram/bot-message-dispatch.test.ts | 204 +++++++++++++++++++ src/telegram/bot-message-dispatch.ts | 34 +++- src/telegram/lane-delivery-text-deliverer.ts | 158 ++++++++++---- src/telegram/lane-delivery.test.ts | 147 ++++++++++++- src/telegram/lane-delivery.ts | 1 + 6 files changed, 492 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f017b834209..e80e2c34ce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai - Tools/web search: treat Brave `llm-context` grounding snippets as plain strings so `web_search` no longer returns empty snippet arrays in LLM Context mode. (#41387) thanks @zheliu2. - Telegram/exec approvals: reject `/approve` commands aimed at other bots, keep deterministic approval prompts visible when tool-result delivery fails, and stop resolved exact IDs from matching other pending approvals by prefix. (#37233) Thanks @huntharo. - Control UI/Sessions: restore single-column session table collapse on narrow viewport or container widths by moving the responsive table override next to the base grid rule and enabling inline-size container queries. (#12175) Thanks @benjipeng. +- Telegram/final preview delivery: split active preview lifecycle from cleanup retention so missing archived preview edits avoid duplicate fallback sends without clearing the live preview or blocking later in-place finalization. (#41662) thanks @hougangdev. ## 2026.3.8 diff --git a/src/telegram/bot-message-dispatch.test.ts b/src/telegram/bot-message-dispatch.test.ts index 7caa7cc3af7..4f5e2484d50 100644 --- a/src/telegram/bot-message-dispatch.test.ts +++ b/src/telegram/bot-message-dispatch.test.ts @@ -906,6 +906,131 @@ describe("dispatchTelegramMessage draft streaming", () => { expect(deliverReplies).not.toHaveBeenCalled(); }); + it("keeps the active preview when an archived final edit target is missing", async () => { + let answerMessageId: number | undefined; + let answerDraftParams: + | { + onSupersededPreview?: (preview: { messageId: number; textSnapshot: string }) => void; + } + | undefined; + const answerDraftStream = { + update: vi.fn().mockImplementation((text: string) => { + if (text.includes("Message B")) { + answerMessageId = 1002; + } + }), + flush: vi.fn().mockResolvedValue(undefined), + messageId: vi.fn().mockImplementation(() => answerMessageId), + clear: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + forceNewMessage: vi.fn().mockImplementation(() => { + answerMessageId = undefined; + }), + }; + const reasoningDraftStream = createDraftStream(); + createTelegramDraftStream + .mockImplementationOnce((params) => { + answerDraftParams = params as typeof answerDraftParams; + return answerDraftStream; + }) + .mockImplementationOnce(() => reasoningDraftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Message A partial" }); + await replyOptions?.onAssistantMessageStart?.(); + await replyOptions?.onPartialReply?.({ text: "Message B partial" }); + answerDraftParams?.onSupersededPreview?.({ + messageId: 1001, + textSnapshot: "Message A partial", + }); + + await dispatcherOptions.deliver({ text: "Message A final" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + editMessageTelegram.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + await dispatchWithContext({ context: createContext(), streamMode: "partial" }); + + expect(editMessageTelegram).toHaveBeenCalledWith( + 123, + 1001, + "Message A final", + expect.any(Object), + ); + expect(answerDraftStream.clear).not.toHaveBeenCalled(); + expect(deliverReplies).not.toHaveBeenCalled(); + }); + + it("still finalizes the active preview after an archived final edit is retained", async () => { + let answerMessageId: number | undefined; + let answerDraftParams: + | { + onSupersededPreview?: (preview: { messageId: number; textSnapshot: string }) => void; + } + | undefined; + const answerDraftStream = { + update: vi.fn().mockImplementation((text: string) => { + if (text.includes("Message B")) { + answerMessageId = 1002; + } + }), + flush: vi.fn().mockResolvedValue(undefined), + messageId: vi.fn().mockImplementation(() => answerMessageId), + clear: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + forceNewMessage: vi.fn().mockImplementation(() => { + answerMessageId = undefined; + }), + }; + const reasoningDraftStream = createDraftStream(); + createTelegramDraftStream + .mockImplementationOnce((params) => { + answerDraftParams = params as typeof answerDraftParams; + return answerDraftStream; + }) + .mockImplementationOnce(() => reasoningDraftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Message A partial" }); + await replyOptions?.onAssistantMessageStart?.(); + await replyOptions?.onPartialReply?.({ text: "Message B partial" }); + answerDraftParams?.onSupersededPreview?.({ + messageId: 1001, + textSnapshot: "Message A partial", + }); + + await dispatcherOptions.deliver({ text: "Message A final" }, { kind: "final" }); + await dispatcherOptions.deliver({ text: "Message B final" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + editMessageTelegram + .mockRejectedValueOnce(new Error("400: Bad Request: message to edit not found")) + .mockResolvedValueOnce({ ok: true, chatId: "123", messageId: "1002" }); + + await dispatchWithContext({ context: createContext(), streamMode: "partial" }); + + expect(editMessageTelegram).toHaveBeenNthCalledWith( + 1, + 123, + 1001, + "Message A final", + expect.any(Object), + ); + expect(editMessageTelegram).toHaveBeenNthCalledWith( + 2, + 123, + 1002, + "Message B final", + expect.any(Object), + ); + expect(answerDraftStream.clear).not.toHaveBeenCalled(); + expect(deliverReplies).not.toHaveBeenCalled(); + }); + it.each(["partial", "block"] as const)( "keeps finalized text preview when the next assistant message is media-only (%s mode)", async (streamMode) => { @@ -1903,4 +2028,83 @@ describe("dispatchTelegramMessage draft streaming", () => { expect(draftA.clear).toHaveBeenCalledTimes(1); expect(draftB.clear).toHaveBeenCalledTimes(1); }); + + it("swallows post-connect network timeout on preview edit to prevent duplicate messages", async () => { + const draftStream = createDraftStream(999); + createTelegramDraftStream.mockReturnValue(draftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Streaming..." }); + await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + // Simulate a post-connect timeout: editMessageTelegram throws a network + // error even though Telegram's server already processed the edit. + editMessageTelegram.mockRejectedValue(new Error("timeout: request timed out after 30000ms")); + + await dispatchWithContext({ context: createContext() }); + + expect(editMessageTelegram).toHaveBeenCalledTimes(1); + const deliverCalls = deliverReplies.mock.calls; + const finalTextSentViaDeliverReplies = deliverCalls.some((call: unknown[]) => + (call[0] as { replies?: Array<{ text?: string }> })?.replies?.some( + (r: { text?: string }) => r.text === "Final answer", + ), + ); + expect(finalTextSentViaDeliverReplies).toBe(false); + }); + + it("falls back to sendPayload on pre-connect error during final edit", async () => { + const draftStream = createDraftStream(999); + createTelegramDraftStream.mockReturnValue(draftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Streaming..." }); + await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + const preConnectErr = new Error("connect ECONNREFUSED 149.154.167.220:443"); + (preConnectErr as NodeJS.ErrnoException).code = "ECONNREFUSED"; + editMessageTelegram.mockRejectedValue(preConnectErr); + + await dispatchWithContext({ context: createContext() }); + + expect(editMessageTelegram).toHaveBeenCalledTimes(1); + const deliverCalls = deliverReplies.mock.calls; + const finalTextSentViaDeliverReplies = deliverCalls.some((call: unknown[]) => + (call[0] as { replies?: Array<{ text?: string }> })?.replies?.some( + (r: { text?: string }) => r.text === "Final answer", + ), + ); + expect(finalTextSentViaDeliverReplies).toBe(true); + }); + + it("falls back when Telegram reports the current final edit target missing", async () => { + const draftStream = createDraftStream(999); + createTelegramDraftStream.mockReturnValue(draftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Streaming..." }); + await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + editMessageTelegram.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + await dispatchWithContext({ context: createContext() }); + + expect(editMessageTelegram).toHaveBeenCalledTimes(1); + const deliverCalls = deliverReplies.mock.calls; + const finalTextSentViaDeliverReplies = deliverCalls.some((call: unknown[]) => + (call[0] as { replies?: Array<{ text?: string }> })?.replies?.some( + (r: { text?: string }) => r.text === "Final answer", + ), + ); + expect(finalTextSentViaDeliverReplies).toBe(true); + }); }); diff --git a/src/telegram/bot-message-dispatch.ts b/src/telegram/bot-message-dispatch.ts index fee56211ae5..4d8d2b678e8 100644 --- a/src/telegram/bot-message-dispatch.ts +++ b/src/telegram/bot-message-dispatch.ts @@ -38,6 +38,7 @@ import { createLaneTextDeliverer, type DraftLaneState, type LaneName, + type LanePreviewLifecycle, } from "./lane-delivery.js"; import { createTelegramReasoningStepState, @@ -239,7 +240,14 @@ export const dispatchTelegramMessage = async ({ answer: createDraftLane("answer", canStreamAnswerDraft), reasoning: createDraftLane("reasoning", canStreamReasoningDraft), }; - const finalizedPreviewByLane: Record = { + // Active preview lifecycle answers "can this current preview still be + // finalized?" Cleanup retention is separate so archived-preview decisions do + // not poison the active lane. + const activePreviewLifecycleByLane: Record = { + answer: "transient", + reasoning: "transient", + }; + const retainPreviewOnCleanupByLane: Record = { answer: false, reasoning: false, }; @@ -288,7 +296,10 @@ export const dispatchTelegramMessage = async ({ // so it remains visible across tool boundaries. const materializedId = await answerLane.stream?.materialize?.(); const previewMessageId = materializedId ?? answerLane.stream?.messageId(); - if (typeof previewMessageId === "number" && !finalizedPreviewByLane.answer) { + if ( + typeof previewMessageId === "number" && + activePreviewLifecycleByLane.answer === "transient" + ) { archivedAnswerPreviews.push({ messageId: previewMessageId, textSnapshot: answerLane.lastPartialText, @@ -301,7 +312,8 @@ export const dispatchTelegramMessage = async ({ resetDraftLaneState(answerLane); if (didForceNewMessage) { // New assistant message boundary: this lane now tracks a fresh preview lifecycle. - finalizedPreviewByLane.answer = false; + activePreviewLifecycleByLane.answer = "transient"; + retainPreviewOnCleanupByLane.answer = false; } return didForceNewMessage; }; @@ -331,7 +343,7 @@ export const dispatchTelegramMessage = async ({ const ingestDraftLaneSegments = async (text: string | undefined) => { const split = splitTextIntoLaneSegments(text); const hasAnswerSegment = split.segments.some((segment) => segment.lane === "answer"); - if (hasAnswerSegment && finalizedPreviewByLane.answer) { + if (hasAnswerSegment && activePreviewLifecycleByLane.answer !== "transient") { // Some providers can emit the first partial of a new assistant message before // onAssistantMessageStart() arrives. Rotate preemptively so we do not edit // the previously finalized preview message with the next message's text. @@ -469,7 +481,8 @@ export const dispatchTelegramMessage = async ({ const deliverLaneText = createLaneTextDeliverer({ lanes, archivedAnswerPreviews, - finalizedPreviewByLane, + activePreviewLifecycleByLane, + retainPreviewOnCleanupByLane, draftMaxChars, applyTextToPayload, sendPayload, @@ -596,7 +609,8 @@ export const dispatchTelegramMessage = async ({ } if (info.kind === "final") { if (reasoningLane.hasStreamedMessage) { - finalizedPreviewByLane.reasoning = true; + activePreviewLifecycleByLane.reasoning = "complete"; + retainPreviewOnCleanupByLane.reasoning = true; } reasoningStepState.resetForNextStep(); } @@ -674,7 +688,8 @@ export const dispatchTelegramMessage = async ({ reasoningStepState.resetForNextStep(); if (skipNextAnswerMessageStartRotation) { skipNextAnswerMessageStartRotation = false; - finalizedPreviewByLane.answer = false; + activePreviewLifecycleByLane.answer = "transient"; + retainPreviewOnCleanupByLane.answer = false; return; } await rotateAnswerLaneForNewAssistantMessage(); @@ -682,7 +697,8 @@ export const dispatchTelegramMessage = async ({ // Even when no forceNewMessage happened (e.g. prior answer had no // streamed partials), the next partial belongs to a fresh lifecycle // and must not trigger late pre-rotation mid-message. - finalizedPreviewByLane.answer = false; + activePreviewLifecycleByLane.answer = "transient"; + retainPreviewOnCleanupByLane.answer = false; }) : undefined, onReasoningEnd: reasoningLane.stream @@ -731,7 +747,7 @@ export const dispatchTelegramMessage = async ({ (p) => p.deleteIfUnused === false && p.messageId === activePreviewMessageId, ); const shouldClear = - !finalizedPreviewByLane[laneState.laneName] && !hasBoundaryFinalizedActivePreview; + !retainPreviewOnCleanupByLane[laneState.laneName] && !hasBoundaryFinalizedActivePreview; const existing = streamCleanupStates.get(stream); if (!existing) { streamCleanupStates.set(stream, { shouldClear }); diff --git a/src/telegram/lane-delivery-text-deliverer.ts b/src/telegram/lane-delivery-text-deliverer.ts index f244d086657..c8eb10a9bb1 100644 --- a/src/telegram/lane-delivery-text-deliverer.ts +++ b/src/telegram/lane-delivery-text-deliverer.ts @@ -1,22 +1,36 @@ import type { ReplyPayload } from "../auto-reply/types.js"; import type { TelegramInlineButtons } from "./button-types.js"; import type { TelegramDraftStream } from "./draft-stream.js"; +import { isRecoverableTelegramNetworkError, isSafeToRetrySendError } from "./network-errors.js"; const MESSAGE_NOT_MODIFIED_RE = /400:\s*Bad Request:\s*message is not modified|MESSAGE_NOT_MODIFIED/i; +const MESSAGE_NOT_FOUND_RE = + /400:\s*Bad Request:\s*message to edit not found|MESSAGE_ID_INVALID|message can't be edited/i; + +function extractErrorText(err: unknown): string { + return typeof err === "string" + ? err + : err instanceof Error + ? err.message + : typeof err === "object" && err && "description" in err + ? typeof err.description === "string" + ? err.description + : "" + : ""; +} function isMessageNotModifiedError(err: unknown): boolean { - const text = - typeof err === "string" - ? err - : err instanceof Error - ? err.message - : typeof err === "object" && err && "description" in err - ? typeof err.description === "string" - ? err.description - : "" - : ""; - return MESSAGE_NOT_MODIFIED_RE.test(text); + return MESSAGE_NOT_MODIFIED_RE.test(extractErrorText(err)); +} + +/** + * Returns true when Telegram rejects an edit because the target message can no + * longer be resolved or edited. The caller still needs preview context to + * decide whether to retain a different visible preview or fall back to send. + */ +function isMissingPreviewMessageError(err: unknown): boolean { + return MESSAGE_NOT_FOUND_RE.test(extractErrorText(err)); } export type LaneName = "answer" | "reasoning"; @@ -35,12 +49,20 @@ export type ArchivedPreview = { deleteIfUnused?: boolean; }; -export type LaneDeliveryResult = "preview-finalized" | "preview-updated" | "sent" | "skipped"; +export type LanePreviewLifecycle = "transient" | "complete"; + +export type LaneDeliveryResult = + | "preview-finalized" + | "preview-retained" + | "preview-updated" + | "sent" + | "skipped"; type CreateLaneTextDelivererParams = { lanes: Record; archivedAnswerPreviews: ArchivedPreview[]; - finalizedPreviewByLane: Record; + activePreviewLifecycleByLane: Record; + retainPreviewOnCleanupByLane: Record; draftMaxChars: number; applyTextToPayload: (payload: ReplyPayload, text: string) => ReplyPayload; sendPayload: (payload: ReplyPayload) => Promise; @@ -80,6 +102,8 @@ type TryUpdatePreviewParams = { previewTextSnapshot?: string; }; +type PreviewEditResult = "edited" | "retained" | "fallback"; + type ConsumeArchivedAnswerPreviewParams = { lane: DraftLaneState; text: string; @@ -139,6 +163,10 @@ function resolvePreviewTarget(params: ResolvePreviewTargetParams): PreviewTarget export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { const getLanePreviewText = (lane: DraftLaneState) => lane.lastPartialText; + const markActivePreviewComplete = (laneName: LaneName) => { + params.activePreviewLifecycleByLane[laneName] = "complete"; + params.retainPreviewOnCleanupByLane[laneName] = true; + }; const isDraftPreviewLane = (lane: DraftLaneState) => lane.stream?.previewMode?.() === "draft"; const canMaterializeDraftFinal = ( lane: DraftLaneState, @@ -184,8 +212,9 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { previewButtons?: TelegramInlineButtons; updateLaneSnapshot: boolean; lane: DraftLaneState; - treatEditFailureAsDelivered: boolean; - }): Promise => { + finalTextAlreadyLanded: boolean; + retainAlternatePreviewOnMissingTarget: boolean; + }): Promise => { try { await params.editPreview({ laneName: args.laneName, @@ -198,26 +227,58 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { args.lane.lastPartialText = args.text; } params.markDelivered(); - return true; + return "edited"; } catch (err) { if (isMessageNotModifiedError(err)) { params.log( `telegram: ${args.laneName} preview ${args.context} edit returned "message is not modified"; treating as delivered`, ); params.markDelivered(); - return true; + return "edited"; } - if (args.treatEditFailureAsDelivered) { + if (args.context === "final") { + if (args.finalTextAlreadyLanded) { + params.log( + `telegram: ${args.laneName} preview final edit failed after stop flush; keeping existing preview (${String(err)})`, + ); + params.markDelivered(); + return "retained"; + } + if (isSafeToRetrySendError(err)) { + params.log( + `telegram: ${args.laneName} preview final edit failed before reaching Telegram; falling back to standard send (${String(err)})`, + ); + return "fallback"; + } + if (isMissingPreviewMessageError(err)) { + if (args.retainAlternatePreviewOnMissingTarget) { + params.log( + `telegram: ${args.laneName} preview final edit target missing; keeping alternate preview without fallback (${String(err)})`, + ); + params.markDelivered(); + return "retained"; + } + params.log( + `telegram: ${args.laneName} preview final edit target missing with no alternate preview; falling back to standard send (${String(err)})`, + ); + return "fallback"; + } + if (isRecoverableTelegramNetworkError(err, { allowMessageMatch: true })) { + params.log( + `telegram: ${args.laneName} preview final edit may have landed despite network error; keeping existing preview (${String(err)})`, + ); + params.markDelivered(); + return "retained"; + } params.log( - `telegram: ${args.laneName} preview ${args.context} edit failed after stop-created flush; treating as delivered (${String(err)})`, + `telegram: ${args.laneName} preview final edit rejected by Telegram; falling back to standard send (${String(err)})`, ); - params.markDelivered(); - return true; + return "fallback"; } params.log( `telegram: ${args.laneName} preview ${args.context} edit failed; falling back to standard send (${String(err)})`, ); - return false; + return "fallback"; } }; @@ -232,8 +293,12 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { context, previewMessageId: previewMessageIdOverride, previewTextSnapshot, - }: TryUpdatePreviewParams): Promise => { - const editPreview = (messageId: number, treatEditFailureAsDelivered: boolean) => + }: TryUpdatePreviewParams): Promise => { + const editPreview = ( + messageId: number, + finalTextAlreadyLanded: boolean, + retainAlternatePreviewOnMissingTarget: boolean, + ) => tryEditPreviewMessage({ laneName, messageId, @@ -242,13 +307,15 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { previewButtons, updateLaneSnapshot, lane, - treatEditFailureAsDelivered, + finalTextAlreadyLanded, + retainAlternatePreviewOnMissingTarget, }); const finalizePreview = ( previewMessageId: number, - treatEditFailureAsDelivered: boolean, + finalTextAlreadyLanded: boolean, hadPreviewMessage: boolean, - ): boolean | Promise => { + retainAlternatePreviewOnMissingTarget = false, + ): PreviewEditResult | Promise => { const currentPreviewText = previewTextSnapshot ?? getLanePreviewText(lane); const shouldSkipRegressive = shouldSkipRegressivePreviewUpdate({ currentPreviewText, @@ -258,12 +325,16 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { }); if (shouldSkipRegressive) { params.markDelivered(); - return true; + return "edited"; } - return editPreview(previewMessageId, treatEditFailureAsDelivered); + return editPreview( + previewMessageId, + finalTextAlreadyLanded, + retainAlternatePreviewOnMissingTarget, + ); }; if (!lane.stream) { - return false; + return "fallback"; } const previewTargetBeforeStop = resolvePreviewTarget({ lane, @@ -282,7 +353,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { context, }); if (typeof previewTargetAfterStop.previewMessageId !== "number") { - return false; + return "fallback"; } return finalizePreview(previewTargetAfterStop.previewMessageId, true, false); } @@ -296,12 +367,15 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { context, }); if (typeof previewTargetAfterStop.previewMessageId !== "number") { - return false; + return "fallback"; } + const activePreviewMessageId = lane.stream?.messageId(); return finalizePreview( previewTargetAfterStop.previewMessageId, false, previewTargetAfterStop.hadPreviewMessage, + typeof activePreviewMessageId === "number" && + activePreviewMessageId !== previewTargetAfterStop.previewMessageId, ); }; @@ -328,9 +402,13 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { previewMessageId: archivedPreview.messageId, previewTextSnapshot: archivedPreview.textSnapshot, }); - if (finalized) { + if (finalized === "edited") { return "preview-finalized"; } + if (finalized === "retained") { + params.retainPreviewOnCleanupByLane.answer = true; + return "preview-retained"; + } } // Send the replacement message first, then clean up the old preview. // This avoids the visual "disappear then reappear" flash. @@ -375,7 +453,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { return archivedResult; } } - if (canEditViaPreview && !params.finalizedPreviewByLane[laneName]) { + if (canEditViaPreview && params.activePreviewLifecycleByLane[laneName] === "transient") { await params.flushDraftLane(lane); if (laneName === "answer") { const archivedResultAfterFlush = await consumeArchivedAnswerPreviewForFinal({ @@ -396,7 +474,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { text, }); if (materialized) { - params.finalizedPreviewByLane[laneName] = true; + markActivePreviewComplete(laneName); return "preview-finalized"; } } @@ -409,10 +487,14 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { skipRegressive: "existingOnly", context: "final", }); - if (finalized) { - params.finalizedPreviewByLane[laneName] = true; + if (finalized === "edited") { + markActivePreviewComplete(laneName); return "preview-finalized"; } + if (finalized === "retained") { + markActivePreviewComplete(laneName); + return "preview-retained"; + } } else if (!hasMedia && !payload.isError && text.length > params.draftMaxChars) { params.log( `telegram: preview final too long for edit (${text.length} > ${params.draftMaxChars}); falling back to standard send`, @@ -452,7 +534,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { skipRegressive: "always", context: "update", }); - if (updated) { + if (updated === "edited") { return "preview-updated"; } } diff --git a/src/telegram/lane-delivery.test.ts b/src/telegram/lane-delivery.test.ts index 1cd1d36cf4c..a2dae1f05b9 100644 --- a/src/telegram/lane-delivery.test.ts +++ b/src/telegram/lane-delivery.test.ts @@ -42,7 +42,8 @@ function createHarness(params?: { const deletePreviewMessage = vi.fn().mockResolvedValue(undefined); const log = vi.fn(); const markDelivered = vi.fn(); - const finalizedPreviewByLane: Record = { answer: false, reasoning: false }; + const activePreviewLifecycleByLane = { answer: "transient", reasoning: "transient" } as const; + const retainPreviewOnCleanupByLane = { answer: false, reasoning: false } as const; const archivedAnswerPreviews: Array<{ messageId: number; textSnapshot: string; @@ -52,7 +53,8 @@ function createHarness(params?: { const deliverLaneText = createLaneTextDeliverer({ lanes, archivedAnswerPreviews, - finalizedPreviewByLane, + activePreviewLifecycleByLane: { ...activePreviewLifecycleByLane }, + retainPreviewOnCleanupByLane: { ...retainPreviewOnCleanupByLane }, draftMaxChars: params?.draftMaxChars ?? 4_096, applyTextToPayload: (payload: ReplyPayload, text: string) => ({ ...payload, text }), sendPayload, @@ -129,7 +131,7 @@ describe("createLaneTextDeliverer", () => { expect(harness.sendPayload).not.toHaveBeenCalled(); }); - it("treats stop-created preview edit failures as delivered", async () => { + it("keeps stop-created preview when follow-up final edit fails", async () => { const harness = createHarness({ answerMessageIdAfterStop: 777 }); harness.editPreview.mockRejectedValue(new Error("500: edit failed after stop flush")); @@ -140,10 +142,12 @@ describe("createLaneTextDeliverer", () => { infoKind: "final", }); - expect(result).toBe("preview-finalized"); + expect(result).toBe("preview-retained"); expect(harness.editPreview).toHaveBeenCalledTimes(1); expect(harness.sendPayload).not.toHaveBeenCalled(); - expect(harness.log).toHaveBeenCalledWith(expect.stringContaining("treating as delivered")); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("failed after stop flush; keeping existing preview"), + ); }); it("treats 'message is not modified' preview edit errors as delivered", async () => { @@ -170,7 +174,7 @@ describe("createLaneTextDeliverer", () => { ); }); - it("falls back to normal delivery when editing an existing preview fails", async () => { + it("falls back to sendPayload when an existing preview final edit is rejected", async () => { const harness = createHarness({ answerMessageId: 999 }); harness.editPreview.mockRejectedValue(new Error("500: preview edit failed")); @@ -186,6 +190,69 @@ describe("createLaneTextDeliverer", () => { expect(harness.sendPayload).toHaveBeenCalledWith( expect.objectContaining({ text: "Hello final" }), ); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("edit rejected by Telegram; falling back"), + ); + }); + + it("falls back when Telegram reports the current final edit target missing", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.editPreview.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("sent"); + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).toHaveBeenCalledWith( + expect.objectContaining({ text: "Hello final" }), + ); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("edit target missing with no alternate preview; falling back"), + ); + }); + + it("falls back to sendPayload when the final edit fails before reaching Telegram", async () => { + const harness = createHarness({ answerMessageId: 999 }); + const err = Object.assign(new Error("connect ECONNREFUSED"), { code: "ECONNREFUSED" }); + harness.editPreview.mockRejectedValue(err); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("sent"); + expect(harness.sendPayload).toHaveBeenCalledWith( + expect.objectContaining({ text: "Hello final" }), + ); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("failed before reaching Telegram; falling back"), + ); + }); + + it("keeps preview when the final edit times out after the request may have landed", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.editPreview.mockRejectedValue(new Error("timeout: request timed out after 30000ms")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("preview-retained"); + expect(harness.sendPayload).not.toHaveBeenCalled(); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("may have landed despite network error; keeping existing preview"), + ); }); it("falls back to normal delivery when stop-created preview has no message id", async () => { @@ -362,6 +429,74 @@ describe("createLaneTextDeliverer", () => { expect(harness.markDelivered).not.toHaveBeenCalled(); }); + // ── Duplicate message regression tests ────────────────────────────────── + // During final delivery, only ambiguous post-connect failures keep the + // preview. Definite non-delivery falls back to a real send. + + it("falls back on API error during final", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.editPreview.mockRejectedValue(new Error("500: Internal Server Error")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("sent"); + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).toHaveBeenCalledTimes(1); + }); + + it("falls back when an archived preview edit target is missing and no alternate preview exists", async () => { + const harness = createHarness(); + harness.archivedAnswerPreviews.push({ + messageId: 5555, + textSnapshot: "Partial streaming...", + deleteIfUnused: true, + }); + harness.editPreview.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Complete final answer", + payload: { text: "Complete final answer" }, + infoKind: "final", + }); + + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).toHaveBeenCalledWith( + expect.objectContaining({ text: "Complete final answer" }), + ); + expect(result).toBe("sent"); + expect(harness.deletePreviewMessage).toHaveBeenCalledWith(5555); + }); + + it("keeps the active preview when an archived final edit target is missing", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.archivedAnswerPreviews.push({ + messageId: 5555, + textSnapshot: "Partial streaming...", + deleteIfUnused: true, + }); + harness.editPreview.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Complete final answer", + payload: { text: "Complete final answer" }, + infoKind: "final", + }); + + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).not.toHaveBeenCalled(); + expect(result).toBe("preview-retained"); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("edit target missing; keeping alternate preview without fallback"), + ); + }); + it("deletes consumed boundary previews after fallback final send", async () => { const harness = createHarness(); harness.archivedAnswerPreviews.push({ diff --git a/src/telegram/lane-delivery.ts b/src/telegram/lane-delivery.ts index 213b05e1158..a9114b281ff 100644 --- a/src/telegram/lane-delivery.ts +++ b/src/telegram/lane-delivery.ts @@ -4,6 +4,7 @@ export { type DraftLaneState, type LaneDeliveryResult, type LaneName, + type LanePreviewLifecycle, } from "./lane-delivery-text-deliverer.js"; export { createLaneDeliveryStateTracker, From 382287026b55e787d28f19d762380344c9f4408d Mon Sep 17 00:00:00 2001 From: futuremind2026 Date: Tue, 10 Mar 2026 13:01:45 +0800 Subject: [PATCH 008/126] cron: record lastErrorReason in job state (#14382) Merged via squash. Prepared head SHA: baa6b5d566a41950dea0a214881eef48697326d8 Co-authored-by: futuremind2026 <258860756+futuremind2026@users.noreply.github.com> Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com> Reviewed-by: @BunsDev --- CHANGELOG.md | 1 + src/cron/cron-protocol-conformance.test.ts | 27 +++++++++++++++++++++- src/cron/service/timer.ts | 6 ++++- src/cron/types.ts | 4 +++- src/gateway/protocol/schema/cron.ts | 10 ++++++++ 5 files changed, 45 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e80e2c34ce4..6bc7bf6f07f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Docs: https://docs.openclaw.ai - Telegram/exec approvals: reject `/approve` commands aimed at other bots, keep deterministic approval prompts visible when tool-result delivery fails, and stop resolved exact IDs from matching other pending approvals by prefix. (#37233) Thanks @huntharo. - Control UI/Sessions: restore single-column session table collapse on narrow viewport or container widths by moving the responsive table override next to the base grid rule and enabling inline-size container queries. (#12175) Thanks @benjipeng. - Telegram/final preview delivery: split active preview lifecycle from cleanup retention so missing archived preview edits avoid duplicate fallback sends without clearing the live preview or blocking later in-place finalization. (#41662) thanks @hougangdev. +- Cron/state errors: record `lastErrorReason` in cron job state and keep the gateway schema aligned with the full failover-reason set, including regression coverage for protocol conformance. (#14382) thanks @futuremind2026. ## 2026.3.8 diff --git a/src/cron/cron-protocol-conformance.test.ts b/src/cron/cron-protocol-conformance.test.ts index 51fe8f4767c..698f5e0038d 100644 --- a/src/cron/cron-protocol-conformance.test.ts +++ b/src/cron/cron-protocol-conformance.test.ts @@ -2,7 +2,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { describe, expect, it } from "vitest"; import { MACOS_APP_SOURCES_DIR } from "../compat/legacy-names.js"; -import { CronDeliverySchema } from "../gateway/protocol/schema.js"; +import { CronDeliverySchema, CronJobStateSchema } from "../gateway/protocol/schema.js"; type SchemaLike = { anyOf?: Array; @@ -29,6 +29,16 @@ function extractDeliveryModes(schema: SchemaLike): string[] { return Array.from(new Set(unionModes)); } +function extractConstUnionValues(schema: SchemaLike): string[] { + return Array.from( + new Set( + (schema.anyOf ?? []) + .map((entry) => entry?.const) + .filter((value): value is string => typeof value === "string"), + ), + ); +} + const UI_FILES = ["ui/src/ui/types.ts", "ui/src/ui/ui-types.ts", "ui/src/ui/views/cron.ts"]; const SWIFT_MODEL_CANDIDATES = [`${MACOS_APP_SOURCES_DIR}/CronModels.swift`]; @@ -88,4 +98,19 @@ describe("cron protocol conformance", () => { expect(swift.includes("struct CronSchedulerStatus")).toBe(true); expect(swift.includes("let jobs:")).toBe(true); }); + + it("cron job state schema keeps the full failover reason set", () => { + const properties = (CronJobStateSchema as SchemaLike).properties ?? {}; + const lastErrorReason = properties.lastErrorReason as SchemaLike | undefined; + expect(lastErrorReason).toBeDefined(); + expect(extractConstUnionValues(lastErrorReason ?? {})).toEqual([ + "auth", + "format", + "rate_limit", + "billing", + "timeout", + "model_not_found", + "unknown", + ]); + }); }); diff --git a/src/cron/service/timer.ts b/src/cron/service/timer.ts index 5320ffdf526..e12c4ae38e7 100644 --- a/src/cron/service/timer.ts +++ b/src/cron/service/timer.ts @@ -1,3 +1,4 @@ +import { resolveFailoverReasonFromError } from "../../agents/failover-error.js"; import type { CronConfig, CronRetryOn } from "../../config/types.cron.js"; import type { HeartbeatRunResult } from "../../infra/heartbeat-wake.js"; import { DEFAULT_AGENT_ID } from "../../routing/session-key.js"; @@ -322,6 +323,10 @@ export function applyJobResult( job.state.lastStatus = result.status; job.state.lastDurationMs = Math.max(0, result.endedAt - result.startedAt); job.state.lastError = result.error; + job.state.lastErrorReason = + result.status === "error" && typeof result.error === "string" + ? (resolveFailoverReasonFromError(result.error) ?? undefined) + : undefined; job.state.lastDelivered = result.delivered; const deliveryStatus = resolveDeliveryStatus({ job, delivered: result.delivered }); job.state.lastDeliveryStatus = deliveryStatus; @@ -670,7 +675,6 @@ export async function onTimer(state: CronServiceState) { if (completedResults.length > 0) { await locked(state, async () => { await ensureLoaded(state, { forceReload: true, skipRecompute: true }); - for (const result of completedResults) { applyOutcomeToStoredJob(state, result); } diff --git a/src/cron/types.ts b/src/cron/types.ts index ef5de924b02..2a93bc30311 100644 --- a/src/cron/types.ts +++ b/src/cron/types.ts @@ -1,3 +1,4 @@ +import type { FailoverReason } from "../agents/pi-embedded-helpers.js"; import type { ChannelId } from "../channels/plugins/types.js"; import type { CronJobBase } from "./types-shared.js"; @@ -105,7 +106,6 @@ type CronAgentTurnPayload = { type CronAgentTurnPayloadPatch = { kind: "agentTurn"; } & Partial; - export type CronJobState = { nextRunAtMs?: number; runningAtMs?: number; @@ -115,6 +115,8 @@ export type CronJobState = { /** Back-compat alias for lastRunStatus. */ lastStatus?: "ok" | "error" | "skipped"; lastError?: string; + /** Classified reason for the last error (when available). */ + lastErrorReason?: FailoverReason; lastDurationMs?: number; /** Number of consecutive execution errors (reset on success). Used for backoff. */ consecutiveErrors?: number; diff --git a/src/gateway/protocol/schema/cron.ts b/src/gateway/protocol/schema/cron.ts index 41e7467bece..3cba5a65781 100644 --- a/src/gateway/protocol/schema/cron.ts +++ b/src/gateway/protocol/schema/cron.ts @@ -56,6 +56,15 @@ const CronDeliveryStatusSchema = Type.Union([ Type.Literal("unknown"), Type.Literal("not-requested"), ]); +const CronFailoverReasonSchema = Type.Union([ + Type.Literal("auth"), + Type.Literal("format"), + Type.Literal("rate_limit"), + Type.Literal("billing"), + Type.Literal("timeout"), + Type.Literal("model_not_found"), + Type.Literal("unknown"), +]); const CronCommonOptionalFields = { agentId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), sessionKey: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), @@ -219,6 +228,7 @@ export const CronJobStateSchema = Type.Object( lastRunStatus: Type.Optional(CronRunStatusSchema), lastStatus: Type.Optional(CronRunStatusSchema), lastError: Type.Optional(Type.String()), + lastErrorReason: Type.Optional(CronFailoverReasonSchema), lastDurationMs: Type.Optional(Type.Integer({ minimum: 0 })), consecutiveErrors: Type.Optional(Type.Integer({ minimum: 0 })), lastDelivered: Type.Optional(Type.Boolean()), From cf9db91b611c79e71281f226a401e51931d6643b Mon Sep 17 00:00:00 2001 From: Laurie Luo Date: Tue, 10 Mar 2026 13:07:44 +0800 Subject: [PATCH 009/126] fix(web-search): recover OpenRouter Perplexity citations from message annotations (#40881) Merged via squash. Prepared head SHA: 66c8bb2c6a4bbc95a5d23661c185f1e551c2929e Co-authored-by: laurieluo <89195476+laurieluo@users.noreply.github.com> Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com> Reviewed-by: @obviyus --- CHANGELOG.md | 1 + src/agents/tools/web-search.ts | 45 ++++++++++++++++- .../tools/web-tools.enabled-defaults.test.ts | 48 +++++++++++++++++-- 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc7bf6f07f..1ba832e4692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Docs: https://docs.openclaw.ai - Control UI/Sessions: restore single-column session table collapse on narrow viewport or container widths by moving the responsive table override next to the base grid rule and enabling inline-size container queries. (#12175) Thanks @benjipeng. - Telegram/final preview delivery: split active preview lifecycle from cleanup retention so missing archived preview edits avoid duplicate fallback sends without clearing the live preview or blocking later in-place finalization. (#41662) thanks @hougangdev. - Cron/state errors: record `lastErrorReason` in cron job state and keep the gateway schema aligned with the full failover-reason set, including regression coverage for protocol conformance. (#14382) thanks @futuremind2026. +- Tools/web search: recover OpenRouter Perplexity citation extraction from `message.annotations` when chat-completions responses omit top-level citations. (#40881) Thanks @laurieluo. ## 2026.3.8 diff --git a/src/agents/tools/web-search.ts b/src/agents/tools/web-search.ts index 4fbbfa95e43..6e9518f1ede 100644 --- a/src/agents/tools/web-search.ts +++ b/src/agents/tools/web-search.ts @@ -396,6 +396,16 @@ type PerplexitySearchResponse = { choices?: Array<{ message?: { content?: string; + annotations?: Array<{ + type?: string; + url?: string; + url_citation?: { + url?: string; + title?: string; + start_index?: number; + end_index?: number; + }; + }>; }; }>; citations?: string[]; @@ -414,6 +424,38 @@ type PerplexitySearchApiResponse = { id?: string; }; +function extractPerplexityCitations(data: PerplexitySearchResponse): string[] { + const normalizeUrl = (value: unknown): string | undefined => { + if (typeof value !== "string") { + return undefined; + } + const trimmed = value.trim(); + return trimmed ? trimmed : undefined; + }; + + const topLevel = (data.citations ?? []) + .map(normalizeUrl) + .filter((url): url is string => Boolean(url)); + if (topLevel.length > 0) { + return [...new Set(topLevel)]; + } + + const citations: string[] = []; + for (const choice of data.choices ?? []) { + for (const annotation of choice.message?.annotations ?? []) { + if (annotation.type !== "url_citation") { + continue; + } + const url = normalizeUrl(annotation.url_citation?.url ?? annotation.url); + if (url) { + citations.push(url); + } + } + } + + return [...new Set(citations)]; +} + function extractGrokContent(data: GrokSearchResponse): { text: string | undefined; annotationCitations: string[]; @@ -1252,7 +1294,8 @@ async function runPerplexitySearch(params: { const data = (await res.json()) as PerplexitySearchResponse; const content = data.choices?.[0]?.message?.content ?? "No response"; - const citations = data.citations ?? []; + // Prefer top-level citations; fall back to OpenRouter-style message annotations. + const citations = extractPerplexityCitations(data); return { content, citations }; }, diff --git a/src/agents/tools/web-tools.enabled-defaults.test.ts b/src/agents/tools/web-tools.enabled-defaults.test.ts index 4951f1c6b5a..ad3345a3e06 100644 --- a/src/agents/tools/web-tools.enabled-defaults.test.ts +++ b/src/agents/tools/web-tools.enabled-defaults.test.ts @@ -113,11 +113,13 @@ function installPerplexitySearchApiFetch(results?: Array }); } -function installPerplexityChatFetch() { - return installMockFetch({ - choices: [{ message: { content: "ok" } }], - citations: ["https://example.com"], - }); +function installPerplexityChatFetch(payload?: Record) { + return installMockFetch( + payload ?? { + choices: [{ message: { content: "ok" } }], + citations: ["https://example.com"], + }, + ); } function createProviderSuccessPayload( @@ -509,6 +511,42 @@ describe("web_search perplexity OpenRouter compatibility", () => { expect(body.search_recency_filter).toBe("week"); }); + it("falls back to message annotations when top-level citations are missing", async () => { + vi.stubEnv("OPENROUTER_API_KEY", "sk-or-v1-test"); // pragma: allowlist secret + const mockFetch = installPerplexityChatFetch({ + choices: [ + { + message: { + content: "ok", + annotations: [ + { + type: "url_citation", + url_citation: { url: "https://example.com/a" }, + }, + { + type: "url_citation", + url_citation: { url: "https://example.com/b" }, + }, + { + type: "url_citation", + url_citation: { url: "https://example.com/a" }, + }, + ], + }, + }, + ], + }); + const tool = createPerplexitySearchTool(); + const result = await tool?.execute?.("call-1", { query: "test" }); + + expect(mockFetch).toHaveBeenCalled(); + expect(result?.details).toMatchObject({ + provider: "perplexity", + citations: ["https://example.com/a", "https://example.com/b"], + content: expect.stringContaining("ok"), + }); + }); + it("fails loud for Search API-only filters on the compatibility path", async () => { vi.stubEnv("OPENROUTER_API_KEY", "sk-or-v1-test"); // pragma: allowlist secret const mockFetch = installPerplexityChatFetch(); From d1a59557b517a93ac40b1892e541d383a604ab83 Mon Sep 17 00:00:00 2001 From: Urian Paul Danut Date: Tue, 10 Mar 2026 05:54:23 +0000 Subject: [PATCH 010/126] fix(security): harden replaceMarkers() to catch space/underscore boundary marker variants (#35983) Merged via squash. Prepared head SHA: ff07dc45a9c9665c0a88c9898684a5c97f76473b Co-authored-by: urianpaul94 <33277984+urianpaul94@users.noreply.github.com> Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com> Reviewed-by: @frankekn --- CHANGELOG.md | 1 + src/security/external-content.test.ts | 16 ++++++++++++++++ src/security/external-content.ts | 12 ++++++++---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ba832e4692..2db4805cee0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Docs: https://docs.openclaw.ai - Telegram/final preview delivery: split active preview lifecycle from cleanup retention so missing archived preview edits avoid duplicate fallback sends without clearing the live preview or blocking later in-place finalization. (#41662) thanks @hougangdev. - Cron/state errors: record `lastErrorReason` in cron job state and keep the gateway schema aligned with the full failover-reason set, including regression coverage for protocol conformance. (#14382) thanks @futuremind2026. - Tools/web search: recover OpenRouter Perplexity citation extraction from `message.annotations` when chat-completions responses omit top-level citations. (#40881) Thanks @laurieluo. +- Security/external content: treat whitespace-delimited `EXTERNAL UNTRUSTED CONTENT` boundary markers like underscore-delimited variants so prompt wrappers cannot bypass marker sanitization. (#35983) Thanks @urianpaul94. ## 2026.3.8 diff --git a/src/security/external-content.test.ts b/src/security/external-content.test.ts index 17076b642b1..b943bdacf72 100644 --- a/src/security/external-content.test.ts +++ b/src/security/external-content.test.ts @@ -138,6 +138,21 @@ describe("external-content security", () => { content: "Before <<>> middle <<>> after", }, + { + name: "sanitizes space-separated boundary markers", + content: + "Before <<>> middle <<>> after", + }, + { + name: "sanitizes mixed space/underscore boundary markers", + content: + "Before <<>> middle <<>> after", + }, + { + name: "sanitizes tab-delimited boundary markers", + content: + "Before <<>> middle <<>> after", + }, ])("$name", ({ content }) => { const result = wrapExternalContent(content, { source: "email" }); expectSanitizedBoundaryMarkers(result); @@ -204,6 +219,7 @@ describe("external-content security", () => { ["\u27EE", "\u27EF"], // flattened parentheses ["\u276C", "\u276D"], // medium angle bracket ornaments ["\u276E", "\u276F"], // heavy angle quotation ornaments + ["\u02C2", "\u02C3"], // modifier letter left/right arrowhead ]; for (const [left, right] of bracketPairs) { diff --git a/src/security/external-content.ts b/src/security/external-content.ts index 60f92584108..ff571871b5e 100644 --- a/src/security/external-content.ts +++ b/src/security/external-content.ts @@ -132,6 +132,8 @@ const ANGLE_BRACKET_MAP: Record = { 0x276d: ">", // medium right-pointing angle bracket ornament 0x276e: "<", // heavy left-pointing angle quotation mark ornament 0x276f: ">", // heavy right-pointing angle quotation mark ornament + 0x02c2: "<", // modifier letter left arrowhead + 0x02c3: ">", // modifier letter right arrowhead }; function foldMarkerChar(char: string): string { @@ -151,25 +153,27 @@ function foldMarkerChar(char: string): string { function foldMarkerText(input: string): string { return input.replace( - /[\uFF21-\uFF3A\uFF41-\uFF5A\uFF1C\uFF1E\u2329\u232A\u3008\u3009\u2039\u203A\u27E8\u27E9\uFE64\uFE65\u00AB\u00BB\u300A\u300B\u27EA\u27EB\u27EC\u27ED\u27EE\u27EF\u276C\u276D\u276E\u276F]/g, + /[\uFF21-\uFF3A\uFF41-\uFF5A\uFF1C\uFF1E\u2329\u232A\u3008\u3009\u2039\u203A\u27E8\u27E9\uFE64\uFE65\u00AB\u00BB\u300A\u300B\u27EA\u27EB\u27EC\u27ED\u27EE\u27EF\u276C\u276D\u276E\u276F\u02C2\u02C3]/g, (char) => foldMarkerChar(char), ); } function replaceMarkers(content: string): string { const folded = foldMarkerText(content); - if (!/external_untrusted_content/i.test(folded)) { + // Intentionally catch whitespace-delimited spoof variants (space, tab, newline) in addition + // to the legacy underscore form because LLMs may still parse them as trusted boundary markers. + if (!/external[\s_]+untrusted[\s_]+content/i.test(folded)) { return content; } const replacements: Array<{ start: number; end: number; value: string }> = []; // Match markers with or without id attribute (handles both legacy and spoofed markers) const patterns: Array<{ regex: RegExp; value: string }> = [ { - regex: /<<>>/gi, + regex: /<<<\s*EXTERNAL[\s_]+UNTRUSTED[\s_]+CONTENT(?:\s+id="[^"]{1,128}")?\s*>>>/gi, value: "[[MARKER_SANITIZED]]", }, { - regex: /<<>>/gi, + regex: /<<<\s*END[\s_]+EXTERNAL[\s_]+UNTRUSTED[\s_]+CONTENT(?:\s+id="[^"]{1,128}")?\s*>>>/gi, value: "[[END_MARKER_SANITIZED]]", }, ]; From 45b74fb56c45dfe40586d6763adf03a021eb09d2 Mon Sep 17 00:00:00 2001 From: Eugene Date: Tue, 10 Mar 2026 15:58:51 +1000 Subject: [PATCH 011/126] fix(telegram): move network fallback to resolver-scoped dispatchers (#40740) Merged via squash. Prepared head SHA: a4456d48b42d6c588b2858831a2391d015260a9b Co-authored-by: sircrumpet <4436535+sircrumpet@users.noreply.github.com> Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com> Reviewed-by: @obviyus --- CHANGELOG.md | 1 + extensions/telegram/src/channel.test.ts | 101 ++- extensions/telegram/src/channel.ts | 21 +- src/infra/net/proxy-fetch.test.ts | 1 + src/infra/net/proxy-fetch.ts | 35 +- src/telegram/audit-membership-runtime.ts | 4 +- src/telegram/audit.test.ts | 24 +- src/telegram/audit.ts | 2 + src/telegram/bot-handlers.ts | 7 +- src/telegram/bot-native-commands.ts | 1 + src/telegram/bot.media.e2e-harness.ts | 11 + src/telegram/bot.ts | 1 + .../bot/delivery.resolve-media-retry.test.ts | 56 ++ src/telegram/bot/delivery.resolve-media.ts | 32 +- src/telegram/fetch.env-proxy-runtime.test.ts | 58 ++ src/telegram/fetch.test.ts | 844 +++++++++++++----- src/telegram/fetch.ts | 415 +++++++-- src/telegram/probe.test.ts | 162 +++- src/telegram/probe.ts | 131 ++- src/telegram/proxy.test.ts | 9 +- src/telegram/proxy.ts | 2 +- src/telegram/send.proxy.test.ts | 36 +- src/telegram/send.ts | 77 +- 23 files changed, 1641 insertions(+), 390 deletions(-) create mode 100644 src/telegram/fetch.env-proxy-runtime.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2db4805cee0..2e2e65653c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ Docs: https://docs.openclaw.ai - Cron/state errors: record `lastErrorReason` in cron job state and keep the gateway schema aligned with the full failover-reason set, including regression coverage for protocol conformance. (#14382) thanks @futuremind2026. - Tools/web search: recover OpenRouter Perplexity citation extraction from `message.annotations` when chat-completions responses omit top-level citations. (#40881) Thanks @laurieluo. - Security/external content: treat whitespace-delimited `EXTERNAL UNTRUSTED CONTENT` boundary markers like underscore-delimited variants so prompt wrappers cannot bypass marker sanitization. (#35983) Thanks @urianpaul94. +- Telegram/network env-proxy: apply configured transport policy to proxied HTTPS dispatchers as well as direct `NO_PROXY` bypasses, so resolver-scoped IPv4 fallback and network settings work consistently for env-proxied Telegram traffic. (#40740) Thanks @sircrumpet. ## 2026.3.8 diff --git a/extensions/telegram/src/channel.test.ts b/extensions/telegram/src/channel.test.ts index c1912db56f0..2bf1b681497 100644 --- a/extensions/telegram/src/channel.test.ts +++ b/extensions/telegram/src/channel.test.ts @@ -57,18 +57,38 @@ function installGatewayRuntime(params?: { probeOk?: boolean; botUsername?: strin const probeTelegram = vi.fn(async () => params?.probeOk ? { ok: true, bot: { username: params.botUsername ?? "bot" } } : { ok: false }, ); + const collectUnmentionedGroupIds = vi.fn(() => ({ + groupIds: [] as string[], + unresolvedGroups: 0, + hasWildcardUnmentionedGroups: false, + })); + const auditGroupMembership = vi.fn(async () => ({ + ok: true, + checkedGroups: 0, + unresolvedGroups: 0, + hasWildcardUnmentionedGroups: false, + groups: [], + elapsedMs: 0, + })); setTelegramRuntime({ channel: { telegram: { monitorTelegramProvider, probeTelegram, + collectUnmentionedGroupIds, + auditGroupMembership, }, }, logging: { shouldLogVerbose: () => false, }, } as unknown as PluginRuntime); - return { monitorTelegramProvider, probeTelegram }; + return { + monitorTelegramProvider, + probeTelegram, + collectUnmentionedGroupIds, + auditGroupMembership, + }; } describe("telegramPlugin duplicate token guard", () => { @@ -149,6 +169,85 @@ describe("telegramPlugin duplicate token guard", () => { ); }); + it("passes account proxy and network settings into Telegram probes", async () => { + const { probeTelegram } = installGatewayRuntime({ + probeOk: true, + botUsername: "opsbot", + }); + + const cfg = createCfg(); + cfg.channels!.telegram!.accounts!.ops = { + ...cfg.channels!.telegram!.accounts!.ops, + proxy: "http://127.0.0.1:8888", + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }; + const account = telegramPlugin.config.resolveAccount(cfg, "ops"); + + await telegramPlugin.status!.probeAccount!({ + account, + timeoutMs: 5000, + cfg, + }); + + expect(probeTelegram).toHaveBeenCalledWith("token-ops", 5000, { + accountId: "ops", + proxyUrl: "http://127.0.0.1:8888", + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }); + }); + + it("passes account proxy and network settings into Telegram membership audits", async () => { + const { collectUnmentionedGroupIds, auditGroupMembership } = installGatewayRuntime({ + probeOk: true, + botUsername: "opsbot", + }); + + collectUnmentionedGroupIds.mockReturnValue({ + groupIds: ["-100123"], + unresolvedGroups: 0, + hasWildcardUnmentionedGroups: false, + }); + + const cfg = createCfg(); + cfg.channels!.telegram!.accounts!.ops = { + ...cfg.channels!.telegram!.accounts!.ops, + proxy: "http://127.0.0.1:8888", + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + groups: { + "-100123": { requireMention: false }, + }, + }; + const account = telegramPlugin.config.resolveAccount(cfg, "ops"); + + await telegramPlugin.status!.auditAccount!({ + account, + timeoutMs: 5000, + probe: { ok: true, bot: { id: 123 }, elapsedMs: 1 }, + cfg, + }); + + expect(auditGroupMembership).toHaveBeenCalledWith({ + token: "token-ops", + botId: 123, + groupIds: ["-100123"], + proxyUrl: "http://127.0.0.1:8888", + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + timeoutMs: 5000, + }); + }); + it("forwards mediaLocalRoots to sendMessageTelegram for outbound media sends", async () => { const sendMessageTelegram = vi.fn(async () => ({ messageId: "tg-1" })); setTelegramRuntime({ diff --git a/extensions/telegram/src/channel.ts b/extensions/telegram/src/channel.ts index 7ea0a7a6525..5893f4e0a2e 100644 --- a/extensions/telegram/src/channel.ts +++ b/extensions/telegram/src/channel.ts @@ -438,11 +438,11 @@ export const telegramPlugin: ChannelPlugin buildTokenChannelStatusSummary(snapshot), probeAccount: async ({ account, timeoutMs }) => - getTelegramRuntime().channel.telegram.probeTelegram( - account.token, - timeoutMs, - account.config.proxy, - ), + getTelegramRuntime().channel.telegram.probeTelegram(account.token, timeoutMs, { + accountId: account.accountId, + proxyUrl: account.config.proxy, + network: account.config.network, + }), auditAccount: async ({ account, timeoutMs, probe, cfg }) => { const groups = cfg.channels?.telegram?.accounts?.[account.accountId]?.groups ?? @@ -468,6 +468,7 @@ export const telegramPlugin: ChannelPlugin { undiciFetch.mockResolvedValue({ ok: true }); const proxyFetch = makeProxyFetch(proxyUrl); + expect(proxyAgentSpy).not.toHaveBeenCalled(); await proxyFetch("https://api.example.com/v1/audio"); expect(proxyAgentSpy).toHaveBeenCalledWith(proxyUrl); diff --git a/src/infra/net/proxy-fetch.ts b/src/infra/net/proxy-fetch.ts index e6c11813959..391387f3cca 100644 --- a/src/infra/net/proxy-fetch.ts +++ b/src/infra/net/proxy-fetch.ts @@ -1,19 +1,46 @@ import { EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici"; import { logWarn } from "../../logger.js"; +export const PROXY_FETCH_PROXY_URL = Symbol.for("openclaw.proxyFetch.proxyUrl"); +type ProxyFetchWithMetadata = typeof fetch & { + [PROXY_FETCH_PROXY_URL]?: string; +}; + /** * Create a fetch function that routes requests through the given HTTP proxy. * Uses undici's ProxyAgent under the hood. */ export function makeProxyFetch(proxyUrl: string): typeof fetch { - const agent = new ProxyAgent(proxyUrl); + let agent: ProxyAgent | null = null; + const resolveAgent = (): ProxyAgent => { + if (!agent) { + agent = new ProxyAgent(proxyUrl); + } + return agent; + }; // undici's fetch is runtime-compatible with global fetch but the types diverge // on stream/body internals. Single cast at the boundary keeps the rest type-safe. - return ((input: RequestInfo | URL, init?: RequestInit) => + const proxyFetch = ((input: RequestInfo | URL, init?: RequestInit) => undiciFetch(input as string | URL, { ...(init as Record), - dispatcher: agent, - }) as unknown as Promise) as typeof fetch; + dispatcher: resolveAgent(), + }) as unknown as Promise) as ProxyFetchWithMetadata; + Object.defineProperty(proxyFetch, PROXY_FETCH_PROXY_URL, { + value: proxyUrl, + enumerable: false, + configurable: false, + writable: false, + }); + return proxyFetch; +} + +export function getProxyUrlFromFetch(fetchImpl?: typeof fetch): string | undefined { + const proxyUrl = (fetchImpl as ProxyFetchWithMetadata | undefined)?.[PROXY_FETCH_PROXY_URL]; + if (typeof proxyUrl !== "string") { + return undefined; + } + const trimmed = proxyUrl.trim(); + return trimmed ? trimmed : undefined; } /** diff --git a/src/telegram/audit-membership-runtime.ts b/src/telegram/audit-membership-runtime.ts index 4f2c5a43710..c710fb92aa7 100644 --- a/src/telegram/audit-membership-runtime.ts +++ b/src/telegram/audit-membership-runtime.ts @@ -5,6 +5,7 @@ import type { TelegramGroupMembershipAudit, TelegramGroupMembershipAuditEntry, } from "./audit.js"; +import { resolveTelegramFetch } from "./fetch.js"; import { makeProxyFetch } from "./proxy.js"; const TELEGRAM_API_BASE = "https://api.telegram.org"; @@ -16,7 +17,8 @@ type TelegramGroupMembershipAuditData = Omit { - const fetcher = params.proxyUrl ? makeProxyFetch(params.proxyUrl) : fetch; + const proxyFetch = params.proxyUrl ? makeProxyFetch(params.proxyUrl) : undefined; + const fetcher = resolveTelegramFetch(proxyFetch, { network: params.network }); const base = `${TELEGRAM_API_BASE}/bot${params.token}`; const groups: TelegramGroupMembershipAuditEntry[] = []; diff --git a/src/telegram/audit.test.ts b/src/telegram/audit.test.ts index c7524c6ca05..e5cc4490e08 100644 --- a/src/telegram/audit.test.ts +++ b/src/telegram/audit.test.ts @@ -2,16 +2,22 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; let collectTelegramUnmentionedGroupIds: typeof import("./audit.js").collectTelegramUnmentionedGroupIds; let auditTelegramGroupMembership: typeof import("./audit.js").auditTelegramGroupMembership; +const undiciFetch = vi.hoisted(() => vi.fn()); + +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: undiciFetch, + }; +}); function mockGetChatMemberStatus(status: string) { - vi.stubGlobal( - "fetch", - vi.fn().mockResolvedValueOnce( - new Response(JSON.stringify({ ok: true, result: { status } }), { - status: 200, - headers: { "Content-Type": "application/json" }, - }), - ), + undiciFetch.mockResolvedValueOnce( + new Response(JSON.stringify({ ok: true, result: { status } }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), ); } @@ -31,7 +37,7 @@ describe("telegram audit", () => { }); beforeEach(() => { - vi.unstubAllGlobals(); + undiciFetch.mockReset(); }); it("collects unmentioned numeric group ids and flags wildcard", async () => { diff --git a/src/telegram/audit.ts b/src/telegram/audit.ts index 24e5f58957a..6b667c37581 100644 --- a/src/telegram/audit.ts +++ b/src/telegram/audit.ts @@ -1,4 +1,5 @@ import type { TelegramGroupConfig } from "../config/types.js"; +import type { TelegramNetworkConfig } from "../config/types.telegram.js"; export type TelegramGroupMembershipAuditEntry = { chatId: string; @@ -64,6 +65,7 @@ export type AuditTelegramGroupMembershipParams = { botId: number; groupIds: string[]; proxyUrl?: string; + network?: TelegramNetworkConfig; timeoutMs: number; }; diff --git a/src/telegram/bot-handlers.ts b/src/telegram/bot-handlers.ts index 78290f342ad..2d1327bcd5f 100644 --- a/src/telegram/bot-handlers.ts +++ b/src/telegram/bot-handlers.ts @@ -123,6 +123,7 @@ export const registerTelegramHandlers = ({ accountId, bot, opts, + telegramFetchImpl, runtime, mediaMaxBytes, telegramCfg, @@ -371,7 +372,7 @@ export const registerTelegramHandlers = ({ for (const { ctx } of entry.messages) { let media; try { - media = await resolveMedia(ctx, mediaMaxBytes, opts.token, opts.proxyFetch); + media = await resolveMedia(ctx, mediaMaxBytes, opts.token, telegramFetchImpl); } catch (mediaErr) { if (!isRecoverableMediaGroupError(mediaErr)) { throw mediaErr; @@ -475,7 +476,7 @@ export const registerTelegramHandlers = ({ }, mediaMaxBytes, opts.token, - opts.proxyFetch, + telegramFetchImpl, ); if (!media) { return []; @@ -986,7 +987,7 @@ export const registerTelegramHandlers = ({ let media: Awaited> = null; try { - media = await resolveMedia(ctx, mediaMaxBytes, opts.token, opts.proxyFetch); + media = await resolveMedia(ctx, mediaMaxBytes, opts.token, telegramFetchImpl); } catch (mediaErr) { if (isMediaSizeLimitError(mediaErr)) { if (sendOversizeWarning) { diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts index aa37c98e9b9..06148b17b33 100644 --- a/src/telegram/bot-native-commands.ts +++ b/src/telegram/bot-native-commands.ts @@ -94,6 +94,7 @@ export type RegisterTelegramHandlerParams = { bot: Bot; mediaMaxBytes: number; opts: TelegramBotOptions; + telegramFetchImpl?: typeof fetch; runtime: RuntimeEnv; telegramCfg: TelegramAccountConfig; allowFrom?: Array; diff --git a/src/telegram/bot.media.e2e-harness.ts b/src/telegram/bot.media.e2e-harness.ts index 58628df522b..d26eff44fb6 100644 --- a/src/telegram/bot.media.e2e-harness.ts +++ b/src/telegram/bot.media.e2e-harness.ts @@ -6,6 +6,9 @@ export const middlewareUseSpy: Mock = vi.fn(); export const onSpy: Mock = vi.fn(); export const stopSpy: Mock = vi.fn(); export const sendChatActionSpy: Mock = vi.fn(); +export const undiciFetchSpy: Mock = vi.fn((input: RequestInfo | URL, init?: RequestInit) => + globalThis.fetch(input, init), +); async function defaultSaveMediaBuffer(buffer: Buffer, contentType?: string) { return { @@ -81,6 +84,14 @@ vi.mock("@grammyjs/transformer-throttler", () => ({ apiThrottler: () => throttlerSpy(), })); +vi.mock("undici", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + fetch: (...args: Parameters) => undiciFetchSpy(...args), + }; +}); + vi.mock("../media/store.js", async (importOriginal) => { const actual = await importOriginal(); const mockModule = Object.create(null) as Record; diff --git a/src/telegram/bot.ts b/src/telegram/bot.ts index 8bfa0b8ac0c..48d0c745b42 100644 --- a/src/telegram/bot.ts +++ b/src/telegram/bot.ts @@ -439,6 +439,7 @@ export function createTelegramBot(opts: TelegramBotOptions) { accountId: account.accountId, bot, opts, + telegramFetchImpl: fetchImpl as unknown as typeof fetch | undefined, runtime, mediaMaxBytes, telegramCfg, diff --git a/src/telegram/bot/delivery.resolve-media-retry.test.ts b/src/telegram/bot/delivery.resolve-media-retry.test.ts index ce8f50abbbe..df6124343fd 100644 --- a/src/telegram/bot/delivery.resolve-media-retry.test.ts +++ b/src/telegram/bot/delivery.resolve-media-retry.test.ts @@ -293,6 +293,62 @@ describe("resolveMedia getFile retry", () => { expect(getFile).toHaveBeenCalledTimes(3); expect(result).toBeNull(); }); + + it("uses caller-provided fetch impl for file downloads", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "documents/file_42.pdf" }); + const callerFetch = vi.fn() as unknown as typeof fetch; + fetchRemoteMedia.mockResolvedValueOnce({ + buffer: Buffer.from("pdf-data"), + contentType: "application/pdf", + fileName: "file_42.pdf", + }); + saveMediaBuffer.mockResolvedValueOnce({ + path: "/tmp/file_42---uuid.pdf", + contentType: "application/pdf", + }); + + const result = await resolveMedia( + makeCtx("document", getFile), + MAX_MEDIA_BYTES, + BOT_TOKEN, + callerFetch, + ); + + expect(result).not.toBeNull(); + expect(fetchRemoteMedia).toHaveBeenCalledWith( + expect.objectContaining({ + fetchImpl: callerFetch, + }), + ); + }); + + it("uses caller-provided fetch impl for sticker downloads", async () => { + const getFile = vi.fn().mockResolvedValue({ file_path: "stickers/file_0.webp" }); + const callerFetch = vi.fn() as unknown as typeof fetch; + fetchRemoteMedia.mockResolvedValueOnce({ + buffer: Buffer.from("sticker-data"), + contentType: "image/webp", + fileName: "file_0.webp", + }); + saveMediaBuffer.mockResolvedValueOnce({ + path: "/tmp/file_0.webp", + contentType: "image/webp", + }); + + const result = await resolveMedia( + makeCtx("sticker", getFile), + MAX_MEDIA_BYTES, + BOT_TOKEN, + callerFetch, + ); + + expect(result).not.toBeNull(); + expect(fetchRemoteMedia).toHaveBeenCalledWith( + expect.objectContaining({ + fetchImpl: callerFetch, + }), + ); + }); }); describe("resolveMedia original filename preservation", () => { diff --git a/src/telegram/bot/delivery.resolve-media.ts b/src/telegram/bot/delivery.resolve-media.ts index 14df1d6e2a8..9f560116a5d 100644 --- a/src/telegram/bot/delivery.resolve-media.ts +++ b/src/telegram/bot/delivery.resolve-media.ts @@ -92,12 +92,20 @@ async function resolveTelegramFileWithRetry( } } -function resolveRequiredFetchImpl(proxyFetch?: typeof fetch): typeof fetch { - const fetchImpl = proxyFetch ?? globalThis.fetch; - if (!fetchImpl) { +function resolveRequiredFetchImpl(fetchImpl?: typeof fetch): typeof fetch { + const resolved = fetchImpl ?? globalThis.fetch; + if (!resolved) { throw new Error("fetch is not available; set channels.telegram.proxy in config"); } - return fetchImpl; + return resolved; +} + +function resolveOptionalFetchImpl(fetchImpl?: typeof fetch): typeof fetch | null { + try { + return resolveRequiredFetchImpl(fetchImpl); + } catch { + return null; + } } /** Default idle timeout for Telegram media downloads (30 seconds). */ @@ -134,7 +142,7 @@ async function resolveStickerMedia(params: { ctx: TelegramContext; maxBytes: number; token: string; - proxyFetch?: typeof fetch; + fetchImpl?: typeof fetch; }): Promise< | { path: string; @@ -145,7 +153,7 @@ async function resolveStickerMedia(params: { | null | undefined > { - const { msg, ctx, maxBytes, token, proxyFetch } = params; + const { msg, ctx, maxBytes, token, fetchImpl } = params; if (!msg.sticker) { return undefined; } @@ -165,15 +173,15 @@ async function resolveStickerMedia(params: { logVerbose("telegram: getFile returned no file_path for sticker"); return null; } - const fetchImpl = proxyFetch ?? globalThis.fetch; - if (!fetchImpl) { + const resolvedFetchImpl = resolveOptionalFetchImpl(fetchImpl); + if (!resolvedFetchImpl) { logVerbose("telegram: fetch not available for sticker download"); return null; } const saved = await downloadAndSaveTelegramFile({ filePath: file.file_path, token, - fetchImpl, + fetchImpl: resolvedFetchImpl, maxBytes, }); @@ -229,7 +237,7 @@ export async function resolveMedia( ctx: TelegramContext, maxBytes: number, token: string, - proxyFetch?: typeof fetch, + fetchImpl?: typeof fetch, ): Promise<{ path: string; contentType?: string; @@ -242,7 +250,7 @@ export async function resolveMedia( ctx, maxBytes, token, - proxyFetch, + fetchImpl, }); if (stickerResolved !== undefined) { return stickerResolved; @@ -263,7 +271,7 @@ export async function resolveMedia( const saved = await downloadAndSaveTelegramFile({ filePath: file.file_path, token, - fetchImpl: resolveRequiredFetchImpl(proxyFetch), + fetchImpl: resolveRequiredFetchImpl(fetchImpl), maxBytes, telegramFileName: resolveTelegramFileName(msg), }); diff --git a/src/telegram/fetch.env-proxy-runtime.test.ts b/src/telegram/fetch.env-proxy-runtime.test.ts new file mode 100644 index 00000000000..0292f465747 --- /dev/null +++ b/src/telegram/fetch.env-proxy-runtime.test.ts @@ -0,0 +1,58 @@ +import { createRequire } from "node:module"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +const require = createRequire(import.meta.url); +const EnvHttpProxyAgent = require("undici/lib/dispatcher/env-http-proxy-agent.js") as { + new (opts?: Record): Record; +}; +const { kHttpsProxyAgent, kNoProxyAgent } = require("undici/lib/core/symbols.js") as { + kHttpsProxyAgent: symbol; + kNoProxyAgent: symbol; +}; + +function getOwnSymbolValue( + target: Record, + description: string, +): Record | undefined { + const symbol = Object.getOwnPropertySymbols(target).find( + (entry) => entry.description === description, + ); + const value = symbol ? target[symbol] : undefined; + return value && typeof value === "object" ? (value as Record) : undefined; +} + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +describe("undici env proxy semantics", () => { + it("uses proxyTls rather than connect for proxied HTTPS transport settings", () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + const connect = { + family: 4, + autoSelectFamily: false, + }; + + const withoutProxyTls = new EnvHttpProxyAgent({ connect }); + const noProxyAgent = withoutProxyTls[kNoProxyAgent] as Record; + const httpsProxyAgent = withoutProxyTls[kHttpsProxyAgent] as Record; + + expect(getOwnSymbolValue(noProxyAgent, "options")?.connect).toEqual( + expect.objectContaining(connect), + ); + expect(getOwnSymbolValue(httpsProxyAgent, "proxy tls settings")).toBeUndefined(); + + const withProxyTls = new EnvHttpProxyAgent({ + connect, + proxyTls: connect, + }); + const httpsProxyAgentWithProxyTls = withProxyTls[kHttpsProxyAgent] as Record< + PropertyKey, + unknown + >; + + expect(getOwnSymbolValue(httpsProxyAgentWithProxyTls, "proxy tls settings")).toEqual( + expect.objectContaining(connect), + ); + }); +}); diff --git a/src/telegram/fetch.test.ts b/src/telegram/fetch.test.ts index 95b26d931cb..dc4c7a5145a 100644 --- a/src/telegram/fetch.test.ts +++ b/src/telegram/fetch.test.ts @@ -1,25 +1,36 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { resolveFetch } from "../infra/fetch.js"; -import { resetTelegramFetchStateForTests, resolveTelegramFetch } from "./fetch.js"; +import { resolveTelegramFetch } from "./fetch.js"; -const setDefaultAutoSelectFamily = vi.hoisted(() => vi.fn()); const setDefaultResultOrder = vi.hoisted(() => vi.fn()); +const setDefaultAutoSelectFamily = vi.hoisted(() => vi.fn()); + +const undiciFetch = vi.hoisted(() => vi.fn()); const setGlobalDispatcher = vi.hoisted(() => vi.fn()); -const getGlobalDispatcherState = vi.hoisted(() => ({ value: undefined as unknown })); -const getGlobalDispatcher = vi.hoisted(() => vi.fn(() => getGlobalDispatcherState.value)); -const EnvHttpProxyAgentCtor = vi.hoisted(() => - vi.fn(function MockEnvHttpProxyAgent(this: { options: unknown }, options: unknown) { +const AgentCtor = vi.hoisted(() => + vi.fn(function MockAgent( + this: { options?: Record }, + options?: Record, + ) { + this.options = options; + }), +); +const EnvHttpProxyAgentCtor = vi.hoisted(() => + vi.fn(function MockEnvHttpProxyAgent( + this: { options?: Record }, + options?: Record, + ) { + this.options = options; + }), +); +const ProxyAgentCtor = vi.hoisted(() => + vi.fn(function MockProxyAgent( + this: { options?: Record | string }, + options?: Record | string, + ) { this.options = options; }), ); - -vi.mock("node:net", async () => { - const actual = await vi.importActual("node:net"); - return { - ...actual, - setDefaultAutoSelectFamily, - }; -}); vi.mock("node:dns", async () => { const actual = await vi.importActual("node:dns"); @@ -29,266 +40,655 @@ vi.mock("node:dns", async () => { }; }); +vi.mock("node:net", async () => { + const actual = await vi.importActual("node:net"); + return { + ...actual, + setDefaultAutoSelectFamily, + }; +}); + vi.mock("undici", () => ({ + Agent: AgentCtor, EnvHttpProxyAgent: EnvHttpProxyAgentCtor, - getGlobalDispatcher, + ProxyAgent: ProxyAgentCtor, + fetch: undiciFetch, setGlobalDispatcher, })); -const originalFetch = globalThis.fetch; - -function expectEnvProxyAgentConstructorCall(params: { nth: number; autoSelectFamily: boolean }) { - expect(EnvHttpProxyAgentCtor).toHaveBeenNthCalledWith(params.nth, { - connect: { - autoSelectFamily: params.autoSelectFamily, - autoSelectFamilyAttemptTimeout: 300, - }, - }); +function resolveTelegramFetchOrThrow( + proxyFetch?: typeof fetch, + options?: { network?: { autoSelectFamily?: boolean; dnsResultOrder?: "ipv4first" | "verbatim" } }, +) { + return resolveTelegramFetch(proxyFetch, options); } -function resolveTelegramFetchOrThrow() { - const resolved = resolveTelegramFetch(); - if (!resolved) { - throw new Error("expected resolved fetch"); +function getDispatcherFromUndiciCall(nth: number) { + const call = undiciFetch.mock.calls[nth - 1] as [RequestInfo | URL, RequestInit?] | undefined; + if (!call) { + throw new Error(`missing undici fetch call #${nth}`); } - return resolved; + const init = call[1] as (RequestInit & { dispatcher?: unknown }) | undefined; + return init?.dispatcher as + | { + options?: { + connect?: Record; + proxyTls?: Record; + }; + } + | undefined; +} + +function buildFetchFallbackError(code: string) { + const connectErr = Object.assign(new Error(`connect ${code} api.telegram.org:443`), { + code, + }); + return Object.assign(new TypeError("fetch failed"), { + cause: connectErr, + }); } afterEach(() => { - resetTelegramFetchStateForTests(); - setDefaultAutoSelectFamily.mockReset(); - setDefaultResultOrder.mockReset(); + undiciFetch.mockReset(); setGlobalDispatcher.mockReset(); - getGlobalDispatcher.mockClear(); - getGlobalDispatcherState.value = undefined; + AgentCtor.mockClear(); EnvHttpProxyAgentCtor.mockClear(); + ProxyAgentCtor.mockClear(); + setDefaultResultOrder.mockReset(); + setDefaultAutoSelectFamily.mockReset(); vi.unstubAllEnvs(); vi.clearAllMocks(); - if (originalFetch) { - globalThis.fetch = originalFetch; - } else { - delete (globalThis as { fetch?: typeof fetch }).fetch; - } }); describe("resolveTelegramFetch", () => { - it("returns wrapped global fetch when available", async () => { - const fetchMock = vi.fn(async () => ({})); - globalThis.fetch = fetchMock as unknown as typeof fetch; + it("wraps proxy fetches and leaves retry policy to caller-provided fetch", async () => { + const proxyFetch = vi.fn(async () => ({ ok: true }) as Response) as unknown as typeof fetch; - const resolved = resolveTelegramFetch(); + const resolved = resolveTelegramFetchOrThrow(proxyFetch); - expect(resolved).toBeTypeOf("function"); - expect(resolved).not.toBe(fetchMock); - }); + await resolved("https://api.telegram.org/botx/getMe"); - it("wraps proxy fetches and normalizes foreign signals once", async () => { - let seenSignal: AbortSignal | undefined; - const proxyFetch = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { - seenSignal = init?.signal as AbortSignal | undefined; - return {} as Response; - }); - - const resolved = resolveTelegramFetch(proxyFetch as unknown as typeof fetch); - expect(resolved).toBeTypeOf("function"); - - let abortHandler: (() => void) | null = null; - const addEventListener = vi.fn((event: string, handler: () => void) => { - if (event === "abort") { - abortHandler = handler; - } - }); - const removeEventListener = vi.fn((event: string, handler: () => void) => { - if (event === "abort" && abortHandler === handler) { - abortHandler = null; - } - }); - const fakeSignal = { - aborted: false, - addEventListener, - removeEventListener, - } as unknown as AbortSignal; - - if (!resolved) { - throw new Error("expected resolved proxy fetch"); - } - await resolved("https://example.com", { signal: fakeSignal }); - - expect(proxyFetch).toHaveBeenCalledOnce(); - expect(seenSignal).toBeInstanceOf(AbortSignal); - expect(seenSignal).not.toBe(fakeSignal); - expect(addEventListener).toHaveBeenCalledTimes(1); - expect(removeEventListener).toHaveBeenCalledTimes(1); + expect(proxyFetch).toHaveBeenCalledTimes(1); + expect(undiciFetch).not.toHaveBeenCalled(); }); it("does not double-wrap an already wrapped proxy fetch", async () => { const proxyFetch = vi.fn(async () => ({ ok: true }) as Response) as unknown as typeof fetch; - const alreadyWrapped = resolveFetch(proxyFetch); + const wrapped = resolveFetch(proxyFetch); - const resolved = resolveTelegramFetch(alreadyWrapped); + const resolved = resolveTelegramFetch(wrapped); - expect(resolved).toBe(alreadyWrapped); + expect(resolved).toBe(wrapped); }); - it("honors env enable override", async () => { - vi.stubEnv("OPENCLAW_TELEGRAM_ENABLE_AUTO_SELECT_FAMILY", "1"); - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - resolveTelegramFetch(); - expect(setDefaultAutoSelectFamily).toHaveBeenCalledWith(true); - }); + it("uses resolver-scoped Agent dispatcher with configured transport policy", async () => { + undiciFetch.mockResolvedValue({ ok: true } as Response); - it("uses config override when provided", async () => { - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - expect(setDefaultAutoSelectFamily).toHaveBeenCalledWith(true); - }); - - it("env disable override wins over config", async () => { - vi.stubEnv("OPENCLAW_TELEGRAM_ENABLE_AUTO_SELECT_FAMILY", "0"); - vi.stubEnv("OPENCLAW_TELEGRAM_DISABLE_AUTO_SELECT_FAMILY", "1"); - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - expect(setDefaultAutoSelectFamily).toHaveBeenCalledWith(false); - }); - - it("applies dns result order from config", async () => { - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - resolveTelegramFetch(undefined, { network: { dnsResultOrder: "verbatim" } }); - expect(setDefaultResultOrder).toHaveBeenCalledWith("verbatim"); - }); - - it("retries dns setter on next call when previous attempt threw", async () => { - setDefaultResultOrder.mockImplementationOnce(() => { - throw new Error("dns setter failed once"); - }); - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - - resolveTelegramFetch(undefined, { network: { dnsResultOrder: "ipv4first" } }); - resolveTelegramFetch(undefined, { network: { dnsResultOrder: "ipv4first" } }); - - expect(setDefaultResultOrder).toHaveBeenCalledTimes(2); - }); - - it("replaces global undici dispatcher with proxy-aware EnvHttpProxyAgent", async () => { - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - - expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); - expectEnvProxyAgentConstructorCall({ nth: 1, autoSelectFamily: true }); - }); - - it("keeps an existing proxy-like global dispatcher", async () => { - getGlobalDispatcherState.value = { - constructor: { name: "ProxyAgent" }, - }; - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - - expect(setGlobalDispatcher).not.toHaveBeenCalled(); - expect(EnvHttpProxyAgentCtor).not.toHaveBeenCalled(); - }); - - it("updates proxy-like dispatcher when proxy env is configured", async () => { - vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); - getGlobalDispatcherState.value = { - constructor: { name: "ProxyAgent" }, - }; - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - - expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); - expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(1); - }); - - it("sets global dispatcher only once across repeated equal decisions", async () => { - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - - expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); - }); - - it("updates global dispatcher when autoSelectFamily decision changes", async () => { - globalThis.fetch = vi.fn(async () => ({})) as unknown as typeof fetch; - resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); - resolveTelegramFetch(undefined, { network: { autoSelectFamily: false } }); - - expect(setGlobalDispatcher).toHaveBeenCalledTimes(2); - expectEnvProxyAgentConstructorCall({ nth: 1, autoSelectFamily: true }); - expectEnvProxyAgentConstructorCall({ nth: 2, autoSelectFamily: false }); - }); - - it("retries once with ipv4 fallback when fetch fails with network timeout/unreachable", async () => { - const timeoutErr = Object.assign(new Error("connect ETIMEDOUT 149.154.166.110:443"), { - code: "ETIMEDOUT", - }); - const unreachableErr = Object.assign( - new Error("connect ENETUNREACH 2001:67c:4e8:f004::9:443"), - { - code: "ENETUNREACH", + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "verbatim", }, - ); - const fetchError = Object.assign(new TypeError("fetch failed"), { - cause: Object.assign(new Error("aggregate"), { - errors: [timeoutErr, unreachableErr], - }), }); - const fetchMock = vi - .fn() - .mockRejectedValueOnce(fetchError) - .mockResolvedValueOnce({ ok: true } as Response); - globalThis.fetch = fetchMock as unknown as typeof fetch; - const resolved = resolveTelegramFetchOrThrow(); + await resolved("https://api.telegram.org/botx/getMe"); - await resolved("https://api.telegram.org/file/botx/photos/file_1.jpg"); + expect(AgentCtor).toHaveBeenCalledTimes(1); + expect(EnvHttpProxyAgentCtor).not.toHaveBeenCalled(); - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(setGlobalDispatcher).toHaveBeenCalledTimes(2); - expectEnvProxyAgentConstructorCall({ nth: 1, autoSelectFamily: true }); - expectEnvProxyAgentConstructorCall({ nth: 2, autoSelectFamily: false }); + const dispatcher = getDispatcherFromUndiciCall(1); + expect(dispatcher).toBeDefined(); + expect(dispatcher?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + expect(typeof dispatcher?.options?.connect?.lookup).toBe("function"); }); - it("retries with ipv4 fallback once per request, not once per process", async () => { - const timeoutErr = Object.assign(new Error("connect ETIMEDOUT 149.154.166.110:443"), { - code: "ETIMEDOUT", + it("uses EnvHttpProxyAgent dispatcher when proxy env is configured", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + undiciFetch.mockResolvedValue({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, }); - const fetchError = Object.assign(new TypeError("fetch failed"), { - cause: timeoutErr, + + await resolved("https://api.telegram.org/botx/getMe"); + + expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(1); + expect(AgentCtor).not.toHaveBeenCalled(); + + const dispatcher = getDispatcherFromUndiciCall(1); + expect(dispatcher?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: false, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + expect(dispatcher?.options?.proxyTls).toEqual( + expect.objectContaining({ + autoSelectFamily: false, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + }); + + it("pins env-proxy transport policy onto proxyTls for proxied HTTPS requests", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + undiciFetch.mockResolvedValue({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, }); - const fetchMock = vi - .fn() + + await resolved("https://api.telegram.org/botx/getMe"); + + const dispatcher = getDispatcherFromUndiciCall(1); + expect(dispatcher?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + expect(dispatcher?.options?.proxyTls).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + }); + + it("keeps resolver-scoped transport policy for OpenClaw proxy fetches", async () => { + const { makeProxyFetch } = await import("./proxy.js"); + const proxyFetch = makeProxyFetch("http://127.0.0.1:7890"); + ProxyAgentCtor.mockClear(); + undiciFetch.mockResolvedValue({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(proxyFetch, { + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }); + + await resolved("https://api.telegram.org/botx/getMe"); + + expect(ProxyAgentCtor).toHaveBeenCalledTimes(1); + expect(EnvHttpProxyAgentCtor).not.toHaveBeenCalled(); + expect(AgentCtor).not.toHaveBeenCalled(); + const dispatcher = getDispatcherFromUndiciCall(1); + expect(dispatcher?.options).toEqual( + expect.objectContaining({ + uri: "http://127.0.0.1:7890", + }), + ); + expect(dispatcher?.options?.proxyTls).toEqual( + expect.objectContaining({ + autoSelectFamily: false, + }), + ); + }); + + it("does not blind-retry when sticky IPv4 fallback is disallowed for explicit proxy paths", async () => { + const { makeProxyFetch } = await import("./proxy.js"); + const proxyFetch = makeProxyFetch("http://127.0.0.1:7890"); + ProxyAgentCtor.mockClear(); + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch.mockRejectedValueOnce(fetchError).mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(proxyFetch, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + await expect(resolved("https://api.telegram.org/botx/sendMessage")).rejects.toThrow( + "fetch failed", + ); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(undiciFetch).toHaveBeenCalledTimes(2); + expect(ProxyAgentCtor).toHaveBeenCalledTimes(1); + + const firstDispatcher = getDispatcherFromUndiciCall(1); + const secondDispatcher = getDispatcherFromUndiciCall(2); + + expect(firstDispatcher).toBe(secondDispatcher); + expect(firstDispatcher?.options?.proxyTls).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + expect(firstDispatcher?.options?.proxyTls?.family).not.toBe(4); + }); + + it("does not blind-retry when sticky IPv4 fallback is disallowed for env proxy paths", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch.mockRejectedValueOnce(fetchError).mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + await expect(resolved("https://api.telegram.org/botx/sendMessage")).rejects.toThrow( + "fetch failed", + ); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(undiciFetch).toHaveBeenCalledTimes(2); + expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(1); + + const firstDispatcher = getDispatcherFromUndiciCall(1); + const secondDispatcher = getDispatcherFromUndiciCall(2); + + expect(firstDispatcher).toBe(secondDispatcher); + expect(firstDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + expect(firstDispatcher?.options?.connect?.family).not.toBe(4); + }); + + it("treats ALL_PROXY-only env as direct transport and arms sticky IPv4 fallback", async () => { + vi.stubEnv("ALL_PROXY", "socks5://127.0.0.1:1080"); + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch .mockRejectedValueOnce(fetchError) .mockResolvedValueOnce({ ok: true } as Response) - .mockRejectedValueOnce(fetchError) .mockResolvedValueOnce({ ok: true } as Response); - globalThis.fetch = fetchMock as unknown as typeof fetch; - const resolved = resolveTelegramFetchOrThrow(); + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); - await resolved("https://api.telegram.org/file/botx/photos/file_1.jpg"); - await resolved("https://api.telegram.org/file/botx/photos/file_2.jpg"); + await resolved("https://api.telegram.org/botx/sendMessage"); + await resolved("https://api.telegram.org/botx/sendChatAction"); - expect(fetchMock).toHaveBeenCalledTimes(4); + expect(EnvHttpProxyAgentCtor).not.toHaveBeenCalled(); + expect(AgentCtor).toHaveBeenCalledTimes(2); + + const firstDispatcher = getDispatcherFromUndiciCall(1); + const secondDispatcher = getDispatcherFromUndiciCall(2); + const thirdDispatcher = getDispatcherFromUndiciCall(3); + + expect(firstDispatcher).not.toBe(secondDispatcher); + expect(secondDispatcher).toBe(thirdDispatcher); + expect(secondDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + family: 4, + autoSelectFamily: false, + }), + ); }); - it("does not retry when fetch fails without fallback network error codes", async () => { - const fetchError = Object.assign(new TypeError("fetch failed"), { - cause: Object.assign(new Error("connect ECONNRESET"), { - code: "ECONNRESET", - }), + it("arms sticky IPv4 fallback when env proxy init falls back to direct Agent", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + EnvHttpProxyAgentCtor.mockImplementationOnce(function ThrowingEnvProxyAgent() { + throw new Error("invalid proxy config"); }); - const fetchMock = vi.fn().mockRejectedValue(fetchError); - globalThis.fetch = fetchMock as unknown as typeof fetch; + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch + .mockRejectedValueOnce(fetchError) + .mockResolvedValueOnce({ ok: true } as Response) + .mockResolvedValueOnce({ ok: true } as Response); - const resolved = resolveTelegramFetchOrThrow(); + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); - await expect(resolved("https://api.telegram.org/file/botx/photos/file_3.jpg")).rejects.toThrow( + await resolved("https://api.telegram.org/botx/sendMessage"); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(undiciFetch).toHaveBeenCalledTimes(3); + expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(1); + expect(AgentCtor).toHaveBeenCalledTimes(2); + + const firstDispatcher = getDispatcherFromUndiciCall(1); + const secondDispatcher = getDispatcherFromUndiciCall(2); + const thirdDispatcher = getDispatcherFromUndiciCall(3); + + expect(firstDispatcher).not.toBe(secondDispatcher); + expect(secondDispatcher).toBe(thirdDispatcher); + expect(secondDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + family: 4, + autoSelectFamily: false, + }), + ); + }); + + it("arms sticky IPv4 fallback when NO_PROXY bypasses telegram under env proxy", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + vi.stubEnv("NO_PROXY", "api.telegram.org"); + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch + .mockRejectedValueOnce(fetchError) + .mockResolvedValueOnce({ ok: true } as Response) + .mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + await resolved("https://api.telegram.org/botx/sendMessage"); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(undiciFetch).toHaveBeenCalledTimes(3); + expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(2); + expect(AgentCtor).not.toHaveBeenCalled(); + + const firstDispatcher = getDispatcherFromUndiciCall(1); + const secondDispatcher = getDispatcherFromUndiciCall(2); + const thirdDispatcher = getDispatcherFromUndiciCall(3); + + expect(firstDispatcher).not.toBe(secondDispatcher); + expect(secondDispatcher).toBe(thirdDispatcher); + expect(secondDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + family: 4, + autoSelectFamily: false, + }), + ); + }); + + it("uses no_proxy over NO_PROXY when deciding env-proxy bypass", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + vi.stubEnv("NO_PROXY", ""); + vi.stubEnv("no_proxy", "api.telegram.org"); + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch + .mockRejectedValueOnce(fetchError) + .mockResolvedValueOnce({ ok: true } as Response) + .mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + await resolved("https://api.telegram.org/botx/sendMessage"); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(2); + const secondDispatcher = getDispatcherFromUndiciCall(2); + expect(secondDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + family: 4, + autoSelectFamily: false, + }), + ); + }); + + it("matches whitespace and wildcard no_proxy entries like EnvHttpProxyAgent", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + vi.stubEnv("no_proxy", "localhost *.telegram.org"); + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch + .mockRejectedValueOnce(fetchError) + .mockResolvedValueOnce({ ok: true } as Response) + .mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + await resolved("https://api.telegram.org/botx/sendMessage"); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(2); + const secondDispatcher = getDispatcherFromUndiciCall(2); + expect(secondDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + family: 4, + autoSelectFamily: false, + }), + ); + }); + + it("fails closed when explicit proxy dispatcher initialization fails", async () => { + const { makeProxyFetch } = await import("./proxy.js"); + const proxyFetch = makeProxyFetch("http://127.0.0.1:7890"); + ProxyAgentCtor.mockClear(); + ProxyAgentCtor.mockImplementationOnce(function ThrowingProxyAgent() { + throw new Error("invalid proxy config"); + }); + + expect(() => + resolveTelegramFetchOrThrow(proxyFetch, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }), + ).toThrow("explicit proxy dispatcher init failed: invalid proxy config"); + }); + + it("falls back to Agent when env proxy dispatcher initialization fails", async () => { + vi.stubEnv("HTTPS_PROXY", "http://127.0.0.1:7890"); + EnvHttpProxyAgentCtor.mockImplementationOnce(function ThrowingEnvProxyAgent() { + throw new Error("invalid proxy config"); + }); + undiciFetch.mockResolvedValue({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: false, + }, + }); + + await resolved("https://api.telegram.org/botx/getMe"); + + expect(EnvHttpProxyAgentCtor).toHaveBeenCalledTimes(1); + expect(AgentCtor).toHaveBeenCalledTimes(1); + + const dispatcher = getDispatcherFromUndiciCall(1); + expect(dispatcher?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: false, + }), + ); + }); + + it("retries once and then keeps sticky IPv4 dispatcher for subsequent requests", async () => { + const fetchError = buildFetchFallbackError("ETIMEDOUT"); + undiciFetch + .mockRejectedValueOnce(fetchError) + .mockResolvedValueOnce({ ok: true } as Response) + .mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + }, + }); + + await resolved("https://api.telegram.org/botx/sendMessage"); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(undiciFetch).toHaveBeenCalledTimes(3); + + const firstDispatcher = getDispatcherFromUndiciCall(1); + const secondDispatcher = getDispatcherFromUndiciCall(2); + const thirdDispatcher = getDispatcherFromUndiciCall(3); + + expect(firstDispatcher).toBeDefined(); + expect(secondDispatcher).toBeDefined(); + expect(thirdDispatcher).toBeDefined(); + + expect(firstDispatcher).not.toBe(secondDispatcher); + expect(secondDispatcher).toBe(thirdDispatcher); + + expect(firstDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + expect(secondDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + family: 4, + autoSelectFamily: false, + }), + ); + }); + + it("preserves caller-provided dispatcher across fallback retry", async () => { + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch.mockRejectedValueOnce(fetchError).mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + }, + }); + + const callerDispatcher = { name: "caller" }; + + await resolved("https://api.telegram.org/botx/sendMessage", { + dispatcher: callerDispatcher, + } as RequestInit); + + expect(undiciFetch).toHaveBeenCalledTimes(2); + + const firstCallInit = undiciFetch.mock.calls[0]?.[1] as + | (RequestInit & { dispatcher?: unknown }) + | undefined; + const secondCallInit = undiciFetch.mock.calls[1]?.[1] as + | (RequestInit & { dispatcher?: unknown }) + | undefined; + + expect(firstCallInit?.dispatcher).toBe(callerDispatcher); + expect(secondCallInit?.dispatcher).toBe(callerDispatcher); + }); + + it("does not arm sticky fallback from caller-provided dispatcher failures", async () => { + const fetchError = buildFetchFallbackError("EHOSTUNREACH"); + undiciFetch + .mockRejectedValueOnce(fetchError) + .mockResolvedValueOnce({ ok: true } as Response) + .mockResolvedValueOnce({ ok: true } as Response); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + }, + }); + + const callerDispatcher = { name: "caller" }; + + await resolved("https://api.telegram.org/botx/sendMessage", { + dispatcher: callerDispatcher, + } as RequestInit); + await resolved("https://api.telegram.org/botx/sendChatAction"); + + expect(undiciFetch).toHaveBeenCalledTimes(3); + + const firstCallInit = undiciFetch.mock.calls[0]?.[1] as + | (RequestInit & { dispatcher?: unknown }) + | undefined; + const secondCallInit = undiciFetch.mock.calls[1]?.[1] as + | (RequestInit & { dispatcher?: unknown }) + | undefined; + const thirdDispatcher = getDispatcherFromUndiciCall(3); + + expect(firstCallInit?.dispatcher).toBe(callerDispatcher); + expect(secondCallInit?.dispatcher).toBe(callerDispatcher); + expect(thirdDispatcher?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + autoSelectFamilyAttemptTimeout: 300, + }), + ); + expect(thirdDispatcher?.options?.connect?.family).not.toBe(4); + }); + + it("does not retry when error codes do not match fallback rules", async () => { + const fetchError = buildFetchFallbackError("ECONNRESET"); + undiciFetch.mockRejectedValue(fetchError); + + const resolved = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + }, + }); + + await expect(resolved("https://api.telegram.org/botx/sendMessage")).rejects.toThrow( "fetch failed", ); - expect(fetchMock).toHaveBeenCalledTimes(1); + expect(undiciFetch).toHaveBeenCalledTimes(1); + }); + + it("keeps per-resolver transport policy isolated across multiple accounts", async () => { + undiciFetch.mockResolvedValue({ ok: true } as Response); + + const resolverA = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }); + const resolverB = resolveTelegramFetchOrThrow(undefined, { + network: { + autoSelectFamily: true, + dnsResultOrder: "verbatim", + }, + }); + + await resolverA("https://api.telegram.org/botA/getMe"); + await resolverB("https://api.telegram.org/botB/getMe"); + + const dispatcherA = getDispatcherFromUndiciCall(1); + const dispatcherB = getDispatcherFromUndiciCall(2); + + expect(dispatcherA).toBeDefined(); + expect(dispatcherB).toBeDefined(); + expect(dispatcherA).not.toBe(dispatcherB); + + expect(dispatcherA?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: false, + }), + ); + expect(dispatcherB?.options?.connect).toEqual( + expect.objectContaining({ + autoSelectFamily: true, + }), + ); + + // Core guarantee: Telegram transport no longer mutates process-global defaults. + expect(setGlobalDispatcher).not.toHaveBeenCalled(); + expect(setDefaultResultOrder).not.toHaveBeenCalled(); + expect(setDefaultAutoSelectFamily).not.toHaveBeenCalled(); }); }); diff --git a/src/telegram/fetch.ts b/src/telegram/fetch.ts index f1e50021e92..3934c10c391 100644 --- a/src/telegram/fetch.ts +++ b/src/telegram/fetch.ts @@ -1,23 +1,43 @@ import * as dns from "node:dns"; -import * as net from "node:net"; -import { EnvHttpProxyAgent, getGlobalDispatcher, setGlobalDispatcher } from "undici"; +import { Agent, EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici"; import type { TelegramNetworkConfig } from "../config/types.telegram.js"; import { resolveFetch } from "../infra/fetch.js"; -import { hasProxyEnvConfigured } from "../infra/net/proxy-env.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { resolveTelegramAutoSelectFamilyDecision, resolveTelegramDnsResultOrderDecision, } from "./network-config.js"; +import { getProxyUrlFromFetch } from "./proxy.js"; -let appliedAutoSelectFamily: boolean | null = null; -let appliedDnsResultOrder: string | null = null; -let appliedGlobalDispatcherAutoSelectFamily: boolean | null = null; const log = createSubsystemLogger("telegram/network"); -function isProxyLikeDispatcher(dispatcher: unknown): boolean { - const ctorName = (dispatcher as { constructor?: { name?: string } })?.constructor?.name; - return typeof ctorName === "string" && ctorName.includes("ProxyAgent"); -} + +const TELEGRAM_AUTO_SELECT_FAMILY_ATTEMPT_TIMEOUT_MS = 300; +const TELEGRAM_API_HOSTNAME = "api.telegram.org"; + +type RequestInitWithDispatcher = RequestInit & { + dispatcher?: unknown; +}; + +type TelegramDispatcher = Agent | EnvHttpProxyAgent | ProxyAgent; + +type TelegramDispatcherMode = "direct" | "env-proxy" | "explicit-proxy"; + +type TelegramDnsResultOrder = "ipv4first" | "verbatim"; + +type LookupCallback = + | ((err: NodeJS.ErrnoException | null, address: string, family: number) => void) + | ((err: NodeJS.ErrnoException | null, addresses: dns.LookupAddress[]) => void); + +type LookupOptions = (dns.LookupOneOptions | dns.LookupAllOptions) & { + order?: TelegramDnsResultOrder; + verbatim?: boolean; +}; + +type LookupFunction = ( + hostname: string, + options: number | dns.LookupOneOptions | dns.LookupAllOptions | undefined, + callback: LookupCallback, +) => void; const FALLBACK_RETRY_ERROR_CODES = [ "ETIMEDOUT", @@ -48,72 +68,215 @@ const IPV4_FALLBACK_RULES: readonly Ipv4FallbackRule[] = [ }, ]; -// Node 22 workaround: enable autoSelectFamily to allow IPv4 fallback on broken IPv6 networks. -// Many networks have IPv6 configured but not routed, causing "Network is unreachable" errors. -// See: https://github.com/nodejs/node/issues/54359 -function applyTelegramNetworkWorkarounds(network?: TelegramNetworkConfig): void { - // Apply autoSelectFamily workaround - const autoSelectDecision = resolveTelegramAutoSelectFamilyDecision({ network }); - if (autoSelectDecision.value !== null && autoSelectDecision.value !== appliedAutoSelectFamily) { - if (typeof net.setDefaultAutoSelectFamily === "function") { - try { - net.setDefaultAutoSelectFamily(autoSelectDecision.value); - appliedAutoSelectFamily = autoSelectDecision.value; - const label = autoSelectDecision.source ? ` (${autoSelectDecision.source})` : ""; - log.info(`autoSelectFamily=${autoSelectDecision.value}${label}`); - } catch { - // ignore if unsupported by the runtime - } - } +function normalizeDnsResultOrder(value: string | null): TelegramDnsResultOrder | null { + if (value === "ipv4first" || value === "verbatim") { + return value; + } + return null; +} + +function createDnsResultOrderLookup( + order: TelegramDnsResultOrder | null, +): LookupFunction | undefined { + if (!order) { + return undefined; + } + const lookup = dns.lookup as unknown as ( + hostname: string, + options: LookupOptions, + callback: LookupCallback, + ) => void; + return (hostname, options, callback) => { + const baseOptions: LookupOptions = + typeof options === "number" + ? { family: options } + : options + ? { ...(options as LookupOptions) } + : {}; + const lookupOptions: LookupOptions = { + ...baseOptions, + order, + // Keep `verbatim` for compatibility with Node runtimes that ignore `order`. + verbatim: order === "verbatim", + }; + lookup(hostname, lookupOptions, callback); + }; +} + +function buildTelegramConnectOptions(params: { + autoSelectFamily: boolean | null; + dnsResultOrder: TelegramDnsResultOrder | null; + forceIpv4: boolean; +}): { + autoSelectFamily?: boolean; + autoSelectFamilyAttemptTimeout?: number; + family?: number; + lookup?: LookupFunction; +} | null { + const connect: { + autoSelectFamily?: boolean; + autoSelectFamilyAttemptTimeout?: number; + family?: number; + lookup?: LookupFunction; + } = {}; + + if (params.forceIpv4) { + connect.family = 4; + connect.autoSelectFamily = false; + } else if (typeof params.autoSelectFamily === "boolean") { + connect.autoSelectFamily = params.autoSelectFamily; + connect.autoSelectFamilyAttemptTimeout = TELEGRAM_AUTO_SELECT_FAMILY_ATTEMPT_TIMEOUT_MS; } - // Node 22's built-in globalThis.fetch uses undici's internal Agent whose - // connect options are frozen at construction time. Calling - // net.setDefaultAutoSelectFamily() after that agent is created has no - // effect on it. Replace the global dispatcher with one that carries the - // current autoSelectFamily setting so subsequent globalThis.fetch calls - // inherit the same decision. - // See: https://github.com/openclaw/openclaw/issues/25676 - if ( - autoSelectDecision.value !== null && - autoSelectDecision.value !== appliedGlobalDispatcherAutoSelectFamily - ) { - const existingGlobalDispatcher = getGlobalDispatcher(); - const shouldPreserveExistingProxy = - isProxyLikeDispatcher(existingGlobalDispatcher) && !hasProxyEnvConfigured(); - if (!shouldPreserveExistingProxy) { - try { - setGlobalDispatcher( - new EnvHttpProxyAgent({ - connect: { - autoSelectFamily: autoSelectDecision.value, - autoSelectFamilyAttemptTimeout: 300, - }, - }), - ); - appliedGlobalDispatcherAutoSelectFamily = autoSelectDecision.value; - log.info(`global undici dispatcher autoSelectFamily=${autoSelectDecision.value}`); - } catch { - // ignore if setGlobalDispatcher is unavailable - } - } + const lookup = createDnsResultOrderLookup(params.dnsResultOrder); + if (lookup) { + connect.lookup = lookup; } - // Apply DNS result order workaround for IPv4/IPv6 issues. - // Some APIs (including Telegram) may fail with IPv6 on certain networks. - // See: https://github.com/openclaw/openclaw/issues/5311 - const dnsDecision = resolveTelegramDnsResultOrderDecision({ network }); - if (dnsDecision.value !== null && dnsDecision.value !== appliedDnsResultOrder) { - if (typeof dns.setDefaultResultOrder === "function") { - try { - dns.setDefaultResultOrder(dnsDecision.value as "ipv4first" | "verbatim"); - appliedDnsResultOrder = dnsDecision.value; - const label = dnsDecision.source ? ` (${dnsDecision.source})` : ""; - log.info(`dnsResultOrder=${dnsDecision.value}${label}`); - } catch { - // ignore if unsupported by the runtime - } + return Object.keys(connect).length > 0 ? connect : null; +} + +function shouldBypassEnvProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env): boolean { + // We need this classification before dispatch to decide whether sticky IPv4 fallback + // can safely arm. EnvHttpProxyAgent does not expose route decisions (proxy vs direct + // NO_PROXY bypass), so we mirror undici's parsing/matching behavior for this host. + // Match EnvHttpProxyAgent behavior (undici): + // - lower-case no_proxy takes precedence over NO_PROXY + // - entries split by comma or whitespace + // - wildcard handling is exact-string "*" only + // - leading "." and "*." are normalized the same way + const noProxyValue = env.no_proxy ?? env.NO_PROXY ?? ""; + if (!noProxyValue) { + return false; + } + if (noProxyValue === "*") { + return true; + } + const targetHostname = TELEGRAM_API_HOSTNAME.toLowerCase(); + const targetPort = 443; + const noProxyEntries = noProxyValue.split(/[,\s]/); + for (let i = 0; i < noProxyEntries.length; i++) { + const entry = noProxyEntries[i]; + if (!entry) { + continue; } + const parsed = entry.match(/^(.+):(\d+)$/); + const entryHostname = (parsed ? parsed[1] : entry).replace(/^\*?\./, "").toLowerCase(); + const entryPort = parsed ? Number.parseInt(parsed[2], 10) : 0; + if (entryPort && entryPort !== targetPort) { + continue; + } + if ( + targetHostname === entryHostname || + targetHostname.slice(-(entryHostname.length + 1)) === `.${entryHostname}` + ) { + return true; + } + } + return false; +} + +function hasEnvHttpProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env): boolean { + // Match EnvHttpProxyAgent behavior (undici) for HTTPS requests: + // - lower-case env vars take precedence over upper-case + // - HTTPS requests use https_proxy/HTTPS_PROXY first, then fall back to http_proxy/HTTP_PROXY + // - ALL_PROXY is ignored by EnvHttpProxyAgent + const httpProxy = env.http_proxy ?? env.HTTP_PROXY; + const httpsProxy = env.https_proxy ?? env.HTTPS_PROXY; + return Boolean(httpProxy) || Boolean(httpsProxy); +} + +function createTelegramDispatcher(params: { + autoSelectFamily: boolean | null; + dnsResultOrder: TelegramDnsResultOrder | null; + useEnvProxy: boolean; + forceIpv4: boolean; + proxyUrl?: string; +}): { dispatcher: TelegramDispatcher; mode: TelegramDispatcherMode } { + const connect = buildTelegramConnectOptions({ + autoSelectFamily: params.autoSelectFamily, + dnsResultOrder: params.dnsResultOrder, + forceIpv4: params.forceIpv4, + }); + const explicitProxyUrl = params.proxyUrl?.trim(); + if (explicitProxyUrl) { + const proxyOptions = connect + ? ({ + uri: explicitProxyUrl, + proxyTls: connect, + } satisfies ConstructorParameters[0]) + : explicitProxyUrl; + try { + return { + dispatcher: new ProxyAgent(proxyOptions), + mode: "explicit-proxy", + }; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + throw new Error(`explicit proxy dispatcher init failed: ${reason}`, { cause: err }); + } + } + if (params.useEnvProxy) { + const proxyOptions = connect + ? ({ + connect, + // undici's EnvHttpProxyAgent passes `connect` only to the no-proxy Agent. + // Real proxied HTTPS traffic reads transport settings from ProxyAgent.proxyTls. + proxyTls: connect, + } satisfies ConstructorParameters[0]) + : undefined; + try { + return { + dispatcher: new EnvHttpProxyAgent(proxyOptions), + mode: "env-proxy", + }; + } catch (err) { + log.warn( + `env proxy dispatcher init failed; falling back to direct dispatcher: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + } + const agentOptions = connect + ? ({ + connect, + } satisfies ConstructorParameters[0]) + : undefined; + return { + dispatcher: new Agent(agentOptions), + mode: "direct", + }; +} + +function withDispatcherIfMissing( + init: RequestInit | undefined, + dispatcher: TelegramDispatcher, +): RequestInitWithDispatcher { + const withDispatcher = init as RequestInitWithDispatcher | undefined; + if (withDispatcher?.dispatcher) { + return init ?? {}; + } + return init ? { ...init, dispatcher } : { dispatcher }; +} + +function resolveWrappedFetch(fetchImpl: typeof fetch): typeof fetch { + return resolveFetch(fetchImpl) ?? fetchImpl; +} + +function logResolverNetworkDecisions(params: { + autoSelectDecision: ReturnType; + dnsDecision: ReturnType; +}): void { + if (params.autoSelectDecision.value !== null) { + const sourceLabel = params.autoSelectDecision.source + ? ` (${params.autoSelectDecision.source})` + : ""; + log.info(`autoSelectFamily=${params.autoSelectDecision.value}${sourceLabel}`); + } + if (params.dnsDecision.value !== null) { + const sourceLabel = params.dnsDecision.source ? ` (${params.dnsDecision.source})` : ""; + log.info(`dnsResultOrder=${params.dnsDecision.value}${sourceLabel}`); } } @@ -151,6 +314,11 @@ function collectErrorCodes(err: unknown): Set { return codes; } +function formatErrorCodes(err: unknown): string { + const codes = [...collectErrorCodes(err)]; + return codes.length > 0 ? codes.join(",") : "none"; +} + function shouldRetryWithIpv4Fallback(err: unknown): boolean { const ctx: Ipv4FallbackContext = { message: @@ -165,44 +333,97 @@ function shouldRetryWithIpv4Fallback(err: unknown): boolean { return true; } -function applyTelegramIpv4Fallback(): void { - applyTelegramNetworkWorkarounds({ - autoSelectFamily: false, - dnsResultOrder: "ipv4first", - }); - log.warn("fetch fallback: forcing autoSelectFamily=false + dnsResultOrder=ipv4first"); -} - // Prefer wrapped fetch when available to normalize AbortSignal across runtimes. export function resolveTelegramFetch( proxyFetch?: typeof fetch, options?: { network?: TelegramNetworkConfig }, -): typeof fetch | undefined { - applyTelegramNetworkWorkarounds(options?.network); - const sourceFetch = proxyFetch ? resolveFetch(proxyFetch) : resolveFetch(); - if (!sourceFetch) { - throw new Error("fetch is not available; set channels.telegram.proxy in config"); - } - // When Telegram media fetch hits dual-stack edge cases (ENETUNREACH/ETIMEDOUT), - // switch to IPv4-safe network mode and retry once. - if (proxyFetch) { +): typeof fetch { + const autoSelectDecision = resolveTelegramAutoSelectFamilyDecision({ + network: options?.network, + }); + const dnsDecision = resolveTelegramDnsResultOrderDecision({ + network: options?.network, + }); + logResolverNetworkDecisions({ + autoSelectDecision, + dnsDecision, + }); + + const explicitProxyUrl = proxyFetch ? getProxyUrlFromFetch(proxyFetch) : undefined; + const undiciSourceFetch = resolveWrappedFetch(undiciFetch as unknown as typeof fetch); + const sourceFetch = explicitProxyUrl + ? undiciSourceFetch + : proxyFetch + ? resolveWrappedFetch(proxyFetch) + : undiciSourceFetch; + + // Preserve fully caller-owned custom fetch implementations. + // OpenClaw proxy fetches are metadata-tagged and continue into resolver-scoped policy. + if (proxyFetch && !explicitProxyUrl) { return sourceFetch; } + + const dnsResultOrder = normalizeDnsResultOrder(dnsDecision.value); + const useEnvProxy = !explicitProxyUrl && hasEnvHttpProxyForTelegramApi(); + const defaultDispatcherResolution = createTelegramDispatcher({ + autoSelectFamily: autoSelectDecision.value, + dnsResultOrder, + useEnvProxy, + forceIpv4: false, + proxyUrl: explicitProxyUrl, + }); + const defaultDispatcher = defaultDispatcherResolution.dispatcher; + const shouldBypassEnvProxy = shouldBypassEnvProxyForTelegramApi(); + const allowStickyIpv4Fallback = + defaultDispatcherResolution.mode === "direct" || + (defaultDispatcherResolution.mode === "env-proxy" && shouldBypassEnvProxy); + const stickyShouldUseEnvProxy = defaultDispatcherResolution.mode === "env-proxy"; + + let stickyIpv4FallbackEnabled = false; + let stickyIpv4Dispatcher: TelegramDispatcher | null = null; + const resolveStickyIpv4Dispatcher = () => { + if (!stickyIpv4Dispatcher) { + stickyIpv4Dispatcher = createTelegramDispatcher({ + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + useEnvProxy: stickyShouldUseEnvProxy, + forceIpv4: true, + proxyUrl: explicitProxyUrl, + }).dispatcher; + } + return stickyIpv4Dispatcher; + }; + return (async (input: RequestInfo | URL, init?: RequestInit) => { + const callerProvidedDispatcher = Boolean( + (init as RequestInitWithDispatcher | undefined)?.dispatcher, + ); + const initialInit = withDispatcherIfMissing( + init, + stickyIpv4FallbackEnabled ? resolveStickyIpv4Dispatcher() : defaultDispatcher, + ); try { - return await sourceFetch(input, init); + return await sourceFetch(input, initialInit); } catch (err) { if (shouldRetryWithIpv4Fallback(err)) { - applyTelegramIpv4Fallback(); - return sourceFetch(input, init); + // Preserve caller-owned dispatchers on retry. + if (callerProvidedDispatcher) { + return sourceFetch(input, init ?? {}); + } + // Proxy routes should not arm sticky IPv4 mode; `family=4` would constrain + // proxy-connect behavior instead of Telegram endpoint selection. + if (!allowStickyIpv4Fallback) { + throw err; + } + if (!stickyIpv4FallbackEnabled) { + stickyIpv4FallbackEnabled = true; + log.warn( + `fetch fallback: enabling sticky IPv4-only dispatcher (codes=${formatErrorCodes(err)})`, + ); + } + return sourceFetch(input, withDispatcherIfMissing(init, resolveStickyIpv4Dispatcher())); } throw err; } }) as typeof fetch; } - -export function resetTelegramFetchStateForTests(): void { - appliedAutoSelectFamily = null; - appliedDnsResultOrder = null; - appliedGlobalDispatcherAutoSelectFamily = null; -} diff --git a/src/telegram/probe.test.ts b/src/telegram/probe.test.ts index 11b0b317eec..7006d14a2f7 100644 --- a/src/telegram/probe.test.ts +++ b/src/telegram/probe.test.ts @@ -1,14 +1,28 @@ -import { type Mock, describe, expect, it, vi } from "vitest"; +import { afterEach, type Mock, describe, expect, it, vi } from "vitest"; import { withFetchPreconnect } from "../test-utils/fetch-mock.js"; -import { probeTelegram } from "./probe.js"; +import { probeTelegram, resetTelegramProbeFetcherCacheForTests } from "./probe.js"; + +const resolveTelegramFetch = vi.hoisted(() => vi.fn()); +const makeProxyFetch = vi.hoisted(() => vi.fn()); + +vi.mock("./fetch.js", () => ({ + resolveTelegramFetch, +})); + +vi.mock("./proxy.js", () => ({ + makeProxyFetch, +})); describe("probeTelegram retry logic", () => { const token = "test-token"; const timeoutMs = 5000; + const originalFetch = global.fetch; const installFetchMock = (): Mock => { const fetchMock = vi.fn(); global.fetch = withFetchPreconnect(fetchMock); + resolveTelegramFetch.mockImplementation((proxyFetch?: typeof fetch) => proxyFetch ?? fetch); + makeProxyFetch.mockImplementation(() => fetchMock as unknown as typeof fetch); return fetchMock; }; @@ -41,6 +55,19 @@ describe("probeTelegram retry logic", () => { expect(result.bot?.username).toBe("test_bot"); } + afterEach(() => { + resetTelegramProbeFetcherCacheForTests(); + resolveTelegramFetch.mockReset(); + makeProxyFetch.mockReset(); + vi.unstubAllEnvs(); + vi.clearAllMocks(); + if (originalFetch) { + global.fetch = originalFetch; + } else { + delete (globalThis as { fetch?: typeof fetch }).fetch; + } + }); + it.each([ { errors: [], @@ -95,6 +122,35 @@ describe("probeTelegram retry logic", () => { } }); + it("respects timeout budget across retries", async () => { + const fetchMock = vi.fn((_input: RequestInfo | URL, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + const signal = init?.signal; + if (signal?.aborted) { + reject(new Error("Request aborted")); + return; + } + signal?.addEventListener("abort", () => reject(new Error("Request aborted")), { + once: true, + }); + }); + }); + global.fetch = withFetchPreconnect(fetchMock as unknown as typeof fetch); + resolveTelegramFetch.mockImplementation((proxyFetch?: typeof fetch) => proxyFetch ?? fetch); + makeProxyFetch.mockImplementation(() => fetchMock as unknown as typeof fetch); + vi.useFakeTimers(); + try { + const probePromise = probeTelegram(`${token}-budget`, 500); + await vi.advanceTimersByTimeAsync(600); + const result = await probePromise; + + expect(result.ok).toBe(false); + expect(fetchMock).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it("should NOT retry if getMe returns a 401 Unauthorized", async () => { const fetchMock = installFetchMock(); const mockResponse = { @@ -114,4 +170,106 @@ describe("probeTelegram retry logic", () => { expect(result.error).toBe("Unauthorized"); expect(fetchMock).toHaveBeenCalledTimes(1); // Should not retry }); + + it("uses resolver-scoped Telegram fetch with probe network options", async () => { + const fetchMock = installFetchMock(); + mockGetMeSuccess(fetchMock); + mockGetWebhookInfoSuccess(fetchMock); + + await probeTelegram(token, timeoutMs, { + proxyUrl: "http://127.0.0.1:8888", + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }); + + expect(makeProxyFetch).toHaveBeenCalledWith("http://127.0.0.1:8888"); + expect(resolveTelegramFetch).toHaveBeenCalledWith(fetchMock, { + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }); + }); + + it("reuses probe fetcher across repeated probes for the same account transport settings", async () => { + const fetchMock = installFetchMock(); + vi.stubEnv("VITEST", ""); + vi.stubEnv("NODE_ENV", "production"); + + mockGetMeSuccess(fetchMock); + mockGetWebhookInfoSuccess(fetchMock); + await probeTelegram(`${token}-cache`, timeoutMs, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + mockGetMeSuccess(fetchMock); + mockGetWebhookInfoSuccess(fetchMock); + await probeTelegram(`${token}-cache`, timeoutMs, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + expect(resolveTelegramFetch).toHaveBeenCalledTimes(1); + }); + + it("does not reuse probe fetcher cache when network settings differ", async () => { + const fetchMock = installFetchMock(); + vi.stubEnv("VITEST", ""); + vi.stubEnv("NODE_ENV", "production"); + + mockGetMeSuccess(fetchMock); + mockGetWebhookInfoSuccess(fetchMock); + await probeTelegram(`${token}-cache-variant`, timeoutMs, { + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + mockGetMeSuccess(fetchMock); + mockGetWebhookInfoSuccess(fetchMock); + await probeTelegram(`${token}-cache-variant`, timeoutMs, { + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }); + + expect(resolveTelegramFetch).toHaveBeenCalledTimes(2); + }); + + it("reuses probe fetcher cache across token rotation when accountId is stable", async () => { + const fetchMock = installFetchMock(); + vi.stubEnv("VITEST", ""); + vi.stubEnv("NODE_ENV", "production"); + + mockGetMeSuccess(fetchMock); + mockGetWebhookInfoSuccess(fetchMock); + await probeTelegram(`${token}-old`, timeoutMs, { + accountId: "main", + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + mockGetMeSuccess(fetchMock); + mockGetWebhookInfoSuccess(fetchMock); + await probeTelegram(`${token}-new`, timeoutMs, { + accountId: "main", + network: { + autoSelectFamily: true, + dnsResultOrder: "ipv4first", + }, + }); + + expect(resolveTelegramFetch).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/telegram/probe.ts b/src/telegram/probe.ts index f988733f0ee..8311506e455 100644 --- a/src/telegram/probe.ts +++ b/src/telegram/probe.ts @@ -1,5 +1,7 @@ import type { BaseProbeResult } from "../channels/plugins/types.js"; +import type { TelegramNetworkConfig } from "../config/types.telegram.js"; import { fetchWithTimeout } from "../utils/fetch-timeout.js"; +import { resolveTelegramFetch } from "./fetch.js"; import { makeProxyFetch } from "./proxy.js"; const TELEGRAM_API_BASE = "https://api.telegram.org"; @@ -17,15 +19,90 @@ export type TelegramProbe = BaseProbeResult & { webhook?: { url?: string | null; hasCustomCert?: boolean | null }; }; +export type TelegramProbeOptions = { + proxyUrl?: string; + network?: TelegramNetworkConfig; + accountId?: string; +}; + +const probeFetcherCache = new Map(); +const MAX_PROBE_FETCHER_CACHE_SIZE = 64; + +export function resetTelegramProbeFetcherCacheForTests(): void { + probeFetcherCache.clear(); +} + +function resolveProbeOptions( + proxyOrOptions?: string | TelegramProbeOptions, +): TelegramProbeOptions | undefined { + if (!proxyOrOptions) { + return undefined; + } + if (typeof proxyOrOptions === "string") { + return { proxyUrl: proxyOrOptions }; + } + return proxyOrOptions; +} + +function shouldUseProbeFetcherCache(): boolean { + return !process.env.VITEST && process.env.NODE_ENV !== "test"; +} + +function buildProbeFetcherCacheKey(token: string, options?: TelegramProbeOptions): string { + const cacheIdentity = options?.accountId?.trim() || token; + const cacheIdentityKind = options?.accountId?.trim() ? "account" : "token"; + const proxyKey = options?.proxyUrl?.trim() ?? ""; + const autoSelectFamily = options?.network?.autoSelectFamily; + const autoSelectFamilyKey = + typeof autoSelectFamily === "boolean" ? String(autoSelectFamily) : "default"; + const dnsResultOrderKey = options?.network?.dnsResultOrder ?? "default"; + return `${cacheIdentityKind}:${cacheIdentity}::${proxyKey}::${autoSelectFamilyKey}::${dnsResultOrderKey}`; +} + +function setCachedProbeFetcher(cacheKey: string, fetcher: typeof fetch): typeof fetch { + probeFetcherCache.set(cacheKey, fetcher); + if (probeFetcherCache.size > MAX_PROBE_FETCHER_CACHE_SIZE) { + const oldestKey = probeFetcherCache.keys().next().value; + if (oldestKey !== undefined) { + probeFetcherCache.delete(oldestKey); + } + } + return fetcher; +} + +function resolveProbeFetcher(token: string, options?: TelegramProbeOptions): typeof fetch { + const cacheEnabled = shouldUseProbeFetcherCache(); + const cacheKey = cacheEnabled ? buildProbeFetcherCacheKey(token, options) : null; + if (cacheKey) { + const cachedFetcher = probeFetcherCache.get(cacheKey); + if (cachedFetcher) { + return cachedFetcher; + } + } + + const proxyUrl = options?.proxyUrl?.trim(); + const proxyFetch = proxyUrl ? makeProxyFetch(proxyUrl) : undefined; + const resolved = resolveTelegramFetch(proxyFetch, { network: options?.network }); + + if (cacheKey) { + return setCachedProbeFetcher(cacheKey, resolved); + } + return resolved; +} + export async function probeTelegram( token: string, timeoutMs: number, - proxyUrl?: string, + proxyOrOptions?: string | TelegramProbeOptions, ): Promise { const started = Date.now(); - const fetcher = proxyUrl ? makeProxyFetch(proxyUrl) : fetch; + const timeoutBudgetMs = Math.max(1, Math.floor(timeoutMs)); + const deadlineMs = started + timeoutBudgetMs; + const options = resolveProbeOptions(proxyOrOptions); + const fetcher = resolveProbeFetcher(token, options); const base = `${TELEGRAM_API_BASE}/bot${token}`; - const retryDelayMs = Math.max(50, Math.min(1000, timeoutMs)); + const retryDelayMs = Math.max(50, Math.min(1000, Math.floor(timeoutBudgetMs / 5))); + const resolveRemainingBudgetMs = () => Math.max(0, deadlineMs - Date.now()); const result: TelegramProbe = { ok: false, @@ -40,19 +117,35 @@ export async function probeTelegram( // Retry loop for initial connection (handles network/DNS startup races) for (let i = 0; i < 3; i++) { + const remainingBudgetMs = resolveRemainingBudgetMs(); + if (remainingBudgetMs <= 0) { + break; + } try { - meRes = await fetchWithTimeout(`${base}/getMe`, {}, timeoutMs, fetcher); + meRes = await fetchWithTimeout( + `${base}/getMe`, + {}, + Math.max(1, Math.min(timeoutBudgetMs, remainingBudgetMs)), + fetcher, + ); break; } catch (err) { fetchError = err; if (i < 2) { - await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); + const remainingAfterAttemptMs = resolveRemainingBudgetMs(); + if (remainingAfterAttemptMs <= 0) { + break; + } + const delayMs = Math.min(retryDelayMs, remainingAfterAttemptMs); + if (delayMs > 0) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } } } } if (!meRes) { - throw fetchError; + throw fetchError ?? new Error(`probe timed out after ${timeoutBudgetMs}ms`); } const meJson = (await meRes.json()) as { @@ -89,16 +182,24 @@ export async function probeTelegram( // Try to fetch webhook info, but don't fail health if it errors. try { - const webhookRes = await fetchWithTimeout(`${base}/getWebhookInfo`, {}, timeoutMs, fetcher); - const webhookJson = (await webhookRes.json()) as { - ok?: boolean; - result?: { url?: string; has_custom_certificate?: boolean }; - }; - if (webhookRes.ok && webhookJson?.ok) { - result.webhook = { - url: webhookJson.result?.url ?? null, - hasCustomCert: webhookJson.result?.has_custom_certificate ?? null, + const webhookRemainingBudgetMs = resolveRemainingBudgetMs(); + if (webhookRemainingBudgetMs > 0) { + const webhookRes = await fetchWithTimeout( + `${base}/getWebhookInfo`, + {}, + Math.max(1, Math.min(timeoutBudgetMs, webhookRemainingBudgetMs)), + fetcher, + ); + const webhookJson = (await webhookRes.json()) as { + ok?: boolean; + result?: { url?: string; has_custom_certificate?: boolean }; }; + if (webhookRes.ok && webhookJson?.ok) { + result.webhook = { + url: webhookJson.result?.url ?? null, + hasCustomCert: webhookJson.result?.has_custom_certificate ?? null, + }; + } } } catch { // ignore webhook errors for probe diff --git a/src/telegram/proxy.test.ts b/src/telegram/proxy.test.ts index 27065d5c50c..4f2ca8f62e6 100644 --- a/src/telegram/proxy.test.ts +++ b/src/telegram/proxy.test.ts @@ -29,7 +29,7 @@ vi.mock("undici", () => ({ setGlobalDispatcher: mocks.setGlobalDispatcher, })); -import { makeProxyFetch } from "./proxy.js"; +import { getProxyUrlFromFetch, makeProxyFetch } from "./proxy.js"; describe("makeProxyFetch", () => { it("uses undici fetch with ProxyAgent dispatcher", async () => { @@ -46,4 +46,11 @@ describe("makeProxyFetch", () => { ); expect(mocks.setGlobalDispatcher).not.toHaveBeenCalled(); }); + + it("attaches proxy metadata for resolver transport handling", () => { + const proxyUrl = "http://proxy.test:8080"; + const proxyFetch = makeProxyFetch(proxyUrl); + + expect(getProxyUrlFromFetch(proxyFetch)).toBe(proxyUrl); + }); }); diff --git a/src/telegram/proxy.ts b/src/telegram/proxy.ts index c4cb7129a17..3ac2bb10159 100644 --- a/src/telegram/proxy.ts +++ b/src/telegram/proxy.ts @@ -1 +1 @@ -export { makeProxyFetch } from "../infra/net/proxy-fetch.js"; +export { getProxyUrlFromFetch, makeProxyFetch } from "../infra/net/proxy-fetch.js"; diff --git a/src/telegram/send.proxy.test.ts b/src/telegram/send.proxy.test.ts index ee47ec765c4..8e16078a67c 100644 --- a/src/telegram/send.proxy.test.ts +++ b/src/telegram/send.proxy.test.ts @@ -51,7 +51,12 @@ vi.mock("grammy", () => ({ InputFile: class {}, })); -import { deleteMessageTelegram, reactMessageTelegram, sendMessageTelegram } from "./send.js"; +import { + deleteMessageTelegram, + reactMessageTelegram, + resetTelegramClientOptionsCacheForTests, + sendMessageTelegram, +} from "./send.js"; describe("telegram proxy client", () => { const proxyUrl = "http://proxy.test:8080"; @@ -76,6 +81,8 @@ describe("telegram proxy client", () => { }; beforeEach(() => { + resetTelegramClientOptionsCacheForTests(); + vi.unstubAllEnvs(); botApi.sendMessage.mockResolvedValue({ message_id: 1, chat: { id: "123" } }); botApi.setMessageReaction.mockResolvedValue(undefined); botApi.deleteMessage.mockResolvedValue(true); @@ -87,6 +94,33 @@ describe("telegram proxy client", () => { resolveTelegramFetch.mockClear(); }); + it("reuses cached Telegram client options for repeated sends with same account transport settings", async () => { + const { fetchImpl } = prepareProxyFetch(); + vi.stubEnv("VITEST", ""); + vi.stubEnv("NODE_ENV", "production"); + + await sendMessageTelegram("123", "first", { token: "tok", accountId: "foo" }); + await sendMessageTelegram("123", "second", { token: "tok", accountId: "foo" }); + + expect(makeProxyFetch).toHaveBeenCalledTimes(1); + expect(resolveTelegramFetch).toHaveBeenCalledTimes(1); + expect(botCtorSpy).toHaveBeenCalledTimes(2); + expect(botCtorSpy).toHaveBeenNthCalledWith( + 1, + "tok", + expect.objectContaining({ + client: expect.objectContaining({ fetch: fetchImpl }), + }), + ); + expect(botCtorSpy).toHaveBeenNthCalledWith( + 2, + "tok", + expect.objectContaining({ + client: expect.objectContaining({ fetch: fetchImpl }), + }), + ); + }); + it.each([ { name: "sendMessage", diff --git a/src/telegram/send.ts b/src/telegram/send.ts index e1b352a0a61..313abf361e8 100644 --- a/src/telegram/send.ts +++ b/src/telegram/send.ts @@ -115,6 +115,12 @@ const MESSAGE_NOT_MODIFIED_RE = const CHAT_NOT_FOUND_RE = /400: Bad Request: chat not found/i; const sendLogger = createSubsystemLogger("telegram/send"); const diagLogger = createSubsystemLogger("telegram/diagnostic"); +const telegramClientOptionsCache = new Map(); +const MAX_TELEGRAM_CLIENT_OPTIONS_CACHE_SIZE = 64; + +export function resetTelegramClientOptionsCacheForTests(): void { + telegramClientOptionsCache.clear(); +} function createTelegramHttpLogger(cfg: ReturnType) { const enabled = isDiagnosticFlagEnabled("telegram.http", cfg); @@ -130,25 +136,74 @@ function createTelegramHttpLogger(cfg: ReturnType) { }; } +function shouldUseTelegramClientOptionsCache(): boolean { + return !process.env.VITEST && process.env.NODE_ENV !== "test"; +} + +function buildTelegramClientOptionsCacheKey(params: { + account: ResolvedTelegramAccount; + timeoutSeconds?: number; +}): string { + const proxyKey = params.account.config.proxy?.trim() ?? ""; + const autoSelectFamily = params.account.config.network?.autoSelectFamily; + const autoSelectFamilyKey = + typeof autoSelectFamily === "boolean" ? String(autoSelectFamily) : "default"; + const dnsResultOrderKey = params.account.config.network?.dnsResultOrder ?? "default"; + const timeoutSecondsKey = + typeof params.timeoutSeconds === "number" ? String(params.timeoutSeconds) : "default"; + return `${params.account.accountId}::${proxyKey}::${autoSelectFamilyKey}::${dnsResultOrderKey}::${timeoutSecondsKey}`; +} + +function setCachedTelegramClientOptions( + cacheKey: string, + clientOptions: ApiClientOptions | undefined, +): ApiClientOptions | undefined { + telegramClientOptionsCache.set(cacheKey, clientOptions); + if (telegramClientOptionsCache.size > MAX_TELEGRAM_CLIENT_OPTIONS_CACHE_SIZE) { + const oldestKey = telegramClientOptionsCache.keys().next().value; + if (oldestKey !== undefined) { + telegramClientOptionsCache.delete(oldestKey); + } + } + return clientOptions; +} + function resolveTelegramClientOptions( account: ResolvedTelegramAccount, ): ApiClientOptions | undefined { - const proxyUrl = account.config.proxy?.trim(); - const proxyFetch = proxyUrl ? makeProxyFetch(proxyUrl) : undefined; - const fetchImpl = resolveTelegramFetch(proxyFetch, { - network: account.config.network, - }); const timeoutSeconds = typeof account.config.timeoutSeconds === "number" && Number.isFinite(account.config.timeoutSeconds) ? Math.max(1, Math.floor(account.config.timeoutSeconds)) : undefined; - return fetchImpl || timeoutSeconds - ? { - ...(fetchImpl ? { fetch: fetchImpl as unknown as ApiClientOptions["fetch"] } : {}), - ...(timeoutSeconds ? { timeoutSeconds } : {}), - } - : undefined; + + const cacheEnabled = shouldUseTelegramClientOptionsCache(); + const cacheKey = cacheEnabled + ? buildTelegramClientOptionsCacheKey({ + account, + timeoutSeconds, + }) + : null; + if (cacheKey && telegramClientOptionsCache.has(cacheKey)) { + return telegramClientOptionsCache.get(cacheKey); + } + + const proxyUrl = account.config.proxy?.trim(); + const proxyFetch = proxyUrl ? makeProxyFetch(proxyUrl) : undefined; + const fetchImpl = resolveTelegramFetch(proxyFetch, { + network: account.config.network, + }); + const clientOptions = + fetchImpl || timeoutSeconds + ? { + ...(fetchImpl ? { fetch: fetchImpl as unknown as ApiClientOptions["fetch"] } : {}), + ...(timeoutSeconds ? { timeoutSeconds } : {}), + } + : undefined; + if (cacheKey) { + return setCachedTelegramClientOptions(cacheKey, clientOptions); + } + return clientOptions; } function resolveToken(explicit: string | undefined, params: { accountId: string; token: string }) { From 8306eabf85ea0c08e02fb0e45c697e22e77dd8c6 Mon Sep 17 00:00:00 2001 From: Frank Yang Date: Tue, 10 Mar 2026 14:18:41 +0800 Subject: [PATCH 012/126] fix(agents): forward memory flush write path (#41761) Merged via squash. Prepared head SHA: 0a8ebf8e5b426c5b402adc34509830f46e4bb849 Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com> Co-authored-by: frankekn <4488090+frankekn@users.noreply.github.com> Reviewed-by: @frankekn --- CHANGELOG.md | 1 + src/agents/pi-embedded-runner/run.ts | 1 + .../usage-reporting.test.ts | 30 +++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e2e65653c0..c3a2c9c2d75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ Docs: https://docs.openclaw.ai - Tools/web search: recover OpenRouter Perplexity citation extraction from `message.annotations` when chat-completions responses omit top-level citations. (#40881) Thanks @laurieluo. - Security/external content: treat whitespace-delimited `EXTERNAL UNTRUSTED CONTENT` boundary markers like underscore-delimited variants so prompt wrappers cannot bypass marker sanitization. (#35983) Thanks @urianpaul94. - Telegram/network env-proxy: apply configured transport policy to proxied HTTPS dispatchers as well as direct `NO_PROXY` bypasses, so resolver-scoped IPv4 fallback and network settings work consistently for env-proxied Telegram traffic. (#40740) Thanks @sircrumpet. +- Agents/memory flush: forward `memoryFlushWritePath` through `runEmbeddedPiAgent` so memory-triggered flush turns keep the append-only write guard without aborting before tool setup. Follows up on #38574. (#41761) Thanks @frankekn. ## 2026.3.8 diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index 298bac9fe9e..7f5f4f525b7 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -850,6 +850,7 @@ export async function runEmbeddedPiAgent( sessionId: params.sessionId, sessionKey: params.sessionKey, trigger: params.trigger, + memoryFlushWritePath: params.memoryFlushWritePath, messageChannel: params.messageChannel, messageProvider: params.messageProvider, agentAccountId: params.agentAccountId, diff --git a/src/agents/pi-embedded-runner/usage-reporting.test.ts b/src/agents/pi-embedded-runner/usage-reporting.test.ts index 48cb586e727..ebab56a841b 100644 --- a/src/agents/pi-embedded-runner/usage-reporting.test.ts +++ b/src/agents/pi-embedded-runner/usage-reporting.test.ts @@ -79,6 +79,36 @@ describe("runEmbeddedPiAgent usage reporting", () => { ); }); + it("forwards memory flush write paths into memory-triggered attempts", async () => { + mockedRunEmbeddedAttempt.mockResolvedValueOnce({ + aborted: false, + promptError: null, + timedOut: false, + sessionIdUsed: "test-session", + assistantTexts: [], + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + + await runEmbeddedPiAgent({ + sessionId: "test-session", + sessionKey: "test-key", + sessionFile: "/tmp/session.json", + workspaceDir: "/tmp/workspace", + prompt: "flush", + timeoutMs: 30000, + runId: "run-memory-forwarding", + trigger: "memory", + memoryFlushWritePath: "memory/2026-03-10.md", + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledWith( + expect.objectContaining({ + trigger: "memory", + memoryFlushWritePath: "memory/2026-03-10.md", + }), + ); + }); + it("reports total usage from the last turn instead of accumulated total", async () => { // Simulate a multi-turn run result. // Turn 1: Input 100, Output 50. Total 150. From 5296147c20954607e8336191035de7ff2f51e571 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Tue, 10 Mar 2026 01:22:41 -0500 Subject: [PATCH 013/126] CI: select Swift 6.2 toolchain for CodeQL (#41787) Merged via squash. Prepared head SHA: 8abc6c16571661450a6b932de17b74607ecacb8e Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com> Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com> Reviewed-by: @BunsDev --- .github/workflows/codeql.yml | 6 +++++- CHANGELOG.md | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 9b78a3c6172..1d8e473af4f 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -93,7 +93,11 @@ jobs: - name: Setup Swift build tools if: matrix.needs_swift_tools - run: brew install xcodegen swiftlint swiftformat + run: | + sudo xcode-select -s /Applications/Xcode_26.1.app + xcodebuild -version + brew install xcodegen swiftlint swiftformat + swift --version - name: Initialize CodeQL uses: github/codeql-action/init@v4 diff --git a/CHANGELOG.md b/CHANGELOG.md index c3a2c9c2d75..ecb7cd16680 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Docs: https://docs.openclaw.ai - Security/external content: treat whitespace-delimited `EXTERNAL UNTRUSTED CONTENT` boundary markers like underscore-delimited variants so prompt wrappers cannot bypass marker sanitization. (#35983) Thanks @urianpaul94. - Telegram/network env-proxy: apply configured transport policy to proxied HTTPS dispatchers as well as direct `NO_PROXY` bypasses, so resolver-scoped IPv4 fallback and network settings work consistently for env-proxied Telegram traffic. (#40740) Thanks @sircrumpet. - Agents/memory flush: forward `memoryFlushWritePath` through `runEmbeddedPiAgent` so memory-triggered flush turns keep the append-only write guard without aborting before tool setup. Follows up on #38574. (#41761) Thanks @frankekn. +- CI/CodeQL Swift toolchain: select Xcode 26.1 before installing Swift build tools so the CodeQL Swift job uses Swift tools 6.2 on `macos-latest`. (#41787) thanks @BunsDev. ## 2026.3.8 From 9d403fd4154ff4eb34aed3e91b4650c8797e65ff Mon Sep 17 00:00:00 2001 From: Austin <112558420+rixau@users.noreply.github.com> Date: Tue, 10 Mar 2026 02:30:31 -0400 Subject: [PATCH 014/126] fix(ui): replace Manual RPC text input with sorted method dropdown (#14967) Merged via squash. Prepared head SHA: 1bb49b2e64675d37882d0975eb19f8fafd3c6fe9 Co-authored-by: rixau <112558420+rixau@users.noreply.github.com> Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com> Reviewed-by: @BunsDev --- CHANGELOG.md | 1 + ui/src/ui/app-render.ts | 1 + ui/src/ui/views/debug.ts | 19 ++++++++++++++----- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecb7cd16680..f7574f71eb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,7 @@ Docs: https://docs.openclaw.ai - MS Teams/authz: keep `groupPolicy: "allowlist"` enforcing sender allowlists even when a team/channel route allowlist is configured, so route matches no longer widen group access to every sender in that route. Thanks @zpbrent. - Security/system.run: bind approved `bun` and `deno run` script operands to on-disk file snapshots so post-approval script rewrites are denied before execution. - Skills/download installs: pin the validated per-skill tools root before writing downloaded archives, so rebinding the lexical tools path cannot redirect download writes outside the intended tools directory. Thanks @tdjackey. +- Control UI/Debug: replace the Manual RPC free-text method field with a sorted dropdown sourced from gateway-advertised methods, and stack the form vertically for narrower layouts. (#14967) thanks @rixau. ## 2026.3.7 diff --git a/ui/src/ui/app-render.ts b/ui/src/ui/app-render.ts index 7fbe38c9ca7..1214bcc93a6 100644 --- a/ui/src/ui/app-render.ts +++ b/ui/src/ui/app-render.ts @@ -1081,6 +1081,7 @@ export function renderApp(state: AppViewState) { models: state.debugModels, heartbeat: state.debugHeartbeat, eventLog: state.eventLog, + methods: (state.hello?.features?.methods ?? []).toSorted(), callMethod: state.debugCallMethod, callParams: state.debugCallParams, callResult: state.debugCallResult, diff --git a/ui/src/ui/views/debug.ts b/ui/src/ui/views/debug.ts index 9ca33725993..3379e881345 100644 --- a/ui/src/ui/views/debug.ts +++ b/ui/src/ui/views/debug.ts @@ -9,6 +9,7 @@ export type DebugProps = { models: unknown[]; heartbeat: unknown; eventLog: EventLogEntry[]; + methods: string[]; callMethod: string; callParams: string; callResult: string | null; @@ -71,14 +72,22 @@ export function renderDebug(props: DebugProps) {
Manual RPC
Send a raw gateway method with JSON params.
-
+