From 8991a5966e4abdb8cd3129fbc5e8d7c596876122 Mon Sep 17 00:00:00 2001
From: Danny Kopping <danny@coder.com>
Date: Mon, 22 Dec 2025 14:18:01 +0200
Subject: [PATCH] fix: wait for initial update before marking API as ready
 (#21363)

_Disclaimer: investigation done by Claude Opus 4.5_

Closes https://github.com/coder/internal/issues/1173
Closes https://github.com/coder/internal/issues/1174

The agent containers API is only marked "ready" under this condition in
`agent/agentcontainers/api.go`:

```go
	// For now, all endpoints require the initial update to be done.
	// If we want to allow some endpoints to be available before
	// the initial update, we can enable this per-route.
```

However, what was actually being checked for was that the _init_ was
done, not the _initial update_.

In agent/agentcontainers/api.go, the `Start()` method:
  1. Called `Init()` which closed `initDone` <--- API marked ready here
  2. Then launched `go api.updaterLoop()` asynchronously
3. `updaterLoop()` performs the initial container update <--- should
have marked it ready after this

This PR fixes these semantics to avoid the race which was causing the
above two flakes.

Signed-off-by: Danny Kopping <danny@coder.com>
---
 agent/agentcontainers/api.go | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go
index 2c6c985ef4..4c00e28892 100644
--- a/agent/agentcontainers/api.go
+++ b/agent/agentcontainers/api.go
@@ -87,7 +87,8 @@ type API struct {
 	agentDirectory string
 
 	mu                       sync.RWMutex  // Protects the following fields.
-	initDone                 chan struct{} // Closed by Init.
+	initDone                 bool          // Whether Init has been called.
+	initialUpdateDone        chan struct{} // Closed after first updateContainers call in updaterLoop.
 	updateChans              []chan struct{}
 	closed                   bool
 	containers               codersdk.WorkspaceAgentListContainersResponse  // Output from the last list operation.
@@ -325,7 +326,7 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
 	api := &API{
 		ctx:                         ctx,
 		cancel:                      cancel,
-		initDone:                    make(chan struct{}),
+		initialUpdateDone:           make(chan struct{}),
 		updateTrigger:               make(chan chan error),
 		updateInterval:              defaultUpdateInterval,
 		logger:                      logger,
@@ -379,20 +380,15 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
 	return api
 }
 
-// Init applies a final set of options to the API and then
-// closes initDone. This method can only be called once.
+// Init applies a final set of options to the API and marks
+// initialization as done. This method can only be called once.
 func (api *API) Init(opts ...Option) {
 	api.mu.Lock()
 	defer api.mu.Unlock()
-	if api.closed {
+	if api.closed || api.initDone {
 		return
 	}
-	select {
-	case <-api.initDone:
-		return
-	default:
-	}
-	defer close(api.initDone)
+	api.initDone = true
 
 	for _, opt := range opts {
 		opt(api)
@@ -651,6 +647,7 @@ func (api *API) updaterLoop() {
 	} else {
 		api.logger.Debug(api.ctx, "initial containers update complete")
 	}
+	close(api.initialUpdateDone)
 
 	// We utilize a TickerFunc here instead of a regular Ticker so that
 	// we can guarantee execution of the updateContainers method after
@@ -715,7 +712,7 @@ func (api *API) UpdateSubAgentClient(client SubAgentClient) {
 func (api *API) Routes() http.Handler {
 	r := chi.NewRouter()
 
-	ensureInitDoneMW := func(next http.Handler) http.Handler {
+	ensureInitialUpdateDoneMW := func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
 			select {
 			case <-api.ctx.Done():
@@ -726,8 +723,8 @@ func (api *API) Routes() http.Handler {
 				return
 			case <-r.Context().Done():
 				return
-			case <-api.initDone:
-				// API init is done, we can start processing requests.
+			case <-api.initialUpdateDone:
+				// Initial update is done, we can start processing requests.
 			}
 			next.ServeHTTP(rw, r)
 		})
@@ -736,7 +733,7 @@ func (api *API) Routes() http.Handler {
 	// For now, all endpoints require the initial update to be done.
 	// If we want to allow some endpoints to be available before
 	// the initial update, we can enable this per-route.
-	r.Use(ensureInitDoneMW)
+	r.Use(ensureInitialUpdateDoneMW)
 
 	r.Get("/", api.handleList)
 	r.Get("/watch", api.watchContainers)