Skip to content

refactor: 拆分 run_subtask 为多个辅助函数 (Issue #3)#21

Merged
urika merged 2 commits into
mainfrom
fix/issue-3-run-subtask-split
May 30, 2026
Merged

refactor: 拆分 run_subtask 为多个辅助函数 (Issue #3)#21
urika merged 2 commits into
mainfrom
fix/issue-3-run-subtask-split

Conversation

@urika
Copy link
Copy Markdown
Owner

@urika urika commented May 29, 2026

Summary

修复 Issue #3: 拆分 run_subtask() 函数

修复

将 ~380 行的 run_subtask 拆分为5个辅助函数:

  • _create_worktree() - worktree 创建
  • _build_task_md() - TASK.md 构建和 Skill 注入
  • _run_claude() - Claude 执行
  • _verify_changes() - 变更验证和重试逻辑
  • _generate_context() - 下游共享上下文生成

测试

  • pytest tests/ -q (163 passed)

Checklist

  • 所有测试通过
  • 函数拆分完成

@urika
Copy link
Copy Markdown
Owner Author

urika commented May 29, 2026

Code Review — PR #21: 拆分 run_subtask 为多个辅助函数

🔴 Critical

1. _verify_changes 硬编码 issue_ref = "",commit 消息丢失 issue 引用

原代码使用 run_subtaskissue_ref 参数传入 _format_commit(subtask['title'], issue_ref, sub_id),但 _verify_changes 设置 issue_ref = "" 并注释 "Not available in this context"。调用方也没有传递 issue_ref_verify_changes

后果: git commit 消息将永远不包含 issue 引用(如 #3),破坏 commit 到 issue 的可追溯性。这是功能回归。

修复: 给 _verify_changes 添加 issue_ref="" 参数,由 run_subtask 传入。

🟡 Major

2. _verify_changeshas_changes 参数被立即覆盖

函数签名接受 has_changes 参数(调用方传 True),但函数体无条件执行 has_changes = bool(status_result.stdout.strip()) 覆盖了它。这个参数是死代码且具有误导性。

修复: 移除 has_changes 参数。

3. verification 变量传给 _generate_context 的值与原代码不同

原代码中,context 生成使用的 verification 来自 subtask.get("verification", "") — 原始未重写路径的值。重构后,传给 _generate_contextverification 来自调用方作用域中已经路径重写的版本。这意味着 context.md 中验证命令的路径会显示 worktree 路径而非原始 repo 路径。

修复: 在 _verify_changes 返回结果中同时返回原始 verification,或显式传递两个版本。

4. _build_task_md 接受 upstream_worktrees 参数但从未使用

函数签名包含 upstream_worktrees=None,但函数体只使用 merge_conflicts。调用方仍传递 upstream_worktrees=upstream_worktrees。这是无用的接口表面。

修复: 移除 upstream_worktrees 参数及对应的调用参数。

🟢 Minor

  1. _boundary_chars 正则模式在 _build_task_md()run_subtask() 中重复出现。应提取为共享辅助函数(如 _rewrite_paths(text, source, target)
  2. _generate_context 同时接受 subtasksub_id 参数,但 subtask 已包含 id,参数冗余
  3. sub_dir.mkdir(parents=True, exist_ok=True)_create_worktree()run_subtask() 中各调用一次,第二次是多余的
  4. _verify_changes 返回 9 个 key 的 dict,建议使用 dataclassNamedTuple 提供类型安全

总结: 需修复 Critical #1(issue_ref 丢失)和 Major #2-4 后方可合并。

@urika
Copy link
Copy Markdown
Owner Author

urika commented May 30, 2026

✅ Review 问题已修复

修复提交已推送,4 个问题全部解决:

  1. _verify_changes 添加 issue_ref="" 参数,run_subtask 传入 → commit 消息恢复 issue 引用
  2. ✅ 移除 _verify_changeshas_changes 死代码参数
  3. ✅ 保存原始 verification 值(original_verification),传原始值给 _generate_context
  4. ✅ 移除 _build_task_md 未使用的 upstream_worktrees 参数

测试结果:163 passed

jinsongwang and others added 2 commits May 30, 2026 09:28
拆分为: _create_worktree(), _build_task_md(), _run_claude(), _verify_changes(), _generate_context()

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1. _verify_changes 添加 issue_ref 参数,调用方传入(修复 commit 消息丢失 issue 引用)
2. 移除 _verify_changes 的 has_changes 死代码参数
3. 保存原始 verification 值传给 _generate_context(修复路径重写问题)
4. 移除 _build_task_md 未使用的 upstream_worktrees 参数

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@urika urika force-pushed the fix/issue-3-run-subtask-split branch from 67edd3a to 5512fed Compare May 30, 2026 01:29
@urika urika merged commit 12c6ba1 into main May 30, 2026
1 check failed
urika added a commit that referenced this pull request May 30, 2026
Rebased onto main (after PRs #20, #21), conflicts resolved.\n\nFixes:\n- executor: _verify_changes 新增 sub_id 参数\n- test: 验证命令改用 pytest --co (适配新安全白名单)\n- test: _is_safe_verification_command 断言适配 (bool, reason) 元组返回值\n\nTests: 275 passed.
urika added a commit that referenced this pull request May 30, 2026
…sue #7)

Rebased onto main (after PRs #20, #21, #27), conflicts resolved.

Key changes:
- 14 modules: type annotations added to all public functions
- cli.py: argparse code preserved from PR #20
- executor.py: refactored functions preserved from PR #21, security functions from PR #27
- Fixed: logger definitions in eval.py, tui.py, git_utils.py

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant