Skip to content

refactor: 近一周 PR 技术债清扫(行为保持) - #8

Merged
randypanding merged 1 commit into
mainfrom
refactor/tech-debt-2026-08-26
Aug 26, 2026
Merged

refactor: 近一周 PR 技术债清扫(行为保持)#8
randypanding merged 1 commit into
mainfrom
refactor/tech-debt-2026-08-26

Conversation

@randypanding

@randypanding randypanding commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

近一周 PR 全量复读结论(#1#7,全部已合并)

PR 处置 备注
#1 仲裁内核 v1 无新债需修 本体清偿:EXIT_CODE 字面量双写、adjudicate 防重放段死包装、LocalGitBackend 子进程样板 ×4(见下)
#2 create_commit 空树 SHA fix 零触碰——正确性关键路径 常量 EMPTY_TREE_SHA 与 POST /git/commits 形状原样保留;wire 回归测试未动
#3 refs/ 前缀剥离 fix 零触碰——正确性关键路径 _quote_ref_path 剥前缀逻辑与其 e2e 实证注释逐字保留
#4 capabilities /trust-status 策略表,不在清扫范围 policy-validate 通过确认 schema 完好
#5 AGENTS.md v1 文档,不动
#6 AGENTS.md v2 文档,不动
#7 ci hygiene + merge_group workflow/README,按约不动

清偿清单(行为保持)

  1. kernel.adjudicate 防重放段删除无操作的外层 except InfraError: raise 纯透传——异常本就由函数级 infra 通道上报;CasConflict→noop/replay-detected 判定不变
  2. kernel.EXIT_CODE 改由 EXIT_ALLOW/EXIT_DENY/EXIT_INFRA 组装(三常量原本定义后从未被引用=死代码;现消除与映射表的双写)
  3. LocalGitBackend 4 处重复的 subprocess.run(["git","-C",…]) 样板收敛为 _exec_git();各调用点异常包装与报错文案逐字保留("git 子进程失败"/"git rev-parse 失败"两通道不合并)
  4. 魔法常量具名化:timeout=30×4 → GIT_TIMEOUT_SECONDS;锁重试 3 次 / 0.15sLOCK_RETRY_MAX_ATTEMPTS/LOCK_RETRY_SLEEP_SECONDS
  5. 函数内 import(time/urllib.*)上提模块顶部——backend.py 是 test_no_llm 白名单中唯一网络模块,静态断言不受影响

明确不做

验证证据

  • python -m py_compile arbiter/*.py
  • python -m unittest discover -s tests73 tests OK ×2 连跑(含并发用例,无 flake)
  • python -m arbiter.policy capabilities.yaml → POLICY-OK commands=['/claim','/release','/retry','/trust-status'] default-deny=verified
  • git diff --stat → 仅 arbiter/kernel.py + arbiter/backend.py 两文件,无 CRLF/格式噪音

Summary by CodeRabbit

  • 错误修复
    • 改进 Git 操作的超时处理,并在锁竞争时自动重试,提高执行稳定性。
    • 防重放记录遇到并发冲突时可安全返回幂等结果,减少重复处理。
    • 统一退出状态映射,确保允许、拒绝和基础设施错误返回正确结果。

清偿项(全部行为保持,裁决语义/退出码/错误通道零变化):
- 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。
Copilot AI lite review requested due to automatic review settings August 26, 2026 18:30

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Refactor kernel and local Git backend without behavior changes

✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Centralize local Git subprocess construction while preserving caller-specific error semantics.
• Replace duplicated timeout, retry, and exit-code literals with named constants.
• Simplify replay handling and hoist standard-library imports without changing adjudication
 behavior.
Diagram

graph TD
  K["Kernel adjudication"] --> P["Backend protocol"] --> L["Local Git backend"] --> E["Git executor"] --> G["Git CLI"]
  E --> M["Caller error mapping"]
Loading
High-Level Assessment

The chosen approach is appropriate: centralize only subprocess construction and leave return-code and exception interpretation at each caller. A broader abstraction that also normalizes errors was considered but would risk merging intentionally distinct infrastructure, not-found, and CAS-conflict channels.

Files changed (2) +44 / -36

Refactor (2) +44 / -36
backend.pyCentralize LocalGitBackend subprocess execution and retry constants +34/-26

Centralize LocalGitBackend subprocess execution and retry constants

• Adds '_exec_git()' as the shared Git subprocess entry point and routes existing local backend operations through it. Names timeout and lock-retry settings, hoists standard-library imports, and preserves caller-specific identity injection, exception wrapping, return-code handling, and messages.

arbiter/backend.py

kernel.pyReuse exit constants and simplify replay exception propagation +10/-10

Reuse exit constants and simplify replay exception propagation

• Builds the verdict-to-exit-code mapping from existing constants instead of duplicated literals. Removes a no-op 'InfraError' rethrow wrapper around replay marker creation while retaining CAS-conflict replay detection and function-level infrastructure handling.

arbiter/kernel.py

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 382d6326-f9dc-4c6c-86b8-da7201505391

📥 Commits

Reviewing files that changed from the base of the PR and between 7e96878 and 9e1bcf2.

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

📝 Walkthrough

Walkthrough

本次变更统一本地Git子进程执行、超时和锁重试配置,并调整引用及提交检查路径。防重放台账现统一处理 create_commitcreate_ref 的CAS冲突。

Changes

Git执行与防重放

Layer / File(s) Summary
Git执行配置与统一执行器
arbiter/backend.py
将相关依赖移至模块级导入。LocalGitBackend 新增Git超时和锁重试配置,并通过 _exec_git 统一执行Git子进程。
Git引用与提交检查接入
arbiter/backend.py
_existsread_refcommit_message 的Git检查改用统一执行器,同时保留原有错误处理语义。
防重放CAS冲突处理
arbiter/kernel.py
EXIT_CODE 改用退出码常量。create_commitcreate_refCasConflict 统一返回 replay-detectednoop

Suggested labels: tech-debt

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/tech-debt-2026-08-26

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

@randypanding
randypanding merged commit 5340a35 into main Aug 26, 2026
16 of 17 checks passed
@randypanding
randypanding deleted the refactor/tech-debt-2026-08-26 branch August 26, 2026 18:32
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Independent refactors bundled together 📘 Rule violation ⚙ Maintainability
Description
The PR combines a LocalGitBackend subprocess abstraction with separate adjudication exception
restructuring and exit-code mapping cleanup in kernel.py. These changes are not mechanically
dependent and therefore exceed a single functional concern.
Code

arbiter/backend.py[R180-184]

+    def _exec_git(self, args, ident: bool = False, stdin: str | None = None):
+        """构造并执行 git 子进程,返回 CompletedProcess(不判 rc、不包装异常)。
+
+        ident=True 时注入兜底 user.name/email。异常是否包成 InfraError 由各
+        调用方决定(只读探测与裁决路径的错误通道语义不同,故不在此统一)。
Relevance

●● Moderate

Scope-bundling concern is subjective; PR explicitly frames these as one coordinated debt cleanup, no
close precedent found.

PR-#1

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rule requires all non-trivial changes to support one concern. The cited backend region
introduces a generalized Git execution helper, while the cited kernel regions independently alter
verdict mapping construction and replay exception structure.

Rule 2908442: Limit pull request changes to a single functional concern
arbiter/backend.py[180-193]
arbiter/kernel.py[28-30]
arbiter/kernel.py[97-110]

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

## Issue description
This PR bundles multiple independent refactoring concerns: Git subprocess consolidation in `backend.py`, plus adjudication exception-scope and exit-code mapping changes in `kernel.py`.

## Issue Context
PR Compliance ID 2908442 requires each PR to contain one functional concern. These edits can be reviewed and reverted independently and have no implementation dependency.

## Fix Focus Areas
- arbiter/backend.py[180-193]
- arbiter/kernel.py[28-30]
- arbiter/kernel.py[97-110]

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


2. Commit conflicts become replays 🐞 Bug ≡ Correctness
Description
The flattened except CasConflict now also catches backend.create_commit(), so a commit-creation
conflict is reported as replay-detected even though no seen ref was created. This silently drops
the delivery instead of surfacing a backend failure for retry.
Code

arbiter/kernel.py[106]

+        except CasConflict:
Relevance

●● Moderate

Behavior change but PR claims CasConflict→noop semantics unchanged; similar exception-flattening in
PR1 had mixed rejection precedent.

PR-#1

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The handler encloses both operations, while the returned reason specifically asserts that the seen
ref exists. LocalGitBackend.create_commit uses the generic _git failure classifier, which can
emit CasConflict; because adjudicate's outer channel catches only InfraError and LeaseError,
the old nested structure did not turn such commit failures into no-ops.

arbiter/kernel.py[103-110]
arbiter/kernel.py[128-134]
arbiter/backend.py[204-210]
arbiter/backend.py[280-286]

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

## Issue description
The replay handler now catches `CasConflict` from both commit creation and seen-ref creation, misclassifying commit failures as successful replay detection.

## Issue Context
Only a conflict while creating `refs/seen/...` proves that the delivery was previously processed. Restore the nested scope so `backend.create_commit()` remains outside the `CasConflict` handler, while `InfraError` continues to flow to the function-level infra channel.

## Fix Focus Areas
- arbiter/kernel.py[103-110]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 13 rules
Review mode: ⚖️ Balanced

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread arbiter/backend.py
Comment on lines +180 to +184
def _exec_git(self, args, ident: bool = False, stdin: str | None = None):
"""构造并执行 git 子进程,返回 CompletedProcess(不判 rc、不包装异常)。

ident=True 时注入兜底 user.name/email。异常是否包成 InfraError 由各
调用方决定(只读探测与裁决路径的错误通道语义不同,故不在此统一)。

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. Independent refactors bundled together 📘 Rule violation ⚙ Maintainability

The PR combines a LocalGitBackend subprocess abstraction with separate adjudication exception
restructuring and exit-code mapping cleanup in kernel.py. These changes are not mechanically
dependent and therefore exceed a single functional concern.
Agent Prompt
## Issue description
This PR bundles multiple independent refactoring concerns: Git subprocess consolidation in `backend.py`, plus adjudication exception-scope and exit-code mapping changes in `kernel.py`.

## Issue Context
PR Compliance ID 2908442 requires each PR to contain one functional concern. These edits can be reviewed and reverted independently and have no implementation dependency.

## Fix Focus Areas
- arbiter/backend.py[180-193]
- arbiter/kernel.py[28-30]
- arbiter/kernel.py[97-110]

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

Comment thread arbiter/kernel.py
except InfraError:
raise
backend.create_ref(seen, marker)
except CasConflict:

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

2. Commit conflicts become replays 🐞 Bug ≡ Correctness

The flattened except CasConflict now also catches backend.create_commit(), so a commit-creation
conflict is reported as replay-detected even though no seen ref was created. This silently drops
the delivery instead of surfacing a backend failure for retry.
Agent Prompt
## Issue description
The replay handler now catches `CasConflict` from both commit creation and seen-ref creation, misclassifying commit failures as successful replay detection.

## Issue Context
Only a conflict while creating `refs/seen/...` proves that the delivery was previously processed. Restore the nested scope so `backend.create_commit()` remains outside the `CasConflict` handler, while `InfraError` continues to flow to the function-level infra channel.

## Fix Focus Areas
- arbiter/kernel.py[103-110]

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants