Repository navigation
Add --refresh to setup-identity for re-running one credential's flow - #34
josephschorr wants to merge 2 commits into
Conversation
setup-identity skips credentials whose bytes are present and resolve cleanly. A static credential (e.g. a Tailscale auth key) carries no expiry, so an expired-but-present key reads as "present and valid" and is never re-run short of --force, which re-walks every credential. --refresh <credential> names a single credential by the name shown in the "already set up: <name>" line and re-runs just that flow, forced past the idempotency check, leaving every other credential untouched. An unknown name fails with a "did you mean?" suggestion (nearest by edit distance) rather than a silent no-op.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| if !req.Force { | ||
| // Idempotency check — skipped for --force and for the --refresh target, both | ||
| // of which mean "re-run this flow even though the credential looks set up". | ||
| if !req.Force && !refresh.active() { |
There was a problem hiding this comment.
--refresh combined with --non-interactive skips isAlreadySetUp, so a credential that is present and valid is never checked. The run then reaches the non-interactive presenter, which refuses with <name> is not set up yet (setup_present.go:194). That message is false here, and the command exits non-zero.
Repro: oap agent setup-identity my-class --refresh tailscale-authkey --non-interactive against a working key.
The two flags contradict each other, so I'd reject the combination at the top of Run, e.g. cannot --refresh without prompting; drop --non-interactive. --force + --non-interactive already behaves this way, so it could get the same guard.
There was a problem hiding this comment.
Rejected up front in Run now — --refresh with --non-interactive errors immediately (drop --non-interactive) rather than reaching the flow and hitting the false "not set up yet". Added the same guard for --force + --non-interactive, which was broken the same way.
| f.seen[name] = true | ||
| f.candidates = append(f.candidates, name) | ||
| } | ||
| if name == f.want { |
There was a problem hiding this comment.
consider matches on SuggestedName alone, but the engine dedups on SuggestedName|ProviderID (engine.go:207). If two targets require the same credential name through different providers, both requirements pass the filter, and both are forced past isAlreadySetUp. The user is prompted twice, and the second store overwrites the first.
Without --refresh, the second one would have been skipped as already set up, so this is a case where --refresh makes things worse. It's an edge case, but the fix is cheap: after the first match, have consider return false for any later requirement with the same name, or match on the same name|provider key the engine uses.
There was a problem hiding this comment.
Fixed — consider runs the named credential once: a second requirement with the same name under a different provider now returns false (first provider wins), matching the no---refresh path where it would be skipped as already set up. Previously it prompted twice and the second store overwrote the first.
| return nil | ||
| } | ||
| if len(f.candidates) == 0 { | ||
| return fmt.Errorf("setup: --refresh %q: this identity's setup requires no credentials", f.want) |
There was a problem hiding this comment.
Run skips non---only targets (engine.go:162) before consider ever sees their credentials, so both errors here can blame the wrong thing when --only is set:
--only mcp:linear --refresh tailscale-authkey→no credential named "tailscale-authkey" to refresh (available: linear-oauth). The name is correct;--onlyfiltered it out.--only toolspec:does-not-exist --refresh github-token→this identity's setup requires no credentials. The real problem is the mistyped--onlyvalue.
Suggestion: when OnlyMatch is set, say so in the error (e.g. … within --only mcp:linear), and/or report an --only that matched no target.
There was a problem hiding this comment.
Done both ways: the error now names the --only scope when set (e.g. … within --only "mcp:linear"), and an --only that matches no target is now a hard error naming the valid targets instead of silently succeeding. Updated the test that pinned the old silent-success behavior.
…nly errors - Reject --refresh/--force combined with --non-interactive up front in Run, instead of reaching the flow and failing with the presenter's false "<name> is not set up yet". - consider runs the named credential once: a second requirement with the same name under a different provider is skipped (first provider wins), matching the no-refresh path where it would be skipped as already set up. - Thread --only into the unmatched-refresh error so it names the scope that narrowed the candidates, and make an --only that matches no target a hard error naming the valid targets rather than a silent no-op.
This branch was successfully deployed
2 participants
What
Adds
--refresh <credential>tooap agent setup-identity.setup-identityskips any credential whose bytes are present and resolve cleanly. Static credentials (e.g. a Tailscale auth key) carry no expiry, so an expired-but-present key reads as "present and valid" and can't be re-run short of--force, which re-walks every credential.--refreshnames one credential — by the name shown in thealready set up: <name>line — and re-runs just that credential's setup flow, forced past the idempotency check, leaving the others untouched.Details
--only.did you mean?suggestion (nearest by edit distance, reusing the existingsmetricsnear-miss idiom) rather than a silent no-op.setup-identityform only, mirroring how--onlyis scoped.Tests
did you mean?on a near-miss name; lists available (no fuzzy guess) on a far-off name.--refreshwired end-to-end through the command, including the suggestion path.mage docs:cli).