fix: Exit with non-zero status if ping or heartbeat fail unrecoverably

[DrJosh9000]

Description

Context: A recent incident at Buildkite caused an issue to occur... wait, where have we heard this before?

It turns out the agent was exiting 0 in cases where the ping or heartbeat are unrecoverable, so this PR changes them to exit 1.

Context

https://linear.app/buildkite/issue/PS-1593/investigate-using-non-zero-exit-code-when-ping-receives-http-401

Changes

• Return the unrecoverable error from runPingLoop.
• Return an error from runHeartbeatLoop. Collect both errors and return either nil or one of the non-nil loop errors from agentWorker.Start. (That error bubbles up to main, which should exit 1.)
• Remove heartbeatCtx, and instead have the heartbeat loop respond to <-a.stop, so that "ordinary" stopping is not an error.
• Move calls to StopUngracefully out of PingandHeartbeat, instead making them the responsibility of runPingLoopandrunHeartbeatLoop`. (Could go either way on this.)
• Replace errors.Is(err, &errUnrecoverable{}) with a slightly nicer looking isUnrecoverable(err), and remove the Is method (note that the method removal could also be achieved with errors.As(err, new(*errUnrecoverable))).
• Add a new test checking that an unrecoverable error bubbles up from agentWorker.Start, and necessary changes to the fake API server to avoid a deadlock

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 .)

Disclosures / Credits

I did not use AI tools at all

[moskyb]

LGTM!

[zhming0]

Hmm, the consensus mechanism between ping loop and heart beat loop isn't quite right I think.

Not a problem.