labctl pr logs: Forgejo log adapter 500s on retried runs (attempt/2), blocking lander validation #259

Closed
opened 2026-08-02 04:56:17 +02:00 by dominik.polakovics · 1 comment

Hit while validating #258 (branch afk/257) as an autoland lander. labctl pr logs could not return any logs, so the failing-check triage step of the validation core was impossible through the CLI.

Symptom

Both forms fail identically:

$ labctl pr logs 258
labctl pr logs: tracker: forge log route did not answer the shape lab's log adapter expects:
GET /Cloonar/coding-lab/actions/runs/353/jobs/0/attempt/2/logs answered 500 "text/plain"
for check "ci / native (pull_request)" — lab's Forgejo log adapter does not match this
forge version; file an issue on coding-lab, then debug from local repro

$ labctl pr logs 258 --check "ci-nix / flake-check (pull_request)"
… same, for runs/352/jobs/0/attempt/2/logs

What is actually going on

The adapter resolves the latest attempt and requests …/attempt/2/logs. On these runs that route genuinely 500s on the forge side — but attempt/1 serves fine:

runs/352 attempt=1  http=200  size=14935
runs/353 attempt=1  http=200  size=21133
runs/352 attempt=2  http=500  size=62
runs/353 attempt=2  http=500  size=62

So this is not purely a shape/version mismatch — the forge has no stored log blob for attempt 2 (these runs were retried during a forge outage; see below), and the adapter treats the 500 as terminal instead of falling back.

Suggested fixes

  1. Fall back to earlier attempts when the latest attempt's log route errors — an older attempt's logs are far more useful than none.
  2. Surface the raw upstream status rather than folding a 500 into "adapter does not match this forge version" — that message sent me looking for a version incompatibility when the real cause was a missing log blob.
  3. Consider exposing --attempt <n> so an operator can reach a specific attempt directly.

Why it matters

The validation core tells a lander to read failing jobs' logs before any local repro. With this broken, a lander on a container without the project toolchain (no nix/go/node) has no path to triage a red check at all, and has to fall back to curl-ing the forge web routes by hand — which is what I did here.

Context

The #258 runs this surfaced on were themselves infra casualties: both attempt-1 logs stop ~75s in with no error recorded, ~10 minutes before the jobs were marked "Failing after 11m42s", and the forge was returning connection refused then 502 Bad Gateway during the same window.

Hit while validating #258 (branch `afk/257`) as an autoland lander. `labctl pr logs` could not return any logs, so the failing-check triage step of the validation core was impossible through the CLI. ## Symptom Both forms fail identically: ``` $ labctl pr logs 258 labctl pr logs: tracker: forge log route did not answer the shape lab's log adapter expects: GET /Cloonar/coding-lab/actions/runs/353/jobs/0/attempt/2/logs answered 500 "text/plain" for check "ci / native (pull_request)" — lab's Forgejo log adapter does not match this forge version; file an issue on coding-lab, then debug from local repro $ labctl pr logs 258 --check "ci-nix / flake-check (pull_request)" … same, for runs/352/jobs/0/attempt/2/logs ``` ## What is actually going on The adapter resolves the **latest attempt** and requests `…/attempt/2/logs`. On these runs that route genuinely 500s on the forge side — but **`attempt/1` serves fine**: ``` runs/352 attempt=1 http=200 size=14935 runs/353 attempt=1 http=200 size=21133 runs/352 attempt=2 http=500 size=62 runs/353 attempt=2 http=500 size=62 ``` So this is not purely a shape/version mismatch — the forge has no stored log blob for attempt 2 (these runs were retried during a forge outage; see below), and the adapter treats the 500 as terminal instead of falling back. ## Suggested fixes 1. **Fall back to earlier attempts** when the latest attempt's log route errors — an older attempt's logs are far more useful than none. 2. **Surface the raw upstream status** rather than folding a 500 into "adapter does not match this forge version" — that message sent me looking for a version incompatibility when the real cause was a missing log blob. 3. Consider exposing `--attempt <n>` so an operator can reach a specific attempt directly. ## Why it matters The validation core tells a lander to read failing jobs' logs *before* any local repro. With this broken, a lander on a container without the project toolchain (no `nix`/`go`/`node`) has no path to triage a red check at all, and has to fall back to `curl`-ing the forge web routes by hand — which is what I did here. ## Context The #258 runs this surfaced on were themselves infra casualties: both attempt-1 logs stop ~75s in with no error recorded, ~10 minutes before the jobs were marked "Failing after 11m42s", and the forge was returning `connection refused` then `502 Bad Gateway` during the same window.
Author
Owner

This was generated by AI during triage.

Agent Brief

Category: bug
Summary: The Forgejo log adapter treats a forge-side 5xx on the latest attempt as a terminal "adapter mismatch", instead of falling back to an earlier attempt's logs and reporting upstream errors honestly.

Current behavior:
Tracker.CheckLog on the Forgejo backend resolves a check's Actions job from its commit-status target_url and probes attempt/1, 2, … until the first 404, serving the last 200 (the ADR-0060 design). Any other answer — including an upstream 500 — falls into a catch-all that wraps tracker.ErrLogAdapterMismatch, whose message claims "lab's Forgejo log adapter does not match this forge version; file an issue on coding-lab". The agentapi PR-logs handler maps that to a 502, and labctl pr logs prints it for both the default (failing subset) and --check forms.

Repro confirmed by code trace and the reporter's HTTP evidence: on runs retried during a forge outage, the forge stores no log blob for attempt 2 and 500s on its log route, while attempt 1 serves 200 with full logs. The adapter dies on the attempt-2 500 and never serves the attempt-1 logs it could have. The misleading "forge version" framing sent the reporter hunting a version incompatibility when the real cause was a missing log blob. An existing adapter fixture test deliberately pins the 500→mismatch fold, so this is designed-then-outgrown behavior, not an accident.

Desired behavior:

  1. Fallback: when a later attempt's log route answers a server error (5xx) but an earlier attempt already answered 200 text/plain, serve the newest attempt's logs that did answer 200. The fallback must be loud per ADR-0060's "loud, never silent" pin: the operator running labctl pr logs must be told which attempt's logs they are reading and that the latest attempt's logs were unavailable (upstream status included), without corrupting the log body on stdout as a parse/pipe surface.
  2. Honest classification: when no attempt serves logs because the forge answers 5xx (e.g. a 500 on attempt 1), the error must state the requested route and raw upstream status and identify it as a forge-side error — it must not claim an adapter/forge-version mismatch. ErrLogAdapterMismatch remains reserved for genuine shape surprises: a 200 that isn't text/plain, a 404 on attempt 1, a target_url that stops matching the Actions job shape, the attempt-probe cap.
  3. The distinction must survive the wire: the agentapi handler should map the two classes to statuses/messages that keep them distinguishable at the CLI (the existing 502-with-verbatim-message convention may carry both, so long as the texts are honest).

Key interfaces:

  • The Forgejo backend's CheckLog attempt-probe loop — its status handling needs the fallback and the new error classification.
  • tracker.ErrLogAdapterMismatch — unchanged in meaning, narrowed in application. A sibling sentinel (or equivalent distinguishable error) for "forge log route upstream error" is expected.
  • The agentapi PR-logs handler's error→status mapping, and labctl pr logs stderr messaging — whatever mechanism carries the "served attempt N, latest attempt M errored" notice must reach the CLI user (stderr), not silently vanish server-side.
  • ADR-0060's pins ("version-coupling is loud, never silent", "latest attempt by probing") — amend the ADR (delta note or successor) to record the fallback and the mismatch/upstream-error split.

Acceptance criteria:

  • A run whose latest attempt 500s but whose earlier attempt serves 200: labctl pr logs (both default and --check forms) exits 0, prints the earlier attempt's logs, and announces the fallback (attempt served, latest-attempt upstream status) out of band of the log body.
  • A run where every probed attempt 5xxs (including attempt 1): the CLI error names the requested log route and upstream status as a forge-side error, and does not contain the "does not match this forge version" claim.
  • Genuine shape mismatches (non-text/plain 200, 404 on attempt 1, unmatched target_url shape, probe cap exceeded) still produce ErrLogAdapterMismatch with the existing actionable message.
  • Adapter fixture tests cover the fallback and the upstream-error classification; the existing test pinning 500→mismatch is updated to the new contract; handler and labctl layers keep their mismatch-path coverage.
  • ADR-0060 is amended (or superseded) to record the changed pins.
  • Redaction still applies to fallback-served logs exactly as to normally-served logs (no new unredacted path).

Out of scope:

  • An --attempt <n> operator flag (the reporter's suggestion 3) — file separately if wanted; the fallback covers the operational need that motivated it.
  • Retry/backoff on transient forge errors beyond the attempt fallback.
  • GitHub backend log support, session-cookie auth for private repos (both deferred by ADR-0060).
  • Fixing the forge-side cause (missing log blob for runs retried during an outage).
> *This was generated by AI during triage.* ## Agent Brief **Category:** bug **Summary:** The Forgejo log adapter treats a forge-side 5xx on the latest attempt as a terminal "adapter mismatch", instead of falling back to an earlier attempt's logs and reporting upstream errors honestly. **Current behavior:** `Tracker.CheckLog` on the Forgejo backend resolves a check's Actions job from its commit-status `target_url` and probes `attempt/1, 2, …` until the first 404, serving the last 200 (the ADR-0060 design). Any other answer — including an upstream 500 — falls into a catch-all that wraps `tracker.ErrLogAdapterMismatch`, whose message claims "lab's Forgejo log adapter does not match this forge version; file an issue on coding-lab". The agentapi PR-logs handler maps that to a 502, and `labctl pr logs` prints it for both the default (failing subset) and `--check` forms. Repro confirmed by code trace and the reporter's HTTP evidence: on runs retried during a forge outage, the forge stores no log blob for attempt 2 and 500s on its log route, while attempt 1 serves 200 with full logs. The adapter dies on the attempt-2 500 and never serves the attempt-1 logs it could have. The misleading "forge version" framing sent the reporter hunting a version incompatibility when the real cause was a missing log blob. An existing adapter fixture test deliberately pins the 500→mismatch fold, so this is designed-then-outgrown behavior, not an accident. **Desired behavior:** 1. **Fallback:** when a later attempt's log route answers a server error (5xx) but an earlier attempt already answered `200 text/plain`, serve the newest attempt's logs that did answer 200. The fallback must be *loud* per ADR-0060's "loud, never silent" pin: the operator running `labctl pr logs` must be told which attempt's logs they are reading and that the latest attempt's logs were unavailable (upstream status included), without corrupting the log body on stdout as a parse/pipe surface. 2. **Honest classification:** when no attempt serves logs because the forge answers 5xx (e.g. a 500 on attempt 1), the error must state the requested route and raw upstream status and identify it as a forge-side error — it must *not* claim an adapter/forge-version mismatch. `ErrLogAdapterMismatch` remains reserved for genuine shape surprises: a 200 that isn't `text/plain`, a 404 on attempt 1, a `target_url` that stops matching the Actions job shape, the attempt-probe cap. 3. The distinction must survive the wire: the agentapi handler should map the two classes to statuses/messages that keep them distinguishable at the CLI (the existing 502-with-verbatim-message convention may carry both, so long as the texts are honest). **Key interfaces:** - The Forgejo backend's `CheckLog` attempt-probe loop — its status handling needs the fallback and the new error classification. - `tracker.ErrLogAdapterMismatch` — unchanged in meaning, narrowed in application. A sibling sentinel (or equivalent distinguishable error) for "forge log route upstream error" is expected. - The agentapi PR-logs handler's error→status mapping, and `labctl pr logs` stderr messaging — whatever mechanism carries the "served attempt N, latest attempt M errored" notice must reach the CLI user (stderr), not silently vanish server-side. - ADR-0060's pins ("version-coupling is loud, never silent", "latest attempt by probing") — amend the ADR (delta note or successor) to record the fallback and the mismatch/upstream-error split. **Acceptance criteria:** - [ ] A run whose latest attempt 500s but whose earlier attempt serves 200: `labctl pr logs` (both default and `--check` forms) exits 0, prints the earlier attempt's logs, and announces the fallback (attempt served, latest-attempt upstream status) out of band of the log body. - [ ] A run where every probed attempt 5xxs (including attempt 1): the CLI error names the requested log route and upstream status as a forge-side error, and does not contain the "does not match this forge version" claim. - [ ] Genuine shape mismatches (non-`text/plain` 200, 404 on attempt 1, unmatched `target_url` shape, probe cap exceeded) still produce `ErrLogAdapterMismatch` with the existing actionable message. - [ ] Adapter fixture tests cover the fallback and the upstream-error classification; the existing test pinning 500→mismatch is updated to the new contract; handler and labctl layers keep their mismatch-path coverage. - [ ] ADR-0060 is amended (or superseded) to record the changed pins. - [ ] Redaction still applies to fallback-served logs exactly as to normally-served logs (no new unredacted path). **Out of scope:** - An `--attempt <n>` operator flag (the reporter's suggestion 3) — file separately if wanted; the fallback covers the operational need that motivated it. - Retry/backoff on transient forge errors beyond the attempt fallback. - GitHub backend log support, session-cookie auth for private repos (both deferred by ADR-0060). - Fixing the forge-side cause (missing log blob for runs retried during an outage).
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Cloonar/coding-lab#259
No description provided.