Concurrently register spawn

[DrJosh9000]

Description

The workers are already started concurrently, but they weren't registered concurrently - until now!

Context

It annoyed me.

Changes

• Add a concurrently package and a generic Map function. I'm sure we can reuse this. I'm also sure there are plenty of packages out there that could do this for us but wanted to write it out myself.
• Use concurrently.Map to perform concurrent register calls.

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 any AI tools to write this code.

[buildsworth-bk-app]

On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/871

[buildsworth-bk-app]

I found a few issues that look load-bearing for making registration concurrent: the per-spawn request data is not preserved, and the shared registration client has mutable state that this path now exercises concurrently.

Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 871, then answer my questions about the findings.

[DrJosh9000]

@buildsworth-bk review

[buildsworth-bk-app]

On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/874

Previous concerns have been addressed.

[buildsworth-bk-app]

The previous blocking concerns look addressed: spawn interpolation is back to the 1-based index, the concurrent callback now uses its per-spawn request, and registration response headers are applied to each per-agent client instead of mutating the shared registration client. I left one non-blocking edge case on the new worker-count option; since this changes registration-path concurrency, I’m leaving this as a comment review rather than an AI-only approval.

Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 874, then answer my questions about the findings.

[DrJosh9000]

@buildsworth-bk review

[buildsworth-bk-app]

On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/879

[buildsworth-bk-app]

The worker-count edge case from the previous pass looks addressed. I found one small CLI-input edge case in the registration setup; because this changes the agent registration path, I’m leaving this as a comment review rather than an AI-only approval.

Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 879, then answer my questions about the findings.

[DrJosh9000]

@buildsworth-bk review

[buildsworth-bk-app]

On it! Follow along with my review here: https://buildkite.com/buildkite/buildsworth-review-pull-request/builds/881

[buildsworth-bk-app]

The previous non-blocking notes around WithWorkerCount(0) and negative --spawn look addressed in the current diff. I didn't find any new inline findings; because this changes the agent registration path and API-client header transfer behavior, I'm leaving this as a comment review rather than an AI-only approval.

Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 881, then answer my questions about the findings.

[zhming0]

LGTM 🚀

[zhming0]

Technically those goroutines aren't overlapping, we might not need outMu

[DrJosh9000]

If the element size is not a multiple of the CPU word size, then there may be a data race, because in order to write an element the goroutine has to write into a neighbouring element at the same time.