OpenHands Agent Canvas 定制代码评审实践:用可执行架构测试约束 AI 与人类评审 OpenHands Agent Canvas 定制代码评审实践用可执行架构测试约束 AI 与人类评审【免费下载链接】OpenHands OpenHands: AI-Driven Development项目地址: https://gitcode.com/GitHub_Trending/ope/OpenHands本文围绕 OpenHands 仓库中.agents/skills/custom-codereview-guide.md这份仓库专属代码评审指南展开讲清它如何把“评审决策规则、仓库所有权边界、可执行架构守卫”落地为可操作的 PR 评审方法论。读完本文你将掌握如何判断一个 PR 应给 APPROVE 还是 COMMENT、Agent Canvas 前端中哪些 API 访问路径属于“被机械禁止”的反模式、依赖精确锁定与测试证据标准是如何用测试代码而非口头约定强制执行的以及useEffect、事件契约、遥测所有权等具体评审检查点的源码级依据。评审指南的定位给 /codereview 技能补充仓库专属规则这份指南位于 .agents/skills/custom-codereview-guide.md文件头部的 YAML frontmatter 声明了它的技能元信息name: custom-codereview-guide description: Repository-specific review rules for the OpenHands Agent Canvas frontend. triggers: - /codereview也就是说当开发者或 AI Agent 在仓库中触发/codereview时这份文档会作为“仓库特定规则”补充通用的code-review技能。文档开宗明义地给出三条总原则先读 AGENTS.md——它是当前架构与测试约定的权威来源source of truth评审指南只是对它的补充直接且有建设性评审的是正确性与架构而不是 lint 或编译器已经检查过的格式问题指南中多处强调“可执行的源码才是权威”——评审规则若能用一个测试守住就不应该靠文档反复口头强调。这一理念贯穿全文下文会逐一对应到具体测试文件。评审决策规则只有 APPROVE 和 COMMENT指南对“提交怎样的评审结论”给出了非常明确的约束值得逐条拆解只提交一次评审且只有两种结果APPROVE 或 COMMENT永远不使用 REQUEST_CHANGES默认倾向 APPROVE没有重要发现时应当批准。“nitpick吹毛求疵和可选的重构建议不是拒绝批准的理由”COMMENT 用于正确性缺陷、安全问题、架构问题、缺少证据、或未满足验收标准。此时让人类维护者来做阻塞性决定blocking decision影响 Agent 行为的变更不能直接批准涉及 prompts、工具选择、会话 payload、终端行为、规划、记忆或评测路径的改动必须经过人类评审和相应轻量级 eval验收标准要落成清单阅读关联 issue为每条验收标准生成一个紧凑 checklist。满足 checklist 是必要条件但不能替代针对回归、安全、可维护性的评审。这套规则的核心思想是评审结论要表达“信心等级”而不是“否决权”——AI 或自动化评审给出 COMMENT 指出问题但阻塞合并的决定权保留在人类维护者手中同时通过“默认批准”抑制无意义的评审噪音。仓库所有权行为应该放在拥有它的仓库里指南给出了一张五仓库所有权矩阵要求评审时把行为放到“拥有该行为”的仓库仓库拥有范围OpenHands/OpenHands本仓库Agent Canvas UI、前端状态、后端选择、前端服务集成、本地栈编排OpenHands/software-agent-sdkAgent Server、agents、tools、conversations、events、workspaces以及规范的 server APIOpenHands/typescript-client浏览器兼容的 Agent Server API 类型化访问层OpenHands/extensions可复用的 skills、plugins 与集成OpenHands/automation调度、webhooks、运行历史、automation 派发正常的依赖方向是单向的Agent Server 契约 → TypeScript client → Canvas本仓库因此评审时要标记三类越界绕过类型化客户端、直接对 Agent Server 端点做原始 HTTP 访问raw endpoint reimplementation在 Canvas 本地复制服务端契约Canvas-local copies of server contracts变更开在了错误的仓库里。指导 Agent 的架构把“常规路径”变成最便宜的路径指南专门有一节讨论“代码库本身就是在指导 Agent 的决策”——因为 AI Agent 倾向于复制最近的模式、选择最短的能编译的路径所以架构必须从产品层面引导这些选择。它给出五条原则让常规路径成本最低新工作应当自然地复用已有的命名 hook、service、store 或 feature 模块而不是在共享的根组件里再加一个分支让被禁止的依赖机械地失败反复出现的评审指导应该沉淀为 lint 规则、编译器边界或架构测试。“当一个小而可执行的守卫更清晰时不要继续增长这份文档”——这正是下一节那些架构测试的由来持久状态只有一个显式写入者后端设置、consent 值、会话缓存条目、持久化的浏览器值都应该有一个命名 owner。要标记“第二个写入者”以及组件本地对权威状态的镜像副本偏好功能自有文件而非共享开关产品工作通常应该扩展 feature 自有模块共享注册表和根级条件分支需要给出具体理由例外要窄且可见例外清单allowlist应该放在执行规则的守卫旁边并按架构变更来评审。指南还给出一个重要的纠偏“deep module深模块是设计启发式不是行数目标”。好的模块接口窄而稳定、隐藏内聚的复杂度不要仅仅因为文件长就拆分也不要创建只做“重命名或转发参数”的中间层。只有当一个小的纯函数 seam接缝能消除重复决策、让所有权显式化、或支持聚焦测试时才值得引入。React effectsuseEffect只用于同步外部系统指南对useEffect的误用给出了一个明确的“红旗清单”评审中遇到 effect 在做以下事情时应标记从 props 或 state 派生渲染数据响应本可以在事件处理器中完成的用户动作初始化本应放在惰性 state 初始化器lazy state initializer里的值把一个 store 或缓存镜像进另一个组件的 state 值修复“竞争性写入者”造成的时序问题。但同时强调effect 并非天然错误。订阅、浏览器 API、定时器、网络同步仍然属于 effect 的正当用途——前提是 cleanup 和依赖语义是显式的。阻塞性架构检查点这一节是整份指南中“可执行守卫”最集中的部分每条规则都对应仓库中真实存在的测试或单一 owner 模块。Agent Server 与 Cloud API 访问架构测试即权威白名单文档声明src/api/no-direct-agent-server-calls.test.ts 是“可执行的权威来源”。不得批准任何新的对 Agent Server 端点的原始fetch、axios、共享openHandsaxios 实例或底层 HTTP 客户端访问正确做法是使用openhands/typescript-client加上 src/api/agent-server-client-options.ts 提供的选项。从源码看这个守卫的实现方式是对src/全目录做文本模式扫描任何命中即失败// src/api/no-direct-agent-server-calls.test.ts#L6-L11 const EXCLUDED_SEGMENTS new Set([mocks, routeTree.gen.ts]); const ALLOWED_AD_HOC_HTTP_FILES new Set([ api/automation-service/automation-service.api.ts, api/cloud/proxy.ts, api/main-app-auth.ts, ]);它检查的违规模式包括见 该测试 L38-L73违规模式说明openHands.调用直接使用共享 axios 实例createHttpClient(直接调用底层客户端工厂从openhands/typescript-client/client/http-client导入直接引用 SDK 的底层HttpClient模块new HttpClient(直接构造底层 HTTP 客户端axios(...)/axios.get/post/...直接用 axios 发请求白名单文件除外fetch(/api/...)用 fetch 直连/api路径白名单文件除外注意一个精妙的细节扫描会排除mocks/与routeTree.gen.tsmock 需要模拟真实 HTTP路由树是生成产物同时只允许三个文件做“ad-hoc HTTP”——它们恰好都是 HTTP 抽象的定义层本身云代理、automation 服务适配、主应用鉴权而不是业务代码。这正是指南第 5 条原则“例外窄且可见、且放在守卫旁边”的实例化白名单不写在指南文档里而是作为测试里的常量随代码一起演进。指南甚至明确警告“不要把它的当前条目复制进这份指南——测试应保持为唯一权威清单。”那“正确做法”长什么样src/api/agent-server-client-options.ts 就是所有类型化访问的统一入口。它导出两个关键函数getAgentServerClientOptions(overrides)L52-L69host 的解析优先级为overrides.host→overrides.conversationUrl经buildHttpBaseUrl归一化→ 当前有效本地后端的backend.hostAPI key 的优先级为sessionApiKey→apiKey→backend.apiKey没有可用后端且无覆盖项时抛出NoBackendAvailableError配套的类型守卫isNoBackendAvailableError支持跨打包边界的识别连name属性也做了兼容判断。getAgentServerHttpClientOptions(overrides)L71-L80把上面的选项转换为 SDK 需要的{ baseUrl, apiKey, timeout }形状超时默认 60000ms。对于 Cloud 与 runtime sandbox 请求规则是必须走callCloudProxy且 runtime 请求必须提供正确的hostOverride与鉴权模式。从 src/api/cloud/proxy.ts 的源码可以确认其契约export interface CloudProxyRequest { backend: Backend; method: CloudRequestOptions[method]; path: string; body?: unknown; headers?: Recordstring, string; timeoutSeconds?: number; hostOverride?: string; // 提供后走 runtime 专用客户端 authMode?: bearer | session-api-key | none; sessionApiKey?: string | null; responseType?: blob; }callCloudProxy内部根据hostOverride是否存在选择createCloudClientForRuntime(backend)指向运行时沙箱或createCloudClient(backend)指向云端authMode缺省按bearer处理。评审时对“代理白名单/守卫的改动”要按架构变更对待而不是普通逻辑修改。事件线契约SDK 是唯一 wire 权威指南确立了事件模型的三方关系SDK 事件模型是 wire 权威TypeScript client 是它的镜像Canvas 消费已发布的 client 类型。评审中不得批准Canvas 本地的重新声明redeclarations局部交叉类型partial intersections对 wire 事件接口的模块增强module augmentation往 wire 事件接口里塞展示presentation字段。契约变更必须按固定顺序落地SDK 模型/ schema 与序列化覆盖TypeScript client 中从 SDK payload 派生的镜像类型已发布的 client 版本Canvas 的消费与渲染/遥测覆盖。只属于 Canvas 的展示状态应放在以事件标识为键的独立 view model中与 wire 契约隔离。这条规则防止的是一类隐蔽腐化UI 需求不断往 wire 类型上“借”字段最终让前端类型与 SDK 真实报文悄悄分叉。遥测与持久化前端状态单一 owner 原则的具体化指南对遥测与持久状态给出五条硬规则并且每一条都能在源码中找到对应实现src/services/telemetry.ts 是 Canvas PostHog 客户端的唯一 owner。从源码看L1-L27 的模块注释与 L45-L54 的常量定义它统一管理 consent 相关的存储键openhands-telemetry-consent、...-pending-cloud-sync、...-pending-local-revocation、...-change事件、first-use 与 session 键和单例 PostHog 实例POSTHOG_INSTANCE_NAME agent-canvasReact 事件必须走 src/hooks/use-tracking.ts 中的类型化函数组件不得直接调用 PostHog。该文件的useTrackinghook 注释明确写道“共享的 PostHog 客户端强制执行 telemetry.ts 配置的规范 consent 状态本 hook 不得以 backend settings 作为 capture 门控因为后端切换时它们可能过期”——这是对“consent 只由单一控制器决定”原则在 hook 层的再次声明consent 渲染使用遥测 consent 的 external store而不是镜像的本地 statesetTelemetryConsent保持为唯一的 consent 控制器见 src/services/telemetry.ts 中的SetTelemetryConsentOptions其syncToCloud选项专门用于区分“用户主动设置”与“从后端镜像回来”两种写入场景避免镜像值被二次回写 Cloud一个业务里程碑只有一个规范 capture 点重复的条件性 capture 要被标记其他持久值优先使用既有的命名 service/store/hook标记来自任意组件的新存储写入。模块注释里还展示了这套 owner 化设计的实际收益consent 语义被显式写成“canvas_install在安装时立即发送、无论是否同意匿名、无 PIIsession/自定义事件仅在用户同意后才发送用户可通过VITE_DO_NOT_TRACK1或浏览器 Do Not Track 完全关闭遥测”。因为规则集中在单一模块评审者只需要审这一个文件就能判断 consent 行为是否被破坏。依赖与发布精确锁定 供应链审慎指南在依赖治理上给出四条规则其中三条由tests/package-library.test.ts 以测试形式强制执行直接依赖精确锁定exact-pinned。通过 npm 保持package.json与package-lock.json同步不要只手改一侧。对应测试用EXACT_SEMVER_PATTERN见 L20-L21校验dependencies与devDependencies中的每个版本必须形如\d\.\d\.\d可带 pre-release/build 元数据依赖豁免、git pin、安全覆盖属于可评审的政策变更。测试中的ALLOWED_STACK_PIN_DEPSL24-L27只豁免openhands/extensions与openhands/typescript-client两个 git 依赖并且源码注释写明了各自豁免的原因与回收条件其中 typescript-client 的豁免挂着TODO(#917)要求其在对应分支合并并发布到 npm 后移除——豁免本身是带理由和到期日期的评审时应把它当作政策变更来审警惕新发布的第三方版本带来的供应链风险。第一方 OpenHands 包豁免“等待期”但不豁免契约与发布顺序评审包版本变更属于显式的 release PR必须与 release workflow 的预期一致。该测试还锁定了openhands/agent-canvas包的发布面L29-L61main/module/types入口以及.、./conversation、./settings、./terminal、./i18n五个 export 子路径的类型/import/require 三元组都必须精确匹配防止发布配置在无意中被改动。测试与证据证据要与行为变更成比例指南的“Testing and Evidence”一节定义了评审时对证据的最低要求证据强度与行为变更成比例UI 行为要用真实应用的截图或视频CLI、API、脚本类变更要求给出精确的运行时命令与观察到的结果“单测本身不构成端到端证据”奖励真实逻辑与可观察状态不要奖励“只证明另一个 mock 被调用过”的 mock 测试保持测试聚焦一个行为一条有意义的断言路径不重复覆盖库自身行为不要脆弱的、纯展示层的快照遵循 AGENTS.md 的测试路由如果变更跨越完整全栈流程且缺少合适覆盖应建议 mock-LLM E2E 并视情况添加e2e-tests标签永不为了方便而扩大 live E2E 触发范围或暴露 secret。结合 AGENTS.md 的测试框架说明评审时的路由决策可以具体化为常规验证命令为npm run lint、npm test、npm run build和npm run build:libmock-LLM E2Etests/e2e/mock-llm/本地npm run test:e2e:mock-llm走“浏览器 → 真实 agent-server → 脚本化 mock LLM”的完整栈不消耗真实 LLM 凭据是绝大多数全栈行为变更的默认 E2E 证据来源其 CI workflow 还通过detect-pr-changes轻量任务对纯文档 PR 跳过重型任务live E2Etests/e2e/live/仅npm run test:e2e:live跑真实 LLM必须与 mocked 路径隔离且强制VITE_DO_NOT_TRACK1、在本地拦截任何 PostHog 请求以防污染分析数据。评审中若发现有人为了让测试“更容易过”而放宽 live E2E 触发条件属于明确的红线问题。不该评论什么抑制评审噪音指南同样明确了负面清单——以下情形不应留下评审评论工具链已处理的格式或小风格问题与变更无关的可选“nice to have”重构纯夸奖式观察应直接 approve对简单数据/配置变更额外索要测试当现有检查已覆盖该风险时临时的.pr/产物仓库自动化会清理它们。提出发现finding时有一条方法论要求沿着相关的调用流或数据流追踪到足以展示具体失败模式并且“一条高信号评论优于同一所有权问题的多条症状评论”。这与前面“持久状态单一写入者”的原则首尾呼应评审的目标是定位所有权缺陷的根因而不是罗列表面症状。沟通风格小步修正不制造反馈指南最后规定了评审沟通的基线简洁、具体、友好解释用户可见或架构层面的后果说明“不改会怎样”而不是只说“这里不对”给出最小可行修正局部修复使用 GitHub 的 suggestion 语法suggestion让作者可一键采纳PR 没问题就批准不要为了产出而制造反馈approve it without manufacturing feedback。小结.agents/skills/custom-codereview-guide.md的价值不在规则条目本身而在它示范的一种工程实践文档只写不可执行的部分——“评审决策倾向”“沟通风格”“证据标准”这类判断性规则留在文档“禁止 ad-hoc HTTP”“禁止 git 依赖”“包入口面锁定”这类可机械验证的规则全部沉淀为 no-direct-agent-server-calls.test.ts、package-library.test.ts 等架构测试文档只引用测试为权威src/api/agent-server-client-options.ts、src/api/cloud/proxy.ts、src/services/telemetry.ts 等则作为“唯一 owner”被指名道姓把 AI Agent 当作被架构影响的决策者——“让常规路径最便宜、让违规机械失败、让例外可见”这套原则同时服务于人类与自动化评审用所有权矩阵与依赖方向约束多仓库协作把“这个改动应该开在哪个仓库”变成可核查的评审点。如果你要在这类多仓库、强类型化、Agent 深度参与的代码库中开展评审人工或 AI 自动化这份指南提供了可直接迁移的检查点清单先确认仓库所有权再过 API 访问守卫、事件契约落地顺序、遥测单一 owner、依赖精确锁定与 E2E 证据成比例性这五道闸门最后用“最小可行修正 后果解释”输出结论。【免费下载链接】OpenHands OpenHands: AI-Driven Development项目地址: https://gitcode.com/GitHub_Trending/ope/OpenHands创作声明:本文部分内容由AI辅助生成(AIGC),仅供参考