From 094a8701fe4703b5b57720357c2b2f18e31fdfdf Mon Sep 17 00:00:00 2001 From: turtle1999 Date: Tue, 29 Sep 2026 12:48:04 +0800 Subject: [PATCH 001/201] Publish weighted approval as a completed check run Unmet weighted approval conditions previously left a pending commit status that CI watchers treat as running work, so gh pr checks --watch and similar monitors never finished. Publish the decision as the weighted approval decision check run instead: action_required blocks merging while completing the check, and failure reports evaluation errors. --- ...eighted-approvals-remain-pending.i18n.yaml | 22 --- ...ocked-weighted-approvals-remain-pending.md | 33 ---- ...ed-weighted-approvals-remain-pending.zh.md | 33 ---- ...pproval-review-workflow-identity.i18n.yaml | 4 +- ...09-10-approval-review-workflow-identity.md | 2 +- ...10-approval-review-workflow-identity.zh.md | 2 +- ...production-blame-approval-weight.i18n.yaml | 8 +- ...-09-11-production-blame-approval-weight.md | 4 +- ...-11-production-blame-approval-weight.zh.md | 4 +- ...026-09-15-pr-approval-delegation.i18n.yaml | 4 +- .../2026-09-15-pr-approval-delegation.md | 2 +- .../2026-09-15-pr-approval-delegation.zh.md | 2 +- ...hted-approval-decision-check-run.i18n.yaml | 22 +++ ...29-weighted-approval-decision-check-run.md | 50 ++++++ ...weighted-approval-decision-check-run.zh.md | 50 ++++++ .github/review-ownership/README.md | 12 +- .github/review-ownership/check-approval.mjs | 66 ++++--- .../review-ownership/check-approval.test.mjs | 169 +++++++++++------- .github/workflows/weighted-approval.yml | 14 +- scripts/ci-workflow.spec.ts | 8 +- 20 files changed, 293 insertions(+), 218 deletions(-) delete mode 100644 .agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.i18n.yaml delete mode 100644 .agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.md delete mode 100644 .agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.zh.md create mode 100644 .agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml create mode 100644 .agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md create mode 100644 .agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md diff --git a/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.i18n.yaml b/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.i18n.yaml deleted file mode 100644 index 3cf9104f6e..0000000000 --- a/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.i18n.yaml +++ /dev/null @@ -1,22 +0,0 @@ -# Bilingual-pair consistency record for 2026-09-09-blocked-weighted-approvals-remain-pending.md (docs/i18n/README.md): per heading -# section, a hash of its English and Chinese blocks outside code blocks and generated regions. -# After editing either side, bring the other along and re-record with: -# pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.md -/agent-note-blocked-weighted-approvals-remain-pending: - en: 6a5da25eb874aece - zh: 654aa967a9222501 -/agent-note-blocked-weighted-approvals-remain-pending/problem: - en: 0afdda30d02b666b - zh: 6624aa9c8c58ea0a -/agent-note-blocked-weighted-approvals-remain-pending/decision: - en: cf7ac63f422b4cfb - zh: 250385e42d546d30 -/agent-note-blocked-weighted-approvals-remain-pending/verification: - en: af674f57088d303a - zh: b9c42f04beec9be5 -/agent-note-blocked-weighted-approvals-remain-pending/alternatives-considered: - en: 13d5f67070bd3396 - zh: c75db3746d31272e -/agent-note-blocked-weighted-approvals-remain-pending/consequences: - en: 284f19608b0d449f - zh: 1d9bd95239ba6ffa diff --git a/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.md b/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.md deleted file mode 100644 index 50ab64dadc..0000000000 --- a/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.md +++ /dev/null @@ -1,33 +0,0 @@ -# Agent Note: Blocked weighted approvals remain pending - -Status: implemented - -English | [中文](2026-09-09-blocked-weighted-approvals-remain-pending.zh.md) - -## Problem - -The weighted approval commit status must distinguish an unmet merge condition from a failed policy evaluation. An effective `CHANGES_REQUESTED` review from a write-capable reviewer prevents a pull request from satisfying the approval policy, but it is a reversible review state rather than an evaluation failure. - -Publishing `failure` for that review state conflates the approval decision with the health of the publisher. It also treats one unmet policy condition differently from a draft pull request or insufficient approval points, which remain pending while contributors can resolve them. - -## Decision - -A completed weighted approval evaluation publishes `pending` when the pull request is a draft, has fewer than the required approval points, or has an effective `CHANGES_REQUESTED` review from a write-capable reviewer. A blocking review dominates the point total, so the status remains pending even when counted approvals reach the threshold. - -The evaluation publishes `success` only when the pull request is ready, the point threshold is met, and no blocking review exists. The separate `weighted approval publisher` Actions job reports whether evaluation and status publication completed. An evaluation failure publishes an `error` commit status and fails that job. - -## Verification - -[Approval policy tests](../../../../.github/review-ownership/check-approval.test.mjs) pin the threshold-reaching blocker case and the exact published `pending` payload. [Workflow tests](../../../../scripts/ci-workflow.spec.ts) pin the separate publisher job name. - -## Alternatives considered - -**Publish `failure` for a blocking review.** This keeps a visibly failed status until the review changes, but it represents an unmet and reversible merge condition as a malfunction and conflates policy outcome with publisher health. - -**Let approval points override a blocking review.** This makes the score the only success condition, but it permits a successful status while a write-capable reviewer's effective decision still requests changes. - -## Consequences - -Required-status branch rules block a pull request because `pending` does not satisfy the required status. Contributors can distinguish review work that remains from a failed approval evaluation, while the publisher job and `error` status retain the operational failure signal. - -Consumers do not receive a failed commit status solely because a blocking review exists. They must inspect the status description or effective reviews when they need to distinguish a blocker from other pending approval conditions. diff --git a/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.zh.md b/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.zh.md deleted file mode 100644 index 58ab10b3ac..0000000000 --- a/.agents/notes/implemented/process/2026-09-09-blocked-weighted-approvals-remain-pending.zh.md +++ /dev/null @@ -1,33 +0,0 @@ -# Agent Note: 阻塞中的加权批准保持 pending - -Status: implemented - -[English](2026-09-09-blocked-weighted-approvals-remain-pending.md) | 中文 - -## 问题 - -`weighted approval` commit status 必须区分尚未满足的合并条件与失败的策略评估。具有写权限的评审人所提交且仍然生效的 `CHANGES_REQUESTED` 评审会阻止 PR 满足批准策略，但它是一种可撤销的评审状态，而不是评估故障。 - -为这种评审状态发布 `failure` 会混淆批准决策与 publisher 的健康状态。它还会让一个尚未满足的策略条件区别于 draft PR 或批准点数不足；后两种情况在贡献者能够解决问题期间会保持 pending。 - -## 决策 - -完成的加权批准评估会在 PR 为 draft、批准点数少于要求，或具有写权限的评审人存在仍然生效的 `CHANGES_REQUESTED` 评审时发布 `pending`。阻塞性评审的优先级高于点数总和，因此即使计入的批准已经达到阈值，状态仍保持 pending。 - -只有在 PR 已进入 ready 状态、达到点数阈值且不存在阻塞性评审时，评估才发布 `success`。独立的 `weighted approval publisher` Actions job 报告评估和状态发布是否完成。评估故障会发布 `error` commit status，并使该 job 失败。 - -## 验证 - -[批准策略测试](../../../../.github/review-ownership/check-approval.test.mjs)锁定已达到阈值但仍有 blocker 的场景，以及准确发布的 `pending` payload。[工作流测试](../../../../scripts/ci-workflow.spec.ts)锁定独立的 publisher job 名称。 - -## 考虑过的替代方案 - -**为阻塞性评审发布 `failure`。** 该方案会在评审改变之前保持明显的失败状态，但它会把尚未满足且可撤销的合并条件表示为故障，并混淆策略结果与 publisher 的健康状态。 - -**允许批准点数覆盖阻塞性评审。** 该方案会让分数成为唯一的成功条件，但也允许在具有写权限的评审人仍然有效地要求修改时发布成功状态。 - -## 后果 - -需要该状态的分支规则会阻止 PR，因为 `pending` 不满足必需状态。贡献者可以区分尚待处理的评审工作与失败的批准评估，而 publisher job 和 `error` 状态保留运行故障信号。 - -消费方不会仅因存在阻塞性评审而收到失败的 commit status。如果需要区分 blocker 与其他 pending 批准条件，它们必须检查状态描述或仍然生效的评审。 diff --git a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml index 521e943a63..1dfe18dbe3 100644 --- a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml @@ -15,5 +15,5 @@ en: f8fdd869a03ac5ff zh: c355eb36e9cba350 /agent-note-identify-approval-review-workflows-by-file-path/consequences: - en: 9f31e023a04a7849 - zh: 66831c450af4c88f + en: 3edc29dc7c926f64 + zh: a84264a691a6248b diff --git a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md index 77164134e8..de1e08ef78 100644 --- a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md +++ b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md @@ -22,4 +22,4 @@ The [approval publisher](../../../../.github/review-ownership/check-approval.mjs Run-title expansion does not prevent approval refreshes, while an unexpected workflow file still fails validation. Moving the review-event workflow requires updating the publisher's expected path. -[Approval policy tests](../../../../.github/review-ownership/check-approval.test.mjs) cover a numbered run name, invalid source paths and events, unsuccessful runs, invalid titles, and superseded heads. The [approval outcome policy](2026-09-09-blocked-weighted-approvals-remain-pending.md) continues to own pending and successful status semantics. +[Approval policy tests](../../../../.github/review-ownership/check-approval.test.mjs) cover a numbered run name, invalid source paths and events, unsuccessful runs, invalid titles, and superseded heads. The [approval decision check run](2026-09-29-weighted-approval-decision-check-run.md) continues to own the published conclusions. diff --git a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md index 736851d6f3..6b3aa3d359 100644 --- a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md +++ b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md @@ -22,4 +22,4 @@ GitHub 可能用展开后的 `run-name` 填充工作流运行的 `name`。审批 运行标题的展开不会阻止审批刷新，而非预期的工作流文件仍无法通过验证。移动评审事件工作流时，需要更新发布器预期的路径。 -[审批策略测试](../../../../.github/review-ownership/check-approval.test.mjs) 覆盖带编号的运行名称、无效的来源路径和事件、未成功的运行、无效标题以及已被替代的头提交。[审批结果策略](2026-09-09-blocked-weighted-approvals-remain-pending.zh.md) 继续负责待定与成功状态的语义。 +[审批策略测试](../../../../.github/review-ownership/check-approval.test.mjs) 覆盖带编号的运行名称、无效的来源路径和事件、未成功的运行、无效标题以及已被替代的头提交。[审批决策 check run](2026-09-29-weighted-approval-decision-check-run.zh.md) 继续负责所发布的 conclusion。 diff --git a/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.i18n.yaml b/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.i18n.yaml index 9272a63484..683ee70272 100644 --- a/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.i18n.yaml @@ -9,14 +9,14 @@ en: 2dbd821b8d65b054 zh: 9bef2993db51a00d /agent-note-weight-approvals-by-changed-production-line-ownership/decision: - en: 7dc225f91538ace1 - zh: a22d05ffd8d32b6d + en: a0b790864eae2f43 + zh: aa2bd4ed8d569180 /agent-note-weight-approvals-by-changed-production-line-ownership/alternatives-considered: en: 5e8af03e3ff6018a zh: 6b518c8eee8c5172 /agent-note-weight-approvals-by-changed-production-line-ownership/consequences: - en: 08495483b77771be - zh: a798d4af0eda55fe + en: bcf33d4ce1d367c1 + zh: 0623ac49e33a59e6 /agent-note-weight-approvals-by-changed-production-line-ownership/verification: en: cb604625373372b9 zh: f9056d20ab612537 diff --git a/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.md b/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.md index d9a223c2f3..84ca47b4d8 100644 --- a/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.md +++ b/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.md @@ -12,7 +12,7 @@ A fixed one-point reviewer weight does not reflect authorship of the code a pull The [approval policy](../../../../.github/review-ownership/README.md) scales a one-point approval by `min(2, 1 + 4 × ownedLines / totalLines)` over changed old production lines. Ownership of 0% gives one point, 12.5% gives 1.5 points, and 25% or more gives two points. Scores are not rounded before comparison with the two-point success threshold. The merge base supplies both classification and blame, while GitHub associates blame commits with reviewer accounts. Unlinked authors remain in the denominator. Additions have no prior owner and contribute no lines; an empty denominator produces no boost. -The publisher marks the head pending before dependency setup and evaluation, so an interrupted history fetch cannot preserve an earlier success. It uses the live base branch and exact reviewed head, reads complete Git history without checking out PR code, and skips attribution when base points or blockers already decide the result. A maintained lexer separates comments from code across the repository’s source languages. Author lookups batch commits and all reviewers share one measurement. Existing [pending-status semantics](2026-09-09-blocked-weighted-approvals-remain-pending.md) and [review-event validation](2026-09-10-approval-review-workflow-identity.md) remain independent requirements. +The publisher marks the decision check run in progress before dependency setup and evaluation, so an interrupted history fetch cannot preserve an earlier success. It uses the live base branch and exact reviewed head, reads complete Git history without checking out PR code, and skips attribution when base points or blockers already decide the result. A maintained lexer separates comments from code across the repository’s source languages. Author lookups batch commits and all reviewers share one measurement. Existing [decision check-run semantics](2026-09-29-weighted-approval-decision-check-run.md) and [review-event validation](2026-09-10-approval-review-workflow-identity.md) remain independent requirements. ## Alternatives considered @@ -28,7 +28,7 @@ The publisher marks the head pending before dependency setup and evaluation, so History fetching dominates cold execution. Complete history is required before selecting the merge base: a shallow apparent ancestor can exclude changes from the denominator, so shallow classification cannot safely decide that blame is unnecessary. On 2026-09-11, local measurements of PRs #3969 and #3977 count 73 and 535 old production lines across 4 and 14 files. Three local runs take 0.31–0.32 seconds and 0.87–0.93 seconds; one batched author query adds approximately one second per PR. A fresh single-branch bare clone of master takes 47 seconds on the same host. These are host observations, not CI time guarantees. -Blame measures last-touch authorship, not review quality or semantic expertise. The production classifier excludes unsupported source locations and file extensions; adding shipped source outside its inventory requires extending that classifier. Lexer classification is lexical rather than a semantic test of executable behavior. Evaluation errors retain the error status instead of silently awarding or withholding the boost. +Blame measures last-touch authorship, not review quality or semantic expertise. The production classifier excludes unsupported source locations and file extensions; adding shipped source outside its inventory requires extending that classifier. Lexer classification is lexical rather than a semantic test of executable behavior. Evaluation errors conclude the check as `failure` instead of silently awarding or withholding the boost. ## Verification diff --git a/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.zh.md b/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.zh.md index f4dfc9d4fc..5c29e0938d 100644 --- a/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.zh.md +++ b/.agents/notes/implemented/process/2026-09-11-production-blame-approval-weight.zh.md @@ -12,7 +12,7 @@ Status: implemented [审批策略](../../../../.github/review-ownership/README.md) 按变更旧生产代码行的归属比例，以 `min(2, 1 + 4 × ownedLines / totalLines)` 调整一分批准的权重。归属比例为 0% 时计一分，12.5% 时计 1.5 分，25% 及以上时计两分。分数在与两分通过线比较前不做舍入。合并基点同时提供代码分类和 blame 依据，GitHub 将归属提交关联到评审者账号。无法关联账号的作者仍计入分母。新增行没有原作者，不计入行数；分母为空时不提升权重。 -发布器在依赖安装和评估前将头提交标记为待定，避免历史拉取中断后保留此前的成功状态。它使用实时 base 分支和被评审的精确 head，读取完整 Git 历史，不检出 PR 代码；基础分数或阻塞评审已决定结果时跳过归属计算。维护中的词法分析器区分仓库各源码语言中的注释与代码。作者查询按提交批量执行，所有评审者共用一次统计。现有的[待定状态语义](2026-09-09-blocked-weighted-approvals-remain-pending.zh.md)和[评审事件验证](2026-09-10-approval-review-workflow-identity.zh.md)仍是独立要求。 +发布器在依赖安装和评估前将决策 check run 标记为进行中，避免历史拉取中断后保留此前的成功状态。它使用实时 base 分支和被评审的精确 head，读取完整 Git 历史，不检出 PR 代码；基础分数或阻塞评审已决定结果时跳过归属计算。维护中的词法分析器区分仓库各源码语言中的注释与代码。作者查询按提交批量执行，所有评审者共用一次统计。现有的[决策 check run 语义](2026-09-29-weighted-approval-decision-check-run.zh.md)和[评审事件验证](2026-09-10-approval-review-workflow-identity.zh.md)仍是独立要求。 ## Alternatives considered @@ -28,7 +28,7 @@ Status: implemented 冷启动的主要开销是拉取历史。选择合并基点前必须具备完整历史：浅历史中看似共同祖先的提交可能使部分变更被排除出分母，因此不能据此安全判定无需 blame。2026-09-11，在本机测量 PR #3969 和 #3977，得到 4 和 14 个文件中的 73 和 535 行旧生产代码。三次本地运行分别耗时 0.31–0.32 秒和 0.87–0.93 秒；每个 PR 的一次批量作者查询额外耗时约一秒。同一主机上，全新单分支裸克隆 master 耗时 47 秒。这些是本机观测值，不是 CI 耗时保证。 -Blame 衡量最后修改者归属，不代表评审质量或语义上的专业能力。生产代码分类器排除未支持的源码位置和扩展名；在其清单之外添加交付源码时，必须扩展分类器。词法分类不判断代码是否在语义上可执行。评估错误保留错误状态，不静默授予或取消加权。 +Blame 衡量最后修改者归属，不代表评审质量或语义上的专业能力。生产代码分类器排除未支持的源码位置和扩展名；在其清单之外添加交付源码时，必须扩展分类器。词法分类不判断代码是否在语义上可执行。评估错误使该检查以 `failure` 结束，不静默授予或取消加权。 ## Verification diff --git a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml index 14bf4d7016..50bca79957 100644 --- a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml @@ -9,8 +9,8 @@ en: 5fe2f3b3cb7ce046 zh: ed27d4efc995b8b5 /agent-note-pr-scoped-approval-delegation/decision: - en: 41c5c90b82d323c6 - zh: 55d5ed09d88bb821 + en: bf8f1ff30114310f + zh: a163e1276dd384f8 /agent-note-pr-scoped-approval-delegation/alternatives-considered: en: a6006217cb29613a zh: 83968328096f1d91 diff --git a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md index 4dcf0e8a8d..0282f9c058 100644 --- a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md +++ b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md @@ -10,7 +10,7 @@ A reviewer may trust another reviewer to decide a particular PR while retaining ## Decision -The [approval policy](../../../../.github/review-ownership/README.md#delegating-points) accepts `/delegate @username` in PR conversation comments. Each eligible sender's points follow the named recipient's effective approval, once, using the sender's weight. Delegation applies only to that PR and only between write-capable accounts other than the PR author. Received points cannot be forwarded. The command dismisses the sender's prior approvals and change requests through GitHub. Other effective blocking reviews still follow [blocking-review policy](2026-09-09-blocked-weighted-approvals-remain-pending.md). +The [approval policy](../../../../.github/review-ownership/README.md#delegating-points) accepts `/delegate @username` in PR conversation comments. Each eligible sender's points follow the named recipient's effective approval, once, using the sender's weight. Delegation applies only to that PR and only between write-capable accounts other than the PR author. Received points cannot be forwarded. The command dismisses the sender's prior approvals and change requests through GitHub. Other effective blocking reviews still follow [blocking-review policy](2026-09-29-weighted-approval-decision-check-run.md). The latest surviving eligible command in comment creation order determines the recipient. Edited commands require the latest editor to match the author’s immutable account ID; other writers cannot transfer or cancel that author’s points by editing a comment. A self-delegation or a subsequently submitted review restores the sender's own decision. Comment-only reviews also reclaim the points; pending reviews do not. Equal timestamps favor the review, and subsequent dismissal cannot revive the delegation. A fresh command after the review can delegate again. Edits and deletions recompute from current comments, including restoration of older commands. The trusted publisher filters conversation comment changes for `/delegate`, resolves the live open PR head, and reads comments and editor identity as API data. Every evaluation reconciles active eligible commands, dismissing prior decision reviews and requesting recipients unless already approved or requested. This covers replaced queued events and draft-to-ready transitions; other requested reviewers remain unchanged. Scores refresh after dismissal, which retains the old submission timestamps and cannot cancel delegation. Removing the recipient's approval removes counted points while retaining delegation for their next approval. Drafts do not dismiss or request reviews. Dismissal failures fail evaluation; request failures are logged separately and retried on later evaluations. Closed or merged PRs are skipped before status writes and dependency setup. It preserves the separate [review-workflow validation](2026-09-10-approval-review-workflow-identity.md). diff --git a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md index bf21724d67..7d26a42d0d 100644 --- a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md +++ b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md @@ -10,7 +10,7 @@ Status: implemented ## Decision -[批准策略](../../../../.github/review-ownership/README.md#delegating-points) 接受 PR 普通评论中的 `/delegate @username`。每位符合资格的委托人，其分数跟随指定受托人的有效批准，按委托人的权重计入一次。委托仅适用于当前 PR，且双方都必须具有写权限，不能是 PR 作者。收到的分数不能继续转交。命令会通过 GitHub 撤销委托人原有的批准和变更请求。其他有效阻塞评审仍遵循[阻塞评审策略](2026-09-09-blocked-weighted-approvals-remain-pending.zh.md)。 +[批准策略](../../../../.github/review-ownership/README.md#delegating-points) 接受 PR 普通评论中的 `/delegate @username`。每位符合资格的委托人，其分数跟随指定受托人的有效批准，按委托人的权重计入一次。委托仅适用于当前 PR，且双方都必须具有写权限，不能是 PR 作者。收到的分数不能继续转交。命令会通过 GitHub 撤销委托人原有的批准和变更请求。其他有效阻塞评审仍遵循[阻塞评审策略](2026-09-29-weighted-approval-decision-check-run.zh.md)。 按评论创建顺序，最新、仍存在且符合资格的命令决定受托人。编辑过的命令要求最近编辑者与作者的不可变账户 ID 一致；其他有写权限的人不能通过编辑评论转交或取消作者的分数。委托给自己或随后提交评审会恢复使用委托人本人的评审决定。仅评论的评审也会收回分数；尚未提交的评审不会。时间戳相同时以评审为准，随后撤销评审也不会重新启用委托。评审之后的新命令可以再次委托。编辑和删除会根据当前评论重算，包括恢复较早的命令。可信发布器按 `/delegate` 筛选普通评论变更，解析实时且未关闭的 PR 头提交，并将评论及编辑者身份作为 API 数据读取。每次评估都会处理所有有效且符合资格的命令，撤销旧的决定性评审，并请求尚未批准、尚未收到请求的受托人进行评审。这覆盖了排队事件被替换以及草稿转为 ready 的情况；其他 requested reviewers 保持不变。撤销后刷新分数；撤销保留旧提交时间戳，不会取消委托。移除受托人的批准只会移除计入的分数，委托仍然保留，供其下次批准使用。草稿 PR 不撤销或请求评审。撤销失败会使评估失败；请求失败单独记录日志，并在后续评估中重试。已关闭或合并的 PR 会在写入状态和安装依赖之前跳过。独立的[评审工作流验证](2026-09-10-approval-review-workflow-identity.zh.md) 保持不变。 diff --git a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml new file mode 100644 index 0000000000..e3d242d672 --- /dev/null +++ b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml @@ -0,0 +1,22 @@ +# Bilingual-pair consistency record for 2026-09-29-weighted-approval-decision-check-run.md (docs/i18n/README.md): per heading +# section, a hash of its English and Chinese blocks outside code blocks and generated regions. +# After editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md +/agent-note-weighted-approval-decisions-are-completed-check-runs: + en: 1b120e96b0f5f340 + zh: ff20c20fe0558964 +/agent-note-weighted-approval-decisions-are-completed-check-runs/problem: + en: 6ba6f6a15a1562ec + zh: 1cbef3e736c258e6 +/agent-note-weighted-approval-decisions-are-completed-check-runs/decision: + en: 933e4c1438771907 + zh: f6056382f3b555c9 +/agent-note-weighted-approval-decisions-are-completed-check-runs/verification: + en: d1f65b5b577b08b8 + zh: 7a193cec1acec003 +/agent-note-weighted-approval-decisions-are-completed-check-runs/alternatives-considered: + en: a7ef91c0506766a8 + zh: d6584e31652b14ca +/agent-note-weighted-approval-decisions-are-completed-check-runs/consequences: + en: 4cafdf12b326c68a + zh: c37a57d26d2f0894 diff --git a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md new file mode 100644 index 0000000000..f4d8f9e639 --- /dev/null +++ b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md @@ -0,0 +1,50 @@ +# Agent Note: Weighted approval decisions are completed check runs + +Status: implemented + +English | [中文](2026-09-29-weighted-approval-decision-check-run.zh.md) + +## Problem + +The weighted approval policy must block merging until its conditions hold, distinguish an unmet condition from a failed evaluation, and let tools that wait for pull-request checks to finish terminate. A draft pull request, insufficient approval points, and an effective `CHANGES_REQUESTED` review from a write-capable reviewer are reversible review states rather than evaluation failures. + +Commit statuses offer only `pending`, `success`, `failure`, and `error`. A `pending` commit status blocks merging, but `gh pr checks --watch` and other CI watchers treat it as running work and wait until someone approves. A `failure` commit status ends the wait but reports an unmet, reversible condition as a malfunction and conflates the policy outcome with publisher health. + +## Decision + +The [publisher](../../../../.github/review-ownership/check-approval.mjs) reports the decision as the `weighted approval decision` check run on the pull-request head, written with the job's GitHub Actions token. Branch rules require that check run with GitHub Actions as its expected source. + +| Condition | Check run | +|---|---| +| Dependency setup or evaluation in progress | `in_progress` | +| Draft, fewer than the required points, or an effective write-capable `CHANGES_REQUESTED` review | `completed`, conclusion `action_required` | +| Ready, threshold met, and no blocking review | `completed`, conclusion `success` | +| Setup or evaluation failure | `completed`, conclusion `failure` | + +A blocking review dominates the point total, so the conclusion remains `action_required` even when counted approvals reach the threshold. GitHub accepts only `success`, `neutral`, and `skipped` for a required check, so `action_required` blocks merging while completing the check. + +The publisher lists the head's check runs with that name, updates every run owned by GitHub Actions, and creates one only when none exists. An incomplete list or a run without a valid ID fails the publication. + +## Verification + +[Approval policy tests](../../../../.github/review-ownership/check-approval.test.mjs) pin the threshold-reaching blocker case, the exact published check-run payloads, updating an existing run instead of creating another, and invalid list responses. [Workflow tests](../../../../scripts/ci-workflow.spec.ts) pin the `checks: write` permission, the `in_progress` and `failure` phase commands, and the separate publisher job name. + +## Alternatives considered + +**Keep the `pending` commit status.** Branch rules block correctly, but CI watchers cannot distinguish an approval wait from running checks and do not finish. + +**Publish `failure` for unmet conditions.** This ends the wait, but it represents an unmet, reversible merge condition as a malfunction and conflates the policy outcome with publisher health. + +**Publish nothing until success.** A required check that was never reported remains expected and is invisible to many watchers. Revoking a success already published on the same head still requires a non-success write, and each watcher's treatment of a missing required check is unspecified. + +**Keep the `weighted approval` name.** When a check and a commit status share a required name, GitHub requires both to pass. Heads that carry a `pending` commit status from the earlier publisher would stay blocked until a new push. + +**Create a new check run on every evaluation.** Several same-name check runs on one head make the result depend on how GitHub and each watcher select among them. + +**Let approval points override a blocking review.** This makes the score the only success condition, but it permits success while a write-capable reviewer's effective decision still requests changes. + +## Consequences + +CI watchers finish while the decision waits on reviewers. `gh pr checks` groups `action_required` with failed checks and exits non-zero; consumers distinguish it by the check name, the `ACTION_REQUIRED` conclusion, or the output title, which carries the score or blocker. The `weighted approval publisher` Actions job and the `failure` conclusion carry operational failures. + +Revoking an earlier success on the same head returns the completed check run to `in_progress`, a transition the GitHub REST reference does not restrict. diff --git a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md new file mode 100644 index 0000000000..4dee85c8f7 --- /dev/null +++ b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md @@ -0,0 +1,50 @@ +# Agent Note: 加权批准决策使用已完成的 check run + +Status: implemented + +[English](2026-09-29-weighted-approval-decision-check-run.md) | 中文 + +## 问题 + +加权批准策略必须在条件满足之前阻止合并，区分尚未满足的条件与失败的评估，并让等待 PR 检查结束的工具能够退出。draft PR、批准点数不足，以及具有写权限的评审人仍然生效的 `CHANGES_REQUESTED` 评审，都是可撤销的评审状态，而不是评估故障。 + +commit status 只提供 `pending`、`success`、`failure` 和 `error`。`pending` commit status 会阻止合并，但 `gh pr checks --watch` 和其他 CI 监视工具把它当作仍在运行的工作，会一直等到有人批准。`failure` commit status 能结束等待，但会把尚未满足且可撤销的条件报告为故障，并混淆策略结果与 publisher 的健康状态。 + +## 决策 + +[publisher](../../../../.github/review-ownership/check-approval.mjs) 使用该 job 的 GitHub Actions token，在 PR head 上以 `weighted approval decision` check run 报告决策。分支规则要求该 check run，并把 GitHub Actions 设为预期来源。 + +| 条件 | Check run | +|---|---| +| 依赖安装或评估进行中 | `in_progress` | +| draft、点数少于要求，或存在具有写权限的评审人仍然生效的 `CHANGES_REQUESTED` 评审 | `completed`，conclusion 为 `action_required` | +| 已 ready、达到阈值且没有阻塞性评审 | `completed`，conclusion 为 `success` | +| 安装或评估失败 | `completed`，conclusion 为 `failure` | + +阻塞性评审的优先级高于点数总和，因此即使计入的批准已经达到阈值，conclusion 仍为 `action_required`。GitHub 只接受 `success`、`neutral` 和 `skipped` 作为必需检查的通过结果，因此 `action_required` 会阻止合并，同时让该检查处于完成状态。 + +publisher 列出 head 上同名的 check run，更新所有属于 GitHub Actions 的 run，只有在不存在时才创建一个。列表不完整或 run 没有有效 ID 时发布失败。 + +## 验证 + +[批准策略测试](../../../../.github/review-ownership/check-approval.test.mjs)锁定已达到阈值但仍有 blocker 的场景、准确发布的 check run payload、更新已有 run 而不是另建一个，以及无效的列表响应。[工作流测试](../../../../scripts/ci-workflow.spec.ts)锁定 `checks: write` 权限、`in_progress` 和 `failure` 阶段命令，以及独立的 publisher job 名称。 + +## 考虑过的替代方案 + +**保留 `pending` commit status。** 分支规则能正确阻止合并，但 CI 监视工具无法区分等待批准与仍在运行的检查，因此不会结束。 + +**为尚未满足的条件发布 `failure`。** 该方案能结束等待，但会把尚未满足且可撤销的合并条件表示为故障，并混淆策略结果与 publisher 的健康状态。 + +**在成功之前不发布任何结果。** 从未上报的必需检查保持 expected 状态，许多监视工具看不到它。撤销同一 head 上已发布的成功仍然需要写入非成功结果，而各监视工具如何对待缺失的必需检查没有规定。 + +**保留 `weighted approval` 名称。** 当 check 和 commit status 使用同一个必需名称时，GitHub 要求两者都通过。带有旧 publisher 所写 `pending` commit status 的 head 会一直被阻止，直到出现新的 push。 + +**每次评估都创建新的 check run。** 同一 head 上存在多个同名 check run 时，结果取决于 GitHub 和各监视工具如何在其中选择。 + +**允许批准点数覆盖阻塞性评审。** 该方案会让分数成为唯一的成功条件，但也允许在具有写权限的评审人仍然有效地要求修改时发布成功结果。 + +## 后果 + +决策等待评审人时，CI 监视工具会结束。`gh pr checks` 把 `action_required` 与失败的检查归为一组并以非零状态退出；消费方可以通过 check 名称、`ACTION_REQUIRED` conclusion 或携带分数或 blocker 的输出标题加以区分。`weighted approval publisher` Actions job 和 `failure` conclusion 承载运行故障信号。 + +撤销同一 head 上此前的成功结果时，已完成的 check run 会回到 `in_progress`；GitHub REST 参考文档没有限制这一转换。 diff --git a/.github/review-ownership/README.md b/.github/review-ownership/README.md index 2da4c5e2cd..7b461ae864 100644 --- a/.github/review-ownership/README.md +++ b/.github/review-ownership/README.md @@ -16,11 +16,11 @@ The [`weighted-approval` workflow](../workflows/weighted-approval.yml) publishes ## Approval scoring -The weighted approval workflow exposes two pull-request checks. The `weighted approval publisher` Actions job reports whether evaluation and status publication completed, while the `weighted approval` commit status carries the approval decision on the pull request head. Branch rules must require only the commit status with GitHub Actions as its expected source; a context-only requirement can accept a same-named status from another integration. The publisher marks the head pending before Python setup or lexer installation, so failed setup or an interrupted history fetch cannot leave a previous success in place. Setup and evaluation failures publish an error status. A completed evaluation returns `pending` below two approval points, while the pull request is a draft, or while a write-capable reviewer has an effective `CHANGES_REQUESTED` review; the blocker keeps the status pending even when counted approvals reach the threshold. It returns `success` only when the threshold is met, the pull request is ready, and no such blocker exists. If evaluation fails, the publisher writes an `error` status. +The weighted approval workflow exposes two pull-request checks. The `weighted approval publisher` Actions job reports whether evaluation and check publication completed, while the `weighted approval decision` check run carries the approval decision on the pull request head. Branch rules must require that check run with GitHub Actions as its expected source; a name-only requirement can accept a same-named check or status from another integration. The publisher updates the head's existing GitHub Actions decision check run and creates one only when none exists. It marks the check run `in_progress` before Python setup or lexer installation, so failed setup or an interrupted history fetch cannot leave a previous success in place. Setup and evaluation failures conclude it as `failure`. A completed evaluation concludes `action_required` below two approval points, while the pull request is a draft, or while a write-capable reviewer has an effective `CHANGES_REQUESTED` review; the blocker keeps that conclusion even when counted approvals reach the threshold. It concludes `success` only when the threshold is met, the pull request is ready, and no such blocker exists. `action_required` blocks merging but completes the check, so tools that wait for checks to finish do not wait for reviewers; the check-run title carries the score or blocker. Reviewers whose calculated base repository permission is `write` or `admin` count. The [approval policy](approval-policy.json) gives `@07akioni`, `@imccyu`, `@tianyicui`, `@tianyicui-bot`, `@turtle1999`, and `@turtle2099` two points each; every other write-capable reviewer gets one point. The pull-request author’s own review and reviewers without write permission do not count. -The PR author contributes `min(100, mergedPRCount) × 11 / 1000` points: 0 merged PRs → 0 points, 50 → 0.55, and 100 or more → 1.1. The publisher counts only merged PRs in this repository using the author’s immutable account ID, excluding the current PR. It stops at 100 matches and rejects incomplete history responses. Counts refresh at the next subscribed evaluation event. Author credit is separate from reviewer approvals and cannot satisfy the two-point requirement alone; the author’s own review remains excluded. Human and bot authors, including Dependabot, use the same rule. Drafts, blocking reviews, and sufficient reviewer points skip history lookup; logs mark credit as not evaluated. Otherwise, logs show author credit and the capped count separately. Below the cap, counting may scan the repository’s entire merged history; a failed required lookup publishes an error. +The PR author contributes `min(100, mergedPRCount) × 11 / 1000` points: 0 merged PRs → 0 points, 50 → 0.55, and 100 or more → 1.1. The publisher counts only merged PRs in this repository using the author’s immutable account ID, excluding the current PR. It stops at 100 matches and rejects incomplete history responses. Counts refresh at the next subscribed evaluation event. Author credit is separate from reviewer approvals and cannot satisfy the two-point requirement alone; the author’s own review remains excluded. Human and bot authors, including Dependabot, use the same rule. Drafts, blocking reviews, and sufficient reviewer points skip history lookup; logs mark credit as not evaluated. Otherwise, logs show author credit and the capped count separately. Below the cap, counting may scan the repository’s entire merged history; a failed required lookup concludes the check as `failure`. A one-point approval receives weight `min(2, 1 + 4 × ownedLines / totalLines)` from modified or deleted old production-code lines, attributed by `git blame` at the merge base of the live base branch and exact reviewed head. Ownership of 0%, 12.5%, and 25% gives 1, 1.5, and 2 points; higher ownership remains capped at 2. The success threshold remains 2 total points, without rounding the score. New lines do not enter the denominator, and an empty denominator gives no boost. Existing two-point weights remain unchanged. GitHub commit-author accounts identify reviewers across author emails; unlinked authors remain in the denominator without contributing to a reviewer. The publisher logs measured ownership. It skips attribution when reviewer points plus author credit already meet the threshold or a blocking review exists. Displayed scores use at most three decimal places; the decision uses unrounded scores with a tolerance of `1e-12` points for floating-point error. The curve endpoints come from the policy’s default and required points. @@ -28,13 +28,13 @@ Production source means supported code files under `src/` in `packages/`, `apps/ Each reviewer contributes only the current `APPROVED` or `CHANGES_REQUESTED` decision that GitHub returns. A `DISMISSED` record clears that reviewer's standing decision, including earlier approvals. Comment-only and pending records do not replace a decision. Reviews from deleted accounts and reviewers without current repository access do not count. The workflow does not invalidate an approval by its review commit; the repository's native pull-request rules own stale-review and latest-push requirements. -The publisher runs when a pull request opens, synchronizes, reopens, becomes ready, becomes a draft, or is edited, including a base-branch change. Conversation comment events run the publisher only when the current or previous body contains `/delegate`. Closed or merged PRs and stale review heads are skipped before status writes or dependency setup; the workflow does not subscribe to master pushes. GitHub associates `workflow_run` publisher runs with the default branch even though approval statuses target the open PR head. Pull-request, review, and comment events share one concurrency group per PR. Review submissions, edits, and dismissals run the no-permission [`weighted-approval-review-event` workflow](../workflows/weighted-approval-review-event.yml); its validated run title supplies the pull-request number to the default-branch publisher. The publisher validates the current head, fetches every review and conversation comment, and resolves current repository permission before publishing the status. Permission changes take effect on the next subscribed event. +The publisher runs when a pull request opens, synchronizes, reopens, becomes ready, becomes a draft, or is edited, including a base-branch change. Conversation comment events run the publisher only when the current or previous body contains `/delegate`. Closed or merged PRs and stale review heads are skipped before check-run writes or dependency setup; the workflow does not subscribe to master pushes. GitHub associates `workflow_run` publisher runs with the default branch even though decision check runs target the open PR head. Pull-request, review, and comment events share one concurrency group per PR. Review submissions, edits, and dismissals run the no-permission [`weighted-approval-review-event` workflow](../workflows/weighted-approval-review-event.yml); its validated run title supplies the pull-request number to the default-branch publisher. The publisher validates the current head, fetches every review and conversation comment, and resolves current repository permission before publishing the check run. Permission changes take effect on the next subscribed event. ## Delegating points -Post `/delegate @username` as the entire text of a PR conversation comment to transfer your reviewer points to that user's effective `APPROVED` decision on this PR. For example, `/delegate @turtle1999` lets @turtle1999's approval count your points alongside their own. Until they approve, your points do not count, even if your own approval remains active. Each evaluation reconciles every active eligible command: it dismisses the sender's existing `APPROVED` and `CHANGES_REQUESTED` reviews, then requests review from recipients who have neither approved nor already been requested. This also handles commands posted on drafts once the PR becomes ready and commands whose original event was replaced in the concurrency queue. Other requested reviewers remain unchanged. The command does not submit a review; drafts do not dismiss or request reviews. Comment-only and pending reviews cannot be dismissed. A failed dismissal publishes an error and prevents review requests; scores are refreshed after successful dismissal. Request failures are logged without changing the published score, and subsequent evaluations retry outstanding requests. +Post `/delegate @username` as the entire text of a PR conversation comment to transfer your reviewer points to that user's effective `APPROVED` decision on this PR. For example, `/delegate @turtle1999` lets @turtle1999's approval count your points alongside their own. Until they approve, your points do not count, even if your own approval remains active. Each evaluation reconciles every active eligible command: it dismisses the sender's existing `APPROVED` and `CHANGES_REQUESTED` reviews, then requests review from recipients who have neither approved nor already been requested. This also handles commands posted on drafts once the PR becomes ready and commands whose original event was replaced in the concurrency queue. Other requested reviewers remain unchanged. The command does not submit a review; drafts do not dismiss or request reviews. Comment-only and pending reviews cannot be dismissed. A failed dismissal concludes the check as `failure` and prevents review requests; scores are refreshed after successful dismissal. Request failures are logged without changing the published score, and subsequent evaluations retry outstanding requests. Both accounts must currently have write or admin permission, and neither may be the PR author. Commands involving ineligible accounts are ignored. A sender's newest surviving eligible, author-authorized command wins, in comment creation order; editing an older comment does not move it after newer comments. Submitting any review after delegation automatically takes the points back, including an approval, change request, or comment-only review. Submitting an inline diff comment with “Add single comment” also creates a comment-only review and reclaims the points. Pending reviews do not count; a later dismissal does not restore the delegation. When timestamps are equal, the review takes precedence. A new command after the review can delegate again; editing an older command cannot reactivate it. Post `/delegate @your-own-username` to restore your own review decision. Editing or deleting a command recomputes delegation from remaining comments, so deleting the latest command can restore an older one. Quoted commands, code blocks, review bodies, inline review comments, and commands mixed with other text do not count. Surrounding whitespace is allowed; account matching is case-insensitive. An edited command is accepted only when GitHub identifies the original author as its latest editor; edits by other accounts cannot delegate or cancel that author’s points. @@ -44,7 +44,7 @@ Each sender contributes at most once, using their own fixed weight or production ## Security -All actions in the status-writing job are pinned to commit SHAs. The job checks out only the repository default branch. It does not check out or execute pull-request code and does not use repository secrets. Only when there is no blocker, reviewer points plus author credit are insufficient, and an approval has the policy’s default weight, it fetches complete history using the job token, passes Git objects to the trusted classifier as data, and resolves commit authors in batches of 50. Fetch credentials exist only in the Git child environment. Missing history, parsing failures, or incomplete author queries fail evaluation rather than producing a partial score. The review-event workflow has no `GITHUB_TOKEN` permissions and passes only a decimal pull-request number in its run title. The publisher accepts only successful `pull_request_review` runs from the review-event workflow file, identified by `workflow_run.path`; GitHub can populate `workflow_run.name` with the expanded run title. The publisher rejects an invalid run title and a number that does not resolve to the workflow run's current pull-request head. Pull-request reviews and comments are treated as API data and escaped in logs. All publisher triggers and steps, including dependency setup, have pull-request write permission; the policy uses it to dismiss eligible senders' prior decision reviews and request recipients. Every evaluation verifies command authors and latest editors by immutable account ID through GraphQL, matching the body to the REST comment history before counting points or acting. Missing or inconsistent editor metadata fails evaluation. Non-author edits are ignored. Only active eligible delegations are reconciled; superseded or revoked commands have no effects. Dismissal failures publish an error; request failures remain separate from the approval decision. +All actions in the check-writing job are pinned to commit SHAs. The job checks out only the repository default branch. It does not check out or execute pull-request code and does not use repository secrets. Only when there is no blocker, reviewer points plus author credit are insufficient, and an approval has the policy’s default weight, it fetches complete history using the job token, passes Git objects to the trusted classifier as data, and resolves commit authors in batches of 50. Fetch credentials exist only in the Git child environment. Missing history, parsing failures, or incomplete author queries fail evaluation rather than producing a partial score. The review-event workflow has no `GITHUB_TOKEN` permissions and passes only a decimal pull-request number in its run title. The publisher accepts only successful `pull_request_review` runs from the review-event workflow file, identified by `workflow_run.path`; GitHub can populate `workflow_run.name` with the expanded run title. The publisher rejects an invalid run title and a number that does not resolve to the workflow run's current pull-request head. Pull-request reviews and comments are treated as API data and escaped in logs. All publisher triggers and steps, including dependency setup, have pull-request write permission; the policy uses it to dismiss eligible senders' prior decision reviews and request recipients. Every evaluation verifies command authors and latest editors by immutable account ID through GraphQL, matching the body to the REST comment history before counting points or acting. Missing or inconsistent editor metadata fails evaluation. Non-author edits are ignored. Only active eligible delegations are reconciled; superseded or revoked commands have no effects. Dismissal failures conclude the check as `failure`; request failures remain separate from the approval decision. Approval policy changes take effect only after they merge into the default branch. This prevents an untrusted pull request from changing the program or policy for its own run. @@ -52,7 +52,7 @@ Approval policy changes take effect only after they merge into the default branc ## Verification -Run `pnpm run test:approval-policy` for policy parsing, effective review decisions, review-event validation, merged-history pagination and account matching, lazy author credit, permission filtering, weighted scoring, delegation conservation and revocation, editor authorization, comment pagination, dismissal and review-request reconciliation, blockers, drafts, closed-PR skips, status publication, and API failures. [Workflow tests](../../scripts/ci-workflow.spec.ts) pin the trusted checkout, no-permission review handoff, permissions, events, and commands. The repository gate graph runs the approval policy and workflow tests in CI. The Python SDK job runs `uv run --python 3.10 --with-requirements .github/review-ownership/requirements.txt python -m unittest discover -s .github/review-ownership -p 'test_*.py'` for real Git histories, lexers, renames, shallow-history rejection, and the publisher’s fetch/analysis integration. +Run `pnpm run test:approval-policy` for policy parsing, effective review decisions, review-event validation, merged-history pagination and account matching, lazy author credit, permission filtering, weighted scoring, delegation conservation and revocation, editor authorization, comment pagination, dismissal and review-request reconciliation, blockers, drafts, closed-PR skips, check-run publication, and API failures. [Workflow tests](../../scripts/ci-workflow.spec.ts) pin the trusted checkout, no-permission review handoff, permissions, events, and commands. The repository gate graph runs the approval policy and workflow tests in CI. The Python SDK job runs `uv run --python 3.10 --with-requirements .github/review-ownership/requirements.txt python -m unittest discover -s .github/review-ownership -p 'test_*.py'` for real Git histories, lexers, renames, shallow-history rejection, and the publisher’s fetch/analysis integration. diff --git a/.github/review-ownership/check-approval.mjs b/.github/review-ownership/check-approval.mjs index 5792299c46..489acd1779 100644 --- a/.github/review-ownership/check-approval.mjs +++ b/.github/review-ownership/check-approval.mjs @@ -10,7 +10,7 @@ import { authorCreditPoints, countMergedAuthorPulls } from './author-weight.mjs' const API_VERSION = '2026-03-10' const MAX_PULL_REQUEST_RECORDS = 3_000 const PAGE_SIZE = 100 -const STATUS_CONTEXT = 'weighted approval' +const CHECK_NAME = 'weighted approval decision' const WRITABLE_PERMISSIONS = new Set(['admin', 'write']) const REVIEW_STATES = new Set(['APPROVED', 'CHANGES_REQUESTED', 'COMMENTED', 'DISMISSED', 'PENDING']) @@ -209,13 +209,13 @@ function timestamp(value, subject) { /** * Evaluate approval points from current reviews and repository permissions. * @param {{event: unknown, policySource: string, api: (path: string, options?: {method?: string, body?: unknown}) => Promise, getOwnership?: typeof productionOwnership, getMergedCount?: typeof countMergedAuthorPulls}} options Runtime inputs. - * @returns {Promise<{pull: {repository: string, number: number, headSha: string}, state: 'pending' | 'success', description: string, points: number, authorCredit: {mergedCount: number, points: number} | null, requiredPoints: number, approvals: Array<{login: string, points: number, delegatedTo?: string, ownership?: {ownedLines: number, totalLines: number}}>, delegations: Array<{login: string, delegatedTo: string, commentId: number, reviewIds: number[], approved: boolean}>, blockers: string[], ignoredReviewers: string[]}>} Approval decision and status payload fields; approval login owns the points, delegatedTo supplies its decision, delegations contains eligible active commands and their superseded decision review IDs, and null author credit means history was not evaluated. + * @returns {Promise<{pull: {repository: string, number: number, headSha: string}, state: 'action_required' | 'success', description: string, points: number, authorCredit: {mergedCount: number, points: number} | null, requiredPoints: number, approvals: Array<{login: string, points: number, delegatedTo?: string, ownership?: {ownedLines: number, totalLines: number}}>, delegations: Array<{login: string, delegatedTo: string, commentId: number, reviewIds: number[], approved: boolean}>, blockers: string[], ignoredReviewers: string[]}>} Approval decision and check-run output fields; approval login owns the points, delegatedTo supplies its decision, delegations contains eligible active commands and their superseded decision review IDs, and null author credit means history was not evaluated. */ export async function evaluateApproval({ event, policySource, api, getOwnership = productionOwnership, getMergedCount = countMergedAuthorPulls }) { const pull = pullRequestFromEvent(event) const policy = parseApprovalPolicy(policySource) if (pull.draft) { - return approvalResult(pull, policy.requiredPoints, [], [], [], 'pending', 'draft pull request') + return approvalResult(pull, policy.requiredPoints, [], [], [], 'action_required', 'draft pull request') } const reviews = await listPullRequestReviews(api, pull.repository, pull.number) @@ -286,11 +286,11 @@ export async function evaluateApproval({ event, policySource, api, getOwnership return next }, authorCredit?.points ?? 0) if (blockers.length > 0) { - return approvalResult(pull, policy.requiredPoints, approvals, blockers, ignoredReviewers, 'pending', + return approvalResult(pull, policy.requiredPoints, approvals, blockers, ignoredReviewers, 'action_required', `${blockers.length} blocking change request${blockers.length === 1 ? '' : 's'}`, authorCredit, activeDelegations) } // Tolerate floating-point addition error without rounding approval scores. - const state = points + 1e-12 >= policy.requiredPoints ? 'success' : 'pending' + const state = points + 1e-12 >= policy.requiredPoints ? 'success' : 'action_required' return approvalResult( pull, policy.requiredPoints, @@ -305,13 +305,13 @@ export async function evaluateApproval({ event, policySource, api, getOwnership } /** - * Evaluate and publish the required commit status, publishing an error status when evaluation fails. + * Evaluate and publish the required check run, concluding it as failure when evaluation fails. * @param {{event: unknown, policySource: string, api: (path: string, options?: {method?: string, body?: unknown}) => Promise, runUrl: string, write?: (line: string) => void, getOwnership?: typeof productionOwnership, getMergedCount?: typeof countMergedAuthorPulls}} options Runtime inputs. * @returns {Promise>>} Published approval decision. */ export async function runApprovalCheck({ event, policySource, api, runUrl, getOwnership = productionOwnership, getMergedCount = countMergedAuthorPulls, write = line => process.stdout.write(`${line}\n`) }) { const pull = pullRequestFromEvent(event) - await publishStatus(api, pull, 'pending', 'Evaluating approval points.', runUrl) + await publishCheck(api, pull, 'in_progress', 'Evaluating approval points.', runUrl) let result try { result = await evaluateApproval({ event, policySource, api, getOwnership, getMergedCount }) @@ -331,7 +331,7 @@ export async function runApprovalCheck({ event, policySource, api, runUrl, getOw result = await evaluateApproval({ event, policySource, api, getOwnership, getMergedCount }) } } catch (error) { - await publishStatus(api, pull, 'error', 'Approval evaluation failed.', runUrl) + await publishCheck(api, pull, 'failure', 'Approval evaluation failed.', runUrl) throw error } write(result.authorCredit @@ -342,8 +342,8 @@ export async function runApprovalCheck({ event, policySource, api, runUrl, getOw `@${login}: ${points}${delegatedTo ? ` (delegated to @${delegatedTo})` : ''}${ownership ? ` (${ownership.ownedLines}/${ownership.totalLines} old production lines)` : ''}`)) writeList(write, 'Blocking change requests', result.blockers.map(login => `@${login}`)) writeList(write, 'Ignored reviewers without write access', result.ignoredReviewers.map(login => `@${login}`)) - await publishStatus(api, pull, result.state, result.description, runUrl) - write(`Published ${JSON.stringify(STATUS_CONTEXT)} status ${JSON.stringify(result.state)}.`) + await publishCheck(api, pull, result.state, result.description, runUrl) + write(`Published ${JSON.stringify(CHECK_NAME)} check run ${JSON.stringify(result.state)}.`) try { await requestDelegatedReviews(result, api, write) } catch (error) { @@ -353,14 +353,14 @@ export async function runApprovalCheck({ event, policySource, api, runUrl, getOw } /** - * Revoke a previous success before dependency installation, or report its failure. - * @param {{event: unknown, api: (path: string, options: object) => Promise, runUrl: string, phase: string}} options Publication inputs. - * @returns {Promise} Completion of the status write. + * Revoke a previous conclusion before dependency installation, or report its failure. + * @param {{event: unknown, api: (path: string, options?: {method?: string, body?: unknown}) => Promise, runUrl: string, phase: string}} options Publication inputs. + * @returns {Promise} Completion of the check-run write. */ export async function publishApprovalPhase({ event, api, runUrl, phase }) { - if (!['pending', 'error'].includes(phase)) throw new Error('invalid approval setup phase') - await publishStatus(api, pullRequestFromEvent(event), phase, - phase === 'pending' ? 'Preparing approval evaluation.' : 'Approval setup or evaluation failed.', runUrl) + if (!['in_progress', 'failure'].includes(phase)) throw new Error('invalid approval setup phase') + await publishCheck(api, pullRequestFromEvent(event), phase, + phase === 'in_progress' ? 'Preparing approval evaluation.' : 'Approval setup or evaluation failed.', runUrl) } /** @@ -414,7 +414,7 @@ export async function approvalEventFromComment({ event, api }) { } /** - * Refresh a pull-request event before any status write; closed PRs and stale heads are skipped. + * Refresh a pull-request event before any check-run write; closed PRs and stale heads are skipped. * @param {{event: unknown, api: (path: string) => Promise}} options Pull-request event and API caller. * @returns {Promise | null>} Live event, or null for a closed PR or superseded head. */ @@ -467,18 +467,26 @@ async function reviewerPermission(api, repository, login) { return response.permission.toLowerCase() } -async function publishStatus(api, pull, state, description, runUrl) { +// Updating the head's existing GitHub Actions check run keeps one decision per head for branch rules and CI watchers. +async function publishCheck(api, pull, state, title, runUrl) { if (!/^https:\/\/[^\s]+$/u.test(runUrl)) throw new Error('workflow run URL must use HTTPS') - if (description.length > 140) throw new Error('commit status description exceeds 140 characters') - await api(`/repos/${pull.repository}/statuses/${pull.headSha}`, { - method: 'POST', - body: { - state, - context: STATUS_CONTEXT, - description, - target_url: runUrl, - }, - }) + const body = { + ...(state === 'in_progress' ? { status: state } : { status: 'completed', conclusion: state }), + details_url: runUrl, + output: { title, summary: `${title} [Workflow run](${runUrl})` }, + } + const listed = await api(`/repos/${pull.repository}/commits/${pull.headSha}/check-runs?check_name=${encodeURIComponent(CHECK_NAME)}&filter=all&per_page=${PAGE_SIZE}`) + if (!isRecord(listed) || !Array.isArray(listed.check_runs) || listed.total_count !== listed.check_runs.length) { + throw new Error('check-run list response is invalid or incomplete') + } + const runs = listed.check_runs.filter(run => isRecord(run) && isRecord(run.app) && run.app.slug === 'github-actions') + if (runs.length === 0) { + await api(`/repos/${pull.repository}/check-runs`, { method: 'POST', body: { name: CHECK_NAME, head_sha: pull.headSha, ...body } }) + } + for (const { id } of runs) { + if (!Number.isSafeInteger(id)) throw new Error('check run has no valid id') + await api(`/repos/${pull.repository}/check-runs/${id}`, { method: 'PATCH', body }) + } } function pullRequestFromEvent(event) { @@ -571,7 +579,7 @@ async function main() { const phase = process.argv[2] if (phase) { await publishApprovalPhase({ event, api, runUrl: process.env.GITHUB_RUN_URL ?? '', phase }) - if (phase === 'pending' && process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, 'active=true\n') + if (phase === 'in_progress' && process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, 'active=true\n') return } await runApprovalCheck({ diff --git a/.github/review-ownership/check-approval.test.mjs b/.github/review-ownership/check-approval.test.mjs index b5d0b6cbb3..186e97d1b3 100644 --- a/.github/review-ownership/check-approval.test.mjs +++ b/.github/review-ownership/check-approval.test.mjs @@ -11,14 +11,18 @@ import { evaluateApproval as evaluateWithHistory, listPullRequestReviews, parseApprovalPolicy, - runApprovalCheck as runWithHistory, + runApprovalCheck as runPublisher, publishApprovalPhase, } from './check-approval.mjs' const policySource = readFileSync(new URL('approval-policy.json', import.meta.url), 'utf8') -const withoutComments = api => (path, options) => path.includes('/comments?') ? [] : api(path, options) -const evaluateApproval = options => evaluateWithHistory({ getMergedCount: async () => 0, ...options, api: withoutComments(options.api) }) -const runApprovalCheck = options => runWithHistory({ getMergedCount: async () => 0, ...options, api: withoutComments(options.api) }) +const withoutCheckRuns = api => (path, options) => + path.includes('/check-runs?') ? { total_count: 0, check_runs: [] } : api(path, options) +const withEmptyReads = api => withoutCheckRuns((path, options) => path.includes('/comments?') ? [] : api(path, options)) +const evaluateApproval = options => evaluateWithHistory({ getMergedCount: async () => 0, ...options, api: withEmptyReads(options.api) }) +const runWithHistory = options => runPublisher({ ...options, api: withoutCheckRuns(options.api) }) +const runApprovalCheck = options => runWithHistory({ getMergedCount: async () => 0, ...options, api: withEmptyReads(options.api) }) +const publishedState = body => body.conclusion ?? body.status const HEAD_SHA = '1234567890abcdef1234567890abcdef12345678' const pullRequestEvent = ({ author = 'author', draft = false } = {}) => ({ @@ -198,7 +202,7 @@ test('accepts two one-point approvals and ignores reviews without write access', assert.deepEqual(result.ignoredReviewers, ['reader']) }) -test('keeps one one-point approval pending without failing the status', async () => { +test('requires action for one one-point approval without failing the check', async () => { const result = await evaluateApproval({ event: pullRequestEvent(), policySource, @@ -209,7 +213,7 @@ test('keeps one one-point approval pending without failing the status', async () throw new Error(`unexpected API path ${path}`) }, }) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') assert.equal(result.points, 1) }) @@ -228,11 +232,11 @@ test('ignores a reviewer whose collaborator permission lookup returns 404', asyn }, }) const result = await evaluateApproval({ event: pullRequestEvent(), policySource, api }) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') assert.deepEqual(result.ignoredReviewers, ['former-writer']) }) -test('keeps the status pending on a write-capable change request while ignoring the author and read-only reviewers', async () => { +test('requires action on a write-capable change request while ignoring the author and read-only reviewers', async () => { const statuses = [] const result = await runApprovalCheck({ event: pullRequestEvent({ author: 'author' }), @@ -251,7 +255,7 @@ test('keeps the status pending on a write-capable change request while ignoring if (path.includes('/collaborators/turtle1999/permission')) return { permission: 'admin' } if (path.includes('/collaborators/blocker/permission')) return { permission: 'write' } if (path.includes('/collaborators/reader/permission')) return { permission: 'read' } - if (path.includes('/statuses/')) { + if (path.includes('/check-runs')) { statuses.push(options.body) return {} } @@ -259,32 +263,35 @@ test('keeps the status pending on a write-capable change request while ignoring }, write: () => {}, }) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') assert.equal(result.description, '1 blocking change request.') assert.equal(result.points, 2) assert.deepEqual(result.blockers, ['blocker']) assert.deepEqual(result.ignoredReviewers, ['reader']) assert.deepEqual(statuses, [{ - state: 'pending', - context: 'weighted approval', - description: 'Evaluating approval points.', - target_url: 'https://github.example/actions/runs/1', + name: 'weighted approval decision', + head_sha: HEAD_SHA, + status: 'in_progress', + details_url: 'https://github.example/actions/runs/1', + output: { title: 'Evaluating approval points.', summary: 'Evaluating approval points. [Workflow run](https://github.example/actions/runs/1)' }, }, { - state: 'pending', - context: 'weighted approval', - description: '1 blocking change request.', - target_url: 'https://github.example/actions/runs/1', + name: 'weighted approval decision', + head_sha: HEAD_SHA, + status: 'completed', + conclusion: 'action_required', + details_url: 'https://github.example/actions/runs/1', + output: { title: '1 blocking change request.', summary: '1 blocking change request. [Workflow run](https://github.example/actions/runs/1)' }, }]) }) -test('keeps drafts pending without reading reviews', async () => { +test('requires action on drafts without reading reviews', async () => { const result = await evaluateApproval({ event: pullRequestEvent({ draft: true }), policySource, getOwnership: async () => ({ totalLines: 0, reviewerLines: {} }), api: async () => { throw new Error('draft evaluation must not call GitHub') }, }) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') assert.equal(result.points, 0) assert.match(result.description, /draft pull request/u) }) @@ -301,21 +308,23 @@ test('publishes the required status and replaces stale success with error on eva calls.push({ path, options }) if (path.includes('/reviews?')) return [review('turtle2099', 'APPROVED')] if (path.includes('/collaborators/turtle2099/permission')) return { permission: 'write' } - if (path.includes('/statuses/')) return {} + if (path.includes('/check-runs')) return {} throw new Error(`unexpected API path ${path}`) }, write: line => output.push(line), }) assert.equal(result.state, 'success') assert.deepEqual(calls.at(-1), { - path: `/repos/deepseek-harness/deepseek-harness/statuses/${HEAD_SHA}`, + path: '/repos/deepseek-harness/deepseek-harness/check-runs', options: { method: 'POST', body: { - state: 'success', - context: 'weighted approval', - description: '2/2 approval points.', - target_url: 'https://github.example/actions/runs/1', + name: 'weighted approval decision', + head_sha: HEAD_SHA, + status: 'completed', + conclusion: 'success', + details_url: 'https://github.example/actions/runs/1', + output: { title: '2/2 approval points.', summary: '2/2 approval points. [Workflow run](https://github.example/actions/runs/1)' }, }, }, }) @@ -330,7 +339,7 @@ test('publishes the required status and replaces stale success with error on eva runUrl: 'https://github.example/actions/runs/2', api: async (path, options = {}) => { if (path.includes('/reviews?')) throw new Error('reviews unavailable') - if (path.includes('/statuses/')) { + if (path.includes('/check-runs')) { failures.push({ path, options }) return {} } @@ -338,9 +347,9 @@ test('publishes the required status and replaces stale success with error on eva }, write: () => {}, }), /reviews unavailable/u) - assert.equal(failures[0].options.body.state, 'pending') - assert.equal(failures[1].options.body.state, 'error') - assert.equal(failures[1].options.body.description, 'Approval evaluation failed.') + assert.equal(publishedState(failures[0].options.body), 'in_progress') + assert.equal(publishedState(failures[1].options.body), 'failure') + assert.equal(failures[1].options.body.output.title, 'Approval evaluation failed.') }) test('sends authenticated JSON and escapes an API error body', async () => { @@ -383,7 +392,7 @@ for (const [ownedLines, totalLines, expectedPoints] of [[9, 100, 1.3599999999999 }) assert.equal(result.points, expectedPoints) assert.equal(result.description, `${Number(expectedPoints.toFixed(3))}/2 approval points (author 0).`) - assert.equal(result.state, expectedPoints === 2 ? 'success' : 'pending') + assert.equal(result.state, expectedPoints === 2 ? 'success' : 'action_required') assert.equal(measurements, 1) assert.deepEqual(result.approvals[0].ownership, { ownedLines, totalLines }) }) @@ -403,7 +412,7 @@ test('does not fetch ownership when a change request blocks approval', async () }) assert.equal(measurements, 0) assert.equal(result.points, 2) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') }) test('does not fetch history when approvals already have two-point weights', async () => { @@ -424,11 +433,11 @@ test('publishes error when production attribution fails', async () => { if (path.includes('/reviews?')) return [review('writer', 'APPROVED')] if (path.includes('/permission')) return { permission: 'write' } if (path.includes('/comments?')) return [] - states.push(options.body.state) + states.push(publishedState(options.body)) return {} }, }), /incomplete history/u) - assert.deepEqual(states, ['pending', 'error']) + assert.deepEqual(states, ['in_progress', 'failure']) }) @@ -437,17 +446,17 @@ test('revokes a previous success before starting expensive attribution', async ( await runApprovalCheck({ event: pullRequestEvent(), policySource, runUrl: 'https://github.example/run/1', write: () => {}, getOwnership: async () => { - assert.deepEqual(states, ['pending']) + assert.deepEqual(states, ['in_progress']) return { totalLines: 100, reviewerLines: { writer: 25 } } }, api: async (path, options) => { if (path.includes('/reviews?')) return [review('writer', 'APPROVED')] if (path.includes('/permission')) return { permission: 'write' } - states.push(options.body.state) + states.push(publishedState(options.body)) return {} }, }) - assert.deepEqual(states, ['pending', 'success']) + assert.deepEqual(states, ['in_progress', 'success']) }) for (const reviewers of [['first', 'second'], ['turtle1999', 'first']]) { @@ -477,14 +486,38 @@ test('publishes setup phases without evaluating or installing dependencies', asy const states = [] const options = { event: pullRequestEvent(), runUrl: 'https://github.example/run/1', - api: async (path, { body }) => { assert.match(path, /\/statuses\//u); states.push(body.state) }, + api: withEmptyReads(async (path, { body }) => { assert.match(path, /\/check-runs$/u); states.push(publishedState(body)) }), } - await publishApprovalPhase({ ...options, phase: 'pending' }) - await publishApprovalPhase({ ...options, phase: 'error' }) - assert.deepEqual(states, ['pending', 'error']) + await publishApprovalPhase({ ...options, phase: 'in_progress' }) + await publishApprovalPhase({ ...options, phase: 'failure' }) + assert.deepEqual(states, ['in_progress', 'failure']) await assert.rejects(publishApprovalPhase({ ...options, phase: 'success' }), /invalid approval setup phase/u) }) +test('updates the head\'s GitHub Actions decision check run instead of creating another', async () => { + const calls = [] + const checkRuns = [ + { id: 7, app: { slug: 'github-actions' } }, + { id: 8, app: { slug: 'other-app' } }, + ] + await publishApprovalPhase({ + event: pullRequestEvent(), runUrl: 'https://github.example/run/1', phase: 'in_progress', + api: async (path, options) => { + calls.push({ path, method: options?.method }) + return { total_count: checkRuns.length, check_runs: checkRuns } + }, + }) + assert.deepEqual(calls, [ + { path: `/repos/deepseek-harness/deepseek-harness/commits/${HEAD_SHA}/check-runs?check_name=weighted%20approval%20decision&filter=all&per_page=100`, method: undefined }, + { path: '/repos/deepseek-harness/deepseek-harness/check-runs/7', method: 'PATCH' }, + ]) + for (const response of [{}, { total_count: 101, check_runs: [] }, { total_count: 1, check_runs: [{ id: '7', app: { slug: 'github-actions' } }] }]) { + await assert.rejects(publishApprovalPhase({ + event: pullRequestEvent(), runUrl: 'https://github.example/run/1', phase: 'failure', api: async () => response, + }), /check-run list response|valid id/u) + } +}) + for (const [mergedCount, credit] of [[0, 0], [1, 0.011], [50, 0.55], [99, 1.089], [100, 1.1], [101, 1.1], [200, 1.1]]) { test(`author with ${mergedCount} merged PRs contributes ${credit} points but cannot approve alone`, async () => { const result = await evaluateApproval({ @@ -493,7 +526,7 @@ for (const [mergedCount, credit] of [[0, 0], [1, 0.011], [50, 0.55], [99, 1.089] }) assert.equal(result.points, credit) assert.equal(result.authorCredit.points, credit) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') assert.deepEqual(result.approvals, []) }) } @@ -509,7 +542,7 @@ test('combines author credit and reviewer ownership at the passing threshold', a } }) -for (const [mergedCount, state] of [[90, 'pending'], [91, 'success'], [100, 'success']]) { +for (const [mergedCount, state] of [[90, 'action_required'], [91, 'success'], [100, 'success']]) { test(`one ordinary approval with ${mergedCount} merged PRs is ${state}`, async () => { const result = await evaluateApproval({ event: pullRequestEvent(), policySource, getMergedCount: async () => mergedCount, @@ -532,7 +565,7 @@ test('blockers skip history and ignore non-write reviewers', async () => { ? [review('writer', 'APPROVED'), review('blocker', 'CHANGES_REQUESTED'), review('reader', 'APPROVED')] : { permission: path.includes('/reader/') ? 'read' : 'write' }, }) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') assert.equal(result.points, 1) assert.equal(result.authorCredit, null) assert.deepEqual(result.blockers, ['blocker']) @@ -550,20 +583,20 @@ test('drafts skip history and history failures revoke success with error', async getMergedCount: async () => { throw new Error('history unavailable') }, api: async (path, options) => { if (path.includes('/reviews?')) return [] - states.push(options.body.state) + states.push(publishedState(options.body)) }, }), /history unavailable/u) - assert.deepEqual(states, ['pending', 'error']) + assert.deepEqual(states, ['in_progress', 'failure']) }) -test('a score just below the threshold remains pending even if its display rounds to two', async () => { +test('a score just below the threshold requires action even if its display rounds to two', async () => { const result = await evaluateApproval({ event: pullRequestEvent(), policySource, getMergedCount: async () => 50, getOwnership: async () => ({ totalLines: 1000000, reviewerLines: { writer: 112499 } }), api: async path => path.includes('/reviews?') ? [review('writer', 'APPROVED')] : { permission: 'write' }, }) - assert.equal(result.state, 'pending') + assert.equal(result.state, 'action_required') assert.ok(result.points < 2) }) @@ -587,14 +620,14 @@ test('the publisher counts merged history through the production API path', asyn } if (path.includes('/reviews?')) return [review('writer', 'APPROVED')] if (path.includes('/permission')) return { permission: 'write' } - states.push(options.body.state) + states.push(publishedState(options.body)) return {} }, }) assert.equal(result.points, 2) assert.equal(result.authorCredit.points, 0.55) assert.equal(result.approvals[0].points, 1.45) - assert.deepEqual(states, ['pending', 'success']) + assert.deepEqual(states, ['in_progress', 'success']) assert.equal(output[0], 'Author credit: 0.55 (50 merged PRs).') }) @@ -612,7 +645,7 @@ test('sufficient reviewer points and drafts publish without querying author hist return {} }, }) - assert.equal(result.state, draft ? 'pending' : 'success') + assert.equal(result.state, draft ? 'action_required' : 'success') assert.equal(result.authorCredit, null) assert.equal(output[0], `Author credit: not evaluated (${draft ? 'draft' : 'reviewer points suffice'}).`) } @@ -678,7 +711,7 @@ test('delegates the sender fixed points once after the recipient approves, retai }) }) const points = recipientState === 'APPROVED' ? 3 : 0 assert.equal(result.points, points) - assert.equal(result.state, points === 3 && senderState !== 'CHANGES_REQUESTED' ? 'success' : 'pending') + assert.equal(result.state, points === 3 && senderState !== 'CHANGES_REQUESTED' ? 'success' : 'action_required') assert.deepEqual(result.blockers, [ ...(senderState === 'CHANGES_REQUESTED' ? ['turtle1999'] : []), ...(recipientState === 'CHANGES_REQUESTED' ? ['writer'] : []), @@ -786,7 +819,7 @@ test('a sender submitted review takes back delegation, including comment-only an }) }) assert.equal(result.delegations.length, state === 'PENDING' ? 1 : 0) assert.equal(result.points, state === 'APPROVED' || state === 'PENDING' ? 3 : 1) - assert.equal(result.state, state === 'APPROVED' || state === 'PENDING' ? 'success' : 'pending') + assert.equal(result.state, state === 'APPROVED' || state === 'PENDING' ? 'success' : 'action_required') if (state === 'APPROVED') assert.equal(result.approvals.find(({ login }) => login === 'Turtle1999').delegatedTo, undefined) } } @@ -829,7 +862,7 @@ test('an active delegate command requests review once and preserves other reques if (options?.method === 'POST') { requests.push(options.body); return {} } return { users: [{ login: 'another-reviewer' }, ...(alreadyRequested ? [{ login: 'WRITER' }] : [])], teams: [] } } - if (path.includes('/statuses/')) return {} + if (path.includes('/check-runs')) return {} return api(path, options) }, }) @@ -857,7 +890,7 @@ test('inactive commands and drafts do not request review', async () => { policySource, runUrl: 'https://github.example/run/1', getMergedCount: async () => 0, write: () => {}, api: async (path, options) => { assert.ok(!path.endsWith('/requested_reviewers')) - if (path.includes('/statuses/')) return {} + if (path.includes('/check-runs')) return {} return api(path, options) }, }) @@ -873,7 +906,7 @@ test('failed review requests preserve the computed score and report the retry', event: { ...pullRequestEvent(), issue: { number: 42, pull_request: {} }, action: 'created', comment: { id: 1 } }, policySource, runUrl: 'https://github.example/run/1', getMergedCount: async () => 0, getOwnership: async () => ({ totalLines: 0, reviewerLines: {} }), write: line => output.push(line), api: async (path, options) => { - if (path.includes('/statuses/')) { statuses.push(options.body.state); return {} } + if (path.includes('/check-runs')) { statuses.push(publishedState(options.body)); return {} } if (path.endsWith('/requested_reviewers')) { if (options?.method === 'POST') throw new Error('request rejected') return failedRead ? {} : { users: [] } @@ -883,7 +916,7 @@ test('failed review requests preserve the computed score and report the retry', }) assert.match(output.at(-1), failedRead ? /no users array/u : /request rejected/u) assert.match(output.at(-1), /score is unchanged.*will retry/u) - assert.deepEqual(statuses, ['pending', 'success']) + assert.deepEqual(statuses, ['in_progress', 'success']) } }) @@ -895,7 +928,7 @@ test('delegation dismisses only the sender old decisions and stays active across const states = [] const read = delegationApi({ comments: [comment('turtle1999', '/delegate @writer')], reviews }) const api = async (path, options) => { - if (path.includes('/statuses/')) { states.push(options.body.state); return {} } + if (path.includes('/check-runs')) { states.push(publishedState(options.body)); return {} } if (path.endsWith('/dismissals')) { calls.push(options) const id = Number(path.split('/').at(-2)) @@ -919,13 +952,13 @@ test('delegation dismisses only the sender old decisions and stays active across assert.deepEqual(result.blockers, ['other']) assert.equal(result.delegations.length, 1) assert.deepEqual(result.delegations[0].reviewIds, []) - assert.deepEqual(states, ['pending', 'pending']) + assert.deepEqual(states, ['in_progress', 'action_required']) reviews.find(review => review.id === 21).state = 'DISMISSED' for (const state of ['APPROVED', 'DISMISSED', 'COMMENTED', 'APPROVED']) { reviews.find(review => review.id === 20).state = state const next = await evaluateDelegation({ api }) assert.equal(next.delegations.length, 1) - assert.equal(next.state, state === 'APPROVED' ? 'success' : 'pending') + assert.equal(next.state, state === 'APPROVED' ? 'success' : 'action_required') } reviews.push(review('turtle1999', 'COMMENTED', '2026-09-16T00:00:00Z', 30)) const reclaimed = await evaluateDelegation({ api }) @@ -939,7 +972,7 @@ test('a new sender review arriving during dismissal is not dismissed or delegate const result = await runWithHistory({ event: { ...pullRequestEvent(), issue: { number: 42, pull_request: {} }, action: 'created', comment: { id: 1 } }, policySource, runUrl: 'https://github.example/run/1', write: () => {}, getMergedCount: async () => 0, api: async (path, options) => { - if (path.includes('/statuses/')) return {} + if (path.includes('/check-runs')) return {} if (path.endsWith('/dismissals')) { assert.ok(path.endsWith('/reviews/10/dismissals')) reviews[0].state = 'DISMISSED' @@ -961,7 +994,7 @@ test('a failed or unconfirmed dismissal prevents review requests and publishes e await assert.rejects(runWithHistory({ event: { ...pullRequestEvent(), issue: { number: 42, pull_request: {} }, action: 'created', comment: { id: 1 } }, policySource, runUrl: 'https://github.example/run/1', write: () => {}, api: async (path, options) => { - if (path.includes('/statuses/')) { states.push(options.body.state); return {} } + if (path.includes('/check-runs')) { states.push(publishedState(options.body)); return {} } if (path.endsWith('/dismissals')) { if (rejected) throw new Error('dismissal denied') return { id: 10, state: 'CHANGES_REQUESTED' } @@ -970,7 +1003,7 @@ test('a failed or unconfirmed dismissal prevents review requests and publishes e return read(path, options) }, }), rejected ? /dismissal denied/u : /dismissal was not confirmed/u) - assert.deepEqual(states, ['pending', 'error']) + assert.deepEqual(states, ['in_progress', 'failure']) } }) @@ -983,7 +1016,7 @@ test('reads all comment pages, logs the score owner, and fails closed on missing event: pullRequestEvent(), policySource, runUrl: 'https://github.example/run/1', getMergedCount: async () => 0, write: line => output.push(line), api: async (path, options) => { - if (path.includes('/statuses/')) { statuses.push(options.body.state); return {} } + if (path.includes('/check-runs')) { statuses.push(publishedState(options.body)); return {} } if (path.includes('/comments?')) return comments(path) return api(path, options) }, @@ -996,7 +1029,7 @@ test('reads all comment pages, logs the score owner, and fails closed on missing }) assert.equal(pages.length, 2) assert.ok(output.includes('- @turtle1999: 2 (delegated to @writer)')) - assert.deepEqual(statuses.splice(0), ['pending', 'success']) + assert.deepEqual(statuses.splice(0), ['in_progress', 'success']) for (const [comments, message] of [ [() => { throw new Error('comment API unavailable') }, /comment API unavailable/u], [() => ({}), /comments response is not an array/u], @@ -1006,7 +1039,7 @@ test('reads all comment pages, logs the score owner, and fails closed on missing [() => Array.from({ length: 100 }, () => comment('writer', 'text')), /comments exceed 3000/u], ]) { await assert.rejects(run(comments), message) - assert.deepEqual(statuses.splice(0), ['pending', 'error']) + assert.deepEqual(statuses.splice(0), ['in_progress', 'failure']) } }) @@ -1090,7 +1123,7 @@ test('later events reconcile all active delegations after a draft or replaced co const effects = [] const requested = [] const api = async (path, options) => { - if (path.includes('/statuses/')) return {} + if (path.includes('/check-runs')) return {} if (path.endsWith('/dismissals')) { const old = reviews.find(review => path.endsWith(`/reviews/${review.id}/dismissals`)) effects.push(old.id) @@ -1122,7 +1155,7 @@ test('already approved delegates need no new request', async () => { }) const result = await runWithHistory({ event: pullRequestEvent(), policySource, runUrl: 'https://github.example/run/1', write: () => {}, api: (path, options) => { - if (path.includes('/statuses/')) return {} + if (path.includes('/check-runs')) return {} assert.ok(!path.endsWith('/requested_reviewers')) return read(path, options) }, diff --git a/.github/workflows/weighted-approval.yml b/.github/workflows/weighted-approval.yml index eb6f2fe3ba..896fc262c4 100644 --- a/.github/workflows/weighted-approval.yml +++ b/.github/workflows/weighted-approval.yml @@ -10,9 +10,9 @@ on: types: [completed] permissions: + checks: write contents: read pull-requests: write - statuses: write concurrency: group: weighted-approval-${{ (github.event.pull_request.number || github.event.issue.number) && format('weighted-approval-review-event:{0}', github.event.pull_request.number || github.event.issue.number) || github.event.workflow_run.display_title }} @@ -29,20 +29,20 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - # SECURITY: the status-writing job executes policy from the trusted default + # SECURITY: the check-writing job executes policy from the trusted default # branch and reads pull-request reviews and comments only as API data. - name: Check out trusted approval policy uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 with: ref: ${{ github.event.repository.default_branch }} persist-credentials: false - - name: Revoke previous approval status + - name: Revoke previous approval decision id: revoke env: GITHUB_TOKEN: ${{ github.token }} GITHUB_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - run: node .github/review-ownership/check-approval.mjs pending - # SECURITY: dependency setup runs in the status-writing job. + run: node .github/review-ownership/check-approval.mjs in_progress + # SECURITY: dependency setup runs in the check-writing job. - if: steps.revoke.outputs.active == 'true' uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 with: @@ -52,7 +52,7 @@ jobs: - name: Install production lexer if: steps.revoke.outputs.active == 'true' run: python3 -m pip install -r .github/review-ownership/requirements.txt - - name: Publish weighted approval status + - name: Publish weighted approval decision if: steps.revoke.outputs.active == 'true' env: GITHUB_TOKEN: ${{ github.token }} @@ -63,4 +63,4 @@ jobs: env: GITHUB_TOKEN: ${{ github.token }} GITHUB_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - run: node .github/review-ownership/check-approval.mjs error + run: node .github/review-ownership/check-approval.mjs failure diff --git a/scripts/ci-workflow.spec.ts b/scripts/ci-workflow.spec.ts index 86179be885..1275d4a7b0 100644 --- a/scripts/ci-workflow.spec.ts +++ b/scripts/ci-workflow.spec.ts @@ -972,7 +972,7 @@ describe('Weighted approval workflow', () => { if (!Array.isArray(recordJob.steps)) throw new TypeError('weighted-approval review event job must define steps') const steps = job.steps.filter(isRecord) const checkout = steps.find(step => step.name === 'Check out trusted approval policy') - const publish = steps.find(step => step.name === 'Publish weighted approval status') + const publish = steps.find(step => step.name === 'Publish weighted approval decision') const recordSteps = recordJob.steps.filter(isRecord) const record = recordSteps.find(step => step.name === 'Record review event') @@ -987,9 +987,9 @@ describe('Weighted approval workflow', () => { expect(review.types).toEqual(['submitted', 'edited', 'dismissed']) expect(reviewEvent.permissions).toEqual({}) expect(publisher.permissions).toEqual({ + checks: 'write', contents: 'read', 'pull-requests': 'write', - statuses: 'write', }) expect(publisher.concurrency).toEqual({ group: "weighted-approval-${{ (github.event.pull_request.number || github.event.issue.number) && format('weighted-approval-review-event:{0}', github.event.pull_request.number || github.event.issue.number) || github.event.workflow_run.display_title }}", @@ -1017,10 +1017,10 @@ describe('Weighted approval workflow', () => { const revokeIndex = steps.findIndex(step => step.id === 'revoke') expect(revokeIndex).toBeGreaterThan(steps.indexOf(checkout!)) expect(revokeIndex).toBeLessThan(setupIndex) - expect(steps[revokeIndex]?.run).toBe('node .github/review-ownership/check-approval.mjs pending') + expect(steps[revokeIndex]?.run).toBe('node .github/review-ownership/check-approval.mjs in_progress') expect(steps.at(-1)).toMatchObject({ if: "failure() && steps.revoke.outputs.active == 'true'", - run: 'node .github/review-ownership/check-approval.mjs error', + run: 'node .github/review-ownership/check-approval.mjs failure', }) const pythonJob = workflowJob(loadWorkflow('.github/workflows/ci.yml'), 'python-sdk') expect(pythonJob.steps).toContainEqual({ From a0ed3981fe287fbe17fbf0cac10e6547595773d6 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Wed, 30 Sep 2026 13:42:02 +0800 Subject: [PATCH 002/201] fix(ci): revoke the approval decision through its conclusion A check run that reached completed never returns to in_progress: GitHub answers 200 and leaves the run completed, so the revoke write was a silent no-op on every re-evaluation and a published success survived an interrupted setup or history fetch. Revoke a published success by concluding it action_required, start a head that has no decision in progress, and drop the status-only check-run update. The setup-failure step ran only when the revoke step succeeded, so a failed or cancelled publisher left the earlier conclusion standing. It now runs on failure and on a cancellation that precedes the decision. The decision note, the review-ownership README, and the two related notes record the measured transition matrix and the fallback condition. --- ...pproval-review-workflow-identity.i18n.yaml | 4 +- ...09-10-approval-review-workflow-identity.md | 2 +- ...10-approval-review-workflow-identity.zh.md | 2 +- ...026-09-15-pr-approval-delegation.i18n.yaml | 4 +- .../2026-09-15-pr-approval-delegation.md | 2 +- .../2026-09-15-pr-approval-delegation.zh.md | 2 +- ...hted-approval-decision-check-run.i18n.yaml | 16 +-- ...29-weighted-approval-decision-check-run.md | 17 +-- ...weighted-approval-decision-check-run.zh.md | 17 +-- .agents/skills/dsh-pre-push-checks/SKILL.md | 2 +- .github/review-ownership/README.md | 2 +- .github/review-ownership/check-approval.mjs | 71 ++++++++---- .../review-ownership/check-approval.test.mjs | 102 +++++++++++------- .github/workflows/weighted-approval.yml | 7 +- scripts/ci-workflow.spec.ts | 5 +- 15 files changed, 162 insertions(+), 93 deletions(-) diff --git a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml index 1dfe18dbe3..335e5a5f86 100644 --- a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.i18n.yaml @@ -6,8 +6,8 @@ en: 8a332b9d080f3d62 zh: 29f1de05d7867a77 /agent-note-identify-approval-review-workflows-by-file-path/problem: - en: 6b927a7545ac2c48 - zh: 0902f03aa478d5ee + en: 63913473297103c4 + zh: 42d7312b9bc439e7 /agent-note-identify-approval-review-workflows-by-file-path/decision: en: 82802438f1b8d16c zh: ffe8c1fcbcc39abe diff --git a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md index de1e08ef78..f6a9cc7c17 100644 --- a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md +++ b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.md @@ -6,7 +6,7 @@ English | [中文](2026-09-10-approval-review-workflow-identity.zh.md) ## Problem -GitHub can populate a workflow run's `name` with its expanded `run-name`. The approval review workflow includes the pull-request number in that title, so comparing `workflow_run.name` with the static workflow name rejects valid review events before refreshing the approval status. +GitHub can populate a workflow run's `name` with its expanded `run-name`. The approval review workflow includes the pull-request number in that title, so comparing `workflow_run.name` with the static workflow name rejects valid review events before refreshing the approval decision. ## Decision diff --git a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md index 6b3aa3d359..2ed453467f 100644 --- a/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md +++ b/.agents/notes/implemented/process/2026-09-10-approval-review-workflow-identity.zh.md @@ -6,7 +6,7 @@ Status: implemented ## 问题 -GitHub 可能用展开后的 `run-name` 填充工作流运行的 `name`。审批评审工作流在该标题中包含拉取请求编号，因此将 `workflow_run.name` 与静态工作流名称比较，会在刷新审批状态前拒绝有效的评审事件。 +GitHub 可能用展开后的 `run-name` 填充工作流运行的 `name`。审批评审工作流在该标题中包含拉取请求编号，因此将 `workflow_run.name` 与静态工作流名称比较，会在刷新审批决策前拒绝有效的评审事件。 ## 决策 diff --git a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml index 50bca79957..f01abc3399 100644 --- a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.i18n.yaml @@ -9,8 +9,8 @@ en: 5fe2f3b3cb7ce046 zh: ed27d4efc995b8b5 /agent-note-pr-scoped-approval-delegation/decision: - en: bf8f1ff30114310f - zh: a163e1276dd384f8 + en: 87114939824c3994 + zh: 34d71c57f0459017 /agent-note-pr-scoped-approval-delegation/alternatives-considered: en: a6006217cb29613a zh: 83968328096f1d91 diff --git a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md index 0282f9c058..cbc7d7fbec 100644 --- a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md +++ b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.md @@ -12,7 +12,7 @@ A reviewer may trust another reviewer to decide a particular PR while retaining The [approval policy](../../../../.github/review-ownership/README.md#delegating-points) accepts `/delegate @username` in PR conversation comments. Each eligible sender's points follow the named recipient's effective approval, once, using the sender's weight. Delegation applies only to that PR and only between write-capable accounts other than the PR author. Received points cannot be forwarded. The command dismisses the sender's prior approvals and change requests through GitHub. Other effective blocking reviews still follow [blocking-review policy](2026-09-29-weighted-approval-decision-check-run.md). -The latest surviving eligible command in comment creation order determines the recipient. Edited commands require the latest editor to match the author’s immutable account ID; other writers cannot transfer or cancel that author’s points by editing a comment. A self-delegation or a subsequently submitted review restores the sender's own decision. Comment-only reviews also reclaim the points; pending reviews do not. Equal timestamps favor the review, and subsequent dismissal cannot revive the delegation. A fresh command after the review can delegate again. Edits and deletions recompute from current comments, including restoration of older commands. The trusted publisher filters conversation comment changes for `/delegate`, resolves the live open PR head, and reads comments and editor identity as API data. Every evaluation reconciles active eligible commands, dismissing prior decision reviews and requesting recipients unless already approved or requested. This covers replaced queued events and draft-to-ready transitions; other requested reviewers remain unchanged. Scores refresh after dismissal, which retains the old submission timestamps and cannot cancel delegation. Removing the recipient's approval removes counted points while retaining delegation for their next approval. Drafts do not dismiss or request reviews. Dismissal failures fail evaluation; request failures are logged separately and retried on later evaluations. Closed or merged PRs are skipped before status writes and dependency setup. It preserves the separate [review-workflow validation](2026-09-10-approval-review-workflow-identity.md). +The latest surviving eligible command in comment creation order determines the recipient. Edited commands require the latest editor to match the author’s immutable account ID; other writers cannot transfer or cancel that author’s points by editing a comment. A self-delegation or a subsequently submitted review restores the sender's own decision. Comment-only reviews also reclaim the points; pending reviews do not. Equal timestamps favor the review, and subsequent dismissal cannot revive the delegation. A fresh command after the review can delegate again. Edits and deletions recompute from current comments, including restoration of older commands. The trusted publisher filters conversation comment changes for `/delegate`, resolves the live open PR head, and reads comments and editor identity as API data. Every evaluation reconciles active eligible commands, dismissing prior decision reviews and requesting recipients unless already approved or requested. This covers replaced queued events and draft-to-ready transitions; other requested reviewers remain unchanged. Scores refresh after dismissal, which retains the old submission timestamps and cannot cancel delegation. Removing the recipient's approval removes counted points while retaining delegation for their next approval. Drafts do not dismiss or request reviews. Dismissal failures fail evaluation; request failures are logged separately and retried on later evaluations. Closed or merged PRs are skipped before check-run writes and dependency setup. It preserves the separate [review-workflow validation](2026-09-10-approval-review-workflow-identity.md). ## Alternatives considered diff --git a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md index 7d26a42d0d..348c62055f 100644 --- a/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md +++ b/.agents/notes/implemented/process/2026-09-15-pr-approval-delegation.zh.md @@ -12,7 +12,7 @@ Status: implemented [批准策略](../../../../.github/review-ownership/README.md#delegating-points) 接受 PR 普通评论中的 `/delegate @username`。每位符合资格的委托人，其分数跟随指定受托人的有效批准，按委托人的权重计入一次。委托仅适用于当前 PR，且双方都必须具有写权限，不能是 PR 作者。收到的分数不能继续转交。命令会通过 GitHub 撤销委托人原有的批准和变更请求。其他有效阻塞评审仍遵循[阻塞评审策略](2026-09-29-weighted-approval-decision-check-run.zh.md)。 -按评论创建顺序，最新、仍存在且符合资格的命令决定受托人。编辑过的命令要求最近编辑者与作者的不可变账户 ID 一致；其他有写权限的人不能通过编辑评论转交或取消作者的分数。委托给自己或随后提交评审会恢复使用委托人本人的评审决定。仅评论的评审也会收回分数；尚未提交的评审不会。时间戳相同时以评审为准，随后撤销评审也不会重新启用委托。评审之后的新命令可以再次委托。编辑和删除会根据当前评论重算，包括恢复较早的命令。可信发布器按 `/delegate` 筛选普通评论变更，解析实时且未关闭的 PR 头提交，并将评论及编辑者身份作为 API 数据读取。每次评估都会处理所有有效且符合资格的命令，撤销旧的决定性评审，并请求尚未批准、尚未收到请求的受托人进行评审。这覆盖了排队事件被替换以及草稿转为 ready 的情况；其他 requested reviewers 保持不变。撤销后刷新分数；撤销保留旧提交时间戳，不会取消委托。移除受托人的批准只会移除计入的分数，委托仍然保留，供其下次批准使用。草稿 PR 不撤销或请求评审。撤销失败会使评估失败；请求失败单独记录日志，并在后续评估中重试。已关闭或合并的 PR 会在写入状态和安装依赖之前跳过。独立的[评审工作流验证](2026-09-10-approval-review-workflow-identity.zh.md) 保持不变。 +按评论创建顺序，最新、仍存在且符合资格的命令决定受托人。编辑过的命令要求最近编辑者与作者的不可变账户 ID 一致；其他有写权限的人不能通过编辑评论转交或取消作者的分数。委托给自己或随后提交评审会恢复使用委托人本人的评审决定。仅评论的评审也会收回分数；尚未提交的评审不会。时间戳相同时以评审为准，随后撤销评审也不会重新启用委托。评审之后的新命令可以再次委托。编辑和删除会根据当前评论重算，包括恢复较早的命令。可信发布器按 `/delegate` 筛选普通评论变更，解析实时且未关闭的 PR 头提交，并将评论及编辑者身份作为 API 数据读取。每次评估都会处理所有有效且符合资格的命令，撤销旧的决定性评审，并请求尚未批准、尚未收到请求的受托人进行评审。这覆盖了排队事件被替换以及草稿转为 ready 的情况；其他 requested reviewers 保持不变。撤销后刷新分数；撤销保留旧提交时间戳，不会取消委托。移除受托人的批准只会移除计入的分数，委托仍然保留，供其下次批准使用。草稿 PR 不撤销或请求评审。撤销失败会使评估失败；请求失败单独记录日志，并在后续评估中重试。已关闭或合并的 PR 会在写入 check run 和安装依赖之前跳过。独立的[评审工作流验证](2026-09-10-approval-review-workflow-identity.zh.md) 保持不变。 ## Alternatives considered diff --git a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml index e3d242d672..89856bb840 100644 --- a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml +++ b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.i18n.yaml @@ -9,14 +9,14 @@ en: 6ba6f6a15a1562ec zh: 1cbef3e736c258e6 /agent-note-weighted-approval-decisions-are-completed-check-runs/decision: - en: 933e4c1438771907 - zh: f6056382f3b555c9 + en: b070a25a6d990e8c + zh: fa555f8e418dad1f /agent-note-weighted-approval-decisions-are-completed-check-runs/verification: - en: d1f65b5b577b08b8 - zh: 7a193cec1acec003 + en: 5e5de198a6fbdcca + zh: 74c6dccdea7984b8 /agent-note-weighted-approval-decisions-are-completed-check-runs/alternatives-considered: - en: a7ef91c0506766a8 - zh: d6584e31652b14ca + en: b8daea66b0a90d26 + zh: fb7079c815b38281 /agent-note-weighted-approval-decisions-are-completed-check-runs/consequences: - en: 4cafdf12b326c68a - zh: c37a57d26d2f0894 + en: 772e9b9c0133d264 + zh: 0c36b0819adaa622 diff --git a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md index f4d8f9e639..2c4550ba4a 100644 --- a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md +++ b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.md @@ -16,18 +16,21 @@ The [publisher](../../../../.github/review-ownership/check-approval.mjs) reports | Condition | Check run | |---|---| -| Dependency setup or evaluation in progress | `in_progress` | +| First evaluation of a head | `in_progress` until the decision is written | +| Re-evaluating a head that published `success` | conclusion `action_required` until the decision is written | | Draft, fewer than the required points, or an effective write-capable `CHANGES_REQUESTED` review | `completed`, conclusion `action_required` | | Ready, threshold met, and no blocking review | `completed`, conclusion `success` | -| Setup or evaluation failure | `completed`, conclusion `failure` | +| Setup, evaluation, or cancellation failure | `completed`, conclusion `failure` | + +A check run that reached `completed` never returns to `in_progress` or `queued`: GitHub answers `200` to that update and leaves the run completed, so only the conclusion is mutable. The publisher therefore revokes a published success by writing the blocking conclusion before dependency setup, which covers the setup and history fetch that dominate a cold evaluation. The head keeps one GitHub Actions run with the required name: the publisher updates the head's existing run and creates one only when none exists. An incomplete list or a run without a valid ID fails the publication. A run cancelled after the decision was published keeps that decision; every other failed or cancelled run concludes `failure`. A blocking review dominates the point total, so the conclusion remains `action_required` even when counted approvals reach the threshold. GitHub accepts only `success`, `neutral`, and `skipped` for a required check, so `action_required` blocks merging while completing the check. -The publisher lists the head's check runs with that name, updates every run owned by GitHub Actions, and creates one only when none exists. An incomplete list or a run without a valid ID fails the publication. - ## Verification -[Approval policy tests](../../../../.github/review-ownership/check-approval.test.mjs) pin the threshold-reaching blocker case, the exact published check-run payloads, updating an existing run instead of creating another, and invalid list responses. [Workflow tests](../../../../scripts/ci-workflow.spec.ts) pin the `checks: write` permission, the `in_progress` and `failure` phase commands, and the separate publisher job name. +[Approval policy tests](../../../../.github/review-ownership/check-approval.test.mjs) pin the threshold-reaching blocker case, the exact published check-run payloads, revoking a published success through its conclusion, updating an existing run instead of creating another, and invalid list responses. [Workflow tests](../../../../scripts/ci-workflow.spec.ts) pin the `checks: write` permission, the `revoke` and `failure` phase commands, the publish step identity, and the fallback that runs when a run fails or is cancelled before publishing. + +The transition matrix and the fallback condition were measured on 2026-09-30 with throwaway tag runs in this repository, using the same GitHub Actions token as the publisher. A run created as `in_progress` and completed as `success` accepted `status: in_progress` and `status: queued` with `200` and stayed `completed`, while conclusion-only updates moved it to `action_required`, back to `success`, and to `failure`. The fallback step ran for a failed publish and for a cancellation during the publish step, and did not run when the job was cancelled after the decision was published. ## Alternatives considered @@ -35,6 +38,8 @@ The publisher lists the head's check runs with that name, updates every run owne **Publish `failure` for unmet conditions.** This ends the wait, but it represents an unmet, reversible merge condition as a malfunction and conflates the policy outcome with publisher health. +**Revoke by returning the completed run to `in_progress`.** GitHub accepts that update without applying it, so the published success would stand through dependency setup and the history fetch. + **Publish nothing until success.** A required check that was never reported remains expected and is invisible to many watchers. Revoking a success already published on the same head still requires a non-success write, and each watcher's treatment of a missing required check is unspecified. **Keep the `weighted approval` name.** When a check and a commit status share a required name, GitHub requires both to pass. Heads that carry a `pending` commit status from the earlier publisher would stay blocked until a new push. @@ -47,4 +52,4 @@ The publisher lists the head's check runs with that name, updates every run owne CI watchers finish while the decision waits on reviewers. `gh pr checks` groups `action_required` with failed checks and exits non-zero; consumers distinguish it by the check name, the `ACTION_REQUIRED` conclusion, or the output title, which carries the score or blocker. The `weighted approval publisher` Actions job and the `failure` conclusion carry operational failures. -Revoking an earlier success on the same head returns the completed check run to `in_progress`, a transition the GitHub REST reference does not restrict. +Re-evaluating a head that already published `success` reports `action_required` until the new decision is written, so a watcher that polls inside that window sees a failed check. Heads created before this change keep their `weighted approval` commit status: no API completes or removes a commit status, so those heads stay pending for watchers until they receive a new commit. diff --git a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md index 4dee85c8f7..d8b894351d 100644 --- a/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md +++ b/.agents/notes/implemented/process/2026-09-29-weighted-approval-decision-check-run.zh.md @@ -16,18 +16,21 @@ commit status 只提供 `pending`、`success`、`failure` 和 `error`。`pending | 条件 | Check run | |---|---| -| 依赖安装或评估进行中 | `in_progress` | +| head 的首次评估 | 写入决策前为 `in_progress` | +| 重新评估已发布 `success` 的 head | 写入决策前 conclusion 为 `action_required` | | draft、点数少于要求，或存在具有写权限的评审人仍然生效的 `CHANGES_REQUESTED` 评审 | `completed`，conclusion 为 `action_required` | | 已 ready、达到阈值且没有阻塞性评审 | `completed`，conclusion 为 `success` | -| 安装或评估失败 | `completed`，conclusion 为 `failure` | +| 安装、评估或取消导致的失败 | `completed`，conclusion 为 `failure` | + +已经到达 `completed` 的 check run 不会再回到 `in_progress` 或 `queued`：GitHub 对该更新返回 `200`，但让 run 保持已完成，因此只有 conclusion 可变。publisher 因此在依赖安装前用阻塞性 conclusion 撤销已发布的成功，覆盖冷启动中占主要耗时的安装与历史拉取。同一 head 上只保留一个同名 GitHub Actions run：publisher 更新 head 上已有的 run，仅在不存在时创建一个。列表不完整或 run 没有有效 ID 时发布失败。决策已发布后被取消的 run 保留该决策；其余失败或被取消的 run 都以 `failure` 结束。 阻塞性评审的优先级高于点数总和，因此即使计入的批准已经达到阈值，conclusion 仍为 `action_required`。GitHub 只接受 `success`、`neutral` 和 `skipped` 作为必需检查的通过结果，因此 `action_required` 会阻止合并，同时让该检查处于完成状态。 -publisher 列出 head 上同名的 check run，更新所有属于 GitHub Actions 的 run，只有在不存在时才创建一个。列表不完整或 run 没有有效 ID 时发布失败。 - ## 验证 -[批准策略测试](../../../../.github/review-ownership/check-approval.test.mjs)锁定已达到阈值但仍有 blocker 的场景、准确发布的 check run payload、更新已有 run 而不是另建一个，以及无效的列表响应。[工作流测试](../../../../scripts/ci-workflow.spec.ts)锁定 `checks: write` 权限、`in_progress` 和 `failure` 阶段命令，以及独立的 publisher job 名称。 +[批准策略测试](../../../../.github/review-ownership/check-approval.test.mjs)锁定已达到阈值但仍有 blocker 的场景、准确发布的 check run payload、通过 conclusion 撤销已发布的成功、更新已有 run 而不是另建一个，以及无效的列表响应。[工作流测试](../../../../scripts/ci-workflow.spec.ts)锁定 `checks: write` 权限、`revoke` 和 `failure` 阶段命令、publish 步骤的身份，以及运行失败或在发布前被取消时执行的兜底步骤。 + +转换矩阵与该兜底条件于 2026-09-30 在本仓库用一次性 tag 运行实测，使用的是与 publisher 相同的 GitHub Actions token。以 `in_progress` 创建、以 `success` 完成的 run，接受 `status: in_progress` 和 `status: queued` 的更新时都返回 `200` 并保持 `completed`；只改 conclusion 的更新把它依次改为 `action_required`、`success` 和 `failure`。兜底步骤在发布失败以及在发布步骤执行期间被取消时运行，而在决策发布之后才取消 job 时不运行。 ## 考虑过的替代方案 @@ -35,6 +38,8 @@ publisher 列出 head 上同名的 check run，更新所有属于 GitHub Actions **为尚未满足的条件发布 `failure`。** 该方案能结束等待，但会把尚未满足且可撤销的合并条件表示为故障，并混淆策略结果与 publisher 的健康状态。 +**通过把已完成的 run 改回 `in_progress` 来撤销。** GitHub 接受该更新但不执行，因此已发布的成功会在依赖安装与历史拉取期间继续存在。 + **在成功之前不发布任何结果。** 从未上报的必需检查保持 expected 状态，许多监视工具看不到它。撤销同一 head 上已发布的成功仍然需要写入非成功结果，而各监视工具如何对待缺失的必需检查没有规定。 **保留 `weighted approval` 名称。** 当 check 和 commit status 使用同一个必需名称时，GitHub 要求两者都通过。带有旧 publisher 所写 `pending` commit status 的 head 会一直被阻止，直到出现新的 push。 @@ -47,4 +52,4 @@ publisher 列出 head 上同名的 check run，更新所有属于 GitHub Actions 决策等待评审人时，CI 监视工具会结束。`gh pr checks` 把 `action_required` 与失败的检查归为一组并以非零状态退出；消费方可以通过 check 名称、`ACTION_REQUIRED` conclusion 或携带分数或 blocker 的输出标题加以区分。`weighted approval publisher` Actions job 和 `failure` conclusion 承载运行故障信号。 -撤销同一 head 上此前的成功结果时，已完成的 check run 会回到 `in_progress`；GitHub REST 参考文档没有限制这一转换。 +重新评估已经发布 `success` 的 head 时，在新决策写入前会报告 `action_required`，因此在该窗口内轮询的监视工具会看到失败的检查。本次变更之前创建的 head 会保留原有的 `weighted approval` commit status：没有任何 API 能完成或删除 commit status，因此这些 head 对监视工具一直是 pending，直到它们收到新的 commit。 diff --git a/.agents/skills/dsh-pre-push-checks/SKILL.md b/.agents/skills/dsh-pre-push-checks/SKILL.md index 6567dde5dc..3ab326a4b3 100644 --- a/.agents/skills/dsh-pre-push-checks/SKILL.md +++ b/.agents/skills/dsh-pre-push-checks/SKILL.md @@ -123,7 +123,7 @@ For GitHub PRs, inspect remote CI after the push: gh pr checks ``` -Report pending checks as pending. Inspect failures before attributing them to the branch or the environment. +Report pending checks as pending. Inspect failures before attributing them to the branch or the environment. A `weighted approval decision` check that `gh pr checks` lists as failed means the approval gate is waiting on reviewers (`ACTION_REQUIRED`), not that the branch broke. When `gh pr checks` reports "no checks reported" and `/actions/runs?head_sha=` returns `total_count: 0`, read mergeability before suspecting the push or a dropped GitHub event: diff --git a/.github/review-ownership/README.md b/.github/review-ownership/README.md index 7b461ae864..0d99299847 100644 --- a/.github/review-ownership/README.md +++ b/.github/review-ownership/README.md @@ -16,7 +16,7 @@ The [`weighted-approval` workflow](../workflows/weighted-approval.yml) publishes ## Approval scoring -The weighted approval workflow exposes two pull-request checks. The `weighted approval publisher` Actions job reports whether evaluation and check publication completed, while the `weighted approval decision` check run carries the approval decision on the pull request head. Branch rules must require that check run with GitHub Actions as its expected source; a name-only requirement can accept a same-named check or status from another integration. The publisher updates the head's existing GitHub Actions decision check run and creates one only when none exists. It marks the check run `in_progress` before Python setup or lexer installation, so failed setup or an interrupted history fetch cannot leave a previous success in place. Setup and evaluation failures conclude it as `failure`. A completed evaluation concludes `action_required` below two approval points, while the pull request is a draft, or while a write-capable reviewer has an effective `CHANGES_REQUESTED` review; the blocker keeps that conclusion even when counted approvals reach the threshold. It concludes `success` only when the threshold is met, the pull request is ready, and no such blocker exists. `action_required` blocks merging but completes the check, so tools that wait for checks to finish do not wait for reviewers; the check-run title carries the score or blocker. +The weighted approval workflow exposes two pull-request checks. The `weighted approval publisher` Actions job reports whether evaluation and check publication completed, while the `weighted approval decision` check run carries the approval decision on the pull request head. Branch rules must require that check run with GitHub Actions as its expected source; a name-only requirement can accept a same-named check or status from another integration. The publisher updates the head's existing GitHub Actions decision check run and creates one only when none exists; a head without one starts `in_progress` and a re-evaluated head that published `success` is concluded `action_required` before Python setup or lexer installation, so failed setup or an interrupted history fetch cannot leave a previous success in place. A completed check run only ever changes conclusion, so every later write completes it again. Setup, evaluation, and cancellation failures conclude it as `failure`. A completed evaluation concludes `action_required` below two approval points, while the pull request is a draft, or while a write-capable reviewer has an effective `CHANGES_REQUESTED` review; the blocker keeps that conclusion even when counted approvals reach the threshold. It concludes `success` only when the threshold is met, the pull request is ready, and no such blocker exists. `action_required` blocks merging but completes the check, so tools that wait for checks to finish do not wait for reviewers; the check-run title carries the score or blocker. Reviewers whose calculated base repository permission is `write` or `admin` count. The [approval policy](approval-policy.json) gives `@07akioni`, `@imccyu`, `@tianyicui`, `@tianyicui-bot`, `@turtle1999`, and `@turtle2099` two points each; every other write-capable reviewer gets one point. The pull-request author’s own review and reviewers without write permission do not count. diff --git a/.github/review-ownership/check-approval.mjs b/.github/review-ownership/check-approval.mjs index 489acd1779..83b6eb68a2 100644 --- a/.github/review-ownership/check-approval.mjs +++ b/.github/review-ownership/check-approval.mjs @@ -311,7 +311,6 @@ export async function evaluateApproval({ event, policySource, api, getOwnership */ export async function runApprovalCheck({ event, policySource, api, runUrl, getOwnership = productionOwnership, getMergedCount = countMergedAuthorPulls, write = line => process.stdout.write(`${line}\n`) }) { const pull = pullRequestFromEvent(event) - await publishCheck(api, pull, 'in_progress', 'Evaluating approval points.', runUrl) let result try { result = await evaluateApproval({ event, policySource, api, getOwnership, getMergedCount }) @@ -353,14 +352,15 @@ export async function runApprovalCheck({ event, policySource, api, runUrl, getOw } /** - * Revoke a previous conclusion before dependency installation, or report its failure. + * Revoke a published success before dependency installation, or conclude a failed setup. * @param {{event: unknown, api: (path: string, options?: {method?: string, body?: unknown}) => Promise, runUrl: string, phase: string}} options Publication inputs. * @returns {Promise} Completion of the check-run write. */ export async function publishApprovalPhase({ event, api, runUrl, phase }) { - if (!['in_progress', 'failure'].includes(phase)) throw new Error('invalid approval setup phase') - await publishCheck(api, pullRequestFromEvent(event), phase, - phase === 'in_progress' ? 'Preparing approval evaluation.' : 'Approval setup or evaluation failed.', runUrl) + const pull = pullRequestFromEvent(event) + if (phase === 'revoke') return revokeApproval(api, pull, runUrl) + if (phase === 'failure') return publishCheck(api, pull, 'failure', 'Approval setup or evaluation failed.', runUrl) + throw new Error('invalid approval setup phase') } /** @@ -467,21 +467,15 @@ async function reviewerPermission(api, repository, login) { return response.permission.toLowerCase() } -// Updating the head's existing GitHub Actions check run keeps one decision per head for branch rules and CI watchers. -async function publishCheck(api, pull, state, title, runUrl) { - if (!/^https:\/\/[^\s]+$/u.test(runUrl)) throw new Error('workflow run URL must use HTTPS') - const body = { - ...(state === 'in_progress' ? { status: state } : { status: 'completed', conclusion: state }), - details_url: runUrl, - output: { title, summary: `${title} [Workflow run](${runUrl})` }, - } - const listed = await api(`/repos/${pull.repository}/commits/${pull.headSha}/check-runs?check_name=${encodeURIComponent(CHECK_NAME)}&filter=all&per_page=${PAGE_SIZE}`) - if (!isRecord(listed) || !Array.isArray(listed.check_runs) || listed.total_count !== listed.check_runs.length) { - throw new Error('check-run list response is invalid or incomplete') - } - const runs = listed.check_runs.filter(run => isRecord(run) && isRecord(run.app) && run.app.slug === 'github-actions') +// One decision per head keeps branch rules and CI watchers unambiguous; the head's existing GitHub +// Actions run is updated and a run is created only when none exists. +async function publishCheck(api, pull, conclusion, title, runUrl) { + requireHttpsRunUrl(runUrl) + const body = { status: 'completed', conclusion, details_url: runUrl, output: { title, summary: `${title} [Workflow run](${runUrl})` } } + const runs = await decisionRuns(api, pull) if (runs.length === 0) { await api(`/repos/${pull.repository}/check-runs`, { method: 'POST', body: { name: CHECK_NAME, head_sha: pull.headSha, ...body } }) + return } for (const { id } of runs) { if (!Number.isSafeInteger(id)) throw new Error('check run has no valid id') @@ -489,6 +483,45 @@ async function publishCheck(api, pull, state, title, runUrl) { } } +// A head without a decision starts in progress; a published success becomes action_required, so an +// interrupted evaluation cannot leave a success that its current inputs no longer support. +async function revokeApproval(api, pull, runUrl) { + requireHttpsRunUrl(runUrl) + const title = 'Preparing approval evaluation.' + const output = { title, summary: `${title} [Workflow run](${runUrl})` } + const runs = await decisionRuns(api, pull) + if (runs.length === 0) { + await api(`/repos/${pull.repository}/check-runs`, { + method: 'POST', + body: { name: CHECK_NAME, head_sha: pull.headSha, status: 'in_progress', details_url: runUrl, output }, + }) + return + } + for (const { id, conclusion } of runs) { + if (!Number.isSafeInteger(id)) throw new Error('check run has no valid id') + // Only a published success satisfies the branch rule; every other conclusion already blocks it. + if (conclusion !== 'success') continue + await api(`/repos/${pull.repository}/check-runs/${id}`, { + method: 'PATCH', + body: { status: 'completed', conclusion: 'action_required', details_url: runUrl, output }, + }) + } +} + +// A check run that reached completed never returns to in_progress: GitHub answers 200 and leaves the +// run completed, so a published decision can only be replaced by another conclusion. +async function decisionRuns(api, pull) { + const listed = await api(`/repos/${pull.repository}/commits/${pull.headSha}/check-runs?check_name=${encodeURIComponent(CHECK_NAME)}&filter=all&per_page=${PAGE_SIZE}`) + if (!isRecord(listed) || !Array.isArray(listed.check_runs) || listed.total_count !== listed.check_runs.length) { + throw new Error('check-run list response is invalid or incomplete') + } + return listed.check_runs.filter(run => isRecord(run) && isRecord(run.app) && run.app.slug === 'github-actions') +} + +function requireHttpsRunUrl(runUrl) { + if (!/^https:\/\/[^\s]+$/u.test(runUrl)) throw new Error('workflow run URL must use HTTPS') +} + function pullRequestFromEvent(event) { const repository = repositoryFromEvent(event) if (!isRecord(event.pull_request) || !isRecord(event.pull_request.user) @@ -579,7 +612,7 @@ async function main() { const phase = process.argv[2] if (phase) { await publishApprovalPhase({ event, api, runUrl: process.env.GITHUB_RUN_URL ?? '', phase }) - if (phase === 'in_progress' && process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, 'active=true\n') + if (phase === 'revoke' && process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, 'active=true\n') return } await runApprovalCheck({ diff --git a/.github/review-ownership/check-approval.test.mjs b/.github/review-ownership/check-approval.test.mjs index 186e97d1b3..fe7582108d 100644 --- a/.github/review-ownership/check-approval.test.mjs +++ b/.github/review-ownership/check-approval.test.mjs @@ -16,8 +16,10 @@ import { } from './check-approval.mjs' const policySource = readFileSync(new URL('approval-policy.json', import.meta.url), 'utf8') +const emptyCheckRuns = { total_count: 0, check_runs: [] } +const isCheckRunsList = path => path.includes('/commits/') && path.includes('/check-runs?') const withoutCheckRuns = api => (path, options) => - path.includes('/check-runs?') ? { total_count: 0, check_runs: [] } : api(path, options) + isCheckRunsList(path) ? emptyCheckRuns : api(path, options) const withEmptyReads = api => withoutCheckRuns((path, options) => path.includes('/comments?') ? [] : api(path, options)) const evaluateApproval = options => evaluateWithHistory({ getMergedCount: async () => 0, ...options, api: withEmptyReads(options.api) }) const runWithHistory = options => runPublisher({ ...options, api: withoutCheckRuns(options.api) }) @@ -237,7 +239,7 @@ test('ignores a reviewer whose collaborator permission lookup returns 404', asyn }) test('requires action on a write-capable change request while ignoring the author and read-only reviewers', async () => { - const statuses = [] + const writes = [] const result = await runApprovalCheck({ event: pullRequestEvent({ author: 'author' }), policySource, @@ -255,8 +257,9 @@ test('requires action on a write-capable change request while ignoring the autho if (path.includes('/collaborators/turtle1999/permission')) return { permission: 'admin' } if (path.includes('/collaborators/blocker/permission')) return { permission: 'write' } if (path.includes('/collaborators/reader/permission')) return { permission: 'read' } + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) { - statuses.push(options.body) + writes.push(options.body) return {} } throw new Error(`unexpected API path ${path}`) @@ -268,13 +271,7 @@ test('requires action on a write-capable change request while ignoring the autho assert.equal(result.points, 2) assert.deepEqual(result.blockers, ['blocker']) assert.deepEqual(result.ignoredReviewers, ['reader']) - assert.deepEqual(statuses, [{ - name: 'weighted approval decision', - head_sha: HEAD_SHA, - status: 'in_progress', - details_url: 'https://github.example/actions/runs/1', - output: { title: 'Evaluating approval points.', summary: 'Evaluating approval points. [Workflow run](https://github.example/actions/runs/1)' }, - }, { + assert.deepEqual(writes, [{ name: 'weighted approval decision', head_sha: HEAD_SHA, status: 'completed', @@ -296,7 +293,7 @@ test('requires action on drafts without reading reviews', async () => { assert.match(result.description, /draft pull request/u) }) -test('publishes the required status and replaces stale success with error on evaluation failure', async () => { +test('publishes the decision check run and concludes failure when evaluation fails', async () => { const calls = [] const output = [] const result = await runApprovalCheck({ @@ -308,6 +305,7 @@ test('publishes the required status and replaces stale success with error on eva calls.push({ path, options }) if (path.includes('/reviews?')) return [review('turtle2099', 'APPROVED')] if (path.includes('/collaborators/turtle2099/permission')) return { permission: 'write' } + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) return {} throw new Error(`unexpected API path ${path}`) }, @@ -339,6 +337,7 @@ test('publishes the required status and replaces stale success with error on eva runUrl: 'https://github.example/actions/runs/2', api: async (path, options = {}) => { if (path.includes('/reviews?')) throw new Error('reviews unavailable') + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) { failures.push({ path, options }) return {} @@ -347,9 +346,10 @@ test('publishes the required status and replaces stale success with error on eva }, write: () => {}, }), /reviews unavailable/u) - assert.equal(publishedState(failures[0].options.body), 'in_progress') - assert.equal(publishedState(failures[1].options.body), 'failure') - assert.equal(failures[1].options.body.output.title, 'Approval evaluation failed.') + assert.deepEqual(failures.map(({ path }) => path), ['/repos/deepseek-harness/deepseek-harness/check-runs']) + assert.equal(failures[0].options.body.status, 'completed') + assert.equal(failures[0].options.body.conclusion, 'failure') + assert.equal(failures[0].options.body.output.title, 'Approval evaluation failed.') }) test('sends authenticated JSON and escapes an API error body', async () => { @@ -424,7 +424,7 @@ test('does not fetch history when approvals already have two-point weights', asy assert.equal(result.points, 2) }) -test('publishes error when production attribution fails', async () => { +test('concludes failure when production attribution fails', async () => { const states = [] await assert.rejects(runApprovalCheck({ event: pullRequestEvent(), policySource, runUrl: 'https://github.example/run/1', @@ -433,30 +433,43 @@ test('publishes error when production attribution fails', async () => { if (path.includes('/reviews?')) return [review('writer', 'APPROVED')] if (path.includes('/permission')) return { permission: 'write' } if (path.includes('/comments?')) return [] + if (isCheckRunsList(path)) return emptyCheckRuns states.push(publishedState(options.body)) return {} }, }), /incomplete history/u) - assert.deepEqual(states, ['in_progress', 'failure']) + assert.deepEqual(states, ['failure']) }) -test('revokes a previous success before starting expensive attribution', async () => { - const states = [] - await runApprovalCheck({ - event: pullRequestEvent(), policySource, runUrl: 'https://github.example/run/1', write: () => {}, - getOwnership: async () => { - assert.deepEqual(states, ['in_progress']) - return { totalLines: 100, reviewerLines: { writer: 25 } } - }, +test('revokes a published success without returning a completed run to in progress', async () => { + const calls = [] + const checkRuns = [ + { id: 7, app: { slug: 'github-actions' }, conclusion: 'success' }, + { id: 8, app: { slug: 'github-actions' }, conclusion: 'action_required' }, + { id: 9, app: { slug: 'other-app' }, conclusion: 'success' }, + ] + await publishApprovalPhase({ + event: pullRequestEvent(), runUrl: 'https://github.example/run/1', phase: 'revoke', api: async (path, options) => { - if (path.includes('/reviews?')) return [review('writer', 'APPROVED')] - if (path.includes('/permission')) return { permission: 'write' } - states.push(publishedState(options.body)) - return {} + calls.push({ path, method: options?.method, body: options?.body }) + return isCheckRunsList(path) ? { total_count: checkRuns.length, check_runs: checkRuns } : {} }, }) - assert.deepEqual(states, ['in_progress', 'success']) + assert.deepEqual(calls, [{ + path: `/repos/deepseek-harness/deepseek-harness/commits/${HEAD_SHA}/check-runs?check_name=weighted%20approval%20decision&filter=all&per_page=100`, + method: undefined, + body: undefined, + }, { + path: '/repos/deepseek-harness/deepseek-harness/check-runs/7', + method: 'PATCH', + body: { + status: 'completed', + conclusion: 'action_required', + details_url: 'https://github.example/run/1', + output: { title: 'Preparing approval evaluation.', summary: 'Preparing approval evaluation. [Workflow run](https://github.example/run/1)' }, + }, + }]) }) for (const reviewers of [['first', 'second'], ['turtle1999', 'first']]) { @@ -488,7 +501,7 @@ test('publishes setup phases without evaluating or installing dependencies', asy event: pullRequestEvent(), runUrl: 'https://github.example/run/1', api: withEmptyReads(async (path, { body }) => { assert.match(path, /\/check-runs$/u); states.push(publishedState(body)) }), } - await publishApprovalPhase({ ...options, phase: 'in_progress' }) + await publishApprovalPhase({ ...options, phase: 'revoke' }) await publishApprovalPhase({ ...options, phase: 'failure' }) assert.deepEqual(states, ['in_progress', 'failure']) await assert.rejects(publishApprovalPhase({ ...options, phase: 'success' }), /invalid approval setup phase/u) @@ -497,11 +510,11 @@ test('publishes setup phases without evaluating or installing dependencies', asy test('updates the head\'s GitHub Actions decision check run instead of creating another', async () => { const calls = [] const checkRuns = [ - { id: 7, app: { slug: 'github-actions' } }, - { id: 8, app: { slug: 'other-app' } }, + { id: 7, app: { slug: 'github-actions' }, conclusion: 'action_required' }, + { id: 8, app: { slug: 'other-app' }, conclusion: 'action_required' }, ] await publishApprovalPhase({ - event: pullRequestEvent(), runUrl: 'https://github.example/run/1', phase: 'in_progress', + event: pullRequestEvent(), runUrl: 'https://github.example/run/1', phase: 'failure', api: async (path, options) => { calls.push({ path, method: options?.method }) return { total_count: checkRuns.length, check_runs: checkRuns } @@ -586,7 +599,7 @@ test('drafts skip history and history failures revoke success with error', async states.push(publishedState(options.body)) }, }), /history unavailable/u) - assert.deepEqual(states, ['in_progress', 'failure']) + assert.deepEqual(states, ['failure']) }) @@ -627,7 +640,7 @@ test('the publisher counts merged history through the production API path', asyn assert.equal(result.points, 2) assert.equal(result.authorCredit.points, 0.55) assert.equal(result.approvals[0].points, 1.45) - assert.deepEqual(states, ['in_progress', 'success']) + assert.deepEqual(states, ['success']) assert.equal(output[0], 'Author credit: 0.55 (50 merged PRs).') }) @@ -862,6 +875,7 @@ test('an active delegate command requests review once and preserves other reques if (options?.method === 'POST') { requests.push(options.body); return {} } return { users: [{ login: 'another-reviewer' }, ...(alreadyRequested ? [{ login: 'WRITER' }] : [])], teams: [] } } + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) return {} return api(path, options) }, @@ -890,6 +904,7 @@ test('inactive commands and drafts do not request review', async () => { policySource, runUrl: 'https://github.example/run/1', getMergedCount: async () => 0, write: () => {}, api: async (path, options) => { assert.ok(!path.endsWith('/requested_reviewers')) + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) return {} return api(path, options) }, @@ -906,6 +921,7 @@ test('failed review requests preserve the computed score and report the retry', event: { ...pullRequestEvent(), issue: { number: 42, pull_request: {} }, action: 'created', comment: { id: 1 } }, policySource, runUrl: 'https://github.example/run/1', getMergedCount: async () => 0, getOwnership: async () => ({ totalLines: 0, reviewerLines: {} }), write: line => output.push(line), api: async (path, options) => { + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) { statuses.push(publishedState(options.body)); return {} } if (path.endsWith('/requested_reviewers')) { if (options?.method === 'POST') throw new Error('request rejected') @@ -916,7 +932,7 @@ test('failed review requests preserve the computed score and report the retry', }) assert.match(output.at(-1), failedRead ? /no users array/u : /request rejected/u) assert.match(output.at(-1), /score is unchanged.*will retry/u) - assert.deepEqual(statuses, ['in_progress', 'success']) + assert.deepEqual(statuses, ['success']) } }) @@ -928,6 +944,7 @@ test('delegation dismisses only the sender old decisions and stays active across const states = [] const read = delegationApi({ comments: [comment('turtle1999', '/delegate @writer')], reviews }) const api = async (path, options) => { + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) { states.push(publishedState(options.body)); return {} } if (path.endsWith('/dismissals')) { calls.push(options) @@ -952,7 +969,7 @@ test('delegation dismisses only the sender old decisions and stays active across assert.deepEqual(result.blockers, ['other']) assert.equal(result.delegations.length, 1) assert.deepEqual(result.delegations[0].reviewIds, []) - assert.deepEqual(states, ['in_progress', 'action_required']) + assert.deepEqual(states, ['action_required']) reviews.find(review => review.id === 21).state = 'DISMISSED' for (const state of ['APPROVED', 'DISMISSED', 'COMMENTED', 'APPROVED']) { reviews.find(review => review.id === 20).state = state @@ -972,6 +989,7 @@ test('a new sender review arriving during dismissal is not dismissed or delegate const result = await runWithHistory({ event: { ...pullRequestEvent(), issue: { number: 42, pull_request: {} }, action: 'created', comment: { id: 1 } }, policySource, runUrl: 'https://github.example/run/1', write: () => {}, getMergedCount: async () => 0, api: async (path, options) => { + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) return {} if (path.endsWith('/dismissals')) { assert.ok(path.endsWith('/reviews/10/dismissals')) @@ -994,6 +1012,7 @@ test('a failed or unconfirmed dismissal prevents review requests and publishes e await assert.rejects(runWithHistory({ event: { ...pullRequestEvent(), issue: { number: 42, pull_request: {} }, action: 'created', comment: { id: 1 } }, policySource, runUrl: 'https://github.example/run/1', write: () => {}, api: async (path, options) => { + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) { states.push(publishedState(options.body)); return {} } if (path.endsWith('/dismissals')) { if (rejected) throw new Error('dismissal denied') @@ -1003,7 +1022,7 @@ test('a failed or unconfirmed dismissal prevents review requests and publishes e return read(path, options) }, }), rejected ? /dismissal denied/u : /dismissal was not confirmed/u) - assert.deepEqual(states, ['in_progress', 'failure']) + assert.deepEqual(states, ['failure']) } }) @@ -1016,6 +1035,7 @@ test('reads all comment pages, logs the score owner, and fails closed on missing event: pullRequestEvent(), policySource, runUrl: 'https://github.example/run/1', getMergedCount: async () => 0, write: line => output.push(line), api: async (path, options) => { + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) { statuses.push(publishedState(options.body)); return {} } if (path.includes('/comments?')) return comments(path) return api(path, options) @@ -1029,7 +1049,7 @@ test('reads all comment pages, logs the score owner, and fails closed on missing }) assert.equal(pages.length, 2) assert.ok(output.includes('- @turtle1999: 2 (delegated to @writer)')) - assert.deepEqual(statuses.splice(0), ['in_progress', 'success']) + assert.deepEqual(statuses.splice(0), ['success']) for (const [comments, message] of [ [() => { throw new Error('comment API unavailable') }, /comment API unavailable/u], [() => ({}), /comments response is not an array/u], @@ -1039,7 +1059,7 @@ test('reads all comment pages, logs the score owner, and fails closed on missing [() => Array.from({ length: 100 }, () => comment('writer', 'text')), /comments exceed 3000/u], ]) { await assert.rejects(run(comments), message) - assert.deepEqual(statuses.splice(0), ['in_progress', 'failure']) + assert.deepEqual(statuses.splice(0), ['failure']) } }) @@ -1123,6 +1143,7 @@ test('later events reconcile all active delegations after a draft or replaced co const effects = [] const requested = [] const api = async (path, options) => { + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) return {} if (path.endsWith('/dismissals')) { const old = reviews.find(review => path.endsWith(`/reviews/${review.id}/dismissals`)) @@ -1155,6 +1176,7 @@ test('already approved delegates need no new request', async () => { }) const result = await runWithHistory({ event: pullRequestEvent(), policySource, runUrl: 'https://github.example/run/1', write: () => {}, api: (path, options) => { + if (isCheckRunsList(path)) return emptyCheckRuns if (path.includes('/check-runs')) return {} assert.ok(!path.endsWith('/requested_reviewers')) return read(path, options) diff --git a/.github/workflows/weighted-approval.yml b/.github/workflows/weighted-approval.yml index 896fc262c4..f72ada5942 100644 --- a/.github/workflows/weighted-approval.yml +++ b/.github/workflows/weighted-approval.yml @@ -41,7 +41,7 @@ jobs: env: GITHUB_TOKEN: ${{ github.token }} GITHUB_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - run: node .github/review-ownership/check-approval.mjs in_progress + run: node .github/review-ownership/check-approval.mjs revoke # SECURITY: dependency setup runs in the check-writing job. - if: steps.revoke.outputs.active == 'true' uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 @@ -53,13 +53,16 @@ jobs: if: steps.revoke.outputs.active == 'true' run: python3 -m pip install -r .github/review-ownership/requirements.txt - name: Publish weighted approval decision + id: publish if: steps.revoke.outputs.active == 'true' env: GITHUB_TOKEN: ${{ github.token }} GITHUB_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: node .github/review-ownership/check-approval.mjs + # A failed or cancelled run must not leave an earlier conclusion standing. A job cancelled + # after the decision was published keeps that decision instead of overwriting it with failure. - name: Publish approval setup failure - if: failure() && steps.revoke.outputs.active == 'true' + if: failure() || (cancelled() && steps.publish.outcome != 'success') env: GITHUB_TOKEN: ${{ github.token }} GITHUB_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} diff --git a/scripts/ci-workflow.spec.ts b/scripts/ci-workflow.spec.ts index 1275d4a7b0..e69d659644 100644 --- a/scripts/ci-workflow.spec.ts +++ b/scripts/ci-workflow.spec.ts @@ -1017,9 +1017,9 @@ describe('Weighted approval workflow', () => { const revokeIndex = steps.findIndex(step => step.id === 'revoke') expect(revokeIndex).toBeGreaterThan(steps.indexOf(checkout!)) expect(revokeIndex).toBeLessThan(setupIndex) - expect(steps[revokeIndex]?.run).toBe('node .github/review-ownership/check-approval.mjs in_progress') + expect(steps[revokeIndex]?.run).toBe('node .github/review-ownership/check-approval.mjs revoke') expect(steps.at(-1)).toMatchObject({ - if: "failure() && steps.revoke.outputs.active == 'true'", + if: "failure() || (cancelled() && steps.publish.outcome != 'success')", run: 'node .github/review-ownership/check-approval.mjs failure', }) const pythonJob = workflowJob(loadWorkflow('.github/workflows/ci.yml'), 'python-sdk') @@ -1032,6 +1032,7 @@ describe('Weighted approval workflow', () => { run: 'python3 -m pip install -r .github/review-ownership/requirements.txt', }) expect(publish).toMatchObject({ + id: 'publish', if: "steps.revoke.outputs.active == 'true'", env: { GITHUB_TOKEN: '${{ github.token }}', From 585d20fc7755c24d971dda4603005d6d3efe2f66 Mon Sep 17 00:00:00 2001 From: Turtle Date: Thu, 15 Oct 2026 17:47:02 +0800 Subject: [PATCH 003/201] fix(ui-chat): pause the running clock while reconnecting The Chat running indicator kept counting while the Client connection was disconnected or reconnecting, which read as the agent still working. The flow now reads the connection state and holds the clock until the connection is back. --- packages/client/ui-chat/package.json | 2 ++ packages/client/ui-chat/src/client/apply.ts | 9 ++++-- .../ui-chat/src/client/chat/ChatFlow.tsx | 5 ++-- .../ui-chat/src/client/chat/RunningStatus.tsx | 10 ++++--- .../ui-chat/src/client/contract/slots.ts | 10 +++++-- .../conversation-nodes/README.i18n.yaml | 4 +-- .../src/client/conversation-nodes/README.md | 2 +- .../client/conversation-nodes/README.zh.md | 2 +- .../tests/apply-inject.client.spec.tsx | 3 ++ .../ui-chat/tests/chat-apply.client.spec.tsx | 1 + .../ui-chat/tests/chat-view.client.spec.tsx | 30 +++++++++++++++++-- .../tests/quota-notice.client.spec.tsx | 1 + .../tests/running-status.client.spec.tsx | 17 +++++++++-- packages/client/ui-chat/tsconfig.client.json | 3 ++ .../tests/quota-notice-hold.client.spec.tsx | 1 + .../tests/assembly-surfaces.client.spec.tsx | 1 + .../tests/chat-ptc-subcalls.client.spec.tsx | 1 + .../tests/toolview-slot.client.spec.tsx | 2 ++ pnpm-lock.yaml | 3 ++ 19 files changed, 87 insertions(+), 20 deletions(-) diff --git a/packages/client/ui-chat/package.json b/packages/client/ui-chat/package.json index d43d47801c..492b4b4b39 100644 --- a/packages/client/ui-chat/package.json +++ b/packages/client/ui-chat/package.json @@ -32,6 +32,7 @@ "inject": [ "@deepseek-ai/dsh-api-session-controller", "@deepseek-ai/dsh-api-workspace-controller", + "@deepseek-ai/dsh-client-connection", "@deepseek-ai/dsh-client-locale", "@deepseek-ai/dsh-client-ui-conversation", "@deepseek-ai/dsh-client-ui-input-trigger", @@ -62,6 +63,7 @@ "@deepseek-ai/dsh-api-workspace-files": "workspace:*", "@deepseek-ai/dsh-attachment": "workspace:*", "@deepseek-ai/dsh-brand": "workspace:*", + "@deepseek-ai/dsh-client-connection": "workspace:*", "@deepseek-ai/dsh-client-locale": "workspace:*", "@deepseek-ai/dsh-client-store": "workspace:*", "@deepseek-ai/dsh-client-ui-approval": "workspace:*", diff --git a/packages/client/ui-chat/src/client/apply.ts b/packages/client/ui-chat/src/client/apply.ts index d51a29d316..6432e3e09e 100644 --- a/packages/client/ui-chat/src/client/apply.ts +++ b/packages/client/ui-chat/src/client/apply.ts @@ -21,6 +21,7 @@ import type {} from '@deepseek-ai/dsh-client-ui-renderer/client' import type {} from '@deepseek-ai/dsh-client-ui-session/client' import type {} from '@deepseek-ai/dsh-client-ui-settings/client' import type {} from '@deepseek-ai/dsh-client-ui-workspace/client' +import type { ConnectionHandle } from '@deepseek-ai/dsh-client-connection/client' import type { ChatFlowDataInjected, ChatFlowInjected, ChatNodeInjected, ChatScrollPosition, ChatViewInjected, QuotaNoticeInjected, QuotaNoticeState, TurnTailOwnerProps, @@ -74,7 +75,7 @@ export const CHAT_NODE_INJECT: ChatNodeInjected = { /** Services required by the Chat target and its presentation registrations. */ export const inject = [ 'slots', 'sessions', 'uiWorkspace', 'uiSession', 'uiConversation', 'locale', - 'configForms', 'remote', 'remote.session', 'sidebarRight', + 'configForms', 'connection', 'remote', 'remote.session', 'sidebarRight', ] /** @@ -203,7 +204,8 @@ export function apply(ctx: Context): void { }), }, TranscriptViewRow)) - const nodeSources = (binding: SessionBinding): ChatFlowDataInjected => { + const connectionState = (ctx.get('connection') as ConnectionHandle).state + const nodeSources = (binding: SessionBinding): Pick => { const chat = chatSource(binding) const conversation = ctx.uiConversation.binding(binding) return { @@ -225,7 +227,8 @@ export function apply(ctx: Context): void { inject: (sessionId: SessionId): ChatFlowDataInjected => { const binding = ctx.sessions.binding(sessionId) if (binding === undefined) throw new Error(`ui-chat: unknown session "${sessionId}"`) - return nodeSources(binding) + const sources = nodeSources(binding) + return { ...sources, hooks: { ...sources.hooks, connectionState } } }, }, ChatFlow)) diff --git a/packages/client/ui-chat/src/client/chat/ChatFlow.tsx b/packages/client/ui-chat/src/client/chat/ChatFlow.tsx index 224d5c7a4e..561632a671 100644 --- a/packages/client/ui-chat/src/client/chat/ChatFlow.tsx +++ b/packages/client/ui-chat/src/client/chat/ChatFlow.tsx @@ -25,12 +25,13 @@ function isTranscriptEcho(item: PendingInput): boolean { /** Render the flow without adding a DOM parent or changing keyed row positions. */ export const ChatFlow = memo(function ChatFlow({ entries, pendingInputs, lastInputTurn, deferCollapse, - useSession, useChat, useChatNode, useChatNodeBottom, useChatNodeProcess, useChatGroup, usePresentation, + useSession, useChat, useChatNode, useChatNodeBottom, useChatNodeProcess, useChatGroup, usePresentation, useConnectionState, useStore, actions, renderSlot, renderFactorySlot, t, useGroupAction, useGroupHeaderAction, cwd, openFile, inspectCall, forkAt, loadImage, fileMentions, }: ChatFlowSlotProps) { const nodeStore = useChat(snapshot => snapshot.nodes) const running = useSession(snapshot => snapshot.running) + const disconnected = useConnectionState(state => state === 'disconnected' || state === 'connecting') const latestTurnAnchor = useChat(snapshot => snapshot.navigation.items().at(-1)?.anchorKey) const runningStartTime = useChatNode(latestTurnAnchor ?? '', (node) => { const location = node?.location @@ -85,7 +86,7 @@ export const ChatFlow = memo(function ChatFlow({ // Rows enter before the keyed status and echoes after it, so React never moves its DOM node. return [ ...rows, ...leadingRows, ...openingRows, - ...running ? [] : [], + ...running ? [] : [], ...pendingRows, ] }) diff --git a/packages/client/ui-chat/src/client/chat/RunningStatus.tsx b/packages/client/ui-chat/src/client/chat/RunningStatus.tsx index fd145b6324..36a202c973 100644 --- a/packages/client/ui-chat/src/client/chat/RunningStatus.tsx +++ b/packages/client/ui-chat/src/client/chat/RunningStatus.tsx @@ -9,22 +9,24 @@ import css from './ChatView.module.css' interface RunningStatusProps { readonly startTime: number | undefined + /** Freeze the shown duration while the Client cannot observe the Session. */ + readonly paused: boolean readonly t: ChatViewSlotProps['t'] } /** * Show live elapsed time after the current Turn's content without announcing ticks. - * @param props - Current Turn start time and localized copy. + * @param props - Current Turn start time, pause state, and localized copy. * @returns the blue running indicator; mount only while the Session is running. */ -export const RunningStatus = memo(function RunningStatus({ startTime, t }: RunningStatusProps) { +export const RunningStatus = memo(function RunningStatus({ startTime, paused, t }: RunningStatusProps) { const [now, setNow] = useState(Date.now) useEffect(() => { - if (startTime === undefined) return + if (startTime === undefined || paused) return setNow(Date.now()) const timer = setInterval(() => { setNow(Date.now()) }, LIVE_RUN_CLOCK_INTERVAL_MS) return () => { clearInterval(timer) } - }, [startTime]) + }, [startTime, paused]) const label = startTime === undefined ? t('chat.deepDiving') : t('chat.deepDivingFor', { duration: formatRunDuration(Math.max(1000, now - startTime), t).map(part => part.text).join(''), }) diff --git a/packages/client/ui-chat/src/client/contract/slots.ts b/packages/client/ui-chat/src/client/contract/slots.ts index a85e28798d..4bd88840f6 100644 --- a/packages/client/ui-chat/src/client/contract/slots.ts +++ b/packages/client/ui-chat/src/client/contract/slots.ts @@ -15,6 +15,7 @@ import type { PropsRenderFactories, PropsStore, SlotHookFactory, SnapshotSelectorHook, } from '@deepseek-ai/dsh-client-ui-slots' import type { ObservableSnapshot } from '@deepseek-ai/dsh-client-store' +import type { ConnectionStateSource } from '@deepseek-ai/dsh-client-connection/client' import type { MarkdownFileMentions, MarkdownLabels } from '@deepseek-ai/dsh-client-ui-primitives' import type {} from '@deepseek-ai/dsh-client-ui-layout/client' // Type-only: the Session-scope access Hook (`useAccess`) the Chat entries read. @@ -325,8 +326,13 @@ export interface ChatViewInjected { fileMentions: (owner: TurnTailOwnerProps) => MarkdownFileMentions | undefined } -/** Existing Session-owned node sources shared by the view and its flow renderer. */ -export type ChatFlowDataInjected = Pick +/** Session-owned node sources shared with the view, plus the flow's connection state. */ +export type ChatFlowDataInjected = Pick & { + hooks: ChatViewInjected['hooks'] & { + /** Client connection lifecycle; the running clock pauses while it is not connected. */ + connectionState: ConnectionStateSource + } +} /** Ordered rows and reconciled local echoes rendered inside one Chat viewport. */ export interface ChatFlowOwnerProps extends Pick