git-remote-entire: Missing Cluster Host Dialog

Claude Code·Opus 4.8[1m]·

Soph·2mo ago·28hr 55min·2 Checkpoints·6 file changes·+249/-25·192.2K tokens

I have everything I need. (The other exploration agent is still finishing, but I read the helper source directly, so I'm not waiting on it.)

Short answer

A picker dialog inside the git helper is technically possible but the wrong layer — and you don't need it, because that picker already exists in entire repo clone. The right fix is to make the helper's error point there.

What actually happens with `entire://gh/entire.io/cli` today

It's worth being precise, because it's not quite the "missing host" branch. net/url parses that as:

Host = "gh", Path = "/entire.io/cli"

So gh lands in the cluster-host slot. The helper (cmd/git-remote-entire/main.go:92) only rejects a literally empty host, so this sails past and fails later at cluster discovery — it tries to fetch https://gh/.well-known/entire-cluster.json and dies with an opaque discovery error. The clean "missing host" case is actually entire:///gh/entire.io/cli (three slashes). Either way the user omitted the cluster host.

Why a dialog in the helper is the wrong tool

Three hard constraints, in order of how damning they are:

The helper can't persist your choice. Git records the remote URL in .git/config verbatim and re-invokes the helper for every future git fetch/git pull/git push. If the stored URL is under-specified, a picker would re-prompt on every operation forever — the helper has no way to rewrite the stored remote. entire repo clone, by contrast, resolves the cluster once and clones a fully-qualified entire://<cluster>/gh/owner/repo, so .git/config is correct and nothing re-prompts. This alone rules the helper out.
stdin/stdout are reserved. The helper's stdout/stdin are git's pkt-line protocol stream (main.go:9-12: "nothing here may write to stdout except the helper protocol itself"). A picker would have to go the askConfirmTTY route — write options to stderr, read the selection from /dev/tty directly (huh would need WithInput/WithOutput pointed at /dev/tty, not the default os.Stdin). Doable, but fiddly and against the "dumb, scriptable transport" design.
It'd need control-plane auth just to list candidates. The candidate mirrors come from the coreapi.ListMirrors API, which needs a resolved login context — the same resolution that's failing when the cluster host is unknown. The helper deliberately doesn't even split the forge prefix; wiring the mirror-list API + a TTY picker into the transport duplicates a chunk of repo_clone.go.

The picker you want already exists

entire repo clone /gh/entire.io/cli does exactly this (repo_clone.go):

Lists mirror placements (ListMirrors), dedupes by cluster host.
One host → clones silently. Several → huh Select titled "This repo is mirrored on more than one cluster — pick one to clone from", labeled <cell> (<jurisdiction>) — <clusterHost>.
No TTY → fails with pass --cluster to choose one of: … (agent/CI-safe).

Recommended change

Keep the helper dumb, but detect this specific mistake and route the user to the existing picker — mirroring the pattern that's already there for the server's invalid_target "lives on <host>" correction (fatalMessage/wrongClusterRe, main.go:185-211). A pre-flight check using gitremote.IsSupportedForge (already exists) cleanly distinguishes "forge id in the host slot" from a real hostname:

For entire://gh/entire.io/cli that prints entire repo clone /gh/entire.io/cli — which then shows the picker. Cheap, safe, keeps the transport a transport, and reuses the actionable-error idiom already in the file.

Want me to implement that error-pointer change (with a test in main_test.go)?

The second investigation confirms the analysis and adds one corroborating detail worth noting: repo_clone.go already explicitly guards against "the entire:///gh/... empty-host bug" on the CLI side — so the front-door command is where this class of malformed URL is already expected and handled. The helper has no TTY detection or prompting anywhere by design (stdin/stdout are strictly the pkt-line stream).

So my recommendation stands unchanged:

A picker in git-remote-entire is the wrong layer (can't persist the choice → re-prompts on every fetch; stdin/stdout reserved; would need to duplicate the mirror-list + auth path).
The picker already exists at entire repo clone /gh/entire.io/cli.
The clean improvement is a pre-flight branch in the helper that detects a forge id in the host slot (gitremote.IsSupportedForge) — and the empty-host case — and fails with an actionable message pointing at entire repo clone, reusing the existing fatalMessage idiom.

I'm ready to implement that error-pointer change (helper branch + main_test.go coverage) whenever you'd like — just say go.

Done. The change is in and verified (tests pass, mise run fmt clean, lint 0 issues).

What I changed

cmd/git-remote-entire/main.go

Added a gitremote import.
Extended the URL-validation switch: the old bare parsedURL.Host == "" case is now parsedURL.Host == "" || gitremote.IsSupportedForge(parsedURL.Host), catching both the empty-host form (entire:///gh/…) and the forge-in-host-slot form (entire://gh/…).
Added two pure helpers: missingClusterHostMessage (decides which message to emit) and clusterHostHint (the actionable text).

cmd/git-remote-entire/main_test.go

Added TestMissingClusterHostMessage covering the forge-in-host case, empty-host-with-forge-path, and three fall-back cases (non-forge path, bare entire://, single-segment path).

Behavior now

git clone entire://gh/entire.io/cli — instead of the opaque https://gh/.well-known/… discovery failure — prints to stderr:

That routes the user to the existing huh picker in entire repo clone, which resolves the mirror and clones a fully-qualified URL (so no re-prompting on later fetches). It reuses the established actionable-error idiom (fatalMessage/wrongClusterRe), keeps the transport dumb — no TTY/stdin/stdout involvement — and is safe in CI (it's just a stderr line + exit 128, no hang).

Two deliberate scoping notes:

Detection keys off gitremote.IsSupportedForge, which today only knows gh. If/when other forges (e.g. et) get added to that shared map, this hint picks them up automatically — I didn't broaden the forge set here.
Nothing is committed. Want me to branch and commit this, or leave it in the working tree?

/simplify → 4 cleanup agents in parallel → apply the fixes

You are improving the quality of the changed code, not hunting for bugs. Review it for reuse, simplification, efficiency, and altitude issues, then fix what you find. Do not look for correctness bugs — that is what /code-review is for.

Phase 0 — Gather the diff

Run git diff @{upstream}...HEAD (or git diff main...HEAD / git diff HEAD~1 if there's no upstream) to get the unified diff under review. If there are uncommitted changes, or the range diff is empty, also run git diff HEAD and include the working-tree changes in scope — the review often runs before the commit. If a PR number, branch name, or file path was passed as an argument, review that target instead. Treat this diff as the review scope.

Phase 1 — Review (4 cleanup agents in parallel)

Launch 4 independent review agents via the Agent tool, all in a single message so they run concurrently. Pass each agent the diff and one of the four angles below. Each returns its findings with file, line, a one-line summary, and the concrete cost (what is duplicated, wasted, or harder to maintain).

Reuse

Flag new code that re-implements something the codebase already has — Grep shared/utility modules and files adjacent to the change, and name the existing helper to call instead.

Simplification

Flag unnecessary complexity the diff adds: redundant or derivable state, copy-paste with slight variation, deep nesting, dead code left behind. Name the simpler form that does the same job.

Efficiency

Flag wasted work the diff introduces: redundant computation or repeated I/O, independent operations run sequentially, blocking work added to startup or hot paths. Also flag long-lived objects built from closures or captured environments — they keep the entire enclosing scope alive for the object's lifetime (a memory leak when that scope holds large values); prefer a class/struct that copies only the fields it needs. Name the cheaper alternative.

Altitude

Check that each change is implemented at the right depth, not as a fragile bandaid. Special cases layered on shared infrastructure are a sign the fix isn't deep enough — prefer generalizing the underlying mechanism over adding special cases.

Phase 2 — Apply the fixes

Wait for all four agents to complete, dedup findings that point at the same line or mechanism, and fix each remaining one directly. Skip any finding whose fix would change intended behavior, require changes well outside the reviewed diff, or that you judge to be a false positive — note the skip rather than arguing with it. Finish with a brief summary of what was fixed and what was skipped (or confirm the code was already clean).

2mo ago·1h 7m

Tests pass, lint is clean.

Summary

The /simplify review across all four angles found the code largely clean. I applied one fix:

Fixed — comment duplication (simplification). The inline switch-case comment and the missingClusterHostMessage doc comment both enumerated the same two malformed-URL shapes, which could drift apart. Trimmed the inline comment to a one-line pointer, leaving the full explanation single-sourced on the function's doc comment.

Skipped, with reasons:

Export splitForgePrefix for reuse (reuse). Real but low-value per both the reuse and simplification agents — it would widen gitremote's public API for a single caller, and the inline strings.Cut(strings.TrimPrefix(...)) is self-contained and readable. Not worth the churn.
Merge the two message helpers / eliminate the double IsSupportedForge call (simplification). The split single-sources the format string, and the repeated predicate is the inherent cost of separating switch-gate from a pure, testable dispatch function. Current form is about as simple as it should be.
run()-level integration test (altitude agent's aside). A genuine test-coverage note, but that's a /code-review concern, not a simplification of the changed code — out of scope here.
Efficiency & altitude — nothing to change; both confirmed correct as-is.

Net effect: 4 lines of near-duplicate comment collapsed to 2, no behavior change, still uncommitted in the working tree.

The PR is updated. Here's my review of the comments and what I did about them.

The comments

Two automated reviewers ran — Cursor Bugbot and GitHub Copilot — and both landed on the same single, valid issue (no human comments yet):

missingClusterHostMessage suggested entire repo clone for any forge id in the host slot, without checking that owner and repo were present. So entire://gh produced the hint entire repo clone /gh, and entire://gh/owner produced /gh/owner — refs that parseMirrorCloneRef (^/?gh/<owner>/<repo>$) rejects immediately. Worse, the empty-host branch already fell back correctly, so entire://gh/owner and entire:///gh/owner — the same mistake — behaved inconsistently.

I verified this against repo_clone.go:24 (mirrorCloneRefRe): the ref must be a complete gh/<owner>/<repo> triple. The finding is correct — a genuine "helpful command that fails immediately" defect. Everything else in the reviews was just the overview/boilerplate (Cursor's "Fix in Cursor" links, Copilot's file summary).

The fix (pushed as `6050a60b3`)

Reworked missingClusterHostMessage to reconstruct the intended shorthand once (forge-in-host-slot or empty-host both normalize to forge/owner/repo), then emit the entire repo clone hint only when it's a complete 3-segment triple with a supported forge. Anything shorter (entire://gh, .../owner), longer (extra segments), or non-forge falls back to the plain missing host error. This also fixes the inconsistency between the two URL shapes.
Collapsing the two branches let me inline the now-single-use clusterHostHint.
Added the 4 regression cases Copilot explicitly asked for: entire://gh, entire://gh/owner, entire:///gh/owner, and a too-many-segments path — all 9 test cases pass.

mise run fmt/lint clean, and test:ci passes (one earlier run had a flaky failure that didn't reproduce on re-run or in a clean go test ./...).

One note surfaced along the way: there are two checkouts on this machine — cli_experiments and cli_experiments_2. The PR and all git operations live in cli_experiments_2 (the working dir); my initial grep this turn accidentally hit the other checkout, which briefly looked like the change was missing. Confirmed the PR repo is correct and now current.

Want me to post a short reply on the two review threads noting the fix (and resolve them), or leave that to you?