fix(ci view): harden bridge/child-pipeline navigation against crashes
What
Fixes a panic in glab ci view when entering a child pipeline from a bridge/trigger job:
runtime error: index out of range [6] with length 4
... internal/commands/ci/view/view.go:591 +0x5c5Also fixes a related latent nil-pointer crash in curPipeline that surfaced during the audit.
Why the primary crash (#8313 (closed))
When the user pressed Enter on a bridge job whose downstream pipeline existed, handleBridgeJobSelection pushed the child pipeline onto the stack but left the navigator carrying the parent's idx/depth. If the child pipeline had fewer jobs than the cursor's position in the parent, the next key event panicked in Navigate at jobs[n.idx].Stage.
The earlier downstream-pipeline crash fix (!7372) covered the no DownstreamPipeline branch (nil deref + informational modal). The bug here lives in the success branch that pushes the pipeline and re-renders, which the existing tests never exercised end-to-end.
How
Navigator reset (#8313 (closed) primary fix):
- Reset the navigator (
*navi = navigator{}) on both pipeline transitions: push (inhandleBridgeJobSelection) and pop (in the Escape handler ofinputCapture). inputCapturenow takesnavi *navigatorinstead of receiving it by value, so the reset is visible across both call sites.- Defensive clamp at the top of
Navigate: ifn.idx >= len(jobs), snap back to0. Safety net so any future code path that forgets to reset cannot reintroduce the crash.
curPipeline hardening (audit finding):
curPipelinepreviously didreturn *commit.LastPipelinewhenever the pipeline stack was empty. IfLastPipelinewas nil (for example, when the user passed--pipelineidfor a pipeline whose commit record does not populateLastPipeline), this panicked the TUI.- Now returns an error in that case, and
updateJobsstops the app with a readable message instead of crashing.
Why we didn't catch the primary crash earlier
Two test gaps:
Test_handleNavigationdrives the navigator against a single fixed jobs slice — it never simulates the navigator's state surviving a switch to a different jobs slice.Test_bridgeWithoutDownstreamPipelineincludes a "bridge with downstream pipeline" case in the table, but only asserts data mapping (DownstreamPipeline != nil). It never invokeshandleBridgeJobSelectionor simulates the resulting re-render.
The new Test_navigatorSurvivesPipelineSwitch covers exactly the missing seam: parks the cursor at idx 6 of a 7-job pipeline, then applies Navigate to a 2-job slice and asserts no panic. Test_curPipeline_nilLastPipeline covers the second fix.
Verification
- Confirmed
Test_navigatorSurvivesPipelineSwitchpanics atview.go:591(the exact line in the bug report) when the navigator fix is reverted, and passes with it in place. - Full
internal/commands/ci/view/...suite passes.
Closes #8313 (closed)