Skip to content

fix(backend): create_commit 直接引用经典空树 SHA(W1-C2 #165,ADR-0054) - #2

Merged
randypanding merged 2 commits into
mainfrom
fix-empty-tree
Aug 21, 2026
Merged

fix(backend): create_commit 直接引用经典空树 SHA(W1-C2 #165,ADR-0054)#2
randypanding merged 2 commits into
mainfrom
fix-empty-tree

Conversation

@randypanding

@randypanding randypanding commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

动机

W1-C3 AC-1 e2e 演习(陌生 agent,卡 .github#206)抓出:GitHubRefBackend.create_commitPOST /git/trees 空 body 建空树,GitHub 对空树创建一律 422 Invalid tree info({}/tree:[]/base_tree 姿势全拒)——租约创建在真实 API 上从未成功过。单测全走 LocalGitBackend 故未暴露(conductor run 32493680834 实证,fail-closed ABORT 语义正确工作:拒绝+不绕过)。

变更

  • create_commit 删除 /git/trees 调用,直接以经典空树 SHA 4b825dc… 为 tree 调 POST /git/commits(租约元数据全在 commit message,无需树内容)
  • 新增 tests/test_github_backend_wire.py(3 例,HTTP 请求形状层回归锁定:trees 零调用、tree=常量、parent 透传、常量值本体断言)

验证

  • 本地全量 unittest72 tests OK(69+3)
  • 修复合入后由主代理在 .github#206 重投 /claim 完成 e2e 闭环(租约 ref 实存验证)

AC 映射(#165 AC-2/AC-5 补强)

真实 GitHub 后端的 CAS 链路至此才有实证;fail-closed 行为已被 e2e 负向验证(infra=ABORT 非绕过)。

回滚:revert 本 PR(内核其余路径零改动)。

Summary by CodeRabbit

  • Bug 修复
    • 优化 GitHub 提交创建流程,创建空提交时更加稳定可靠。
    • 避免不必要的空树创建请求,减少 API 调用并提升处理效率。
    • 修复并验证带有父提交和无父提交两种场景下的提交数据传递。

Copilot AI lite review requested due to automatic review settings August 21, 2026 15:51
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2811f608-3ffb-49ab-8e10-5bbb1b384caa

📥 Commits

Reviewing files that changed from the base of the PR and between 2f344cd and 50ec39e.

📒 Files selected for processing (2)
  • arbiter/backend.py
  • tests/test_github_backend_wire.py

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

Changes

GitHub 空提交创建

Layer / File(s) Summary
空提交请求与回归验证
arbiter/backend.py, tests/test_github_backend_wire.py
GitHubRefBackend.create_commit 使用固定的 EMPTY_TREE_SHA 调用 /git/commits,不再调用 /git/trees。测试覆盖无父提交、有父提交、请求调用次数及 canonical 常量值。

Suggested labels: bug

Merge Risk: ⚪ Minimal · up to 50ec3

This change uses the GitHub-compatible classic empty-tree SHA when creating commits and adds regression coverage for the request shape; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning 标题准确描述了修复内容,并使用了有效的 Conventional Commits 前缀,但长度为 61 个字符,超过 50 字符限制。 将标题缩短至 50 个字符以内,同时保留有效前缀和核心修复内容。
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-empty-tree

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix GitHubRefBackend create_commit to use canonical empty tree SHA

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Fix GitHub create_commit by removing failing empty-tree /git/trees call
• Always create commits using Git’s canonical empty tree SHA
• Add wire-level tests to lock HTTP request shape and parent passthrough
Diagram

graph TD
  A["GitHubRefBackend.create_commit"] --> B["EMPTY_TREE_SHA constant"] --> C["GitHub API /git/commits"]
  A --> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use parent commit’s tree when parent is provided
  • ➕ Avoids relying on a magic constant
  • ➕ Ensures tree is always known-valid in GitHub
  • ➖ Doesn’t solve the no-parent case (new lease) by itself
  • ➖ Couples lease commit creation to repository state more than necessary
2. Create a minimal tree with a placeholder blob
  • ➕ Avoids empty-tree edge case entirely
  • ➕ Tree SHA is derived from content rather than a hardcoded constant
  • ➖ Introduces unnecessary repo content semantics and storage
  • ➖ More API calls (blobs/trees) and more failure modes

Recommendation: Keep the PR’s approach: using git’s canonical empty-tree SHA is the simplest and most stable fix given that lease/ledger metadata is stored entirely in the commit message. It removes a GitHub API call path that is known to fail (422) and is now protected by wire-level regression tests.

Files changed (2) +74 / -8

Bug fix (1) +8 / -8
backend.pyUse canonical empty-tree SHA for GitHubRefBackend.create_commit +8/-8

Use canonical empty-tree SHA for GitHubRefBackend.create_commit

• Stops calling POST /git/trees to create an empty tree (GitHub rejects this with 422). Introduces EMPTY_TREE_SHA and always posts commits with that tree; parent commit SHA is still passed through when provided.

arbiter/backend.py

Tests (1) +66 / -0
test_github_backend_wire.pyAdd wire regression tests for GitHubRefBackend create_commit +66/-0

Add wire regression tests for GitHubRefBackend create_commit

• Adds three unittest cases that mock _request to assert create_commit makes exactly one POST to /git/commits, uses the canonical empty-tree SHA, and correctly passes parents when provided. Also pins the constant value to prevent accidental drift without real API evidence.

tests/test_github_backend_wire.py

@coderabbitai coderabbitai Bot added the bug Something isn't working label Aug 21, 2026
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Parent 分支未锁请求形状 🐞 Bug ⚙ Maintainability
Description
tests/test_github_backend_wire.py 的 parent 用例未断言只发生一次请求、且请求路径必须是 /git/commits,因此未来若仅在 parent
分支回归引入额外调用(例如再调用 /git/trees)可能不会被该用例稳定捕获。该测试文件的意图是“HTTP 请求形状回归锁定”,建议对 parent 场景同样加上请求次数与路径断言。
Code

tests/test_github_backend_wire.py[R46-49]

+        def fake_request(method, path, body=None):
+            calls.append((method, path, body))
+            return 201, {"sha": "beef"}
+
Evidence
parent 场景测试目前对 _request 的 fake 实现对任意 path 都返回 201,且用例未断言请求次数/路径;而生产代码在 if parent: 分支会改写
body(未来也可能被人扩展为额外请求),因此该用例没有把“请求形状”在 parent 分支上锁死。

tests/test_github_backend_wire.py[43-55]
arbiter/backend.py[124-133]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`test_create_commit_with_parent` 目前只断言了 `parents` 透传,但没有锁定:
- 仅一次 HTTP 调用
- 调用路径必须是 `.../git/commits`
- 不允许任何额外调用(例如回归引入 `/git/trees`)

这与该文件的“请求形状回归测试”目标不完全一致。

## Issue Context
`create_commit` 存在 `if parent:` 分支(parent 场景与非 parent 场景可能在未来被人改出差异),因此需要在 parent 用例里也做同等强度的 wire-level 断言。

## Fix Focus Areas
- tests/test_github_backend_wire.py[43-55]
- arbiter/backend.py[126-129]

## Suggested change
在 `test_create_commit_with_parent` 中:
1) 让 `fake_request` 对非 `/git/commits` 的调用直接 `AssertionError`(与第一个用例一致)。
2) 在调用后断言 `len(calls) == 1`。
3) 断言 `method == "POST"` 且 `path.endswith("/git/commits")`。
4) 同时断言 `body["tree"] == GitHubRefBackend.EMPTY_TREE_SHA`,确保 parent 场景也使用空树常量。

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
  +2 more
Review mode: ⚖️ Balanced: 该变更修改 GitHub 后端真实 API 的提交链路并引入固定协议常量,虽逻辑集中且有回归测试,但属于有行为和外部契约影响的风险变更,需完整单次审查。

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 修复 GitHubRefBackend.create_commit 在真实 GitHub Git Data API 上无法创建“空树”的问题:不再调用 POST /git/trees 试图创建空树,而是直接使用 Git 的经典空树 SHA 作为 commit 的 tree,从而让租约/台账的 commit 能在真实 API 上成功落地(元数据仍仅存于 commit message)。

Changes:

  • GitHubRefBackend.create_commit 移除 /git/trees 调用,改为使用常量 EMPTY_TREE_SHA 直接创建 commit。
  • 新增 wire-level 回归测试,锁定 create_commit 的 HTTP 请求形状(确保不再回归到 /git/trees)。

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
arbiter/backend.py 为 GitHub 后端引入经典空树 SHA 常量,并让 create_commit 直接引用该 tree 创建 commit。
tests/test_github_backend_wire.py 新增对 GitHubRefBackend.create_commit 的请求形状回归测试(mock _request)。

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +10 to +13
import unittest
from unittest.mock import patch

from arbiter.backend import GitHubRefBackend
Comment on lines +46 to +49
def fake_request(method, path, body=None):
calls.append((method, path, body))
return 201, {"sha": "beef"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Parent 分支未锁请求形状 🐞 Bug ⚙ Maintainability

tests/test_github_backend_wire.py 的 parent 用例未断言只发生一次请求、且请求路径必须是 /git/commits,因此未来若仅在 parent
分支回归引入额外调用(例如再调用 /git/trees)可能不会被该用例稳定捕获。该测试文件的意图是“HTTP 请求形状回归锁定”,建议对 parent 场景同样加上请求次数与路径断言。
Agent Prompt
## Issue description
`test_create_commit_with_parent` 目前只断言了 `parents` 透传,但没有锁定:
- 仅一次 HTTP 调用
- 调用路径必须是 `.../git/commits`
- 不允许任何额外调用(例如回归引入 `/git/trees`)

这与该文件的“请求形状回归测试”目标不完全一致。

## Issue Context
`create_commit` 存在 `if parent:` 分支(parent 场景与非 parent 场景可能在未来被人改出差异),因此需要在 parent 用例里也做同等强度的 wire-level 断言。

## Fix Focus Areas
- tests/test_github_backend_wire.py[43-55]
- arbiter/backend.py[126-129]

## Suggested change
在 `test_create_commit_with_parent` 中:
1) 让 `fake_request` 对非 `/git/commits` 的调用直接 `AssertionError`(与第一个用例一致)。
2) 在调用后断言 `len(calls) == 1`。
3) 断言 `method == "POST"` 且 `path.endswith("/git/commits")`。
4) 同时断言 `body["tree"] == GitHubRefBackend.EMPTY_TREE_SHA`,确保 parent 场景也使用空树常量。

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@randypanding
randypanding merged commit 3dff55d into main Aug 21, 2026
14 checks passed
@randypanding
randypanding deleted the fix-empty-tree branch August 21, 2026 15:54
randypanding added a commit that referenced this pull request Aug 26, 2026
清偿项(全部行为保持,裁决语义/退出码/错误通道零变化):
- kernel.adjudicate:删除防重放段无操作的 try/except InfraError: raise 纯透传
  包装(异常本就由函数级 infra 通道统一上报);防重放语义(seen ref + 台账)
  与 CasConflict→noop/replay-detected 路径逐字节不变
- kernel.EXIT_CODE:改由 EXIT_ALLOW/EXIT_DENY/EXIT_INFRA 常量组装,
  消除字面量双写(原三常量从未被引用,属死代码)
- backend.LocalGitBackend:4 处重复的 subprocess.run(git -C …) 样板收敛为
  _exec_git 单一入口;各调用方的异常包装与报错文案逐字保留
  ("git 子进程失败"/"git rev-parse 失败"两通道不合并)
- 魔法常量具名化:timeout=30×4 → GIT_TIMEOUT_SECONDS;重试 3 次/0.15s →
  LOCK_RETRY_MAX_ATTEMPTS / LOCK_RETRY_SLEEP_SECONDS
- 函数内 import(time / urllib.request|error|parse)上提至模块顶部
  (backend.py 是静态扫描白名单中的唯一网络模块,tests/test_no_llm.py 不受限)

不动项:#2 空树 SHA 链路与 _quote_ref_path refs/ 剥离逻辑零触碰;
capabilities.yaml / AGENTS.md / ci.yml / README / tests 断言数量不减。

验证:python -m py_compile arbiter/*.py ✓;
python -m unittest discover -s tests → 73 tests OK ×2 连跑;
python -m arbiter.policy capabilities.yaml → POLICY-OK default-deny=verified。

Co-authored-by: randypanding <randypanding@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants