15fc7d014d
* 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>
156 lines
No EOL
9.4 KiB
Markdown
156 lines
No EOL
9.4 KiB
Markdown
# 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 падали бы. |