Repository navigation
fix(ci): refresh stale pull request diff bases - #8344
huangruiteng wants to merge 1 commit into
Conversation
Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent, gpt-6.1-sol, OpenAI, runtime_reported, reasoning_effort=max
Request changes conclusion
动机 / Motivation
更新 PR、需要准确审查差异并保留自己选择的目标分支的作者和维护者。源分支纳入目标提交后,GitHub 的旧比较快照可能继续显示无关文件;新维护任务尝试用同 base 的 PATCH 刷新它,但最后一次 GET 与 PATCH 之间的目标分支修改会被旧 base 覆盖。正常 stale 输入可以刷新并保持 source;独立执行原工作流 JavaScript 时,合法并发 retarget 从 release/stable 被改回 main,任务仍报告 Refreshed,故该交付尚未满足不改目标的边界。限定既有可信 CI Maintenance、贡献指南及相关测试;不修改 PR 源码历史,不授予合并权限,也不把本地模拟执行说成已上线托管任务。
评审从实际读者或维护者要完成的工作判断:有可读的事实、原要求与可信回读,才是有用交付。编译成功、元数据一致或通过数量不能单独证明这些结果。这里保留当前证据边界,不要求本 PR 完成无关的整体 runtime 或长期实验。
改动思路 / Approach
复用现有可信维护工作流和 GitHub PR owner 的位置合理;需要先收紧无条件 PATCH 的效果边界,增加一次 GET 不能关闭同一个竞态窗口。 限定既有可信 CI Maintenance、贡献指南及相关测试;不修改 PR 源码历史,不授予合并权限,也不把本地模拟执行说成已上线托管任务。
需要分开观察和效果:GET得到的是当时的PR,PATCH会在另一个时刻设置base。可信代码、同一个布尔身份谓词和事后回读都不能把这两步变成一个有条件的事务。普通维护不应把作者已经选择的新目标改回旧值;安全边界应在效果真正应用处成立。
具体改动 / Changes
规范依据是修改前的 CONTRIBUTING.md,spec_revision ecec9f4efd65c84f890af3c58ddb8e2e053b1290。逐项判断 What CI runs on a pull request;Pull Request Checklist;Validation disclosure。可信代码与失败披露有对应证据,但 Pull Request Checklist 要求的 scoped outcome 在并发目标上未满足;本次未用新增指南替代原目标和实际效果。
关键代码讲解
sameIdentity:核对 open、source SHA/ref/repository 与目标 ref/repository;只能证明读取时身份。pulls.update / readPR:对旧 base 发无条件 PATCH 后读回;并发 retarget 可先被覆盖,读回因此仍匹配旧事件。Verify the published diff:要求比较准确 target 的三点差异并读回发布库存;指南不能使非原子 provider 写入获得条件保护。
完整五路径 +186/−3。既有 workflow 增加 opened/reopened/edited 触发和一个45行左右的 refresh job,只有该 job 得到 pull-requests:write;全局 contents:read、旧 cancel-obsolete-pr 的触发条件和资格工作流隔离保留。指南增加真实 target 三点差异与远端库存/行总数回读;repair catalog 仅增加一条有界诊断;测试执行真实 inline JavaScript,并覆盖 fork、non-main、fresh、写前漂移、失败和回读。
[P2] 最后一次读取之后,PATCH 会覆盖一个新目标。 这不是仅凭延迟猜测:独立 adapter 在 provider 真正应用 PATCH 前把 target 改成 release/stable,再按文档执行 params.base 赋值。原工作流最终把它改回 main、head不变、error为null,并输出 Refreshed;相同输入下 baseline 没有该写入,新目标保留。GitHub Update a pull request 将 base 定义为要设置的目标分支;当前调用没有附加任何条件。现有 base_during_write 用例在 update 后才改变 ref,未实现这个字段的实际赋值,故绿色并不能覆盖该反例。
对主干的风险 / Risks
四个相关仓库模块190 passed;标准premerge的直接检查、6项选择检查和公共边界通过。正常 stale/同源刷新可用,既有 closed、错source/head/base、API拒绝、未应用和后读漂移等用例保留。独立 late-retarget 反例仍失败,不能用这些绿项消除它。
该反例在真实 Node24.21 执行未改的 workflow JavaScript,GitHub服务由遵守文档字段效果的 adapter 模拟;没有向其他真实PR发PATCH,也没有声称已执行合入后的托管job。它的适用界限明确,但无条件赋值与检查窗口直接存在于本 head。
该错误直接改变PR的目标与随之计算的diff和资格,而不只是日志不准确;当新目标已被覆盖,后续GET与旧event相等会隐藏这一事实。维护者需要重新判断并恢复目标,不能对原PATCH盲重试或根据旧事件自动改回另一个值。
按本 Goal 策略未查询、轮询或等待 CI。仓库绿测和scope外拒绝证据保留,late-retarget作为当前PR回归单列,未放宽权限或测试断言。 尚未运行合入后的 GitHub 托管任务;未对其他真实 PR 发 PATCH 复现竞态,也未授予或执行合并。
我的整体评价 / Verdict
REQUEST_CHANGES。修复真实比较问题的动机成立,位置和可信代码/权限隔离也基本正确;但自动动作会在可控反例中撤销维护者的新目标,当前不能批准。最小边界是让实际写入有可验证的目标条件/协调,或先保留只读自动检测与受控人工同base刷新,并用真实赋值语义补齐反例。不能仅再加一次GET或在覆盖之后打印“发生变化”。
复用现有可信维护工作流和 GitHub PR owner 的位置合理;需要先收紧无条件 PATCH 的效果边界,增加一次 GET 不能关闭同一个竞态窗口。 限定既有可信 CI Maintenance、贡献指南及相关测试;不修改 PR 源码历史,不授予合并权限,也不把本地模拟执行说成已上线托管任务。
检查的是可信工作流、指南和测试,没有新增渲染界面;网页布局与截图验收不适用。 尚未运行合入后的 GitHub 托管任务;未对其他真实 PR 发 PATCH 复现竞态,也未授予或执行合并。
English verdict: REQUEST_CHANGES at d9438d7. The unconditional stale-base PATCH can overwrite a concurrent maintainer retarget and still report success. Preserve the current target at the provider effect boundary and add the faithful late-retarget counterexample before approval.
| if (!sameIdentity(before) || before.base.sha !== pr.base.sha) { | ||
| return core.info('PR changed before update; stop diff refresh.'); | ||
| } | ||
| await github.rest.pulls.update({...repo, pull_number, base: pr.base.ref}); |
There was a problem hiding this comment.
[P2] Preserve a concurrent target edit before applying the stale base
在最后一次 readPR 后、PATCH 真正应用前,维护者仍可把目标从 main 改到 release/stable。这里发送的 base 仍为旧 main;该 REST 字段会直接设置目标,因此会覆盖这次合法修改。之后 sameIdentity(after) 又匹配旧事件,任务甚至报告 Refreshed。原样执行这段 JavaScript,按文档实现 params.base 赋值并在写前插入 retarget 即可复现;baseline 无该任务时新目标保留。现有 base_during_write mock 只在更新后改 ref,没有模拟 PATCH 覆盖它。请使 provider 写入具备真正的目标条件/协调,或先将自动路径保持只读,并增加这个最后 GET 与实际 PATCH 之间的反例;再多一次 GET 仍有相同窗口。
Goal and delivered outcome
GitHub can retain an old PR base snapshot after target commits enter the source branch. The PR then counts unrelated target changes despite an unchanged source head. Re-selecting the same base recalculates the comparison.
The existing CI Maintenance workflow now refreshes a stale snapshot on opened, reopened, edited and synchronize events. It checks the event's source repository, branch, head and target before the write, repeats that check immediately before updating, and reads back the unchanged identity and refreshed base. It never checks out PR content, rewrites source history, temporarily retargets, or posts comments. Only this job gains
pull-requests: write; qualification and obsolete-run cleanup keep their existing permissions and behavior.CONTRIBUTING.mdnow requires published-file and line-total readback against the actual target's three-dot diff. The self-repair catalog records the diagnosis and bounded repair.Author declaration and specification
Written by: model_agent, OpenAI Codex.
Basis: reproduced GitHub comparison defect and the repository's existing Worktree And PR Gate / Pull Request Checklist at base
ecec9f4efd65c84f890af3c58ddb8e2e053b1290. No separate written specification.Placement: existing repository CI maintenance, with no new LoopX capability, provider, setting or decision owner. The shared identity predicate keeps all three checks consistent.
Validation
Tested revision:
d9438d7879cf0b3937d3a2c46380224bee760578.Run state: finished. Input classes: synthetic, public_fixture.
python -m pytest -q tests/test_ci_resource_policy.py tests/test_sonarcloud_workflow.py tests/test_repair_pattern_lookup.py tests/test_repository_instruction_delivery.py: 190 passed. Executes the exact inline workflow JavaScript under Node, including fork and non-main targets, identity races, no-op, failure and readback cases.The new hosted job becomes available after this workflow reaches the target branch. Target advancement alone does not sweep open PRs; author handoff still requires the documented readback. Full repository tests and hosted execution of the new job were not run. No merge is included.
Scope and continuation
Complete within the maintenance scope; no product-runtime or frontend behavior changes. Merge remains a separate maintainer decision. UI impact: none; visual evidence is not applicable.