feat(team): UUID pass-through + auto-refresh on cache miss (#190)#245
Merged
feat(team): UUID pass-through + auto-refresh on cache miss (#190)#245
Conversation
Two usability wins for team resolution in `jr issue create/edit/list`:
1. UUID pass-through
Detect standard Atlassian team UUID format (36-char, hex+hyphen,
8-4-4-4-12) at resolve_team_field's entry. Pre-cache-load — a cold
cache no longer forces a teams fetch just to validate an ID the
caller already knows. Useful for agents and scripts with pre-known
team IDs.
2. Auto-refresh on miss
When partial_match returns None and the cache came from disk (not
freshly fetched), fetch once transparently and retry. Bounded to a
single retry via `cache_was_fresh` — no infinite loop. Uses a
`refreshed` flag so the post-refresh miss message says "checked a
fresh team list" rather than misleadingly advising to run the
refresh the user just effectively ran.
Observability (under `--verbose`):
- "[verbose] team resolved via UUID pass-through: <uuid>"
- "[verbose] team <name> not in cache, refreshing from server..."
Tests:
- 7 unit tests on is_team_uuid (standard, uppercase/mixed hex, wrong
length, missing hyphen, non-hex, team name with hyphens)
- 3 integration tests in tests/cli_handler.rs:
* UUID pass-through: .expect(0) on GraphQL + teams, .expect(1) on PUT
* Auto-refresh on miss: stale cache + missing name → 1 GraphQL +
1 teams fetch + PUT with refreshed UUID
* Bounded retry: name missing from stale+fresh → 1 fetch, no PUT,
bails with "checked a fresh team list" (not the stale advice)
Local review caught two Important findings, both addressed:
- Post-refresh miss message was stale (suggesting the refresh that
just ran) — fixed with `refreshed` flag + distinct message
- Auto-refresh and UUID pass-through were silent even under --verbose
— fixed with two [verbose] eprintln hints
Closes #190
There was a problem hiding this comment.
Pull request overview
Updates team resolution for jr issue create/edit/list --team to support direct UUID inputs and to transparently refresh the team cache on a name-resolution miss, improving UX and avoiding unnecessary network calls.
Changes:
- Add UUID format detection to short-circuit team cache/name resolution and pass the UUID directly as the custom field value.
- Add a single bounded “refresh once and retry” behavior when a team name is missing from a disk-loaded cache.
- Add handler-level tests covering UUID pass-through and auto-refresh behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/cli/issue/helpers.rs |
Implements UUID pass-through, auto-refresh-on-miss logic, updated error messaging path, plus unit tests for UUID detection. |
tests/cli_handler.rs |
Adds integration/handler tests to assert UUID pass-through avoids network calls and auto-refresh is bounded and observable via behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Per Copilot review on PR #245: 1. `refreshed` flag under-specified: it only tracked the retry fetch at step 5, missing the initial cold-cache fetch at step 3. A fresh-fetch miss via cold cache (no teams.json on disk) wrongly advised "Run `jr team list --refresh`" — the user just effectively did. Replace with `fetched_fresh = cache_was_fresh || retry_fetched` so the bail message covers both fetch paths. 2. `is_team_uuid_rejects_team_name_with_hyphens` used a 37-char input rejected by the length check, not the hyphen/hex logic it claimed to test. Rename to reflect actual coverage and add two new 36-char tests exercising the hex-position branches: - `is_team_uuid_rejects_hyphens_in_wrong_position_at_36_chars` - `is_team_uuid_rejects_non_hex_at_hex_position` 3. Add integration test `test_edit_team_cold_cache_miss_avoids_stale_advice` that pins the cold-cache miss scenario (cache_was_fresh=true, retry_fetched=false, fetched_fresh=true).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two usability wins for team resolution on
jr issue create/edit/list --team:1. UUID pass-through
Detect standard Atlassian team UUID format (36-char, hex+hyphen, 8-4-4-4-12) at
resolve_team_field's entry — skip cache load + name match entirely. Pre-cache-load means a cold cache no longer forces a teams fetch just to validate an ID the caller already knows. Useful for agents and scripts with pre-known team IDs.2. Auto-refresh on miss
When name resolution returns
MatchResult::Noneand the cache came from disk (not freshly fetched), fetch once transparently and retry. Bounded to a single retry via acache_was_freshflag — no infinite loop.Observability
Under
--verbose, both paths emit hints so users can debug latency spikes or confirm the short-circuit fired:Post-refresh-miss message is context-aware: if the retry also missed, the bail says "No team matching "X" (checked a fresh team list). Verify the team name or check access permissions." — not the stale "Run `jr team list --refresh`" advice (which the user just effectively ran).
Findings from local review (all addressed)
refreshedflag; distinct message path--verbose[verbose]eprintln hintsresolve_team_fieldcalled from 3 sites; only edit path coveredTest plan
cargo fmt --all -- --checkcleancargo clippy --all-targets -- -D warningscleancargo test— 824 passed, 0 failed (baseline 814 + 10 new)is_team_uuid.expect(n)on each endpoint:.expect(0)on GraphQL + teams,.expect(1)on PUTCloses #190