Fail-fast Forgejo timeouts: --connect-timeout + tighter --max-time #396

Merged

joshtronic merged 2 commits from fix/forgejo-connect-timeout into master

2026-07-16 15:01:06 +00:00

igor commented

2026-07-16 13:39:09 +00:00

Collaborator

Fixes #395 — a few-second git.sherver.org blip hung a tick ~30s then died (curl exit 28), tripping a false task fail health page.

Root cause: lib/forgejo.sh curl wrappers used --max-time 30 with no --connect-timeout, so a briefly-unreachable server stalled a call for the full max-time before failing. Under the tick's errexit that aborted the tick; the cleanup() trap fired hc_ping task fail.

Fix: add --connect-timeout 5 and tighten --max-time 30 → 15 across the Forgejo curl paths (_fj, _forgejo_post_reviewers; the image upload keeps --max-time 60 but gains the connect-timeout). A blip now rides through or fails in ~5s instead of ~30s — keeps the agent moving. Values hardcoded (one operator).

Not aggressiveness: validation git-fetches are cached 15 min/repo; a tick makes only ~10-30 small API calls. The problem was timeout length, not call volume.

Test: shims curl and asserts _fj passes the fail-fast flags — fails without them (verified). make test + shellcheck green.

Follow-up (noted in #395, not here): unguarded best-effort recovery-sweep callers still propagate a transient failure fatally, so a blip can still page — just faster; worth making those best-effort. The co-located basename reaper bug rides this path too — PR #393.

Closes #395

Fixes #395 — a few-second git.sherver.org blip hung a tick ~30s then died (curl exit 28), tripping a false `task fail` health page. **Root cause:** `lib/forgejo.sh` curl wrappers used `--max-time 30` with **no `--connect-timeout`**, so a briefly-unreachable server stalled a call for the full max-time before failing. Under the tick's errexit that aborted the tick; the `cleanup()` trap fired `hc_ping task fail`. **Fix:** add `--connect-timeout 5` and tighten `--max-time` 30 → 15 across the Forgejo curl paths (`_fj`, `_forgejo_post_reviewers`; the image upload keeps `--max-time 60` but gains the connect-timeout). A blip now rides through or fails in ~5s instead of ~30s — keeps the agent moving. Values hardcoded (one operator). **Not aggressiveness:** validation git-fetches are cached 15 min/repo; a tick makes only ~10-30 small API calls. The problem was timeout length, not call volume. **Test:** shims `curl` and asserts `_fj` passes the fail-fast flags — fails without them (verified). `make test` + shellcheck green. Follow-up (noted in #395, not here): unguarded best-effort recovery-sweep callers still propagate a transient failure fatally, so a blip can still page — just faster; worth making those best-effort. The co-located `basename` reaper bug rides this path too — PR #393. Closes #395

igor added 1 commit

2026-07-16 13:39:09 +00:00

Fail-fast Forgejo timeouts: add --connect-timeout, tighten --max-time

Lint / check-sync (push) Successful in 5s

Details

Lint / check-sync (pull_request) Successful in 4s

Details

c3002edc8a

A brief (~few-second) git.sherver.org blip hung a tick ~30s and then died
with curl exit 28: the _fj wrapper used --max-time 30 with NO
--connect-timeout, so a briefly-unreachable server stalled the call for the
full max-time before failing. Under the tick's errexit that aborted the tick,
and the cleanup() trap fired hc_ping task fail -> a false task-fail health page
(the harness self-recovered on the very next tick).

Add --connect-timeout 5 and tighten --max-time 30 -> 15 across the Forgejo curl
paths (_fj, _forgejo_post_reviewers; the image upload keeps --max-time 60 but
gains the connect-timeout). A blip now rides through or fails in ~5s instead of
~30s, keeping the agent moving. Values hardcoded (one operator).

Not an aggressiveness problem: validation git-fetches are cached 15 min/repo and
a tick makes only ~10-30 small API calls -- the issue was timeout length.

Test shims curl and asserts _fj passes the fail-fast flags (fails without them).

Closes #395

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4YUVfbfUWdUwifGJ2TvkP

igor commented

2026-07-16 13:41:37 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for c3002edc: success

Adds --connect-timeout 5 and tightens --max-time 30→15 on the Forgejo curl wrappers so a brief server blip fails fast instead of wedging a tick. Clean, focused, well-tested. Approve.

lib/forgejo.sh: Introduces FORGEJO_CONNECT_TIMEOUT=5 / FORGEJO_MAX_TIME=15 and threads them through _fj (both body/no-body branches), _forgejo_post_reviewers, and the image-upload path (which correctly keeps its --max-time 60 per the description). The fix is sound — a bare --max-time with no connect-timeout does stall the full max-time on an unreachable host, and bounding the connect phase is the right lever.
bin/test-forgejo.sh: The new test shims curl to echo args, runs the real _fj GET /version (no-body → else branch) before fixtures stub _fj, and asserts both flags are present plus that the connect timeout is < 30. This genuinely fails without the fix. Good targeted coverage.
Scope/checklist: ~30 lines, one issue (#395), no drive-by changes. Description matches the diff exactly (values hardcoded as vars, image upload keeps 60s, follow-ups deferred to #395/#393). CI green.

No security, correctness, or scope concerns. The 5s/15s values are a reasonable operator judgment call for small same-network JSON calls.
===BODY===

_{Independent review by the harness on claude-opus-4-8 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `APPROVE` _(automated)_ CI for `c3002edc`: **success** Adds `--connect-timeout 5` and tightens `--max-time` 30→15 on the Forgejo curl wrappers so a brief server blip fails fast instead of wedging a tick. Clean, focused, well-tested. Approve. - **`lib/forgejo.sh`**: Introduces `FORGEJO_CONNECT_TIMEOUT=5` / `FORGEJO_MAX_TIME=15` and threads them through `_fj` (both body/no-body branches), `_forgejo_post_reviewers`, and the image-upload path (which correctly keeps its `--max-time 60` per the description). The fix is sound — a bare `--max-time` with no connect-timeout does stall the full max-time on an unreachable host, and bounding the connect phase is the right lever. - **`bin/test-forgejo.sh`**: The new test shims `curl` to echo args, runs the *real* `_fj GET /version` (no-body → else branch) before fixtures stub `_fj`, and asserts both flags are present plus that the connect timeout is `< 30`. This genuinely fails without the fix. Good targeted coverage. - **Scope/checklist**: ~30 lines, one issue (#395), no drive-by changes. Description matches the diff exactly (values hardcoded as vars, image upload keeps 60s, follow-ups deferred to #395/#393). CI green. No security, correctness, or scope concerns. The 5s/15s values are a reasonable operator judgment call for small same-network JSON calls. ===BODY=== --- <sub>Independent review by the harness on `claude-opus-4-8` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-16 13:41:38 +00:00

22 seconds

igor requested review from joshtronic

2026-07-16 13:41:38 +00:00

igor referenced this pull request

2026-07-16 13:59:57 +00:00

agent.service tick exited status=28 during recovery sweep #394

joshtronic added 1 commit

2026-07-16 15:00:40 +00:00