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 <agent@opencode.local>
This commit is contained in:
parent
89a4d947fd
commit
f96acaaa75
12 changed files with 167 additions and 28 deletions
|
|
@ -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).
|
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.
|
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
|
## Rules
|
||||||
|
|
||||||
1. ALWAYS checkout the PR branch first.
|
1. ALWAYS checkout the PR branch first.
|
||||||
|
|
|
||||||
|
|
@ -282,6 +282,14 @@ Body format (without heading — tool adds `## Code Review Summary` and `### Ver
|
||||||
Do NOT attempt merge. Stop and wait for discussion.
|
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.
|
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
|
## Severity Levels
|
||||||
|
|
||||||
| Level | Meaning | Action |
|
| Level | Meaning | Action |
|
||||||
|
|
|
||||||
|
|
@ -73,9 +73,14 @@ Review PR#M в текущем репо (pre-merge, режим docs).
|
||||||
"docs: update project map + handoff + ADR" && git push`.
|
"docs: update project map + handoff + ADR" && git push`.
|
||||||
8. **ВСЕГДА** оставь PR comment (даже если structural changes нет) — это
|
8. **ВСЕГДА** оставь PR comment (даже если structural changes нет) — это
|
||||||
детерминированный marker для `check_docs` в pipeline-status.py. Без comment
|
детерминированный marker для `check_docs` в pipeline-status.py. Без comment
|
||||||
pipeline блокируется на DOCS phase.
|
pipeline блокируется на DOCS phase. Используй tool `post_docs_review` (НЕ
|
||||||
`gh pr comment M --body "## Docs Review Summary\n- Project map: ...\n- Handoff: ...\n- ADR: ...\n\n### Verdict: APPROVE|FIXED|NO_CHANGES"`.
|
raw bash-вызов `gh`) — tool гарантирует heading `## Docs Review Summary` и
|
||||||
Heading `## Docs Review Summary` — обязательно (regex `Docs Review`).
|
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)
|
### Template C (code_review)
|
||||||
|
|
@ -88,9 +93,14 @@ Review PR#M в текущем репо.
|
||||||
через `skill("<name>")`.
|
через `skill("<name>")`.
|
||||||
4. Проверь: code quality, architecture, error handling, security, testing,
|
4. Проверь: code quality, architecture, error handling, security, testing,
|
||||||
duplication, project-specific rules, PR hygiene, handoff/ADR (quick check).
|
duplication, project-specific rules, PR hygiene, handoff/ADR (quick check).
|
||||||
5. Оставь review как PR comment (НЕ `gh pr review --approve` — GitHub блокирует
|
5. Оставь review через tool `post_review` (НЕ `gh pr review --approve` —
|
||||||
self-approve):
|
GitHub блокирует self-approve; НЕ raw bash-вызов `gh`) — tool гарантирует
|
||||||
`gh pr comment M --body "## Code Review Summary\n...\n### Verdict: APPROVE|REQUEST_CHANGES"`.
|
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.
|
6. НЕ МЕРДЖИТЬ — merge делает основной агент через run-pipeline.
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ export default tool({
|
||||||
},
|
},
|
||||||
async execute(args, context) {
|
async execute(args, context) {
|
||||||
const comment = `## Docs Review Summary\n\n${args.body}\n\n### Verdict: ${args.verdict}`
|
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",
|
encoding: "utf-8",
|
||||||
cwd: context.worktree,
|
cwd: context.worktree,
|
||||||
})
|
})
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,7 @@ export default tool({
|
||||||
},
|
},
|
||||||
async execute(args, context) {
|
async execute(args, context) {
|
||||||
const comment = `## Code Review Summary\n\n${args.body}\n\n### Verdict: ${args.verdict}`
|
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",
|
encoding: "utf-8",
|
||||||
cwd: context.worktree,
|
cwd: context.worktree,
|
||||||
})
|
})
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,20 @@
|
||||||
## Статус
|
## Статус
|
||||||
Accepted (2026-07-24)
|
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 <comment>` без `--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:
|
Reviewer и docs-reviewer агенты постят verdict-комментарии на PR через raw `gh pr comment` с ручным форматированием heading + verdict line:
|
||||||
|
|
|
||||||
|
|
@ -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 <comment>` без `--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 <comment>`.
|
||||||
|
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).
|
||||||
33
docs/handoff/pr-61-drop-hardcoded-repo-in-review-tools.md
Normal file
33
docs/handoff/pr-61-drop-hardcoded-repo-in-review-tools.md
Normal file
|
|
@ -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 <comment>` без `--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 <comment>`. 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: <V>`, `## Docs Review Summary`), который парсят `REVIEW_VERDICT_RE` и `DOCS_REVIEW_RE`.
|
||||||
|
|
@ -15,7 +15,8 @@ Modes used:
|
||||||
→ "Docs review posted on PR #N: verdict=<V>",
|
→ "Docs review posted on PR #N: verdict=<V>",
|
||||||
(b) comment heading: body passed to gh contains "## Docs Review Summary",
|
(b) comment heading: body passed to gh contains "## Docs Review Summary",
|
||||||
(c) comment verdict: body passed to gh contains "### Verdict: <V>",
|
(c) comment verdict: body passed to gh contains "### Verdict: <V>",
|
||||||
(d) spawnSync args: gh pr comment N --body <comment> --repo slaid098/opencode-config.
|
(d) spawnSync args: gh pr comment N --body <comment> (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.
|
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():
|
def test_spawnsync_args():
|
||||||
"""spawnSync called with gh pr comment <N> --body <comment> --repo slaid098/opencode-config."""
|
"""spawnSync called with gh pr comment <N> --body <comment> (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(
|
out = _run_exec(
|
||||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
||||||
[COMMENT_OK_RESPONSE],
|
[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[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 args[2] == "45", f"expected PR number '45', got: {args[2]!r}"
|
||||||
assert "--body" in args, "missing --body flag"
|
assert "--body" in args, "missing --body flag"
|
||||||
assert "--repo" in args, "missing --repo flag"
|
# --repo MUST NOT be present — gh auto-detects from cwd (context.worktree).
|
||||||
repo_idx = args.index("--repo") + 1
|
# Hardcoded --repo caused silent misroute (PR#60 root cause).
|
||||||
assert args[repo_idx] == "slaid098/opencode-config", (
|
assert "--repo" not in args, (
|
||||||
f"expected repo 'slaid098/opencode-config', got: {args[repo_idx]!r}"
|
f"--repo must not be hardcoded; gh auto-detects from cwd. args: {args!r}"
|
||||||
|
)
|
||||||
|
# args must end with --body <comment> (no trailing --repo slaid098/...).
|
||||||
|
assert args[-2] == "--body", f"expected args to end with --body <comment>, 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}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -154,7 +154,7 @@ describe("post-docs-review tool", () => {
|
||||||
expect(comment).toContain("### Verdict: APPROVE")
|
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 capturedCmd
|
||||||
let capturedArgs
|
let capturedArgs
|
||||||
mock.module("child_process", () => ({
|
mock.module("child_process", () => ({
|
||||||
|
|
@ -175,8 +175,11 @@ describe("post-docs-review tool", () => {
|
||||||
expect(capturedArgs[1]).toBe("comment")
|
expect(capturedArgs[1]).toBe("comment")
|
||||||
expect(capturedArgs[2]).toBe("45")
|
expect(capturedArgs[2]).toBe("45")
|
||||||
expect(capturedArgs).toContain("--body")
|
expect(capturedArgs).toContain("--body")
|
||||||
expect(capturedArgs).toContain("--repo")
|
// --repo MUST NOT be present — gh auto-detects from cwd (PR#60).
|
||||||
const repoIdx = capturedArgs.indexOf("--repo") + 1
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
expect(capturedArgs[repoIdx]).toBe("slaid098/opencode-config")
|
// args end with --body <comment> (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)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
@ -15,7 +15,8 @@ Modes used:
|
||||||
→ "Review posted on PR #N: verdict=<V>",
|
→ "Review posted on PR #N: verdict=<V>",
|
||||||
(b) comment heading: body passed to gh contains "## Code Review Summary",
|
(b) comment heading: body passed to gh contains "## Code Review Summary",
|
||||||
(c) comment verdict: body passed to gh contains "### Verdict: <V>",
|
(c) comment verdict: body passed to gh contains "### Verdict: <V>",
|
||||||
(d) spawnSync args: gh pr comment N --body <comment> --repo slaid098/opencode-config.
|
(d) spawnSync args: gh pr comment N --body <comment> (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.
|
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():
|
def test_spawnsync_args():
|
||||||
"""spawnSync called with gh pr comment <N> --body <comment> --repo slaid098/opencode-config."""
|
"""spawnSync called with gh pr comment <N> --body <comment> (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(
|
out = _run_exec(
|
||||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body."},
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body."},
|
||||||
[COMMENT_OK_RESPONSE],
|
[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[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 args[2] == "45", f"expected PR number '45', got: {args[2]!r}"
|
||||||
assert "--body" in args, "missing --body flag"
|
assert "--body" in args, "missing --body flag"
|
||||||
assert "--repo" in args, "missing --repo flag"
|
# --repo MUST NOT be present — gh auto-detects from cwd (context.worktree).
|
||||||
repo_idx = args.index("--repo") + 1
|
# Hardcoded --repo caused silent misroute (PR#60 root cause).
|
||||||
assert args[repo_idx] == "slaid098/opencode-config", (
|
assert "--repo" not in args, (
|
||||||
f"expected repo 'slaid098/opencode-config', got: {args[repo_idx]!r}"
|
f"--repo must not be hardcoded; gh auto-detects from cwd. args: {args!r}"
|
||||||
|
)
|
||||||
|
# args must end with --body <comment> (no trailing --repo slaid098/...).
|
||||||
|
assert args[-2] == "--body", f"expected args to end with --body <comment>, 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}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -157,7 +157,7 @@ describe("post-review tool", () => {
|
||||||
expect(comment).toContain("### Verdict: APPROVE")
|
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 capturedCmd
|
||||||
let capturedArgs
|
let capturedArgs
|
||||||
mock.module("child_process", () => ({
|
mock.module("child_process", () => ({
|
||||||
|
|
@ -178,8 +178,11 @@ describe("post-review tool", () => {
|
||||||
expect(capturedArgs[1]).toBe("comment")
|
expect(capturedArgs[1]).toBe("comment")
|
||||||
expect(capturedArgs[2]).toBe("45")
|
expect(capturedArgs[2]).toBe("45")
|
||||||
expect(capturedArgs).toContain("--body")
|
expect(capturedArgs).toContain("--body")
|
||||||
expect(capturedArgs).toContain("--repo")
|
// --repo MUST NOT be present — gh auto-detects from cwd (PR#60).
|
||||||
const repoIdx = capturedArgs.indexOf("--repo") + 1
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
expect(capturedArgs[repoIdx]).toBe("slaid098/opencode-config")
|
// args end with --body <comment> (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)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
Loading…
Add table
Reference in a new issue