docs(readme): add interactive oblique code-map SVG - #54
Conversation
On Cloud API-token mode, self-identity is resolved via GET /2.0/user. A successful response carrying an empty uuid was stored verbatim, leaving a.self == "". isSelf then never matches, silently disabling the reply-loop guard so the bot's own comments are dispatched. Treat an empty resolved identity like a resolve failure (deps.Done + close listener), matching the GitLab sibling's fatal-on-unresolved-self posture.
📝 WalkthroughWalkthroughThe README adds a linked repository terrain graphic. Bitbucket connection setup now treats an empty successful self-identity result as fatal and reports ChangesREADME terrain graphic
Bitbucket identity validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review posted — 2 finding(s).
|
![botbooter-test[bot]](https://avatars.githubusercontent.com/u/197033?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: comment · Grade: 7/10
To reach a higher grade: Split the undescribed bitbucket logic change out of a docs PR (or accurately describe it) and ensure the referenced SVG asset is actually committed.
📝 Summary of changes
The PR is described as a docs-only change adding an interactive oblique code-map SVG to the README, but the diff also contains an unrelated behavior change in internal/bitbucket/server.go: it now treats a Cloud whoami that succeeds with an empty identity as a fatal error (via ErrMissingConfig), with a matching test.
The bitbucket logic change itself is correct and well-tested — an empty self silently disables the reply-loop guard, so failing fast mirrors the GitLab sibling's discipline, and the if a.cfg.Self == "" gate means it only affects the Cloud whoami path (DC's Self is validated at New). The added test asserts the fatal path via deps.Done.
My concerns are process/consistency rather than correctness: the substantive adapter change is hidden inside a PR labeled and described as a README image addition, and the README references an asset (.github/assets/lao_botbooter_oblique.svg) that does not appear in the diff.
🤖 AI prompt to fix all 2 finding(s) (review before running)
Fix 2 issue(s) found during code review of lao/botbooter (PR #54).
--- Issue 1 ---
File: internal/bitbucket/server.go:254 (side RIGHT)
Severity: medium
Issue: Behavior change buried in a docs-titled PR
This PR is titled and described as adding a README SVG code map, but it also changes the Bitbucket adapter's Connect-path self-resolution so an empty-but-no-error whoami now fails fatally via `ErrMissingConfig`. The change itself is sound (an empty `self` disables the reply-loop guard, so failing fast matches the GitLab sibling, and it only affects the Cloud whoami path since DC validates `Self` at `New`), and it is covered by a new test. The problem is discoverability: an adapter behavior change hidden inside a documentation PR is easy to merge unreviewed. Either move this into its own PR or update the title/description to call it out. If keeping it, consider sweeping the sibling webhook adapters (GitHub, Signal) for the same empty-identity gap, per CLAUDE.md's note that a fix in one adapter's self-resolve scaffolding likely applies to the others.
--- Issue 2 ---
File: README.md:12 (side RIGHT)
Severity: low
Issue: README references an SVG asset not present in the diff
The new README block embeds and links `.github/assets/lao_botbooter_oblique.svg`, but that file is not added anywhere in this diff (and no `.github/assets/` path appears in the indexed file tree). If the asset was not committed alongside this change, the README will render a broken image and the raw-file links will 404. I'm uncertain whether the SVG is committed via a path not shown here — please confirm the asset is included in the PR before merging.
Apply minimal, correct fixes that resolve these issues. Match the surrounding code's existing style and conventions, and do not change unrelated behavior.
Reviewed by prbooter. Comment /recheck to re-run, /security-review for a security audit, /auto-fix to have me attempt the fixes, or /clean to remove my comments.
| // is as dangerous as a failure: isSelf would never match, silently | ||
| // disabling the reply-loop guard. Treat it as fatal, like GitLab does. | ||
| if resolveErr == nil && self == "" { | ||
| resolveErr = fmt.Errorf("%w: whoami returned an empty identity", ErrMissingConfig) |
There was a problem hiding this comment.
[medium] Behavior change buried in a docs-titled PR
This PR is titled and described as adding a README SVG code map, but it also changes the Bitbucket adapter's Connect-path self-resolution so an empty-but-no-error whoami now fails fatally via ErrMissingConfig. The change itself is sound (an empty self disables the reply-loop guard, so failing fast matches the GitLab sibling, and it only affects the Cloud whoami path since DC validates Self at New), and it is covered by a new test. The problem is discoverability: an adapter behavior change hidden inside a documentation PR is easy to merge unreviewed. Either move this into its own PR or update the title/description to call it out. If keeping it, consider sweeping the sibling webhook adapters (GitHub, Signal) for the same empty-identity gap, per CLAUDE.md's note that a fix in one adapter's self-resolve scaffolding likely applies to the others.
🤖 AI prompt to fix (review before running)
Fix an issue found during code review of lao/botbooter (PR #54).
File: internal/bitbucket/server.go:254 (side RIGHT)
Severity: medium
Issue: Behavior change buried in a docs-titled PR
This PR is titled and described as adding a README SVG code map, but it also changes the Bitbucket adapter's Connect-path self-resolution so an empty-but-no-error whoami now fails fatally via `ErrMissingConfig`. The change itself is sound (an empty `self` disables the reply-loop guard, so failing fast matches the GitLab sibling, and it only affects the Cloud whoami path since DC validates `Self` at `New`), and it is covered by a new test. The problem is discoverability: an adapter behavior change hidden inside a documentation PR is easy to merge unreviewed. Either move this into its own PR or update the title/description to call it out. If keeping it, consider sweeping the sibling webhook adapters (GitHub, Signal) for the same empty-identity gap, per CLAUDE.md's note that a fix in one adapter's self-resolve scaffolding likely applies to the others.
Apply a minimal, correct fix that resolves this issue. Match the surrounding code's existing style and conventions, and do not change unrelated behavior.
|
|
||
| <p align="center"> | ||
| <a href="https://raw.githubusercontent.com/lao/botbooter/main/.github/assets/lao_botbooter_oblique.svg"> | ||
| <img src=".github/assets/lao_botbooter_oblique.svg" alt="lao/botbooter repository terrain — 145 files rendered as an oblique code map; hover or focus a file to inspect it, activate its link to open the file on GitHub" width="100%"> |
There was a problem hiding this comment.
[low] README references an SVG asset not present in the diff
The new README block embeds and links .github/assets/lao_botbooter_oblique.svg, but that file is not added anywhere in this diff (and no .github/assets/ path appears in the indexed file tree). If the asset was not committed alongside this change, the README will render a broken image and the raw-file links will 404. I'm uncertain whether the SVG is committed via a path not shown here — please confirm the asset is included in the PR before merging.
🤖 AI prompt to fix (review before running)
Fix an issue found during code review of lao/botbooter (PR #54).
File: README.md:12 (side RIGHT)
Severity: low
Issue: README references an SVG asset not present in the diff
The new README block embeds and links `.github/assets/lao_botbooter_oblique.svg`, but that file is not added anywhere in this diff (and no `.github/assets/` path appears in the indexed file tree). If the asset was not committed alongside this change, the README will render a broken image and the raw-file links will 404. I'm uncertain whether the SVG is committed via a path not shown here — please confirm the asset is included in the PR before merging.
Apply a minimal, correct fix that resolves this issue. Match the surrounding code's existing style and conventions, and do not change unrelated behavior.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 17: Remove the blank line within the blockquote in README.md, or prefix
it with “>” if it belongs to the quote, so the markdownlint MD028 violation is
resolved. Run make all to verify the fix.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6595ce6d-f1d1-41a2-a532-2383a9324caf
⛔ Files ignored due to path filters (1)
.github/assets/lao_botbooter_oblique.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
README.mdinternal/bitbucket/server.gointernal/bitbucket/server_test.go
| </p> | ||
|
|
||
| > _Interactive map — [open the SVG directly](https://raw.githubusercontent.com/lao/botbooter/main/.github/assets/lao_botbooter_oblique.svg) to hover, focus, and click files (GitHub renders it statically inline)._ | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the blank line inside the blockquote.
markdownlint-cli2 reports MD028 on Line 17. Remove the blank line, or prefix it with > if the following content must remain in the same blockquote. Run make all after the fix.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 17-17: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 17, Remove the blank line within the blockquote in
README.md, or prefix it with “>” if it belongs to the quote, so the markdownlint
MD028 violation is resolved. Run make all to verify the fix.
Sources: Coding guidelines, Linters/SAST tools
1 participant
Adds the interactive
lao_botbooter_oblique.svgcode map to the README, placed right after the badges..github/assets/lao_botbooter_oblique.svg.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation