fix(create-pr): pass base field for Forgejo POST /pulls #33
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/create-pr/forgejo-base-field"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Что сделано
create-pr.ts: auto-detectdefault_branchчерезgetRepo()(эталон —merge-pr.ts) и передача--base <defaultBranch>вrunGh(["pr", "create", ...]). ИмпортированыresolveForgejoRepoиgetRepoиз_shared.ts._shared.ts: добавлен симметричный fail-loud guard дляbaseвcallForgejoGh(после guard'а дляhead) — POST body теперь{ title, body, head, base }вместо условного...(base ? { base } : {}). При отсутствииbaseвозвращается явная ошибка вместо молчаливой отправки malformed-запроса, который Forgejo всё равно отвергнет 422.tests/test_create_pr_tool.py: добавленаREPO_OK_RESPONSE(по образцуtest_merge_pr_tool.py:50); перевернут assertiontest_head_explicit_override(baseтеперь обязателен, не omitted); обновлены 9 success-path тестов — добавленREPO_OK_RESPONSEпервым в массив responses (getRepo вызывается ДО POST /pulls),fetch_calls1→2, assertions на POST body перенесены сfetch_calls[0]наfetch_calls[1];test_execute_uses_cwd_from_context—git_calls2→3,remote_calls1→2 (появился 2-й remote lookup от create-pr.ts).Почему
Forgejo
POST /repos/{owner}/{repo}/pullsтребует полеbase(target branch) безусловно — в отличие от GitHub API, где base дефолтится к default_branch. БезbaseForgejo возвращает422 {"message":"[Base]: Required"}. Корневая причина бага:create-pr.ts:70не передавал--baseв argv, а_shared.ts:220через...(base ? { base } : {})опускал поле, когда base был undefined — то есть всегда. Тестtest_head_explicit_overrideзакреплял этот баг как "expected behavior" с ложным комментарием "Forgejo defaults to default_branch".Watch out
resolveForgejoRepoвcreate-pr.ts(для getRepo pre-flight) +resolveForgejoRepoвнутриrunGh→callForgejoGh(для POST /pulls). Это не баг — acceptable overhead (2 spawnSync вместо 1, ~миллисекунды). Тесты обновлены:test_execute_uses_cwd_from_contextожидает 3 git-вызова (rev-parse + 2 remote) и 2 remote lookup.formatResultиспользуется только для ошибки POST /pulls; ошибка getRepo (GET /repos) имеет собственный формат⚠️ create_pr failed for issue #N: ...— сознательно отличается (pre-flight failure, не gh-вызов).Pending
fullнапрямую вrunGh(новая сигнатура опций), чтобы избежать 2-го remote lookup внутриcallForgejoGh. Отдельный issue если будет нужно — текущий overhead пренебрежимо мал.Closes #26
Code Review Summary
Чистый баг-фикс:
create-pr.tsне передавал--baseв Forgejo POST /pulls → 422{"message":"[Base]: Required"}. Корневая причина (условное...(base ? { base } : {})опускало поле всегда) устранена: auto-detectdefault_branchчерезgetRepo()(эталон —merge-pr.ts:16-36) + симметричный fail-loud guard дляbaseв_shared.ts+ обновление 9 success-path тестов. Перевёрнутый assertion вtest_head_explicit_overrideправильно фиксирует регрессию (старый тест закреплял баг с ложным комментарием "Forgejo defaults to default_branch").Positives
resolveForgejoRepo → getRepo → JSON.parse(stdout).default_branch(create-pr.ts:70-76) точно повторяет merge-pr.ts:16-36 — консистентность между tool'ами.base(_shared.ts:220-228) зеркалит существующий guard дляhead(_shared.ts:205-216) — одинаковая структура, понятные сообщения.REPO_OK_RESPONSEпервым в массиве (getRepo вызывается ДО POST),fetch_calls1→2, assertions перенесеныfetch_calls[0]→fetch_calls[1],git_calls2→3,remote_calls1→2. Validation-fail пути обоснованно не тронуты (падают до fetch/git).assert "base" not in body→assert body.get("base") == "main"— правильно ловит фикс. Обновлённый docstring объясняет почемуbaseобязателен.## Watch outдокументирует overhead 2 remote lookups (acceptable, ~ms),## Pendingпредлагает оптимизацию (передать pre-resolvedfullвrunGh). Оба осмысленны.Suggestions (info, not blocking)
create-pr.ts makes 1 spawnSync call (gh pr create) on the success path.Теперь success path делает 3 spawnSync (rev-parse + 2 remote lookup) + 2 fetch (getRepo + POST /pulls). Обновить до актуального счета — иначе следующий ревьюер запутается.⚠️ create_pr failed: cannot resolve Forgejo repoне включаетissue_number, в отличие от line 74 (⚠️ create_pr failed for issue #${args.issue_number || "?"}: ...). Для консистентности добавить issue number и в первый message — хотя resolveForgejoRepo failure происходит до знания repo, issue_number уже доступен из args.baseне имеет прямого unit-теста (вызовcallForgejoGhсpr createargv без--base). Guard косвенно покрыт (create-pr всегда передаёт--base), но прямой тест защитил бы от регрессии, если кто-то вызоветrunGh(["pr","create",...])без--baseнапрямую. Опционально — текущее покрытие достаточное для fix.Verdict: APPROVE