fix(tracker): fall back to an older log attempt, stop blaming the adapter for forge 5xx #268

Merged

dominik.polakovics merged 2 commits from afk/259 into main

2026-08-03 11:43:43 +02:00

dominik.polakovics commented

2026-08-03 02:38:44 +02:00

Owner

Closes #259

labctl pr logs was unusable on runs retried during a forge outage. The Forgejo log adapter probes attempt/{k}/logs and folded any non-200/404 answer into ErrLogAdapterMismatch — "lab's Forgejo log adapter does not match this forge version". On those runs the forge has no stored log blob for attempt 2 and 500s on its route, while attempt 1 serves 200 text/plain with the full log. So a recoverable read failed, and the message pointed at a Forgejo version incompatibility that never existed. A lander on a container without the project toolchain then had no path to triage a red check at all.

What changed

Fallback (adapter). A 5xx no longer aborts the probe — the first 404 remains the single terminator — and the newest attempt that answered 200 text/plain is served. Keeping the probe going is what lets an attempt-3 log still win over a broken attempt 2.

Provenance through the seam. Tracker.CheckLog now returns CheckLogResult{Log, Attempt, FallbackFrom, FallbackStatus} instead of a bare []byte, so "which attempt is this?" survives to the caller. The handler turns a fallback into a repeated X-Lab-Log-Notice response header; labctl pr logs prints it to stderr:

labctl pr logs: check "ci / native (pull_request)": served rerun attempt 1; attempt 2's log route answered HTTP 500 — the latest attempt's logs are unavailable

The log body is untouched — stdout stays the byte-clean pipe/grep surface ADR-0060 and ADR-0032 pin. The forge-supplied check name is scrubbed of control bytes and length-bounded where the header value is built, so a check name carrying CR/LF cannot smuggle a header.

Honest classification. New sentinel tracker.ErrLogUpstream — "the forge failed to serve this log" — names the requested route and the raw upstream status. ErrLogAdapterMismatch narrows to genuine shape surprises: a 200 that is not text/plain, a 404 on attempt 1, a target_url that stops matching the Actions job shape, any other unexpected status, the probe cap. Both map to 502 with their message verbatim, so the CLI keeps them apart by text and sends the reader at the right system.

Redaction is unchanged — fallback-served logs take the same fail-closed route, no new byte path.

Acceptance criteria

Latest attempt 500s, earlier serves 200 → exit 0, earlier attempt's logs on stdout, fallback announced out of band, both default and --check forms — TestPRLogsCheckFallbackNotice, TestPRLogsDefaultFallbackNotices
Every probed attempt 5xxs → error names the route and upstream status as forge-side, no "does not match this forge version" — TestCheckLog_everyAttemptServerErrorIsUpstream, TestCheckLog_serverErrorThen404IsUpstream, TestPRLogsUpstreamError (handler and labctl)
Genuine shape mismatches still ErrLogAdapterMismatch with the existing actionable message — 404-on-attempt-1, non-text/plain 200, 403, unmatched target_url, probe cap; TestPRLogsAdapterMismatch at the CLI
Adapter fixture tests cover fallback and upstream classification; the test that pinned 500→mismatch is rewritten to the new contract; handler and labctl keep their mismatch coverage
ADR-0060 amended (Amended by issue #259), and the three pin sentences the amendment falsified are corrected in place
Redaction applies to fallback-served logs — TestPRLogsFallbackRedaction

Verification

gofmt -l . clean · go build ./... · go vet ./... · go test -count=1 ./... fully green · golangci-lint run → 0 issues.

Out of scope (per the issue)

--attempt <n>, retry/backoff beyond the attempt fallback, GitHub backend log support, session-cookie auth, and the forge-side missing-blob cause.

🤖 Generated with Claude Code

Closes #259 `labctl pr logs` was unusable on runs retried during a forge outage. The Forgejo log adapter probes `attempt/{k}/logs` and folded **any** non-200/404 answer into `ErrLogAdapterMismatch` — "lab's Forgejo log adapter does not match this forge version". On those runs the forge has no stored log blob for attempt 2 and 500s on its route, while attempt 1 serves `200 text/plain` with the full log. So a recoverable read failed, and the message pointed at a Forgejo version incompatibility that never existed. A lander on a container without the project toolchain then had no path to triage a red check at all. ## What changed **Fallback (adapter).** A 5xx no longer aborts the probe — the first 404 remains the single terminator — and the newest attempt that answered `200 text/plain` is served. Keeping the probe going is what lets an attempt-3 log still win over a broken attempt 2. **Provenance through the seam.** `Tracker.CheckLog` now returns `CheckLogResult{Log, Attempt, FallbackFrom, FallbackStatus}` instead of a bare `[]byte`, so "which attempt is this?" survives to the caller. The handler turns a fallback into a repeated `X-Lab-Log-Notice` response header; `labctl pr logs` prints it to **stderr**: ``` labctl pr logs: check "ci / native (pull_request)": served rerun attempt 1; attempt 2's log route answered HTTP 500 — the latest attempt's logs are unavailable ``` The log **body is untouched** — stdout stays the byte-clean pipe/grep surface ADR-0060 and ADR-0032 pin. The forge-supplied check name is scrubbed of control bytes and length-bounded where the header value is built, so a check name carrying CR/LF cannot smuggle a header. **Honest classification.** New sentinel `tracker.ErrLogUpstream` — "the forge failed to serve this log" — names the requested route and the raw upstream status. `ErrLogAdapterMismatch` narrows to genuine shape surprises: a 200 that is not `text/plain`, a 404 on attempt 1, a `target_url` that stops matching the Actions job shape, any other unexpected status, the probe cap. Both map to 502 with their message verbatim, so the CLI keeps them apart by text and sends the reader at the right system. Redaction is unchanged — fallback-served logs take the same fail-closed route, no new byte path. ## Acceptance criteria - [x] Latest attempt 500s, earlier serves 200 → exit 0, earlier attempt's logs on stdout, fallback announced out of band, both default and `--check` forms — `TestPRLogsCheckFallbackNotice`, `TestPRLogsDefaultFallbackNotices` - [x] Every probed attempt 5xxs → error names the route and upstream status as forge-side, no "does not match this forge version" — `TestCheckLog_everyAttemptServerErrorIsUpstream`, `TestCheckLog_serverErrorThen404IsUpstream`, `TestPRLogsUpstreamError` (handler and labctl) - [x] Genuine shape mismatches still `ErrLogAdapterMismatch` with the existing actionable message — 404-on-attempt-1, non-`text/plain` 200, 403, unmatched `target_url`, probe cap; `TestPRLogsAdapterMismatch` at the CLI - [x] Adapter fixture tests cover fallback and upstream classification; the test that pinned 500→mismatch is rewritten to the new contract; handler and labctl keep their mismatch coverage - [x] ADR-0060 amended (`Amended by issue #259`), and the three pin sentences the amendment falsified are corrected in place - [x] Redaction applies to fallback-served logs — `TestPRLogsFallbackRedaction` ## Verification `gofmt -l .` clean · `go build ./...` · `go vet ./...` · `go test -count=1 ./...` fully green · `golangci-lint run` → 0 issues. ## Out of scope (per the issue) `--attempt <n>`, retry/backoff beyond the attempt fallback, GitHub backend log support, session-cookie auth, and the forge-side missing-blob cause. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dominik.polakovics added 1 commit

2026-08-03 02:38:44 +02:00

fix(tracker): fall back to an older log attempt, stop blaming the adapter for forge 5xx

ci / native (pull_request) Successful in 8m24s

Details

c62483ece2

`labctl pr logs` died on runs retried during a forge outage: the Forgejo log
adapter probes `attempt/{k}/logs` and folded ANY non-200/404 answer into
`ErrLogAdapterMismatch` ("lab's Forgejo log adapter does not match this forge
version"). On those runs the forge has no stored log blob for attempt 2 and
500s, while attempt 1 serves 200 with the full log — so a recoverable read
failed, and the message sent the reporter hunting a version incompatibility
that did not exist. A lander with no local toolchain then had no way to triage
a red check at all.

Two changes, both pinned by an ADR-0060 amendment:

- Fallback. A 5xx no longer aborts the probe (the first 404 is still the single
  terminator); the newest attempt that answered `200 text/plain` is served.
  `Tracker.CheckLog` returns `CheckLogResult{Log, Attempt, FallbackFrom,
  FallbackStatus}` so the provenance survives the seam: the handler turns it
  into a repeated `X-Lab-Log-Notice` response header and `labctl pr logs`
  prints it to stderr. The log body is untouched — stdout stays the byte-clean
  pipe/grep surface — and the notice's forge-supplied check name is sanitized
  and bounded before it reaches the wire.
- Honest classification. A new `tracker.ErrLogUpstream` carries "the forge
  failed to serve this log", naming the route and the raw upstream status;
  `ErrLogAdapterMismatch` narrows to genuine shape surprises (non-text/plain
  200, 404 on attempt 1, an unmatched target_url, other statuses, the probe
  cap). Both map to 502 with their message verbatim, so the CLI keeps them
  apart by text and points the reader at the right system.

Redaction is unchanged: fallback-served logs take the same fail-closed route.

Closes #259

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

dominik.polakovics commented

2026-08-03 02:48:41 +02:00

Author

Owner

[autoland] verdict: pass

dominik.polakovics added 1 commit

2026-08-03 11:31:27 +02:00

Merge branch 'main' into afk/259

agent-tools / publish (pull_request) Has been skipped

Details

agent-tools / smoke (pull_request) Successful in 6m4s

Details