fix(skills): 修复 issue review skill 评审问题

- 修复 issue-review 脚本的代理回退、GitHub Token 认证与 JSON 输出契约

- 调整非 bug issue 的澄清判定并补充 docs、feature 分诊回归测试

- 更新 skill 示例占位符与 ai-plan 跟踪记录,收敛 PR #328 follow-up
This commit is contained in:
gewuyou 2026-05-06 16:25:29 +08:00
parent ab9829044f
commit f25ccccad2
5 changed files with 174 additions and 29 deletions

View File

@ -34,11 +34,11 @@ Shortcut: `$gframework-issue-review`
- Default:
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py`
- Force a specific issue:
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --issue 312`
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --issue <issue-number>`
- Machine-readable output:
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --format json`
- Write machine-readable output to a file instead of stdout:
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --issue 312 --format json --json-output /tmp/issue312-review.json`
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --issue <issue-number> --format json --json-output /tmp/issue-review.json`
- Inspect only a high-signal section:
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --section summary`
- Combine triage with a boot handoff:
@ -79,5 +79,5 @@ The script should produce:
- `Use $gframework-issue-review on the current repository issue`
- `Check the open GitHub issue and summarize what should be verified locally`
- `Inspect issue 312 and tell me whether this looks like bug triage or a feature request`
- `Inspect issue <issue-number> and tell me whether this looks like bug triage or a feature request`
- `先用 $gframework-issue-review 看当前 open issue再用 $gframework-boot 继续`

View File

@ -1,4 +1,7 @@
#!/usr/bin/env python3
# Copyright (c) 2025-2026 GeWuYou
# SPDX-License-Identifier: Apache-2.0
"""
Fetch the current GFramework GitHub issue and extract the signals needed for
local follow-up work without relying on gh CLI.
@ -14,17 +17,19 @@ import re
import shutil
import subprocess
import sys
import urllib.error
import urllib.request
from typing import Any
OWNER = "GeWuYou"
REPO = "GFramework"
WORKTREE_ROOT_DIRECTORY_NAME = "GFramework-WorkTree"
DEFAULT_WINDOWS_GIT = "/mnt/d/Tool/Development Tools/Git/cmd/git.exe"
GIT_ENVIRONMENT_KEY = "GFRAMEWORK_WINDOWS_GIT"
GIT_DIR_ENVIRONMENT_KEY = "GFRAMEWORK_GIT_DIR"
WORK_TREE_ENVIRONMENT_KEY = "GFRAMEWORK_WORK_TREE"
REQUEST_TIMEOUT_ENVIRONMENT_KEY = "GFRAMEWORK_ISSUE_REVIEW_TIMEOUT_SECONDS"
GITHUB_TOKEN_ENVIRONMENT_KEYS = ("GFRAMEWORK_GITHUB_TOKEN", "GITHUB_TOKEN", "GH_TOKEN")
PROXY_ENVIRONMENT_KEYS = ("http_proxy", "https_proxy", "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "all_proxy")
DEFAULT_REQUEST_TIMEOUT_SECONDS = 60
USER_AGENT = "codex-gframework-issue-review"
DISPLAY_SECTION_CHOICES = (
@ -100,7 +105,6 @@ def resolve_git_command() -> str:
"""Resolve the git executable to use for this repository."""
candidates = [
os.environ.get(GIT_ENVIRONMENT_KEY),
DEFAULT_WINDOWS_GIT,
"git.exe",
"git",
]
@ -195,14 +199,58 @@ def get_current_branch() -> str:
return run_command([*resolve_git_invocation(), "rev-parse", "--abbrev-ref", "HEAD"])
def open_url(url: str, accept: str) -> tuple[str, Any]:
"""Open a URL with proxy variables disabled and return decoded text plus headers."""
opener = urllib.request.build_opener(urllib.request.ProxyHandler({}))
request = urllib.request.Request(url, headers={"Accept": accept, "User-Agent": USER_AGENT})
def resolve_github_token() -> str | None:
"""Return the first configured GitHub token for authenticated API requests."""
for environment_key in GITHUB_TOKEN_ENVIRONMENT_KEYS:
token = os.environ.get(environment_key)
if token:
return token
return None
def build_request_headers(accept: str) -> dict[str, str]:
"""Build GitHub request headers and include auth when a token is available."""
headers = {"Accept": accept, "User-Agent": USER_AGENT}
token = resolve_github_token()
if token:
headers["Authorization"] = f"Bearer {token}"
return headers
def has_proxy_environment() -> bool:
"""Return whether the current process is configured to use an outbound proxy."""
return any(os.environ.get(environment_key) for environment_key in PROXY_ENVIRONMENT_KEYS)
def perform_request(url: str, headers: dict[str, str], *, disable_proxy: bool) -> tuple[str, Any]:
"""Execute a single HTTP request and return decoded text plus response headers."""
opener = (
urllib.request.build_opener(urllib.request.ProxyHandler({}))
if disable_proxy
else urllib.request.build_opener()
)
request = urllib.request.Request(url, headers=headers)
with opener.open(request, timeout=resolve_request_timeout_seconds()) as response:
return response.read().decode("utf-8", "replace"), response.headers
def open_url(url: str, accept: str) -> tuple[str, Any]:
"""Open a URL, retrying without proxies only when the configured proxy path fails."""
headers = build_request_headers(accept)
try:
return perform_request(url, headers, disable_proxy=False)
except urllib.error.HTTPError:
raise
except (urllib.error.URLError, TimeoutError, OSError):
if not has_proxy_environment():
raise
return perform_request(url, headers, disable_proxy=True)
def fetch_json(url: str, accept: str = "application/vnd.github+json") -> tuple[Any, Any]:
"""Fetch a JSON payload and its response headers from GitHub."""
text, headers = open_url(url, accept=accept)
@ -491,18 +539,31 @@ def build_references(issue: dict[str, Any], comments: list[dict[str, Any]], even
}
def build_information_flags(issue: dict[str, Any], comments: list[dict[str, Any]]) -> dict[str, bool]:
"""Derive missing-information and readiness flags from the issue discussion."""
def build_information_flags(
issue: dict[str, Any],
comments: list[dict[str, Any]],
issue_type_candidates: list[str],
) -> dict[str, bool]:
"""Derive missing-information and readiness flags with issue-type-aware heuristics."""
text_blocks = gather_text_blocks(issue, comments)
has_reproduction_steps = has_any_pattern(text_blocks, REPRODUCTION_PATTERNS)
has_expected_behavior = has_any_pattern(text_blocks, EXPECTED_BEHAVIOR_PATTERNS)
has_actual_behavior = has_any_pattern(text_blocks, ACTUAL_BEHAVIOR_PATTERNS)
has_environment_details = has_any_pattern(text_blocks, ENVIRONMENT_PATTERNS)
has_acceptance_signals = has_any_pattern(text_blocks, ACCEPTANCE_PATTERNS)
needs_clarification = not (
(has_actual_behavior and (has_reproduction_steps or has_environment_details))
or has_acceptance_signals
)
primary_issue_type = issue_type_candidates[0] if issue_type_candidates else "bug"
if primary_issue_type == "bug":
needs_clarification = not (
(has_actual_behavior and (has_reproduction_steps or has_environment_details))
or has_acceptance_signals
)
elif primary_issue_type in {"feature", "docs"}:
needs_clarification = not (has_expected_behavior or has_acceptance_signals)
elif primary_issue_type == "maintenance":
needs_clarification = not (has_expected_behavior or has_actual_behavior or has_acceptance_signals)
else:
needs_clarification = not (has_expected_behavior or has_actual_behavior or has_acceptance_signals)
return {
"has_reproduction_steps": has_reproduction_steps,
@ -544,7 +605,7 @@ def build_triage_hints(issue: dict[str, Any], comments: list[dict[str, Any]]) ->
"""Build lightweight, reviewable triage hints for boot follow-up."""
text_blocks = gather_text_blocks(issue, comments)
issue_type_candidates = choose_issue_type_candidates(issue, text_blocks)
information_flags = build_information_flags(issue, comments)
information_flags = build_information_flags(issue, comments, issue_type_candidates)
affected_topics = choose_affected_topics(issue, comments)
next_action = choose_next_action(information_flags, issue_type_candidates, affected_topics)
@ -776,10 +837,6 @@ def main() -> None:
json_output_path = write_json_output(result, args.json_output)
if args.format == "json":
if json_output_path:
print(json_output_path)
return
print(json.dumps(result, ensure_ascii=False, indent=2))
return

View File

@ -1,4 +1,7 @@
#!/usr/bin/env python3
# Copyright (c) 2025-2026 GeWuYou
# SPDX-License-Identifier: Apache-2.0
"""Regression tests for the GFramework issue review fetch helper."""
from __future__ import annotations
@ -51,5 +54,41 @@ class ExtractReferencesFromTextTests(unittest.TestCase):
self.assertEqual(references["file_paths"], ["GFramework.Core/Systems/Runner.cs"])
class BuildTriageHintsTests(unittest.TestCase):
"""Cover next-action classification for non-bug issue flows."""
def test_build_triage_hints_routes_docs_issue_to_docs_topic_without_bug_style_clarification(self) -> None:
"""Docs issues with a clear requested change should not be forced through bug-style clarification."""
triage_hints = MODULE.build_triage_hints(
{
"title": "Update documentation landing page",
"labels": ["docs"],
"body": "The guide should explain the landing-page layout for new contributors.",
},
[],
)
self.assertEqual(triage_hints["issue_type_candidates"][0], "docs")
self.assertEqual(triage_hints["affected_active_topics"], [])
self.assertFalse(triage_hints["information_flags"]["needs_clarification"])
self.assertEqual(triage_hints["next_action"], "start-new-docs-topic-with-boot")
def test_build_triage_hints_routes_feature_issue_to_new_topic_when_request_is_clear(self) -> None:
"""Feature requests with explicit desired behavior should stay actionable without fake bug repro gates."""
triage_hints = MODULE.build_triage_hints(
{
"title": "Support release note previews",
"labels": ["enhancement"],
"body": "The workflow should support previewing generated notes before completion.",
},
[],
)
self.assertEqual(triage_hints["issue_type_candidates"][0], "feature")
self.assertEqual(triage_hints["affected_active_topics"], [])
self.assertFalse(triage_hints["information_flags"]["needs_clarification"])
self.assertEqual(triage_hints["next_action"], "start-new-topic-with-boot")
if __name__ == "__main__":
unittest.main()

View File

@ -13,12 +13,12 @@
## 当前恢复点
- 恢复点编号:`ISSUE-SKILL-RP-001`
- 当前阶段:`Phase 2`
- 恢复点编号:`ISSUE-SKILL-RP-002`
- 当前阶段:`Phase 3`
- 当前焦点:
- 保持 `$gframework-issue-review` 可供后续 issue 分诊直接复用
- 通过 `$gframework-boot` 继续 issue `#327` 的澄清优先处理路径
- 若后续 issue 数量从 `1` 变为 `0``>1`,要求显式传 `--issue`
- 收敛 PR #328 上仍然有效的 AI review 评论,避免新 skill 在仓库中留下已知漂移
- 保持 `$gframework-issue-review` 的 GitHub API 抓取在代理、认证与 JSON CLI 契约上更稳健
- 确保非 bug issue 的 triage 结果不会被错误导向 `clarify-issue-before-code`
### 已知风险
@ -28,6 +28,8 @@
- 缓解措施:脚本明确报错并要求 `--issue <number>`,验证时同时保留显式 issue 号路径
- issue 文本中的模块归因和处理建议只能是启发式结果,不能替代本地代码验证
- 缓解措施skill 文档明确要求后续仍通过 `$gframework-boot` 与本地源码核实
- GitHub API 仍可能在无 token 环境下命中匿名 rate limit
- 缓解措施:脚本现已支持从 `GFRAMEWORK_GITHUB_TOKEN``GITHUB_TOKEN``GH_TOKEN` 读取认证;无 token 时保持匿名降级
## 已完成
@ -46,24 +48,35 @@
- 支持“仅当当前仓库恰好一个 open issue 时自动解析,否则要求显式传号”
- 已修正新脚本在当前 WSL 会话下误回退到 `git.exe` 的兼容问题:
- 在主仓库根目录且存在 Linux `git` 时,也优先绑定 `--git-dir` / `--work-tree`
- 已根据 PR #328 review 收敛仍然有效的问题:
- 为 `fetch_current_issue_review.py` 与回归测试补齐 shebang 后 license header
- 去掉开发机特定的 Windows Git 绝对路径回退,改为环境变量覆盖 + `git.exe` / `git`
- GitHub 请求先走环境代理,并在代理请求失败且检测到代理环境变量时再无代理重试
- 支持通过标准 token 环境变量附带 `Authorization` 头,避免高频运行时过早命中匿名限流
- 将 `needs_clarification` 改为按 issue 主类型分支,避免 feature / docs issue 被 bug 规则误判
- 修正 `--format json --json-output` 时 stdout 仍输出 JSON文件写入只作为附加副作用
- 补充 docs / feature 场景回归测试,并将 skill 示例 issue 号改为占位符
## 验证
- `python3 scripts/license-header.py --check`
- 结果:通过
- 备注:本轮修改涉及的受支持文件均包含 Apache-2.0 license header
- `python3 .agents/skills/gframework-issue-review/scripts/test_fetch_current_issue_review.py`
- 结果:通过
- 备注:`3` 个脚本级测试全部通过
- 备注:`5` 个脚本级测试全部通过,新增 docs / feature 分诊回归覆盖
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --section summary --section warnings`
- 结果:通过
- 备注:真实 GitHub API 抓取成功,自动解析到当前唯一 open issue `#327`
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --format json --json-output /tmp/gframework-open-issue-review.json`
- 结果:通过
- 备注:JSON 文件成功写出,`resolution_mode=auto-single-open-issue``next_action=clarify-issue-before-code`
- 备注:stdout 输出 JSON文件也成功写出显式抓取 `#327``next_action=clarify-issue-before-code`
- `dotnet build GFramework.sln -c Release`
- 结果:通过
- 备注:`0 Warning(s)``0 Error(s)`
## 下一步
1. 使用 `$gframework-issue-review` 重新抓取或显式抓取目标 issue并把 triage 结果带入 `$gframework-boot`
2. 针对 issue `#327` 先执行“澄清优先”路径,再决定是否创建新的代码改动 topic
1. 将本轮 PR review 修复提交到当前分支,并回到 PR 线程确认相关评论是否可关闭
2. 需要继续处理 issue `#327` 时,重新用 `$gframework-issue-review` 抓取目标 issue并把结果带入 `$gframework-boot`
3. 若后续需要更细的 issue 事件语义,再补强 timeline 解析与脚本级回归测试

View File

@ -46,3 +46,39 @@
- `comment_count=0`
- `next_action=clarify-issue-before-code`
- `affected_active_topics=cqrs-rewrite`
### 阶段PR review 跟进修复ISSUE-SKILL-RP-002
- 使用 `$gframework-pr-review` 抓取当前分支 PR #328 后,确认以下评论在本地代码中仍然有效:
- `fetch_current_issue_review.py` 和回归测试缺少 shebang 后 license header
- issue-review 脚本仍保留开发机特定的 Windows Git 绝对路径回退
- `open_url()` 无条件禁用代理,且未支持 GitHub token 认证
- `build_information_flags()` 仍把 bug 场景的澄清门槛套用到 feature / docs issue
- `--format json --json-output` 组合时 stdout 只输出路径而不是 JSON
- skill 文档命令和示例中把 issue 号 `312` 写死
- 已在活跃 topic 下同步恢复点:
- 跟踪文件更新为 `ISSUE-SKILL-RP-002`
- 记录本轮修复范围、验证待办与后续恢复入口
- 已落盘修复:
- 为 issue-review 脚本和测试补齐 license header
- 将 GitHub 请求改为“先按环境代理请求,代理失败再无代理重试”
- 支持 `GFRAMEWORK_GITHUB_TOKEN` / `GITHUB_TOKEN` / `GH_TOKEN`
- 将 triage 澄清逻辑改为按主 issue 类型分支
- 为 docs / feature issue 增加 next-action 回归测试
- 将 skill 示例 issue 号改为占位符
- 让 `--format json --json-output` 同时保留 stdout JSON 与落盘副作用
- 已完成验证:
- `python3 scripts/license-header.py --check`
- `python3 .agents/skills/gframework-issue-review/scripts/test_fetch_current_issue_review.py`
- `python3 .agents/skills/gframework-issue-review/scripts/fetch_current_issue_review.py --issue 327 --format json --json-output /tmp/gframework-open-issue-review.json`
- `dotnet build GFramework.sln -c Release`
- 本轮验证结论:
- license header 检查通过
- 脚本级测试 `5/5` 通过
- `--format json --json-output` 现在会同时输出 stdout JSON 并写出 JSON 文件
- 仓库 Release build 通过,`0 Warning(s)` / `0 Error(s)`
### 下一步
1. 按仓库规范提交本轮 PR review 修复
2. 需要继续跟进 issue `#327` 时,再切回 `$gframework-issue-review` + `$gframework-boot` 路径