fix(git-forges): resolve @me to a real nickname for Bitbucket issue filters#1777
Conversation
…ilters getBitbucketIssueList silently dropped the assignee/author @me filter and never applied it client-side despite a test comment claiming it did, so `coco issues --mine` returned every issue in the repo instead of just the user's own. Resolve @me via GET /user before building the issue query, so the existing assignee.nickname / reporter.nickname clauses fire server-side with the real nickname. If resolution fails, return an empty list rather than rendering everyone's issues as "mine".
![gfargo-horizon-agent[bot]](https://avatars.githubusercontent.com/in/4083186?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔎 Agent review (sonnet) — LGTM
REVIEW: LGTM
RESOLVES: full
The PR correctly resolves '@me' to a real Bitbucket nickname via GET /user before building the issues query, with a safe empty-list fallback when resolution fails, matching the implementation plan exactly and scoped to the three files it named. Tests (including the previously-stale @me test) and lint pass, and the change avoids scope creep into the analogous PR-list defect noted as out-of-scope in the plan.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
The revise correctly moves @me resolution before the fetch so the Bitbucket query is now scoped server-side, and adds solid test coverage for assignee/author/@me and named-filter paths. However, it silently swallows unresolvable @me into an empty issue list instead of surfacing an error, which is inconsistent with the sibling PR-list path in the same file that still throws a descriptive 'Could not resolve "@me"' message.
⚠️ Unresolvable @me now fails silently as empty list —src/git/bitbucketListData.ts:342— Previously (and still for the PR-list loader in this same file) an unresolvable '@me' throws a descriptive error ('Could not resolve "@me" to a Bitbucket user...') that loadForgeList surfaces via the envelope'smessagefield. This change instead returns{ issues: [] }with no message, sococo issues --minesilently renders zero issues when the nickname lookup fails (e.g. auth token lacks a nickname), giving the user no indication their identity couldn't be resolved — they'll just believe they have no assigned issues. This creates a UX inconsistency between the PR and issue list code paths for the identical failure mode.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
The revise correctly fixes the original bug by resolving '@me' to a real nickname and scoping the Bitbucket query server-side, but it stacks this on top of the pre-existing (already-merged) client-side '@me' resolution/filter block instead of replacing it, so every --mine request now hits GET /user twice and runs a now-redundant client-side filter.
⚠️ New @me resolution duplicates pre-existing resolveBitbucketMeNickname call —src/git/bitbucketListData.ts:378— getBitbucketIssueList now resolves '@me' twice: once via the new getBitbucketCurrentUserNickname (line ~341, used to build effectiveFilter for the server-side query) and again via the older resolveBitbucketMeNickname (line 378, still present from a prior revision, used for a now-redundant client-side re-filter of the already-scoped results). Both call GET /user with identical logic (bitbucketCli.ts's new getBitbucketCurrentUserNickname duplicates the nicknameOf-based parsing resolveBitbucketMeNickname already did). This doubles the network round-trip for every --mine request and introduces an inconsistent failure mode: if the first resolution succeeds but the second (entirely superfluous) call fails, the request throws 'Could not resolve "@me"...' even though the correctly-filtered data was already fetched.
There was a problem hiding this comment.
🔎 Agent re-review (sonnet, delta) — CONCERNS
REVIEW: CONCERNS
RESOLVES: full
The revise correctly fixes the reported bug: '@me' is now resolved to a real Bitbucket nickname via GET /user and substituted into the query before buildIssueEndpoint runs, with tests covering resolution, fallback, and non-'@me' isolation. Two quality issues remain: a failed '@me' resolution silently renders as zero issues instead of an error, and the fix duplicates an already-merged, more complete fix on main (commit 34020b3 / PR #1778) that also covers the PR list and fails loudly, so this branch will need reconciliation before merge.
3 concerns — 3 inline on the diff
| let effectiveFilter = filter | ||
| if (filter.assignee === '@me' || filter.author === '@me') { | ||
| const me = await getBitbucketCurrentUserNickname(runner) | ||
| if (!me) return { issues: [] } |
There was a problem hiding this comment.
getBitbucketCurrentUserNickname swallows all failures (network error, malformed JSON, missing nickname field) into undefined, and the caller responds by returning { issues: [] } with no error or message. A user running --mine during an auth hiccup sees an indistinguishable "you have zero issues" instead of being told resolution failed. Note that main's independently-merged fix (commit 34020b3) throws an explicit error in this exact situation instead.
| fetch: async (project) => { | ||
| let effectiveFilter = filter | ||
| if (filter.assignee === '@me' || filter.author === '@me') { | ||
| const me = await getBitbucketCurrentUserNickname(runner) |
There was a problem hiding this comment.
loadForgeList's auth probe already calls getBitbucketStatus, which itself calls runner('user') and discards the body just to check the request succeeds. getBitbucketCurrentUserNickname then issues a second, identical runner('user') call to extract the nickname. Every issue-list request filtered by '@me' now makes two round trips to the same endpoint where one would do.
|
|
||
| /** | ||
| * Resolve the authenticated user's nickname via GET /user, so callers can | ||
| * translate a literal `@me` filter into a real Bitbucket query clause. |
There was a problem hiding this comment.
origin/main (commit 34020b3, PR #1778, 'fix(git): resolve @me to Bitbucket nickname in PR/issue list filters') already solves this exact bug for both the PR list and issue list, using an explicit-error approach rather than a silent empty-list fallback. This branch was cut before that landed and reimplements the same fix independently and less completely (issue list only, silent failure). Merging/rebasing this PR onto current main will conflict with and likely need to be dropped in favor of the already-shipped fix.
What
Fixes
coco issues --mineon Bitbucket remotes returning every issue in the repo instead of only the authenticated user's issues.Why
Closes #1692
Plane: OSS-980
buildIssueEndpointskipped the assignee/reporter query clause whenever the filter value was the literal@me, andgetBitbucketIssueListnever applied any client-side filtering to compensate — despite a test comment claiming it did. The result:coco issues --minefetched and rendered the full unfiltered issue list under a "mine" header.How
getBitbucketCurrentUserNickname(runner)inbitbucketCli.ts, which callsGET /userand returns the authenticated user'snickname(mirrors the existing/userusage ingetBitbucketStatus).getBitbucketIssueList, resolve@me(forassigneeand/orauthor) to the real nickname before building the issue endpoint, so the existingassignee.nickname = "<me>"/reporter.nickname = "<me>"query clauses fire server-side with correct pagination./userresolution fails or returns no nickname, fall back to an empty issue list rather than ever rendering an unfiltered list as "mine".bitbucketListData.test.ts:121("handled client-side") and added coverage for:--assignee @me,--author @me, failed resolution → empty list, and that a named (non-@me) filter doesn't trigger an extra/usercall.Out of scope:
getBitbucketPullRequestListhas the analogous defect forcoco prs --mine— flagged for a follow-up, not fixed here.Testing
npm run test:jest)🤖 Generated by the harbor agent loop. Reviewed by a human before merge.
Closes #1692