From b6646910514d37ee2c0cbc6afe173bdaa4120d9e Mon Sep 17 00:00:00 2001
From: "claude-ceo-assistant (Claude Opus 4.7 on Hongming's MacBook)"
 <claude-ceo-assistant@agents.moleculesai.app>
Date: Thu, 7 May 2026 16:01:11 -0700
Subject: [PATCH] fix(eic-tunnel-pool): capture poolJanitorInterval at pool
 construction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes the chronic -race flake on TestPooledWithEICTunnel_PanicPoisonsEntry
and the handlers package as a whole (CI / Platform (Go) was intermittent
on staging, ~50% red on workspace-server-touching commits since 2026-04).

The race: tests swap the package-level poolJanitorInterval via t.Cleanup
(eic_tunnel_pool_test.go:61) AFTER an earlier test caused the global pool's
janitor goroutine to start. The janitor loops on time.NewTicker(poolJanitorInterval)
on every tick — so the cleanup write races the goroutine read for the rest
of the process. Caught locally + on PR #84's CI run on Gitea.

Fix: capture the interval as a field on eicTunnelPool at newEICTunnelPool().
The janitor now reads p.janitorInterval, which never changes after construction.
Tests that override poolJanitorInterval before freshPool() still get the new
value (they set the package var before construction). The global pool's
janitor — created lazily once via sync.Once on first getEICTunnelPool() —
is now immune to t.Cleanup-driven swaps from later tests.

Surfaced while verifying #84 (SaaS plugin install via EIC SSH); folded
into this PR per the "fix root not symptom" rule rather than merging
around a chronic-red CI signal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../internal/handlers/eic_tunnel_pool.go      | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/workspace-server/internal/handlers/eic_tunnel_pool.go b/workspace-server/internal/handlers/eic_tunnel_pool.go
index 03bfd01f..20b2e269 100644
--- a/workspace-server/internal/handlers/eic_tunnel_pool.go
+++ b/workspace-server/internal/handlers/eic_tunnel_pool.go
@@ -108,6 +108,18 @@ type eicTunnelPool struct {
 	// First acquirer takes the slot; later ones wait on the channel.
 	pendingSetups map[string]chan struct{}
 	stopJanitor   chan struct{}
+	// janitorInterval is captured at pool construction from the
+	// package-level poolJanitorInterval var. Captured (not re-read on
+	// every tick) so a test that swaps the package var via t.Cleanup
+	// after a global pool's janitor is already running can't race
+	// with that goroutine's ticker read. The global pool is created
+	// lazily once per process via sync.Once; before this capture
+	// landed, every test that touched poolJanitorInterval after the
+	// global pool's first-touch raced the janitor (caught by -race
+	// on staging tip 249dbc6a — TestPooledWithEICTunnel_PanicPoisonsEntry).
+	// Tests still get the new value on a freshPool() because they
+	// set the package var BEFORE calling newEICTunnelPool().
+	janitorInterval time.Duration
 }
 
 var (
@@ -127,11 +139,16 @@ func getEICTunnelPool() *eicTunnelPool {
 
 // newEICTunnelPool constructs an empty pool. Exported so tests can
 // build isolated pools without sharing the singleton.
+//
+// Captures poolJanitorInterval at construction time so the janitor
+// goroutine doesn't race with t.Cleanup-driven swaps of the package
+// var. See the janitorInterval field comment for the failure mode.
 func newEICTunnelPool() *eicTunnelPool {
 	return &eicTunnelPool{
-		entries:       map[string]*pooledTunnel{},
-		pendingSetups: map[string]chan struct{}{},
-		stopJanitor:   make(chan struct{}),
+		entries:         map[string]*pooledTunnel{},
+		pendingSetups:   map[string]chan struct{}{},
+		stopJanitor:     make(chan struct{}),
+		janitorInterval: poolJanitorInterval,
 	}
 }
 
@@ -290,8 +307,11 @@ func (p *eicTunnelPool) evictLRUIfFullLocked(skipInstance string) {
 // janitor periodically scans for entries that are idle AND expired,
 // closing their tunnels. Runs forever (per pool lifetime); cancelled
 // by close(p.stopJanitor) for tests that build short-lived pools.
+//
+// Reads p.janitorInterval (captured at construction) instead of the
+// package-level poolJanitorInterval — see janitorInterval field comment.
 func (p *eicTunnelPool) janitor() {
-	t := time.NewTicker(poolJanitorInterval)
+	t := time.NewTicker(p.janitorInterval)
 	defer t.Stop()
 	for {
 		select {