diff --git a/.github/aicodingflow-tests/test_product_docs_sync.py b/.github/aicodingflow-tests/test_product_docs_sync.py index 369d2cd..9466db5 100644 --- a/.github/aicodingflow-tests/test_product_docs_sync.py +++ b/.github/aicodingflow-tests/test_product_docs_sync.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import subprocess import tempfile import unittest from pathlib import Path @@ -62,6 +63,94 @@ def test_prepare_reads_linked_specs_and_product_docs(self) -> None: self.assertEqual(specs[0]["path"], "specs/issue-87/product.md") self.assertEqual(docs[0]["path"], "docs/product/raw/overview.md") + def test_fetch_existing_issues_skips_issue_view_failures(self) -> None: + def fetch_issue(_repo: str, number: int) -> dict: + if number == 999999: + raise subprocess.CalledProcessError(1, ["gh", "issue", "view", str(number)]) + return {"number": number, "title": f"Issue {number}"} + + with patch.object(prepare, "fetch_issue", side_effect=fetch_issue): + issues, skipped = prepare.fetch_existing_issues("owner/repo", [87, 999999, 88]) + + self.assertEqual([issue["number"] for issue in issues], [87, 88]) + self.assertEqual(skipped, [999999]) + + def test_main_skips_missing_linked_issue_and_reads_specs_for_existing_issues(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + context_output = root / "context.json" + markdown_output = root / "context.md" + diff_output = root / "diff.md" + existing_docs_output = root / "existing.md" + github_output = root / "github-output.txt" + (root / "specs/issue-87").mkdir(parents=True) + (root / "specs/issue-87/product.md").write_text("# Existing issue spec\n", encoding="utf-8") + (root / "specs/issue-999999").mkdir(parents=True) + (root / "specs/issue-999999/product.md").write_text("# Missing issue spec\n", encoding="utf-8") + pr = { + "number": 123, + "title": "Implement workflow", + "body": "Refs #87 and #999999", + "url": "https://example.test/pull/123", + "state": "MERGED", + "mergedAt": "2026-05-25T02:00:00Z", + "author": {"login": "maintainer"}, + "headRefName": "feature/docs", + "baseRefName": "main", + "mergeCommit": {"oid": "abc123"}, + "files": [], + "closingIssuesReferences": [], + "labels": [], + } + + def fetch_issue(_repo: str, number: int) -> dict: + if number == 999999: + raise subprocess.CalledProcessError(1, ["gh", "issue", "view", str(number)]) + return { + "number": number, + "title": "Existing issue", + "body": "Readable issue.", + "url": "https://example.test/issues/87", + "state": "OPEN", + "labels": [], + "comments": [], + } + + with patch.object(prepare.Path, "cwd", return_value=root): + with patch.object(prepare, "fetch_default_branch", return_value="main"): + with patch.object(prepare, "fetch_pr", return_value=pr): + with patch.object(prepare, "fetch_issue", side_effect=fetch_issue): + with patch.object(prepare, "fetch_pr_diff", return_value="diff"): + with patch( + "sys.argv", + [ + "prepare_product_docs_sync_context.py", + "--repo", + "owner/repo", + "--pr-number", + "123", + "--context-output", + str(context_output), + "--markdown-output", + str(markdown_output), + "--diff-output", + str(diff_output), + "--existing-docs-output", + str(existing_docs_output), + "--github-output", + str(github_output), + ], + ): + self.assertEqual(prepare.main(), 0) + + payload = json.loads(context_output.read_text(encoding="utf-8")) + markdown = markdown_output.read_text(encoding="utf-8") + self.assertEqual([issue["number"] for issue in payload["linked_issues"]], [87]) + self.assertEqual([spec["path"] for spec in payload["specs"]], ["specs/issue-87/product.md"]) + self.assertNotIn("## Issue #999999", markdown) + self.assertNotIn("specs/issue-999999/product.md", markdown) + self.assertIn("should_run=true", github_output.read_text(encoding="utf-8")) + def test_ledger_selects_first_unprocessed_pr(self) -> None: prs = [ {"number": 87, "title": "done", "url": "https://example.test/87", "mergedAt": "2026-05-25T01:00:00Z"}, diff --git a/.github/scripts/prepare_product_docs_sync_context.py b/.github/scripts/prepare_product_docs_sync_context.py index c07b57e..249857c 100644 --- a/.github/scripts/prepare_product_docs_sync_context.py +++ b/.github/scripts/prepare_product_docs_sync_context.py @@ -223,6 +223,17 @@ def fetch_issue(repo: str, number: int) -> dict[str, Any]: ) +def fetch_existing_issues(repo: str, numbers: list[int]) -> tuple[list[dict[str, Any]], list[int]]: + issues: list[dict[str, Any]] = [] + skipped: list[int] = [] + for number in numbers: + try: + issues.append(fetch_issue(repo, number)) + except subprocess.CalledProcessError: + skipped.append(number) + return issues, skipped + + def fetch_pr_diff(repo: str, pr_number: str, max_chars: int) -> str: diff = run_gh_text(["pr", "diff", pr_number, "--repo", repo, "--patch"]) if len(diff) <= max_chars: @@ -472,8 +483,14 @@ def main() -> int: return 0 numbers = issue_numbers(pr) - issues = [fetch_issue(args.repo, number) for number in numbers] - specs = read_specs(root, numbers) + issues, _skipped_issue_numbers = fetch_existing_issues(args.repo, numbers) + existing_issue_numbers = [] + for issue in issues: + try: + existing_issue_numbers.append(int(issue.get("number"))) + except (TypeError, ValueError): + continue + specs = read_specs(root, existing_issue_numbers) product_docs = read_existing_product_docs(root) should_run = "true" if pr.get("mergedAt") else "false" diff --git a/specs/issue-214/product.md b/specs/issue-214/product.md new file mode 100644 index 0000000..c1d0696 --- /dev/null +++ b/specs/issue-214/product.md @@ -0,0 +1,101 @@ +# Product Spec: Product Docs Sync 跳过不存在的 issue 引用 + +## 1. Summary + +Product Docs Sync 在准备 merged PR 的稳定上下文时,会收集 PR 关联的 issue,并把 issue 内容、评论和相关 specs 提供给后续 docs sync agent。当前当 PR 标题、正文或 `closingIssuesReferences` 中包含不存在的 issue 编号时,上下文准备会失败,导致整个 product docs sync 任务中断。 + +目标结果是:不存在或无法读取的 issue reference 不应阻止 Product Docs Sync 继续处理目标 PR。系统应跳过这些 missing issue reference,继续生成 PR 上下文、diff、现有产品文档上下文和可读取 issue 的上下文。 + +## 2. Problem + +维护者和 agent 经常在 PR 描述中使用 `Refs #...`、`Fixes #...`、`Closes #...` 等文本引用 issue。引用可能因为拼写错误、issue 被删除、跨仓库编号混淆或权限不可见而不存在。Product Docs Sync 的核心任务是评估已合并 PR 是否需要更新长期产品文档;一个无效 issue 引用不应让整个同步流程失败。 + +当前失败会造成两个问题: + +- docs sync workflow 不能为该 PR 生成上下文和决策。 +- ledger 不会记录处理结果,后续 scheduled run 可能重复卡在同一个 PR 上。 + +## 3. Goals + +- 当 PR 引用不存在的 issue 编号时,Product Docs Sync 继续运行。 +- 可读取的 linked issue 仍然出现在稳定上下文中。 +- 不存在或不可读取的 issue 不出现在 `linked_issues` 列表中。 +- 缺失 issue 不阻止读取相关 specs、现有 product docs 或 PR diff。 +- 缺失 issue 不改变 Product Docs Sync 对 PR 本身是否可处理的判断。 +- 行为可通过单元测试覆盖,避免后续回归。 + +## 4. Non-goals + +- 不修改 issue reference 的提取规则。 +- 不支持跨仓库 issue 引用解析。 +- 不创建、修复、重命名或关闭任何 GitHub issue。 +- 不改变 Product Docs Sync 的 docs update 决策标准、ledger schema、PR 创建策略或 Codex prompt。 +- 不把缺失 issue 视为 docs update required。 +- 不扩大 workflow 权限或写入范围。 + +## 5. Figma / design references + +Figma: none provided。该变更是 GitHub Actions 自动化脚本的容错行为,没有 UI 或交互设计输入。 + +## 6. User experience + +### 默认行为 + +- 当 Product Docs Sync 处理一个 merged PR 时,仍然从 PR 的 `closingIssuesReferences`、标题和正文中提取 issue 编号。 +- 如果某个 issue 编号可以通过 GitHub API 读取,该 issue 仍然作为 linked issue 写入稳定上下文。 +- 如果某个 issue 编号不存在或 GitHub CLI 返回找不到资源的错误,该编号应被跳过。 +- 跳过 missing issue 后,workflow 应继续生成: + - `product-docs-sync-context.json` + - `product-docs-sync-context.md` + - `product-docs-sync-diff.md` + - `product-docs-existing.md` +- 后续 docs sync agent 应收到缺失 issue 之外的全部可用上下文,并继续做 `required`、`uncertain` 或 `not-needed` 决策。 + +### 混合引用 + +- 如果 PR 同时引用存在和不存在的 issue,存在的 issue 必须保留。 +- 不存在的 issue 不应导致存在 issue 的 specs 被丢失。 +- 输出中的 `linked_issues` 只包含成功读取的 issue,不能包含空对象、错误字符串或部分填充的占位数据。 + +### 全部引用缺失 + +- 如果 PR 提取出的 issue 编号全部缺失,但 PR 本身可读取且符合处理条件,Product Docs Sync 仍然继续。 +- 此时 `linked_issues` 应为空列表,specs 也只应基于可确认存在的 issue 编号读取。 +- workflow 不应把“没有可读取 linked issue”当作 `should_run=false`。 + +### 错误边界 + +- 只有 issue reference 缺失或不可读取这一类 linked issue fetch 失败应被容错跳过。 +- 获取 PR、搜索 merged PR、读取 diff、写上下文文件、解析 ledger 等非 issue-reference 错误仍应按现有失败行为暴露。 +- 权限、网络或 GitHub CLI 临时故障可能与 missing issue 表现相似;第一版可保守地只跳过 GitHub CLI 对单个 issue view 的失败,同时通过日志或上下文可见性保留排查空间。 + +## 7. Success criteria + +- PR 正文包含 `Refs #999999` 这类不存在 issue 引用时,Product Docs Sync context preparation 不会中断。 +- PR 同时引用存在的 issue 和不存在的 issue 时,生成的 `linked_issues` 只包含存在的 issue。 +- `product-docs-sync-context.md` 中只渲染可读取 issue 的详情。 +- related specs 只从成功读取或确认可用的 issue 编号对应目录读取,避免因为 missing issue 产生无效 spec 依赖。 +- `should_run` 仍由 PR 是否可处理决定,不由 linked issue 是否全部可读取决定。 +- 单元测试覆盖 missing issue reference 被跳过且上下文仍写出的场景。 + +## 8. Validation + +- 增加 Product Docs Sync 脚本单元测试,模拟一个 PR 引用存在 issue 和缺失 issue,确认缺失 issue 不会让准备流程失败。 +- 增加或更新测试断言,确认输出 payload 中只包含成功读取的 linked issue。 +- 运行 Product Docs Sync 相关窄测试: + +```bash +PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s .github/aicodingflow-tests -p 'test_product_docs_sync.py' +``` + +- 如果实现触及共享 helper 或 workflow contract,再运行完整测试: + +```bash +PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s .github/aicodingflow-tests +``` + +- 运行 `git diff --check`。 + +## 9. Open questions + +- 是否需要在 markdown context 或 JSON context 中显式记录 skipped issue 编号,方便维护者排查错误引用。第一版建议只跳过,不新增上下文字段,除非实现阶段发现现有日志不足以定位问题。 diff --git a/specs/issue-214/tech.md b/specs/issue-214/tech.md new file mode 100644 index 0000000..d454178 --- /dev/null +++ b/specs/issue-214/tech.md @@ -0,0 +1,140 @@ +# Tech Spec: Product Docs Sync missing issue reference 容错 + +## 1. Problem + +`.github/scripts/prepare_product_docs_sync_context.py` 在准备 Product Docs Sync 上下文时,会从目标 PR 提取 issue 编号,然后对每个编号调用 `fetch_issue()`。`fetch_issue()` 通过 `gh issue view` 获取 issue JSON;当 PR 引用了不存在或不可读取的 issue 时,GitHub CLI 返回非零状态,`subprocess.run(check=True)` 抛出异常,脚本中断。 + +技术目标是在 linked issue fetch 路径上对单个 missing issue reference 做容错:跳过失败的 issue,继续为目标 PR 生成稳定上下文,同时保留其他错误路径的现有失败语义。 + +## 2. Relevant code + +- `.github/scripts/prepare_product_docs_sync_context.py:25` — `run_gh_json()` 使用 `subprocess.run(..., check=True)` 调用 GitHub CLI。 +- `.github/scripts/prepare_product_docs_sync_context.py:212` — `fetch_issue()` 封装 `gh issue view `,当前不捕获失败。 +- `.github/scripts/prepare_product_docs_sync_context.py:233` — `issue_numbers()` 从 `closingIssuesReferences`、PR 标题和正文中的引用关键词提取去重 issue 编号。 +- `.github/scripts/prepare_product_docs_sync_context.py:296` — `read_specs()` 根据 issue 编号读取 `specs/issue-/product.md` 和 `tech.md`。 +- `.github/scripts/prepare_product_docs_sync_context.py:318` — `write_context_json()` 把 `linked_issues` 和 specs 写入 JSON 上下文。 +- `.github/scripts/prepare_product_docs_sync_context.py:342` — `write_markdown()` 渲染 linked issue 详情。 +- `.github/scripts/prepare_product_docs_sync_context.py:474` — `main()` 当前通过 `[fetch_issue(args.repo, number) for number in numbers]` 一次性读取全部 issue;任意失败都会中断。 +- `.github/aicodingflow-tests/test_product_docs_sync.py:36` — Product Docs Sync 脚本测试类,已有 issue 编号提取、ledger、workflow 和 validator 覆盖。 + +## 3. Current state + +当前数据流: + +1. `main()` 选择待处理 PR。 +2. `issue_numbers(pr)` 返回去重后的 issue 编号列表。 +3. `issues = [fetch_issue(args.repo, number) for number in numbers]` 逐个读取 linked issue。 +4. `read_specs(root, numbers)` 根据原始编号列表读取 specs。 +5. `write_context_json()` 和 `write_markdown()` 写出 PR、linked issue、specs 和 product docs 上下文。 + +失败点在第 3 步:`fetch_issue()` 没有区分“单个 linked issue 不存在”和“脚本无法继续”的错误。由于列表推导没有局部异常处理,任何 `gh issue view` 失败都会阻止后续 diff、docs 和 ledger 流程。 + +## 4. Proposed changes + +### 新增容错读取 helper + +在 `.github/scripts/prepare_product_docs_sync_context.py` 中增加一个小 helper,例如: + +```python +def fetch_existing_issues(repo: str, numbers: list[int]) -> tuple[list[dict[str, Any]], list[int]]: + issues: list[dict[str, Any]] = [] + skipped: list[int] = [] + for number in numbers: + try: + issues.append(fetch_issue(repo, number)) + except subprocess.CalledProcessError: + skipped.append(number) + return issues, skipped +``` + +实现可按现有脚本风格调整命名,但应满足: + +- 只包住单个 `fetch_issue()` 调用。 +- 捕获 `subprocess.CalledProcessError`,因为这是 `run_gh_json()` 当前对 GitHub CLI 非零退出的实际异常。 +- 不捕获 JSON 解析错误、文件写入错误、ledger 错误或其他脚本 bug。 +- 返回成功读取的 issue 列表,并可返回 skipped 编号供调试或后续扩展使用。 + +### 调整 main 数据流 + +把当前列表推导替换为容错 helper: + +```python +numbers = issue_numbers(pr) +issues, skipped_issue_numbers = fetch_existing_issues(args.repo, numbers) +existing_issue_numbers = [int(issue.get("number")) for issue in issues if issue.get("number")] +specs = read_specs(root, existing_issue_numbers) +``` + +关键边界: + +- `issues` 只包含成功读取的 issue。 +- `read_specs()` 应基于成功读取的 issue 编号,而不是原始 `numbers`,避免 missing issue 编号意外拉入本地同名旧 specs。 +- 如果没有成功读取任何 issue,`issues` 和 `specs` 都可以为空列表,workflow 仍继续。 +- `should_run` 仍保持 `true if pr.get("mergedAt") else "false"` 的现有逻辑。 + +`skipped_issue_numbers` 第一版可以只用于简短 stderr/stdout 日志,避免扩大 JSON context schema。如果选择写入日志,保持纯诊断性质,不影响输出 contract。 + +### 保持不变 + +不修改以下行为: + +- `issue_numbers()` 的关键词和去重规则。 +- `fetch_pr()`、`fetch_merged_prs()`、`fetch_pr_diff()` 的失败语义。 +- `write_context_json()` 的顶层 schema。 +- `write_markdown()` 对 linked issue 的渲染格式。 +- Product Docs Sync workflow YAML、Codex prompt、permissions、ledger schema 和 allowed write roots。 + +## 5. End-to-end flow + +1. Product Docs Sync 选中一个 merged PR。 +2. `issue_numbers(pr)` 从 PR metadata 和正文中提取编号,例如 `[214, 999999]`。 +3. `fetch_existing_issues()` 逐个调用 `fetch_issue()`。 +4. issue `214` 成功读取后进入 `issues`。 +5. issue `999999` 的 `gh issue view` 返回非零状态,被记录为 skipped 并继续循环。 +6. `read_specs()` 只读取成功 issue 编号对应的 specs。 +7. JSON 和 markdown context 正常写出,其中 `linked_issues` 只包含 issue `214`。 +8. 后续 diff、existing docs、GitHub outputs、Codex docs sync 和 ledger 流程按现有逻辑继续。 + +## 6. Risks and mitigations + +- 风险:捕获过宽导致真实 GitHub CLI 故障被静默跳过。 + - 缓解:只在 linked issue fetch helper 中捕获 `subprocess.CalledProcessError`;其他阶段仍失败。 +- 风险:GitHub CLI 因认证或网络问题读取所有 issue 都失败,workflow 继续但上下文缺少 issue。 + - 缓解:当前需求明确要求跳过 missing issue reference;实现可输出 skipped 编号诊断,后续如需区分 exit code 或 stderr 可另开增强。 +- 风险:仍按原始 issue 编号读取 specs,会把不存在 issue 的本地 spec 加进上下文。 + - 缓解:`read_specs()` 改用成功读取 issue 的编号列表。 +- 风险:新增 skipped issue 字段破坏下游 context consumer。 + - 缓解:第一版不改变 `write_context_json()` schema,只调整内部读取流程。 +- 风险:测试只覆盖 helper,不覆盖 main 写出路径。 + - 缓解:增加至少一个围绕 `main()` 或上下文写出路径的测试,断言脚本继续写文件且 linked issues 过滤正确。 + +## 7. Testing and validation + +在 `.github/aicodingflow-tests/test_product_docs_sync.py` 增加测试覆盖: + +- `fetch_existing_issues()` 在第二个 issue 抛出 `subprocess.CalledProcessError` 时返回第一个 issue,并记录 skipped 编号。 +- `main()` 或接近 main 的集成式测试中,构造 PR 引用两个 issue,mock `fetch_issue` 对其中一个编号抛出 `CalledProcessError`,确认: + - 脚本返回成功。 + - `product-docs-sync-context.json` 存在。 + - `linked_issues` 只包含成功读取的 issue。 + - specs 只读取成功 issue 编号对应目录。 + - `should_run` 输出仍由 merged PR 状态决定。 + +推荐运行: + +```bash +PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s .github/aicodingflow-tests -p 'test_product_docs_sync.py' +PYTHONPYCACHEPREFIX=/tmp/aicodingflow-pycache python3 -m py_compile .github/scripts/prepare_product_docs_sync_context.py +git diff --check +``` + +如果实现过程中调整了共享测试 helper 或 workflow contract,再运行完整 suite: + +```bash +PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s .github/aicodingflow-tests +``` + +## 8. Follow-ups + +- 如果维护者需要更强可观测性,可在后续 issue 中为 context JSON 增加 `skipped_issue_references` 字段,并同步更新 docs sync prompt 和 tests。 +- 如果需要区分 not found、权限不足、rate limit、网络错误,可后续扩展 `run_gh_json()` 返回 stderr 或引入更细粒度错误分类。