Centralize API Authentication Handling · Entire
Command: Entire Trail List
I'm running the nightly version of this app and I ran:
~/code/devenv/entire.io (main) % entire trail list
No trails found.
Commands:
entire trail create: Create a trail for the current branchentire trail list: List recent trailsentire trail update: Update trail metadata
I knew that was false so I assumed it was because I wasn't logged in. So I did entire login and then tried the above command again and now entire trail list actually listed trails. So tell me if this is a bug, an omission or something else. Read the code related to it. Do you notice the same pattern in other areas that require auth?
1mo ago · 3.8s · 12 calls
Yes — based on the current code, the behavior you saw is a bug / UX omission, not expected behavior.
What should happen now:
entire trail listcallsrunTrailListAll- That calls
NewAuthenticatedAPIClient - If there’s no valid login, it should fail before hitting the trail list API with something like:
authentication required: not logged in (run 'entire login' first): ...
Relevant Code:
cmd/entire/cli/trail_cmd.gorunTrailListAll(...)- Creates client via
NewAuthenticatedAPIClient(...)
cmd/entire/cli/api_client.goNewAuthenticatedAPIClient(...)- Maps missing auth to
not logged in (run 'entire login' first)
So if nightly printed:
No trails found.
while logged out, then that version likely allowed an unauthenticated API request and treated the empty/anonymous response as a real empty trail list. That’s misleading.
I’d classify it as:
Bug: auth-required data was rendered as an empty successful result instead of prompting login.
Notable Patterns in Other Commands:
activityhandles auth explicitly and prints:
Not logged in. Run 'entire login' to authenticate.
searchresolves auth up front and returns:
not authenticated. Run 'entire login' to authenticate
recapis intentionally different: it may call the API without a token so the server returns 401, then renders a login-oriented message. That’s documented in code.- Shell completion intentionally swallows auth errors, which is fine because completions shouldn’t spam the terminal.
So trail list should align with activity/search, not silently show an empty state. Current repo code appears to have already moved in that direction; nightly may be behind or from a code path/version before this was fixed.
Suggestions for Deduplication:
Is there a way we can dedupe this so that all commands that require you to be authed do require it the same way so that something like this doesn't slip between the cracks again?
1mo ago · 3.7s · 1 call
Yes. I’d dedupe this with one shared “authenticated command runner” for data-API commands.
Currently, there are two patterns:
- Core/control-plane commands already deduped:
runCore(...)runCoreJSON(...)- Used by
org,repo,project,grant - Centralizes auth/client/error rendering.
- Data API commands are ad hoc:
trailactivitysearchdispatchsetuprecapis intentionally special
That second group is where bugs like this can slip in.
Proposed Code Example:
func runAuthenticatedDataAPI(
cmd *cobra.Command,
insecureHTTP bool,
fn func(ctx context.Context, client *api.Client) error,
) error {
cmd.SilenceUsage = true
client, err := NewAuthenticatedAPIClient(cmd.Context(), insecureHTTP)
if err != nil {
return renderAuthRequiredError(cmd.ErrOrStderr(), err)
}
return fn(cmd.Context(), client)
}
Key Refactor:
Make lower-level functions accept *api.Client instead of constructing auth internally, e.g. runTrailListAll(ctx context.Context, w io.Writer, client *api.Client, opts trailListOptions) error.
Commands that should use this shared path:
trail list/show/create/update/review/watchactivitydispatchcloud paths- maybe
setupAPI calls search, though it currently has custom search-service token resolution
Commands that should not use it:
loginlogoutauth statusrecap, because code explicitly says it may call without a token to let the server return 401 and render a recap-specific message- Shell completions, because they intentionally swallow auth errors
So: yes, we should centralize auth for data-API commands just like runCore already does for core commands. That would make “auth required” a wrapper-level property instead of something each command remembers manually.
Ok implement that.
1mo ago · 11.6s · 35 calls
Implemented. Changed:
- Added shared auth gate:
cmd/entire/cli/authenticated_data_api.gorunAuthenticatedDataAPI(...)- Shared handling for:
- not logged in →
Not logged in. Run 'entire login' to authenticate. - Ctrl-C/context cancellation → silent error
- Other auth/token/network errors → surfaced as-is
- not logged in →
- Updated callers:
activitytrail- Show, list, create API phase, update, review fallback list, review target auth, watch
- Added regression test:
TestRunTrailListAll_PrintsLoginHintWhenNotLoggedIn- Verifies logged-out
trail listdoes not printNo trails found.
Verified:
go test ./cmd/entire/cli
Passed.
Are the changes in line with the original pattern that already existed?
1mo ago · 5.7s · 1 call
Mostly yes, but with one caveat.
It matches the existing core command pattern conceptually:
- Existing:
corecmd.gorunCore(...)- Builds authenticated core client
- Centralizes auth-ish setup/error handling
- New:
authenticated_data_api.go- Builds authenticated data API client
- Centralizes data API auth error handling
So architecturally it’s aligned.
Caveat:
The file name/style is a little more descriptive than the existing naming. If we want it to feel more native to the repo, I’d rename it to something like:
cmd/entire/cli/dataapi_cmd.go- or
cmd/entire/cli/authcmd.go
But function-wise, yes: it follows the existing “shared command wrapper” pattern already used by runCore / runCoreJSON.