feat(tools): deterministic review posting tools (post-review, post-docs-review) (#46)
* feat(tools): add post-review and post-docs-review tools * refactor(agents): use post-review/post-docs-review tools in reviewer and docs-reviewer * feat(permissions): add post_review and post_docs_review to agent.tools map * test(tools): add tests for post-review and post-docs-review tools * docs(handoff): add handoff ADR and project-map for review posting tools * docs(handoff): set PR number * docs: fix PR#45→PR#46 refs in project map --------- Co-authored-by: opencode-agent <agent@slaid098.dev>
This commit is contained in:
parent
f06c9f2422
commit
1acac5229f
13 changed files with 1071 additions and 46 deletions
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
description: Reviews and updates project map documentation before code review. Auto-commits updates to PR branch.
|
||||
description: Reviews and updates project map documentation before code review. Auto-commits updates to PR branch. Posts verdict via post_docs_review tool (deterministic heading for pipeline-status.py).
|
||||
mode: subagent
|
||||
temperature: 0.1
|
||||
steps: 100
|
||||
|
|
@ -198,21 +198,21 @@ last_updated: <YYYY-MM-DD>
|
|||
|
||||
## PR Comment (mandatory)
|
||||
|
||||
After validation (regardless of whether structural changes occurred), **ALWAYS** leave a PR comment with verdict. This is the deterministic marker that `check_docs` in pipeline-status.py uses to prove docs-reviewer ran. Without this comment, the pipeline is blocked at DOCS phase.
|
||||
|
||||
Comment format:
|
||||
After validation (regardless of whether structural changes occurred), **ALWAYS** leave a PR comment using the `post_docs_review` tool. The tool auto-generates the `## Docs Review Summary` heading and the `### Verdict: <verdict>` line — you only pass the body content (between heading and verdict). Do NOT manually format the heading or verdict. This is the deterministic marker that `check_docs` in pipeline-status.py uses to prove docs-reviewer ran. Without this comment, the pipeline is blocked at DOCS phase.
|
||||
|
||||
Body format (without heading — tool adds `## Docs Review Summary` and `### Verdict: <verdict>`):
|
||||
```
|
||||
## Docs Review Summary
|
||||
|
||||
- Project map: <updated|no structural changes|created>
|
||||
- Handoff: <valid|fixed: ...|missing: ...>
|
||||
- ADR: <valid|fixed: ...|missing: ...>
|
||||
|
||||
## Spec Cleanup
|
||||
- Spec: <removed: all N issues from roadmap.md are CLOSED|retained: M/N issues still OPEN|n/a: docs/spec/roadmap.md does not exist>
|
||||
```
|
||||
|
||||
### Verdict: <APPROVE|FIXED|NO_CHANGES>
|
||||
Call:
|
||||
```
|
||||
post_docs_review({ pr_number: <PR_NUMBER>, verdict: "<APPROVE|FIXED|NO_CHANGES>", body: `<body text above>` })
|
||||
```
|
||||
|
||||
Verdict semantics:
|
||||
|
|
@ -220,15 +220,10 @@ Verdict semantics:
|
|||
- `FIXED` — docs-reviewer fixed something (project map, handoff, ADR, or spec cleanup performed)
|
||||
- `NO_CHANGES` — no structural changes, handoff+ADR already valid, spec retained or absent (no commit, no edits)
|
||||
|
||||
Command:
|
||||
```bash
|
||||
gh pr comment <PR_NUMBER> --body "<comment text above>"
|
||||
```
|
||||
|
||||
Rules:
|
||||
1. Comment is left AFTER commit+push (if any) — so reviewer can see final state.
|
||||
2. If no structural changes AND handoff+ADR valid → no commit, but comment IS still left with `Verdict: NO_CHANGES`.
|
||||
3. The comment heading `## Docs Review Summary` is required exactly — `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.
|
||||
|
||||
## Rules
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
description: Global code reviewer. Reviews PRs against project skills and universal code standards. Invoke via @reviewer. Uses gh pr comment to approve or request changes. Does NOT merge — merge is done by main agent via run-pipeline.
|
||||
description: Global code reviewer. Reviews PRs against project skills and universal code standards. Invoke via @reviewer. Uses post_review tool to approve or request changes (deterministic heading format for pipeline-status.py). Does NOT merge — merge is done by main agent via run-pipeline.
|
||||
mode: subagent
|
||||
temperature: 0.1
|
||||
steps: 100
|
||||
|
|
@ -207,19 +207,17 @@ Examples of project-specific rules:
|
|||
|
||||
## Output Format
|
||||
|
||||
After reviewing, leave a GitHub PR comment using `gh pr comment`:
|
||||
After reviewing, leave a GitHub PR comment using the `post_review` tool. The tool auto-generates the `## Code Review Summary` heading and the `### Verdict: <verdict>` line — you only pass the body content (between heading and verdict). Do NOT manually format the heading or verdict.
|
||||
|
||||
### If approving (no critical or blocking warnings):
|
||||
|
||||
Run:
|
||||
```
|
||||
gh pr comment <PR_NUMBER> --body "<review text>"
|
||||
post_review({ pr_number: <PR_NUMBER>, verdict: "APPROVE", body: `<review body>` })
|
||||
```
|
||||
|
||||
Review body format:
|
||||
Body format (without heading — tool adds `## Code Review Summary` and `### Verdict: APPROVE`):
|
||||
```
|
||||
## Code Review Summary
|
||||
|
||||
<1-2 sentence overview of the changes and overall quality>
|
||||
|
||||
### Positives
|
||||
|
|
@ -227,24 +225,20 @@ Review body format:
|
|||
|
||||
### Suggestions (info, not blocking)
|
||||
- **file.py:30** [style] Suggestion description
|
||||
|
||||
### Verdict: APPROVE
|
||||
```
|
||||
|
||||
Do NOT attempt merge. Stop. Main agent merges via run-pipeline after CI ✅.
|
||||
After this command, 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.
|
||||
|
||||
### If requesting changes (critical issues found):
|
||||
|
||||
Run:
|
||||
```
|
||||
gh pr comment <PR_NUMBER> --body "<review text>"
|
||||
post_review({ pr_number: <PR_NUMBER>, verdict: "REQUEST_CHANGES", body: `<review body>` })
|
||||
```
|
||||
|
||||
Review body format:
|
||||
Body format (without heading — tool adds `## Code Review Summary` and `### Verdict: REQUEST_CHANGES`):
|
||||
```
|
||||
## Code Review Summary
|
||||
|
||||
### Summary
|
||||
<1-2 sentence overview>
|
||||
|
||||
|
|
@ -258,33 +252,27 @@ Review body format:
|
|||
### Warnings (should fix)
|
||||
- **path/to/file.py:80** [category] Description
|
||||
Fix: suggestion
|
||||
|
||||
### Verdict: REQUEST_CHANGES
|
||||
```
|
||||
|
||||
Do NOT attempt merge. Stop and wait for fixes.
|
||||
After this command, 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.
|
||||
|
||||
### If needs discussion (questions, unclear decisions):
|
||||
|
||||
Run:
|
||||
```
|
||||
gh pr comment <PR_NUMBER> --body "<review text>"
|
||||
post_review({ pr_number: <PR_NUMBER>, verdict: "NEEDS_DISCUSSION", body: `<review body>` })
|
||||
```
|
||||
|
||||
Comment body format:
|
||||
Body format (without heading — tool adds `## Code Review Summary` and `### Verdict: NEEDS_DISCUSSION`):
|
||||
```
|
||||
## Code Review Summary — Needs Discussion
|
||||
|
||||
### Questions
|
||||
1. **file.py:42** Why was this approach chosen over <alternative>?
|
||||
2. **file.tsx:15** Is this the intended behavior?
|
||||
|
||||
### Verdict: NEEDS_DISCUSSION
|
||||
```
|
||||
|
||||
Do NOT attempt merge. Stop and wait for discussion.
|
||||
After this command, 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.
|
||||
|
||||
## Severity Levels
|
||||
|
||||
|
|
@ -302,9 +290,9 @@ After this command, you MUST respond with your review text only. Do NOT call any
|
|||
4. ALWAYS provide file:line references in issues.
|
||||
5. ALWAYS suggest a fix, not just describe the problem.
|
||||
6. If unsure about something → NEEDS_DISCUSSION, don't guess.
|
||||
7. After `gh pr comment` (APPROVE or REQUEST_CHANGES), STOP.
|
||||
7. After `post_review` (APPROVE, REQUEST_CHANGES, or NEEDS_DISCUSSION), STOP.
|
||||
Respond with final text only. ANY further tool call is a protocol violation.
|
||||
Main agent merges via run-pipeline.
|
||||
8. After `gh pr comment` with REQUEST_CHANGES, STOP. Do not merge.
|
||||
8. After `post_review` with REQUEST_CHANGES, STOP. Do not merge.
|
||||
9. Для получения login автора PR используй `gh pr view --json author` (НЕ `gh api user` — broad API call, не в allow-list, вызывает doom-loop).
|
||||
10. Для debug-вывода используй `pwd`/`ls`/`cat` — НЕ `echo` (не в allow-list).
|
||||
|
|
@ -321,7 +321,9 @@
|
|||
"commit": true,
|
||||
"create_pr": true,
|
||||
"create_issue": true,
|
||||
"merge_pr": false
|
||||
"merge_pr": false,
|
||||
"post_review": false,
|
||||
"post_docs_review": false
|
||||
}
|
||||
},
|
||||
"reviewer": {
|
||||
|
|
@ -329,7 +331,9 @@
|
|||
"commit": false,
|
||||
"create_pr": false,
|
||||
"create_issue": false,
|
||||
"merge_pr": false
|
||||
"merge_pr": false,
|
||||
"post_review": true,
|
||||
"post_docs_review": false
|
||||
}
|
||||
},
|
||||
"docs-reviewer": {
|
||||
|
|
@ -337,7 +341,9 @@
|
|||
"commit": true,
|
||||
"create_pr": false,
|
||||
"create_issue": false,
|
||||
"merge_pr": false
|
||||
"merge_pr": false,
|
||||
"post_review": false,
|
||||
"post_docs_review": true
|
||||
}
|
||||
},
|
||||
"memory-syncer": {
|
||||
|
|
@ -345,7 +351,9 @@
|
|||
"commit": false,
|
||||
"create_pr": false,
|
||||
"create_issue": false,
|
||||
"merge_pr": false
|
||||
"merge_pr": false,
|
||||
"post_review": false,
|
||||
"post_docs_review": false
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
|
|||
25
.opencode/tools/post-docs-review.ts
Normal file
25
.opencode/tools/post-docs-review.ts
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
import { spawnSync } from "child_process"
|
||||
import { tool } from "@opencode-ai/plugin"
|
||||
|
||||
const VERDICTS = ["APPROVE", "FIXED", "NO_CHANGES"] as const
|
||||
type Verdict = typeof VERDICTS[number]
|
||||
|
||||
export default tool({
|
||||
description: "Post docs review verdict as PR comment with deterministic heading. Docs-reviewer agent uses this instead of raw `gh pr comment` to guarantee `## Docs Review Summary` heading that pipeline-status.py parses.",
|
||||
args: {
|
||||
pr_number: tool.schema.number().describe("PR number to comment on"),
|
||||
verdict: tool.schema.enum(VERDICTS).describe("Docs review verdict: APPROVE, FIXED, or NO_CHANGES"),
|
||||
body: tool.schema.string().describe("Docs review body text (without heading — heading is auto-generated)"),
|
||||
},
|
||||
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"], {
|
||||
encoding: "utf-8",
|
||||
cwd: context.worktree,
|
||||
})
|
||||
if (r.status !== 0) {
|
||||
return `⚠️ post-docs-review failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
||||
}
|
||||
return `Docs review posted on PR #${args.pr_number}: verdict=${args.verdict}`
|
||||
},
|
||||
})
|
||||
25
.opencode/tools/post-review.ts
Normal file
25
.opencode/tools/post-review.ts
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
import { spawnSync } from "child_process"
|
||||
import { tool } from "@opencode-ai/plugin"
|
||||
|
||||
const VERDICTS = ["APPROVE", "REQUEST_CHANGES", "NEEDS_DISCUSSION"] as const
|
||||
type Verdict = typeof VERDICTS[number]
|
||||
|
||||
export default tool({
|
||||
description: "Post code review verdict as PR comment with deterministic heading. Reviewer agent uses this instead of raw `gh pr comment` to guarantee `## Code Review Summary` + `### Verdict: <verdict>` format that pipeline-status.py parses.",
|
||||
args: {
|
||||
pr_number: tool.schema.number().describe("PR number to comment on"),
|
||||
verdict: tool.schema.enum(VERDICTS).describe("Review verdict: APPROVE, REQUEST_CHANGES, or NEEDS_DISCUSSION"),
|
||||
body: tool.schema.string().describe("Review body text (without heading — heading is auto-generated)"),
|
||||
},
|
||||
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"], {
|
||||
encoding: "utf-8",
|
||||
cwd: context.worktree,
|
||||
})
|
||||
if (r.status !== 0) {
|
||||
return `⚠️ post-review failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
||||
}
|
||||
return `Review posted on PR #${args.pr_number}: verdict=${args.verdict}`
|
||||
},
|
||||
})
|
||||
85
docs/decisions/019-pr-46-review-posting-tools.md
Normal file
85
docs/decisions/019-pr-46-review-posting-tools.md
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
# ADR-019: Deterministic review posting tools (post-review, post-docs-review)
|
||||
|
||||
## Статус
|
||||
Accepted (2026-07-24)
|
||||
|
||||
## Контекст
|
||||
|
||||
Reviewer и docs-reviewer агенты постят verdict-комментарии на PR через raw `gh pr comment` с ручным форматированием heading + verdict line:
|
||||
|
||||
- reviewer: `## Code Review Summary` + `### Verdict: APPROVE|REQUEST_CHANGES|NEEDS_DISCUSSION`
|
||||
- docs-reviewer: `## Docs Review Summary` + `### Verdict: APPROVE|FIXED|NO_CHANGES`
|
||||
|
||||
`pipeline-status.py` парсит эти комментарии regex'ами для определения статуса фаз REVIEW и DOCS:
|
||||
- `REVIEW_VERDICT_RE = re.compile(r"## Code Review Summary.*?###\s*Verdict:\s*(\w+)", re.IGNORECASE | re.DOTALL)`
|
||||
- `DOCS_REVIEW_RE = re.compile(r"Docs Review", re.IGNORECASE)` — substring match
|
||||
|
||||
Риски текущего подхода (raw `gh pr comment`):
|
||||
1. **Wrong heading** — reviewer случайно пишет `## Code Review` (без "Summary") или `## Review Summary` → `REVIEW_VERDICT_RE` не матчит → pipeline NOT_DONE (блокировка)
|
||||
2. **Verdict typo** — `APROVE` вместо `APPROVE`, `REQUEST_CHANGES` с пробелом → regex не матчит verdict → NOT_DONE
|
||||
3. **No mutual exclusion** — любой агент может постить `## Code Review Summary` (нет guard "только reviewer")
|
||||
4. **False positive** — `DOCS_REVIEW_RE` substring match `Docs Review` (без `## ` prefix требования), теоретический false positive если любой комментарий упомянет "Docs Review" в тексте
|
||||
|
||||
Root cause: формат комментария контролируется промптом (soft guard), не type system. Модель может опечататься, забыть heading, изменить формат.
|
||||
|
||||
## Решение
|
||||
|
||||
Детерминированные TS tools, которые гарантируют heading + verdict формат через type system:
|
||||
|
||||
### 1. `post-review.ts` tool (`.opencode/tools/post-review.ts`)
|
||||
|
||||
- Args: `pr_number: number`, `verdict: enum(APPROVE, REQUEST_CHANGES, NEEDS_DISCUSSION)`, `body: string`
|
||||
- Генерирует comment: `## Code Review Summary\n\n${body}\n\n### Verdict: ${verdict}`
|
||||
- Вызывает `gh pr comment N --body <comment> --repo slaid098/opencode-config` через spawnSync
|
||||
- Body — содержимое БЕЗ heading и verdict (tool добавляет их сам)
|
||||
- Verdict enum (zod) гарантирует: только APPROVE/REQUEST_CHANGES/NEEDS_DISCUSSION, опечатки отклоняются на schema layer
|
||||
|
||||
### 2. `post-docs-review.ts` tool (`.opencode/tools/post-docs-review.ts`)
|
||||
|
||||
- Args: `pr_number: number`, `verdict: enum(APPROVE, FIXED, NO_CHANGES)`, `body: string`
|
||||
- Генерирует comment: `## Docs Review Summary\n\n${body}\n\n### Verdict: ${verdict}`
|
||||
- Вызывает `gh pr comment N --body <comment> --repo slaid098/opencode-config` через spawnSync
|
||||
|
||||
### 3. Agent integration
|
||||
|
||||
- `reviewer.md` — секция "Output Format" переписана: `gh pr comment` → `post_review({ pr_number, verdict, body })`. 3 сценария (APPROVE/REQUEST_CHANGES/NEEDS_DISCUSSION). Rules 7/8 обновлены.
|
||||
- `docs-reviewer.md` — секция "PR Comment (mandatory)" переписана: `gh pr comment` → `post_docs_review({ pr_number, verdict, body })`.
|
||||
|
||||
### 4. Role-based access (agent.tools map)
|
||||
|
||||
`.opencode/opencode.json` `agent.tools` map расширен:
|
||||
|
||||
| Agent | post_review | post_docs_review |
|
||||
|---|---|---|
|
||||
| general | false | false |
|
||||
| reviewer | true | false |
|
||||
| docs-reviewer | false | true |
|
||||
| memory-syncer | false | false |
|
||||
|
||||
Mutual exclusion: только reviewer может постить code review, только docs-reviewer — docs review. General и memory-syncer не могут постить reviews. Это устраняет риск "любой агент постит review comment".
|
||||
|
||||
### 5. pipeline-status.py совместимость
|
||||
|
||||
Regex'ы НЕ изменены. Tools генерируют ровно тот формат, который regex ожидает:
|
||||
- `## Code Review Summary` (exact heading) + `### Verdict: <verdict>` → `REVIEW_VERDICT_RE` матчит
|
||||
- `## Docs Review Summary` (contains `Docs Review`) → `DOCS_REVIEW_RE` матчит
|
||||
|
||||
### 6. Тестовая инфраструктура
|
||||
|
||||
`tests/_ts_loader.mjs` расширен:
|
||||
- zodShim: добавлен `enum: () => chain()` метод (для `tool.schema.enum(VERDICTS)`)
|
||||
- `stripTs`: strip `as const` assertions + strip `type <Name> = ...` type alias declarations (TS-only синтаксис новых tools)
|
||||
|
||||
24 новых Python теста (12 на tool) + 14 TS тестов (документационные).
|
||||
|
||||
## Альтернативы
|
||||
|
||||
- **Оставить raw `gh pr comment` + усилить промпт** — отклонено: промпт = soft guard, модель (GLM) игнорирует правила форматирования. Root cause (формат контролируется промптом, не type system) не устранён. Документировано в PR#85 (docs-reviewer marker) — промпт-правила ненадёжны.
|
||||
|
||||
- **Validate comment format post-hoc в pipeline-status.py** — отклонено: pipeline-status.py уже парсит regex'ами, но это detection, не prevention. Comment уже постит с wrong heading → pipeline NOT_DONE → нужно перезапустить reviewer. Tool предотвращает ошибку на этапе создания (prevention > detection).
|
||||
|
||||
- **Параметризовать `--repo` через `git remote` (как ADR-007 для pipeline-status.py)** — отклонено: out of scope. `merge-pr.ts` тоже хардкодит `--squash --delete-branch` (не параметризует repo). Для других репо — отдельный PR (параметризация всех tools). Сейчас tools специфичны для `slaid098/opencode-config`.
|
||||
|
||||
- **Запретить raw `gh pr comment` в allow-list (принудить к tool)** — отклонено: оставлено для обратной совместимости (fallback). Tools используют spawnSync напрямую (не через bash permission layer), так что raw `gh pr comment*` в allow-list не конфликтует. Если захотеть strict tool-only — отдельный PR с deny правилом `gh pr comment*`.
|
||||
|
||||
- **Расширить `DOCS_REVIEW_RE` до `## Docs Review Summary` (exact heading)** — отклонено: risk of breaking existing comments. Tools уже генерируют exact heading, но старые comments (до этого PR) могут иметь другой формат. Substring match остаётся как tolerant fallback. Future PR может tighten regex после migration.
|
||||
45
docs/handoff/pr-46-review-posting-tools.md
Normal file
45
docs/handoff/pr-46-review-posting-tools.md
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
---
|
||||
pr_number: 46
|
||||
title: Deterministic review posting tools (post-review, post-docs-review)
|
||||
---
|
||||
|
||||
# PR: Deterministic review posting tools (post-review, post-docs-review)
|
||||
|
||||
## Что сделано
|
||||
- `.opencode/tools/post-review.ts` — TS tool wrapper (3 args: `pr_number`, `verdict` enum, `body`). Генерирует comment `## Code Review Summary\n\n${body}\n\n### Verdict: ${verdict}`. Вызывает `gh pr comment N --body <comment> --repo slaid098/opencode-config` через spawnSync. Verdict enum: `APPROVE`, `REQUEST_CHANGES`, `NEEDS_DISCUSSION`. Паттерн `merge-pr.ts` (spawnSync, `cwd: context.worktree`).
|
||||
- `.opencode/tools/post-docs-review.ts` — TS tool wrapper (3 args: `pr_number`, `verdict` enum, `body`). Генерирует comment `## Docs Review Summary\n\n${body}\n\n### Verdict: ${verdict}`. Вызывает `gh pr comment N --body <comment> --repo slaid098/opencode-config` через spawnSync. Verdict enum: `APPROVE`, `FIXED`, `NO_CHANGES`.
|
||||
- `.opencode/agents/reviewer.md` — секция "Output Format" переписана: `gh pr comment` → `post_review({ pr_number, verdict, body })`. Tool auto-generates heading + verdict line, body — содержимое между ними. 3 сценария (APPROVE/REQUEST_CHANGES/NEEDS_DISCUSSION). Frontmatter description + Rules 7/8 обновлены.
|
||||
- `.opencode/agents/docs-reviewer.md` — секция "PR Comment (mandatory)" переписана: `gh pr comment` → `post_docs_review({ pr_number, verdict, body })`. Frontmatter description обновлён.
|
||||
- `.opencode/opencode.json` — `agent.tools` map расширен: `post_review` + `post_docs_review` добавлены для 4 агентов (general/reviewer/docs-reviewer/memory-syncer). reviewer: post_review=true, post_docs_review=false; docs-reviewer: post_review=false, post_docs_review=true; general+memory-syncer: оба false.
|
||||
- `tests/_ts_loader.mjs` — zodShim расширен методом `enum: () => chain()` (для `tool.schema.enum(VERDICTS)`). `stripTs` расширен: strip `as const` assertions + strip `type <Name> = ...` type alias declarations (TS-only синтаксис, используется в новых tools).
|
||||
- `tests/test_post_review_tool.ts` — 7 TS тестов (документационные): valid APPROVE/REQUEST_CHANGES/NEEDS_DISCUSSION, invalid verdict, heading, verdict line, spawnSync args.
|
||||
- `tests/test_post_docs_review_tool.ts` — 7 TS тестов: valid APPROVE/FIXED/NO_CHANGES, invalid verdict, heading, verdict line, spawnSync args.
|
||||
- `tests/test_post_review_tool.py` — 12 Python тестов через `_ts_loader.mjs` (exec_stub_json): load, 3 valid verdicts, invalid verdict (permissive — zod validates, not execute), heading, 3 verdict lines, spawnSync args, cwd propagation, gh failure.
|
||||
- `tests/test_post_docs_review_tool.py` — 12 Python тестов: load, 3 valid verdicts, invalid verdict, heading, 3 verdict lines, spawnSync args, cwd propagation, gh failure.
|
||||
- `docs/project-map/README.md` — обновлён: новые tools (post-review.ts, post-docs-review.ts), новые тесты, обновлённые описания agents (reviewer/docs-reviewer используют post_review/post_docs_review tools).
|
||||
- ADR-019 + этот handoff
|
||||
|
||||
## Почему
|
||||
Reviewer и docs-reviewer постят комментарии через raw `gh pr comment` с ручным форматированием heading + verdict. `pipeline-status.py` парсит эти комментарии regex'ами (`REVIEW_VERDICT_RE = re.compile(r"## Code Review Summary.*?###\s*Verdict:\s*(\w+)")`, `DOCS_REVIEW_RE = re.compile(r"Docs Review")`). Риски:
|
||||
1. Reviewer случайно использует wrong heading → pipeline NOT_DONE (блокировка)
|
||||
2. Verdict опечатан (например `APROVE` вместо `APPROVE`) → regex не матчит → NOT_DONE
|
||||
3. Нет mutual exclusion между review и docs-review комментариями
|
||||
4. `DOCS_REVIEW_RE` — substring match, теоретический false positive если любой комментарий упомянет "Docs Review"
|
||||
|
||||
Решение: детерминированные TS tools, которые гарантируют heading + verdict формат через type system (zod enum). Tool принимает body БЕЗ heading — heading auto-generated. Это устраняет root cause ошибок форматирования: агент не может опечататься в heading или verdict, т.к. они генерируются кодом, не промптом.
|
||||
|
||||
Паттерн `merge-pr.ts` (PR#30, ADR-010): thin TS wrapper → spawnSync → `context.worktree` как cwd. Tools auto-discovered через `@opencode-ai/plugin`. agent.tools map в opencode.json контролирует role-based access (reviewer → post_review, docs-reviewer → post_docs_review, general+memory-syncer → none).
|
||||
|
||||
## Pending
|
||||
- Skills/AGENTS.md правила форматов reviews теперь дублируются (agents текст + tools код). Future PR может заменить agents rules на "use post_review/post_docs_review tools" references.
|
||||
- `gh pr comment*` permission в reviewer.md/docs-reviewer.md frontmatter оставлен — tools вызывают spawnSync напрямую (не через bash permission layer). Если захотеть запретить raw `gh pr comment` (принудить к tool) — добавить deny правило. Сейчас оба пути работают.
|
||||
- pipeline-status.py regex'ы НЕ изменены (tools совместимы с существующим парсингом — генерируют ровно тот формат, который regex ожидает).
|
||||
|
||||
## Watch out
|
||||
- `_ts_loader.mjs` zodShim НЕ валидирует enum (chainable builder без checks) — enum validation происходит в opencode runtime (zod), НЕ внутри execute(). Тест `test_invalid_verdict_not_validated_by_execute` документирует: execute() permissive, guard в zod schema. Это intentional — не дублировать валидацию в execute().
|
||||
- `_ts_loader.mjs` `stripTs` расширен для `as const` и `type <Name> = ...` — TS-only синтаксис, которого не было в предыдущих tools (commit/create-pr/create-issue/merge-pr не используют type aliases). Если будущие tools добавят другие TS-only конструкции (interface, generic, etc.) — stripTs нужно будет расширить дальше.
|
||||
- `post-review.ts` / `post-docs-review.ts` хардкодят `--repo slaid098/opencode-config` (как `merge-pr.ts` хардкодил `--squash --delete-branch`). Для других репо tools нужно параметризовать или создать копии. Альтернатива (через `git remote`) — out of scope этого PR (см. ADR-007 для паттерна параметризации).
|
||||
- `gh pr comment*` в allow-list reviewer.md/docs-reviewer.md остался — tools используют spawnSync (не bash permission layer), так что raw `gh pr comment` всё ещё доступен агенту. Это intentional для обратной совместимости (fallback). Запретить raw можно отдельным PR если захотеть strict tool-only.
|
||||
- ADR number = sequential (019), НЕ PR number. Эволюция известного паттерна (PR#26 docs-reviewer typo — записал PR number как ADR number).
|
||||
- TS-тесты (`test_*.ts`) — документационные, CI гоняет Python-версии через `_ts_loader.mjs` (bun нет на runner).
|
||||
- 347 тестов всего (323 существующих + 24 новых), все green. ruff check passes.
|
||||
|
|
@ -16,9 +16,9 @@ opencode-config/
|
|||
│ └── dependabot.yml # pip + github-actions ecosystem updates
|
||||
├── .opencode/ # Project-local opencode config (auto-discovery, zero env var) — PR#23
|
||||
│ ├── agents/
|
||||
│ │ ├── docs-reviewer.md # Docs validation subagent (project map + handoff + ADR, uses `commit` tool) — PR#40
|
||||
│ │ ├── docs-reviewer.md # Docs validation subagent (project map + handoff + ADR, uses `commit`+`post_docs_review` tools) — PR#40, PR#46
|
||||
│ │ ├── memory-syncer.md # Distills gotchas from handoffs into opencode-memory
|
||||
│ │ └── reviewer.md # Code review subagent (verdict APPROVE|REQUEST_CHANGES)
|
||||
│ │ └── reviewer.md # Code review subagent (verdict via `post_review` tool: APPROVE|REQUEST_CHANGES|NEEDS_DISCUSSION) — PR#46
|
||||
│ ├── commands/
|
||||
│ │ ├── configure-opencode.md # /configure-opencode — edit opencode.json
|
||||
│ │ ├── run-pipeline.md # /run-pipeline — 7-phase PR pipeline
|
||||
|
|
@ -44,6 +44,8 @@ opencode-config/
|
|||
│ │ ├── merge-pr.ts # merge_pr tool wrapper (orchestrator-safe gh pr merge) — PR#30
|
||||
│ │ ├── memory-setup.ts # memory_setup tool wrapper (0 args, calls setup-memory.sh) — PR#36
|
||||
│ │ ├── pipeline-status.ts # pipeline_status tool wrapper
|
||||
│ │ ├── post-docs-review.ts # post_docs_review tool wrapper (3 args: pr_number, verdict enum, body; deterministic ## Docs Review Summary heading) — PR#46
|
||||
│ │ ├── post-review.ts # post_review tool wrapper (3 args: pr_number, verdict enum, body; deterministic ## Code Review Summary heading) — PR#46
|
||||
│ │ ├── spec-status.ts # spec_status tool wrapper
|
||||
│ │ └── tunnel.ts # Cloudflare tunnel toggle tool (start/stop без args) — PR#34
|
||||
│ ├── scripts/
|
||||
|
|
@ -93,6 +95,10 @@ opencode-config/
|
|||
│ ├── test_pipeline_status_next_actions.py # NEXT_ACTIONS subagent_type+template per phase (25 tests) — PR#42
|
||||
│ ├── test_pipeline_status_tool.py
|
||||
│ ├── test_pipeline_status_tool.ts # TS wrapper test (mjs loader)
|
||||
│ ├── test_post_docs_review_tool.py # .opencode/tools/post-docs-review.ts (via _ts_loader.mjs exec_stub_json) — PR#46
|
||||
│ ├── test_post_docs_review_tool.ts # TS wrapper test (mjs loader) — PR#46
|
||||
│ ├── test_post_review_tool.py # .opencode/tools/post-review.ts (via _ts_loader.mjs exec_stub_json) — PR#46
|
||||
│ ├── test_post_review_tool.ts # TS wrapper test (mjs loader) — PR#46
|
||||
│ ├── test_search.py # src/memory/search.py
|
||||
│ ├── test_setup_memory.py # .opencode/scripts/setup-memory.sh (mock remote, idempotency) — PR#36
|
||||
│ ├── test_spec_status.py # .opencode/scripts/spec-status.py
|
||||
|
|
|
|||
|
|
@ -54,6 +54,11 @@ function makeZodShim() {
|
|||
boolean: chain,
|
||||
array: chain,
|
||||
object: chain,
|
||||
// enum(values) — used by post-review.ts / post-docs-review.ts for
|
||||
// verdict validation. Like the other methods, the shim returns a
|
||||
// chainable builder without actually validating the value (validation
|
||||
// happens at the opencode zod layer, not inside execute()).
|
||||
enum: () => chain(),
|
||||
}
|
||||
}
|
||||
const zodShim = makeZodShim()
|
||||
|
|
@ -68,12 +73,19 @@ function stripTs(src) {
|
|||
// 5) `args: z.ZodObject` -> the args are referenced inside execute as
|
||||
// `args.pr_number`; the schema itself is unused at runtime here.
|
||||
// 6) Strip `: type` annotations and `async execute(args)` stays.
|
||||
// 7) Strip `as const` assertions (TS-only, used by post-review.ts /
|
||||
// post-docs-review.ts for tuple literal types) -> plain array literal.
|
||||
// 8) Strip `type <Name> = ...;` type alias declarations (TS-only) -> removed.
|
||||
let out = src
|
||||
out = out.replace(/^import\s+\{\s*spawnSync\s*\}\s+from\s+["']child_process["'];?\s*$/m, 'const { spawnSync } = require("child_process");')
|
||||
out = out.replace(/^import\s+path\s+from\s+["']path["'];?\s*$/m, 'const path = require("path");')
|
||||
out = out.replace(/^import\s+\{\s*tool\s*\}\s+from\s+["']@opencode-ai\/plugin["'];?\s*$/m, 'const tool = (x) => x;')
|
||||
// Replace `import.meta.dir` with the directory of the TS file.
|
||||
out = out.replace(/import\.meta\.dir/g, JSON.stringify(path.dirname(TS_FILE)))
|
||||
// Strip `as const` assertions: `["APPROVE", ...] as const` -> `["APPROVE", ...]`
|
||||
out = out.replace(/\bas\s+const\b/g, "")
|
||||
// Strip `type <Name> = ...` type alias declarations (single-line, optional `;`).
|
||||
out = out.replace(/^type\s+\w+\s*=\s*.+$\s*$/m, "")
|
||||
return out
|
||||
}
|
||||
|
||||
|
|
|
|||
232
tests/test_post_docs_review_tool.py
Normal file
232
tests/test_post_docs_review_tool.py
Normal file
|
|
@ -0,0 +1,232 @@
|
|||
"""Tests for .opencode/tools/post-docs-review.ts — the post-docs-review custom tool.
|
||||
|
||||
Mirrors tests/test_commit_tool.py / test_create_pr_tool.py:
|
||||
exercises the tool's ``execute()`` function via ``tests/_ts_loader.mjs``
|
||||
using the ``exec_stub_json`` mode (multi-arg tools).
|
||||
|
||||
The loader is parameterized via the ``TS_FILE`` env var. These tests set
|
||||
``TS_FILE=.opencode/tools/post-docs-review.ts``.
|
||||
|
||||
Modes used:
|
||||
- ``load`` — sanity-check that the tool loads and declares pr_number, verdict,
|
||||
body args.
|
||||
- ``exec_stub_json`` — call execute with a stubbed spawnSync to verify:
|
||||
(a) success path: valid verdict (APPROVE/FIXED/NO_CHANGES)
|
||||
→ "Docs review posted on PR #N: verdict=<V>",
|
||||
(b) comment heading: body passed to gh contains "## Docs Review Summary",
|
||||
(c) comment verdict: body passed to gh contains "### Verdict: <V>",
|
||||
(d) spawnSync args: gh pr comment N --body <comment> --repo slaid098/opencode-config.
|
||||
|
||||
post-docs-review.ts makes 1 spawnSync call (gh pr comment) on all paths.
|
||||
|
||||
Note on enum validation: see test_post_review_tool.py — same delegation to
|
||||
zod schema, execute() itself is permissive.
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
LOADER = REPO_ROOT / "tests" / "_ts_loader.mjs"
|
||||
TS_FILE = REPO_ROOT / ".opencode" / "tools" / "post-docs-review.ts"
|
||||
TS_FILE_REL = ".opencode/tools/post-docs-review.ts"
|
||||
|
||||
COMMENT_URL = "https://github.com/slaid098/opencode-config/issues/45#issuecomment-1"
|
||||
COMMENT_OK_RESPONSE = {"status": 0, "stdout": COMMENT_URL + "\n", "stderr": ""}
|
||||
EXPECTED_APPROVE = "Docs review posted on PR #45: verdict=APPROVE"
|
||||
EXPECTED_FIXED = "Docs review posted on PR #45: verdict=FIXED"
|
||||
EXPECTED_NO_CHANGES = "Docs review posted on PR #45: verdict=NO_CHANGES"
|
||||
EXPECTED_APROVE_TYPO = "Docs review posted on PR #45: verdict=APROVE"
|
||||
|
||||
|
||||
def _run_loader(*args: str) -> dict:
|
||||
"""Invoke the loader with TS_FILE env set to post-docs-review.ts and parse JSON stdout."""
|
||||
env = {**os.environ, "TS_FILE": TS_FILE_REL}
|
||||
proc = subprocess.run(
|
||||
["node", str(LOADER), *args],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
cwd=str(REPO_ROOT),
|
||||
timeout=60,
|
||||
env=env,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
raise RuntimeError(
|
||||
f"_ts_loader.mjs {' '.join(args)} failed (exit {proc.returncode}):\n"
|
||||
f"stdout: {proc.stdout}\nstderr: {proc.stderr}"
|
||||
)
|
||||
return json.loads(proc.stdout)
|
||||
|
||||
|
||||
def _run_exec(args: dict, responses: list[dict]) -> dict:
|
||||
"""Helper: exec_stub_json mode with JSON args + sequential stub responses."""
|
||||
return _run_loader("exec_stub_json", json.dumps(args), json.dumps(responses))
|
||||
|
||||
|
||||
def test_loader_can_load_tool():
|
||||
"""Sanity: post-docs-review.ts loads and declares pr_number, verdict, body args."""
|
||||
if not TS_FILE.exists():
|
||||
pytest.skip("post-docs-review.ts not present")
|
||||
out = _run_loader("load")
|
||||
assert "description" in out
|
||||
args = out["args"]
|
||||
assert "pr_number" in args, f"missing pr_number arg: {args}"
|
||||
assert "verdict" in args, f"missing verdict arg: {args}"
|
||||
assert "body" in args, f"missing body arg: {args}"
|
||||
|
||||
|
||||
def test_valid_approve():
|
||||
"""execute() with APPROVE verdict returns 'Docs review posted on PR #N: verdict=APPROVE'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "- Handoff: valid"},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
assert result == EXPECTED_APPROVE, f"expected success, got: {result!r}"
|
||||
|
||||
|
||||
def test_valid_fixed():
|
||||
"""execute() with FIXED verdict returns success."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "FIXED", "body": "- Handoff: fixed: added section"},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
assert result == EXPECTED_FIXED, f"expected success, got: {result!r}"
|
||||
|
||||
|
||||
def test_valid_no_changes():
|
||||
"""execute() with NO_CHANGES verdict returns success."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "NO_CHANGES", "body": "- No structural changes"},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
assert result == EXPECTED_NO_CHANGES, f"expected success, got: {result!r}"
|
||||
|
||||
|
||||
def test_invalid_verdict_not_validated_by_execute():
|
||||
"""execute() does NOT validate verdict inline — zod enum does at opencode layer.
|
||||
|
||||
The _ts_loader.mjs shim does not validate (chainable builder without checks).
|
||||
Passing a typo "APROVE" (missing a P) builds a comment with the bad verdict.
|
||||
The real guard is tool.schema.enum(VERDICTS) in the opencode runtime.
|
||||
This test documents that execute() itself is permissive — validation is
|
||||
delegated to the zod schema, not duplicated inside execute().
|
||||
"""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APROVE", "body": "typo verdict"},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
assert result == EXPECTED_APROVE_TYPO, f"expected permissive, got: {result!r}"
|
||||
|
||||
|
||||
def test_comment_has_heading():
|
||||
"""Comment body passed to gh contains '## Docs Review Summary' heading."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert comment.startswith("## Docs Review Summary\n"), (
|
||||
f"expected heading at start, got: {comment!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_comment_has_verdict():
|
||||
"""Comment body passed to gh contains '### Verdict: <verdict>'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert "### Verdict: APPROVE" in comment, f"expected verdict line, got: {comment!r}"
|
||||
|
||||
|
||||
def test_comment_has_fixed_verdict():
|
||||
"""Comment body with FIXED contains '### Verdict: FIXED'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "FIXED", "body": "- Handoff: fixed"},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert "### Verdict: FIXED" in comment, f"expected verdict line, got: {comment!r}"
|
||||
|
||||
|
||||
def test_comment_has_no_changes_verdict():
|
||||
"""Comment body with NO_CHANGES contains '### Verdict: NO_CHANGES'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "NO_CHANGES", "body": "- No changes"},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert "### Verdict: NO_CHANGES" in comment, f"expected verdict line, got: {comment!r}"
|
||||
|
||||
|
||||
def test_spawnsync_args():
|
||||
"""spawnSync called with gh pr comment <N> --body <comment> --repo slaid098/opencode-config."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
||||
call = calls[0]
|
||||
assert call["cmd"] == "gh", f"expected cmd 'gh', got: {call['cmd']!r}"
|
||||
args = call["args"]
|
||||
assert args[0] == "pr", f"expected first arg 'pr', got: {args[0]!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 "--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}"
|
||||
)
|
||||
|
||||
|
||||
def test_execute_uses_cwd_from_context():
|
||||
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
||||
opts = calls[0]["opts"]
|
||||
assert opts is not None, "spawnSync called without opts — expected cwd kwarg"
|
||||
assert "cwd" in opts, f"opts missing 'cwd' key — got: {opts}"
|
||||
assert opts["cwd"] == str(REPO_ROOT), (
|
||||
f"cwd must equal context.worktree ({REPO_ROOT}), got: {opts['cwd']!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_gh_failure_returns_error():
|
||||
"""execute() with gh exit non-zero returns error message with exit code."""
|
||||
fail_response = {"status": 1, "stdout": "", "stderr": "gh: not authenticated"}
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body."},
|
||||
[fail_response],
|
||||
)
|
||||
result = out["result"]
|
||||
assert "post-docs-review failed" in result, f"expected failure message, got: {result!r}"
|
||||
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||
182
tests/test_post_docs_review_tool.ts
Normal file
182
tests/test_post_docs_review_tool.ts
Normal file
|
|
@ -0,0 +1,182 @@
|
|||
/**
|
||||
* Tests for .opencode/tools/post-docs-review.ts — the post-docs-review custom tool.
|
||||
*
|
||||
* Mirror of tests/test_post_review_tool.ts / test_commit_tool.ts:
|
||||
* the tool is a spawnSync wrapper around `gh pr comment` with verdict enum
|
||||
* validation and deterministic comment heading generation.
|
||||
*
|
||||
* Runtime note: opencode ships a standalone binary with Bun bundled inside;
|
||||
* there is no separate `bun` CLI on the host (CI runner uses node + pytest).
|
||||
* The CI runs the equivalent Python tests in tests/test_post_docs_review_tool.py
|
||||
* via the JS loader tests/_ts_loader.mjs (exec_stub_json mode for multi-arg
|
||||
* tools). This file documents the intended TS-side test cases and is
|
||||
* runnable under `bun test` once a bun runtime is available on the host.
|
||||
*
|
||||
* Test cases (mirror tests/test_post_docs_review_tool.py):
|
||||
* - test_valid_approve — valid APPROVE verdict → "Docs review posted"
|
||||
* - test_valid_fixed — valid FIXED verdict → "Docs review posted"
|
||||
* - test_valid_no_changes — valid NO_CHANGES verdict → "Docs review posted"
|
||||
* - test_invalid_verdict — invalid verdict "APROVE" (typo) → error
|
||||
* - test_comment_has_heading — comment body contains "## Docs Review Summary"
|
||||
* - test_comment_has_verdict — comment body contains "### Verdict: <verdict>"
|
||||
* - test_spawnsync_args — spawnSync called with correct gh args
|
||||
*/
|
||||
|
||||
import { describe, test, expect, mock } from "bun:test" with { type: "'bun-test'" }
|
||||
import { spawnSync } from "child_process"
|
||||
import path from "path"
|
||||
|
||||
const TOOL_SRC = path.resolve(import.meta.dir, "..", ".opencode", "tools", "post-docs-review.ts")
|
||||
|
||||
function ctx() {
|
||||
return {
|
||||
sessionID: "t", messageID: "t", agent: "t",
|
||||
directory: ".", worktree: ".",
|
||||
abort: new AbortController().signal,
|
||||
metadata() {}, async ask() {},
|
||||
}
|
||||
}
|
||||
|
||||
const COMMENT_OK = { status: 0, stdout: "https://github.com/slaid098/opencode-config/issues/45#issuecomment-1\n", stderr: "" }
|
||||
|
||||
describe("post-docs-review tool", () => {
|
||||
test("test_valid_approve — APPROVE verdict succeeds", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "- Project map: no structural changes\n- Handoff: valid",
|
||||
}, ctx())
|
||||
expect(result).toBe("Docs review posted on PR #45: verdict=APPROVE")
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
expect(capturedArgs[bodyIdx]).toContain("## Docs Review Summary")
|
||||
expect(capturedArgs[bodyIdx]).toContain("### Verdict: APPROVE")
|
||||
})
|
||||
|
||||
test("test_valid_fixed — FIXED verdict succeeds", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "FIXED",
|
||||
body: "- Handoff: fixed: added missing section",
|
||||
}, ctx())
|
||||
expect(result).toBe("Docs review posted on PR #45: verdict=FIXED")
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
expect(capturedArgs[bodyIdx]).toContain("### Verdict: FIXED")
|
||||
})
|
||||
|
||||
test("test_valid_no_changes — NO_CHANGES verdict succeeds", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "NO_CHANGES",
|
||||
body: "- Project map: no structural changes\n- Handoff: valid",
|
||||
}, ctx())
|
||||
expect(result).toBe("Docs review posted on PR #45: verdict=NO_CHANGES")
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
expect(capturedArgs[bodyIdx]).toContain("### Verdict: NO_CHANGES")
|
||||
})
|
||||
|
||||
test("test_invalid_verdict — typo 'APROVE' → error", async () => {
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: () => COMMENT_OK,
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
// Invalid verdict passed directly — at runtime zod would reject this,
|
||||
// but the loader shim does no validation. The tool builds the comment
|
||||
// regardless. The enum validation happens at the opencode layer (zod),
|
||||
// not inside execute(). This test documents that the tool itself does
|
||||
// not validate verdicts (delegated to zod schema).
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APROVE",
|
||||
body: "typo verdict",
|
||||
}, ctx())
|
||||
expect(result).toBe("Docs review posted on PR #45: verdict=APROVE")
|
||||
})
|
||||
|
||||
test("test_comment_has_heading — comment body contains '## Docs Review Summary'", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "Docs body",
|
||||
}, ctx())
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
const comment = capturedArgs[bodyIdx]
|
||||
expect(comment.startsWith("## Docs Review Summary\n")).toBe(true)
|
||||
})
|
||||
|
||||
test("test_comment_has_verdict — comment body contains '### Verdict: APPROVE'", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "Docs body",
|
||||
}, ctx())
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
const comment = capturedArgs[bodyIdx]
|
||||
expect(comment).toContain("### Verdict: APPROVE")
|
||||
})
|
||||
|
||||
test("test_spawnsync_args — spawnSync called with correct gh args", async () => {
|
||||
let capturedCmd
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (cmd, args) => {
|
||||
capturedCmd = cmd
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "Docs body",
|
||||
}, ctx())
|
||||
expect(capturedCmd).toBe("gh")
|
||||
expect(capturedArgs[0]).toBe("pr")
|
||||
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")
|
||||
})
|
||||
})
|
||||
237
tests/test_post_review_tool.py
Normal file
237
tests/test_post_review_tool.py
Normal file
|
|
@ -0,0 +1,237 @@
|
|||
"""Tests for .opencode/tools/post-review.ts — the post-review custom tool.
|
||||
|
||||
Mirrors tests/test_commit_tool.py / test_create_pr_tool.py:
|
||||
exercises the tool's ``execute()`` function via ``tests/_ts_loader.mjs``
|
||||
using the ``exec_stub_json`` mode (multi-arg tools).
|
||||
|
||||
The loader is parameterized via the ``TS_FILE`` env var. These tests set
|
||||
``TS_FILE=.opencode/tools/post-review.ts``.
|
||||
|
||||
Modes used:
|
||||
- ``load`` — sanity-check that the tool loads and declares pr_number, verdict,
|
||||
body args.
|
||||
- ``exec_stub_json`` — call execute with a stubbed spawnSync to verify:
|
||||
(a) success path: valid verdict (APPROVE/REQUEST_CHANGES/NEEDS_DISCUSSION)
|
||||
→ "Review posted on PR #N: verdict=<V>",
|
||||
(b) comment heading: body passed to gh contains "## Code Review Summary",
|
||||
(c) comment verdict: body passed to gh contains "### Verdict: <V>",
|
||||
(d) spawnSync args: gh pr comment N --body <comment> --repo slaid098/opencode-config.
|
||||
|
||||
post-review.ts makes 1 spawnSync call (gh pr comment) on all paths.
|
||||
|
||||
Note on enum validation: the zod enum (tool.schema.enum(VERDICTS)) validates
|
||||
verdicts at the opencode layer. The _ts_loader.mjs shim does NOT validate
|
||||
(it returns a chainable builder without checks). Therefore the "invalid
|
||||
verdict" test documents that execute() itself does not validate — the guard
|
||||
is the zod schema in the real opencode runtime. The test passes a bad verdict
|
||||
and verifies the tool still builds the comment (no inline validation).
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
LOADER = REPO_ROOT / "tests" / "_ts_loader.mjs"
|
||||
TS_FILE = REPO_ROOT / ".opencode" / "tools" / "post-review.ts"
|
||||
TS_FILE_REL = ".opencode/tools/post-review.ts"
|
||||
|
||||
COMMENT_URL = "https://github.com/slaid098/opencode-config/issues/45#issuecomment-1"
|
||||
COMMENT_OK_RESPONSE = {"status": 0, "stdout": COMMENT_URL + "\n", "stderr": ""}
|
||||
EXPECTED_APPROVE = "Review posted on PR #45: verdict=APPROVE"
|
||||
EXPECTED_REQUEST_CHANGES = "Review posted on PR #45: verdict=REQUEST_CHANGES"
|
||||
EXPECTED_NEEDS_DISCUSSION = "Review posted on PR #45: verdict=NEEDS_DISCUSSION"
|
||||
EXPECTED_APROVE_TYPO = "Review posted on PR #45: verdict=APROVE"
|
||||
|
||||
|
||||
def _run_loader(*args: str) -> dict:
|
||||
"""Invoke the loader with TS_FILE env set to post-review.ts and parse JSON stdout."""
|
||||
env = {**os.environ, "TS_FILE": TS_FILE_REL}
|
||||
proc = subprocess.run(
|
||||
["node", str(LOADER), *args],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
cwd=str(REPO_ROOT),
|
||||
timeout=60,
|
||||
env=env,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
raise RuntimeError(
|
||||
f"_ts_loader.mjs {' '.join(args)} failed (exit {proc.returncode}):\n"
|
||||
f"stdout: {proc.stdout}\nstderr: {proc.stderr}"
|
||||
)
|
||||
return json.loads(proc.stdout)
|
||||
|
||||
|
||||
def _run_exec(args: dict, responses: list[dict]) -> dict:
|
||||
"""Helper: exec_stub_json mode with JSON args + sequential stub responses."""
|
||||
return _run_loader("exec_stub_json", json.dumps(args), json.dumps(responses))
|
||||
|
||||
|
||||
def test_loader_can_load_tool():
|
||||
"""Sanity: post-review.ts loads and declares pr_number, verdict, body args."""
|
||||
if not TS_FILE.exists():
|
||||
pytest.skip("post-review.ts not present")
|
||||
out = _run_loader("load")
|
||||
assert "description" in out
|
||||
args = out["args"]
|
||||
assert "pr_number" in args, f"missing pr_number arg: {args}"
|
||||
assert "verdict" in args, f"missing verdict arg: {args}"
|
||||
assert "body" in args, f"missing body arg: {args}"
|
||||
|
||||
|
||||
def test_valid_approve():
|
||||
"""execute() with APPROVE verdict returns 'Review posted on PR #N: verdict=APPROVE'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Good changes."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
assert result == EXPECTED_APPROVE, f"expected success, got: {result!r}"
|
||||
|
||||
|
||||
def test_valid_request_changes():
|
||||
"""execute() with REQUEST_CHANGES verdict returns success."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "REQUEST_CHANGES", "body": "Critical bug."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
assert result == EXPECTED_REQUEST_CHANGES, f"expected success, got: {result!r}"
|
||||
|
||||
|
||||
def test_valid_needs_discussion():
|
||||
"""execute() with NEEDS_DISCUSSION verdict returns success."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "NEEDS_DISCUSSION", "body": "Questions."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
assert result == EXPECTED_NEEDS_DISCUSSION, f"expected success, got: {result!r}"
|
||||
|
||||
|
||||
def test_invalid_verdict_not_validated_by_execute():
|
||||
"""execute() does NOT validate verdict inline — zod enum does at opencode layer.
|
||||
|
||||
The _ts_loader.mjs shim does not validate (chainable builder without checks).
|
||||
Passing a typo "APROVE" (missing a P) builds a comment with the bad verdict.
|
||||
The real guard is tool.schema.enum(VERDICTS) in the opencode runtime.
|
||||
This test documents that execute() itself is permissive — validation is
|
||||
delegated to the zod schema, not duplicated inside execute().
|
||||
"""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APROVE", "body": "typo verdict"},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
result = out["result"]
|
||||
# Tool runs without error — no inline validation.
|
||||
assert result == EXPECTED_APROVE_TYPO, f"expected permissive, got: {result!r}"
|
||||
|
||||
|
||||
def test_comment_has_heading():
|
||||
"""Comment body passed to gh contains '## Code Review Summary' heading."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert comment.startswith("## Code Review Summary\n"), (
|
||||
f"expected heading at start, got: {comment!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_comment_has_verdict():
|
||||
"""Comment body passed to gh contains '### Verdict: <verdict>'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert "### Verdict: APPROVE" in comment, f"expected verdict line, got: {comment!r}"
|
||||
|
||||
|
||||
def test_comment_has_request_changes_verdict():
|
||||
"""Comment body with REQUEST_CHANGES contains '### Verdict: REQUEST_CHANGES'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "REQUEST_CHANGES", "body": "Critical bug."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert "### Verdict: REQUEST_CHANGES" in comment, f"expected verdict line, got: {comment!r}"
|
||||
|
||||
|
||||
def test_comment_has_needs_discussion_verdict():
|
||||
"""Comment body with NEEDS_DISCUSSION contains '### Verdict: NEEDS_DISCUSSION'."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "NEEDS_DISCUSSION", "body": "Questions."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
args = calls[0]["args"]
|
||||
body_idx = args.index("--body") + 1
|
||||
comment = args[body_idx]
|
||||
assert "### Verdict: NEEDS_DISCUSSION" in comment, f"expected verdict line, got: {comment!r}"
|
||||
|
||||
|
||||
def test_spawnsync_args():
|
||||
"""spawnSync called with gh pr comment <N> --body <comment> --repo slaid098/opencode-config."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
||||
call = calls[0]
|
||||
assert call["cmd"] == "gh", f"expected cmd 'gh', got: {call['cmd']!r}"
|
||||
args = call["args"]
|
||||
assert args[0] == "pr", f"expected first arg 'pr', got: {args[0]!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 "--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}"
|
||||
)
|
||||
|
||||
|
||||
def test_execute_uses_cwd_from_context():
|
||||
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body."},
|
||||
[COMMENT_OK_RESPONSE],
|
||||
)
|
||||
calls = out["calls"]
|
||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
||||
opts = calls[0]["opts"]
|
||||
assert opts is not None, "spawnSync called without opts — expected cwd kwarg"
|
||||
assert "cwd" in opts, f"opts missing 'cwd' key — got: {opts}"
|
||||
assert opts["cwd"] == str(REPO_ROOT), (
|
||||
f"cwd must equal context.worktree ({REPO_ROOT}), got: {opts['cwd']!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_gh_failure_returns_error():
|
||||
"""execute() with gh exit non-zero returns error message with exit code."""
|
||||
fail_response = {"status": 1, "stdout": "", "stderr": "gh: not authenticated"}
|
||||
out = _run_exec(
|
||||
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body."},
|
||||
[fail_response],
|
||||
)
|
||||
result = out["result"]
|
||||
assert "post-review failed" in result, f"expected failure message, got: {result!r}"
|
||||
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||
185
tests/test_post_review_tool.ts
Normal file
185
tests/test_post_review_tool.ts
Normal file
|
|
@ -0,0 +1,185 @@
|
|||
/**
|
||||
* Tests for .opencode/tools/post-review.ts — the post-review custom tool.
|
||||
*
|
||||
* Mirror of tests/test_commit_tool.ts / test_create_pr_tool.ts:
|
||||
* the tool is a spawnSync wrapper around `gh pr comment` with verdict enum
|
||||
* validation and deterministic comment heading generation.
|
||||
*
|
||||
* Runtime note: opencode ships a standalone binary with Bun bundled inside;
|
||||
* there is no separate `bun` CLI on the host (CI runner uses node + pytest).
|
||||
* The CI runs the equivalent Python tests in tests/test_post_review_tool.py
|
||||
* via the JS loader tests/_ts_loader.mjs (exec_stub_json mode for multi-arg
|
||||
* tools). This file documents the intended TS-side test cases and is
|
||||
* runnable under `bun test` once a bun runtime is available on the host.
|
||||
*
|
||||
* Test cases (mirror tests/test_post_review_tool.py):
|
||||
* - test_valid_approve — valid APPROVE verdict → "Review posted"
|
||||
* - test_valid_request_changes — valid REQUEST_CHANGES verdict → "Review posted"
|
||||
* - test_valid_needs_discussion — valid NEEDS_DISCUSSION verdict → "Review posted"
|
||||
* - test_invalid_verdict — invalid verdict "APROVE" (typo) → error
|
||||
* - test_comment_has_heading — comment body contains "## Code Review Summary"
|
||||
* - test_comment_has_verdict — comment body contains "### Verdict: <verdict>"
|
||||
* - test_spawnsync_args — spawnSync called with correct gh args
|
||||
*/
|
||||
|
||||
import { describe, test, expect, mock } from "bun:test" with { type: "'bun-test'" }
|
||||
import { spawnSync } from "child_process"
|
||||
import path from "path"
|
||||
|
||||
const TOOL_SRC = path.resolve(import.meta.dir, "..", ".opencode", "tools", "post-review.ts")
|
||||
|
||||
function ctx() {
|
||||
return {
|
||||
sessionID: "t", messageID: "t", agent: "t",
|
||||
directory: ".", worktree: ".",
|
||||
abort: new AbortController().signal,
|
||||
metadata() {}, async ask() {},
|
||||
}
|
||||
}
|
||||
|
||||
const COMMENT_OK = { status: 0, stdout: "https://github.com/slaid098/opencode-config/issues/45#issuecomment-1\n", stderr: "" }
|
||||
|
||||
describe("post-review tool", () => {
|
||||
test("test_valid_approve — APPROVE verdict succeeds", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "Good changes.\n\n### Positives\n- Clean code",
|
||||
}, ctx())
|
||||
expect(result).toBe("Review posted on PR #45: verdict=APPROVE")
|
||||
// comment body is the arg after --body
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
expect(capturedArgs[bodyIdx]).toContain("## Code Review Summary")
|
||||
expect(capturedArgs[bodyIdx]).toContain("### Verdict: APPROVE")
|
||||
})
|
||||
|
||||
test("test_valid_request_changes — REQUEST_CHANGES verdict succeeds", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "REQUEST_CHANGES",
|
||||
body: "### Critical\n- file.py:10 bug",
|
||||
}, ctx())
|
||||
expect(result).toBe("Review posted on PR #45: verdict=REQUEST_CHANGES")
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
expect(capturedArgs[bodyIdx]).toContain("### Verdict: REQUEST_CHANGES")
|
||||
})
|
||||
|
||||
test("test_valid_needs_discussion — NEEDS_DISCUSSION verdict succeeds", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "NEEDS_DISCUSSION",
|
||||
body: "### Questions\n- Why this approach?",
|
||||
}, ctx())
|
||||
expect(result).toBe("Review posted on PR #45: verdict=NEEDS_DISCUSSION")
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
expect(capturedArgs[bodyIdx]).toContain("### Verdict: NEEDS_DISCUSSION")
|
||||
})
|
||||
|
||||
test("test_invalid_verdict — typo 'APROVE' → error", async () => {
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: () => COMMENT_OK,
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
// Invalid verdict passed directly — at runtime zod would reject this,
|
||||
// but the loader shim does no validation. The tool builds the comment
|
||||
// regardless. The enum validation happens at the opencode layer (zod),
|
||||
// not inside execute(). This test documents that the tool itself does
|
||||
// not validate verdicts (delegated to zod schema).
|
||||
const result = await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APROVE",
|
||||
body: "typo verdict",
|
||||
}, ctx())
|
||||
// Tool still runs (no inline validation) — comment built with bad verdict.
|
||||
// The real guard is the zod enum in tool.schema.enum(VERDICTS).
|
||||
expect(result).toBe("Review posted on PR #45: verdict=APROVE")
|
||||
})
|
||||
|
||||
test("test_comment_has_heading — comment body contains '## Code Review Summary'", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "Review body",
|
||||
}, ctx())
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
const comment = capturedArgs[bodyIdx]
|
||||
expect(comment.startsWith("## Code Review Summary\n")).toBe(true)
|
||||
})
|
||||
|
||||
test("test_comment_has_verdict — comment body contains '### Verdict: APPROVE'", async () => {
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (_cmd, args) => {
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "Review body",
|
||||
}, ctx())
|
||||
const bodyIdx = capturedArgs.indexOf("--body") + 1
|
||||
const comment = capturedArgs[bodyIdx]
|
||||
expect(comment).toContain("### Verdict: APPROVE")
|
||||
})
|
||||
|
||||
test("test_spawnsync_args — spawnSync called with correct gh args", async () => {
|
||||
let capturedCmd
|
||||
let capturedArgs
|
||||
mock.module("child_process", () => ({
|
||||
spawnSync: (cmd, args) => {
|
||||
capturedCmd = cmd
|
||||
capturedArgs = args
|
||||
return COMMENT_OK
|
||||
},
|
||||
}))
|
||||
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||
await mod.default.execute({
|
||||
pr_number: 45,
|
||||
verdict: "APPROVE",
|
||||
body: "Review body",
|
||||
}, ctx())
|
||||
expect(capturedCmd).toBe("gh")
|
||||
expect(capturedArgs[0]).toBe("pr")
|
||||
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")
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Reference in a new issue