refactor(tools): add repo parameter + shared module + tunnel tests (#65)
* refactor(tools): extract shared module for gh spawnSync logic * feat(tools): add repo parameter to 5 GitHub tools * test(tools): add tunnel tool tests * test(tools): add repo parameter test cases for 5 tools * docs(handoff): scaffold handoff and ADR for PR * docs(handoff): set PR number * docs(project-map): update after PR#65 structural changes --------- Co-authored-by: opencode-agent <agent@opencode.local>
This commit is contained in:
parent
a2666183f8
commit
15fc7d014d
22 changed files with 1501 additions and 50 deletions
39
.opencode/tools/_shared.ts
Normal file
39
.opencode/tools/_shared.ts
Normal file
|
|
@ -0,0 +1,39 @@
|
||||||
|
import { spawnSync } from "child_process"
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build the `--repo <owner/repo>` argv fragment for `gh`.
|
||||||
|
*
|
||||||
|
* If `repo` is provided, returns `["--repo", repo]` (explicit target).
|
||||||
|
* If omitted, returns `[]` so `gh` auto-detects the repo from its `cwd`
|
||||||
|
* (set to `context.worktree` by `runGh`). Auto-detect is the default and
|
||||||
|
* backward-compatible behaviour for all 5 GitHub tools (create-issue,
|
||||||
|
* create-pr, post-review, post-docs-review, merge-pr) — see ADR-027.
|
||||||
|
*/
|
||||||
|
export function parseRepo(repo?: string): string[] {
|
||||||
|
if (repo) return ["--repo", repo]
|
||||||
|
return []
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Run `gh` with the given subcommand args, optionally targeting an explicit
|
||||||
|
* repo. When `repo` is omitted, `gh` auto-detects the repo from `opts.cwd`
|
||||||
|
* (callers pass `context.worktree`). Returns the raw spawnSync result so the
|
||||||
|
* caller can inspect `status`/`stdout`/`stderr` directly, or pass it to
|
||||||
|
* `formatResult` for the standard error string.
|
||||||
|
*/
|
||||||
|
export function runGh(args: string[], repo?: string, opts?: { cwd?: string }) {
|
||||||
|
const fullArgs = [...parseRepo(repo), ...args]
|
||||||
|
return spawnSync("gh", fullArgs, { encoding: "utf-8", cwd: opts?.cwd })
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Standard success/error formatter for GitHub tools.
|
||||||
|
*
|
||||||
|
* On success (exit 0) returns `stdout.trim()`. On failure returns the
|
||||||
|
* canonical error string `⚠️ <toolName> failed (exit <code>): <stderr or
|
||||||
|
* stdout>`, matching the format previously duplicated across 5 tools.
|
||||||
|
*/
|
||||||
|
export function formatResult(r: { status: number | null; stdout: string; stderr: string }, toolName: string): string {
|
||||||
|
if (r.status === 0) return r.stdout.trim()
|
||||||
|
return `⚠️ ${toolName} failed (exit ${r.status}): ${r.stderr || r.stdout}`
|
||||||
|
}
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
import { spawnSync } from "child_process"
|
|
||||||
import { tool } from "@opencode-ai/plugin"
|
import { tool } from "@opencode-ai/plugin"
|
||||||
|
import { runGh, formatResult } from "./_shared"
|
||||||
|
|
||||||
const TITLE_REGEX = /^(feat|fix|chore|docs|refactor|test|style|perf)\([^)]+\): .{1,80}$/
|
const TITLE_REGEX = /^(feat|fix|chore|docs|refactor|test|style|perf)\([^)]+\): .{1,80}$/
|
||||||
const CYRILLIC = /[\u0400-\u04FF]/
|
const CYRILLIC = /[\u0400-\u04FF]/
|
||||||
|
|
@ -20,6 +20,7 @@ export default tool({
|
||||||
title: tool.schema.string().describe("Issue title (conventional format: type(scope): description, <=80 chars)"),
|
title: tool.schema.string().describe("Issue title (conventional format: type(scope): description, <=80 chars)"),
|
||||||
body: tool.schema.string().describe("Issue body in Russian with ## Контекст, ## Задача, ## Критерии приемки headings"),
|
body: tool.schema.string().describe("Issue body in Russian with ## Контекст, ## Задача, ## Критерии приемки headings"),
|
||||||
labels: tool.schema.array(tool.schema.string()).optional().describe("Labels to assign (e.g. ['bug', 'enhancement'])"),
|
labels: tool.schema.array(tool.schema.string()).optional().describe("Labels to assign (e.g. ['bug', 'enhancement'])"),
|
||||||
|
repo: tool.schema.string().optional().describe("Optional repo (owner/name). If omitted, gh auto-detects from context.worktree."),
|
||||||
},
|
},
|
||||||
async execute(args, context) {
|
async execute(args, context) {
|
||||||
const title = args.title
|
const title = args.title
|
||||||
|
|
@ -49,12 +50,9 @@ export default tool({
|
||||||
ghArgs.push("--label", args.labels.join(","))
|
ghArgs.push("--label", args.labels.join(","))
|
||||||
}
|
}
|
||||||
|
|
||||||
const r = spawnSync("gh", ghArgs, {
|
const r = runGh(ghArgs, args.repo, { cwd: context.worktree })
|
||||||
encoding: "utf-8",
|
|
||||||
cwd: context.worktree,
|
|
||||||
})
|
|
||||||
if (r.status !== 0) {
|
if (r.status !== 0) {
|
||||||
return `⚠️ gh issue create failed (exit ${r.status}): ${r.stderr || r.stdout}`
|
return formatResult(r, "gh issue create")
|
||||||
}
|
}
|
||||||
|
|
||||||
return `Issue created: ${r.stdout.trim()}`
|
return `Issue created: ${r.stdout.trim()}`
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
import { spawnSync } from "child_process"
|
|
||||||
import { tool } from "@opencode-ai/plugin"
|
import { tool } from "@opencode-ai/plugin"
|
||||||
|
import { runGh, formatResult } from "./_shared"
|
||||||
|
|
||||||
const TITLE_REGEX = /^(feat|fix|chore|docs|refactor|test|style|perf)\([^)]+\): .{1,72}$/
|
const TITLE_REGEX = /^(feat|fix|chore|docs|refactor|test|style|perf)\([^)]+\): .{1,72}$/
|
||||||
const CYRILLIC = /[\u0400-\u04FF]/
|
const CYRILLIC = /[\u0400-\u04FF]/
|
||||||
|
|
@ -19,6 +19,7 @@ export default tool({
|
||||||
title: tool.schema.string().describe("PR title (conventional format: type(scope): description)"),
|
title: tool.schema.string().describe("PR title (conventional format: type(scope): description)"),
|
||||||
body: tool.schema.string().describe("PR body in Russian with ## Что сделано and ## Почему headings"),
|
body: tool.schema.string().describe("PR body in Russian with ## Что сделано and ## Почему headings"),
|
||||||
issue_number: tool.schema.number().optional().describe("Issue number to link (appends 'Closes #N' to body)"),
|
issue_number: tool.schema.number().optional().describe("Issue number to link (appends 'Closes #N' to body)"),
|
||||||
|
repo: tool.schema.string().optional().describe("Optional repo (owner/name). If omitted, gh auto-detects from context.worktree."),
|
||||||
},
|
},
|
||||||
async execute(args, context) {
|
async execute(args, context) {
|
||||||
const title = args.title
|
const title = args.title
|
||||||
|
|
@ -44,12 +45,9 @@ export default tool({
|
||||||
body = body + "\n\nCloses #" + args.issue_number
|
body = body + "\n\nCloses #" + args.issue_number
|
||||||
}
|
}
|
||||||
|
|
||||||
const r = spawnSync("gh", ["pr", "create", "--title", title, "--body", body], {
|
const r = runGh(["pr", "create", "--title", title, "--body", body], args.repo, { cwd: context.worktree })
|
||||||
encoding: "utf-8",
|
|
||||||
cwd: context.worktree,
|
|
||||||
})
|
|
||||||
if (r.status !== 0) {
|
if (r.status !== 0) {
|
||||||
return `⚠️ gh pr create failed (exit ${r.status}): ${r.stderr || r.stdout}`
|
return formatResult(r, "gh pr create")
|
||||||
}
|
}
|
||||||
|
|
||||||
return `PR created: ${r.stdout.trim()}`
|
return `PR created: ${r.stdout.trim()}`
|
||||||
|
|
|
||||||
|
|
@ -1,19 +1,18 @@
|
||||||
import { spawnSync } from "child_process"
|
|
||||||
import { tool } from "@opencode-ai/plugin"
|
import { tool } from "@opencode-ai/plugin"
|
||||||
|
import { runGh } from "./_shared"
|
||||||
|
|
||||||
export default tool({
|
export default tool({
|
||||||
description: "Merge a PR via squash + delete branch. Orchestrator-safe wrapper for `gh pr merge N --squash --delete-branch`. Main agent calls this tool instead of raw bash, aligning with the pure-orchestrator model (tool-led philosophy, see ADR-010).",
|
description: "Merge a PR via squash + delete branch. Orchestrator-safe wrapper for `gh pr merge N --squash --delete-branch`. Main agent calls this tool instead of raw bash, aligning with the pure-orchestrator model (tool-led philosophy, see ADR-010).",
|
||||||
args: {
|
args: {
|
||||||
pr_number: tool.schema.number().describe("PR number to merge"),
|
pr_number: tool.schema.number().describe("PR number to merge"),
|
||||||
|
repo: tool.schema.string().optional().describe("Optional repo (owner/name). If omitted, gh auto-detects from context.worktree."),
|
||||||
},
|
},
|
||||||
async execute(args, context) {
|
async execute(args, context) {
|
||||||
const r = spawnSync("gh", [
|
const r = runGh(
|
||||||
"pr", "merge", String(args.pr_number),
|
["pr", "merge", String(args.pr_number), "--squash", "--delete-branch"],
|
||||||
"--squash", "--delete-branch",
|
args.repo,
|
||||||
], {
|
{ cwd: context.worktree },
|
||||||
encoding: "utf-8",
|
)
|
||||||
cwd: context.worktree,
|
|
||||||
})
|
|
||||||
|
|
||||||
if (r.status !== 0) {
|
if (r.status !== 0) {
|
||||||
return `⚠️ merge_pr failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
return `⚠️ merge_pr failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
||||||
|
|
@ -21,4 +20,4 @@ export default tool({
|
||||||
|
|
||||||
return `PR #${args.pr_number} merged successfully (squash, branch deleted).`
|
return `PR #${args.pr_number} merged successfully (squash, branch deleted).`
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
import { spawnSync } from "child_process"
|
|
||||||
import { tool } from "@opencode-ai/plugin"
|
import { tool } from "@opencode-ai/plugin"
|
||||||
|
import { runGh } from "./_shared"
|
||||||
|
|
||||||
const VERDICTS = ["APPROVE", "FIXED", "NO_CHANGES"] as const
|
const VERDICTS = ["APPROVE", "FIXED", "NO_CHANGES"] as const
|
||||||
type Verdict = typeof VERDICTS[number]
|
type Verdict = typeof VERDICTS[number]
|
||||||
|
|
@ -10,13 +10,11 @@ export default tool({
|
||||||
pr_number: tool.schema.number().describe("PR number to comment on"),
|
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"),
|
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)"),
|
body: tool.schema.string().describe("Docs review body text (without heading — heading is auto-generated)"),
|
||||||
|
repo: tool.schema.string().optional().describe("Optional repo (owner/name). If omitted, gh auto-detects from context.worktree."),
|
||||||
},
|
},
|
||||||
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], {
|
const r = runGh(["pr", "comment", String(args.pr_number), "--body", comment], args.repo, { cwd: context.worktree })
|
||||||
encoding: "utf-8",
|
|
||||||
cwd: context.worktree,
|
|
||||||
})
|
|
||||||
if (r.status !== 0) {
|
if (r.status !== 0) {
|
||||||
return `⚠️ post-docs-review failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
return `⚠️ post-docs-review failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
import { spawnSync } from "child_process"
|
|
||||||
import { tool } from "@opencode-ai/plugin"
|
import { tool } from "@opencode-ai/plugin"
|
||||||
|
import { runGh } from "./_shared"
|
||||||
|
|
||||||
const VERDICTS = ["APPROVE", "REQUEST_CHANGES", "NEEDS_DISCUSSION"] as const
|
const VERDICTS = ["APPROVE", "REQUEST_CHANGES", "NEEDS_DISCUSSION"] as const
|
||||||
type Verdict = typeof VERDICTS[number]
|
type Verdict = typeof VERDICTS[number]
|
||||||
|
|
@ -10,13 +10,11 @@ export default tool({
|
||||||
pr_number: tool.schema.number().describe("PR number to comment on"),
|
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"),
|
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)"),
|
body: tool.schema.string().describe("Review body text (without heading — heading is auto-generated)"),
|
||||||
|
repo: tool.schema.string().optional().describe("Optional repo (owner/name). If omitted, gh auto-detects from context.worktree."),
|
||||||
},
|
},
|
||||||
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], {
|
const r = runGh(["pr", "comment", String(args.pr_number), "--body", comment], args.repo, { cwd: context.worktree })
|
||||||
encoding: "utf-8",
|
|
||||||
cwd: context.worktree,
|
|
||||||
})
|
|
||||||
if (r.status !== 0) {
|
if (r.status !== 0) {
|
||||||
return `⚠️ post-review failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
return `⚠️ post-review failed for PR #${args.pr_number} (exit ${r.status}): ${r.stderr || r.stdout}`
|
||||||
}
|
}
|
||||||
|
|
|
||||||
156
docs/decisions/027-pr-65-tools-refactor-shared-module.md
Normal file
156
docs/decisions/027-pr-65-tools-refactor-shared-module.md
Normal file
|
|
@ -0,0 +1,156 @@
|
||||||
|
# ADR-027: Shared module for GitHub tools + repo parameter + tunnel tests
|
||||||
|
|
||||||
|
## Статус
|
||||||
|
|
||||||
|
Accepted (2026-07-25)
|
||||||
|
|
||||||
|
## Контекст
|
||||||
|
|
||||||
|
Research (subagent explore, PR#63) выявил 3 проблемы в GitHub tools
|
||||||
|
инфраструктуре репо `slaid098/opencode-config`:
|
||||||
|
|
||||||
|
### Проблема 1: Дублирование spawnSync логики
|
||||||
|
|
||||||
|
Каждый из 5 GitHub tools (`create-issue.ts`, `create-pr.ts`, `post-review.ts`,
|
||||||
|
`post-docs-review.ts`, `merge-pr.ts`) дублировал:
|
||||||
|
|
||||||
|
- `spawnSync("gh", [...], { encoding: "utf-8", cwd: context.worktree })` —
|
||||||
|
~5 строк на tool
|
||||||
|
- `if (r.status !== 0) return "⚠️ ... failed (exit ...): ..."` — ~3 строки
|
||||||
|
на tool
|
||||||
|
- Итого: ~10 строк × 5 файлов = ~50 строк дублирования
|
||||||
|
|
||||||
|
Дублирование означает: изменение canonical error-формата или spawnSync
|
||||||
|
options (например, добавление timeout) требует правки 5 файлов, дрейф
|
||||||
|
вероятен.
|
||||||
|
|
||||||
|
### Проблема 2: Нет `repo` параметра
|
||||||
|
|
||||||
|
Tools работали только через auto-detect: `gh` CLI сам определяет owner/repo
|
||||||
|
из `git remote get-url origin` в текущей директории (`context.worktree`).
|
||||||
|
Ограничения:
|
||||||
|
|
||||||
|
- Из не-git-директории (например `/root/workspace`) — `gh` не может
|
||||||
|
определить remote → fall back на hardcoded `--repo` (что было багом
|
||||||
|
issue #60, исправлено в PR#61 / ADR-025).
|
||||||
|
- Нет явного способа указать repo при вызове tool — теряется гибкость.
|
||||||
|
- Для pipeline оркестрации было бы полезно передавать repo явно (единый
|
||||||
|
стандарт вместо полагания на cwd).
|
||||||
|
|
||||||
|
ADR-025 (PR#61) убрал хардкод `--repo slaid098/opencode-config`, оставив
|
||||||
|
auto-detect. Этот PR делает следующий шаг: `repo` становится опциональным
|
||||||
|
параметром (явный путь вместо хардкода), auto-detect остаётся default.
|
||||||
|
|
||||||
|
### Проблема 3: Tunnel без тестов
|
||||||
|
|
||||||
|
Tool `tunnel` — единственный из 10 tools без парных `.py` + `.ts` тестов.
|
||||||
|
Все остальные tools покрыты (`tests/test_commit_tool.py/.ts`,
|
||||||
|
`tests/test_create_issue_tool.py/.ts`, etc.). Tunnel tool: беспараметровый,
|
||||||
|
toggle-логика в `tunnel.sh` (PID-файл `/tmp/tunnel.pid`, `kill -0` проверка
|
||||||
|
живости, stale cleanup, `CLOUDFLARE_TUNNEL_TOKEN` обязателен). Регрессии в
|
||||||
|
toggle/PID-логике проходят незамеченными.
|
||||||
|
|
||||||
|
## Решение
|
||||||
|
|
||||||
|
### 1. Shared module `.opencode/tools/_shared.ts`
|
||||||
|
|
||||||
|
Создан модуль с 3 функциями:
|
||||||
|
|
||||||
|
- `parseRepo(repo?: string): string[]` — если `repo` передан, возвращает
|
||||||
|
`["--repo", repo]`; если нет — `[]` (auto-detect через gh из cwd).
|
||||||
|
- `runGh(args: string[], repo?: string, opts?: { cwd?: string })` —
|
||||||
|
`spawnSync("gh", [...parseRepo(repo), ...args], { encoding: "utf-8", cwd:
|
||||||
|
opts?.cwd })`. Prepends `--repo <name>` к gh argv когда repo явный.
|
||||||
|
- `formatResult(r, toolName: string): string` — на успехе (exit 0)
|
||||||
|
`r.stdout.trim()`, на ошибке canonical `⚠️ <toolName> failed (exit <code>):
|
||||||
|
<stderr || stdout>`.
|
||||||
|
|
||||||
|
5 GitHub tools используют `runGh` (устраняет дублирование spawnSync).
|
||||||
|
`formatResult` используют только `create-issue.ts` / `create-pr.ts` (их
|
||||||
|
error-формат точно совпадает). `post-review.ts` / `post-docs-review.ts` /
|
||||||
|
`merge-pr.ts` сохраняют кастомный error с PR номером (`⚠️ ... failed for PR
|
||||||
|
#N (exit K): ...`) для диагностики — `formatResult` потерял бы PR номер.
|
||||||
|
|
||||||
|
### 2. `repo?: string` параметр (backward-compatible)
|
||||||
|
|
||||||
|
Все 5 GitHub tools приняли опциональный `repo?: string`:
|
||||||
|
|
||||||
|
- При `repo` передан → `runGh` prepends `["--repo", repo]` → gh targetит
|
||||||
|
явный owner/name независимо от cwd.
|
||||||
|
- При `repo` omitted → `parseRepo` возвращает `[]` → `--repo` НЕ
|
||||||
|
добавляется → gh auto-detect'ит из `context.worktree` (cwd) — поведение
|
||||||
|
идентично pre-refactor (ADR-025 / PR#61).
|
||||||
|
|
||||||
|
Backward-compatibility подтверждена: существующие тесты `test_spawnsync_args`
|
||||||
|
(ассертят `"--repo" not in args`) + новые `test_repo_omitted_no_repo_flag`
|
||||||
|
проходят. 51→401 тестов (375 baseline + 26 новых).
|
||||||
|
|
||||||
|
### 3. `_ts_loader.mjs` extension для relative imports
|
||||||
|
|
||||||
|
Loader (`tests/_ts_loader.mjs`) использует `new Function` sandbox и умел
|
||||||
|
только `child_process` / `path` / `@opencode-ai/plugin`. Relative imports
|
||||||
|
(`./_shared`) падали с ENOENT. Расширение:
|
||||||
|
|
||||||
|
- `stripTs` стриппает `import { ... } from "./..."` + `export` на top-level
|
||||||
|
declarations + type annotations в function params (`function foo(a: Type):
|
||||||
|
Ret {` → `function foo(a) {`).
|
||||||
|
- `inlineShared()` инлайнит код shared-модуля в sandbox: читает оригинальный
|
||||||
|
файл для детекта import, грузит + стриппет shared-модуль, prepends к
|
||||||
|
`new Function` body. Авто-добавление `.ts` к specifier (`./_shared` →
|
||||||
|
`./_shared.ts`).
|
||||||
|
|
||||||
|
Без этого тесты отрефакторенных tools падали бы — loader не поддерживал
|
||||||
|
local imports.
|
||||||
|
|
||||||
|
### 4. Tunnel tests
|
||||||
|
|
||||||
|
Созданы `tests/test_tunnel_tool.py` (6 pytest) + `tests/test_tunnel_tool.ts`
|
||||||
|
(4 TS). `.py` тесты запускают `tunnel.sh` напрямую с изоляцией (копия в
|
||||||
|
`tmp_path`, переписанные PID/LOG пути, fake `cloudflared` = `sleep 30` на
|
||||||
|
PATH). Покрыты все ветви: start без токена (exit 1), start с токеном, start
|
||||||
|
с `TUNNEL_DOMAIN`, stop при живом процессе, stale PID cleanup + start,
|
||||||
|
toggle (повторный start → stop).
|
||||||
|
|
||||||
|
### 5. 5 test-файлов обновлены для `repo` параметра
|
||||||
|
|
||||||
|
Каждый из 5 tools получил 3 новых кейса: `test_repo_explicit_passed_to_gh`
|
||||||
|
(`--repo foo/bar` prepended), `test_repo_omitted_no_repo_flag` (no `--repo`,
|
||||||
|
backward-compat), `test_repo_invalid_gh_error` (invalid repo + gh failure →
|
||||||
|
tool-specific error). `test_merge_pr_tool.py/.ts` созданы с нуля (merge-pr
|
||||||
|
не имел тестов ранее) — 8 .py + 5 .ts покрывают базовые кейсы + repo.
|
||||||
|
|
||||||
|
## Альтернативы
|
||||||
|
|
||||||
|
- **Оставить дублирование как есть** — отклонено: ~50 строк дублирования
|
||||||
|
растёт с каждым новым gh tool, дрейф в error-формате/spawnSync options
|
||||||
|
вероятен. Shared module — canonical источник, изменение в 1 месте.
|
||||||
|
|
||||||
|
- **Mandatory `repo` параметр (убрать auto-detect)** — отклонено: теряет
|
||||||
|
backward-compatibility. Существующие вызовы tools (из pipeline, из
|
||||||
|
агентов) не передают `repo` — полагаются на auto-detect из
|
||||||
|
`context.worktree`. Mandatory `repo` сломал бы все вызовы, потребовал бы
|
||||||
|
обновления skills/agents. Опциональный `repo?: string` сохраняет
|
||||||
|
auto-detect default + даёт явный путь когда нужно.
|
||||||
|
|
||||||
|
- **`formatResult` для всех 5 tools (убрать кастомный error с PR номером)** —
|
||||||
|
отклонено: post-review/post-docs-review/merge-pr error-сообщения
|
||||||
|
содержат `for PR #N` — полезно для диагностики (какой PR fail'нул).
|
||||||
|
`formatResult` дал бы generic `⚠️ post-review failed (exit K): ...` без
|
||||||
|
PR. Потеря информации > экономия 3 строк × 3 файла. `formatResult`
|
||||||
|
используется где error-формат точно совпадает (create-issue/create-pr).
|
||||||
|
|
||||||
|
- **Параметризовать `--repo` через `git remote get-url origin` (как
|
||||||
|
ADR-007 для pipeline-status.py)** — отклонено: достаточно опционального
|
||||||
|
`repo` + auto-detect. Параметризация через git remote добавила бы spawn
|
||||||
|
вызов в каждый tool без выгоды (auto-detect из cwd справляется, а явный
|
||||||
|
`repo` покрывает edge-cases). ADR-025 rejected alt пересмотрена.
|
||||||
|
|
||||||
|
- **Inline shared-логику в каждый tool без отдельного модуля** — отклонено:
|
||||||
|
это и есть текущее дублирование (проблема 1). Вынос в `_shared.ts` —
|
||||||
|
canonical источник, тестируемый отдельно.
|
||||||
|
|
||||||
|
- **Не расширять `_ts_loader.mjs`, тестировать только через `.ts` (bun)** —
|
||||||
|
отклонено: CI runner не имеет `bun` (opencode binary с bundled bun, нет
|
||||||
|
отдельного CLI). `.ts` тесты документационные; реальные проверки идут
|
||||||
|
через `.py` + loader. Без расширения loader'а `.py` тесты отрефакторенных
|
||||||
|
tools падали бы.
|
||||||
104
docs/handoff/pr-65-tools-refactor-shared-module.md
Normal file
104
docs/handoff/pr-65-tools-refactor-shared-module.md
Normal file
|
|
@ -0,0 +1,104 @@
|
||||||
|
---
|
||||||
|
pr: 65
|
||||||
|
title: refactor(tools): add repo parameter + shared module + tunnel tests
|
||||||
|
---
|
||||||
|
|
||||||
|
## Что сделано
|
||||||
|
|
||||||
|
- **`.opencode/tools/_shared.ts`** — новый shared module: `parseRepo(repo?)`
|
||||||
|
(возвращает `["--repo", repo]` или `[]` для auto-detect), `runGh(args, repo?,
|
||||||
|
opts?)` (spawnSync с prepended `--repo` при явном repo + `cwd` из opts),
|
||||||
|
`formatResult(r, toolName)` (canonical `⚠️ <tool> failed (exit K): ...` на
|
||||||
|
ошибке, `stdout.trim()` на успехе). Устраняет ~50 строк дублирования
|
||||||
|
spawnSync-логики в 5 GitHub tools.
|
||||||
|
- **`tests/_ts_loader.mjs`** — расширение для поддержки relative imports
|
||||||
|
(`./_shared`): `stripTs` стриппает `import { ... } from "./..."` + `export`
|
||||||
|
на top-level declarations + type annotations в function params; новая
|
||||||
|
`inlineShared()` инлайнит код shared-модуля в `new Function` sandbox (с
|
||||||
|
авто-добавлением `.ts` расширения к specifier). Без этого loader падал на
|
||||||
|
ENOENT — local imports не резолвились.
|
||||||
|
- **5 GitHub tools отрефакторены** на `runGh`/`formatResult` + добавлен
|
||||||
|
опциональный `repo?: string` параметр (backward-compatible):
|
||||||
|
- `create-issue.ts` — `runGh` + `formatResult(r, "gh issue create")`
|
||||||
|
- `create-pr.ts` — `runGh` + `formatResult(r, "gh pr create")`
|
||||||
|
- `post-review.ts` — `runGh` (error-ветка оставлена кастомной с PR номером)
|
||||||
|
- `post-docs-review.ts` — `runGh` (аналогично)
|
||||||
|
- `merge-pr.ts` — `runGh` (аналогично)
|
||||||
|
При `repo` omitted → `parseRepo` возвращает `[]`, gh auto-detect'ит из
|
||||||
|
`context.worktree` (cwd) — поведение идентично pre-refactor (ADR-025 / PR#61).
|
||||||
|
- **Tunnel tests созданы** (issue #64 проблема 3 — единственный tool без
|
||||||
|
тестов):
|
||||||
|
- `tests/test_tunnel_tool.py` — 6 pytest-тестов: start без токена (exit 1),
|
||||||
|
start с токеном (`started (PID: N)`), start с `TUNNEL_DOMAIN` (`domain:
|
||||||
|
...`), stop при живом процессе (`stopped` + PID-файл удалён), stale PID-файл
|
||||||
|
(чистка + start), toggle (повторный start → stop). Изоляция: копия
|
||||||
|
`tunnel.sh` в tmp с переписанными PID/LOG путями + fake `cloudflared`
|
||||||
|
(sleep 30s) на PATH.
|
||||||
|
- `tests/test_tunnel_tool.ts` — 4 TS-теста: spawnSync с `bash` + путём к
|
||||||
|
скрипту, success → trimmed stdout, failure → `⚠️ tunnel failed (exit K)`,
|
||||||
|
cwd из context.
|
||||||
|
- **5 test-файлов обновлены** кейсами для `repo?: string` (явный / auto-detect /
|
||||||
|
invalid):
|
||||||
|
- `test_create_issue_tool.py/.ts` — +3 кейса (explicit, omitted, invalid)
|
||||||
|
- `test_create_pr_tool.py/.ts` — +3 кейса
|
||||||
|
- `test_post_review_tool.py/.ts` — +3 кейса
|
||||||
|
- `test_post_docs_review_tool.py/.ts` — +3 кейса
|
||||||
|
- `test_merge_pr_tool.py/.ts` — созданы с нуля (8 .py + 5 .ts: базовые
|
||||||
|
кейсы + repo; merge-pr не имел тестов ранее)
|
||||||
|
- **ADR-027** + этот handoff.
|
||||||
|
|
||||||
|
## Почему
|
||||||
|
|
||||||
|
Research (subagent explore, PR#63) выявил 3 проблемы в GitHub tools инфраструктуре:
|
||||||
|
|
||||||
|
1. **Дублирование spawnSync логики** — каждый из 5 GitHub tools дублировал
|
||||||
|
`spawnSync("gh", [...], { encoding, cwd })` + `if (r.status !== 0) return
|
||||||
|
"⚠️ ... failed (exit ...): ..."` — ~10 строк × 5 файлов = ~50 строк
|
||||||
|
дублирования. Shared module устраняет дублирование, единый canonical
|
||||||
|
error-формат.
|
||||||
|
2. **Нет `repo` параметра** — tools работали только через auto-detect (`gh`
|
||||||
|
определяет owner/repo из `git remote` в `context.worktree`). Ограничение:
|
||||||
|
из не-git-директории `gh` не может определить remote → fall back на
|
||||||
|
хардкод (баг issue #60, исправлен в PR#61). Явный `repo?: string` даёт
|
||||||
|
гибкость для pipeline оркестрации без потери auto-detect по умолчанию
|
||||||
|
(backward-compatible). Связано с ADR-025 (PR#61 убрал хардкод, этот PR
|
||||||
|
делает `repo` параметром — явный путь вместо хардкода).
|
||||||
|
3. **Tunnel без тестов** — `tunnel` единственный из 10 tools без парных
|
||||||
|
`.py` + `.ts` тестов. Toggle-логика (PID-файл, kill -0, stale cleanup)
|
||||||
|
не покрыта → регрессии проходят незамеченными. 6 .py + 4 .ts тестов
|
||||||
|
покрывают все ветви `tunnel.sh`.
|
||||||
|
|
||||||
|
## Pending
|
||||||
|
|
||||||
|
- После merge: на хосте `git pull` + рестарт opencode-контейнера чтобы
|
||||||
|
подхватились обновлённые tools (auto-discovered через @opencode-ai/plugin).
|
||||||
|
- Зависимости: supersedes partial PR#61 (ADR-025) — убрал хардкод `--repo`,
|
||||||
|
этот PR параметризует; related PR#63 (ADR-026) — tool-usage policy
|
||||||
|
описывает 10 tools, этот PR улучшает их реализацию.
|
||||||
|
|
||||||
|
## Watch out
|
||||||
|
|
||||||
|
- **`_ts_loader.mjs` extension** — `inlineShared()` добавляет `.ts` к
|
||||||
|
relative import specifier если его нет (`./_shared` → `./_shared.ts`).
|
||||||
|
Это необходимо потому что TS import specifiers опускают расширение, а
|
||||||
|
`readFileSync` требует полный путь. Если будущие shared-модули используют
|
||||||
|
другие расширения (`.tsx`, `.mts`) — потребуется расширить логику.
|
||||||
|
- **`formatResult` только для create-issue/create-pr** — post-review,
|
||||||
|
post-docs-review, merge-pr сохраняют кастомный error с PR номером
|
||||||
|
(`⚠️ ... failed for PR #N (exit K): ...`) для диагностики. `formatResult`
|
||||||
|
дал бы `⚠️ ... failed (exit K): ...` без PR — потеря информации. Тесты
|
||||||
|
`test_gh_failure_returns_error` проверяют substring `"post-review failed"`
|
||||||
|
/ `"exit 1"` — оба варианта проходят.
|
||||||
|
- **Backward-compat критична** — существующие `test_spawnsync_args` в
|
||||||
|
post-review/post-docs-review ассертят `"--repo" not in args`. При `repo`
|
||||||
|
omitted `parseRepo` возвращает `[]` → `--repo` не добавляется → тесты
|
||||||
|
проходят (51→62 на 4 tools). `test_repo_omitted_no_repo_flag` явно
|
||||||
|
закрепляет это поведение.
|
||||||
|
- **Tunnel test isolation** — `tunnel.sh` хардкодит `/tmp/tunnel.pid`. Тесты
|
||||||
|
копируют скрипт в `tmp_path` и переписывают PID/LOG пути через `replace()`.
|
||||||
|
Если в `tunnel.sh` изменится формат строк `PID_FILE="..."` — паттерн
|
||||||
|
replace сломается, тесты упадут (раннее обнаружение).
|
||||||
|
- **`commit.ts` НЕ в списке 5** — это `git`, не `gh` tool (нет `--repo`
|
||||||
|
флага). Оставлен как есть.
|
||||||
|
- **`.opencode/package-lock.json` untracked** — не относится к этому PR,
|
||||||
|
оставлен вне коммитов.
|
||||||
|
|
@ -39,14 +39,15 @@ opencode-config/
|
||||||
│ │ ├── run-tests/SKILL.md # Test runner guide
|
│ │ ├── run-tests/SKILL.md # Test runner guide
|
||||||
│ │ └── spec/SKILL.md # 9-phase spec generation
|
│ │ └── spec/SKILL.md # 9-phase spec generation
|
||||||
│ ├── tools/
|
│ ├── tools/
|
||||||
|
│ │ ├── _shared.ts # Shared module for GitHub tools: parseRepo(repo?), runGh(args, repo?, opts?), formatResult(r, toolName) — PR#65
|
||||||
│ │ ├── commit.ts # commit tool wrapper (1 arg message, validates format+staged) — PR#38
|
│ │ ├── commit.ts # commit tool wrapper (1 arg message, validates format+staged) — PR#38
|
||||||
│ │ ├── create-issue.ts # create-issue tool wrapper (3 args, validates format+labels) — PR#38
|
│ │ ├── create-issue.ts # create-issue tool wrapper (3 args, validates format+labels; optional repo?: string) — PR#38, PR#65
|
||||||
│ │ ├── create-pr.ts # create-pr tool wrapper (3 args, validates format+Closes #N) — PR#38
|
│ │ ├── create-pr.ts # create-pr tool wrapper (3 args, validates format+Closes #N; optional repo?: string) — PR#38, PR#65
|
||||||
│ │ ├── merge-pr.ts # merge_pr tool wrapper (orchestrator-safe gh pr merge) — PR#30
|
│ │ ├── merge-pr.ts # merge_pr tool wrapper (orchestrator-safe gh pr merge; optional repo?: string) — PR#30, PR#65
|
||||||
│ │ ├── memory-setup.ts # memory_setup tool wrapper (0 args, calls setup-memory.sh) — PR#36
|
│ │ ├── memory-setup.ts # memory_setup tool wrapper (0 args, calls setup-memory.sh) — PR#36
|
||||||
│ │ ├── pipeline-status.ts # pipeline_status tool wrapper
|
│ │ ├── 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-docs-review.ts # post_docs_review tool wrapper (3 args: pr_number, verdict enum, body; deterministic ## Docs Review Summary heading; optional repo?: string) — PR#46, PR#65
|
||||||
│ │ ├── post-review.ts # post_review tool wrapper (3 args: pr_number, verdict enum, body; deterministic ## Code Review Summary heading) — PR#46
|
│ │ ├── post-review.ts # post_review tool wrapper (3 args: pr_number, verdict enum, body; deterministic ## Code Review Summary heading; optional repo?: string) — PR#46, PR#65
|
||||||
│ │ ├── spec-status.ts # spec_status tool wrapper
|
│ │ ├── spec-status.ts # spec_status tool wrapper
|
||||||
│ │ └── tunnel.ts # Cloudflare tunnel toggle tool (start/stop без args) — PR#34
|
│ │ └── tunnel.ts # Cloudflare tunnel toggle tool (start/stop без args) — PR#34
|
||||||
│ ├── scripts/
|
│ ├── scripts/
|
||||||
|
|
@ -74,7 +75,7 @@ opencode-config/
|
||||||
│ ├── index.py # Indexing
|
│ ├── index.py # Indexing
|
||||||
│ └── search.py # Search
|
│ └── search.py # Search
|
||||||
├── tests/ # pytest + TS/MJS test suite — PR#17
|
├── tests/ # pytest + TS/MJS test suite — PR#17
|
||||||
│ ├── _ts_loader.mjs # TS test loader (load/exec_stub/exec_stub_json/exec_real modes) — PR#38
|
│ ├── _ts_loader.mjs # TS test loader (load/exec_stub/exec_stub_json/exec_real modes; relative import inlining via inlineShared()) — PR#38, PR#65
|
||||||
│ ├── test_agent_frontmatter.py # Agent frontmatter validators (no top-level doom_loop, permission.doom_loop present, steps:100) — PR#49
|
│ ├── test_agent_frontmatter.py # Agent frontmatter validators (no top-level doom_loop, permission.doom_loop present, steps:100) — PR#49
|
||||||
│ ├── test_check_adr_refs.py # adr-check.yml validator
|
│ ├── test_check_adr_refs.py # adr-check.yml validator
|
||||||
│ ├── test_check_permissions.py # permissions-check.yml validator
|
│ ├── test_check_permissions.py # permissions-check.yml validator
|
||||||
|
|
@ -83,14 +84,16 @@ opencode-config/
|
||||||
│ ├── test_dockerfile.py # Dockerfile npm install (opencode-ai + repomix + @mathew-cf/opencode-memory) — PR#57
|
│ ├── test_dockerfile.py # Dockerfile npm install (opencode-ai + repomix + @mathew-cf/opencode-memory) — PR#57
|
||||||
│ ├── test_commit_tool.py # .opencode/tools/commit.ts (via _ts_loader.mjs exec_stub_json) — PR#38
|
│ ├── test_commit_tool.py # .opencode/tools/commit.ts (via _ts_loader.mjs exec_stub_json) — PR#38
|
||||||
│ ├── test_commit_tool.ts # TS wrapper test (mjs loader) — PR#38
|
│ ├── test_commit_tool.ts # TS wrapper test (mjs loader) — PR#38
|
||||||
│ ├── test_create_issue_tool.py # .opencode/tools/create-issue.ts (via _ts_loader.mjs exec_stub_json) — PR#38
|
│ ├── test_create_issue_tool.py # .opencode/tools/create-issue.ts (via _ts_loader.mjs exec_stub_json; +repo explicit/omitted/invalid) — PR#38, PR#65
|
||||||
│ ├── test_create_issue_tool.ts # TS wrapper test (mjs loader) — PR#38
|
│ ├── test_create_issue_tool.ts # TS wrapper test (mjs loader; +repo cases) — PR#38, PR#65
|
||||||
│ ├── test_create_pr_tool.py # .opencode/tools/create-pr.ts (via _ts_loader.mjs exec_stub_json) — PR#38
|
│ ├── test_create_pr_tool.py # .opencode/tools/create-pr.ts (via _ts_loader.mjs exec_stub_json; +repo explicit/omitted/invalid) — PR#38, PR#65
|
||||||
│ ├── test_create_pr_tool.ts # TS wrapper test (mjs loader) — PR#38
|
│ ├── test_create_pr_tool.ts # TS wrapper test (mjs loader; +repo cases) — PR#38, PR#65
|
||||||
│ ├── test_embedder.py # src/memory/embedder.py (mocks AI_PROVIDER_API_URL)
|
│ ├── test_embedder.py # src/memory/embedder.py (mocks AI_PROVIDER_API_URL)
|
||||||
│ ├── test_index.py # src/memory/index.py
|
│ ├── test_index.py # src/memory/index.py
|
||||||
│ ├── test_memory_setup_tool.py # .opencode/tools/memory-setup.ts (via _ts_loader.mjs) — PR#36
|
│ ├── test_memory_setup_tool.py # .opencode/tools/memory-setup.ts (via _ts_loader.mjs) — PR#36
|
||||||
│ ├── test_memory_setup_tool.ts # TS wrapper test (mjs loader) — PR#36
|
│ ├── test_memory_setup_tool.ts # TS wrapper test (mjs loader) — PR#36
|
||||||
|
│ ├── test_merge_pr_tool.py # .opencode/tools/merge-pr.ts (via _ts_loader.mjs; 8 tests: base + repo explicit/omitted/invalid) — PR#65
|
||||||
|
│ ├── test_merge_pr_tool.ts # TS wrapper test (mjs loader; 5 tests: base + repo) — PR#65
|
||||||
│ ├── test_observability.py # .opencode/scripts/observability.py
|
│ ├── test_observability.py # .opencode/scripts/observability.py
|
||||||
│ ├── test_permissions.py # Global deny rules + agent.<name>.tools role-based access (8 tests) — PR#40
|
│ ├── test_permissions.py # Global deny rules + agent.<name>.tools role-based access (8 tests) — PR#40
|
||||||
│ ├── test_pipeline_status.py # .opencode/scripts/pipeline-status.py (REVIEW verdict branching)
|
│ ├── test_pipeline_status.py # .opencode/scripts/pipeline-status.py (REVIEW verdict branching)
|
||||||
|
|
@ -99,14 +102,16 @@ opencode-config/
|
||||||
│ ├── test_pipeline_status_next_actions.py # NEXT_ACTIONS subagent_type+template per phase (25 tests) — PR#42
|
│ ├── 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.py
|
||||||
│ ├── test_pipeline_status_tool.ts # TS wrapper test (mjs loader)
|
│ ├── 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.py # .opencode/tools/post-docs-review.ts (via _ts_loader.mjs exec_stub_json; +repo cases) — PR#46, PR#65
|
||||||
│ ├── test_post_docs_review_tool.ts # TS wrapper test (mjs loader) — PR#46
|
│ ├── test_post_docs_review_tool.ts # TS wrapper test (mjs loader; +repo cases) — PR#46, PR#65
|
||||||
│ ├── test_post_review_tool.py # .opencode/tools/post-review.ts (via _ts_loader.mjs exec_stub_json) — PR#46
|
│ ├── test_post_review_tool.py # .opencode/tools/post-review.ts (via _ts_loader.mjs exec_stub_json; +repo cases) — PR#46, PR#65
|
||||||
│ ├── test_post_review_tool.ts # TS wrapper test (mjs loader) — PR#46
|
│ ├── test_post_review_tool.ts # TS wrapper test (mjs loader; +repo cases) — PR#46, PR#65
|
||||||
│ ├── test_search.py # src/memory/search.py
|
│ ├── test_search.py # src/memory/search.py
|
||||||
│ ├── test_setup_memory.py # .opencode/scripts/setup-memory.sh (mock remote, idempotency) — PR#36
|
│ ├── test_setup_memory.py # .opencode/scripts/setup-memory.sh (mock remote, idempotency) — PR#36
|
||||||
│ ├── test_spec_status.py # .opencode/scripts/spec-status.py
|
│ ├── test_spec_status.py # .opencode/scripts/spec-status.py
|
||||||
│ └── test_spec_status_tool.py
|
│ ├── test_spec_status_tool.py
|
||||||
|
│ ├── test_tunnel_tool.py # .opencode/scripts/tunnel.sh (6 pytest: start/stop/stale PID/toggle, isolated tmp copy + fake cloudflared) — PR#65
|
||||||
|
│ └── test_tunnel_tool.ts # .opencode/tools/tunnel.ts (4 TS: spawnSync wiring, success/error/cwd) — PR#65
|
||||||
├── pyproject.toml # Python project (uv, ruff, pytest config)
|
├── pyproject.toml # Python project (uv, ruff, pytest config)
|
||||||
├── uv.lock # Locked deps for Python project
|
├── uv.lock # Locked deps for Python project
|
||||||
├── .pre-commit-config.yaml # ruff + UV hooks
|
├── .pre-commit-config.yaml # ruff + UV hooks
|
||||||
|
|
|
||||||
|
|
@ -76,23 +76,80 @@ function stripTs(src) {
|
||||||
// 7) Strip `as const` assertions (TS-only, used by post-review.ts /
|
// 7) Strip `as const` assertions (TS-only, used by post-review.ts /
|
||||||
// post-docs-review.ts for tuple literal types) -> plain array literal.
|
// post-docs-review.ts for tuple literal types) -> plain array literal.
|
||||||
// 8) Strip `type <Name> = ...;` type alias declarations (TS-only) -> removed.
|
// 8) Strip `type <Name> = ...;` type alias declarations (TS-only) -> removed.
|
||||||
|
// 9) Strip relative imports (`import { X } from "./_shared"`) — the
|
||||||
|
// referenced module is inlined by loadTool() via inlineShared(). The
|
||||||
|
// import line is removed here; the bindings come from the prepended
|
||||||
|
// shared-module code.
|
||||||
|
// 10) Strip `export ` keyword on top-level `function`/`const` declarations
|
||||||
|
// (shared modules use `export function parseRepo`). After stripping,
|
||||||
|
// the declarations become plain statements in the sandbox scope.
|
||||||
|
// 11) Strip type annotations on function parameters and return types:
|
||||||
|
// `function foo(a: Type, b?: Type2): RetType {` -> `function foo(a, b) {`.
|
||||||
|
// Needed for _shared.ts which declares typed function params.
|
||||||
let out = src
|
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+\{\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+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;')
|
out = out.replace(/^import\s+\{\s*tool\s*\}\s+from\s+["']@opencode-ai\/plugin["'];?\s*$/m, 'const tool = (x) => x;')
|
||||||
|
// Strip relative imports (`./_shared`, `../foo`, etc.) — loadTool inlines them.
|
||||||
|
out = out.replace(/^import\s+\{[^}]*\}\s+from\s+["']\.[^"']+["'];?\s*$/m, "")
|
||||||
// Replace `import.meta.dir` with the directory of the TS file.
|
// Replace `import.meta.dir` with the directory of the TS file.
|
||||||
out = out.replace(/import\.meta\.dir/g, JSON.stringify(path.dirname(TS_FILE)))
|
out = out.replace(/import\.meta\.dir/g, JSON.stringify(path.dirname(TS_FILE)))
|
||||||
// Strip `as const` assertions: `["APPROVE", ...] as const` -> `["APPROVE", ...]`
|
// Strip `as const` assertions: `["APPROVE", ...] as const` -> `["APPROVE", ...]`
|
||||||
out = out.replace(/\bas\s+const\b/g, "")
|
out = out.replace(/\bas\s+const\b/g, "")
|
||||||
// Strip `type <Name> = ...` type alias declarations (single-line, optional `;`).
|
// Strip `type <Name> = ...` type alias declarations (single-line, optional `;`).
|
||||||
out = out.replace(/^type\s+\w+\s*=\s*.+$\s*$/m, "")
|
out = out.replace(/^type\s+\w+\s*=\s*.+$\s*$/m, "")
|
||||||
|
// Strip `export ` keyword on top-level declarations (shared modules).
|
||||||
|
out = out.replace(/^export\s+(function|const|let|var)\b/gm, "$1")
|
||||||
|
// Strip type annotations on function params + return type:
|
||||||
|
// `function foo(a: Type, b?: Type2): RetType {` -> `function foo(a, b) {`
|
||||||
|
// Handles single-line function signatures (used by _shared.ts). Object
|
||||||
|
// types like `{ cwd?: string }` are matched non-greedily within a param.
|
||||||
|
out = out.replace(/^(\s*function\s+\w+\s*\()([^)]*)\)(\s*:\s*[^{]+)?\s*\{/gm, (line, head, params, _ret) => {
|
||||||
|
const cleaned = params
|
||||||
|
.split(",")
|
||||||
|
.map((p) => p.replace(/^\s*\w+/, (n) => n).replace(/:.*/, "").replace(/\?$/, "").trim())
|
||||||
|
.filter((p) => p.length > 0)
|
||||||
|
.join(", ")
|
||||||
|
return `${head}${cleaned}) {`
|
||||||
|
})
|
||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function inlineShared(rawSrc, spawnSyncImpl) {
|
||||||
|
// Detect relative imports (`./_shared`, `../foo`, etc.) in the ORIGINAL
|
||||||
|
// source (before stripTs blanks the line) and inline the shared module so
|
||||||
|
// its exported functions are in scope inside the new Function sandbox. We
|
||||||
|
// load and strip the shared module the same way as the tool, then strip its
|
||||||
|
// own `require("child_process")` / `require("path")` lines so they pick up
|
||||||
|
// the sandbox bindings instead of re-declaring identifiers.
|
||||||
|
const sharedMatch = rawSrc.match(/^import\s+\{[^}]*\}\s+from\s+["'](\.[^"']+)["'];?\s*$/m)
|
||||||
|
if (!sharedMatch) return { sharedCode: "" }
|
||||||
|
const rel = sharedMatch[1]
|
||||||
|
// Resolve the shared module relative to the tool's directory. The import
|
||||||
|
// specifier in TS omits the `.ts` extension (e.g. `./_shared`), so append it
|
||||||
|
// when the specifier has no extension.
|
||||||
|
const spec = rel.endsWith(".ts") ? rel : rel + ".ts"
|
||||||
|
const sharedPath = path.resolve(path.dirname(TS_FILE), spec)
|
||||||
|
let sharedSrc = stripTs(readFileSync(sharedPath, "utf-8"))
|
||||||
|
// Strip the same sandbox-replaced declarations so no duplicate identifiers
|
||||||
|
// clash with the new Function parameters / require shim.
|
||||||
|
sharedSrc = sharedSrc.replace(/^const\s+\{\s*spawnSync\s*\}\s*=\s*require\(["']child_process["']\);?\s*$/m, "")
|
||||||
|
sharedSrc = sharedSrc.replace(/^const\s+path\s*=\s*require\(["']path["']\);?\s*$/m, "")
|
||||||
|
sharedSrc = sharedSrc.replace(/^const\s+tool\s*=\s*\(x\)\s*=>\s*x;?\s*$/m, "")
|
||||||
|
// Remove any relative import line from the shared module (defensive — _shared
|
||||||
|
// has none today, but keeps the path open for deeper nesting).
|
||||||
|
sharedSrc = sharedSrc.replace(/^import\s+\{[^}]*\}\s+from\s+["']\.[^"']+["'];?\s*$/m, "")
|
||||||
|
return { sharedCode: sharedSrc + "\n" }
|
||||||
|
}
|
||||||
|
|
||||||
function loadTool(spawnSyncImpl) {
|
function loadTool(spawnSyncImpl) {
|
||||||
// Provide a CommonJS module sandbox so the tool file's `export default`
|
// Provide a CommonJS module sandbox so the tool file's `export default`
|
||||||
// becomes accessible via `module.exports.default`.
|
// becomes accessible via `module.exports.default`.
|
||||||
let src = stripTs(readFileSync(TS_FILE, "utf-8"))
|
const rawSrc = readFileSync(TS_FILE, "utf-8")
|
||||||
|
let src = stripTs(rawSrc)
|
||||||
|
// Inline relative imports (e.g. `./_shared`) so the tool's imported
|
||||||
|
// functions (parseRepo/runGh/formatResult) are in scope inside the sandbox.
|
||||||
|
const { sharedCode } = inlineShared(rawSrc, spawnSyncImpl)
|
||||||
// Strip the require/const declarations we replace with sandbox args, so
|
// Strip the require/const declarations we replace with sandbox args, so
|
||||||
// we don't get "Identifier already declared" between Function args and
|
// we don't get "Identifier already declared" between Function args and
|
||||||
// the in-source `const { spawnSync } = require(...)` lines.
|
// the in-source `const { spawnSync } = require(...)` lines.
|
||||||
|
|
@ -110,7 +167,7 @@ function loadTool(spawnSyncImpl) {
|
||||||
"spawnSync",
|
"spawnSync",
|
||||||
"path",
|
"path",
|
||||||
"tool",
|
"tool",
|
||||||
cjs + "\nreturn module.exports.default;",
|
sharedCode + cjs + "\nreturn module.exports.default;",
|
||||||
)
|
)
|
||||||
return fn({ exports: {} }, (name) => {
|
return fn({ exports: {} }, (name) => {
|
||||||
if (name === "child_process") return { spawnSync: spawnSyncImpl }
|
if (name === "child_process") return { spawnSync: spawnSyncImpl }
|
||||||
|
|
|
||||||
|
|
@ -190,10 +190,66 @@ def test_execute_uses_cwd_from_context():
|
||||||
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
||||||
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [ISSUE_OK_RESPONSE])
|
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [ISSUE_OK_RESPONSE])
|
||||||
calls = out["calls"]
|
calls = out["calls"]
|
||||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
opts = calls[0]["opts"]
|
opts = calls[0]["opts"]
|
||||||
assert opts is not None, "spawnSync called without opts — expected cwd kwarg"
|
assert opts is not None, "spawnSync called without opts — expected cwd kwarg"
|
||||||
assert "cwd" in opts, f"opts missing 'cwd' key — got: {opts}"
|
assert "cwd" in opts, f"opts missing 'cwd' key — got: {opts}"
|
||||||
assert opts["cwd"] == str(REPO_ROOT), (
|
assert opts["cwd"] == str(REPO_ROOT), (
|
||||||
f"cwd must equal context.worktree ({REPO_ROOT}), got: {opts['cwd']!r}"
|
f"cwd must equal context.worktree ({REPO_ROOT}), got: {opts['cwd']!r}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_explicit_passed_to_gh():
|
||||||
|
"""execute() with repo='foo/bar' → gh receives '--repo foo/bar' before the subcommand.
|
||||||
|
|
||||||
|
The shared runGh helper prepends ['--repo', <repo>] to the gh argv so an
|
||||||
|
explicit repo targets the right owner/name regardless of context.worktree.
|
||||||
|
"""
|
||||||
|
out = _run_exec(
|
||||||
|
{"title": VALID_TITLE, "body": VALID_BODY, "repo": "foo/bar"},
|
||||||
|
[ISSUE_OK_RESPONSE],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert result == f"Issue created: {ISSUE_URL}", f"expected success, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert args[0] == "--repo", f"expected --repo first, got: {args[0]!r}"
|
||||||
|
assert args[1] == "foo/bar", f"expected repo value, got: {args[1]!r}"
|
||||||
|
assert args[2] == "issue", f"expected 'issue' after --repo <name>, got: {args[2]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_omitted_no_repo_flag():
|
||||||
|
"""execute() without repo → gh argv has NO --repo (auto-detect from cwd).
|
||||||
|
|
||||||
|
Backward-compatibility: when repo is omitted, runGh returns [] from
|
||||||
|
parseRepo, so gh auto-detects the repo from context.worktree (cwd) —
|
||||||
|
matching the pre-refactor behaviour (ADR-025 / PR#61).
|
||||||
|
"""
|
||||||
|
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [ISSUE_OK_RESPONSE])
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" not in args, (
|
||||||
|
f"--repo must NOT be added when repo arg omitted; gh auto-detects. args: {args!r}"
|
||||||
|
)
|
||||||
|
assert args[0] == "issue", f"expected 'issue' first, got: {args[0]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_invalid_gh_error():
|
||||||
|
"""execute() with invalid repo + gh failure → error mentions gh issue create + exit code.
|
||||||
|
|
||||||
|
gh rejects an invalid owner/name with non-zero exit; the tool formats the
|
||||||
|
error via the shared formatResult helper (toolName='gh issue create').
|
||||||
|
"""
|
||||||
|
fail_response = {"status": 1, "stdout": "", "stderr": 'expected the "owner/repo" format'}
|
||||||
|
out = _run_exec(
|
||||||
|
{"title": VALID_TITLE, "body": VALID_BODY, "repo": "not-a-valid-repo"},
|
||||||
|
[fail_response],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert "gh issue create failed" in result, f"expected tool failure, got: {result!r}"
|
||||||
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" in args, f"expected --repo in args even on failure, got: {args!r}"
|
||||||
|
|
|
||||||
|
|
@ -119,4 +119,56 @@ describe("create-issue tool", () => {
|
||||||
// Body lacks Russian headings → heading check fires (not Cyrillic check).
|
// Body lacks Russian headings → heading check fires (not Cyrillic check).
|
||||||
expect(result).toContain("## Контекст")
|
expect(result).toContain("## Контекст")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("test_repo_explicit — repo='foo/bar' prepends --repo to gh argv", async () => {
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (_cmd, args) => {
|
||||||
|
capturedArgs = args
|
||||||
|
return { status: 0, stdout: "https://github.com/foo/bar/issues/1\n", stderr: "" }
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: VALID_BODY,
|
||||||
|
repo: "foo/bar",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toBe("Issue created: https://github.com/foo/bar/issues/1")
|
||||||
|
expect(capturedArgs[0]).toBe("--repo")
|
||||||
|
expect(capturedArgs[1]).toBe("foo/bar")
|
||||||
|
expect(capturedArgs[2]).toBe("issue")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_omitted — no --repo flag (auto-detect from cwd)", async () => {
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (_cmd, args) => {
|
||||||
|
capturedArgs = args
|
||||||
|
return { status: 0, stdout: "https://github.com/x/y/issues/1\n", stderr: "" }
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: VALID_BODY,
|
||||||
|
}, ctx())
|
||||||
|
// Backward-compat: --repo MUST NOT be present when repo arg omitted.
|
||||||
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
|
expect(capturedArgs[0]).toBe("issue")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_invalid — invalid repo + gh failure → tool error", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 1, stdout: "", stderr: 'expected the "owner/repo" format' }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: VALID_BODY,
|
||||||
|
repo: "not-a-valid-repo",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toContain("gh issue create failed")
|
||||||
|
expect(result).toContain("exit 1")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
@ -151,10 +151,66 @@ def test_execute_uses_cwd_from_context():
|
||||||
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
||||||
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [PR_OK_RESPONSE])
|
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [PR_OK_RESPONSE])
|
||||||
calls = out["calls"]
|
calls = out["calls"]
|
||||||
assert len(calls) == 1, f"expected 1 spawnSync call, got {len(calls)}"
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
opts = calls[0]["opts"]
|
opts = calls[0]["opts"]
|
||||||
assert opts is not None, "spawnSync called without opts — expected cwd kwarg"
|
assert opts is not None, "spawnSync called without opts — expected cwd kwarg"
|
||||||
assert "cwd" in opts, f"opts missing 'cwd' key — got: {opts}"
|
assert "cwd" in opts, f"opts missing 'cwd' key — got: {opts}"
|
||||||
assert opts["cwd"] == str(REPO_ROOT), (
|
assert opts["cwd"] == str(REPO_ROOT), (
|
||||||
f"cwd must equal context.worktree ({REPO_ROOT}), got: {opts['cwd']!r}"
|
f"cwd must equal context.worktree ({REPO_ROOT}), got: {opts['cwd']!r}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_explicit_passed_to_gh():
|
||||||
|
"""execute() with repo='foo/bar' → gh receives '--repo foo/bar' before the subcommand.
|
||||||
|
|
||||||
|
The shared runGh helper prepends ['--repo', <repo>] to the gh argv so an
|
||||||
|
explicit repo targets the right owner/name regardless of context.worktree.
|
||||||
|
"""
|
||||||
|
out = _run_exec(
|
||||||
|
{"title": VALID_TITLE, "body": VALID_BODY, "repo": "foo/bar"},
|
||||||
|
[PR_OK_RESPONSE],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert result == f"PR created: {PR_URL}", f"expected success, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert args[0] == "--repo", f"expected --repo first, got: {args[0]!r}"
|
||||||
|
assert args[1] == "foo/bar", f"expected repo value, got: {args[1]!r}"
|
||||||
|
assert args[2] == "pr", f"expected 'pr' after --repo <name>, got: {args[2]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_omitted_no_repo_flag():
|
||||||
|
"""execute() without repo → gh argv has NO --repo (auto-detect from cwd).
|
||||||
|
|
||||||
|
Backward-compatibility: when repo is omitted, runGh returns [] from
|
||||||
|
parseRepo, so gh auto-detects the repo from context.worktree (cwd) —
|
||||||
|
matching the pre-refactor behaviour (ADR-025 / PR#61).
|
||||||
|
"""
|
||||||
|
out = _run_exec({"title": VALID_TITLE, "body": VALID_BODY}, [PR_OK_RESPONSE])
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" not in args, (
|
||||||
|
f"--repo must NOT be added when repo arg omitted; gh auto-detects. args: {args!r}"
|
||||||
|
)
|
||||||
|
assert args[0] == "pr", f"expected 'pr' first, got: {args[0]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_invalid_gh_error():
|
||||||
|
"""execute() with invalid repo + gh failure → error mentions gh pr create + exit code.
|
||||||
|
|
||||||
|
gh rejects an invalid owner/name with non-zero exit; the tool formats the
|
||||||
|
error via the shared formatResult helper (toolName='gh pr create').
|
||||||
|
"""
|
||||||
|
fail_response = {"status": 1, "stdout": "", "stderr": 'expected the "owner/repo" format'}
|
||||||
|
out = _run_exec(
|
||||||
|
{"title": VALID_TITLE, "body": VALID_BODY, "repo": "not-a-valid-repo"},
|
||||||
|
[fail_response],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert "gh pr create failed" in result, f"expected tool failure, got: {result!r}"
|
||||||
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" in args, f"expected --repo in args even on failure, got: {args!r}"
|
||||||
|
|
|
||||||
|
|
@ -125,4 +125,56 @@ describe("create-pr tool", () => {
|
||||||
const bodyArg = capturedArgs[capturedArgs.indexOf("--body") + 1]
|
const bodyArg = capturedArgs[capturedArgs.indexOf("--body") + 1]
|
||||||
expect(bodyArg).toContain("Closes #37")
|
expect(bodyArg).toContain("Closes #37")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("test_repo_explicit — repo='foo/bar' prepends --repo to gh argv", async () => {
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (_cmd, args) => {
|
||||||
|
capturedArgs = args
|
||||||
|
return { status: 0, stdout: "https://github.com/foo/bar/pull/1\n", stderr: "" }
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: VALID_BODY,
|
||||||
|
repo: "foo/bar",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toBe("PR created: https://github.com/foo/bar/pull/1")
|
||||||
|
expect(capturedArgs[0]).toBe("--repo")
|
||||||
|
expect(capturedArgs[1]).toBe("foo/bar")
|
||||||
|
expect(capturedArgs[2]).toBe("pr")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_omitted — no --repo flag (auto-detect from cwd)", async () => {
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (_cmd, args) => {
|
||||||
|
capturedArgs = args
|
||||||
|
return { status: 0, stdout: "https://github.com/x/y/pull/1\n", stderr: "" }
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: VALID_BODY,
|
||||||
|
}, ctx())
|
||||||
|
// Backward-compat: --repo MUST NOT be present when repo arg omitted.
|
||||||
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
|
expect(capturedArgs[0]).toBe("pr")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_invalid — invalid repo + gh failure → tool error", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 1, stdout: "", stderr: 'expected the "owner/repo" format' }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: VALID_BODY,
|
||||||
|
repo: "not-a-valid-repo",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toContain("gh pr create failed")
|
||||||
|
expect(result).toContain("exit 1")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
185
tests/test_merge_pr_tool.py
Normal file
185
tests/test_merge_pr_tool.py
Normal file
|
|
@ -0,0 +1,185 @@
|
||||||
|
"""Tests for .opencode/tools/merge-pr.ts — the merge-pr custom tool.
|
||||||
|
|
||||||
|
Mirrors tests/test_post_review_tool.py / test_commit_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/merge-pr.ts``.
|
||||||
|
|
||||||
|
Modes used:
|
||||||
|
- ``load`` — sanity-check that the tool loads and declares ``pr_number`` and
|
||||||
|
the optional ``repo`` args.
|
||||||
|
- ``exec_stub_json`` — call execute with a stubbed spawnSync to verify:
|
||||||
|
(a) success path: valid pr_number → "PR #N merged successfully (squash,
|
||||||
|
branch deleted).",
|
||||||
|
(b) spawnSync args: gh pr merge N --squash --delete-branch (no --repo when
|
||||||
|
repo arg omitted — gh auto-detects from context.worktree, ADR-025),
|
||||||
|
(c) repo parameter: explicit repo → "--repo <owner/name>" prepended;
|
||||||
|
omitted → no --repo (backward-compatible auto-detect, ADR-027),
|
||||||
|
(d) failure: gh exit non-zero → "⚠️ merge_pr failed for PR #N (exit K): ...".
|
||||||
|
|
||||||
|
merge-pr.ts makes 1 spawnSync call (gh pr merge) on all paths.
|
||||||
|
"""
|
||||||
|
|
||||||
|
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" / "merge-pr.ts"
|
||||||
|
TS_FILE_REL = ".opencode/tools/merge-pr.ts"
|
||||||
|
|
||||||
|
MERGE_OK_RESPONSE = {"status": 0, "stdout": "", "stderr": ""}
|
||||||
|
|
||||||
|
|
||||||
|
def _run_loader(*args: str) -> dict:
|
||||||
|
"""Invoke the loader with TS_FILE env set to merge-pr.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: merge-pr.ts loads and declares pr_number + optional repo args."""
|
||||||
|
if not TS_FILE.exists():
|
||||||
|
pytest.skip("merge-pr.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 "repo" in args, f"missing repo arg: {args}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_valid_merge():
|
||||||
|
"""execute() with valid pr_number returns the squash-merged success message."""
|
||||||
|
out = _run_exec({"pr_number": 42}, [MERGE_OK_RESPONSE])
|
||||||
|
result = out["result"]
|
||||||
|
assert result == "PR #42 merged successfully (squash, branch deleted).", (
|
||||||
|
f"expected success, got: {result!r}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_spawnsync_args():
|
||||||
|
"""spawnSync called with gh pr merge <N> --squash --delete-branch (no --repo).
|
||||||
|
|
||||||
|
When repo is omitted, runGh returns [] from parseRepo, so gh auto-detects
|
||||||
|
the repo from context.worktree (cwd) — matching create-pr.ts and the
|
||||||
|
pre-refactor behaviour (ADR-025 / PR#61).
|
||||||
|
"""
|
||||||
|
out = _run_exec({"pr_number": 42}, [MERGE_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] == "merge", f"expected second arg 'merge', got: {args[1]!r}"
|
||||||
|
assert args[2] == "42", f"expected PR number '42', got: {args[2]!r}"
|
||||||
|
assert "--squash" in args, "missing --squash flag"
|
||||||
|
assert "--delete-branch" in args, "missing --delete-branch flag"
|
||||||
|
# --repo MUST NOT be present — gh auto-detects from cwd (context.worktree).
|
||||||
|
assert "--repo" not in args, (
|
||||||
|
f"--repo must not be hardcoded; gh auto-detects from cwd. args: {args!r}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_execute_uses_cwd_from_context():
|
||||||
|
"""execute passes cwd=context.worktree to spawnSync (ADR-023 pattern)."""
|
||||||
|
out = _run_exec({"pr_number": 42}, [MERGE_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: PR not mergeable"}
|
||||||
|
out = _run_exec({"pr_number": 42}, [fail_response])
|
||||||
|
result = out["result"]
|
||||||
|
assert "merge_pr failed" in result, f"expected failure message, got: {result!r}"
|
||||||
|
assert "PR #42" in result, f"expected PR number in failure, got: {result!r}"
|
||||||
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_explicit_passed_to_gh():
|
||||||
|
"""execute() with repo='foo/bar' → gh receives '--repo foo/bar' before the subcommand.
|
||||||
|
|
||||||
|
The shared runGh helper prepends ['--repo', <repo>] to the gh argv so an
|
||||||
|
explicit repo targets the right owner/name regardless of context.worktree.
|
||||||
|
"""
|
||||||
|
out = _run_exec({"pr_number": 42, "repo": "foo/bar"}, [MERGE_OK_RESPONSE])
|
||||||
|
result = out["result"]
|
||||||
|
assert result == "PR #42 merged successfully (squash, branch deleted).", (
|
||||||
|
f"expected success, got: {result!r}"
|
||||||
|
)
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert args[0] == "--repo", f"expected --repo first, got: {args[0]!r}"
|
||||||
|
assert args[1] == "foo/bar", f"expected repo value, got: {args[1]!r}"
|
||||||
|
assert args[2] == "pr", f"expected 'pr' after --repo <name>, got: {args[2]!r}"
|
||||||
|
# The squash/delete-branch flags must still be present after the repo prefix.
|
||||||
|
assert "--squash" in args, "missing --squash flag with explicit repo"
|
||||||
|
assert "--delete-branch" in args, "missing --delete-branch flag with explicit repo"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_omitted_no_repo_flag():
|
||||||
|
"""execute() without repo → gh argv has NO --repo (auto-detect from cwd).
|
||||||
|
|
||||||
|
Backward-compatibility: when repo is omitted, runGh returns [] from
|
||||||
|
parseRepo, so gh auto-detects the repo from context.worktree (cwd) —
|
||||||
|
matching the pre-refactor behaviour (ADR-025 / PR#61). This complements
|
||||||
|
test_spawnsync_args which asserts the same.
|
||||||
|
"""
|
||||||
|
out = _run_exec({"pr_number": 42}, [MERGE_OK_RESPONSE])
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" not in args, (
|
||||||
|
f"--repo must NOT be added when repo arg omitted; gh auto-detects. args: {args!r}"
|
||||||
|
)
|
||||||
|
assert args[0] == "pr", f"expected 'pr' first, got: {args[0]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_invalid_gh_error():
|
||||||
|
"""execute() with invalid repo + gh failure → error mentions merge_pr + exit code.
|
||||||
|
|
||||||
|
gh rejects an invalid owner/name with non-zero exit; the tool formats the
|
||||||
|
error with its tool-specific message (merge_pr failed for PR #N).
|
||||||
|
"""
|
||||||
|
fail_response = {"status": 1, "stdout": "", "stderr": 'expected the "owner/repo" format'}
|
||||||
|
out = _run_exec({"pr_number": 42, "repo": "not-a-valid-repo"}, [fail_response])
|
||||||
|
result = out["result"]
|
||||||
|
assert "merge_pr failed" in result, f"expected tool failure, got: {result!r}"
|
||||||
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" in args, f"expected --repo in args even on failure, got: {args!r}"
|
||||||
113
tests/test_merge_pr_tool.ts
Normal file
113
tests/test_merge_pr_tool.ts
Normal file
|
|
@ -0,0 +1,113 @@
|
||||||
|
/**
|
||||||
|
* Tests for .opencode/tools/merge-pr.ts — the merge-pr custom tool.
|
||||||
|
*
|
||||||
|
* Mirror of tests/test_post_review_tool.ts / test_commit_tool.ts:
|
||||||
|
* the tool is a spawnSync wrapper around `gh pr merge N --squash --delete-branch`.
|
||||||
|
*
|
||||||
|
* 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_merge_pr_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_merge_pr_tool.py):
|
||||||
|
* - test_valid_merge — valid pr_number → "PR #N merged successfully..."
|
||||||
|
* - test_spawnsync_args — spawnSync called with correct gh args (no --repo)
|
||||||
|
* - test_repo_explicit — repo='foo/bar' prepends --repo to gh argv
|
||||||
|
* - test_repo_omitted — no --repo flag (auto-detect from cwd)
|
||||||
|
* - test_repo_invalid — invalid repo + gh failure → tool error
|
||||||
|
*/
|
||||||
|
|
||||||
|
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", "merge-pr.ts")
|
||||||
|
|
||||||
|
function ctx() {
|
||||||
|
return {
|
||||||
|
sessionID: "t", messageID: "t", agent: "t",
|
||||||
|
directory: ".", worktree: ".",
|
||||||
|
abort: new AbortController().signal,
|
||||||
|
metadata() {}, async ask() {},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const MERGE_OK = { status: 0, stdout: "", stderr: "" }
|
||||||
|
|
||||||
|
describe("merge-pr tool", () => {
|
||||||
|
test("test_valid_merge — valid pr_number succeeds", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => MERGE_OK,
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({ pr_number: 42 }, ctx())
|
||||||
|
expect(result).toBe("PR #42 merged successfully (squash, branch deleted).")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_spawnsync_args — spawnSync called with gh pr merge N --squash --delete-branch (no --repo)", async () => {
|
||||||
|
let capturedCmd
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (cmd, args) => {
|
||||||
|
capturedCmd = cmd
|
||||||
|
capturedArgs = args
|
||||||
|
return MERGE_OK
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
await mod.default.execute({ pr_number: 42 }, ctx())
|
||||||
|
expect(capturedCmd).toBe("gh")
|
||||||
|
expect(capturedArgs[0]).toBe("pr")
|
||||||
|
expect(capturedArgs[1]).toBe("merge")
|
||||||
|
expect(capturedArgs[2]).toBe("42")
|
||||||
|
expect(capturedArgs).toContain("--squash")
|
||||||
|
expect(capturedArgs).toContain("--delete-branch")
|
||||||
|
// --repo MUST NOT be present — gh auto-detects from cwd (PR#60).
|
||||||
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_explicit — repo='foo/bar' prepends --repo to gh argv", async () => {
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (_cmd, args) => {
|
||||||
|
capturedArgs = args
|
||||||
|
return MERGE_OK
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({ pr_number: 42, repo: "foo/bar" }, ctx())
|
||||||
|
expect(result).toBe("PR #42 merged successfully (squash, branch deleted).")
|
||||||
|
expect(capturedArgs[0]).toBe("--repo")
|
||||||
|
expect(capturedArgs[1]).toBe("foo/bar")
|
||||||
|
expect(capturedArgs[2]).toBe("pr")
|
||||||
|
expect(capturedArgs).toContain("--squash")
|
||||||
|
expect(capturedArgs).toContain("--delete-branch")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_omitted — no --repo flag (auto-detect from cwd)", async () => {
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (_cmd, args) => {
|
||||||
|
capturedArgs = args
|
||||||
|
return MERGE_OK
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
await mod.default.execute({ pr_number: 42 }, ctx())
|
||||||
|
// Backward-compat: --repo MUST NOT be present when repo arg omitted.
|
||||||
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
|
expect(capturedArgs[0]).toBe("pr")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_invalid — invalid repo + gh failure → tool error", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 1, stdout: "", stderr: 'expected the "owner/repo" format' }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({ pr_number: 42, repo: "not-a-valid-repo" }, ctx())
|
||||||
|
expect(result).toContain("merge_pr failed")
|
||||||
|
expect(result).toContain("exit 1")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -242,3 +242,63 @@ def test_gh_failure_returns_error():
|
||||||
result = out["result"]
|
result = out["result"]
|
||||||
assert "post-docs-review failed" in result, f"expected failure message, got: {result!r}"
|
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}"
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_explicit_passed_to_gh():
|
||||||
|
"""execute() with repo='foo/bar' → gh receives '--repo foo/bar' before the subcommand.
|
||||||
|
|
||||||
|
The shared runGh helper prepends ['--repo', <repo>] to the gh argv so an
|
||||||
|
explicit repo targets the right owner/name regardless of context.worktree.
|
||||||
|
"""
|
||||||
|
out = _run_exec(
|
||||||
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body.", "repo": "foo/bar"},
|
||||||
|
[COMMENT_OK_RESPONSE],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert result == EXPECTED_APPROVE, f"expected success, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert args[0] == "--repo", f"expected --repo first, got: {args[0]!r}"
|
||||||
|
assert args[1] == "foo/bar", f"expected repo value, got: {args[1]!r}"
|
||||||
|
assert args[2] == "pr", f"expected 'pr' after --repo <name>, got: {args[2]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_omitted_no_repo_flag():
|
||||||
|
"""execute() without repo → gh argv has NO --repo (auto-detect from cwd).
|
||||||
|
|
||||||
|
Backward-compatibility: when repo is omitted, runGh returns [] from
|
||||||
|
parseRepo, so gh auto-detects the repo from context.worktree (cwd) —
|
||||||
|
matching the pre-refactor behaviour (ADR-025 / PR#61). This complements
|
||||||
|
test_spawnsync_args which asserts the same on the success path.
|
||||||
|
"""
|
||||||
|
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"]
|
||||||
|
assert "--repo" not in args, (
|
||||||
|
f"--repo must NOT be added when repo arg omitted; gh auto-detects. args: {args!r}"
|
||||||
|
)
|
||||||
|
assert args[0] == "pr", f"expected 'pr' first, got: {args[0]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_invalid_gh_error():
|
||||||
|
"""execute() with invalid repo + gh failure → error mentions post-docs-review + exit code.
|
||||||
|
|
||||||
|
gh rejects an invalid owner/name with non-zero exit; the tool formats the
|
||||||
|
error with its tool-specific message (post-docs-review failed for PR #N).
|
||||||
|
"""
|
||||||
|
fail_response = {"status": 1, "stdout": "", "stderr": 'expected the "owner/repo" format'}
|
||||||
|
out = _run_exec(
|
||||||
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Docs body.", "repo": "not-a-valid-repo"},
|
||||||
|
[fail_response],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert "post-docs-review failed" in result, f"expected tool failure, got: {result!r}"
|
||||||
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" in args, f"expected --repo in args even on failure, got: {args!r}"
|
||||||
|
|
|
||||||
|
|
@ -182,4 +182,59 @@ describe("post-docs-review tool", () => {
|
||||||
expect(capturedArgs[lastIdx - 1]).toBe("--body")
|
expect(capturedArgs[lastIdx - 1]).toBe("--body")
|
||||||
expect(capturedArgs[lastIdx].startsWith("## Docs Review Summary\n")).toBe(true)
|
expect(capturedArgs[lastIdx].startsWith("## Docs Review Summary\n")).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("test_repo_explicit — repo='foo/bar' prepends --repo to gh argv", 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: "Docs body",
|
||||||
|
repo: "foo/bar",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toBe("Docs review posted on PR #45: verdict=APPROVE")
|
||||||
|
expect(capturedArgs[0]).toBe("--repo")
|
||||||
|
expect(capturedArgs[1]).toBe("foo/bar")
|
||||||
|
expect(capturedArgs[2]).toBe("pr")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_omitted — no --repo flag (auto-detect from cwd)", 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())
|
||||||
|
// Backward-compat: --repo MUST NOT be present when repo arg omitted.
|
||||||
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
|
expect(capturedArgs[0]).toBe("pr")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_invalid — invalid repo + gh failure → tool error", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 1, stdout: "", stderr: 'expected the "owner/repo" format' }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
pr_number: 45,
|
||||||
|
verdict: "APPROVE",
|
||||||
|
body: "Docs body",
|
||||||
|
repo: "not-a-valid-repo",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toContain("post-docs-review failed")
|
||||||
|
expect(result).toContain("exit 1")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
@ -247,3 +247,63 @@ def test_gh_failure_returns_error():
|
||||||
result = out["result"]
|
result = out["result"]
|
||||||
assert "post-review failed" in result, f"expected failure message, got: {result!r}"
|
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}"
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_explicit_passed_to_gh():
|
||||||
|
"""execute() with repo='foo/bar' → gh receives '--repo foo/bar' before the subcommand.
|
||||||
|
|
||||||
|
The shared runGh helper prepends ['--repo', <repo>] to the gh argv so an
|
||||||
|
explicit repo targets the right owner/name regardless of context.worktree.
|
||||||
|
"""
|
||||||
|
out = _run_exec(
|
||||||
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body.", "repo": "foo/bar"},
|
||||||
|
[COMMENT_OK_RESPONSE],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert result == EXPECTED_APPROVE, f"expected success, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
assert len(calls) == 1, f"expected 1 spawnSync call, got: {len(calls)}"
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert args[0] == "--repo", f"expected --repo first, got: {args[0]!r}"
|
||||||
|
assert args[1] == "foo/bar", f"expected repo value, got: {args[1]!r}"
|
||||||
|
assert args[2] == "pr", f"expected 'pr' after --repo <name>, got: {args[2]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_omitted_no_repo_flag():
|
||||||
|
"""execute() without repo → gh argv has NO --repo (auto-detect from cwd).
|
||||||
|
|
||||||
|
Backward-compatibility: when repo is omitted, runGh returns [] from
|
||||||
|
parseRepo, so gh auto-detects the repo from context.worktree (cwd) —
|
||||||
|
matching the pre-refactor behaviour (ADR-025 / PR#61). This complements
|
||||||
|
test_spawnsync_args which asserts the same on the success path.
|
||||||
|
"""
|
||||||
|
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"]
|
||||||
|
assert "--repo" not in args, (
|
||||||
|
f"--repo must NOT be added when repo arg omitted; gh auto-detects. args: {args!r}"
|
||||||
|
)
|
||||||
|
assert args[0] == "pr", f"expected 'pr' first, got: {args[0]!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_repo_invalid_gh_error():
|
||||||
|
"""execute() with invalid repo + gh failure → error mentions post-review + exit code.
|
||||||
|
|
||||||
|
gh rejects an invalid owner/name with non-zero exit; the tool formats the
|
||||||
|
error with its tool-specific message (post-review failed for PR #N).
|
||||||
|
"""
|
||||||
|
fail_response = {"status": 1, "stdout": "", "stderr": 'expected the "owner/repo" format'}
|
||||||
|
out = _run_exec(
|
||||||
|
{"pr_number": 45, "verdict": "APPROVE", "body": "Review body.", "repo": "not-a-valid-repo"},
|
||||||
|
[fail_response],
|
||||||
|
)
|
||||||
|
result = out["result"]
|
||||||
|
assert "post-review failed" in result, f"expected tool failure, got: {result!r}"
|
||||||
|
assert "exit 1" in result, f"expected exit 1 mention, got: {result!r}"
|
||||||
|
calls = out["calls"]
|
||||||
|
args = calls[0]["args"]
|
||||||
|
assert "--repo" in args, f"expected --repo in args even on failure, got: {args!r}"
|
||||||
|
|
|
||||||
|
|
@ -185,4 +185,59 @@ describe("post-review tool", () => {
|
||||||
expect(capturedArgs[lastIdx - 1]).toBe("--body")
|
expect(capturedArgs[lastIdx - 1]).toBe("--body")
|
||||||
expect(capturedArgs[lastIdx].startsWith("## Code Review Summary\n")).toBe(true)
|
expect(capturedArgs[lastIdx].startsWith("## Code Review Summary\n")).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("test_repo_explicit — repo='foo/bar' prepends --repo to gh argv", 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: "Review body",
|
||||||
|
repo: "foo/bar",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toBe("Review posted on PR #45: verdict=APPROVE")
|
||||||
|
expect(capturedArgs[0]).toBe("--repo")
|
||||||
|
expect(capturedArgs[1]).toBe("foo/bar")
|
||||||
|
expect(capturedArgs[2]).toBe("pr")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_omitted — no --repo flag (auto-detect from cwd)", 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())
|
||||||
|
// Backward-compat: --repo MUST NOT be present when repo arg omitted.
|
||||||
|
expect(capturedArgs).not.toContain("--repo")
|
||||||
|
expect(capturedArgs[0]).toBe("pr")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_repo_invalid — invalid repo + gh failure → tool error", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 1, stdout: "", stderr: 'expected the "owner/repo" format' }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
pr_number: 45,
|
||||||
|
verdict: "APPROVE",
|
||||||
|
body: "Review body",
|
||||||
|
repo: "not-a-valid-repo",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toContain("post-review failed")
|
||||||
|
expect(result).toContain("exit 1")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
262
tests/test_tunnel_tool.py
Normal file
262
tests/test_tunnel_tool.py
Normal file
|
|
@ -0,0 +1,262 @@
|
||||||
|
"""Tests for .opencode/scripts/tunnel.sh — the cloudflare tunnel toggle script.
|
||||||
|
|
||||||
|
The tunnel tool (.opencode/tools/tunnel.ts) is a thin wrapper that spawns
|
||||||
|
``bash .opencode/scripts/tunnel.sh``. The toggle logic, PID-file management
|
||||||
|
and error handling all live in the bash script, so these tests exercise the
|
||||||
|
script directly (the .ts wrapper is covered by tests/test_tunnel_tool.ts).
|
||||||
|
|
||||||
|
Isolation strategy: ``tunnel.sh`` hardcodes ``PID_FILE=/tmp/tunnel.pid`` and
|
||||||
|
``LOG_FILE=/tmp/tunnel.log``. To avoid clobbering a real tunnel on the host
|
||||||
|
(or interference between tests), each test copies the script to a temp dir
|
||||||
|
and rewrites the PID/LOG paths to point inside that temp dir. ``cloudflared``
|
||||||
|
is shadowed by a fake binary on PATH that sleeps long enough for the script
|
||||||
|
to verify the process is alive (``kill -0``), so the "started" branch fires.
|
||||||
|
|
||||||
|
Covered cases (issue #64):
|
||||||
|
- Start without ``CLOUDFLARE_TUNNEL_TOKEN`` → ``❌ ... is not set``, exit 1.
|
||||||
|
- Start with token (no domain) → ``started (PID: N)``, PID file created.
|
||||||
|
- Start with token + ``TUNNEL_DOMAIN`` → ``started (PID: N, domain: <d>)``.
|
||||||
|
- Stop when process alive (PID file present, process live) → ``stopped``,
|
||||||
|
PID file removed.
|
||||||
|
- Stale PID file (PID file present, process dead) → cleaned up, then start.
|
||||||
|
- Toggle: second start while process alive → ``stopped`` (toggle semantics).
|
||||||
|
"""
|
||||||
|
|
||||||
|
import contextlib
|
||||||
|
import os
|
||||||
|
import stat
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
|
import textwrap
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||||
|
SCRIPT_SRC = REPO_ROOT / ".opencode" / "scripts" / "tunnel.sh"
|
||||||
|
|
||||||
|
|
||||||
|
def _make_fake_cloudflared(tmp_path: Path) -> Path:
|
||||||
|
"""Write a fake `cloudflared` that sleeps long enough for kill -0 to pass.
|
||||||
|
|
||||||
|
The real cloudflared runs the tunnel; for tests we only need a long-lived
|
||||||
|
child so the script's `kill -0 $PID` check succeeds and the "started"
|
||||||
|
branch fires. The fake ignores its args and sleeps 30s (cleaned up when
|
||||||
|
the test process exits or the script kills it via the toggle/stop path).
|
||||||
|
"""
|
||||||
|
fake = tmp_path / "bin" / "cloudflared"
|
||||||
|
fake.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
fake.write_text(
|
||||||
|
textwrap.dedent("""\
|
||||||
|
#!/usr/bin/env bash
|
||||||
|
# Fake cloudflared: sleep so the parent script sees us alive (kill -0).
|
||||||
|
exec sleep 30
|
||||||
|
""")
|
||||||
|
)
|
||||||
|
fake.chmod(fake.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
|
||||||
|
return fake
|
||||||
|
|
||||||
|
|
||||||
|
def _make_isolated_script(tmp_path: Path) -> Path:
|
||||||
|
"""Copy tunnel.sh to tmp and rewrite PID/LOG paths to live inside tmp.
|
||||||
|
|
||||||
|
tunnel.sh hardcodes /tmp/tunnel.pid and /tmp/tunnel.log. Running the
|
||||||
|
tests as-is would clobber a real tunnel on the host. We rewrite both
|
||||||
|
paths to point at <tmp>/tunnel.pid and <tmp>/tunnel.log so each test is
|
||||||
|
isolated and cleanup is automatic (tmp_path is pytest-managed).
|
||||||
|
"""
|
||||||
|
if not SCRIPT_SRC.exists():
|
||||||
|
pytest.skip("tunnel.sh not present")
|
||||||
|
dst = tmp_path / "tunnel.sh"
|
||||||
|
src = SCRIPT_SRC.read_text()
|
||||||
|
src = src.replace('PID_FILE="/tmp/tunnel.pid"', f'PID_FILE="{tmp_path}/tunnel.pid"')
|
||||||
|
src = src.replace('LOG_FILE="/tmp/tunnel.log"', f'LOG_FILE="{tmp_path}/tunnel.log"')
|
||||||
|
dst.write_text(src)
|
||||||
|
dst.chmod(dst.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
|
||||||
|
return dst
|
||||||
|
|
||||||
|
|
||||||
|
def _run(script: Path, env: dict, timeout: float = 10.0) -> subprocess.CompletedProcess:
|
||||||
|
"""Run the (isolated) tunnel.sh with the given env and return the result."""
|
||||||
|
return subprocess.run(
|
||||||
|
["bash", str(script)],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
timeout=timeout,
|
||||||
|
env=env,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_start_without_token(tmp_path: Path):
|
||||||
|
"""Start without CLOUDFLARE_TUNNEL_TOKEN → error message, exit 1.
|
||||||
|
|
||||||
|
The script checks the token first (set -euo pipefail + [[ -z ... ]]) and
|
||||||
|
exits before touching the PID file or spawning cloudflared.
|
||||||
|
"""
|
||||||
|
script = _make_isolated_script(tmp_path)
|
||||||
|
env = {**os.environ}
|
||||||
|
env.pop("CLOUDFLARE_TUNNEL_TOKEN", None)
|
||||||
|
env.pop("TUNNEL_DOMAIN", None)
|
||||||
|
# Ensure PATH has no real cloudflared that could shadow the check — the
|
||||||
|
# token check fires before cloudflared is invoked, but be defensive.
|
||||||
|
proc = _run(script, env)
|
||||||
|
assert proc.returncode == 1, f"expected exit 1, got {proc.returncode}; stderr={proc.stderr!r}"
|
||||||
|
assert "CLOUDFLARE_TUNNEL_TOKEN is not set" in proc.stdout, (
|
||||||
|
f"expected token-missing message, got stdout={proc.stdout!r}"
|
||||||
|
)
|
||||||
|
# PID file must NOT be created on the early-exit path.
|
||||||
|
assert not (tmp_path / "tunnel.pid").exists(), "PID file created despite missing token"
|
||||||
|
|
||||||
|
|
||||||
|
def test_start_with_token(tmp_path: Path):
|
||||||
|
"""Start with token (no domain) → 'started (PID: N)', PID file created.
|
||||||
|
|
||||||
|
The fake cloudflared sleeps 30s so kill -0 succeeds and the script reports
|
||||||
|
'started (PID: <N>)'. The PID file must contain the child PID.
|
||||||
|
"""
|
||||||
|
script = _make_isolated_script(tmp_path)
|
||||||
|
fake = _make_fake_cloudflared(tmp_path)
|
||||||
|
base_env = {**os.environ}
|
||||||
|
base_env.pop("TUNNEL_DOMAIN", None)
|
||||||
|
env = {
|
||||||
|
**base_env,
|
||||||
|
"CLOUDFLARE_TUNNEL_TOKEN": "fake-token",
|
||||||
|
"PATH": f"{fake.parent}:{base_env['PATH']}",
|
||||||
|
}
|
||||||
|
proc = _run(script, env)
|
||||||
|
assert proc.returncode == 0, f"expected exit 0, got {proc.returncode}; stderr={proc.stderr!r}"
|
||||||
|
assert "started (PID:" in proc.stdout, (
|
||||||
|
f"expected 'started (PID:...)', got stdout={proc.stdout!r}"
|
||||||
|
)
|
||||||
|
pid_file = tmp_path / "tunnel.pid"
|
||||||
|
assert pid_file.exists(), "PID file not created on start"
|
||||||
|
pid = pid_file.read_text().strip()
|
||||||
|
assert pid.isdigit(), f"PID file content is not a number: {pid!r}"
|
||||||
|
# Clean up: kill the sleeping fake cloudflared so it doesn't linger.
|
||||||
|
with contextlib.suppress(ProcessLookupError, PermissionError):
|
||||||
|
os.kill(int(pid), 9)
|
||||||
|
|
||||||
|
|
||||||
|
def test_start_with_token_and_domain(tmp_path: Path):
|
||||||
|
"""Start with token + TUNNEL_DOMAIN → 'started (PID: N, domain: <d>)'.
|
||||||
|
|
||||||
|
When TUNNEL_DOMAIN is set, the script appends ', domain: <d>' to the
|
||||||
|
started line (display only — the tunnel itself is the same).
|
||||||
|
"""
|
||||||
|
script = _make_isolated_script(tmp_path)
|
||||||
|
fake = _make_fake_cloudflared(tmp_path)
|
||||||
|
base_env = {**os.environ}
|
||||||
|
env = {
|
||||||
|
**base_env,
|
||||||
|
"CLOUDFLARE_TUNNEL_TOKEN": "fake-token",
|
||||||
|
"TUNNEL_DOMAIN": "example.com",
|
||||||
|
"PATH": f"{fake.parent}:{base_env['PATH']}",
|
||||||
|
}
|
||||||
|
proc = _run(script, env)
|
||||||
|
assert proc.returncode == 0, f"expected exit 0, got {proc.returncode}; stderr={proc.stderr!r}"
|
||||||
|
assert "started (PID:" in proc.stdout, (
|
||||||
|
f"expected 'started (PID:...)', got stdout={proc.stdout!r}"
|
||||||
|
)
|
||||||
|
assert "domain: example.com" in proc.stdout, (
|
||||||
|
f"expected domain in output, got stdout={proc.stdout!r}"
|
||||||
|
)
|
||||||
|
pid = (tmp_path / "tunnel.pid").read_text().strip()
|
||||||
|
with contextlib.suppress(ProcessLookupError, PermissionError):
|
||||||
|
os.kill(int(pid), 9)
|
||||||
|
|
||||||
|
|
||||||
|
def test_stop_when_process_alive(tmp_path: Path):
|
||||||
|
"""Stop when PID file exists + process alive → 'stopped', PID file removed.
|
||||||
|
|
||||||
|
Toggle semantics: first start creates the PID file, second invocation sees
|
||||||
|
the live process and stops it. We simulate a 'live' process by writing the
|
||||||
|
PID of the current test process (which is alive) into the PID file, then
|
||||||
|
run the script — it should kill... wait, it would kill the test process.
|
||||||
|
Instead we start a long-lived child (sleep) ourselves, write its PID, then
|
||||||
|
run the script and verify it stops the child and removes the PID file.
|
||||||
|
"""
|
||||||
|
script = _make_isolated_script(tmp_path)
|
||||||
|
# Start a long-lived child whose PID we control.
|
||||||
|
child = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(30)"])
|
||||||
|
pid_file = tmp_path / "tunnel.pid"
|
||||||
|
pid_file.write_text(str(child.pid))
|
||||||
|
env = {**os.environ, "CLOUDFLARE_TUNNEL_TOKEN": "fake-token"}
|
||||||
|
env.pop("TUNNEL_DOMAIN", None)
|
||||||
|
proc = _run(script, env)
|
||||||
|
assert proc.returncode == 0, f"expected exit 0, got {proc.returncode}; stderr={proc.stderr!r}"
|
||||||
|
assert proc.stdout.strip() == "stopped", f"expected 'stopped', got stdout={proc.stdout!r}"
|
||||||
|
assert not pid_file.exists(), "PID file not removed on stop"
|
||||||
|
# The child must have been terminated by the script's `kill $PID`.
|
||||||
|
child.wait(timeout=5)
|
||||||
|
assert child.poll() is not None, "child still alive after stop"
|
||||||
|
|
||||||
|
|
||||||
|
def test_stale_pid_file_cleaned_then_start(tmp_path: Path):
|
||||||
|
"""Stale PID file (process dead) → cleaned up, then start.
|
||||||
|
|
||||||
|
If the PID file points at a dead process, kill -0 fails and the script
|
||||||
|
removes the stale file (rm -f) before falling through to the start path.
|
||||||
|
We write a PID that is guaranteed dead (a recently-exited subprocess).
|
||||||
|
"""
|
||||||
|
script = _make_isolated_script(tmp_path)
|
||||||
|
fake = _make_fake_cloudflared(tmp_path)
|
||||||
|
# Use a PID that is guaranteed dead: a very high PID that no real process
|
||||||
|
# holds on this system. kill -0 fails (ESRCH) → the script treats the PID
|
||||||
|
# file as stale, removes it, and falls through to the start path. Using a
|
||||||
|
# recently-reaped PID risks PID reuse by the OS between reap and script
|
||||||
|
# run; a high sentinel PID avoids that race entirely.
|
||||||
|
dead_pid = 999999
|
||||||
|
pid_file = tmp_path / "tunnel.pid"
|
||||||
|
pid_file.write_text(str(dead_pid))
|
||||||
|
base_env = {**os.environ}
|
||||||
|
base_env.pop("TUNNEL_DOMAIN", None)
|
||||||
|
env = {
|
||||||
|
**base_env,
|
||||||
|
"CLOUDFLARE_TUNNEL_TOKEN": "fake-token",
|
||||||
|
"PATH": f"{fake.parent}:{base_env['PATH']}",
|
||||||
|
}
|
||||||
|
proc = _run(script, env)
|
||||||
|
assert proc.returncode == 0, f"expected exit 0, got {proc.returncode}; stderr={proc.stderr!r}"
|
||||||
|
assert "started (PID:" in proc.stdout, (
|
||||||
|
f"expected start after stale cleanup, got stdout={proc.stdout!r}"
|
||||||
|
)
|
||||||
|
# PID file now holds the NEW cloudflared PID, not the stale one.
|
||||||
|
new_pid = pid_file.read_text().strip()
|
||||||
|
assert new_pid.isdigit() and int(new_pid) != dead_pid, (
|
||||||
|
f"PID file still holds stale PID {new_pid} (expected new cloudflared PID)"
|
||||||
|
)
|
||||||
|
with contextlib.suppress(ProcessLookupError, PermissionError):
|
||||||
|
os.kill(int(new_pid), 9)
|
||||||
|
|
||||||
|
|
||||||
|
def test_toggle_second_start_stops(tmp_path: Path):
|
||||||
|
"""Toggle: a second start while a process is alive → 'stopped'.
|
||||||
|
|
||||||
|
The script is a toggle: if the PID file exists and the process is alive,
|
||||||
|
the next invocation stops it (instead of starting a second tunnel). We
|
||||||
|
start a tunnel first, then run the script again and expect 'stopped'.
|
||||||
|
"""
|
||||||
|
script = _make_isolated_script(tmp_path)
|
||||||
|
fake = _make_fake_cloudflared(tmp_path)
|
||||||
|
base_env = {**os.environ}
|
||||||
|
base_env.pop("TUNNEL_DOMAIN", None)
|
||||||
|
env = {
|
||||||
|
**base_env,
|
||||||
|
"CLOUDFLARE_TUNNEL_TOKEN": "fake-token",
|
||||||
|
"PATH": f"{fake.parent}:{base_env['PATH']}",
|
||||||
|
}
|
||||||
|
# First call: start.
|
||||||
|
proc1 = _run(script, env)
|
||||||
|
assert "started (PID:" in proc1.stdout, f"first call should start, got {proc1.stdout!r}"
|
||||||
|
pid = (tmp_path / "tunnel.pid").read_text().strip()
|
||||||
|
assert pid.isdigit(), f"PID file should hold a number after start, got {pid!r}"
|
||||||
|
# Second call: toggle → stop.
|
||||||
|
proc2 = _run(script, env)
|
||||||
|
assert proc2.returncode == 0, (
|
||||||
|
f"expected exit 0, got {proc2.returncode}; stderr={proc2.stderr!r}"
|
||||||
|
)
|
||||||
|
assert proc2.stdout.strip() == "stopped", (
|
||||||
|
f"second call should stop (toggle), got stdout={proc2.stdout!r}"
|
||||||
|
)
|
||||||
|
assert not (tmp_path / "tunnel.pid").exists(), "PID file not removed on toggle-stop"
|
||||||
93
tests/test_tunnel_tool.ts
Normal file
93
tests/test_tunnel_tool.ts
Normal file
|
|
@ -0,0 +1,93 @@
|
||||||
|
/**
|
||||||
|
* Tests for .opencode/tools/tunnel.ts — the tunnel custom tool.
|
||||||
|
*
|
||||||
|
* The tool is a thin spawnSync wrapper around `bash .opencode/scripts/tunnel.sh`
|
||||||
|
* (toggle: 1st call starts, 2nd call stops). The toggle/PID-file logic lives
|
||||||
|
* in the bash script and is covered by tests/test_tunnel_tool.py (which runs
|
||||||
|
* the script directly with a fake cloudflared). These TS tests stub spawnSync
|
||||||
|
* to verify the tool wiring: it invokes `bash <script>` with cwd from context,
|
||||||
|
* returns stdout on success, and formats the canonical `⚠️ tunnel failed` error
|
||||||
|
* on non-zero exit.
|
||||||
|
*
|
||||||
|
* 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_tunnel_tool.py.
|
||||||
|
*
|
||||||
|
* Test cases (mirror tests/test_tunnel_tool.py success/error wiring):
|
||||||
|
* - test_starts_with_bash_script — spawnSync called with bash + script path
|
||||||
|
* - test_success_returns_stdout — exit 0 + stdout → returns trimmed stdout
|
||||||
|
* - test_failure_returns_error — exit non-zero → "⚠️ tunnel failed (exit K): ..."
|
||||||
|
* - test_uses_cwd_from_context — spawnSync opts.cwd == context.worktree
|
||||||
|
*/
|
||||||
|
|
||||||
|
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", "tunnel.ts")
|
||||||
|
|
||||||
|
function ctx() {
|
||||||
|
return {
|
||||||
|
sessionID: "t", messageID: "t", agent: "t",
|
||||||
|
directory: ".", worktree: ".",
|
||||||
|
abort: new AbortController().signal,
|
||||||
|
metadata() {}, async ask() {},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("tunnel tool", () => {
|
||||||
|
test("test_starts_with_bash_script — spawnSync called with bash + script path", async () => {
|
||||||
|
let capturedCmd
|
||||||
|
let capturedArgs
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (cmd, args) => {
|
||||||
|
capturedCmd = cmd
|
||||||
|
capturedArgs = args
|
||||||
|
return { status: 0, stdout: "started (PID: 12345)\n", stderr: "" }
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({}, ctx())
|
||||||
|
expect(capturedCmd).toBe("bash")
|
||||||
|
// The single arg is the path to .opencode/scripts/tunnel.sh (resolved
|
||||||
|
// relative to the tool's own directory via import.meta.dir/../scripts).
|
||||||
|
expect(capturedArgs).toHaveLength(1)
|
||||||
|
expect(capturedArgs[0]).toContain("tunnel.sh")
|
||||||
|
expect(result).toBe("started (PID: 12345)")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_success_returns_stdout — exit 0 + stdout → returns trimmed stdout", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 0, stdout: "stopped\n", stderr: "" }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({}, ctx())
|
||||||
|
expect(result).toBe("stopped")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_failure_returns_error — exit non-zero → '⚠️ tunnel failed (exit K): ...'", async () => {
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 1, stdout: "", stderr: "❌ CLOUDFLARE_TUNNEL_TOKEN is not set" }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({}, ctx())
|
||||||
|
expect(result).toContain("⚠️ tunnel failed")
|
||||||
|
expect(result).toContain("exit 1")
|
||||||
|
expect(result).toContain("CLOUDFLARE_TUNNEL_TOKEN is not set")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_uses_cwd_from_context — spawnSync opts.cwd == context.worktree", async () => {
|
||||||
|
let capturedOpts
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: (_cmd, _args, opts) => {
|
||||||
|
capturedOpts = opts
|
||||||
|
return { status: 0, stdout: "started (PID: 1)\n", stderr: "" }
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
await mod.default.execute({}, ctx())
|
||||||
|
expect(capturedOpts).not.toBeNull()
|
||||||
|
expect(capturedOpts).toHaveProperty("cwd")
|
||||||
|
expect(capturedOpts.cwd).toBe(".")
|
||||||
|
})
|
||||||
|
})
|
||||||
Loading…
Add table
Reference in a new issue