Cancelled jobs exit 255 with no signal instead of 128 + signal

[goingforstudying-ctrl]

Description

Cancelled jobs sometimes come back with exit_status 255 and no signal, and I've also seen plain -1, where the docs say it should be 128 + SIGNAL (so 143 for a SIGTERM cancel). I dug into it and there are two cooperating problems:

1. The -1 sentinel escapes into exit-code space. When the command is killed by a signal, Go's ExitError.ExitCode() is -1. That gets stored in BUILDKITE_COMMAND_EXIT_STATUS, returned from Executor.Run, and passed to os.Exit — which truncates to 8 bits, so the bootstrap exits 255 and all signal information is gone. The same -1 shows up verbatim in the finish payload when the bootstrap itself dies by signal, where it's indistinguishable from the "command never ran" sentinel.

2. A race between the bootstrap's signal goroutine and the executor returning. The agent's Interrupt() signals the whole process group, so the command can die from the same SIGTERM that the bootstrap's signal goroutine is still processing. The goroutine only set cancelled = true after bootstrap.Cancel() returned, so the main goroutine could observe the signalled command exit with cancelled == false and skip the re-signal-self path — straight to the 255 exit above.

The fix:

• shell.ExitCode now translates a signal death into the shell convention 128 + N (this is already the convention cache_exec documents internally). The three places that used ExitCode(err) == -1 as a "killed by signal" check now say IsExitSignaled(err) instead, which is what they meant anyway — and IsExitSignaled now also recognises the shell.ExitError{Code: -1} form, since a wrapped exit error with code -1 can only mean death by signal.
• The bootstrap signal goroutine records cancelled/received under the mutex before calling Cancel, and the main goroutine drains the (buffered) signal channel after Run returns, so a queued-but-unprocessed signal still takes the re-signal path.
• When the bootstrap process itself was signalled, the agent reports 128 + N as the exit status alongside the signal name instead of -1.

I can reproduce both symptoms deterministically on main: a command of kill -TERM $$ makes the bootstrap exit 255, and a signalled bootstrap reports exit_status: "-1". With this patch both come out as 143 with the signal intact.

Context

Fixes #1509

Changes

• internal/shell: ExitCode maps signal deaths to 128 + N; IsExitSignaled recognises ExitError{Code: -1}
• internal/job: checkout.go / git_errors.go sentinel checks use IsExitSignaled
• clicommand/bootstrap.go: record cancellation before Cancel, drain pending signals after Run
• agent/run_job.go: report a signalled bootstrap as 128 + N instead of -1
• Tests: unit coverage for ExitCode on a signalled process; end-to-end coverage that a self-signalling command exits the bootstrap 143, that a cancelled job reports 143 to hooks and dies by the received signal, and that a signalled bootstrap is reported to the API as 143 + SIGTERM

Public documentation

Status: not needed

Testing

• Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
• Code is formatted (with go tool gofumpt -extra -w .)

Ran the new tests against main first to confirm they capture the reported behaviour (-1 env var, bootstrap exit 255, API exit_status: "-1"), then against the patch. Also ran the cancel path under -race. Full go test on internal/shell, internal/job, internal/job/integration, clicommand, agent, and agent/integration passes; one pre-existing flake (TestCheckoutErrorCaptureDeliveryDoesNotCauseTimeout, a 1s checkout timeout vs slow git clean fork) passed on rerun and is unrelated.