Skip to content

feat(browser): 下载审批释放到工作区工件 — #491 P1 下载收口(层叠 #520) - #523

Merged
Qiyuanqiii merged 11 commits into
mainfrom
Qiyuanqiii/browser-download-release
Nov 2, 2026
Merged

Qiyuanqiii merged 11 commits into
mainfrom
Qiyuanqiii/browser-download-release

Conversation

@Qiyuanqiii

@Qiyuanqiii Qiyuanqiii commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

动机 / Motivation

收口 #491 P1 的下载故事(层叠在 #520 隔离捕获之上):捕获到的下载此前只躺在会话隔离区里随 profile 消亡,操作员没有任何"审批后放进工作区"的通道。

变更 / Changes

  • browserruntime ReadDownload:只对 completed 且已有摘要的回执放行;读出字节后重算 SHA-256 与回执比对,篡改/消失/未知一律拒绝;读失败统一包装为 ErrAgentBrowserDownloadGone——隔离区路径永不离开 runtime 层(评审 F1,安全修复)。
  • application ReleaseBrowserDownload:操作员释放调用本身即审批记录——control token 绑定精确 run+GUID;捕获与溯源 sidecar 以 screenshot 同款 O_EXCL + same-bytes 幂等模式写入 .cyberagent-workbench/browser-artifacts/downloads/<runID>/<GUID>(+.json);GUID 是磁盘上唯一文件名,页面控制的建议文件名只进 sidecar 元数据(sha256/url/suggested_filename/run_id/workspace_id/captured_at,确定性 JSON,无 operator/released_at 以保幂等)。
  • httpapi:GET /api/v1/runs/{run_id}/agent-browser/downloads(read token,回执列表,无路径字段)与 POST .../downloads/{guid}/release(control token,鉴权分离与 close 同构);OpenAPI 注册并再生成 docs/openapi.json。
  • Gone 错误在应用层映射为脱敏的 unavailable 文案(评审 F1);响应 operator 回显与生效身份归一化一致(F5)。

验证 / Verification

  • go vet 三包无告警;go test -race ./internal/application/ ./internal/browserruntime/ 通过;go test ./internal/httpapi/ 全套通过(OpenAPI golden + live-handler 路由检查)。
  • 三层测试覆盖:runtime(摘要重算/篡改拒绝/清理后 Gone/非 UUID 拒绝)、application(释放成功落盘 + sidecar 字段断言/幂等重放/未知/Gone 脱敏)、httpapi(列表 200、释放 200 + locator、read token 被拒 401)。
  • 评审记录(1 条 P2 阻塞 + 4 条 P3,全部修复并复测)见 docs/reviews/browser-download-release/。

后续 / Follow-ups(#491 剩余)

站点级授权(免全局 Full access)、会话级登录 profile(headed 接管承载)、上传、可配置预算——建议按 ADR 逐项推进。

关联 #491、#520。

评审跟进 / Review follow-up

外部评审提出两处问题,均已修复(commit 172deae8、fb1e3ae4)。

1. 释放审批的审计记录不完整(P2)

问题:释放虽已用 control token 授权,但落盘的 sidecar 只记录捕获来源(GUID、URL、摘要、run、workspace),不记录谁批准了释放、何时批准;且 operator 来自请求体,是调用方自述名称,不能证明操作者身份。

修复——把授权与审计拆开:

  • 保留原有的确定性捕获 sidecar,内容逐字节不变(新增断言钉死它的精确字节,它是对外契约)。
  • 新增独立的释放审批事件 <guid>.approval.json(O_CREATE|O_EXCL 独占创建、0600、与 sidecar 同目录):记录 guid、run_id、workspace_id、已校验的 sha256/bytes、approved_at(UTC)、authorized_principal、authorized_principal_scope、declared_operator。
  • 认证主体与调用方声明名分离:新增 application.AgentBrowserDownloadRelease{Principal, Operator};主体由服务端在 control token 校验通过后决定(常量 control_plane),应用层拒绝任何其它主体,调用方无法自称;声明名只作为 declared_operator 单独落盘。落盘内容不含 token、token hash 或任何可逆派生值(测试额外断言事件字节中不出现 token),并以 authorized_principal_scope: credential_class 明示该主体标识的是授权凭据类别而非自然人。
  • 不可覆盖:独占创建保证首次批准权威;重复释放(相同字节)读取既有事件并校验其身份字段,匹配则保留首次批准与其时间戳、文件不重写(测试同时断言字节与 mtime 未变),不匹配则拒绝(approval event conflicts with the released capture)且不覆盖既有文件。
  • 审批事件在捕获文件落盘之后写入,保证它引用的字节一定已在磁盘上。
  • HTTP 契约零变更:请求/响应 JSON 形状不变,docs/openapi.json、internal/httpapi/openapi.go、web/src/api/schema.d.ts、protocols/registry.json、docs/convergence/protocol-registry.md 全部未改动;cmd/protocolregistry -check 通过(复用已注册的 agent_browser_download_release.v1 版本串 + kind 判别,没有引入新的 *.vN 标识符),cmd/surfacecheck 通过。

2. PR 差异中的无关格式化噪声

本 PR 相对 base 原改 35 个文件,其中 20 个是与释放功能无关的纯格式化改动(store 迁移、waitgraph、llm/mcp/analyzer/sandbox/httpapi 测试与工具文件)。已按 base 分支状态逐字节还原(git diff origin/base -- <paths> 现为空),有效差异收敛为 15 个功能文件。还原后这些文件回到 base 自身的格式状态(base 本身未 gofmt 化),仓库 CI 无 Go gofmt 门禁,故不影响门禁。

验证(Go 1.26.10 / windows-amd64,commit fb1e3ae4)

  • gofmt -l 对本次改动的 4 个文件无输出;go vet ./internal/application/... ./internal/httpapi/... 无告警;go build ./... 通过。
  • go test -count=1 -run 'TestAgentBrowserDownload' ./internal/application/ 通过(0.832s);go test -count=1 -run 'AgentBrowser|BrowserDownload' ./internal/httpapi/ 通过(3.945s);go test -count=1 ./internal/httpapi/... 全量通过(303.260s,含 OpenAPI golden 契约测试)。
  • go test -count=1 ./internal/application/... 在本机跑不完:10m/40m/45m 三次超时但全程零 --- FAIL 行,卡住的都是与本改动无关的 drydock/yarn 项目准备用例(TestYarnPreparationAndDetectedTestReceipts、TestDrydockRewindSupportsCheckpointsAtACommittedDescendant);浏览器/下载相关子集全绿(-run 'AgentBrowser|FullCDP|Browser' → ok 9.866s)。该包在本机需要更快的机器,属既有环境限制,非本 PR 引入。
  • 合并冲突(internal/browserruntime/agent_browser_download_test.go 两侧同时在文件末尾追加用例)已按"保留双方"解决:合并结果 = base(含会话预算用例)+ 本 PR 的释放读取用例,14 个测试函数齐全,gofmt 干净,go test ./internal/browserruntime/ 通过。

合并前置条件(按评审的合并顺序)

  1. feat(browser): quarantine agent browser downloads — #491 P1 地基(隔离捕获 + 16MB 硬限 + SHA-256 回执) #520(含本轮 P1 会话预算修复)先合入 main;
  2. 随后把本 PR 的 base 从 Qiyuanqiii/browser-download-quarantine 改为 main 并重新验证,再合入 main——否则直接 Merge 只会把提交合进上游功能分支;
  3. 本 PR 已把 feat(browser): quarantine agent browser downloads — #491 P1 地基(隔离捕获 + 16MB 硬限 + SHA-256 回执) #520 的修复(2cd5cbb9)以 merge commit 并入本分支,因此 base 切换后 diff 仍只包含释放功能本身。

第二轮外部复审:审批记录的一致性与写失败恢复(两处 P2,已修)

F6 重复释放把"请求方"当成了"审批人"

释放是幂等的:第二次请求保留第一次写入的 .approval.json。但响应回显的是本次请求的
operator 与 time.Now()——Alice 先释放、Bob 再对同一 GUID 重放,磁盘上记的是 Alice,
HTTP 却报 Bob 和第二次时间。任何按响应建审计日志的客户端都会凭空多出一个审批人。

修法是让 service 返回真正落盘的那条审批元数据(新导出类型
application.AgentBrowserDownloadApproval{DeclaredOperator, ApprovedAt}):首次释放返回刚写的值,
重放返回磁盘事件里的 declared_operator/approved_at,httpapi 据此应答。同时给审批事件的完整性
校验补一条 declared_operator 非空,使回显值不可能为空。响应字段形状没变,
internal/httpapi/openapi.go 与 docs/openapi.json 本轮未改动,OpenAPI golden 契约测试通过。

上一轮的 F5 结论(用同一归一化回显请求方)已被取代:归一化后的请求身份仍然不是生效身份,
生效身份是磁盘上那条事件,releaseOperator 随之删除。

F7 工件写失败会把该 GUID 永久楔死

三个工件(GUID 命名的捕获、.json 来源记录、.approval.json)都是 O_CREATE | O_EXCL
新建后交给 writeFullCDPArtifact,写/同步/关失败时不清理。半截文件留在原地,下次释放走
"已存在→比对字节"分支即永久失败;半截审批 JSON 则报 approval event is invalid。

后果实测确认(一次性探针,未提交):临时摘掉回滚后,首次写失败 → 恢复写入口 → 重试固定报
browser artifact already contains different bytes。

修法是新增 writeNewAgentBrowserDownloadArtifact:写失败即 root.Remove 删掉本次刚创建的
那一个文件,与 internal/application/full_cdp_browser_actions.go 里截图工件的既有做法一致;
O_EXCL 不覆盖已完成工件的性质不变,前序已写成的工件下次按字节比对复用。

边界(要说清):这覆盖的是同一次调用内的写失败。进程在写的中途被强杀仍会留下半截文件,
那种残留需要临时文件 + 原子发布(或 RENAME_NOREPLACE 一类不覆盖式改名)才能消除,本 PR 不做——
它只把"可重试的错误路径"恢复成可重试,不改变"绝不覆盖已完成工件"的既有语义。
另外,删除失败本身仍是 best-effort 忽略错误(与截图分支一致),未改成 errors.Join。

验证(Go 1.26.10 / windows-amd64,commit 9e3c02a1)

三条新回归均先证红再证绿:

  • TestAgentBrowserDownloadReleaseReportsTheRecordedApproval:Alice 释放后 Bob 重放,断言返回值与
    磁盘 .approval.json 的 declared_operator/approved_at 一致。摘掉回显即红:
    replay reported the replaying caller: first={operator-alice …} replay={operator-bob …}。
  • TestAgentBrowserDownloadReleaseRollsBackFailedArtifactWrite:分别让第 1/2/3 次写入失败,断言本次
    新建的文件不留存、前序工件仍在、恢复写入口后同一释放重试成功。旧实现在三个子用例都红:
    failed create left a partial artifact。
  • TestAgentBrowserDownloadListAndReleaseEndpoint:响应必须报记录的审批而非请求方;旧实现 body 里是
    "operator":"openapi-operator"、released_at 是当次时间。
  • 既有幂等用例补了断言:重放返回值必须等于首次返回值,且磁盘事件字节与 mtime 不变。
  • gofmt -l 对本次改动 4 个文件无输出;go vet ./internal/application/ ./internal/httpapi/ 无告警;
    go test -count=1 -run TestAgentBrowserDownload ./internal/application/ 通过(0.155s);
    go test -count=1 ./internal/httpapi/ 全量通过(223.97s),含 OpenAPI golden 与 live-handler 路由;
    go test -count=1 ./internal/browserruntime/ 通过(9.19s)。
  • 上游同步:feat(browser): quarantine agent browser downloads — #491 P1 地基(隔离捕获 + 16MB 硬限 + SHA-256 回执) #520 的 context 取消修复(f77832b9)与其评审记录(d005b49f)已以 merge commit 并入本分支,
    base 切到 main 后 diff 仍只含释放功能。feat(browser): 接管模式 — 可见 headed 窗口承载会话级登录与人工处理(#491 P2) #524 也已继续携带(merge 到 2dcf89ba)。
  • 提交前跑 OCR delegate:ocr delegate preview(工作树模式)检出 2 个生产文件,两个测试文件按
    default_path 排除后连同 feat(browser): quarantine agent browser downloads — #491 P1 地基(隔离捕获 + 16MB 硬限 + SHA-256 回执) #520 的测试文件一并人工全文过;规则取 ocr delegate rule(Rule Group 1
    system Go)。独立评审子代理复核 7 个疑点并给出文件行号证据,报出 1 个 medium(失败注入测试的包级钩子
    只在内联路径恢复,中途 t.Fatalf 会把故障留给同包后续用例)——已改为换钩子后立即 t.Cleanup 恢复,
    记为 F8;1 个 low(忽略回滚删除的错误)按与截图分支保持一致的取舍保留。
  • internal/application 整包在本机仍跑不完(既有环境限制,与本改动无关),以包内浏览器/下载子集 +
    CI 的 Store/Application 分片为准。

…ace artifacts

Completes the #491 P1 download story on top of the quarantine capture:
an authenticated operator releases one completed capture into the run's
workspace artifacts. The release call is the recorded approval — it is
control-token bound to the exact run and GUID, the digest is recomputed
from the actual bytes before anything is copied, and the GUID remains
the only file name on disk with the page-controlled suggestion kept in a
provenance sidecar (sha256, url, suggested filename, run/workspace ids).
Repeating a release with identical bytes is idempotent.

- browserruntime: ReadDownload re-verifies the digest and refuses
  unknown, uncompleted, or tampered captures.
- application: BrowserDownloads list + ReleaseBrowserDownload with the
  O_EXCL/same-bytes artifact pattern shared with screenshots.
- httpapi: GET /agent-browser/downloads (read token) and POST
  /agent-browser/downloads/{guid}/release (control token), OpenAPI
  registered and regenerated.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ttp-openapi-contract family

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

…failed artifact writes

Two audit-integrity defects on the download release path.

A repeat release is idempotent: the first approval event survives on disk.
The response, however, echoed the replaying request — its declared operator
and a fresh timestamp — so Alice's approval was reported as Bob's to anyone
building an audit trail from the API. ReleaseBrowserDownload now returns the
approval event that is actually persisted, and the endpoint answers with its
declared operator and approved_at. An approval event without a declared
operator is rejected as conflicting, so the echoed value can never be empty.

The three artifacts are created exclusively and written in place. When the
write, sync, or close failed, the half-written file stayed behind and every
later release hit the exists-then-compare branch and failed permanently with
"browser artifact already contains different bytes"; that wedge was confirmed
on the pre-fix tree. A failed create now removes the file it just made,
matching the Full CDP screenshot artifact, so the next release can retry.
Base automatically changed from Qiyuanqiii/browser-download-quarantine to main November 2, 2026 07:18
@Qiyuanqiii
Qiyuanqiii merged commit 6a7625d into main Nov 2, 2026
25 of 26 checks passed
@Qiyuanqiii
Qiyuanqiii deleted the Qiyuanqiii/browser-download-release branch November 2, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants