fix(actions): fetch job logs via the REST API when the instance has it #21
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/actions-logs-api"
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?
get_run_logs/get_job_logs404 on every run of a private repo — successful runs included, seconds after they finish. That cost hours of misdiagnosis during twok3s-clusterreviews, because the failure looks exactly like "the logs are gone."Root cause
Logs were fetched from a web route via
DoRaw(no/api/v1). Forgejo honoursAuthorization: token Xon/api/v1only; web routes authenticate by session cookie. The request is therefore anonymous, and for a private repo Forgejo answers 404 fromRepoAssignment— hiding the repo's existence rather than returning 401 — so it never reached the log lookup at all. It would have worked against a public repo, which is very likely why it shipped and tested fine.Confirmed directly, on a disposable 16.0.4 with a private repo and the same token:
The fix
Forgejo ≥ 16.0 exposes the endpoints this needs, and
fetchJobLogsnow uses them, falling back to the web route +diagnoseLogs404when they are absent — correct on both sides of an upgrade, so this can land before the server is upgraded.Corrections after live verification
The earlier revisions of this PR were written from the release notes and Forgejo's route names. Two load-bearing details were wrong, and neither was visible to the unit tests, because those tests asserted fixtures this PR invented. Verified against a disposable 16.0.4 (
sha256:a3e33d03e771d3e58b27de5573c3a25dc4f670583a6724c1878a6d0bbecf3556) with a private repo, token auth and real Actions runs:1. The jobs listing is a bare JSON array, not
{"jobs":[…]}.ListActionRunJobsdoesctx.JSON(200, []*api.ActionRunJob):Decoding the envelope produced an
UnmarshalTypeError, whichclassifyRouteErrreads as route absent — so every fetch silently fell back to the web route and 404'd on private repos. The PR fixed nothing on a v16 server.2. Every
/api/v1actions route keys on the run's DATABASE id; the number in/actions/runs/<N>is the per-repo index.ListActionRunJobs→GetRunByID, while the web handler →GetRunByIndex. On the test instance:Passing the index into an id-keyed route reads a different run wherever the spaces have diverged — a silent wrong answer, not a failure. Resolution now goes through the documented
?run_number=filter and re-checksindex_in_repoon the row it gets back, so a server that ignores the filter falls back rather than answering with another run.Defect 1 masked defect 2: the envelope always fell back, so the wrong id never reached a route that would act on it. Fixing either alone is worse than fixing neither.
3. Attempts are addressable on 16.x — the previous handoff was unnecessary.
GET .../jobs/{job_id}/logstakes a documented?attempt=Nmatching the listing'sattemptfield. Proven by re-running one job of a two-job run:The previous revision refused the API for
attempt>1and handed off to the web route — which cannot serve a private repo at all, making "give me attempt 2" a guaranteed failure rather than a supported query.4. Two diagnostics on the fallback path (now the only path pre-16 servers take):
diagnoseLogs404's "you passed a DATABASE id" branch now also requiresrun.RunNumber != runID. Where the two id spaces coincide it fired and told the caller to "retry with N" when they had just passed N. Reproduced on 15.0.5, where run index 1 is also db id 1.Live matrix — Forgejo 16.0.4, private repo, token auth
Driving the actual
get_run_logstool (livecheck/), with run index → DB id diverged on purpose by seeding a decoy repo first:run_idjob_index=0task 6 of job alphajob_index=1task 7 of job beta(distinct)task 5 of job boomjob_indexjob_index 5 out of range for run 3: 2 job(s) [0=alpha, 1=beta]attempt=2task 8 of job alpha(distinct from attempt 1)attempt=99run 3 job 0 ("alpha") has 2 attempt(s); attempt 99 does not existno run with index 77 … use list_workflow_runs to find itBefore this fix, at head
a653b29, every one of those cases failed — and the error told the caller to check thatrun_idwas the URL-style index, which is exactly what they had passed. The tool's two messages pointed at each other in a loop.Forgejo 15.0.5 — fallback reconfirmed
Disposable 15.0.5 (
sha256:eda2e378442d2f18cfa563994f8ad66e71f04ac9c3bb4259cc57bdd641890f5c), private repo, real run:The unregistered routes are detected as unsupported, the diagnostic fallback is reached, and the private-repo web 404 is not described as log expiry — it names the session-cookie cause and the upgrade as the fix.
Tests
Fixtures are now copied from the live server. Mutation-checked on the merged suite:
{"jobs":[…]}envelope?attempt=jobs[jobIndex]→jobs[0]go build,go vet,go test -race -shuffle=on ./...all pass (14 packages); a full Go 1.25 build passes;gofmtclean on the touched files (the repo has 23 pre-existing unformatted files onmain, unchanged here).livecheck/is an opt-in harness that skips unlessLIVE_URLis set, so it is inert in CI. It exists because this PR's first revision was green on invented fixtures — and it is what re-verifies the matrix after the server upgrade instead of trusting this evidence indefinitely.Verdict
Forgejo 16 does solve authenticated Actions-log retrieval for private repos, and with these corrections this PR delivers it. The server still needs upgrading 15.0.5 → 16.0.4; this change makes that upgrade sufficient rather than requiring a second code change afterwards.
Not done here, and not authorized in this session: no production deployment, and no merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_018zetaP8acDgC57KdzbdAoa
get_run_logs/get_job_logs 404 on EVERY run of a private repo — successful runs included, seconds after they finish. The message blamed a missing run or expired logs; neither was true. Root cause, from Forgejo's own access log: GET /Philflow/k3s-cluster/actions/runs/1489/jobs/0/attempt/1/logs -> 404 @ context/repo.go:456(context.RepoAssignment) GET /api/v1/repos/Philflow/flow.raven/actions/runs?... (same token, same second) -> 200 OK Logs were fetched from a WEB route via DoRaw (no /api/v1). Forgejo honours "Authorization: token X" on /api/v1 only; web routes authenticate by session cookie. The request is therefore anonymous, and for a PRIVATE repo Forgejo answers 404 from RepoAssignment to hide the repo's existence rather than 401 — so it never reached the log lookup at all. It would have appeared to work against a public repo, which is likely why it shipped. Forgejo >= 16.0 added the endpoints this needs (release 2026-07): GET /api/v1/repos/{o}/{r}/actions/runs/{run}/jobs GET /api/v1/repos/{o}/{r}/actions/jobs/{job_id}/logs (text/plain) fetchJobLogs now tries those first and falls back to the existing web route plus diagnoseLogs404 when they are absent, so this is correct on both sides of an upgrade. Probed against the live 15.0.5 instance: both API routes return the router's bare `404 page not found`, the detection reports handled=false, and the existing fallback diagnostics are reached unchanged. Distinguishing "route absent" from "run absent" needs care: both are HTTP 404. A registered route answers with Forgejo's typed JSON error, an unregistered one with the router's bare text — apiHasJobsRoute probes run id 0 (which never exists) and branches on the shape, so a pre-16 instance falls back instead of reporting a missing run. Added pkg/forgejo.DoRawAPI (DoRaw against /api/v1); DoRaw's body is factored into doRawAt so both share one path. Tests: 4 new, each mutation-checked -- reintroduce the bug (DoRawAPI -> same-origin) -> FAIL (caught) always claim the route exists (kills the fallback) -> FAIL (caught) drop the job-index bounds check -> FAIL (caught) restored -> ok TestLogFetchUsesAPIPrefix asserts every log request carries /api/v1, which is the single assertion that would have caught the original defect. Also refreshed the diagnostic's stale "Forgejo v14 / until v16 ships" wording, since v16 has now shipped and the tool picks the API up automatically. Full suite green (12 packages), go vet clean, gofmt clean on the touched files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WnSBZGYDQzT8wBWvFUFLGSelf-review of the previous commit caught two things in my own work. 1. `?attempt=N` on the run-jobs listing was INVENTED. Forgejo 16's release notes document GET /repos/{o}/{r}/actions/jobs/{job_id}/logs and a run-level /actions/runs/{run_id}/logs zip; no attempt filter on the job listing is documented anywhere I could find. Sending a parameter the server may ignore risks silently returning the LATEST attempt's jobs to a caller who asked for an older one — a quiet wrong answer, which is worse than a visible failure. Removed. 2. With the parameter gone, `attempt` would have been silently ignored on the API path. Instead, attempt>1 now returns handled=false and hands off to the web route, which does take an attempt segment. The fallback's honest 404 is better than confidently serving the wrong attempt. Covered by a test that also asserts NO API call is made in that case. Also marked fetchJobLogsViaAPI as UNVERIFIED AGAINST A REAL v16 INSTANCE. The job-LISTING route it needs to turn job_index into job_id is not confirmed in v16's release notes, and the only instance available here is 15.0.5 where it does not exist. The design makes being wrong harmless — an absent route trips the detection, reports handled=false, and falls back exactly as on 15.x — but the claim should not be stronger than the evidence, so it says so in the code. Tests 4 -> 5, all passing; full suite green (12 packages), vet clean, gofmt clean on the touched files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WnSBZGYDQzT8wBWvFUFLGThree experts (A 86, B 84, C 89). Their findings converged on one theme: my own comment claimed "being wrong is harmless — it falls back", and that was only true for route ABSENCE. Every other way v16 could differ produced a confident wrong answer instead. Now falls back (errNoAPIRoute) on all of them: - decode error / unexpected shape (a bare array, a type mismatch) - a successful decode with ZERO jobs -- almost certainly a different envelope key, not a run without jobs. It previously reported "0 job(s)" and never tried the web route. - an opaque 404 body (proxy-rewritten HTML, empty) -- unknown now resolves to "fall back", which costs one request instead of lying about the run_id - a non-text content-type from the logs route And reports honestly rather than falling back where falling back would hide a real fault: a 5xx, a timeout, or any non-404 is now surfaced as itself, WITHOUT the "run_id is the URL-style index" hint that was previously appended to every failure and sent callers to fix the wrong thing. Security (expert A): owner/repo were interpolated into the API paths unescaped, so a crafted repo could re-target the authenticated GET (`r?private=true` truncated the path into a query). Both now use url.PathEscape, matching the convention already in operation/issue/label.go:116. Design (expert B): replaced the (result, error, bool) signature -- error in a non-final position, flagged by ST1008 -- with the package's own sentinel idiom, errNoAPIRoute + errors.Is, as pkg/forgejo already does with ErrNotFound et al. Six `r, e := to.X(...); return r, e, true` triples collapse back to plain `return to.X(...)`. Also dropped the second capability probe: the ORIGINAL listing error already carries the bare-text-vs-typed-JSON signal, so pre-16 now costs one request per fetch instead of two. Testing (expert C) found a surviving mutant: deleting the API call site from fetchJobLogs left the entire suite green, because every test either called the helper directly or expected the web route anyway. TestAPIIsPreferredOverThe- WebRoute pins the ordering against a v16-shaped stub. Tests 5 -> 12; full battery now: delete the API call site (C's survivor) -> killed empty-jobs fallback -> killed owner/repo escaping -> killed attempt>1 handoff -> killed decode-error fallback -> killed typed-JSON detection -> killed reintroduce THE original bug -> killed One test was asserting the wrong thing and I fixed the test, not the code: TestOwnerRepoArePathEscaped read r.URL.Path, which is the DECODED view, so a correctly escaped %2F reads back as "/" and it failed on working code. It now asserts on r.RequestURI, the wire form. Also: ErrUnauthorized now gets its own actionable message; the job_index ordering ambiguity between the API listing, the web route and list_run_jobs is documented at the point of use; the "unverified" caveat moved to the doc comment a reader reaches first. Ground truth: build, vet, 12 packages, gofmt clean on the touched files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WnSBZGYDQzT8wBWvFUFLGTakeover / delegation prompt
Take over Forgejo MCP PR #21 at exact head
4dc0c175b5358b0729d6127e61f9666254e99952.Scope and repository rules:
forgejo.philflow.me/Phil/forgejo-mcp(module path remainscodeberg.org/goern/forgejo-mcp/v2).origin/fix/actions-logs-api; do not overwrite the dirty~/.cache/forgejo-mcp-srccheckout or another agent's worktree.Primary objective:
Determine with current evidence whether Forgejo 16 actually solves authenticated Actions-log retrieval for private repositories. Do not treat the release post, route names, Swagger operation IDs, or source inspection alone as live proof.
Required investigation and verification:
GET /api/v1/repos/{owner}/{repo}/actions/runs/{run}/jobsGET /api/v1/repos/{owner}/{repo}/actions/jobs/{job_id}/logsjob_index=0andjob_index=1job_index, which must return a clear errorgofmt,go test ./...,go vet ./..., and a full Go 1.25 build. Check the PR's exact-head CI separately.origin/main. Fix confirmed defects on the PR branch with focused commits and update the PR body with exact evidence.Decision gate:
Expert B flagged the stale preamble in round 1 and I did not fix it. Re-reading it, it was the worst comment in the file: it stated the exact misconception the PR exists to correct -- "The web UI fetches both from same-origin routes that accept the standard `Authorization: token X` header thanks to Forgejo's middleware auth chain." That sentence is why the code fetched logs from a web route. Forgejo honours API tokens on /api/v1 ONLY; web routes authenticate by session cookie, so the request is anonymous and a private repo answers 404 from RepoAssignment. Leaving that claim in place would have invited the next person to undo the fix. The preamble now states both routes, which server version has which, why the web route cannot work with a token on a private repo, and that it DOES work against a public repo -- which is how the original claim survived testing. Also scoped, rather than deleted, two neighbouring comments: - the "Known caveat" block now says it applies to the pre-16 fallback only, and records that 15.0.5 behaves the same as the 14.0.3 it was written for - the tool description no longer tells callers to probe "until you get a 404"; on >= 16 an out-of-range job_index reports how many jobs exist and names them No behaviour change: build, 12 packages, gofmt clean on the touched files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Expert A, HIGH, and it is my miss: round 1 escaped owner/repo on the two API paths and left the web-route fallback ten lines below interpolating them raw. A demonstrated it end to end — with the API leg 404ing so the fallback engages, `fetchJobLogs(ctx, "..", "../../admin/users?", 7, 0, 1)` returned the target handler's body to the caller as a SUCCESSFUL result, err=nil. Two mechanisms: dot-segments that any normalising front end resolves, and a `?` in the repo name that truncates the fixed /logs suffix into a query string, removing the only thing bounding the traversal. It is an authenticated same-origin GET whose body goes back to the model, and owner/repo come from a tool call. Pre-existing on base, but fixing two paths and leaving the third is worse than not having started. Now url.PathEscape'd, with TestWebRouteFallbackAlsoEscapesOwnerRepo reproducing A's exact input and asserting on r.RequestURI (the wire form). Un-escaping it again turns the test red. Also from A's round-2 review: - classifyRouteErr's comment promised that an undecodable body falls back, but only *json.SyntaxError and *json.UnmarshalTypeError were matched. An empty body (io.EOF) or a truncated one (ErrUnexpectedEOF) surfaced exactly the unactionable decode error the comment said it avoided. Both now fall back. - A typed-JSON 404 from the jobs listing was reported unconditionally as "run not found — check run_id", but the same shape covers no-such-repo, repo-not-visible-to-this-token and actions-disabled. It now leads with the server's own message via a new serverMessage() helper, and offers the run_id hint only as a follow-up. - The listing had no ErrUnauthorized branch (the logs fetch got one in round 1), so a token-scope problem dumped a raw *HTTPError including the absolute URL. And from B's round-2 review: the fetchJobLogsViaAPI doc still described the `handled bool` that round 1 deleted; diagnoseLogs404's second branch still hardcoded v14 and pointed at the artifact API rather than the job-logs one; the tool description now states that on >= 16 `attempt` is unused unless > 1, which forces the older route; fork-specific strings ("Phil's", the instance hostname) are generalised so the change is offerable upstream; and the helper's godoc no longer names its pre-rename identifier. Ground truth: build, vet, 12 packages, -race green, gofmt clean on touched files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Round-2 mutation testing found the suite asserted the shape of a result but never its content: `to.TextResult(string(buf)) -> to.TextResult("")` survived on BOTH legs, and `job := jobs[jobIndex] -> jobs[0]` survived because every fixture had exactly one job. A log fetcher whose tests never read a log back is not tested. Adds a logText() helper that unwraps the {"Result":...} envelope, a two-job fixture pinning that job_index=1 fetches job id 22 and returns DEPLOY LOGS, and body assertions on all three success paths. TestWebRouteFallbackAlsoEscapesOwnerRepo passed for the wrong reason: it used ".." as the owner, and url.PathEscape("..") == "..", so the owner half of the regression test held with the owner escape deleted. Same class of mistake the test was written to catch. That led to the real finding behind it. PathEscape does not neutralise dot segments and Go's client transmits them verbatim, so owner=".." climbed out of /api/v1/repos/ and aimed an authenticated GET at another API path, whose body is returned to the model. Escaping bounds separators and query truncation, not traversal. Adds validateOwnerRepo() at the entry, rejecting anything outside a Forgejo name, before any request leaves the process; escaping stays as the second layer. The web leg is split into fetchJobLogsViaWeb so both defences stay independently testable. The content-type guard accepted `text/` — which is `text/html`, i.e. exactly the proxy error page and sign-in redirect it existed to catch — and handed the page to the caller as log output. Tightened to text/plain on both legs; the test that revealed it was written to cover the guard, not to fix it. Finally, every fallback reported "Forgejo >= 16.0 adds ... upgrade", including the five sites reached only after the v16 jobs route already ANSWERED us. Splits errRESTPresent out of errNoAPIRoute (still errors.Is-compatible, control flow unchanged) so the diagnostic tells a v16 operator what is actually wrong instead of telling them to upgrade to the version they run. Mutation battery: 17/17 killed (was 16 survivors of 28). fetchJobLogs 53.8% -> 100%, diagnoseLogs404 0% -> 63.2% (it log.Fatal'd the test binary until the mock answered /api/v1/version, per repo convention). build/vet/test/-race -count=3 -shuffle=on all pass; gofmt clean on touched files.Verified against a disposable Forgejo 16.0.4 (digest sha256:a3e33d03e771d3e58b27de5573c3a25dc4f670583a6724c1878a6d0bbecf3556) with a PRIVATE repo and token auth, driving real Actions runs. Both defects made the API path a no-op, and neither was visible in the unit tests because the fixtures were written from the release notes rather than copied off a server. 1. The jobs listing is a BARE JSON ARRAY, not {"jobs": [...]}. ListActionRunJobs does ctx.JSON(200, []*api.ActionRunJob). Decoding the envelope yielded an UnmarshalTypeError, which classifyRouteErr reads as "route absent", so every fetch fell back to the web route and 404'd on private repos — the exact symptom this PR set out to fix. 2. Every /api/v1 actions route keys on the run's DATABASE id, while the number the caller reads in /actions/runs/<N> is the per-repo index (ListActionRunJobs -> GetRunByID; the web handler -> GetRunByIndex). Passing the index through reads a DIFFERENT run wherever the two spaces have diverged. On the test instance run index 3 is database id 9. Resolution now goes through the documented ?run_number= filter and re-checks index_in_repo on the row it gets back, so a server that ignores the filter falls back instead of answering with another run. Defect 1 masked defect 2: the envelope always fell back, so the wrong id never reached a route that would act on it. Fixing either alone is worse than fixing neither — the second is a silent wrong answer, not a failure. Also: attempts ARE addressable on 16.x. GET .../jobs/{job_id}/logs takes a documented ?attempt=N matching the listing's `attempt` field. The previous revision handed attempt>1 to the web route, which cannot serve a private repo at all, making "give me attempt 2" a guaranteed failure. Confirmed by re-running one job of a two-job run: attempts 1 and 2 return different task logs, and an attempt beyond the job's count 404s rather than silently serving the latest. Two fixes to the pre-16 fallback's diagnostics, which this change makes the only path older servers ever take: - diagnoseLogs404's "you passed a DATABASE id" branch now also requires run.RunNumber != runID. Where the two id spaces coincide — a first repo, or any run whose db id equals its own index — the branch fired and told the caller to "retry with N" when they had just passed N. Reproduced on a disposable 15.0.5, where run index 1 is also database id 1. - The advice no longer says "Forgejo v14" or "until the artifact REST API lands in v16". v16 has shipped and has the API; the message now names the real cause (pre-16 web route needs a session cookie, so a private repo 404s regardless) and the real fix (upgrade to >= 16.0). Test fixtures are now copied from the live server. The empty-jobs case becomes an authoritative "run has no jobs" rather than a fallback, since the shape is verified rather than guessed; TestV16FallbackDoesNotAdviseUpgrading ToV16 drives the still-present errRESTPresent path (jobs route answers, logs route absent) instead. Mutation-checked on the merged suite: restoring the envelope (11 kills), sending the index to the jobs route (6), dropping ?attempt= (1), trusting an unfiltered listing (1), jobs[jobIndex] -> jobs[0] (1), and emptying the returned body (6) each fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zetaP8acDgC57KdzbdAoa5bc038bf76to7aac0d94beTakeover report
Verdict: Forgejo 16 does solve authenticated Actions-log retrieval for private repos — but this PR, as it stood, did not deliver it. Two defects made the API path a no-op; both are now fixed and verified live. Full evidence is in the updated PR description.
Note on the handoff
The prompt pins exact head
4dc0c175. That was stale —6f515a0anda653b29had already landed on top, and871fb3f(round-2) landed during this work. I verified against the real head and rebased onto871fb3frather than clobbering it; its findings (the..traversal, thetext/html-as-logs guard,errRESTPresent) are all preserved. New head:7aac0d9.What was actually wrong
Neither defect was visible to the unit tests, because the tests asserted fixtures this PR invented rather than shapes a server sends.
{"jobs":[…]}. The envelope decode raised anUnmarshalTypeError, whichclassifyRouteErrreads as route absent → silent fallback to the web route → 404 on every private repo. The symptom the PR set out to fix./api/v1actions routes key on the run's DATABASE id, while the caller passes the index from/actions/runs/<N>(GetRunByIDvsGetRunByIndex). On the test instance run index 3 is db id 9. This one is a silent wrong answer — it can return another run's logs.Defect 1 masked defect 2. Fixing either alone would have been worse than fixing neither, which is the main reason I did not stop at the first finding.
Also corrected:
?attempt=Nis documented and works on 16.0.4 (proven with a real re-run — attempts 1 and 2 return different task logs), so the previous "hand attempt>1 to the web route" path was an unnecessary guaranteed failure on private repos.Evidence
Disposable 16.0.4 (
sha256:a3e33d0…) and 15.0.5 (sha256:eda2e37…) containers, private repos, real runner, real jobs. Run index and DB id were deliberately diverged by seeding a decoy repo first — on a single-repo test instance they coincide and both defects hide. All four required cases pass on 16.0.4; the 15.0.5 fallback is reached and its diagnostic no longer misreports a private-repo 404 as expiry. 1 MiB cap and spill-to-disk intact (DoRawAPInow has its own cap test). Six mutations, each killed.Two things to flag
.forgejo/workflows/release.ymltriggers only onv*tags, so nothing ran against the exact head —list_workflow_runsreturns none for the repo at all. The checks reported here are local. Worth adding a build/test workflow separately.gofmt: 23 files onmainare unformatted (pre-existing). I left them alone; the touched files are clean.Not done — not authorized here
No production deployment (prod remains 15.0.5, untouched), and no merge. The upgrade 15.0.5 → 16.0.4 is what actually makes logs readable; this change makes that upgrade sufficient on its own. Re-run
livecheck/against prod after the upgrade rather than trusting this evidence indefinitely.🤖 Generated with Claude Code
https://claude.ai/code/session_018zetaP8acDgC57KdzbdAoa
Round 4's mutation pass found 14 survivors, a fourth vacuous assertion, and a live harness that cannot fail. Also recovers coverage I lost in the rebase: when the branch was reconciled onto the live-verified v16 work I took their version of run_logs_api_test.go wholesale and never diffed my own round-3 additions against it, so the serverMessage assertion, both payload/auth branches and the web-leg attempt assertion were silently unpinned again. Diff the tests you drop, not just the code. attempt was not actually plumbed. The web fallback discarded it (hardcoding attempt/1 in the path left the suite green) and get_job_logs ignored its own attempt argument entirely — no test drove that tool past a validation rejection, which was the uncovered third of it. Both are now asserted on the wire, along with job_index, the attempt<1 clamp and a success path that returns the second job's body. TestUndecodableListingFallsBack never reached the code it names. Its fixture was `[{"id":42,"name":"build"}]`, commented "bare array, not the envelope" — written before the bare array was established as the REAL shape. It decodes perfectly; the test passed because the same body was then served as the job log with Content-Type: application/json and tripped the content-type guard, and errors.Is could not tell the legs apart because errRESTPresent wraps errNoAPIRoute. It now serves truncated JSON on the jobs route only and asserts the fallback did NOT come from a later leg. A fifth vacuous assertion, in a test written this round: TestThreeHundredIsNot Success served "choices", so with the status check relaxed DoJSON still failed on the decode and the assertion held either way. Valid JSON now makes the status check the thing under test. Also pinned: the other two classifyRouteErr decode arms (empty 200 body, HTML served as 200 — likelier in the wild than a type mismatch); that a JSON-shaped 404 WITHOUT Forgejo's typed message falls back rather than being treated as authoritative; that a content-type fallback still reports the server as v16 (only the sibling site was covered, so this one could tell a v16 operator to upgrade); both halves of resolveRunDBID's ignored-filter conjunction; the resolve query's filter and bound; the negative job_index default; the diagnostic surviving a failed task lookup; and doRawAt's/DoJSON's 300 boundary. livecheck asserted nothing — it printed and returned, so it passed against a server where every fetch failed. Verified by reproducing exactly that. Cases now carry an expected outcome (ok / err / err=<substring>), empty successful bodies fail, and the same stub run now fails while LIVE_URL-unset still skips. Observability: the four fallback decisions were log.Debugf, invisible at the default level, so taking a leg that cannot serve a private repo left no trace. Promoted to Warn. CI: runs-on is now k3s-prod-ci, the label this instance's pools advertise (codeberg-small came from upstream Codeberg and exists nowhere here). This does NOT make the workflow run — the k3s-prod-ci pool is org-scoped to owner_id=35 and this repo has a different owner, so a runner whose scope includes it is still required. The header says so rather than implying a gate. Also corrected the file count in that comment (20 repo-wide, not 8), added livecheck to the gofmt list, and made the shuffle run twice, since a single random order was thin justification for the comment's own rationale. Test helpers now restore the SDK singleton they replace, instead of leaving it bound to a closed httptest server — the exact mechanism behind vacuous assertion #2. Mutation battery: 16/18 killed. The two survivors are equivalent mutants, not gaps: the `tasks != nil` half of the diagnostic's guard is unreachable because the SDK returns a non-nil struct alongside its error, and the entry-point attempt clamp is redundant with the one in fetchJobLogs. DoMultipart's and DoRawToFile's 300 boundaries are also unpinned, but those are pre-existing helpers this branch does not touch. GetJobLogsFn 64.7% -> 94.1%; diagnoseLogs404 and resolveRunDBID 100%. build/vet/test/-race -count=3 -shuffle=on green across 13 packages; all 60 actions tests also pass run individually.Correction to the verdict: 16.x is off the table (not LTS)
The description's closing recommendation — "upgrade 15.0.5 → 16.0.4" — is withdrawn. Per policy the instance stays on the LTS line, and I confirmed against Forgejo's own docs and source:
Latest (v16.0.4)/LTS (v15.0.8)— v15 is the current LTS. v16 is "Stable/latest": full support for three months plus a two-week window after the next stable, i.e. already out of its support window now that nothing newer has superseded it on that track./runs/{id}/jobs, no/jobs/{id}/logs, and no artifacts API either. LTS receives "only critical bugfixes and security support", so these will not be backported.What this means for this PR: the API path is unreachable on the LTS line and will stay that way until the Q1-2027 LTS. On 15.0.x this change is a no-op for log retrieval — it resolves the run, gets a bare
404 page not foundfrom the jobs route, and falls back exactly as before (one extra HTTP request per call).It is still worth landing, for two reasons that apply today on 15.0.x:
get_run_logswould have started returning another run's logs silently. Landing it now means that never happens.The honest framing is: this is a correct, verified, dormant change that becomes live at the next LTS, plus a real diagnostics fix for the interim. Re-run
livecheck/against the instance after that upgrade rather than trusting this evidence a year from now.🤖 Generated with Claude Code
https://claude.ai/code/session_018zetaP8acDgC57KdzbdAoa
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.