feat(browser): add sandboxed local HTML preview - #1803
Conversation
Agents can now verify workspace-local HTML pages (screenshot/snapshot) through a managed loopback preview server instead of escaping to a bare Chrome process (which historically led to killing the user's browser via taskkill /F /IM chrome.exe). Security model: - new previewLocalHtml action + token-scoped loopback HTTP server (127.0.0.1:<random port>, 256-bit capability token per preview) - serving root = the entry's directory; per-request path re-validation (lexical + realpath nearest-existing-ancestor), dotfile/extension whitelist, GET/HEAD only, nosniff + no-store + CSP (connect-src 'self'; form-action 'none') - SSRF exception is exact-origin only (scheme+host+port) via the upstream allowedOrigins/resolveSsrFPolicyForUrl primitives, wired through LOCAL_PATCHES; other loopback hosts/ports and file:// stay blocked - origin grant is revoked on listener error/close (incl. crash) so a freed port is never trusted by policy Also fixes sync.mjs on Windows (drive-letter paths for msys tar, path.sep normalization for LOCAL_PATCHES keys and the leaf prefix filter). Fixes makecindy#1766 Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/mcp-integrations/local-html-preview-server.ts | 新增 token 化 loopback 文件服务、路径与句柄身份校验、严格 CSP 和 Origin 生命周期管理。 |
| apps/desktop/src/main/mcp-integrations/browser-preview-tabs.ts | 新增双后端预览标签登记与撤销清理，但已报告的同端口跨代匹配问题仍然存在。 |
| apps/desktop/src/main/mcp-integrations/browser-backend/preview-guard.ts | 为 RSB 预览页增加提交级身份维护、实时 Origin 授权、导航拦截和 WebRTC 初始化保护。 |
| apps/desktop/src/main/mcp-integrations/browser-backend/rsb-webview-backend.ts | 将预览防护、撤销 generation 和标签 provenance 接入 RSB open/navigate 生命周期。 |
| packages/browser-control-runtime/src/_generated/extension/src/browser/pw-session.ts | 外置 Chrome 获得持续 Origin 守卫和预览失败关闭，但 WebRTC 初始化失败仍可能返回无守卫活标签。 |
| apps/desktop/src/renderer/features/right-sidebar/store.ts | 将预览标签设为 ephemeral，并通过 revision 与串行队列协调持久行转换、关闭及并发状态更新。 |
Sequence Diagram
sequenceDiagram
participant Agent
participant Preview as Local Preview Server
participant Policy as SSRF/Origin Policy
participant Browser as Managed Browser
Agent->>Preview: previewLocalHtml(workspace HTML)
Preview->>Preview: 校验路径并签发 token URL
Preview->>Policy: 授权精确 loopback Origin
Agent->>Browser: 打开预览 URL
Browser->>Browser: 安装导航守卫与 WebRTC 闸
Browser->>Preview: 加载 HTML 与相对资源
Preview-->>Browser: CSP sandbox 响应
Agent->>Preview: 撤销/退出
Preview->>Policy: 移除 Origin 授权
Preview->>Browser: 关闭对应预览标签
Prompt To Fix All With AI
### Issue 1
packages/browser-control-runtime/src/_generated/extension/src/browser/pw-session.ts:1506-1509
**初始化失败留下无守卫标签**
当预览 URL 的 `page.addInitScript` 安装失败时，此分支移除 route guard、保留页面并抛出异常；`createPageViaPlaywright` 随后吞掉该非策略异常，并可将仍存活的 `about:blank` 页面作为成功创建的标签返回，导致后续导航不再经过预览 Origin 守卫且 WebRTC 闸也未安装。
**How this was verified:** 跟踪该异常可见此处分支未关闭页面，而上层仅重新抛出策略拒绝类错误并继续返回页面 target。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (48): Last reviewed commit: "fix(browser): serialize preview URL tran..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5eeed534df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
本 PR 为 cindy_browser 增加受控的本地 HTML 预览通道：通过桌面端 main 进程启动 token 化的 loopback 预览服务，向现有 SSRF 策略授予**精确 origin（scheme+host+port）**例外，使 agent 能在受管浏览器中打开工作区内生成的 HTML 并继续复用现有 snapshot/screenshot/act 流程，从而避免回退到裸跑 Chrome。
Changes:
- 新增
previewLocalHtmlaction：L2 MCP 负责参数/会话上下文校验并向 host 申请预览 URL，再通过 runtimeopen/navigate打开。 - 桌面端新增本地预览 HTTP 服务：监听
127.0.0.1:<ephemeral>，按 token+目录根提供静态资源，配合 runtime SSRFallowedOrigins做精确放行。 - 补齐 vendored SSRF 适配与导航级测试；同时修复
scripts/browser-runtime/sync.mjs的 Windows(msys) 路径/patch key 兼容。
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/browser-runtime/sync.mjs | 修复 Windows(msys) 下 tar/路径与 patch key 匹配，确保 _generated 可复现生成 |
| packages/lizi-mcps/src/types.ts | Browser MCP deps 增加懒加载会话上下文与本地预览 URL 发行接口类型 |
| packages/lizi-mcps/src/providers.ts | 注入 getSessionContext，确保 tool-call 时能拿到真实 workingDir |
| packages/lizi-mcps/src/browser/tools.ts | 新增 previewLocalHtml action 处理与错误返回语义 |
| packages/lizi-mcps/src/browser/prompts/rules/browser-workflow.md | 更新 agent 工作流文档，引导使用本地预览而非 file:///裸跑 Chrome |
| packages/lizi-mcps/src/browser/browserMcpServer.test.ts | 为 previewLocalHtml 增加 L2 行为测试（无 workingDir/无注入/打开方式等） |
| packages/browser-control-runtime/upstream/browser-runtime.lock.json | 更新 vendored 补丁记录与 contentHash |
| packages/browser-control-runtime/src/types.ts | 新增本地预览相关 errorCode（L2 产生） |
| packages/browser-control-runtime/src/shim/security-runtime.ts | 透传导出 resolveSsrFPolicyForUrl |
| packages/browser-control-runtime/src/shim/_local/config-types-browser.ts | SSRF 配置类型新增 allowedOrigins（精确 origin allowlist） |
| packages/browser-control-runtime/src/_generated/extension/src/sdk-security-runtime.ts | 生成层补齐 resolveSsrFPolicyForUrl re-export |
| packages/browser-control-runtime/src/_generated/extension/src/infra/net/ssrf.ts | 生成层补齐 resolveSsrFPolicyForUrl re-export |
| packages/browser-control-runtime/src/_generated/extension/src/browser/navigation-guard.ts | 导航 guard 在私网判定前按 URL 解析 allowedOrigins（每次跳转重算） |
| packages/browser-control-runtime/src/_generated/extension/src/browser/config.ts | 解析 host config 时保留 allowedOrigins，避免被 resolver 丢弃 |
| packages/browser-control-runtime/src/tests/navigation-allowed-origins.test.ts | 增加导航级 exact-origin allowlist 测试（同 host 不同端口/不同 scheme 等保持 fail-closed） |
| apps/desktop/src/main/mcp-integrations/local-html-preview-server.ts | 新增 token 化 loopback 预览服务（路径边界/扩展名白名单/CSP/生命周期撤销） |
| apps/desktop/src/main/mcp-integrations/browser.ts | 将预览服务接入浏览器运行时配置热更新，并暴露 createLocalPreviewUrl 给 MCP |
| apps/desktop/src/main/mcp-integrations/browser-managed-config.ts | buildManagedConfig 支持注入 previewOrigins→ssrfPolicy.allowedOrigins |
| apps/desktop/src/main/mcp-integrations/tests/local-html-preview-server.test.ts | 覆盖预览服务的路径安全、白名单、CSP/headers 与 origin grant 生命周期 |
Suppressed comments (1)
apps/desktop/src/main/mcp-integrations/local-html-preview-server.ts:196
close事件里只做了revokeOrigin()，但没有重置starting/server或清空 token。若 listener 因异常关闭（非 dispose），origin会被置空但starting仍是已 resolve 的旧 Promise，后续ensureStarted()会直接返回旧 origin，导致发放的预览 URL 指向已释放端口（且 tokens 也永远无法再访问）。建议在 close 时清空 tokens 并把starting/server置空，允许后续自动重启。
// Covers normal dispose AND listener crash — never leave the SSRF
// policy trusting a port nobody is serving (a local process could
// otherwise bind the freed port and intercept the token URL).
revokeOrigin();
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
⏸️ 本 PR 触发了维护者确认门（product: feat 类型 + arch: 核心路径大改动），已创建讨论 issue 等待维护者确认。 确认方式：维护者直接在本 PR 上 Approve 即放行；需要修改请 Request Changes，改完再 Approve。 当前状态：等待维护者确认。 讨论 issue:#1808 |
MagicLizi
added
awaiting-discussion
…eview Review findings (Greptile on makecindy#1803) — both confirmed: - reject hidden directory SEGMENTS (not just dotfiles), so .git/.config/… content matching the extension whitelist is unreachable - resolve the real path and open by file descriptor, then re-verify the handle (fstat), closing the check-then-open race where a concurrent directory swap could redirect the read outside the serving root - normalize the serving root to its real path (8.3 short names on win32) - 500 (not hang) on any internal request error Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 737be2093b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (5)
packages/lizi-mcps/src/browser/prompts/rules/browser-workflow.md:63
- 这里写“禁止向外网发数据(connect-src 'self')”容易让人误以为完全无外联，但当前服务端 CSP 仍放行了
https:（default-src），并不等价于“禁止外网”。建议把表述改为和实际 CSP/安全边界一致（或在 CSP 收紧后明确说明仅允许同源资源）。
- 只支持 `.html` / `.htm` 入口;入口同目录下的相对 CSS / JS / 图片 / 字体资源可以加载。
- **不要**尝试 `file://` 导航(策略拒绝),也**不要**通过 shell 裸跑 Chrome 截图——受管浏览器有护栏,裸跑会失去一切保护(历史上曾因此误杀用户浏览器)。
- 预览页禁止向外网发数据(`connect-src 'self'`);依赖外网 fetch / WebSocket 的页面需把资源放到本地再预览。
packages/lizi-mcps/src/browser/tools.ts:278
previewLocalHtml已新增BROWSER_RUNTIME_LOCAL_PREVIEW_UNSUPPORTED_FILE，但这里仅识别PATH_NOT_ALLOWED，导致宿主抛出UNSUPPORTED_FILE:时仍被映射成 UNAVAILABLE，错误语义会丢失。建议按LocalPreviewError的 code 前缀再补一档映射。
const message = err instanceof Error ? err.message : String(err);
const code = message.includes('PATH_NOT_ALLOWED')
? 'BROWSER_RUNTIME_LOCAL_PREVIEW_PATH_NOT_ALLOWED'
: 'BROWSER_RUNTIME_LOCAL_PREVIEW_UNAVAILABLE';
return errorResult('previewLocalHtml', `本地预览创建失败: ${message}`, code);
apps/desktop/src/main/mcp-integrations/local-html-preview-server.ts:115
- 当前 CSP 使用
default-src 'self' data: https:，即使connect-src 'self'也仍允许页面通过<img src=https://...>/<script src=https://...>等非 connect-src 通道对外发起请求，和 PR/文档里“不能向外网发数据”的安全模型不一致。建议把外网源从 CSP 中移除，并显式收紧 img/script/style/font/media 等指令，仅允许同源（必要时允许 inline）。
const CSP =
"default-src 'self' data: https:; connect-src 'self'; form-action 'none'; base-uri 'none'";
apps/desktop/src/main/mcp-integrations/local-html-preview-server.ts:10
- 头注释写“token 绑定 serving root + session identity”，但实现侧并未在请求中校验 sessionId（handleRequest 只按 token 查 root）。建议避免在注释里宣称有 session 绑定，以免误导安全审计。
* - each preview gets an unguessable 256-bit capability token bound to the
* entry's canonical directory (the serving root) + session identity;
apps/desktop/src/main/mcp-integrations/browser.ts:109
applyPreviewOrigins这里用setBrowserControlRuntimeConfig(buildManagedConfig(...))直接整对象替换 runtime config，会覆盖/丢失 runtime 在运行期通过mutateConfigFile写入的配置（例如 gateway.auth 自动生成 token、profile 端口/颜色分配等）。这可能导致 auth 反复重置或影响已有的 loopback client。建议改成“在现有 config 上做最小 patch（仅更新 ssrfPolicy.allowedOrigins）”，而不是整对象覆盖。
const localPreviewServer = createLocalPreviewServer({
logger,
applyPreviewOrigins: (previewOrigins) => {
setBrowserControlRuntimeConfig(buildManagedConfig({ previewOrigins }));
},
|
@happy9zhang 👋 这个 PR 还有 8 条 review conversation 没 resolve(apps/desktop/src/main/mcp-integrations/local-html-preview-server.ts / packages/lizi-mcps/src/browser/tools.ts / packages/lizi-mcps/src/types.ts / packages/lizi-mcps/src/browser/prompts/rules/browser-workflow.md),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
…or mapping Follow-up to the 8 unresolved review conversations on makecindy#1803: - CSP: drop https: from every directive; preview pages now load local resources only (script/style/img/font/media 'self' + inline + data:), closing the third-party-script injection + exfiltration path - SSH remote sessions: reject previewLocalHtml when the session has a remoteHostId (workingDir lives on the remote host) - capability tokens now expire (TTL, default 24h) and the comment no longer over-promises session-bound semantics - map UNSUPPORTED_FILE host errors to their dedicated error code instead of collapsing into UNAVAILABLE - browser-workflow.md wording matches the actual CSP Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
|
已修复全部 8 条 review 意见（commit
测试：preview server 11/11、mcps 440、desktop typecheck 全部通过。 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/lizi-mcps/src/browser/tools.ts:289
- 这里通过
message.includes(...)来推断 host 侧错误码比较脆弱：一旦错误信息正文里偶然包含这些子串，会被误判；同时LocalPreviewError还会抛NOT_FOUND:，当前会落到BROWSER_RUNTIME_LOCAL_PREVIEW_UNAVAILABLE，对调用方语义不准确（缺失/非文件更像参数错误）。建议按消息前缀严格解析 code，并为 NOT_FOUND 映射到更合适的错误码。
const message = err instanceof Error ? err.message : String(err);
const code = message.includes('PATH_NOT_ALLOWED')
? 'BROWSER_RUNTIME_LOCAL_PREVIEW_PATH_NOT_ALLOWED'
: message.includes('UNSUPPORTED_FILE')
? 'BROWSER_RUNTIME_LOCAL_PREVIEW_UNSUPPORTED_FILE'
: 'BROWSER_RUNTIME_LOCAL_PREVIEW_UNAVAILABLE';
return errorResult('previewLocalHtml', `本地预览创建失败: ${message}`, code);
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f9f9ce7ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The sync.mjs Windows compatibility fixes moved to their own PR (makecindy#1834); this PR keeps only the LOCAL_PATCHES allowedOrigins wiring. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/desktop/src/renderer/features/right-sidebar/store.ts:47
- 这里的
isSandboxPreviewUrl注释写“Mirror the main-side shape check”，但实现只判断 protocol/hostname/pathname，没有像 main 侧isPreviewUrl那样 fail-closed 拒绝 userinfo 变体（parsed.username/password === ''）。建议与 main 侧保持一致，避免两处“预览 URL”判定语义漂移、后续维护误判。
return (
parsed.protocol === 'http:' &&
parsed.hostname === '127.0.0.1' &&
/^\/preview\/[a-f0-9]{64}\//.test(parsed.pathname)
);
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae2f77a9de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…eview round 27k) round 27f skipped only addTab's first upsert; subsequent writes re-created the persisted row: patchTabState (title/favicon reports) upserts again, setActiveTab hits NOT_FOUND (no DB row) and rolls back, reorder references a missing id (codex-connector P2, round 27k). - ephemeralTabIds Set marks a sandbox-preview tab at addTab time. - patchTabState / setActiveTab / closeTab skip the IPC write for ephemeral tabs; reorderTabs persists only the non-ephemeral ids. - forgetClosedTab drops the ephemeral marker; _resetStore clears the set. - tests: preview tab addTab/patch/setActive/close all stay memory-only. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
apps/desktop/src/main/mcp-integrations/browser.ts:514
- 这里的
/** ... */注释块没有挂在任何声明上（与下面的clearStaleServiceWorkers/closePreviewTabs都隔了一段），容易被误认为是 JSDoc 且在文档/规则检查里无效或触发告警。建议改成普通块注释（/* ... */）或移动到要描述的函数上方。
/**
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1753c94ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… reject userinfo variants in the renderer preview check (review round 27l) - createLocalPreviewUrl: if the RSB partition SW clear fails or times out, the origin was ALREADY published via applyPreviewOrigins() by ensureStarted() — leaving it granted would let a stale scope=/ SW intercept /preview/<64hex>/... before token validation and answer with a no-CSP page. dispose() now revokes the grant and closes the listener (codex-connector P1, round 27l). - renderer isSandboxPreviewUrl mirrors the main-side userinfo rejection so http://x@127.0.0.1:.../preview/... is not mis-marked as a preview tab for ephemeral/hydrate-purge decisions (Copilot P2, round 27l). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edce0be23a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…(path B) Round 27 收敛决策：updateService.ts 的强退清理预览改动整体撤出本 PR—— ①它是唯一冲突文件（vs upstream/main），撤出即解除 CONFLICTING ②其高危 门禁裁决（cindy-updater.md，round 19 流程违规）悬而未决、无维护者回应， 撤出后不再阻塞本 PR ③符合 §9.6-3 残余判据（强退清理不在预览主路径）。 - updateService.ts 恢复为 merge-base 版本（纯撤出，无任何调和优化） - 删除 preview-cleanup.ts（唯一消费者是 updateService，撤出即死代码） - browser.ts 移除 setPreviewCleanupImpl import 与注册块（同逻辑已存在于 disposeBrowserRuntime，不损失能力） 强退时清理预览能力另行跟踪（残余 issue 起草中）。 Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
…ndary and sync persisted active on ephemeral close (round 27l XQory/XQGws) - XQory: ephemeral identity is no longer frozen at addTab time. patchTabState now re-derives it from the state URL on every web-browser patch: preview → normal restores persistence (row re-created), normal → preview marks the tab ephemeral and clears the stale persisted row (cleanupOrphanTabRow). Fixes the resurrect-after-hydrate / host-migration misclassification. - XQGws: closing an ephemeral (preview) tab no longer short-circuits the whole persisted-active sync block. The replacement active is written to the DB when it is a persistent tab (skipped only when the survivor is itself ephemeral, which has no row). Fixes hydrate jumping back to a stale active. Verified: store 68/68, right-sidebar+mcp-integrations 1122/1122, typecheck 0. Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e12efc64d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ist activation on preview→ordinary transition (round 27l Xw9-h/Xw9-l) - Xw9-h: converting a normal tab to a preview URL now settles that tab's queued state writes (settleTabStateWrites) before cleanupOrphanTabRow deletes the row, so a pending upsert can no longer recreate the supposedly-ephemeral tab's persistent row after the delete (renderer migration / restart restored the old ordinary state). - Xw9-l: when the ACTIVE preview tab navigates to an ordinary URL, the conversion now persists activation (ipc.setActive) after removing the ephemeral marker and re-creating the row — otherwise SQLite kept the old active and host detach/reattach or restart switched back to it. - syncEphemeralStatus is now async and only invoked when the URL actually crosses the preview boundary (non-boundary patches like title/favicon no longer touch the write queue). Verified: store 70/70, right-sidebar+mcp-integrations 1124/1124, typecheck 0. Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b914de8b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…it when a navigation fails/stops (Greptile P1 XzI5E / codex-connector P1 XzOZH) The identity was cleared in did-start-navigation (navigation START). A loadURL that later fails/times out/stops never commits, leaving the OLD preview document alive but disarmed — a later page-initiated escape would then pass will-navigate (no identity, no preview shape). Fix: clear the identity only on did-navigate (main-frame COMMIT), when the old preview document is confirmed replaced. A failed navigation never fires did-navigate, so the identity stays armed and the guard keeps blocking escapes. Also fixes the CI typecheck error in store.test.ts (mockImplementationOnce signature) and adds a regression test for the failed-navigation case. Verified: full monorepo typecheck 0; preview-guard 13/13, store 70/70, right-sidebar+mcp-integrations 1125/1125. Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faa2cec535
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…fails, and skip superseded URL-boundary writes (round 27l Xz_39/Xz_34) - Xz_39: normal→preview conversion whose orphan-row delete exhausts retries now rolls back the ephemeral marker and throws — no more 'mark ephemeral but report success' leaving a stale ordinary row that hydrate's preview filter cannot purge (migration/restart resurrects it). - Xz_34: after syncEphemeralStatus awaits (settle/delete/setActive), re-read the tab's current URL; if a newer patch already crossed the preview boundary again, skip the stale write — otherwise cache shows the new URL while SQLite holds the obsolete one (detach/restart restores wrong state). - Also restores the ephemeralTabIds.add() that was dropped from the normal→preview branch during the Xz_39 edit (identity must be marked before the awaited operations). Verified: store 72/72, right-sidebar+mcp-integrations 1127/1127, monorepo typecheck 0. Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62225c61bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ls, and persist active only after the row upsert (round 27l X1gEf/X1gEa) - X1gEf: normal→preview conversion whose orphan-row delete exhausts retries now rolls back the CACHE url to the ordinary URL (not just the ephemeral marker). Without it, a later title/favicon patch (no longer crossing the URL boundary) runs a plain upsert that writes the dead preview token back into the still-live DB row — the exact stale-preview persistence round 27e eliminated. - X1gEa: setActive for a preview→ordinary conversion now runs AFTER the row upsert. Before, it ran first: main clears all active markers then throws NOT_FOUND (swallowed), and the later upsert created the row with isActive:false → hydrate restored no active tab. Verified: store 73/73 (added order + rollback assertions), right-sidebar +mcp-integrations 1128/1128, monorepo typecheck 0. Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f4f44d0d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- serialize per-tab URL-boundary transitions; keep the identity flip synchronous - revision-check the delete itself before every attempt and after a failure - wait for an in-flight create before deleting the converted row - persist the tab's latest state after a transition, not the captured one - let closeTab take over the delete while a conversion is still in flight Signed-off-by: happy9zhang <happy9zhang@users.noreply.github.com>
| // route that aborts its own navigations. | ||
| await opts.page.unroute("**", handler).catch(() => {}); | ||
| previewRouteGuards.delete(opts.page); | ||
| throw err; |
There was a problem hiding this comment.
当预览 URL 的 page.addInitScript 安装失败时，此分支移除 route guard、保留页面并抛出异常；createPageViaPlaywright 随后吞掉该非策略异常，并可将仍存活的 about:blank 页面作为成功创建的标签返回，导致后续导航不再经过预览 Origin 守卫且 WebRTC 闸也未安装。
How this was verified: 跟踪该异常可见此处分支未关闭页面，而上层仅重新抛出策略拒绝类错误并继续返回页面 target。
Context Used: 使用和PR描述相同的语言进行评论 (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser-control-runtime/src/_generated/extension/src/browser/pw-session.ts
Line: 1506-1509
Comment:
**初始化失败留下无守卫标签**
当预览 URL 的 `page.addInitScript` 安装失败时，此分支移除 route guard、保留页面并抛出异常；`createPageViaPlaywright` 随后吞掉该非策略异常，并可将仍存活的 `about:blank` 页面作为成功创建的标签返回，导致后续导航不再经过预览 Origin 守卫且 WebRTC 闸也未安装。
**How this was verified:** 跟踪该异常可见此处分支未关闭页面，而上层仅重新抛出策略拒绝类错误并继续返回页面 target。
**Context Used:** 使用和PR描述相同的语言进行评论 ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
本轮推送后的状态。114 条 review thread 全部 resolve，CI 全绿，与 main 无冲突。残余项都写在 description 里，本轮新增两条，都标了合并后再提跟踪 issue。 本地验证跑了 store 82 条、right-sidebar 全量 547 条、多包 typecheck 八个包全绿，eslint 与 HEAD 基线逐字相同。有两处要说明，仓库根的 现在的卡点只剩一个 approve（作者不能自批）和产品确认门 #1808。麻烦您看一下能否 approve，或者告诉我还差什么。 另外这个 PR 有 5308 行，确实偏大。您觉得我怎么做比较合适？ |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef83539457
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const wasPreview = isSandboxPreviewUrl(oldUrl); | ||
| const isPreview = isSandboxPreviewUrl(newUrl); |
There was a problem hiding this comment.
When preview HTML calls history.pushState() or replaceState() with a non-preview path, did-navigate-in-page propagates that URL through BrowserTabBody, and these shape checks classify the still-running preview document as ordinary. The store then removes its ephemeral marker and persists the rewritten loopback URL; a sidebar detach reloads it as an unrecognized 404, while a crash followed by port reuse can restore content from another local service because hydration and preview-guard no longer recognize the row. Fresh evidence after the earlier History API guard fix is that main now retains WebContents preview identity, but the renderer persistence boundary still derives identity solely from the page-controlled pathname; only a committed cross-document navigation should make this tab persistent.
AGENTS.md reference: AGENTS.md:L30-L31
Useful? React with 👍 / 👎.
| await enqueueTabStateWrite( | ||
| key, | ||
| { | ||
| id: tabId, | ||
| sessionId, | ||
| kind: oldTab.kind, | ||
| position: idx, | ||
| state: newState, | ||
| state: stateToPersist, | ||
| }, | ||
| revision, | ||
| ); |
There was a problem hiding this comment.
When a preview tab first navigates to an ordinary URL and this queued upsert fails, rollbackTabStateWrite() restores the cache baseline to the old preview URL, but the ephemeral marker removed at the start of the transition is not restored. A later title/favicon patch therefore treats that preview URL as persistent and can recreate the stale tokenized row once the database recovers, bringing back the invalid-preview hydration problem this change is intended to prevent. Catch this write failure and reconcile both the cache URL and ephemeral identity together.
AGENTS.md reference: AGENTS.md:L36-L37
Useful? React with 👍 / 👎.
| const currentTab = now.tabs.find((t) => t.id === tabId); | ||
| return !!currentTab && extractStateUrl(currentTab.state) === newUrl; | ||
| }); | ||
| if (!stillCurrent) return; |
There was a problem hiding this comment.
When a normal tab has an earlier state upsert in flight and that upsert fails while a normal-to-preview transition waits in settleTabStateWrites(), the write pump rolls the cache URL back to its ordinary baseline. The conversion still deletes the database row, then this stillCurrent check returns false without removing the ephemeral marker, so subsequent ordinary patches skip persistence and the tab disappears on detach or restart. Distinguish a newer URL-boundary transition from a state-write rollback and restore a consistent marker/row before returning.
AGENTS.md reference: AGENTS.md:L36-L37
Useful? React with 👍 / 👎.
|
更正一处表述：我上一条说「CI 全绿」不确切，落地后有一条红—— 失败的是 其余全绿：Linux unit ×2、Windows unit (1/2)、DCO、verify、verify-checks、Greptile、Desktop Git integration、pr-design-basis。 |
|
撤回这个 PR，缩小范围后重新提一个。 先说一处对不上的地方。「已声明残余」第 3 条我写的是"当前 fail-closed 已保证页面不会加载无防护内容（停在 about:blank），有意不修"。这个说法不准确。createPageViaPlaywright 里，预览守卫的安装脚本一旦失败，catch 只重抛策略拒绝类的错误，CDP/target 这类错误会被整个吞掉，然后把一个 route guard 已摘除、WebRTC 闸也没装上的标签当作创建成功返回。页面确实停在 about:blank，但"失败被报成了成功"是确凿的。给维护者的免修理由站不住，我在这里更正。 撤回的原因。这个 PR 从功能完成时的约 2400 行长到 5300 行，多出来的部分不是 #1766 要的三条诉求，而是预览功能自身副作用引发的连锁修复。为了让"预览标签重启后不复活"，我把预览标签做成了不落库的临时身份，这逼着我改写了右栏标签 store 的全部写路径——而那是每个用右栏浏览器的人每次开关切拖都会走到的公共代码。这是架构选型的问题，不是实现不仔细。 另外注意到 #2445（allow localhost browser previews）用很小的改动覆盖了"能不能打开本地地址"这一层。两个方案是互补的：它解决"能不能开"，剩下的需求是"开什么、谁来提供、边界在哪"。所以新 PR 只保留它不覆盖的部分。 重新提的方案（基于最新 main，短分支）。保留：HTTP 服务本体、路径 containment、token/TTL、CSP 响应头、Service Worker 清理、WebRTC 遮蔽、一个最小的导航拦截（防预览页自发导航到外部 origin 把内容带在 URL 里）、previewLocalHtml action、工具描述引导。撤掉：临时身份那一套（ephemeral 标记、转换队列、代次号、browser-preview-tabs.ts）、allowedOrigins 授权链、守卫的生命周期复杂度，以及右栏标签 store 的全部改动。预览标签只在托管浏览器里打开，不进右栏侧栏——#1766 的诉求是 agent 截图验证本地 HTML，agent 截图走的就是托管浏览器，侧栏可见是额外诉求。原先打到被撤代码上的意见随之作废，保留代码上的修复会随代码带进新 PR。 有两条已知问题会在新 PR 里一并修掉，不当残余：一是上面说的守卫安装失败被吞错，二是端口复用时预览页可借 BroadcastChannel 与同端口旧页通信（换 localhost 不解决这条，需要单独处理）。 想先请您确认这个方向是否合适。确认后我关闭本 PR，并基于最新 main 开一个新的。 |
这次改了什么
agent 需要"截图验证本地生成的 HTML"时，此前会因
file://与严格 SSRF 策略被拒而逃逸到裸跑 Chrome（历史上因此发生taskkill /F /IM chrome.exe误杀用户浏览器的事故，见 #1766）。本 PR 提供受控的本地 HTML 预览通道：新增previewLocalHtmlaction，通过 token 化的 loopback 预览服务在受管浏览器中打开工作区内 HTML，agent 无需再逃逸。怎么验证的
自动验证
（lizi-mcps 的
contactsTools15 个失败为环境性失败——stash 全部改动后仍失败，与本次改动无关）桌面端到端验证（2A，真实环境）
3 种 Agent 底座 × 2 种浏览器后端 = 6 个组合全部通过（Claude/Codex/Pi × 外置 Chrome/侧边栏 RSB）：每轮预览、snapshot、截图；多轮截图复核渲染一致（CSS/JS/相对资源均生效）。越界（
../）、隐藏目录、无工作区会话三种攻击路径在三底座下全部被拒。用户日常 Chrome 全程未被影响。评审迭代
已回应 22 轮自动评审（Greptile / Copilot / chatgpt-codex-connector / 本地独立 Codex 审查者），全部意见已修复并附测试：
default-src 'none'+connect-src 'none'+ sandbox；navigate-to经实测为 Chromium 无效指令，已弃用--disable-webrtc经实测无效，未采用更新器强退路径接入预览清理本轮（round 27l）已撤出：该改动经 review 判定不属本 PR 核心诉求主路径（§9.6-3 残余判据），连同preview-cleanup中间模块一并移出（见「风险」节更新器声明），转后续 issue 跟踪tabId → sessionIdMap（成功导航后登记、导航离开/手动关闭注销、撤销时经渲染器桥删除持久行）、以实际did-navigate提交事件维护 provenance（用户地址栏导航不再留下会被误删的登记）、撤销代际（generation）防导航-撤销竞态UI 变化
SSH 远程会话适配
SSH 远程会话下
previewLocalHtml返回 fail-closed 的UNAVAILABLE（本地预览服务无法读取远端工作目录），不泄露数据。适配方向三选一结论：明确降级 + 跟踪讨论远程侧预览方案——见关联 issue #2000（本 PR 不扩展 SSH 实现）。风险
previewLocalHtml时生效：启动一个127.0.0.1随机端口 listener，并向 SSRF 策略授予该精确 origin。不调用时行为与之前完全一致（默认配置不含任何本地网络放行）。预览页带 CSP + 无 fetch + 无 WebRTC + 导航锁定在预览 origin，无外泄通道。sync.mjs的 Windows 兼容修复拆为独立 PR chore(browser-runtime): make sync.mjs runnable on Windows #1834（已合并，本 PR 不含）。updateService.ts已恢复原状），原 round 19 的裁决请求撤回。此前经preview-cleanup中间模块接入的「强退时清理预览」能力待提 issue 跟踪（feat(browser): add sandboxed local HTML preview #1803 合并后提交）。撤出同时移除了preview-cleanup.ts（其唯一消费者即 updateService，撤出后为死代码）与browser.ts中的注册块——共 3 文件，纯撤出无新增改动。已声明残余（有意不修，经审查循环评估后记录）
以下为评审循环中经过评估、有意不修的已知理论边界，均已在对应 review thread 中声明（不阻塞本 PR，部分转后续 issue 跟踪）：
祖先链再次换入需句柄级 containment（Codex P1，thread XCGOO）：逐级 lstat 校验后并发进程仍可能换入祖先 symlink，彻底修复需要 Windows 没有的 openat/O_NOFOLLOW 等价物（句柄级逐级打开）。当前 lstat 链 + realpath 重验 + postStat 绑定已把窗口压缩到微秒级多步时序，有意不修；根治需平台级 API 支持。
撤销→标签关闭之间的窗口（Codex P1，thread XAkmq）：vendored 持久守卫无法实时读取运行时策略（policy 按调用传入、无全局注册表），撤销到标签失效之间是理论窗口。当前缓解：撤销与关闭绑定同一时刻 + 关闭等待有界，有意不修；如维护者认为必须闭合，需上游开放策略读取接口。
WebRTC 闸失败结果不入 open 应答（Codex P2，thread XBh3T）：把闸安装结果纳入 open 应答需改 renderer tab-op 协议，超出本 PR 范围。当前 fail-closed 已保证页面不会加载无防护内容（停在 about:blank），有意不修。
SSH 远程会话适配（Codex P1，thread XDFRt）：fail-closed 拒绝（UNAVAILABLE），完整支持需远端文件物化/流式到本地，属独立新功能，有意不修；见上「SSH 远程会话适配」节。
授权发布先于清理的串行化（Codex P1，thread XPIed）：前提已被 round 27f 消解（预览标签不再持久化、hydrate 过滤遗留行），实际窗口已无实体可命中；若仍有顾虑可在后续 issue 跟踪「授权发布与清理的串行化」，不阻塞本 PR。
端口复用破坏代际隔离（Greptile P1，thread QDt-）：刚释放的随机端口被下一代预览复用时，旧 closePreviewTabs 按 origin 匹配会误关新代同 origin 标签。极小概率边角（随机端口恰好立即复用 + 代际交替瞬间同发），完整修法需将 generation 贯穿登记项与双 sweep（改动大、收益极低），按 WORKFLOW §9.6 判定为已声明残余，转后续 issue 跟踪，不阻塞本 PR。
更新强退时清理预览（round 27l 撤出项）：原经
updateServiceforce-quit 路径接入的预览清理能力本轮整体撤出（唯一冲突文件 + 高危门禁裁决悬置），待提 issue（feat(browser): add sandboxed local HTML preview #1803 合并后提交）。撤出纯移除，未引入新行为。侧栏宿主迁移时预览标签消失（Codex P2，thread XQGwp）：预览标签刻意不落库（round 27e 防失效 URL 复活空白标签），但内嵌↔弹出切换会 invalidateSessionCaches + 释放 WebView 池，新 renderer 只能从 DB hydrate → 预览标签在宿主迁移瞬间消失。修复需改变「ephemeral 不落库」持久化策略（落库 + 按 token 生命周期冷启动清理），属对 round 27e 方案的重新设计，超出本 PR 范围；本项 P2、场景稀有，按 §9.6-3 登记残余，待提 issue（feat(browser): add sandboxed local HTML preview #1803 合并后提交），不阻塞本 PR。
端口复用 + 旧同源页 BroadcastChannel 外泄（Codex P1，thread Xz_3v）：随机端口被其他本地服务占用且其页面仍开着时，预览脚本可借 BroadcastChannel 与旧页通信（旧页有不受限出站）。修复需关闭/隔离同源旧页面或改隔离 partition，触及浏览器实例创建模型（成本中高）；场景需端口复用 + 旧页存活 + 恰好用 BroadcastChannel 三巧合同时发生。与残余第 6 条「端口复用破坏代际隔离」（Greptile P1，thread QDt-）同主题，同域同处理，判定为已声明残余，待提 issue（feat(browser): add sandboxed local HTML preview #1803 合并后提交），不阻塞本 PR。
清空激活标签未同步持久化（Codex P2，thread XzOZL）：
setActiveTab(sessionId, null)清空选择时跳过ipc.setActive({id:null})，SQLite 保留旧 active。场景（暂时清空选中但保留标签）不属本 PR 核心主路径，按 §9.6-3 登记残余，待提 issue（feat(browser): add sandboxed local HTML preview #1803 合并后提交），不阻塞本 PR。转换与拖动排序并发时位置写回旧值：边界转换的异步段结束后写库仍带调用时的 position，期间若发生 reorder 会把刚落库的新顺序改回旧位置；失败回滚路径还可能按旧下标覆盖到另一个标签。完整修复需要先定清转换与 reorder 之间的排序语义和失败重排语义，不是改一个下标能了事的，故登记为已声明残余，待本 PR 合并后提跟踪 issue。
closeTab 接管删除失败后的所有权：转换在途时 closeTab 接管删除并提升代次让旧转换放弃；若接管的删除自身重试耗尽，旧转换已被剥夺所有权而无人补偿，旧行残留。触发需「转换在途 + 用户关标签 + 数据库持续失败」三者同时，属错误路径边角；接管本身修复的是更易触发的两条件场景（净收益为正）。登记为已声明残余，待本 PR 合并后提跟踪 issue。