Refactor Grant Revoke Routing and Tests · Entire

Reviewer Concerns

Some issues on this branch still, from reviewers:

revokeProjectGrantee/revokeRepoGrantee are near-identical (~18 lines each) — confirm dupl (threshold 75) stays green; if it trips, a //nolint:dupl with rationale per CLAUDE.md is the expected fix.
No test for granteeName or the new list columns. The granteeName(name, id) fallback (friendly name vs ULID) and the expanded grantColumns/projectGrantRow/repoGrantRow are entirely untested. It's trivial branching, but it's user-facing output and exactly the kind of thing a 3-line table test pins cheaply — especially the empty-OptString→ULID fallback. Worth adding before merge.

Both reviewer concerns addressed.

Dupl (concern 1)

Confirmed green. mise run lint (threshold 75, CI-enforcing) reports nothing for grant.go — revokeProjectGrantee/revokeRepoGrantee don't trip at 75. No //nolint:dupl needed. The advisory mise run dup at threshold 50 flags the list commands, not the revoke pair, and is advisory-only.

Tests (concern 2)

Added to grant_test.go:

TestGranteeName — friendly name wins; unset OptString→ULID; empty-string OptString→ULID fallback.
TestGrantRows — pins grantColumns width (5) plus full row output for projectGrantRow (resolved name) and repoGrantRow (unresolved → ULID), checking column order/alignment.

All pass; lint clean. Nothing committed — yours to push.

Can we dedupe the list or revoke commands at all?

Done. Both functions now delegate to a shared revokeGrantee helper; the ULID-vs-handle routing and message formatting live in one place. Message output is byte-identical to before (account X from project Y / X from project Y).

Lint clean, tests pass.
List commands left as-is (deliberately — marginal gain, readability cost, not a CI blocker).

Nothing committed — yours to push.

Does 'list' need deduping?

No — and I'd actively avoid it.

It's not real logic duplication, it's three distinct API shapes. Each list differs in ref resolver (resolveOrgRef/resolveProjectRef/resolveRepoRef), param type + ID field (OrgId/ProjectId/RepoId), list method, and result field (out.Members vs out.Grants). Repo also carries --project. The only literally-identical part is the cursor/error boilerplate inside the fetchAllPages closure.
A generic helper costs more than it saves. To collapse it you'd thread 3–4 type params and 3–4 function args (newParams, setPageToken, call, items, next) per call site. The call sites wouldn't get shorter — they'd get harder to read, swapping obvious code for clever code.
No maintenance win. If one endpoint's pagination changes, it changes independently regardless. There's no shared invariant to protect.
No CI pressure. It only trips advisory dup (threshold 50), not lint (75).

The revoke dedup was worth it because the route-decision logic (ULID vs handle + message formatting) was genuinely shared. List has no equivalent shared decision — just parallel boilerplate. Leave it.

Okay, cool. Commit and push.

Pushed. 4bf8bdf11 on paul/cor-699-cant-repo-grant-with-entire-cli.

Commit Message

refactor(grant): share revoke routing; test granteeName + grant rows
Extract the ULID-vs-handle revoke routing and message formatting shared by revokeProjectGrantee/revokeRepoGrantee into revokeGrantee. Add tests for granteeName's name/ULID fallback and the project/repo grant row builders.