fix(client): resolve a bare repo name against what the forge grants
Found by running the real flow on a fresh clone against production, not by a test: `granthi-sync get Ai-Assistant` failed with "Repository not found". `get <name>` meant `<your-login>/<name>` and nothing else, but of 172 repos granted to this estate's own admin, 157 are owned by an ORG -- so the bare name failed for 91% of what a user can actually see, with an error that reads like a permissions problem rather than a naming one. A bare name is now matched against the granted list, which is the forge's own answer about what this account may have. An owner-qualified name is taken as given. An ambiguous bare name is REFUSED with its candidates rather than guessed -- picking one of two repos called `notes` owned by different teams is not a guess worth making for someone. Best effort by design: if the listing is unreachable, a fully-qualified name still clones and a bare name degrades to the caller's own namespace, so a network blip cannot block a clone. The clone that follows reports the real problem precisely. cmd_bootstrap carried the same assumption and is fixed with it. 203 tests (was 198). Verified live afterwards: the failing command now clones Nirlabinc/Ai-Assistant, and `get shreai` lists both candidates instead of guessing.
This commit is contained in:
@@ -833,6 +833,49 @@ def clone_one(cfg, full_name, dest, mode=None):
|
||||
return meta
|
||||
|
||||
|
||||
def resolve_granted(cfg, want, repos=None):
|
||||
"""Turn what the user typed into the repo the forge actually grants them.
|
||||
|
||||
`get notes` used to mean `<your-login>/notes` and nothing else. On a real
|
||||
account that is wrong for almost everything: of 172 repos granted to this
|
||||
estate's own admin, 157 are owned by an ORG, so the bare name failed for
|
||||
91% of what the user could see and the error said "Repository not found"
|
||||
-- which reads like a permissions problem rather than a naming one.
|
||||
|
||||
So a bare name is resolved against the granted list, which is the forge's
|
||||
own answer about what this account may have. An owner-qualified name is
|
||||
taken as given. An ambiguous bare name is REFUSED with the candidates
|
||||
rather than guessed: picking one of two repos called `notes` owned by
|
||||
different teams is not a guess worth making for someone.
|
||||
"""
|
||||
if "/" in want:
|
||||
return parse_repo_arg(want, cfg["login"])
|
||||
if repos is None:
|
||||
try:
|
||||
repos, _truncated = list_repos(cfg)
|
||||
except (SystemExit, ValueError, OSError) as e:
|
||||
# Best effort. If the listing is unreachable, a name the user
|
||||
# typed in full must still clone, and a bare name should degrade
|
||||
# to their own namespace rather than refusing outright -- the
|
||||
# clone that follows reports the real problem precisely.
|
||||
log(f"could not read your repo list ({e}); assuming "
|
||||
f"{cfg['login']}/{want}")
|
||||
return parse_repo_arg(want, cfg["login"])
|
||||
matches = [r.get("full_name") for r in repos
|
||||
if (r.get("full_name") or "").rsplit("/", 1)[-1] == want]
|
||||
if len(matches) == 1:
|
||||
return matches[0]
|
||||
if len(matches) > 1:
|
||||
listed = "\n".join(f" {m}" for m in sorted(matches))
|
||||
raise SystemExit(
|
||||
f"{want!r} is ambiguous — {len(matches)} repos you can see have "
|
||||
f"that name:\n{listed}\nRe-run with the owner, e.g. "
|
||||
f"granthi-sync get {sorted(matches)[0]}")
|
||||
# Nothing granted by that name. Fall back to the caller's own namespace so
|
||||
# the message names a concrete repo instead of a guess.
|
||||
return parse_repo_arg(want, cfg["login"])
|
||||
|
||||
|
||||
def cmd_get(args):
|
||||
cfg = require_linked(load_config())
|
||||
device_id(cfg)
|
||||
@@ -840,7 +883,7 @@ def cmd_get(args):
|
||||
return _get_all(cfg, args)
|
||||
if not args.repo:
|
||||
raise SystemExit("give a repo name, or --all")
|
||||
full_name = parse_repo_arg(args.repo, cfg["login"])
|
||||
full_name = resolve_granted(cfg, args.repo)
|
||||
name = full_name.rsplit("/", 1)[-1]
|
||||
clone_one(cfg, full_name, os.path.abspath(args.into or name), args.mode)
|
||||
return 0
|
||||
@@ -1293,12 +1336,17 @@ def cmd_bootstrap(args):
|
||||
into = os.path.abspath(args.into or ".")
|
||||
print(f"workspace: {name}\n")
|
||||
|
||||
granted = {r.get("full_name") for r in list_repos(cfg)[0]}
|
||||
granted |= {(r.get("full_name") or "").rsplit("/", 1)[-1] for r in []}
|
||||
granted_repos, _trunc = list_repos(cfg)
|
||||
granted = {r.get("full_name") for r in granted_repos}
|
||||
pulled = skipped = denied = 0
|
||||
for repo in ws["repos"]:
|
||||
want = repo["name"]
|
||||
full = want if "/" in want else f"{cfg['login']}/{want}"
|
||||
try:
|
||||
full = resolve_granted(cfg, want, repos=granted_repos)
|
||||
except SystemExit as e:
|
||||
log(f"AMBIGUOUS {want}: {e}")
|
||||
denied += 1
|
||||
continue
|
||||
if full not in granted:
|
||||
# The forge decides. A manifest asking for a repo this account was
|
||||
# not granted is a permissions answer, not an error to route round.
|
||||
|
||||
Reference in New Issue
Block a user