From f96acaaa75ff18f589f116ef30c5aad5925f2974 Mon Sep 17 00:00:00 2001 From: Sergey <93754860+slaid098@users.noreply.github.com> Date: Sat, 25 Jul 2026 03:55:50 +0300 Subject: [PATCH] fix(tools): drop hardcoded --repo in post-review/post-docs-review (#61) * fix(tools): drop hardcoded --repo in post-review/post-docs-review * fix(skills): sync run-pipeline templates to use review tools * fix(agents): add tool failure handling to reviewer/docs-reviewer * docs(adr): supersede ADR-019 with hardcoded repo removal note * docs(handoff): scaffold handoff and ADR for PR * docs(handoff): set PR number * fix(ci): reformat post-review/post-docs-review test asserts for ruff format --------- Co-authored-by: opencode-agent --- .opencode/agents/docs-reviewer.md | 8 +++++ .opencode/agents/reviewer.md | 8 +++++ .opencode/skills/run-pipeline/SKILL.md | 22 ++++++++---- .opencode/tools/post-docs-review.ts | 2 +- .opencode/tools/post-review.ts | 2 +- .../019-pr-46-review-posting-tools.md | 14 ++++++++ ...-61-drop-hardcoded-repo-in-review-tools.md | 36 +++++++++++++++++++ ...-61-drop-hardcoded-repo-in-review-tools.md | 33 +++++++++++++++++ tests/test_post_docs_review_tool.py | 24 +++++++++---- tests/test_post_docs_review_tool.ts | 11 +++--- tests/test_post_review_tool.py | 24 +++++++++---- tests/test_post_review_tool.ts | 11 +++--- 12 files changed, 167 insertions(+), 28 deletions(-) create mode 100644 docs/decisions/025-pr-61-drop-hardcoded-repo-in-review-tools.md create mode 100644 docs/handoff/pr-61-drop-hardcoded-repo-in-review-tools.md diff --git a/.opencode/agents/docs-reviewer.md b/.opencode/agents/docs-reviewer.md index b478759..2b4e2c4 100644 --- a/.opencode/agents/docs-reviewer.md +++ b/.opencode/agents/docs-reviewer.md @@ -225,6 +225,14 @@ Rules: 3. The comment heading `## Docs Review Summary` is guaranteed by the `post_docs_review` tool — `check_docs` matches regex `Docs Review` (case-insensitive). 4. Never skip the comment, even on edge cases — use `Verdict: NO_CHANGES` instead of silence. +## Tool failure handling + +If `post_docs_review` returns a string starting with `⚠️ ...failed` (e.g. `⚠️ post-docs-review failed for PR #N (exit 1): ...`): +- **СООБЩИ оркестратору о сбое tool и STOP.** Не продолжай молча, не пытайся fallback на raw `gh pr comment` через bash. +- Причина сбоя обычно: gh не аутентифицирован, PR не найден в текущем репо (cwd не git-репо или нет origin remote), или network error. +- Возвращай текст вида: `⚠️ post_docs_review tool failed: <сообщение от tool>. Pipeline заблокирован на DOCS phase — требуется вмешательство.` +- Любой дальнейший tool call после сбоя = protocol violation. + ## Rules 1. ALWAYS checkout the PR branch first. diff --git a/.opencode/agents/reviewer.md b/.opencode/agents/reviewer.md index 9a56163..4ec4a8f 100644 --- a/.opencode/agents/reviewer.md +++ b/.opencode/agents/reviewer.md @@ -282,6 +282,14 @@ Body format (without heading — tool adds `## Code Review Summary` and `### Ver Do NOT attempt merge. Stop and wait for discussion. After this call, you MUST respond with your review text only. Do NOT call any more tools. +## Tool failure handling + +If `post_review` returns a string starting with `⚠️ ...failed` (e.g. `⚠️ post-review failed for PR #N (exit 1): ...`): +- **СООБЩИ оркестратору о сбое tool и STOP.** Не продолжай молча, не пытайся fallback на raw `gh pr comment` через bash. +- Причина сбоя обычно: gh не аутентифицирован, PR не найден в текущем репо (cwd не git-репо или нет origin remote), или network error. +- Возвращай текст вида: `⚠️ post_review tool failed: <сообщение от tool>. Pipeline заблокирован на REVIEW phase — требуется вмешательство.` +- Любой дальнейший tool call после сбоя = protocol violation (как и после успешного `post_review`). + ## Severity Levels | Level | Meaning | Action | diff --git a/.opencode/skills/run-pipeline/SKILL.md b/.opencode/skills/run-pipeline/SKILL.md index 627d85b..5889e7a 100644 --- a/.opencode/skills/run-pipeline/SKILL.md +++ b/.opencode/skills/run-pipeline/SKILL.md @@ -73,9 +73,14 @@ Review PR#M в текущем репо (pre-merge, режим docs). "docs: update project map + handoff + ADR" && git push`. 8. **ВСЕГДА** оставь PR comment (даже если structural changes нет) — это детерминированный marker для `check_docs` в pipeline-status.py. Без comment - pipeline блокируется на DOCS phase. - `gh pr comment M --body "## Docs Review Summary\n- Project map: ...\n- Handoff: ...\n- ADR: ...\n\n### Verdict: APPROVE|FIXED|NO_CHANGES"`. - Heading `## Docs Review Summary` — обязательно (regex `Docs Review`). + pipeline блокируется на DOCS phase. Используй tool `post_docs_review` (НЕ + raw bash-вызов `gh`) — tool гарантирует heading `## Docs Review Summary` и + verdict-enum (zod), формат который парсит `check_docs`. + `post_docs_review({ pr_number: M, verdict: "APPROVE|FIXED|NO_CHANGES", body: "- Project map: ...\n- Handoff: ...\n- ADR: ..." })`. + Heading `## Docs Review Summary` — обязательно (regex `Docs Review`), + tool добавляет его автоматически — НЕ форматируй heading/verdict вручную. + Если tool вернул строку начинающуюся с `⚠️ ...failed` — СООБЩИ оркестратору + о сбое и STOP (не fallback на raw bash). ``` ### Template C (code_review) @@ -88,9 +93,14 @@ Review PR#M в текущем репо. через `skill("")`. 4. Проверь: code quality, architecture, error handling, security, testing, duplication, project-specific rules, PR hygiene, handoff/ADR (quick check). -5. Оставь review как PR comment (НЕ `gh pr review --approve` — GitHub блокирует - self-approve): - `gh pr comment M --body "## Code Review Summary\n...\n### Verdict: APPROVE|REQUEST_CHANGES"`. +5. Оставь review через tool `post_review` (НЕ `gh pr review --approve` — + GitHub блокирует self-approve; НЕ raw bash-вызов `gh`) — tool гарантирует + heading `## Code Review Summary` + verdict-enum которые парсит + `pipeline-status.py`: + `post_review({ pr_number: M, verdict: "APPROVE|REQUEST_CHANGES|NEEDS_DISCUSSION", body: "..." })`. + Tool добавляет heading + verdict line автоматически — НЕ форматируй их + вручную. Если tool вернул строку начинающуюся с `⚠️ ...failed` — СООБЩИ + оркестратору о сбое и STOP (не fallback на raw bash). 6. НЕ МЕРДЖИТЬ — merge делает основной агент через run-pipeline. ``` diff --git a/.opencode/tools/post-docs-review.ts b/.opencode/tools/post-docs-review.ts index a470196..e2a054b 100644 --- a/.opencode/tools/post-docs-review.ts +++ b/.opencode/tools/post-docs-review.ts @@ -13,7 +13,7 @@ export default tool({ }, async execute(args, context) { const comment = `## Docs Review Summary\n\n${args.body}\n\n### Verdict: ${args.verdict}` - const r = spawnSync("gh", ["pr", "comment", String(args.pr_number), "--body", comment, "--repo", "slaid098/opencode-config"], { + const r = spawnSync("gh", ["pr", "comment", String(args.pr_number), "--body", comment], { encoding: "utf-8", cwd: context.worktree, }) diff --git a/.opencode/tools/post-review.ts b/.opencode/tools/post-review.ts index c6badcb..ceb23e5 100644 --- a/.opencode/tools/post-review.ts +++ b/.opencode/tools/post-review.ts @@ -13,7 +13,7 @@ export default tool({ }, async execute(args, context) { const comment = `## Code Review Summary\n\n${args.body}\n\n### Verdict: ${args.verdict}` - const r = spawnSync("gh", ["pr", "comment", String(args.pr_number), "--body", comment, "--repo", "slaid098/opencode-config"], { + const r = spawnSync("gh", ["pr", "comment", String(args.pr_number), "--body", comment], { encoding: "utf-8", cwd: context.worktree, }) diff --git a/docs/decisions/019-pr-46-review-posting-tools.md b/docs/decisions/019-pr-46-review-posting-tools.md index 13c57d4..d6a6945 100644 --- a/docs/decisions/019-pr-46-review-posting-tools.md +++ b/docs/decisions/019-pr-46-review-posting-tools.md @@ -3,6 +3,20 @@ ## Статус Accepted (2026-07-24) +## Superseding note (PR#60, 2026-07-25) + +Часть решений ADR-019 пересмотрена в PR#60 (`fix(tools): drop hardcoded --repo in post-review/post-docs-review`): + +1. **Хардкод `--repo slaid098/opencode-config` убран** из `post-review.ts:16` и `post-docs-review.ts:16`. Теперь `gh pr comment N --body ` без `--repo` — gh auto-detect'ит репо из `context.worktree` (cwd), симметрично с `create-pr.ts`/`merge-pr.ts`/`commit.ts`. Tools больше не привязаны к `slaid098/opencode-config` и работают из любого репо с `origin` remote. + +2. **Rejected alternative "Параметризовать `--repo` через `git remote`" пересмотрена.** В ADR-019 эта альтернатива была отклонена как "out of scope. Сейчас tools специфичны для slaid098/opencode-config". После того как tools установили глобально (применимы ко всем репо), хардкод стал багом: comment постился в `slaid098/opencode-config` независимо от cwd → silent misroute (PR#29 cross-post incident: 5 комментов предназначавшихся `opencode-voice-dictation` PR#29 ушли в `opencode-config` PR#29). Достаточно убрать хардкод — параметризация через `git remote get-url` не нужна, gh auto-detect справляется. + +3. **Расхождение skill-шаблонов ↔ промптов устранено.** `run-pipeline/SKILL.md` Template B (docs-review) и Template C (code_review) предписывали raw `gh pr comment M --body "## ... Summary\n..."`. Промпты `reviewer.md`/`docs-reviewer.md` предписывали `post_review`/`post_docs_review` tool. Оба пути технически разрешены (`gh pr comment*` в allow-list), но расхождение — code smell, модель могла выбрать любой. Template B/C обновлены использовать tool (один canonical путь, формат гарантирован zod-enum). + +4. **Добавлена инструкция обработки сбоя tool** в `reviewer.md` и `docs-reviewer.md`: если tool вернул `⚠️ ...failed` — СООБЩИ оркестратору и STOP, не fallback на raw `gh pr comment`. Раньше такой инструкции не было (см. memory `post-review-post-docs-review-tool-vs-skill-discrepancy.md` — "Fallback instructions — НЕТ"), что позволяло агенту игнорировать сбой (вероятная причина PR#29 silent failure). + +5. **Из не-git директории tool'зы падают с явной ошибкой** (gh exit non-zero → `⚠️ ...failed`), а не тихим misroute'ом. Условие работы: `context.worktree` указывает на git-репо с `origin` remote. + ## Контекст Reviewer и docs-reviewer агенты постят verdict-комментарии на PR через raw `gh pr comment` с ручным форматированием heading + verdict line: diff --git a/docs/decisions/025-pr-61-drop-hardcoded-repo-in-review-tools.md b/docs/decisions/025-pr-61-drop-hardcoded-repo-in-review-tools.md new file mode 100644 index 0000000..c934335 --- /dev/null +++ b/docs/decisions/025-pr-61-drop-hardcoded-repo-in-review-tools.md @@ -0,0 +1,36 @@ +# ADR-025: Drop hardcoded --repo in post-review/post-docs-review tools + +## Статус +Accepted (2026-07-25) + +## Контекст +`post-review.ts` и `post-docs-review.ts` (созданы в ADR-019, PR#46) хардкодили `--repo slaid098/opencode-config` в вызове `gh pr comment`: + +```ts +const r = spawnSync("gh", ["pr","comment", String(args.pr_number), "--body", comment, "--repo", "slaid098/opencode-config"], { encoding:"utf-8", cwd: context.worktree }) +``` + +Флаг `--repo` перегружает auto-detect — `gh` игнорирует `cwd` (context.worktree) и всегда постит в `slaid098/opencode-config`, независимо от репо в котором идёт работа. ADR-019 сознательно отклонил параметризацию `--repo` как "out of scope. Сейчас tools специфичны для slaid098/opencode-config". Затем tools установили глобально (применимы ко всем репо), но хардкод не пересмотрели — стал багом. + +**Симптомы:** +1. При работе в репо ≠ opencode-config коммент постится в opencode-config PR (по номеру), а не в целевой репо. +2. При коллизии номеров PR (PR#29 существует в `opencode-config` и `opencode-voice-dictation`) `gh` возвращает exit 0 даже на MERGED PR — tool рапортует успех без верификации. +3. `pipeline-status.py` (правильно деривит репо из `git remote`) видит 0 комментов на целевом PR → pipeline застревает на DOCS/REVIEW phase. +4. Подтверждено: 5 misrouted "Docs Review" комментов на opencode-config PR#29, предназначавшихся voice-dictation PR#29. + +**Асимметрия:** `create-issue.ts`, `create-pr.ts`, `merge-pr.ts` НЕ передают `--repo` — полагаются на auto-detect из `context.worktree`. Только post-review/post-docs-review хардкодили. + +**Дополнительно:** расхождение между промптами агентов (`reviewer.md:218`, `docs-reviewer.md:198` — предписывают tool) и skill-шаблонами (`run-pipeline/SKILL.md` Template B/C — предписывали raw `gh pr comment`). Оба пути разрешены, но code smell. + +## Решение +1. **Убрать хардкод `--repo slaid098/opencode-config`** из `post-review.ts:16` и `post-docs-review.ts:16`. Теперь `gh pr comment N --body ` без `--repo` → gh auto-detect'ит репо из `context.worktree` (cwd), симметрично с `create-pr.ts`/`merge-pr.ts`/`commit.ts`. +2. **Синхронизировать `run-pipeline/SKILL.md`** Template B (docs-review) и Template C (code_review): raw `gh pr comment M --body "## ... Summary\n..."` заменён на `post_docs_review({ pr_number: M, verdict: ..., body: ... })` / `post_review({...})`. Один canonical путь (tool), формат гарантирован zod-enum. +3. **Добавить инструкцию обработки сбоя tool** в `reviewer.md` и `docs-reviewer.md`: если tool вернул `⚠️ ...failed` — СООБЩИ оркестратору и STOP, не fallback на raw `gh pr comment`. Раньше инструкции не было — агент мог игнорировать сбой (вероятная причина PR#29 silent failure). +4. **Тесты обновлены:** `test_spawnsync_args` утверждает `--repo` НЕ в args (было `args[repo_idx] == "slaid098/opencode-config"`) и что args заканчиваются на `--body `. +5. **ADR-019 дополнен** секцией "Superseding note (PR#60)" — зафиксирован пересмотр решений. + +## Альтернативы +- **Параметризовать `--repo` через `git remote get-url origin`** (как ADR-007 для pipeline-status.py) — отклонено: достаточно убрать хардкод, gh auto-detect из cwd справляется. Параметризация добавила бы код без выгоды (create-pr/merge-pr уже работают без неё). ADR-019 rejected alt пересмотрена. +- **Оставить хардкод + параметризовать через env var `GH_REPO`** — отклонено: env var неявная зависимость, auto-detect из cwd надёжнее и симметрична с остальными tools. +- **Запретить raw `gh pr comment*` в allow-list (принудить к tool)** — отклонено для этого PR: оставлено для обратной совместимости. SKILL.md теперь однозначно предписывает tool, расхождение устранено. Strict deny — отдельный PR (см. ADR-019 rejected alt). +- **Валидировать что comment реально запощен (post-hoc check через `gh pr view N --json comments`)** — отклонено: добавляет network round-trip и сложность. Tool уже возвращает `⚠️ ...failed` при non-zero exit — достаточно инструкции "STOP при ⚠️" в промптах (шаг 3). \ No newline at end of file diff --git a/docs/handoff/pr-61-drop-hardcoded-repo-in-review-tools.md b/docs/handoff/pr-61-drop-hardcoded-repo-in-review-tools.md new file mode 100644 index 0000000..d9b354c --- /dev/null +++ b/docs/handoff/pr-61-drop-hardcoded-repo-in-review-tools.md @@ -0,0 +1,33 @@ +--- +pr: 61 +title: fix(tools): drop hardcoded --repo in post-review/post-docs-review +--- + +## Что сделано +- Убран хардкод `,"--repo","slaid098/opencode-config"` из `tools/post-review.ts:16` и `tools/post-docs-review.ts:16`. Теперь `gh pr comment N --body ` без `--repo` — gh auto-detect'ит репо из `context.worktree` (cwd), симметрично с `create-pr.ts`/`merge-pr.ts`/`commit.ts`. +- `run-pipeline/SKILL.md` Template B (docs-review) и Template C (code_review) синхронизированы с промптами агентов: raw `gh pr comment M --body "## ... Summary\n..."` заменён на вызовы `post_docs_review`/`post_review` tool. Устранено расхождение skill-шаблон ↔ промпт (code smell, модель могла выбрать любой путь). +- В `agents/reviewer.md` и `agents/docs-reviewer.md` добавлена секция "Tool failure handling": если tool вернул `⚠️ ...failed` — СООБЩИ оркестратору и STOP, не fallback на raw `gh pr comment`. Раньше инструкции не было (см. memory `post-review-post-docs-review-tool-vs-skill-discrepancy.md` — "Fallback instructions — НЕТ"). +- Тесты обновлены: `test_spawnsync_args` в `tests/test_post_review_tool.{py,ts}` и `tests/test_post_docs_review_tool.{py,ts}` теперь утверждают `--repo` НЕ в args (было `args[repo_idx] == "slaid098/opencode-config"`) и что args заканчиваются на `--body `. Docstring обновлены. Все 24 Python теста проходят. +- ADR-019 (`docs/decisions/019-pr-46-review-posting-tools.md`) дополнен секцией "Superseding note (PR#60)": зафиксировано убирание хардкода, пересмотр rejected alternative "Параметризовать --repo", устранение расхождения skill-шаблонов, добавление инструкции обработки сбоя. + +## Почему +`post-review.ts`/`post-docs-review.ts` хардкодили `--repo slaid098/opencode-config` в `spawnSync("gh", [..., "--repo", "slaid098/opencode-config"])`. Флаг `--repo` перегружает auto-detect — `gh` игноряет `cwd` (context.worktree) и всегда постит в `slaid098/opencode-config`, независимо от репо в котором идёт работа. + +**Симптомы (из issue #60):** +1. При работе в репо ≠ opencode-config (например voice-dictation) коммент постится в opencode-config PR (по номеру), а не в целевой репо. +2. При коллизии номеров PR (PR#29 существует в обоих репо) `gh` возвращает exit 0 даже на MERGED PR — tool рапортует успех без верификации. +3. `pipeline-status.py` (который правильно деривит репо из `git remote`) видит 0 комментов на целевом PR → pipeline застревает на DOCS/REVIEW phase. +4. Подтверждено: 5 misrouted "Docs Review" комментов найдено на opencode-config PR#29, предназначавшихся voice-dictation PR#29 (cross-post incident). + +**Асимметрия:** `create-issue.ts`, `create-pr.ts`, `merge-pr.ts` НЕ передают `--repo` — полагаются на auto-detect. Только post-review/post-docs-review хардкодили — legacy/oversight из ADR-019 (rejected alt "out of scope. Сейчас tools специфичны для slaid098/opencode-config"). + +**Расхождение skill ↔ промпт:** промпты `reviewer.md:218`, `docs-reviewer.md:198` предписывали tool, а `run-pipeline/SKILL.md` Template B/C предписывали raw `gh pr comment`. Оба пути разрешены (`gh pr comment*` в allow-list), но расхождение — code smell. + +## Pending +— (после мерджа: обновить memory notes `technical/post-docs-review-silent-failure-pr-29.md` и `technical/post-review-post-docs-review-tool-vs-skill-discrepancy.md` — пометить resolved, importance high → medium. Делается в этом же PR, шаг 6 спеки issue #60.) + +## Watch out +- Tool'зы теперь требуют, чтобы `context.worktree` указывал на git-репо с `origin` remote. Из не-git директории `/root/workspace` gh упадёт с явной ошибкой (лучше тихого misroute). Это ожидаемое поведение — см. ADR-019 superseding note п.5. +- `gh pr comment*` остался в allow-list `reviewer.md`/`docs-reviewer.md` — raw bash путь технически разрешён, но промпт + SKILL.md теперь однозначно говорят "используй tool". Если захотеть strict tool-only — отдельный PR с deny правилом (см. ADR-019 rejected alt "Запретить raw gh pr comment в allow-list"). +- TS-тесты (`test_post_review_tool.ts`, `test_post_docs_review_tool.ts`) — документационные, запускаются под `bun test` (bun runtime недоступен на CI). Реальные assertions в Python-зеркалах, которые запускаются через `tests/_ts_loader.mjs`. +- Pipeline-status.py regex'ы НЕ изменены — tools генерируют ровно тот формат (`## Code Review Summary` + `### Verdict: `, `## Docs Review Summary`), который парсят `REVIEW_VERDICT_RE` и `DOCS_REVIEW_RE`. \ No newline at end of file diff --git a/tests/test_post_docs_review_tool.py b/tests/test_post_docs_review_tool.py index 438a01e..6b571b6 100644 --- a/tests/test_post_docs_review_tool.py +++ b/tests/test_post_docs_review_tool.py @@ -15,7 +15,8 @@ Modes used: → "Docs review posted on PR #N: verdict=", (b) comment heading: body passed to gh contains "## Docs Review Summary", (c) comment verdict: body passed to gh contains "### Verdict: ", - (d) spawnSync args: gh pr comment N --body --repo slaid098/opencode-config. + (d) spawnSync args: gh pr comment N --body (no --repo; gh auto-detects + from context.worktree — ADR-019 superseding note, PR#60). post-docs-review.ts makes 1 spawnSync call (gh pr comment) on all paths. @@ -183,7 +184,13 @@ def test_comment_has_no_changes_verdict(): def test_spawnsync_args(): - """spawnSync called with gh pr comment --body --repo slaid098/opencode-config.""" + """spawnSync called with gh pr comment --body (no --repo). + + --repo was hardcoded to slaid098/opencode-config (ADR-019) and caused silent + misroute to the wrong repo when working outside opencode-config. Removed in + PR#60 — gh now auto-detects the repo from context.worktree (cwd), matching + create-pr.ts/merge-pr.ts. Symmetric with the success-path assertions. + """ out = _run_exec( {"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."}, [COMMENT_OK_RESPONSE], @@ -197,10 +204,15 @@ def test_spawnsync_args(): assert args[1] == "comment", f"expected second arg 'comment', got: {args[1]!r}" assert args[2] == "45", f"expected PR number '45', got: {args[2]!r}" assert "--body" in args, "missing --body flag" - assert "--repo" in args, "missing --repo flag" - repo_idx = args.index("--repo") + 1 - assert args[repo_idx] == "slaid098/opencode-config", ( - f"expected repo 'slaid098/opencode-config', got: {args[repo_idx]!r}" + # --repo MUST NOT be present — gh auto-detects from cwd (context.worktree). + # Hardcoded --repo caused silent misroute (PR#60 root cause). + assert "--repo" not in args, ( + f"--repo must not be hardcoded; gh auto-detects from cwd. args: {args!r}" + ) + # args must end with --body (no trailing --repo slaid098/...). + assert args[-2] == "--body", f"expected args to end with --body , got: {args[-2:]!r}" + assert args[-1].startswith("## Docs Review Summary\n"), ( + f"expected last arg to be the comment body, got: {args[-1]!r}" ) diff --git a/tests/test_post_docs_review_tool.ts b/tests/test_post_docs_review_tool.ts index f2b8df9..9bba646 100644 --- a/tests/test_post_docs_review_tool.ts +++ b/tests/test_post_docs_review_tool.ts @@ -154,7 +154,7 @@ describe("post-docs-review tool", () => { expect(comment).toContain("### Verdict: APPROVE") }) - test("test_spawnsync_args — spawnSync called with correct gh args", async () => { + test("test_spawnsync_args — spawnSync called with correct gh args (no --repo)", async () => { let capturedCmd let capturedArgs mock.module("child_process", () => ({ @@ -175,8 +175,11 @@ describe("post-docs-review tool", () => { expect(capturedArgs[1]).toBe("comment") expect(capturedArgs[2]).toBe("45") expect(capturedArgs).toContain("--body") - expect(capturedArgs).toContain("--repo") - const repoIdx = capturedArgs.indexOf("--repo") + 1 - expect(capturedArgs[repoIdx]).toBe("slaid098/opencode-config") + // --repo MUST NOT be present — gh auto-detects from cwd (PR#60). + expect(capturedArgs).not.toContain("--repo") + // args end with --body (no trailing --repo slaid098/...). + const lastIdx = capturedArgs.length - 1 + expect(capturedArgs[lastIdx - 1]).toBe("--body") + expect(capturedArgs[lastIdx].startsWith("## Docs Review Summary\n")).toBe(true) }) }) \ No newline at end of file diff --git a/tests/test_post_review_tool.py b/tests/test_post_review_tool.py index 713b003..0cf1f50 100644 --- a/tests/test_post_review_tool.py +++ b/tests/test_post_review_tool.py @@ -15,7 +15,8 @@ Modes used: → "Review posted on PR #N: verdict=", (b) comment heading: body passed to gh contains "## Code Review Summary", (c) comment verdict: body passed to gh contains "### Verdict: ", - (d) spawnSync args: gh pr comment N --body --repo slaid098/opencode-config. + (d) spawnSync args: gh pr comment N --body (no --repo; gh auto-detects + from context.worktree — ADR-019 superseding note, PR#60). post-review.ts makes 1 spawnSync call (gh pr comment) on all paths. @@ -188,7 +189,13 @@ def test_comment_has_needs_discussion_verdict(): def test_spawnsync_args(): - """spawnSync called with gh pr comment --body --repo slaid098/opencode-config.""" + """spawnSync called with gh pr comment --body (no --repo). + + --repo was hardcoded to slaid098/opencode-config (ADR-019) and caused silent + misroute to the wrong repo when working outside opencode-config. Removed in + PR#60 — gh now auto-detects the repo from context.worktree (cwd), matching + create-pr.ts/merge-pr.ts. Symmetric with the success-path assertions. + """ out = _run_exec( {"pr_number": 45, "verdict": "APPROVE", "body": "Review body."}, [COMMENT_OK_RESPONSE], @@ -202,10 +209,15 @@ def test_spawnsync_args(): assert args[1] == "comment", f"expected second arg 'comment', got: {args[1]!r}" assert args[2] == "45", f"expected PR number '45', got: {args[2]!r}" assert "--body" in args, "missing --body flag" - assert "--repo" in args, "missing --repo flag" - repo_idx = args.index("--repo") + 1 - assert args[repo_idx] == "slaid098/opencode-config", ( - f"expected repo 'slaid098/opencode-config', got: {args[repo_idx]!r}" + # --repo MUST NOT be present — gh auto-detects from cwd (context.worktree). + # Hardcoded --repo caused silent misroute (PR#60 root cause). + assert "--repo" not in args, ( + f"--repo must not be hardcoded; gh auto-detects from cwd. args: {args!r}" + ) + # args must end with --body (no trailing --repo slaid098/...). + assert args[-2] == "--body", f"expected args to end with --body , got: {args[-2:]!r}" + assert args[-1].startswith("## Code Review Summary\n"), ( + f"expected last arg to be the comment body, got: {args[-1]!r}" ) diff --git a/tests/test_post_review_tool.ts b/tests/test_post_review_tool.ts index 928d440..a364beb 100644 --- a/tests/test_post_review_tool.ts +++ b/tests/test_post_review_tool.ts @@ -157,7 +157,7 @@ describe("post-review tool", () => { expect(comment).toContain("### Verdict: APPROVE") }) - test("test_spawnsync_args — spawnSync called with correct gh args", async () => { + test("test_spawnsync_args — spawnSync called with correct gh args (no --repo)", async () => { let capturedCmd let capturedArgs mock.module("child_process", () => ({ @@ -178,8 +178,11 @@ describe("post-review tool", () => { expect(capturedArgs[1]).toBe("comment") expect(capturedArgs[2]).toBe("45") expect(capturedArgs).toContain("--body") - expect(capturedArgs).toContain("--repo") - const repoIdx = capturedArgs.indexOf("--repo") + 1 - expect(capturedArgs[repoIdx]).toBe("slaid098/opencode-config") + // --repo MUST NOT be present — gh auto-detects from cwd (PR#60). + expect(capturedArgs).not.toContain("--repo") + // args end with --body (no trailing --repo slaid098/...). + const lastIdx = capturedArgs.length - 1 + expect(capturedArgs[lastIdx - 1]).toBe("--body") + expect(capturedArgs[lastIdx].startsWith("## Code Review Summary\n")).toBe(true) }) }) \ No newline at end of file