fix(webauth): sync models.providers to config after webauth, remove s…#93
Open
NghiaDinh03 wants to merge 2 commits intolinuxhsj:mainfrom
Open
fix(webauth): sync models.providers to config after webauth, remove s…#93NghiaDinh03 wants to merge 2 commits intolinuxhsj:mainfrom
NghiaDinh03 wants to merge 2 commits intolinuxhsj:mainfrom
Conversation
📝 WalkthroughWalkthrough移除并删除了两个一键启动脚本（ Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant AuthScript as "loginPerplexityWeb\n(src/providers/perplexity-web-auth.ts)"
participant Browser as "Browser (Playwright/CDP)"
participant Perplexity as "perplexity.ai"
participant Result as "Auth Result"
User->>AuthScript: 启动登录流程
AuthScript->>Browser: 启动或附加到 Chrome/CDP
Browser->>Perplexity: 导航到 perplexity.ai
User->>Browser: 完成交互式登录
Browser->>AuthScript: 捕获 cookies 与 userAgent
AuthScript->>Result: 返回 { cookie, userAgent }
sequenceDiagram
participant App as "客户端 / StreamFn 调用"
participant PerplexityClient as "PerplexityWebClientBrowser\n(src/agents/perplexity-web-stream.ts)"
participant PerplexityAPI as "perplexity.ai chatCompletions"
participant AssistantStream as "Assistant Stream（事件）"
App->>PerplexityClient: 调用 StreamFn（包含 cookie/json、提示等）
PerplexityClient->>PerplexityAPI: 发送合并后的 prompt
PerplexityAPI-->>PerplexityClient: 增量数据流（data: lines）
PerplexityClient->>AssistantStream: emit text_start（首段）/text_delta（增量）
PerplexityClient->>AssistantStream: 插入 tool result block（如有）
PerplexityAPI-->>PerplexityClient: 完成响应
PerplexityClient->>AssistantStream: emit done（包含 AssistantMessage 与元数据）
代码审查工作量🎯 4 (Complex) | ⏱️ ~45 minutes 诗歌
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/commands/onboard-web-auth.ts (1)
304-307: 建议按“本轮授权成功”再触发同步。当前只要有选择就同步（
selectedProviders.length > 0），即使本轮全部授权失败也会执行一次同步。建议改为“至少一个 provider 授权成功后再同步”。🔧 建议修复
+ let successfulAuthCount = 0; // 逐个授权 for (const provider of selectedProviders) { @@ console.log(` ✓ ${provider.name} 授权成功!`); + successfulAuthCount += 1; } catch (error) { console.error(` ✗ ${provider.name} 授权失败:`, error); } } // 将 agent models.json 的 providers 同步到 openclaw.json，避免首次运行时报错 - if (selectedProviders.length > 0) { + if (successfulAuthCount > 0) { await syncModelsProvidersToConfig(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/onboard-web-auth.ts` around lines 304 - 307, Change the sync trigger so it runs only when at least one provider was authorized successfully: instead of checking selectedProviders.length use the collection that represents successful authorizations (e.g., authorizedProviders, successfulProviders, or filter selectedProviders by a success flag returned from the authorization step) and call syncModelsProvidersToConfig() only when that array's length > 0; update the code around the authorization result handling where selectedProviders are processed so it produces this successful array and then uses it to decide whether to call syncModelsProvidersToConfig().src/agents/perplexity-web-stream.ts (4)
14-17: 空操作函数应移除或实现
stripForWebProvider函数当前只是原样返回输入，没有任何实际处理逻辑。如果这是待实现的功能，建议添加 TODO 注释说明；如果不需要，应移除以避免代码混淆。♻️ 建议移除或添加说明
如果暂时不需要此功能：
-// Helper to strip messages for web providers -function stripForWebProvider(prompt: string): string { - return prompt; -}然后同时移除第 63 行的调用：
- if (m.role === "user" && content) { - content = stripForWebProvider(content) || content; - }或者，如果计划实现此功能，添加 TODO 注释：
// Helper to strip messages for web providers +// TODO: Implement stripping logic for web provider compatibility function stripForWebProvider(prompt: string): string { return prompt; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/perplexity-web-stream.ts` around lines 14 - 17, The helper function stripForWebProvider currently returns the input unchanged; either remove this no-op function and delete all its call sites in this file (search for stripForWebProvider and remove the invocation), or implement the intended behavior and mark it with a clear TODO explaining what should be stripped (and add unit tests). Update callers accordingly so there are no leftover no-op calls.
71-71: 考虑移除或条件化调试日志
console.log在生产环境中可能产生噪音。建议使用结构化日志工具，或者添加条件判断仅在调试模式下输出。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/perplexity-web-stream.ts` at line 71, 日志语句 console.log(`[PerplexityWebStream] Starting run`) 在 PerplexityWebStream 中会在生产环境制造噪音；请将其改为结构化/可控的日志调用（例如使用现有的 logger/processLogger 的 debug/info 方法）或包裹在调试条件下（例如基于 NODE_ENV 或 DEBUG 标志），确保仅在开发/调试模式输出，且日志级别与项目日志约定一致。
36-47: 类型断言可改进为更安全的方式多处使用
as unknown as进行类型转换（第 37、47 行），这绕过了 TypeScript 的类型检查。考虑为context参数定义更精确的类型，或使用类型守卫进行安全检查。♻️ 建议使用类型守卫
+interface PerplexityContext { + messages?: Array<{ role: string; content: unknown }>; + systemPrompt?: string; +} + +function isToolResultMessage(m: unknown): m is ToolResultMessage { + return typeof m === 'object' && m !== null && (m as { role?: string }).role === 'toolResult'; +} + return (model, context, streamOptions) => { const stream = createAssistantMessageEventStream(); const run = async () => { try { await client.init(); const messages = context.messages || []; - const systemPrompt = (context as unknown as { systemPrompt?: string }).systemPrompt || ""; + const systemPrompt = (context as PerplexityContext).systemPrompt || "";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/perplexity-web-stream.ts` around lines 36 - 47, The code uses unsafe "as unknown as" casts for context and ToolResultMessage; replace them with precise types or type guards: add a Context type with optional systemPrompt and messages, update the function signature to accept Context, and create type guard functions (e.g., isContextWithSystemPrompt(ctx): ctx is Context & { systemPrompt: string } and isToolResultMessage(m): m is ToolResultMessage) and use these when reading systemPrompt and when treating a message as ToolResultMessage (replace the casts around context and the m cast). Also update the branches that check m.role to narrow types via those guards so historyParts construction and the ToolResultMessage handling are type-safe without "as unknown as".
113-115: 静默忽略 JSON 解析错误可能隐藏问题空的 catch 块会吞掉所有解析错误，使得调试变得困难。建议至少在开发环境记录警告，或者对特定预期的非 JSON 数据进行更明确的处理。
♻️ 建议添加调试日志
} catch { - // ignore + // Non-JSON SSE data is expected for keep-alive or metadata lines + // Uncomment for debugging: console.debug('[PerplexityWebStream] Non-JSON line:', dataStr); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/perplexity-web-stream.ts` around lines 113 - 115, The empty catch in src/agents/perplexity-web-stream.ts should be replaced to surface JSON parse errors: catch the exception (e) instead of swallowing it, and in the function handling the incoming SSE/chunk parsing (perplexityWebStream or the function that calls JSON.parse for event chunks) log a warning/debug entry (e.g., console.warn or processLogger.warn) when NODE_ENV !== 'production' or when running tests, include the raw chunk and the error, and only silently ignore expected non-JSON cases by explicitly checking for SyntaxError and known non-JSON payload patterns before swallowing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agents/perplexity-web-stream.ts`:
- Around line 145-160: Replace the ad-hoc typed error object (the object pushed
to stream inside the catch block) and the trailing "as any" with a call to the
typed helper buildStreamErrorAssistantMessage: import
buildStreamErrorAssistantMessage from src/agents/stream-message-shared.ts, build
the error payload using the caught err (err instanceof Error ? err.message :
String(err)), model info (model.api, model.provider, model.id), usage defaults,
and timestamp, then push the result of buildStreamErrorAssistantMessage(...) to
stream (instead of the current object and "as any"); update the catch block in
perplexity-web-stream.ts to use buildStreamErrorAssistantMessage so TypeScript
infers the correct type.
In `@src/commands/onboard-web-auth.ts`:
- Around line 146-152: The current selection of firstEntry from providers can
pick non-web providers because the find predicate doesn't check providerId;
update the predicate used when computing firstEntry to require the provider key
(providerId) to end with "-web" (e.g., add providerId.endsWith("-web") to the
condition), so only web providers are considered while keeping the existing
checks for p being an object and having a non-empty models array.
- Around line 127-129: The empty catch block after the config sync (the
try/catch in src/commands/onboard-web-auth.ts) is silently swallowing errors;
update the catch to accept an error (e.g. catch (err)) and log a warning with
the error details instead of just returning. Distinguish common cases: if
err.code === 'ENOENT' log a specific "file missing" warning, if err is a
SyntaxError (JSON.parse) log a "JSON parse" warning including err.message,
otherwise log a generic warning with the full error; keep the current control
flow (return or rethrow) after logging so failures are observable.
---
Nitpick comments:
In `@src/agents/perplexity-web-stream.ts`:
- Around line 14-17: The helper function stripForWebProvider currently returns
the input unchanged; either remove this no-op function and delete all its call
sites in this file (search for stripForWebProvider and remove the invocation),
or implement the intended behavior and mark it with a clear TODO explaining what
should be stripped (and add unit tests). Update callers accordingly so there are
no leftover no-op calls.
- Line 71: 日志语句 console.log(`[PerplexityWebStream] Starting run`) 在
PerplexityWebStream 中会在生产环境制造噪音；请将其改为结构化/可控的日志调用（例如使用现有的 logger/processLogger 的
debug/info 方法）或包裹在调试条件下（例如基于 NODE_ENV 或 DEBUG 标志），确保仅在开发/调试模式输出，且日志级别与项目日志约定一致。
- Around line 36-47: The code uses unsafe "as unknown as" casts for context and
ToolResultMessage; replace them with precise types or type guards: add a Context
type with optional systemPrompt and messages, update the function signature to
accept Context, and create type guard functions (e.g.,
isContextWithSystemPrompt(ctx): ctx is Context & { systemPrompt: string } and
isToolResultMessage(m): m is ToolResultMessage) and use these when reading
systemPrompt and when treating a message as ToolResultMessage (replace the casts
around context and the m cast). Also update the branches that check m.role to
narrow types via those guards so historyParts construction and the
ToolResultMessage handling are type-safe without "as unknown as".
- Around line 113-115: The empty catch in src/agents/perplexity-web-stream.ts
should be replaced to surface JSON parse errors: catch the exception (e) instead
of swallowing it, and in the function handling the incoming SSE/chunk parsing
(perplexityWebStream or the function that calls JSON.parse for event chunks) log
a warning/debug entry (e.g., console.warn or processLogger.warn) when NODE_ENV
!== 'production' or when running tests, include the raw chunk and the error, and
only silently ignore expected non-JSON cases by explicitly checking for
SyntaxError and known non-JSON payload patterns before swallowing.
In `@src/commands/onboard-web-auth.ts`:
- Around line 304-307: Change the sync trigger so it runs only when at least one
provider was authorized successfully: instead of checking
selectedProviders.length use the collection that represents successful
authorizations (e.g., authorizedProviders, successfulProviders, or filter
selectedProviders by a success flag returned from the authorization step) and
call syncModelsProvidersToConfig() only when that array's length > 0; update the
code around the authorization result handling where selectedProviders are
processed so it produces this successful array and then uses it to decide
whether to call syncModelsProvidersToConfig().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7990eec-23c6-446b-bdb8-6b357cd40fad
📒 Files selected for processing (7)
README.mdREADME.zh-CN.mdsrc/agents/perplexity-web-stream.tssrc/commands/onboard-web-auth.tsstart-chrome-debug.shstart-quick.shstart.sh
💤 Files with no reviewable changes (3)
- start.sh
- README.md
- start-quick.sh
Comment on lines
+145
to
+160
| } catch (err) { | ||
| stream.push({ | ||
| type: "error", | ||
| reason: "error", | ||
| error: { | ||
| role: "assistant", | ||
| content: [], | ||
| stopReason: "error", | ||
| errorMessage: err instanceof Error ? err.message : String(err), | ||
| api: model.api, | ||
| provider: model.provider, | ||
| model: model.id, | ||
| usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, | ||
| timestamp: Date.now(), | ||
| }, | ||
| } as any); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查 AssistantMessageEventStream 的类型定义，确认支持哪些事件类型
ast-grep --pattern 'type $_ = {
$$$
type: $_
$$$
}'
# 搜索其他文件中如何处理 error 事件
rg -n "type.*error" --type ts -C3Repository: linuxhsj/openclaw-zero-token
Length of output: 50385
🏁 Script executed:
#!/bin/bash
# Find the AssistantMessageEventStream type definition
fd -t f "\.ts$" -x grep -l "AssistantMessageEventStream\|createAssistantMessageEventStream" {}
# Also look at the perplexity-web-stream.ts file directly
head -20 src/agents/perplexity-web-stream.tsRepository: linuxhsj/openclaw-zero-token
Length of output: 1404
🏁 Script executed:
#!/bin/bash
# Check imports and type definitions in perplexity-web-stream.ts
grep -n "import\|type\|interface" src/agents/perplexity-web-stream.ts | head -30Repository: linuxhsj/openclaw-zero-token
Length of output: 1084
🏁 Script executed:
#!/bin/bash
# Look at similar agent files to see how they handle errors
grep -A 20 "catch (err)" src/agents/claude-web-stream.ts | head -30
grep -A 20 "catch (err)" src/agents/deepseek-web-stream.ts | head -30
grep -A 20 "catch (err)" src/agents/openai-ws-stream.ts | head -30Repository: linuxhsj/openclaw-zero-token
Length of output: 1391
🏁 Script executed:
#!/bin/bash
# Check what the actual stream.push calls look like in the perplexity file
sed -n '140,165p' src/agents/perplexity-web-stream.tsRepository: linuxhsj/openclaw-zero-token
Length of output: 1012
🏁 Script executed:
#!/bin/bash
# Let's examine the deepseek file to see if it has the same pattern
sed -n '140,165p' src/agents/deepseek-web-stream.ts
# Also check other files
rg "stream\.push.*type.*error" --type ts -A 3 -B 1Repository: linuxhsj/openclaw-zero-token
Length of output: 1079
🏁 Script executed:
#!/bin/bash
# Let's look for type definitions related to stream events
rg "type.*Event\|interface.*Event" src/agents/ --type ts | grep -i stream
# Also search for createAssistantMessageEventStream usage/definition
rg "createAssistantMessageEventStream" --type ts -B 2 -A 5 | head -50Repository: linuxhsj/openclaw-zero-token
Length of output: 2874
🏁 Script executed:
#!/bin/bash
# Search for error handling in all web stream files to see the pattern
rg "stream\.push.*type.*error" src/agents/ --type ts -A 10 | head -100
# Also specifically look at chatgpt, claude, and other agents
grep -n "as any" src/agents/chatgpt-web-stream.ts
grep -n "as any" src/agents/claude-web-stream.ts
grep -n "as any" src/agents/gemini-web-stream.tsRepository: linuxhsj/openclaw-zero-token
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Check if there are any TypeScript config or declaration files that might help
find . -name "*.d.ts" -o -name "tsconfig.json" | head -10
# Look for pi-ai package or its types in node_modules or package references
cat package.json | grep -A 2 -B 2 pi-aiRepository: linuxhsj/openclaw-zero-token
Length of output: 612
🏁 Script executed:
#!/bin/bash
# Let's see all the files using stream.push with type "error" and check for as any
rg "stream\.push.*type.*error" src/agents/ --type ts -B 5 -A 15
# Check if any file handles error without as any
rg "type.*error.*reason" src/agents/ --type ts -B 3 -A 8Repository: linuxhsj/openclaw-zero-token
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Let's search for all stream.push calls with error handling
rg "stream\.push" src/agents/ --type ts -B 3 -A 8 | grep -A 15 "type.*error"Repository: linuxhsj/openclaw-zero-token
Length of output: 13296
🏁 Script executed:
#!/bin/bash
# Check the actual line counts and content around line 147-160 in perplexity-web-stream.ts
wc -l src/agents/perplexity-web-stream.ts
# Get the full catch block
sed -n '145,165p' src/agents/perplexity-web-stream.tsRepository: linuxhsj/openclaw-zero-token
Length of output: 768
🏁 Script executed:
#!/bin/bash
# Let's examine ollama-stream.ts to see how it handles errors differently
sed -n '140,170p' src/agents/ollama-stream.ts
# Also check buildStreamErrorAssistantMessage
rg "buildStreamErrorAssistantMessage" --type ts -B 2 -A 10Repository: linuxhsj/openclaw-zero-token
Length of output: 4464
🏁 Script executed:
#!/bin/bash
# Check what files use as any for stream.push with error
grep -l "as any.*stream.push\|stream.push.*as any" src/agents/*.ts
# Count how many files have this pattern
grep -r "stream.push.*as any" src/agents/ --type ts | wc -lRepository: linuxhsj/openclaw-zero-token
Length of output: 208
🏁 Script executed:
#!/bin/bash
# Check if perplexity-web-stream.ts imports buildStreamErrorAssistantMessage
grep "buildStreamErrorAssistantMessage\|stream-message-shared" src/agents/perplexity-web-stream.ts
# Check the full imports section
head -20 src/agents/perplexity-web-stream.tsRepository: linuxhsj/openclaw-zero-token
Length of output: 669
🏁 Script executed:
#!/antml/bin
# Let's also verify the type signature of buildStreamErrorAssistantMessage
sed -n '1,50p' src/agents/stream-message-shared.ts | grep -A 15 "buildStreamErrorAssistantMessage"Repository: linuxhsj/openclaw-zero-token
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Let's look at the full stream-message-shared.ts file
cat src/agents/stream-message-shared.ts | head -80Repository: linuxhsj/openclaw-zero-token
Length of output: 2084
🏁 Script executed:
#!/bin/bash
# Also look at how chatgpt-web-stream uses the error pattern
grep -A 20 "catch (err)" src/agents/chatgpt-web-stream.ts | tail -25Repository: linuxhsj/openclaw-zero-token
Length of output: 731
删除 as any 类型断言，使用类型化的错误消息构造函数
第 160 行的 as any 违反了编码规范。应该导入并使用 buildStreamErrorAssistantMessage 辅助函数（在 src/agents/stream-message-shared.ts 中定义），它已经提供了正确的类型推断，无需类型转换。
建议的修复方案
-import {
- PerplexityWebClientBrowser,
- type PerplexityWebClientOptions,
-} from "../providers/perplexity-web-client-browser.js";
+import {
+ buildStreamErrorAssistantMessage,
+} from "./stream-message-shared.js";
+import {
+ PerplexityWebClientBrowser,
+ type PerplexityWebClientOptions,
+} from "../providers/perplexity-web-client-browser.js";
} catch (err) {
- stream.push({
- type: "error",
- reason: "error",
- error: {
- role: "assistant",
- content: [],
- stopReason: "error",
- errorMessage: err instanceof Error ? err.message : String(err),
- api: model.api,
- provider: model.provider,
- model: model.id,
- usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } },
- timestamp: Date.now(),
- },
- } as any);
+ stream.push({
+ type: "error",
+ reason: "error",
+ error: buildStreamErrorAssistantMessage({
+ model,
+ errorMessage: err instanceof Error ? err.message : String(err),
+ }),
+ });该模式已在其他代理文件（如 ollama-stream.ts 和 openai-ws-stream.ts）中成功应用，无需 as any 转换。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/perplexity-web-stream.ts` around lines 145 - 160, Replace the
ad-hoc typed error object (the object pushed to stream inside the catch block)
and the trailing "as any" with a call to the typed helper
buildStreamErrorAssistantMessage: import buildStreamErrorAssistantMessage from
src/agents/stream-message-shared.ts, build the error payload using the caught
err (err instanceof Error ? err.message : String(err)), model info (model.api,
model.provider, model.id), usage defaults, and timestamp, then push the result
of buildStreamErrorAssistantMessage(...) to stream (instead of the current
object and "as any"); update the catch block in perplexity-web-stream.ts to use
buildStreamErrorAssistantMessage so TypeScript infers the correct type.
Comment on lines
+127
to
+129
| } catch { | ||
| return; | ||
| } |
There was a problem hiding this comment.
不要静默吞掉同步失败异常。
Line 127 到 Line 129 的 catch { return; } 会让同步失败无日志，用户只看到“授权完成”，但配置可能未落盘，排障成本高。建议至少输出 warning，并区分文件缺失和 JSON 异常。
🔧 建议修复
- } catch {
- return;
+ } catch (error) {
+ const code = (error as NodeJS.ErrnoException)?.code;
+ if (code === "ENOENT") return;
+ const message = error instanceof Error ? error.message : String(error);
+ console.warn(` > 同步 models.providers 失败: ${modelsPath} (${message})`);
+ return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch { | |
| return; | |
| } | |
| } catch (error) { | |
| const code = (error as NodeJS.ErrnoException)?.code; | |
| if (code === "ENOENT") return; | |
| const message = error instanceof Error ? error.message : String(error); | |
| console.warn(` > 同步 models.providers 失败: ${modelsPath} (${message})`); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/onboard-web-auth.ts` around lines 127 - 129, The empty catch
block after the config sync (the try/catch in src/commands/onboard-web-auth.ts)
is silently swallowing errors; update the catch to accept an error (e.g. catch
(err)) and log a warning with the error details instead of just returning.
Distinguish common cases: if err.code === 'ENOENT' log a specific "file missing"
warning, if err is a SyntaxError (JSON.parse) log a "JSON parse" warning
including err.message, otherwise log a generic warning with the full error; keep
the current control flow (return or rethrow) after logging so failures are
observable.
Comment on lines
+146
to
+152
| const firstEntry = Object.entries(providers).find( | ||
| ([, p]) => | ||
| p && | ||
| typeof p === "object" && | ||
| Array.isArray((p as { models?: unknown[] }).models) && | ||
| (p as { models: { id?: string }[] }).models.length > 0, | ||
| ); |
There was a problem hiding this comment.
默认主模型选择应与注释一致，仅限 web provider。
注释写的是“首个 web provider”，但当前 find 条件没有限制 providerId，可能选到非 web provider。建议在筛选条件里加 providerId.endsWith("-web")。
🔧 建议修复
- const firstEntry = Object.entries(providers).find(
- ([, p]) =>
+ const firstEntry = Object.entries(providers).find(
+ ([providerId, p]) =>
+ providerId.endsWith("-web") &&
p &&
typeof p === "object" &&
Array.isArray((p as { models?: unknown[] }).models) &&
(p as { models: { id?: string }[] }).models.length > 0,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const firstEntry = Object.entries(providers).find( | |
| ([, p]) => | |
| p && | |
| typeof p === "object" && | |
| Array.isArray((p as { models?: unknown[] }).models) && | |
| (p as { models: { id?: string }[] }).models.length > 0, | |
| ); | |
| const firstEntry = Object.entries(providers).find( | |
| ([providerId, p]) => | |
| providerId.endsWith("-web") && | |
| p && | |
| typeof p === "object" && | |
| Array.isArray((p as { models?: unknown[] }).models) && | |
| (p as { models: { id?: string }[] }).models.length > 0, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/onboard-web-auth.ts` around lines 146 - 152, The current
selection of firstEntry from providers can pick non-web providers because the
find predicate doesn't check providerId; update the predicate used when
computing firstEntry to require the provider key (providerId) to end with "-web"
(e.g., add providerId.endsWith("-web") to the condition), so only web providers
are considered while keeping the existing checks for p being an object and
having a non-empty models array.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/providers/perplexity-web-auth.ts`:
- Around line 41-44: The error message thrown in the Perplexity web auth
connection check (the throw new Error that includes profile.cdpUrl) still
references the removed "./start-chrome-debug.sh" script; update that message to
a generic remote-debugging guidance instead. Locate the throw in
src/providers/perplexity-web-auth.ts (the block that throws when it "Failed to
connect to Chrome at ${profile.cdpUrl}") and replace the hardcoded script
reference with a concise, general instruction such as advising to ensure Chrome
is running with remote debugging enabled and pointing to the debug port/URL
(include profile.cdpUrl in the message for context).
- Around line 25-50: The headless option from PerplexityWebAuthOptions is
ignored; ensure the headless flag is applied when launching Chrome by passing it
into the launch path. Update the code in the non-attach branch so that
launchOpenClawChrome receives the headless setting (e.g., merge headless from
the local options into the config you pass or set browserConfig.headless =
headless before calling launchOpenClawChrome), referencing the symbols headless,
options, browserConfig, and launchOpenClawChrome.
- Around line 71-75: The CDP connection opened via chromium.connectOverCDP (the
variable browser) is not being closed, causing resource leaks; update the
function around the connectOverCDP call so that browser, context and page are
cleaned up in a finally block: ensure page.close() (if created), context.close()
(or context.pages cleanup) and await browser.close() are called safely (checking
existence) to release Playwright local resources; reference the variables
browser, context and page around the connectOverCDP/wsUrl usage and wrap the
main logic in try { ... } finally { /* close page/context/browser */ } to
guarantee cleanup.
- Around line 86-103: The login-detection predicate in page.waitForFunction is
too broad and context.cookies() is returning cookies for all domains; narrow the
detection to robust signals (e.g., wait for a specific post-login URL or a known
auth cookie name tied to the target domain) and only read cookies for the app
domain to avoid cross-domain leaks; update the wait logic in waitForFunction to
check for a domain-scoped cookie or location change (use page.url() or
window.location.href) and replace the global context.cookies() call with a
domain-scoped call (context.cookies({ url: '<your-app-origin>' }) or
page.context().cookies([ '<your-app-origin>' ])) before forming cookieString and
keep calling onProgress("Login detected, capturing cookies...") after the
targeted check succeeds so only domain-relevant cookies are collected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1f4fe7c9-8f51-4854-9159-c5cfc4c9398e
📒 Files selected for processing (1)
src/providers/perplexity-web-auth.ts
Comment on lines
+25
to
+50
| const { onProgress = console.log, headless = false } = options; | ||
|
|
||
| const rootConfig = loadConfig(); | ||
| const browserConfig = resolveBrowserConfig(rootConfig.browser, rootConfig); | ||
| const profile = resolveProfile(browserConfig, browserConfig.defaultProfile); | ||
| if (!profile) { | ||
| throw new Error(`Could not resolve browser profile '${browserConfig.defaultProfile}'`); | ||
| } | ||
|
|
||
| let running: Awaited<ReturnType<typeof launchOpenClawChrome>> | { cdpPort: number }; | ||
| let didLaunch = false; | ||
|
|
||
| if (browserConfig.attachOnly) { | ||
| onProgress("Connecting to existing Chrome (attach mode)..."); | ||
| const wsUrl = await getChromeWebSocketUrl(profile.cdpUrl, 5000); | ||
| if (!wsUrl) { | ||
| throw new Error( | ||
| `Failed to connect to Chrome at ${profile.cdpUrl}. ` + | ||
| "Make sure Chrome is running in debug mode (./start-chrome-debug.sh)", | ||
| ); | ||
| } | ||
| running = { cdpPort: profile.cdpPort }; | ||
| } else { | ||
| onProgress("Launching browser..."); | ||
| running = await launchOpenClawChrome(browserConfig, profile); | ||
| didLaunch = true; |
There was a problem hiding this comment.
headless 选项当前未生效，接口行为与定义不一致。
PerplexityWebAuthOptions 暴露了 headless，但启动分支没有使用它，调用方传参不会产生任何效果。
💡 建议修复
export async function loginPerplexityWeb(
options: PerplexityWebAuthOptions = {},
): Promise<PerplexityWebAuthResult> {
- const { onProgress = console.log, headless = false } = options;
+ const { onProgress = console.log, headless } = options;
const rootConfig = loadConfig();
- const browserConfig = resolveBrowserConfig(rootConfig.browser, rootConfig);
+ const resolvedConfig = resolveBrowserConfig(rootConfig.browser, rootConfig);
+ const browserConfig =
+ headless === undefined ? resolvedConfig : { ...resolvedConfig, headless };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/providers/perplexity-web-auth.ts` around lines 25 - 50, The headless
option from PerplexityWebAuthOptions is ignored; ensure the headless flag is
applied when launching Chrome by passing it into the launch path. Update the
code in the non-attach branch so that launchOpenClawChrome receives the headless
setting (e.g., merge headless from the local options into the config you pass or
set browserConfig.headless = headless before calling launchOpenClawChrome),
referencing the symbols headless, options, browserConfig, and
launchOpenClawChrome.
Comment on lines
+41
to
+44
| throw new Error( | ||
| `Failed to connect to Chrome at ${profile.cdpUrl}. ` + | ||
| "Make sure Chrome is running in debug mode (./start-chrome-debug.sh)", | ||
| ); |
There was a problem hiding this comment.
错误提示仍引用已删除脚本，用户指引会误导。
当前提示里的 ./start-chrome-debug.sh 与本 PR“移除启动脚本”的目标冲突，建议改成通用的远程调试说明。
💡 建议修复
if (!wsUrl) {
throw new Error(
`Failed to connect to Chrome at ${profile.cdpUrl}. ` +
- "Make sure Chrome is running in debug mode (./start-chrome-debug.sh)",
+ "Make sure Chrome is running with remote debugging enabled (e.g. --remote-debugging-port).",
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new Error( | |
| `Failed to connect to Chrome at ${profile.cdpUrl}. ` + | |
| "Make sure Chrome is running in debug mode (./start-chrome-debug.sh)", | |
| ); | |
| throw new Error( | |
| `Failed to connect to Chrome at ${profile.cdpUrl}. ` + | |
| "Make sure Chrome is running with remote debugging enabled (e.g. --remote-debugging-port).", | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/providers/perplexity-web-auth.ts` around lines 41 - 44, The error message
thrown in the Perplexity web auth connection check (the throw new Error that
includes profile.cdpUrl) still references the removed "./start-chrome-debug.sh"
script; update that message to a generic remote-debugging guidance instead.
Locate the throw in src/providers/perplexity-web-auth.ts (the block that throws
when it "Failed to connect to Chrome at ${profile.cdpUrl}") and replace the
hardcoded script reference with a concise, general instruction such as advising
to ensure Chrome is running with remote debugging enabled and pointing to the
debug port/URL (include profile.cdpUrl in the message for context).
Comment on lines
+71
to
+75
| const browser = await chromium.connectOverCDP(wsUrl, { | ||
| headers: getHeadersWithAuth(wsUrl), | ||
| }); | ||
| const context = browser.contexts()[0]; | ||
| const page = context.pages()[0] || (await context.newPage()); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
在 Playwright-core 1.58.2 中，使用 chromium.connectOverCDP() 连接到已存在 Chrome 后调用 browser.close()，是“仅断开连接”还是会“关闭远端浏览器进程”？
💡 Result:
在 Playwright-core 1.58.2 里，通过 chromium.connectOverCDP() 连接到“已存在的”Chrome 之后调用 await browser.close()，语义是清理由本次连接创建的 contexts 并断开与浏览器服务器的连接（disconnect），不会去关闭远端 Chrome 浏览器进程。[1]
connectOverCDP() 本身也被定义为“附加(attach)到已存在的浏览器实例”，因此其对应的 browser.close() 行为就是“断开连接”，而不是“结束该浏览器进程”。[2]
参考来源
[1] Playwright API：Browser.close 说明（connected 情况下会 disconnect） (playwright.dev)
[2] Playwright API：BrowserType.connectOverCDP 说明（attach 到已存在实例） (playwright.dev)
Citations:
- 1: https://playwright.dev/docs/api/class-browser
- 2: https://playwright.dev/docs/api/class-browsertype
🏁 Script executed:
cat -n src/providers/perplexity-web-auth.ts | head -150Repository: linuxhsj/openclaw-zero-token
Length of output: 4914
CDP 连接未显式释放，导致长生命周期进程下的本地资源累积泄漏。
当前代码在第 71 行建立 CDP 连接但未在 finally 块中关闭，导致 Playwright 的 context 和 page 对象无法被清理。虽然 browser.close() 仅断开与远端浏览器的连接（不会关闭远端 Chrome 进程），但其必要性在于清理本地分配的上下文和页面资源。
💡 建议修复
+ let browser: Awaited<ReturnType<typeof chromium.connectOverCDP>> | null = null;
try {
const cdpUrl = browserConfig.attachOnly
- const browser = await chromium.connectOverCDP(wsUrl, {
+ browser = await chromium.connectOverCDP(wsUrl, {
headers: getHeadersWithAuth(wsUrl),
});
} finally {
+ if (browser) {
+ await browser.close().catch(() => {});
+ }
if (didLaunch && running && "proc" in running) {
await stopOpenClawChrome(running);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/providers/perplexity-web-auth.ts` around lines 71 - 75, The CDP
connection opened via chromium.connectOverCDP (the variable browser) is not
being closed, causing resource leaks; update the function around the
connectOverCDP call so that browser, context and page are cleaned up in a
finally block: ensure page.close() (if created), context.close() (or
context.pages cleanup) and await browser.close() are called safely (checking
existence) to release Playwright local resources; reference the variables
browser, context and page around the connectOverCDP/wsUrl usage and wrap the
main logic in try { ... } finally { /* close page/context/browser */ } to
guarantee cleanup.
Comment on lines
+86
to
+103
| await page.waitForFunction( | ||
| () => { | ||
| return ( | ||
| document.cookie.includes("__Secure-next-auth.session-token") || | ||
| document.cookie.includes("intercom_session") || | ||
| document.cookie.includes("perplexity_") || | ||
| document.cookie.includes("next-auth.session-token") || | ||
| (window.location.pathname === "/" && !document.querySelector('button[data-testid="login-button"]')) | ||
| ); | ||
| }, | ||
| { timeout: 300000 }, | ||
| ); | ||
|
|
||
| onProgress("Login detected, capturing cookies..."); | ||
|
|
||
| const cookies = await context.cookies(); | ||
| const cookieString = cookies.map((c) => `${c.name}=${c.value}`).join("; "); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Playwright-core 1.58.2 中，BrowserContext.cookies() 不传 urls 参数时，是否会返回该 context 下所有域名的 cookies？
💡 Result:
是的。Playwright-core 1.58.2 里 BrowserContext.cookies() 的 urls 参数默认值就是空数组 []，也就是不传（或传空）时会按“不过滤 URL”处理，从而返回该 context 中的全部 cookies（跨所有域名）。[1][2]
依据：
- 官方 API 文档明确写道：“If no URLs are specified, this method returns all cookies.”[1]
- 1.58.2 源码中
cookies(urls ... = [])，未传时实际会调用doGetCookies([])（空列表）。[2]
参考来源：
[1] (playwright.dev)
[2] (raw.githubusercontent.com)
Citations:
- 1: https://playwright.dev/docs/api/class-browsercontext?utm_source=openai
- 2: https://raw.githubusercontent.com/microsoft/playwright/v1.58.2/packages/playwright-core/src/server/browserContext.ts
🏁 Script executed:
fd "perplexity-web-auth.ts" -t fRepository: linuxhsj/openclaw-zero-token
Length of output: 108
🏁 Script executed:
head -n 110 src/providers/perplexity-web-auth.ts | tail -n 30Repository: linuxhsj/openclaw-zero-token
Length of output: 1257
🏁 Script executed:
cat -n src/providers/perplexity-web-auth.ts | sed -n '70,110p'Repository: linuxhsj/openclaw-zero-token
Length of output: 1997
登录完成判定条件过宽，context.cookies() 未限域，存在误判与跨域 Cookie 泄露风险。
当前 login-button 选择器检查是脆弱信号，可能在未真正登录时误判为登录成功。同时 context.cookies() 未传 URL 参数，会返回浏览器上下文中所有域名的 Cookie，造成敏感信息泄露。
建议修复：
💡 修复方案
- await page.waitForFunction(
- () => {
- return (
- document.cookie.includes("__Secure-next-auth.session-token") ||
- document.cookie.includes("intercom_session") ||
- document.cookie.includes("perplexity_") ||
- document.cookie.includes("next-auth.session-token") ||
- (window.location.pathname === "/" && !document.querySelector('button[data-testid="login-button"]'))
- );
- },
- { timeout: 300000 },
- );
+ const deadline = Date.now() + 300000;
+ while (Date.now() < deadline) {
+ const scoped = await context.cookies(["https://www.perplexity.ai"]);
+ const loggedIn = scoped.some(
+ (c) =>
+ c.name === "__Secure-next-auth.session-token" ||
+ c.name === "next-auth.session-token" ||
+ c.name.startsWith("perplexity_"),
+ );
+ if (loggedIn) break;
+ await new Promise((r) => setTimeout(r, 1000));
+ }
onProgress("Login detected, capturing cookies...");
- const cookies = await context.cookies();
+ const cookies = await context.cookies(["https://www.perplexity.ai"]);
+ if (cookies.length === 0) {
+ throw new Error("Login not confirmed: no Perplexity cookies found.");
+ }
const cookieString = cookies.map((c) => `${c.name}=${c.value}`).join("; ");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/providers/perplexity-web-auth.ts` around lines 86 - 103, The
login-detection predicate in page.waitForFunction is too broad and
context.cookies() is returning cookies for all domains; narrow the detection to
robust signals (e.g., wait for a specific post-login URL or a known auth cookie
name tied to the target domain) and only read cookies for the app domain to
avoid cross-domain leaks; update the wait logic in waitForFunction to check for
a domain-scoped cookie or location change (use page.url() or
window.location.href) and replace the global context.cookies() call with a
domain-scoped call (context.cookies({ url: '<your-app-origin>' }) or
page.context().cookies([ '<your-app-origin>' ])) before forming cookieString and
keep calling onProgress("Login detected, capturing cookies...") after the
targeted check succeeds so only domain-relevant cookies are collected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…tart scripts
Summary
Describe the problem and fix in 2–5 bullets:
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.Security Impact (required)
Yes/No)Yes/No)Yes/No)Yes/No)Yes/No)Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No)Yes/No)Yes/No)Failure Recovery (if this breaks)
Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.Summary by CodeRabbit
发布说明
文档
新功能
脚本调整