cli: address Bugbot/Copilot review on cell routing fallback

32e8a17·

Soph·1w ago·3 files·+24 added/-9 removed

• Bound currentRepoID's control-plane lookup with a 5s timeout (currentRepoIDTimeout, mirroring expertsCellResolveTimeout): the lookup is best-effort decoration, so a stalled core must not hang recap.
• Case-fold the cluster catalog row's jurisdiction in resolveCellAPIBaseURL: the claim side is already folded, so a differently-cased row misreported ErrNoCellForJurisdiction.
• Rewrite newRecapClient's fallback contract comment to match the implementation: ANY cell-client failure falls back to the data API (expected cases silent, others debug-logged), not just the two named errors.
• Rename runRecap's repoSlug local to repoScope: once cell-routed it holds a repo_id ULID, only the data-API fallback passes a slug.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

76ad7ee5e8b2View transcript

Changes

3

• cmd/entire/cli
  • auth
    • Mcell_data_api.go+4/-1
  • Mentireapi_client.go+9
  • Mrecap.go+11/-8

546 unmodified lines

var matches []clusterListingRow
sawJurisdiction := false
for _, row := range listing.Clusters {
    if row.Jurisdiction != jurisdiction {
        // jurisdiction is already a folded lowercase label (resolveJurisdiction);
        // fold the catalog row too so a differently-cased row still matches
        // instead of misreporting "no cell for jurisdiction".
        if !strings.EqualFold(strings.TrimSpace(row.Jurisdiction), jurisdiction) {
            continue
        }
        sawJurisdiction = true
    }
}

Mcmd/entire/cli/auth/cell_data_api.go+4/-1

4 unmodified lines

// currentRepoIDTimeout bounds currentRepoID's control-plane lookup. The lookup
// is best-effort decoration (recap degrades to personal-only without it), so a
// stalled core must not hang the command — mirror expertsCellResolveTimeout.
const currentRepoIDTimeout = 5 * time.Second

// runAuthenticatedActivityAPI runs fn with an authenticated client for the
// activity/recap surface. It prefers the caller's home entire-api cell (the same
// shared client the experts commands use), which serves the /me/* endpoints

func currentRepoID(ctx context.Context) string {
    ctx, cancel := context.WithTimeout(ctx, currentRepoIDTimeout)
    defer cancel()

// implementation details omitted
}

Mcmd/entire/cli/entireapi_client.go+9

122 unmodified lines

if err != nil {
    return err
}
client, repoSlug, err := newRecapClient(ctx, f.insecureHTTP)
client, repoScope, err := newRecapClient(ctx, f.insecureHTTP)
if err != nil {
    if errors.Is(err, api.ErrInsecureHTTP) {
        fmt.Fprintf(errW, "ENTIRE_API_BASE_URL is set to an insecure http:// URL (%s). Use https:// for production, or pass --insecure-http-auth for local dev.\n", api.BaseURL())
    }
}

Mcmd/entire/cli/recap.go+11/-8

// the data API (which addresses them by name). Empty when the current repo // can't be resolved — recap then shows the personal side only.

// It prefers the caller's home entire-api cell (the shared client), falling back // to the data API when the region has no cell yet (ErrNoCellForJurisdiction) or // the caller isn't logged in — recap tolerates the latter, rendering and letting // the server answer 401 rather than hard-failing. Every other failure surfaces. // It prefers the caller's home entire-api cell (the shared client) and falls // back to the data API on ANY cell-client failure — the cell path is a // best-effort upgrade, so a cell problem must never break a command that worked // before it existed. Expected fallbacks (region has no cell yet, not logged in) // are silent; unexpected ones are debug-logged (logCellClientFallback). Only // failures of the data-API path itself surface — except ErrNotLoggedIn, which // recap tolerates, rendering and letting the server answer 401. func newRecapClient(ctx context.Context, insecureHTTP bool) (*api.Client, string, error) { // Best-effort upgrade: on any cell failure fall back to the data API path // below, which tolerates a missing login (renders, lets the server 401) so }