fix(tools): forgejo fetch timeout + retries #41
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/tools/forgejo-fetch-timeout"
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?
Что сделано
Обёрнут
fetch()вcallForgejo(_shared.ts) в retry-цикл с таймаутом черезAbortSignal.any([userSignal, AbortSignal.timeout(timeoutMs)]). Retry на timeout/5xx, НЕ на 4xx. Skip sleep на первой попытке (какpipeline-status.py:596). Худший кейс latency: 10×3 + 2×1 = 32с (вместо 5 минут).resolveForgejoRepo:spawnSyncполучилtimeout: 5000(раньше вис бесконечно).callForgejoGh,runGh,getPull,getRepo,resolveLabelIds: добавлен опциональныйsignal?: AbortSignal, проброшен доcallForgejo.create-issue.ts,create-pr.ts,merge-pr.ts,post-review.ts: пробросcontext.abort(SDKToolContext.abort: AbortSignal) — пользовательская отмена tool-call из UI теперь работает.OPENCODE_FORGEJO_TIMEOUT(10с),OPENCODE_FORGEJO_RETRY(3),OPENCODE_FORGEJO_RETRY_INTERVAL(1с) через helper_envInt.process.stderr.writeс[forgejo]prefix (конвенция репо, NOTconsole.*). Успешные вызовы не логируются..opencode/tools/_shared.test.ts(Bun test) — 11 кейсов: timeout→retry, 5xx→retry, 4xx no-retry, retries-exhausted readable error, user-abort no-retry, success no-retry, retry-after-5xx-success, RETRY=0 fallback, resolveLabelIds timeout, resolveForgejoRepo spawnSync timeout, explicit repo.test_create_issue_tool.py::test_labels_lookup_failureобновлён: 500 теперь ретраит (3 вызова), тест используетOPENCODE_FORGEJO_RETRY=1для мгновенного фейла.Почему
create-issuetool висел ~5 минут после инвокации.fetch()к Forgejo API вызывался безsignal/таймаута — Bun.fetch не имеет дефолтного таймаута. Forgejo за Nginx + SQLite (write-lock contention) — 5 минут серверсайд-таймаут, пока fetch покорно ждал.context.abortсуществовал в SDK, но нигде не использовался — отмена из UI не работала.Watch out
AbortSignal.anyиAbortSignal.timeoutтребуют Bun 1.2+ (подтверждено через Context7, C++ реализация вAbortSignal.cpp). Node 22+ также поддерживает (проверено — тесты через_ts_loader.mjsработают)._ts_loader.mjsне стрипаетasс составными выражениями (r.error as NodeJS.ErrnoException) — избегалasв выражениях, использовалString()проверку для ETIMEDOUT.OPENCODE_FORGEJO_RETRY=1в env.OPENCODE_FORGEJO_TIMEOUT=0— невалидный, fallback на дефолт через_envInt(Number.isFinite + n >= 0, но 0×1000=0ms = мгновенный таймаут — в проде не ставьте 0).node_modules/и untracked SVG-шаблоны в.opencode/draw-image/templates/НЕ закоммичены (были до меня, вне scope).Pending
_shared.test.ts) не запускается в CI (bun не установлен в Docker-образе) — файл будет работать на машине с bun. Python-тесты через_ts_loader.mjsпокрывают retry-логику.PRAGMA busy_timeout, Nginxproxy_read_timeout) — отдельный issue вforgejo-infra.Closes #40
Closes #40
Code Review Summary
PR добавляет таймаут и retry-логику в
callForgejo(.opencode/tools/_shared.ts) и пробрасываетcontext.abortво все 4 tool-файла (create-issue,create-pr,merge-pr,post-review). 8 файлов, +389/−32. Качество реализации высокое, тесты покрывают все ветки.Что правильно
userSignal?.abortedперед каждой попыткой и в catch-блоке. AbortController пользователя уважается._sleepс cleanup:clearTimeout+{ once: true }abort-listener — нет утечки таймеров. Приms <= 0— no-op (skip-sleep-on-first не нужен, т.к. sleep только между попыткамиattempt < maxAttempts)._envInt:Number.isFinite+>= 0+Math.trunc— защита от NaN, Infinity, отрицательных и дробных значений. Fallback при undefined/"".resolveForgejoRepo:spawnSyncсtimeout: 5000— нет зависания при подвисшем git. Проверка ETIMEDOUT логирует в stderr._forgejoLogв stderr — не засоряет stdout (который парсится как JSON tool-результат).signal— optional параметр в конце сигнатуры всех функций (runGh,getPull,getRepo,resolveLabelIds,callForgejoGh). Существующие вызовы без signal продолжают работать..env.example: документация с формулой worst-case latency (10×3 + 2×1 = 32s) — помогает оператору понять потолок задержки.OPENCODE_FORGEJO_RETRY=1вtest_labels_lookup_failure— тест остаётся детерминированным (1 fetch, не 3 retry).Замечания (info, не блокирующие)
_shared.ts:55-58[dead-log] Проверкаr.error && String(r.error).includes("ETIMEDOUT")стоит послеif (r.status !== 0) return null. При ETIMEDOUTr.status= null (≠ 0) → return null срабатывает раньше → log-line недостижим. Функционально return null корректен, но log-сообщение мёртвое. Можно перенести проверку error выше проверки status, либо убрать log._shared.test.ts:256[style] Нет trailing newline (EOF without newline). Minor, не влияет на работу._shared.test.ts[test-hygiene]globalThis.fetch = fetchMock— глобальный mock, но вafterEachвосстанавливается только env, не fetch. Между тестами в одномdescribemock накапливается, но т.к. каждый тест заново делаетawait import("./_shared")после установки mock — работает. Для строгости можно добавитьoriginalFetchsave/restore в beforeEach/afterEach.Замечаний критического уровня нет. PR готов к merge.
Verdict: APPROVE