Skip to content

forge: add optional Forge integration#75

Open
ArthurHeymans wants to merge 1 commit into
0WD0:mainfrom
ArthurHeymans:Forge
Open

forge: add optional Forge integration#75
ArthurHeymans wants to merge 1 commit into
0WD0:mainfrom
ArthurHeymans:Forge

Conversation

@ArthurHeymans

@ArthurHeymans ArthurHeymans commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Add majutsu-forge-mode' as an opt-in bridge that reuses Forge's local database, topic renderers, and commands inside Majutsu log buffers. It inserts pull request, issue, and discussion sections into majutsu-log-sections-hook', initializes Forge buffer state, and refreshes
Majutsu buffers when the mode changes or Forge refreshes a repository.

Bind N' and 'in Majutsu buffers toforge-dispatch' through a Majutsu
wrapper, and remap Forge topic section keymaps so `majutsu-visit-thing'
opens topic lists, topics, and repositories. Save and restore all modified
bindings when the mode is disabled.

Keep the integration defensive: hide Forge's Git-log based pull-request
commit body in Majutsu buffers, tolerate repositories that Forge cannot
identify or has not tracked, and record suppressed section errors in
`majutsu-forge-section-errors' for optional inspection.

Document setup and limitations for colocated jj/Git repositories. Add tests
for hook installation, mode enable/disable refreshes, key binding restoration,
section remaps, pull-request commit suppression, and section error clearing.

Summary by CodeRabbit

  • New Features

    • Added optional Forge integration for Majutsu users.
    • Forge-related sections can now appear in Majutsu log buffers, with quick access bindings for Forge actions.
    • Pull request commit lists can be replaced with a placeholder in Majutsu views.
  • Bug Fixes

    • Improved handling of section insertion errors so rendering can continue more gracefully.
    • Refreshes Majutsu buffers after Forge updates to keep displayed information current.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@ArthurHeymans, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 15 minutes and 18 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: db6e350b-c0f3-492f-8bdd-311ff55b4c7c

📥 Commits

Reviewing files that changed from the base of the PR and between acdce8f and 555e726.

📒 Files selected for processing (3)
  • README.org
  • majutsu-forge.el
  • test/majutsu-forge-test.el

Walkthrough

Adds majutsu-forge.el, a new optional integration library wiring Magit Forge's topic sections (pull requests, issues, discussions) into Majutsu log buffers. It provides a global majutsu-forge-mode with section hook management, keymap remapping, a PR commit placeholder, post-Forge refresh propagation, and five defcustom options. An ERT test suite and README documentation accompany the implementation.

Changes

Majutsu–Forge Integration

Layer / File(s) Summary
File header, defcustoms, and Forge buffer initialization
majutsu-forge.el
Establishes the new file with header metadata, dependency declarations, five defcustom options (add-default-sections, add-default-bindings, show-section-errors, suppress-pullreq-commits, expand-sections), Magit right-margin resolution, Forge buffer-local topic state initialization, and one-time database connection.
Section error handling, PR commit placeholder, and insertion utilities
majutsu-forge.el
Implements majutsu-forge--with-section-errors macro for condition-case suppression with per-section error recording, defines the placeholder PR commit renderer for Majutsu buffers, provides logic to show Forge topic sections, and adds helpers to clear stale errors and expand inserted topic list sections post-refresh.
Section hook commands, wiring, and related tests
majutsu-forge.el, test/majutsu-forge-test.el
Defines autoloaded insertion commands for pull requests, issues, and discussions; installs/removes them as hooks on majutsu-log-sections-hook with idempotency tracking. Tests verify idempotent hook add/remove, the PR commit placeholder text, and that the PR commit advice suppresses output only inside majutsu-log-mode.
Keymap remapping, dispatcher, and binding tests
majutsu-forge.el, test/majutsu-forge-test.el
Saves prior bindings and remaps Forge section keymaps so <remap> <majutsu-visit-thing> routes to appropriate Forge list/visit functions; adds majutsu-forge-dispatch forwarding to forge-dispatch; wires optional N/' key bindings into majutsu-mode-map. Tests verify dispatcher binding, saved-binding state cleanup, and full remap install/restore cycle.
Refresh propagation, advices, mode lifecycle, and mode tests
majutsu-forge.el, test/majutsu-forge-test.el
Derives Majutsu root directories to scope post-Forge refresh propagation to matching live buffers; installs :after/:around advices on forge-refresh-buffer and forge--insert-pullreq-commits. Defines global majutsu-forge-mode with full enable/disable lifecycle. Tests cover section error recording/clearing and mode enable/disable hook and flag lifecycle.
README Optional Forge Integration section
README.org
Adds the "Optional Forge Integration" subsection describing require/enable steps, added UI sections and key bindings, colocated jj/Git repository requirements, and the PR commit list placeholder behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 Hop, hop! The Forge is linked at last,
sections bloom where commits once passed.
Pull requests, issues, discussions too—
all nestled in Majutsu's view.
With hooks and maps and advices fine,
the rabbit's integration will shine! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding optional Forge integration for Majutsu.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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 `@majutsu-forge.el`:
- Around line 403-417: The enable path in majutsu-forge-mode only resets the
mode flag on failure, leaving partial setup from
majutsu-forge--add-section-hooks, majutsu-log-mode-hook additions,
majutsu-forge--add-mode-bindings, and majutsu-forge--add-advices behind. Update
the error branch in the condition-case around the enable sequence to undo any
installed hooks, bindings, and advices before re-signaling the error, so a
failed enable leaves majutsu-forge in a clean disabled state.
- Around line 169-183: The majutsu-forge--with-section-errors macro currently
suppresses errors but leaves any partially inserted section text behind when a
Forge renderer fails. Update the error-handling flow around
majutsu-forge--ensure-buffer and the body evaluation to capture the insertion
start before rendering, then on error remove the inserted range before calling
majutsu-forge--record-section-error and returning nil. Use
majutsu-forge--with-section-errors and majutsu-forge--record-section-error as
the key places to adjust so failed section inserts are fully rolled back.
- Around line 346-349: Guard majutsu-forge--same-root-p against stale or
inaccessible paths by wrapping the file-truename/file-equal-p comparison in
error handling so refresh advice cannot abort on a bad buffer, deleted
directory, or remote path. Keep the existing root/other nil checks, but ensure
any signaling from file-truename or file-equal-p falls back to a safe non-match
result so the refresh flow continues for the remaining buffers.

In `@test/majutsu-forge-test.el`:
- Around line 148-155: The test
`majutsu-forge-with-section-errors/suppresses-by-default` is asserting exact
equality on `majutsu-forge-section-errors` without isolating that variable, so
state from earlier tests can leak in and make the result order-dependent. Update
this test to bind or reset `majutsu-forge-section-errors` within the same `let`
that already controls `majutsu-forge-show-section-errors`, so the check around
`majutsu-forge--with-section-errors` always starts from a clean state.
- Around line 157-173: The cleanup in
majutsu-forge-section-errors/clears-stale-errors-on-render only restores
majutsu-log-sections-hook, but majutsu-forge--add-section-hooks also mutates
majutsu-forge--sections-installed. Update the unwind-protect cleanup to save and
restore that state as well so the test leaves the section-hook install state
unchanged for later tests. Use the existing test body and
majutsu-forge--add-section-hooks as the anchor points for the fix.
🪄 Autofix (Beta)

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 57ea5a49-d514-4429-b393-d364fdaeef5a

📥 Commits

Reviewing files that changed from the base of the PR and between 9c93891 and acdce8f.

📒 Files selected for processing (3)
  • README.org
  • majutsu-forge.el
  • test/majutsu-forge-test.el

Comment thread majutsu-forge.el Outdated
Comment thread majutsu-forge.el Outdated
Comment thread majutsu-forge.el
Comment thread test/majutsu-forge-test.el
Comment thread test/majutsu-forge-test.el
Add `majutsu-forge-mode' as an opt-in bridge that reuses Forge's local
database, topic renderers, and commands inside Majutsu log buffers.  It
inserts pull request, issue, and discussion sections into
`majutsu-log-sections-hook', initializes Forge buffer state, and refreshes
Majutsu buffers when the mode changes or Forge refreshes a repository.

Bind `N' and `'` in Majutsu buffers to `forge-dispatch' through a Majutsu
wrapper, and remap Forge topic section keymaps so `majutsu-visit-thing'
opens topic lists, topics, and repositories.  Save and restore all modified
bindings when the mode is disabled.

Keep the integration defensive: hide Forge's Git-log based pull-request
commit body in Majutsu buffers, tolerate repositories that Forge cannot
identify or has not tracked, roll back partial Forge section output on
renderer failures, and clean up hooks, bindings, and advices if enabling the
mode fails.

Document setup and limitations for colocated jj/Git repositories.  Add tests
for hook installation, mode enable/disable refreshes, failed-enable cleanup,
key binding restoration, section remaps, pull-request commit suppression, and
section error clearing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant