Feat/mcp enhancements and browse fix

[xiedewei]

Summary

• Add httpclient.GetRaw and CurrentUserSlug methods (use X-AUSERNAME response header instead of temp-comment hack)
• Fix browse raw/file commands: bypass OpenAPI client URL-encoding of / in file paths
• Add MCP tools: get_pr_diff, get_file_content
• Enhance MCP list_pull_requests: dashboard mode, role filter, validate project/repo params
• Enhance MCP add_pr_comment: inline comments (path/line) and replies (parent_id)
• Enhance MCP submit_pr_review: add needs_work action
• Add PR service methods: NeedsWork, AddComment, AddInlineComment
• Use reviewers field with participants fallback for PR reviewer data (tested on Bitbucket Data Center 9.4.16 where participants is always empty)
• Remove unnecessary withAttributes=true query parameter from PR Get
• Add bb.exe to .gitignore

[vriesdemichael]

Thanks for this — genuinely useful contribution. The %2F browse bug is a real one, and the discovery that participants comes back empty on Data Center 9.4.16 while reviewers is populated is exactly the kind of field report that's hard to get otherwise. Nice work tracking that down, and thanks for replacing the temp-comment hack for resolving the current user with the X-AUSERNAME header — that's a much cleaner approach.

I've reviewed the branch locally: it builds, go vet is clean, and the full test suite passes. A few things need addressing before I can merge.

Needs fixing:

1. Path escaping in browse.Raw / browse.File. internal/services/browse/service.go interpolates the user-supplied path straight into the URL and hands it to url.Parse with no escaping. A path containing ? or # reshapes the request, and .. can traverse to a different REST endpoint. Via the CLI that's mostly self-inflicted, but get_file_content takes path directly from a model, so it becomes an API-pivot vector. Could you escape per segment (url.PathEscape on each element, joined with /) rather than dropping escaping altogether? That still fixes the original bug.

2. Formatting. gofmt isn't clean — internal/cli/repo_cat_edit.go lines 33 and 83 are indented with 2 tabs instead of 3 (which also re-indents adjacent untouched lines), and internal/cli/browse.go has strings.TrimRight(cfg.BitbucketURL, "/" ) with a stray space in four places. A gofmt -w pass over the changed files should cover it; CI will fail on this as-is.

3. Test coverage. The new service methods NeedsWork, AddComment, and AddInlineComment are at 0%, as are CurrentUserSlug and GetRaw's retry/error-mapping paths. That's roughly 130 of ~300 new lines untested, and the Codecov patch gate will flag it. Also, switching the existing PR fixtures from participants to reviewers moves coverage rather than adding it — the participants branch of mapReviewers now has no fixture at all. A fixture for each path would be better.

4. Stale comment. The doc comment on NeedsWork still says the user slug is resolved "by posting a temporary comment and reading the author from the response" — that's the hack this PR removes. Just needs deleting.

5. skills/bb/SKILL.md documents the MCP tool catalog and hasn't been updated: get_pr_diff and get_file_content are missing, submit_pr_review is still described as approve/unapprove only, and list_pull_requests doesn't mention dashboard mode.

6. Output shape change. list_pull_requests now returns {"pull_requests": [...]} instead of a bare array. I'm fine with the change — just worth calling out in the description since it changes the tool's output contract.

Smaller things, take or leave:

• browse.Service.baseURL is stored but never read (GetRaw uses the httpclient's own base URL). Dropping it removes the parameter from six call sites and the six TrimRight expressions with it.
• File() only bypasses the generated client when the path contains /, so root-level files take the old route. That leaves two code paths with different retry and error-mapping behaviour for the same operation — probably cleanest to route both through GetRaw.
• X-AUSERNAME extraction already exists in internal/cli/cmd/auth/auth.go around line 505. Worth consolidating on your new CurrentUserSlug.
• add_pr_comment silently ignores parent_id when path+line are set, and silently ignores path when line is missing. For a model-facing tool a validation error is friendlier than a silent fallback to a non-inline comment.
• Do you know whether inline anchors need diffType: EFFECTIVE on 9.4.16? It's required on some versions for the comment to attach correctly, and the payload doesn't set it.

Happy either way on who does the work — if you'd like to push the fixes yourself that's great, or if you'd rather hand it off, say so and we'll take it from here and get it landed with credit to you. No rush either way.

One logistical note: the PR is opened from your fork's main branch, which can make follow-up pushes awkward. Not a blocker, just something to keep in mind for next time.