Skip to content

fix(installer): safely register plugins without duplication - #5

Draft
knowlet wants to merge 2 commits into
mainfrom
fix/installer-idempotent-registration
Draft

knowlet wants to merge 2 commits into
mainfrom
fix/installer-idempotent-registration

Conversation

@knowlet

@knowlet knowlet commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Prevent duplicate registrations across compact, spaced and pretty-printed JSON.
  • Decode the actual RPG Maker plugin array before checking top-level plugin names and choosing the insertion point. Brackets or matching names in comments, parameters and later statements no longer cause corruption or skipped installation.
  • Preserve existing registrations, enabled/disabled state, parameters, surrounding comments and BOM/CRLF bytes. Reject malformed/unsupported arrays and array-starting expressions without changing plugins.js.
  • Return a nonzero CLI result on registration parse failure rather than claiming completion. The plugin file may already have been copied; this is not a fully transactional installer.
  • Add 18 regression cases and document the supported JSON-array boundary. Reuse Python's standard-library JSONDecoder.raw_decode; no new dependency or JavaScript runtime change.

Reproduction and provenance

Base main: 83ceac969392e7ea1948652d0b2dd6d06233ddeb.
Current head: 5f9d20f8327bc0209ed7679689d3a7a0bb781c04.
Previous head: 5cd013fd3bc636bdc6e2d13fcf8cdd1f9bc6e906.

The previous implementation corrupts the valid fixture var $plugins = [];\n// documentation [optional]\n by inserting at the final comment bracket. A leading comment containing {"name":"DynamicTranslation"} incorrectly suppresses registration. Nested parameter names and escaped existing names are also covered now.

All 15 original source files were reconstructed from the previous exact head and matched Git blob SHA. The verified local implementation tree c5e58d5d6d46f6fc342ce957a0920c425672e668 matches the published commit tree. This updates the existing Draft rather than opening a duplicate branch or PR.

Verification for the current revision

Exact-head CI 41938753929 completed successfully on 2026-11-05 at 00:06:13 UTC:

  • Python 3.11.17: 49/49 passed.
  • Node 18.20.8 and Node 20: 68/68 Jest tests passed on each; npm ci succeeded.
  • Job checkout logs identify merge ref 2cf9c1ad49526b8e7430df9647682d696168bb18, integrating the exact head into the unchanged base. Its tree equals the head tree above. This is merge-ref CI, not a claim that the jobs checked out the bare head.

Independent assistant-cloud checks:

  • Python 3.12.14 / pytest 9.1.1: 49/49 passed, also independently rerun.
  • Negative control using the previous implementation and the final test suite: 17 failed / 32 passed. All original 31 tests continue to pass; one newly added positive format case also passes unchanged.
  • Full installer subprocess executed twice against an authored temporary project. Plugin copying, translation-file byte equality, idempotent registration and Node 24.22.1 evaluation of the actual generated plugins.js all passed.
  • Python syntax compilation, Node syntax checks and git diff whitespace checks passed. No dedicated build/lint command is configured for this Python/JavaScript project.
  • Local offline Jest installation could not complete because yocto-queue was absent from npm's cache. No alternate network route was used; the passing hosted runs above provide current-revision Jest evidence.

Boundaries and remaining risks

  • Tests use authored project fixtures and mocked RPG Maker globals. No real game/runtime, paid translation service, user game files or API credentials were used. Real MV/MZ runtime acceptance remains unverified.
  • Automatic registration supports generated JSON arrays with optional leading comments and a standard $plugins assignment. Custom JavaScript values, array-internal comments and trailing commas require manual registration. Later statements are preserved, not evaluated by the installer.
  • The pre-existing missing-plugins.js warning policy and translation-file selection policy are unchanged.
  • CI npm ci reports 11 existing dependency audit findings (7 low, 1 moderate, 3 high). Dependency files are unchanged; these findings were not remediated or individually triaged in this installer fix. A green test run is not a clean security audit.
  • No inline review threads were present. No review comment or thread resolution was posted. No merge, deployment, release, permissions or credentials were changed.

Research/source decision: docs/research/plugin-registration.md.

Summary by Sourcery

Prevent duplicate DynamicTranslation plugin registrations during repeated installation.

Bug Fixes:

  • Make DynamicTranslation plugin registration detection tolerate JSON whitespace so repeated installer runs do not create duplicate entries.

Tests:

  • Add regression coverage for compact, spaced, pretty-printed, and repeated plugin registrations while preserving existing content.

Summary by cubic

Prevents the installer from registering DynamicTranslation twice when run repeatedly. The old compact-only check ("name":"DynamicTranslation") failed to match the spaced JSON the installer itself writes, so a second invocation appended a duplicate entry.

  • The registration check now tolerates JSON whitespace.
  • Existing registrations keep their enabled/disabled state, parameters, and file formatting.
  • Adds four regression tests covering pretty-printed JSON and repeated installs with empty and non-empty plugin lists.

Written for commit 5cd013f. Summary will update on new commits.

View guided diff Turn on auto-fix

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@sourcery-ai

sourcery-ai Bot commented Nov 4, 2026

Copy link
Copy Markdown

Reviewer's Guide

The installer now detects DynamicTranslation registrations regardless of JSON whitespace, preserving existing plugin entries untouched and preventing duplicate additions on repeated runs; regression tests cover compact, spaced, pretty-printed, empty-list, and nonempty-list cases.

Flow diagram for idempotent plugin registration

flowchart TD
    A[Read plugins.js] --> B{DynamicTranslation registration found?}
    B -->|Yes| C[Preserve existing entry]
    B -->|No| D[Add DynamicTranslation registration]
    C --> E[Finish without duplicate]
    D --> E
Loading

File-Level Changes

Change Details Files
Make existing plugin detection whitespace-tolerant to prevent duplicate DynamicTranslation registrations.
  • Replace compact JSON substring matching with a regular expression accepting whitespace around the name key, colon, and value.
  • Retain early-return behavior so existing entries, including their state, parameters, and formatting, are not rewritten.
install_plugin.py
Add regression coverage for formatted registrations and repeated installer execution.
  • Verify compact, spaced, and pretty-printed registrations are skipped without modifying file contents.
  • Verify a second installation is byte-for-byte idempotent for both empty and nonempty plugin lists.
  • Assert exactly one DynamicTranslation entry remains after repeated registration.
tests/test_install_plugin.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Bound registration lookup and insertion to the decoded JSON array, preserve
comments and existing plugin settings, reject unsupported expressions without
modifying plugins.js, and propagate parse failure to the CLI.

Add 18 regression cases. Local Python 3.12.14 / pytest 9.1.1: 49 passed.
Pre-fix negative control: 17 failed, 32 passed. Actual installer repeated twice
and resulting authored plugins.js evaluated with Node 24.22.1 successfully.

Local verification commit f8d287bb37ad7fabe39cc2348c2cdeab21af7cfc has the
identical Git tree c5e58d5.
@knowlet knowlet changed the title fix(installer): prevent duplicate plugin registration fix(installer): safely register plugins without duplication Nov 5, 2026
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