fix(ci): show the correct job's log in child pipelines, and the requested pipeline's jobs

Kai Armstrongrequested to merge
phikai/fix-child-pipeline-job-logs into main

Fixes #8495 (closed) and closes the second half of #7997 (closed).

Problem

Two bugs, both from the same root cause: glab ci view had more than one answer to "which pipeline am I looking at".

#8495 (closed) — wrong job's log in a child pipeline. Pressing Enter on a job inside a child pipeline showed the log of an unrelated job from the parent. The Enter and Ctrl+Space paths resolved a job by name via RunTraceSha(… projectID, commitSHA, curJob.Name), where projectID and commitSHA were captured once at startup and never updated on descent. Job names are only unique within one pipeline, so the lookup fell through PipelineJobWithSha's last-running → first-pending → last-job fallback chain and silently streamed a different job. The job grid rendered correctly the whole time because updateJobs read the live pipeline stack — the correct pipeline was right there, just not used to fetch logs.

#7997 (closed) — wrong pipeline's jobs. glab ci view -p 100 titled the window "Pipeline #100" while rendering jobs from whatever pipeline was newest on that commit; -b <branch> had the same split. The requested pipeline reached only the title and the --web URL. 0dabef9f stopped using commit.LastPipeline to resolve the pipeline but left curPipeline using it to decide which pipeline's jobs to render, so the original symptom survived in the grid after the title was fixed.

Fix

One source of truth: the navigation stack. run() seeds it with the pipeline that was actually requested, descending into a child pushes onto it, and every downstream request — job list, logs, cancel, retry — reads the top of it.

  • Resolve jobs by ID against the current pipeline. RunTraceSha is replaced by RunTraceJob(ctx, client, w, pid, jobID, pollInterval). No name-based resolution remains, so there is no fallback chain left to pick the wrong job.
  • Key the cached log page by job ID. New logsPageKey helper applied at all six sites. Name keying also collided for retried jobs, which share a name with the original attempt but have a new ID — they were showing the failed attempt's cached trace.
  • Seed the stack in run(). curPipeline no longer needs the commit.LastPipeline fallback, which lets commit drop out of updateJobs, jobsView, and inputCapture entirely — it was only ever threaded through to reach that fallback.
  • Esc / q ascends only while more than one pipeline is stacked, since the requested pipeline now permanently occupies the bottom.

Also fixed, same root cause

  • Cancel and retry used the startup project too, so they addressed the wrong project for multi-project triggers. PlayOrRetryJobs took repo string; widened to pid any to match the package convention.
  • A goroutine race: the trace goroutine read the curJob global after being scheduled, so navigating during a fetch could stream a different job. Both trace sites now snapshot the job and project first.

Removed

PipelineJobWithSha and pipelineJobsWithSha, whose only caller was the deleted RunTraceSha. Their pipeline lookup was independently unsound: it listed project pipelines by SHA and took pipelines[0], and a child pipeline shares its parent's SHA.

Seeding the stack also removed both GetCommit calls and a redundant second GetPipeline — three fewer API requests at startup, and run() is ~25 lines shorter.

Testing

make test (4347 tests), make lint (0 issues), go test -race on the touched packages, and the full lefthook run pre-push all pass.

New coverage:

  • TestRunTraceJob — traces by ID against both string- and numerically-identified projects (the cross-project child case), plus its error path.
  • Test_logsPageKey — same-name-different-pipeline, stable key, and retried-job cases.
  • Test_curPipeline_tracksDescentIntoChildPipeline — descent into a child pipeline in another project and the return trip.
  • Test_pipelineInfo — the seed conversion carries the pipeline and project IDs.
  • Test_curPipeline_emptyStack replaces Test_curPipeline_nilLastPipeline, which covered the fallback this MR deliberately removes.

The two existing TestCIView --web cases had mocked GetCommit; gomock failed them when the call disappeared, which was a useful confirmation it was really gone. The mocks are removed and both cases still assert the same output. No test cases were repurposed or dropped.

Please verify by hand before merging

Neither behaviour is reachable from a unit test — run() returns early without a TTY, and the existing tests only exercise --web. Two things worth a manual pass:

  1. On a pipeline with a child pipeline: descend into it, open a job's log, and confirm it is that job's own trace. Then Esc back and open a parent job with the same name.
  2. glab ci view -p <an older pipeline ID on a SHA that has a newer one> — the grid should match the title. And Esc at the top level should still quit.

Note on ownership

@bobby060 reported #8495 (closed) with an accurate root-cause analysis and said they had a fix in progress. I built this before checking in with them — happy to close this in favour of theirs, or to have them review. The diagnosis in the issue is theirs; this MR extends it to the cancel/retry paths, the retried-job cache collision, and #7997 (closed).

Merge request reports