fix(cli): address review feedback · Entire

fix(cli): address review feedback

06ec7a3→main·

suhaanthayyil·4d ago·2 files·+60 added/-3 removed

Make the detached __refresh_trail_enablement subprocess best-effort like __send_analytics: a transient network/API failure while refreshing the trails-enablement cache is logged but no longer propagated as a non-zero process exit, since stdout/stderr are discarded and nothing observes the exit code. Also harden the unresponsive-host timeout test to assert a connection was actually attempted, so it can't silently pass via an early return that skips the dial.

Changes

2

2 unmodified lines

3
4
5
6
7
8
9
10
5 unmodified lines

16
17
18
19
20
21
22
2302 unmodified lines

2325
2326
2327
2328
2329
2330
2331
2332
2333
2334
2335
2336
2337
2338
11 unmodified lines

2350
2351
2352
2353
2354
2355
2356
2357
2358
2359
2360
2361
2362
2363
2364
2365
2366
2367
2368
2369
2370
2371
2372
2373
2374
2375
2376
2377
2378
2379
2380
2381
2382
2383
2384
2385
2386
2387
2388
2389
2390
2391

2 unmodified lines

import (
    "context"
    "net"
    "net/http"
    "net/http/httptest"
    "os"
    "os/exec"
    "path/filepath"
5 unmodified lines

"github.com/entireio/cli/cmd/entire/cli/agent"
    "github.com/entireio/cli/cmd/entire/cli/agent/opencode"
    "github.com/entireio/cli/cmd/entire/cli/agent/types"
    "github.com/entireio/cli/cmd/entire/cli/api"
    "github.com/entireio/cli/cmd/entire/cli/investigate"
    "github.com/entireio/cli/cmd/entire/cli/paths"
    "github.com/entireio/cli/cmd/entire/cli/review"
2302 unmodified lines

ln, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:0")
    require.NoError(t, err)
    defer ln.Close()
    var accepted int32
    go func() {
        for {
            conn, acceptErr := ln.Accept()
            if acceptErr != nil {
                return
            }
            atomic.AddInt32(&accepted, 1)
            // Accept the connection but never write anything back (no TLS
            // handshake, no HTTP response) — simulates a blackholed/firewalled
            // host, which is what triggered the original 1s stall per call.
11 unmodified lines

if elapsed > trailEnablementRefreshTimeout+2*time.Second {
        t.Fatalf("runTrailEnablementRefresh took %v, expected to give up within roughly %v", elapsed, trailEnablementRefreshTimeout)
    }
    // Prove the test actually exercised the network path rather than passing
    // via an early return (e.g. scope resolution or auth failing before any
    // dial): the blackholed listener must have accepted at least one
    // connection attempt.
    if got := atomic.LoadInt32(&accepted); got == 0 {
        t.Fatalf("expected at least one dial attempt against the unresponsive host, got %d", got)
    }
}

// TestNewRefreshTrailEnablementCmd_APIFailureExitsZero guards against the
// detached __refresh_trail_enablement subprocess exiting non-zero on a
// transient network/API failure. The refresh is best-effort cache warming
// with stdout/stderr discarded (see newRefreshTrailEnablementCmd) — there is
// no one watching the exit code, so a failing TrailsEnabled call must be
// logged (already covered by TestRefreshTrailEnablementCmd_LogsBackgroundFailureToFile-
// style tests) and swallowed, never propagated as a command error, mirroring
// __send_analytics.
func TestNewRefreshTrailEnablementCmd_APIFailureExitsZero(t *testing.T) {
    setupStopTestRepo(t)
    runGitInDir(t, ".", "remote", "add", "origin", "https://github.com/entirehq/example.git")

srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
        w.WriteHeader(http.StatusInternalServerError)
    }))
    t.Cleanup(srv.Close)

prevClient := trailRefreshAPIClient
    trailRefreshAPIClient = func(context.Context, bool) (*api.Client, error) {
        return api.NewClientWithBaseURL("test-token", srv.URL), nil
    }
    t.Cleanup(func() { trailRefreshAPIClient = prevClient })

cmd := newRefreshTrailEnablementCmd()
    cmd.SetArgs([]string{})
    require.NoError(t, cmd.ExecuteContext(context.Background()),
        "detached refresh command must exit 0 even when the API call fails (best-effort cache warming)")
}

// TestRefreshTrailEnablementCmd_LogsBackgroundFailureToFile guards the #450