gl duplicates the owner-resolution helper across 4 command modules, keeping a node-DID fallback repo.rs already dropped as a bug

[beardthelion]

gl has four private copies of the same "resolve a bare repo name to (owner, name)" helper, and they disagree with the one module that got the behavior right.

The duplicates

Each is the same three-tier logic: if repo contains /, split it; else derive the short DID from the local keypair (did.split(':').next_back()); else fall back to the node's own DID via GET /.

• crates/gl/src/cert.rs:60 resolve_repo
• crates/gl/src/issue.rs:132 resolve_repo
• crates/gl/src/protect.rs:71 resolve_owner_repo
• crates/gl/src/visibility.rs:77 resolve_owner_repo

issue, protect, and visibility are near-identical. cert.rs reaches the same fetch by a different route (signed_client + get_authed), but in the fallback branch the keypair load has already failed, so signed_client builds a keyless client and get_authed falls through to an unsigned GET /, making it behaviorally the same. None of the four is pub; each is a private copy.

PR #159 adds a fifth: it converts webhook.rs's resolve_owner (currently node-DID-only) into this same three-tier shape, citing cert.rs as the pattern to follow.

The node-DID fallback is the wrong half to copy

crates/gl/src/repo.rs:215 resolve_owner_did deliberately omits the node-DID fallback, with a comment explaining why:

Never falls back to the node's own DID (that produced bogus "owned by the node" results for repos that don't exist) — if there's no local identity the caller must pass an explicit owner/name.

So the four duplicates carry a fallback that another module already found to be a bug and removed. With a bare repo name and no local identity, they resolve the node operator's DID as the owner and issue a request against {node-did}/{repo}, which 404s for anyone who is not the operator. This is the same failure mode #159's description attributes to webhook.rs. Reachability is limited (the fallback only fires when no local keypair is present, which most gl commands already treat as an error), so it is latent rather than actively breaking, but it is wrong wherever it fires.

Suggested direction

Extract one shared helper (natural home: identity.rs, next to load_keypair_from_dir) with the repo.rs:215 semantics: split on /, else local keypair, else hard-error with the "pass owner/name" message. Migrate cert, issue, protect, visibility (and repo.rs's own resolve_owner_repo_pair) onto it, and drop the node-DID branch rather than preserving it. Coordinate with #159 so it adopts the shared helper instead of adding a fifth copy of the fallback.

Genuinely different, leave out of the duplicate set: star.rs:56 (sync, no fallback), pr.rs:184 (keypair-only, returns String), mcp.rs:1231 (JSON-args based).

Related

The did.split(':').next_back() short-DID derivation used by all of these is method-blind: it collapses did:key:zX and did:gitlawb:zX to the same zX. A single shared helper is the natural place to switch to a method-aware normalization once, but that is a separate concern and need not block the consolidation.