cli: simplify `entire api` (dedup git + mirror filter, drop stdin seam) · Entire
cli: simplify entire api (dedup git + mirror filter, drop stdin seam)
8fbdae4→main·
Soph·2w ago·3 files·+52 added/-55 removed
Cleanups from a simplify pass:
- expandAPIPlaceholders resolves the origin remote once and threads forge/owner/repo into resolveCurrentRepoID, instead of both resolving it — a path with {owner}/{repo} and {repo_id} no longer does two git calls.
- Extract isActiveMirror(coreapi.Mirror): the archived + failed/suspended placement filter now has one home, shared by resolveCurrentRepoID and distinctActiveClusterHosts.
- Drop the unused stdinReader test seam (and the ctx it dragged through readAPIInput/buildAPIRequestBody); read os.Stdin directly.
- Hoist method inference in buildAPIRequestBody so it's decided once up front rather than re-checked inside the --input branch.
No behaviour change.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Sessions
2c1bcacae2d6View transcript
Changes
cmd/entire/cli
Mapi_cmd.go+34/-43
Mapi_cmd_test.go+5/-6
Mexperts_cell_target.go+13/-6
87 unmodified lines
...
// Resolution is lazy: {owner}/{repo} need only the git remote; {repo_id} costs
// a control-plane mirror lookup, so it's only done when actually referenced.
func expandAPIPlaceholders(ctx context.Context, s string) (string, error) {
if !strings.Contains(s, "{") {
needOwnerRepo := strings.Contains(s, "{owner}") || strings.Contains(s, "{repo}")
needRepoID := strings.Contains(s, "{repo_id}")
if !needOwnerRepo && !needRepoID {
return s, nil
}
if strings.Contains(s, "{owner}") || strings.Contains(s, "{repo}") {
_, owner, repo, err := gitremote.ResolveRemoteRepo(ctx, "origin")
if err != nil {
return "", fmt.Errorf("resolve {owner}/{repo} from origin remote: %w", err)
}
// Resolve the origin remote once, even when both {owner}/{repo} and
// {repo_id} are present, and thread it into the mirror lookup.
forge, owner, repo, err := gitremote.ResolveRemoteRepo(ctx, "origin")
if err != nil {
return "", fmt.Errorf("resolve current repo from origin remote: %w", err)
}
if needOwnerRepo {
s = strings.ReplaceAll(s, "{owner}", owner)
s = strings.ReplaceAll(s, "{repo}", repo)
}
if strings.Contains(s, "{repo_id}") {
id, err := resolveCurrentRepoID(ctx)
if needRepoID {
id, err := resolveCurrentRepoID(ctx, forge, owner, repo)
if err != nil {
return "", err
}
}
}
}
}
return s, nil
}
// resolveCurrentRepoID resolves the current repo's Entire ULID from its mirror
// (the mirror id, which entire-api uses as the repo_id). Picks the first active
// (non-archived, non-failed/suspended) placement.
func resolveCurrentRepoID(ctx context.Context) (string, error) {
forge, owner, repo, err := gitremote.ResolveRemoteRepo(ctx, "origin")
if err != nil {
return "", fmt.Errorf("resolve current repo: %w", err)
}
// ... additional logic ...
}
Mcmd/entire/cli/api_cmd.go+34/-43
// Test cases for the buildAPIRequestBody function
func TestBuildAPIRequestBody_MethodInference(t *testing.T) {
t.Parallel()
ctx := context.Background()
// No fields, no method → GET, no body.
r, err := buildAPIRequestBody(ctx, "/p", &apiFlags{}, nil)
if err != nil || r.method != http.MethodGet || r.body != nil {
t.Fatalf("bare = %+v, err %v", r, err)
}
// Fields, no method → POST with JSON body.
r, err = buildAPIRequestBody(ctx, "/p", &apiFlags{}, map[string]any{"a": "b"})
if err != nil || r.method != http.MethodPost || r.body == nil {
t.Fatalf("fields = %+v, err %v", r, err)
}
// ... additional test cases ...
}
Mcmd/entire/cli/api_cmd_test.go+5/-6
// Implementation of isActiveMirror function
func isActiveMirror(m coreapi.Mirror) bool {
if m.IsArchived.Or(false) {
return false
}
st := m.Status.Or(coreapi.MirrorStatusReady)
return st != coreapi.MirrorStatusFailed && st != coreapi.MirrorStatusSuspended
}
Mcmd/entire/cli/experts_cell_target.go+13/-6