Skip to content

feat(plugin-api): enhance remark/rehype hooks with error isolation, priority, and hot-reload#121

Merged
tomymaritano merged 20 commits into
developfrom
feature/remark-rehype-hooks-enhancement
Mar 12, 2026
Merged

feat(plugin-api): enhance remark/rehype hooks with error isolation, priority, and hot-reload#121
tomymaritano merged 20 commits into
developfrom
feature/remark-rehype-hooks-enhancement

Conversation

@tomymaritano

@tomymaritano tomymaritano commented Mar 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Error isolation: safePluginWrapper catches plugin failures at transformer level, injects inline error marker instead of crashing the entire preview
  • Priority ordering: Plugins declare execution priority (0-100); getPlugins() returns sorted array
  • Plugin metadata: PluginHookOptions type carries name, version, priority through registration
  • Hot-reload: Toggling a plugin in Settings triggers immediate preview update via requestReload()
  • Error block CSS: Subtle red-left-border style for .plugin-error-block in preview

Test plan

  • Typecheck passes (18/18 tasks)
  • All tests pass (153/153)
  • Store tests updated with metadata field
  • Priority sorting verified in tests
  • Registry test updated for wrapped plugin functions
  • Manual: toggle plugin in Settings → preview updates immediately
  • Manual: plugin with intentional error → error block appears in preview

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Plugin toggles in settings now instantly refresh live previews
    • Plugin errors are caught and displayed as styled error blocks in previews instead of crashing
  • Style

    • Added visual styling for plugin error displays

tomymaritano and others added 8 commits March 11, 2026 11:15
…ration

Add optional metadata (name, version, priority) to registerRemarkPlugin
and registerRehypePlugin signatures for debugging and execution ordering.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wraps remark/rehype plugins so that if a transformer throws, the error
is caught, logged, and an error marker node is injected into the AST
instead of crashing the entire preview pipeline.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update registerRemarkPlugin and registerRehypePlugin lambdas in
activate() to accept optional PluginHookOptions and build metadata
from options with manifest defaults (name, version, priority=100).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add requestReload() call after setEnabled in handleToggle so the
markdown preview updates immediately without needing the manual
"Reload Plugins" button.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…y, and safe wrapping

- Add metadata field (name, version, priority) to registrations
- Wrap plugins with safePluginWrapper on register for error isolation
- Sort getPlugins() output by priority (ascending)
- Log registrations with console.debug

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add metadata field to all store test registrations
- Add priority sorting tests for both remark and rehype stores
- Update registry test to expect wrapped plugin functions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Mar 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cf604fd6-f09c-40c8-bb98-1638be0b6506

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR introduces a plugin metadata and error-handling system across the plugin API and desktop app. It adds name, version, and priority metadata to plugin registrations, wraps plugins with try/catch error handlers that inject error boundary nodes into the AST, styles error blocks in previews, enables hot-reload on plugin toggle, and updates stores to sort plugins by priority.

Changes

Cohort / File(s) Summary
Plugin Metadata Infrastructure
packages/plugin-api/src/types.ts, packages/plugin-api/src/preview/safePluginWrapper.ts
Introduces PluginHookOptions interface for metadata (name, version, priority) and safePluginWrapper function that wraps plugins with per-node error handling, logging, and AST error boundary injection.
Plugin Stores
packages/plugin-api/src/preview/remarkPluginStore.ts, packages/plugin-api/src/preview/rehypePluginStore.ts
Updates both stores to accept metadata during registration, wrap plugins with safePluginWrapper, apply priority-based sorting to getPlugins output, and add registration logging.
Plugin Registry & Runtime
packages/plugin-api/src/lifecycle/PluginRegistry.ts, apps/desktop/src/renderer/stores/pluginRuntimeStore.ts
Extends plugin registration methods to accept PluginHookOptions, propagate metadata to stores, add pluginId to metadata, implement togglePlugin for hot-reload, and handle deactivation via unregister-by-pluginId.
Public API Exports
packages/plugin-api/src/index.ts
Exports new PluginHookOptions type, safePluginWrapper function, and PluginMetadata type.
Desktop App UI & Styling
apps/desktop/src/renderer/pages/settings/sections/PluginsSection.tsx, apps/desktop/src/renderer/styles/preview.css
Adds window reload trigger after plugin toggle to refresh previews; adds CSS styling for .plugin-error-block with red theme, monospace font, and error presentation.
Tests
packages/plugin-api/tests/registry.test.ts, packages/plugin-api/tests/remarkPluginStore.test.ts, packages/plugin-api/tests/rehypePluginStore.test.ts
Updates test assertions to verify plugin wrapping, adds metadata to test registrations, refactors store tests to validate priority-based sorting behavior.
Documentation
docs/plans/2026-03-11-remark-rehype-hooks-enhancement.md
Comprehensive plan document detailing the complete enhancement: metadata system, error wrapping, storage, registry integration, styling, hot-reload flow, built-in plugin updates, and verification plan.

Sequence Diagram(s)

sequenceDiagram
    participant PluginSetting as Plugin Setting UI
    participant Runtime as Plugin Runtime Store
    participant Registry as Plugin Registry
    participant RemarkStore as Remark Store
    participant RehypeStore as Rehype Store
    participant Wrapper as Safe Wrapper
    participant Preview as Markdown Preview

    PluginSetting->>Runtime: togglePlugin(pluginId, enabled)
    alt Plugin Enabled
        Runtime->>Registry: activate(pluginId)
        Registry->>RemarkStore: register(plugin, {name, version, priority})
        RemarkStore->>Wrapper: safePluginWrapper(plugin, metadata)
        Wrapper-->>RemarkStore: wrapped plugin
        RemarkStore->>RemarkStore: sort by priority
        Registry->>RehypeStore: register(plugin, {name, version, priority})
        RehypeStore->>Wrapper: safePluginWrapper(plugin, metadata)
        Wrapper-->>RehypeStore: wrapped plugin
        RehypeStore->>RehypeStore: sort by priority
    else Plugin Disabled
        Runtime->>Registry: deactivate(pluginId)
        Registry->>RemarkStore: unregister(pluginId)
        Registry->>RehypeStore: unregister(pluginId)
    end
    Runtime->>Preview: reload()
    Preview->>RemarkStore: getPlugins() [sorted]
    Preview->>RehypeStore: getPlugins() [sorted]
    Note over Wrapper: On plugin error:<br/>try/catch transforms<br/>inject error boundary node
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: error isolation (safePluginWrapper), priority ordering, and hot-reload functionality.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/remark-rehype-hooks-enhancement

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 and usage tips.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tomymaritano

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Mar 11, 2026

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@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: 12

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/desktop/src/renderer/styles/preview.css`:
- Around line 554-559: Replace the hard-coded red values in this preview.css
block (background, border-left, color) with semantic theme tokens so error
styling follows the app theme; update the background, border-left and color
declarations to use centralized CSS variables (e.g. --error-background,
--error-border, --error-foreground or the existing theme token names used
elsewhere) and ensure padding, margin, and font-size remain unchanged; modify
only the property values in this rule so other styles continue to apply and
later set those token values in the global theme variables file.
- Around line 553-562: The .plugin-error-block can be broken by very long
unbroken error.message text injected by safePluginWrapper.ts; update the
.plugin-error-block CSS to allow long words to wrap and prevent horizontal
overflow by adding rules such as overflow-wrap: anywhere (or overflow-wrap:
break-word), word-break: break-word (or break-word/normal combos for
cross-browser), and ensure white-space is not preventing wrapping (e.g.,
white-space: normal); apply these changes to the .plugin-error-block rule so
arbitrary error.message content from safePluginWrapper.ts wraps instead of
expanding the preview width.

In `@docs/plans/2026-03-11-remark-rehype-hooks-enhancement.md`:
- Around line 52-54: The document uses two different CSS class names for the
injected error node—plugin-error-boundary and plugin-error-block—causing
inconsistency; pick one canonical class name (e.g., plugin-error-block) and
update all references across the plan (mentions in Task 2, the node example, and
the CSS) to use that single name consistently, replacing every occurrence of the
alternate symbol (plugin-error-boundary) with the chosen symbol so the
implementation steps, examples, and styles match.
- Around line 108-109: Update the docs to use the correct bulk-removal method
name: replace references to unregister(pluginId) with unregisterAll(pluginId)
and keep the existing single-remove API as unregister(id); also ensure the
backward-compatible note about register(plugin) remains (defaults: priority=100,
name='unknown', version='0.0.0') and update any other occurrences that mention
unregister(pluginId) to unregisterAll(pluginId).
- Line 13: The top-level task headings are H3 but should be H2 to maintain
proper outline and avoid MD001; update the heading "### Task 1: Define
PluginHookOptions type" (and all other "### Task N" headings) to "## Task 1:
Define PluginHookOptions type" so the document goes H1 -> H2 for tasks and
retains consistent lower-level headings under each task.
- Around line 203-220: The togglePlugin flow updates plugin state but never
triggers a preview refresh; modify togglePlugin (the function used by
PluginsSection after replacing setEnabled with
pluginRuntimeStore.getState().togglePlugin) to call requestReload() after making
changes—specifically, after pluginRegistry.deactivate/pluginStore.unregister
calls when disabling and after await get().reloadPlugins() when enabling—so the
active preview is explicitly refreshed whenever a plugin is toggled.

In `@packages/plugin-api/src/preview/rehypePluginStore.ts`:
- Around line 39-41: Replace the disallowed console.debug call with an allowed
console method (use console.warn to match ESLint constraints) in the
registration logging statement inside rehypePluginStore.ts — update the
console.debug invocation that logs `[RehypePlugins] Registered:
${registration.metadata.name}@${registration.metadata.version} (priority:
${registration.metadata.priority})` to use console.warn (keeping the same
message and interpolation) so the log uses an allowed method; ensure no other
console.debug occurrences remain in the rehype plugin registration code path.

In `@packages/plugin-api/src/preview/remarkPluginStore.ts`:
- Around line 39-41: Replace the disallowed console.debug call with an allowed
console method (e.g., console.warn) or remove it; specifically update the
statement that logs `[RemarkPlugins] Registered:
${registration.metadata.name}@${registration.metadata.version} (priority:
${registration.metadata.priority})` so it uses console.warn (or the project's
logger) instead of console.debug, keeping the same message and references to
registration.metadata.name/version/priority.

In `@packages/plugin-api/src/preview/safePluginWrapper.ts`:
- Line 25: The code uses the overly-permissive Function type when invoking
plugins (e.g., the assignment transformer = (plugin as Function)(...args); and
the similar occurrence at the other site), causing the ESLint
no-unsafe-function-type error; replace the cast with a concrete function
signature such as (plugin as (...args: unknown[]) => unknown)(...args) (or a
more specific parameter/return signature if you know the plugin shape), and
update any downstream typing to treat the result as unknown/appropriate typed
value instead of any so the call is both safe and satisfies
`@typescript-eslint/no-unsafe-function-type`.
- Around line 57-60: The HTML injected into children in safePluginWrapper.ts
uses metadata.name and message without full escaping; create/use an escapeHtml
helper and replace all special chars (& < > " ' /` ) in metadata.name and
message (instead of only replacing `"` and `<`) before embedding them in the
value string so the error marker cannot introduce executable HTML; update the
children.push call to use escapedMetadataName and escapedMessage variables
(referencing metadata.name and message) and ensure the data-plugin and
data-error attributes and the element text all use the escaped values.

In `@packages/plugin-api/tests/rehypePluginStore.test.ts`:
- Around line 4-5: Add a test that registers a plugin without passing metadata
to ensure backward compatibility: create a registration call using the existing
register function (the same one used by other tests) with no metadata argument,
then assert that the stored plugin metadata equals the defaultMeta object
(name:'test' or appropriate default fields) and that the plugin ordering
respects the default priority/order; reference the existing defaultMeta constant
in the test to compare expected values and place the test alongside the other
registration tests in rehypePluginStore.test.ts.

In `@packages/plugin-api/tests/remarkPluginStore.test.ts`:
- Around line 4-5: Add a test that covers the metadata-free registration path:
register a plugin without passing the metadata argument and assert that the
store fills in the defaults (name 'test'/'1.0.0'/priority 100 as represented by
defaultMeta). Locate the registration call used in this file (the function that
currently takes a metadata parameter) and add one case that omits that
parameter, then assert the stored plugin's name, version and priority match the
defaults (use the defaultMeta constant to compare). Ensure the test name clearly
indicates "metadata-free registration" so this backward-compatible path remains
exercised.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d02bad46-2a90-45aa-bd04-014f95a8f6e3

📥 Commits

Reviewing files that changed from the base of the PR and between 73f9938 and 3902345.

📒 Files selected for processing (12)
  • apps/desktop/src/renderer/pages/settings/sections/PluginsSection.tsx
  • apps/desktop/src/renderer/styles/preview.css
  • docs/plans/2026-03-11-remark-rehype-hooks-enhancement.md
  • packages/plugin-api/src/index.ts
  • packages/plugin-api/src/lifecycle/PluginRegistry.ts
  • packages/plugin-api/src/preview/rehypePluginStore.ts
  • packages/plugin-api/src/preview/remarkPluginStore.ts
  • packages/plugin-api/src/preview/safePluginWrapper.ts
  • packages/plugin-api/src/types.ts
  • packages/plugin-api/tests/registry.test.ts
  • packages/plugin-api/tests/rehypePluginStore.test.ts
  • packages/plugin-api/tests/remarkPluginStore.test.ts

Comment on lines +553 to +562
.plugin-error-block {
background: rgba(239, 68, 68, 0.08);
border-left: 3px solid #ef4444;
padding: 4px 8px;
margin: 4px 0;
font-size: 0.75rem;
color: #ef4444;
border-radius: 2px;
font-family: var(--font-mono, monospace);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Prevent long plugin messages from forcing horizontal overflow.

packages/plugin-api/src/preview/safePluginWrapper.ts:38-65 injects arbitrary error.message text into this block. Long unbroken messages will not wrap under the current rules and can blow out the preview width.

Suggested fix
 .plugin-error-block {
   background: rgba(239, 68, 68, 0.08);
   border-left: 3px solid `#ef4444`;
   padding: 4px 8px;
   margin: 4px 0;
   font-size: 0.75rem;
   color: `#ef4444`;
   border-radius: 2px;
   font-family: var(--font-mono, monospace);
+  overflow-wrap: anywhere;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.plugin-error-block {
background: rgba(239, 68, 68, 0.08);
border-left: 3px solid #ef4444;
padding: 4px 8px;
margin: 4px 0;
font-size: 0.75rem;
color: #ef4444;
border-radius: 2px;
font-family: var(--font-mono, monospace);
}
.plugin-error-block {
background: rgba(239, 68, 68, 0.08);
border-left: 3px solid `#ef4444`;
padding: 4px 8px;
margin: 4px 0;
font-size: 0.75rem;
color: `#ef4444`;
border-radius: 2px;
font-family: var(--font-mono, monospace);
overflow-wrap: anywhere;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/styles/preview.css` around lines 553 - 562, The
.plugin-error-block can be broken by very long unbroken error.message text
injected by safePluginWrapper.ts; update the .plugin-error-block CSS to allow
long words to wrap and prevent horizontal overflow by adding rules such as
overflow-wrap: anywhere (or overflow-wrap: break-word), word-break: break-word
(or break-word/normal combos for cross-browser), and ensure white-space is not
preventing wrapping (e.g., white-space: normal); apply these changes to the
.plugin-error-block rule so arbitrary error.message content from
safePluginWrapper.ts wraps instead of expanding the preview width.

Comment on lines +554 to +559
background: rgba(239, 68, 68, 0.08);
border-left: 3px solid #ef4444;
padding: 4px 8px;
margin: 4px 0;
font-size: 0.75rem;
color: #ef4444;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Use semantic error tokens instead of fixed red literals.

This file is otherwise token-driven, but this block hard-codes its background, border, and text colors. Moving those to theme variables will keep the error state consistent across themes and easier to tune centrally.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/styles/preview.css` around lines 554 - 559, Replace
the hard-coded red values in this preview.css block (background, border-left,
color) with semantic theme tokens so error styling follows the app theme; update
the background, border-left and color declarations to use centralized CSS
variables (e.g. --error-background, --error-border, --error-foreground or the
existing theme token names used elsewhere) and ensure padding, margin, and
font-size remain unchanged; modify only the property values in this rule so
other styles continue to apply and later set those token values in the global
theme variables file.


---

### Task 1: Define PluginHookOptions type

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Promote the task headings to ##.

The document jumps from H1 to H3 at Line 13, which triggers MD001 and makes the outline harder to scan. Use ## Task N for these top-level sections and keep the rest consistent.

🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-03-11-remark-rehype-hooks-enhancement.md` at line 13, The
top-level task headings are H3 but should be H2 to maintain proper outline and
avoid MD001; update the heading "### Task 1: Define PluginHookOptions type" (and
all other "### Task N" headings) to "## Task 1: Define PluginHookOptions type"
so the document goes H1 -> H2 for tasks and retains consistent lower-level
headings under each task.

Comment on lines +52 to +54
- On error: logs `[PluginPipeline] ${name}@${version} failed on node ${node.type}: ${error.message}`
- On error: injects an error marker node into the AST (a `div` with class `plugin-error-boundary` and data attributes for plugin name + error message)
- On success: returns the transformed node normally

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use one error-block class name throughout the plan.

Task 2 refers to plugin-error-boundary, but the node example and the CSS both use plugin-error-block. That inconsistency makes the implementation steps self-contradictory.

Also applies to: 84-85, 167-180

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-03-11-remark-rehype-hooks-enhancement.md` around lines 52 -
54, The document uses two different CSS class names for the injected error
node—plugin-error-boundary and plugin-error-block—causing inconsistency; pick
one canonical class name (e.g., plugin-error-block) and update all references
across the plan (mentions in Task 2, the node example, and the CSS) to use that
single name consistently, replacing every occurrence of the alternate symbol
(plugin-error-boundary) with the chosen symbol so the implementation steps,
examples, and styles match.

Comment on lines +108 to +109
- `unregister(pluginId)` — remove all plugins registered by a given pluginId
- Backward-compatible: `register(plugin)` still works (defaults: priority=100, name='unknown', version='0.0.0')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Document the bulk-removal API with the right method name.

These sections say unregister(pluginId), but the store contract exercised in this PR still separates unregister(id) from unregisterAll(pluginId). Documenting the wrong call here will mislead the next change in this area.

Suggested doc diff
- - `unregister(pluginId)` — remove all plugins registered by a given pluginId
+ - `unregisterAll(pluginId)` — remove all plugins registered by a given pluginId- - On plugin deactivate: call `remarkPluginStore.unregister(pluginId)` and `rehypePluginStore.unregister(pluginId)`
+ - On plugin deactivate: call `remarkPluginStore.unregisterAll(pluginId)` and `rehypePluginStore.unregisterAll(pluginId)`

Also applies to: 151-152

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plans/2026-03-11-remark-rehype-hooks-enhancement.md` around lines 108 -
109, Update the docs to use the correct bulk-removal method name: replace
references to unregister(pluginId) with unregisterAll(pluginId) and keep the
existing single-remove API as unregister(id); also ensure the
backward-compatible note about register(plugin) remains (defaults: priority=100,
name='unknown', version='0.0.0') and update any other occurrences that mention
unregister(pluginId) to unregisterAll(pluginId).

Comment on lines +39 to +41
console.debug(
`[RemarkPlugins] Registered: ${registration.metadata.name}@${registration.metadata.version} (priority: ${registration.metadata.priority})`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace console.debug with an allowed console method.

The ESLint rule no-console only allows warn and error. The CI is emitting a warning for this statement.

🔧 Proposed fix

Either remove the debug statement for production or change to an allowed level:

-    console.debug(
+    console.warn(
       `[RemarkPlugins] Registered: ${registration.metadata.name}@${registration.metadata.version} (priority: ${registration.metadata.priority})`,
     );

Alternatively, if this is intentional debug logging, consider using a conditional or the plugin's logger infrastructure instead.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.debug(
`[RemarkPlugins] Registered: ${registration.metadata.name}@${registration.metadata.version} (priority: ${registration.metadata.priority})`,
);
console.warn(
`[RemarkPlugins] Registered: ${registration.metadata.name}@${registration.metadata.version} (priority: ${registration.metadata.priority})`,
);
🧰 Tools
🪛 GitHub Actions: CI

[warning] 39-39: ESLint: Unexpected console statement. Only these console methods are allowed: warn, error (no-console).

🪛 GitHub Check: lint

[warning] 39-39:
Unexpected console statement. Only these console methods are allowed: warn, error

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/plugin-api/src/preview/remarkPluginStore.ts` around lines 39 - 41,
Replace the disallowed console.debug call with an allowed console method (e.g.,
console.warn) or remove it; specifically update the statement that logs
`[RemarkPlugins] Registered:
${registration.metadata.name}@${registration.metadata.version} (priority:
${registration.metadata.priority})` so it uses console.warn (or the project's
logger) instead of console.debug, keeping the same message and references to
registration.metadata.name/version/priority.

return function safePlugin(...args: unknown[]) {
let transformer: unknown;
try {
transformer = (plugin as Function)(...args);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix ESLint error: Replace Function type with explicit function signature.

The CI is failing because Function is too permissive. Use a more specific type to satisfy the @typescript-eslint/no-unsafe-function-type rule.

🔧 Proposed fix
-      transformer = (plugin as Function)(...args);
+      transformer = (plugin as (...args: unknown[]) => unknown)(...args);

And at line 40:

-        return (transformer as Function)(tree, file);
+        return (transformer as (tree: unknown, file: unknown) => unknown)(tree, file);

Also applies to: 40-40

🧰 Tools
🪛 ESLint

[error] 25-25: The Function type accepts any function-like value.
Prefer explicitly defining any function parameters and return type.

(@typescript-eslint/no-unsafe-function-type)

🪛 GitHub Actions: CI

[error] 25-25: ESLint: The Function type accepts any function-like value. Prefer explicitly defining any function parameters and return type (no-unsafe-function-type).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/plugin-api/src/preview/safePluginWrapper.ts` at line 25, The code
uses the overly-permissive Function type when invoking plugins (e.g., the
assignment transformer = (plugin as Function)(...args); and the similar
occurrence at the other site), causing the ESLint no-unsafe-function-type error;
replace the cast with a concrete function signature such as (plugin as (...args:
unknown[]) => unknown)(...args) (or a more specific parameter/return signature
if you know the plugin shape), and update any downstream typing to treat the
result as unknown/appropriate typed value instead of any so the call is both
safe and satisfies `@typescript-eslint/no-unsafe-function-type`.

Comment on lines +57 to +60
children.push({
type: 'html',
value: `<div class="plugin-error-block" data-plugin="${metadata.name}" data-error="${message.replace(/"/g, '&quot;')}">⚠ ${metadata.name} plugin failed: ${message.replace(/</g, '&lt;')}</div>`,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Sanitize metadata.name to prevent XSS in error marker.

The plugin name is injected directly into HTML without sanitization. A malicious plugin could register with a name like <script>alert(1)</script> and execute arbitrary JavaScript when the error marker renders. Additionally, the > character in the error message is not escaped.

🔒 Proposed fix for proper HTML escaping
+// Helper to escape HTML special characters
+function escapeHtml(str: string): string {
+  return str
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;');
+}
+
 // Inside the catch block:
           children.push({
             type: 'html',
-            value: `<div class="plugin-error-block" data-plugin="${metadata.name}" data-error="${message.replace(/"/g, '&quot;')}">⚠ ${metadata.name} plugin failed: ${message.replace(/</g, '&lt;')}</div>`,
+            value: `<div class="plugin-error-block" data-plugin="${escapeHtml(metadata.name)}" data-error="${escapeHtml(message)}">⚠ ${escapeHtml(metadata.name)} plugin failed: ${escapeHtml(message)}</div>`,
           });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/plugin-api/src/preview/safePluginWrapper.ts` around lines 57 - 60,
The HTML injected into children in safePluginWrapper.ts uses metadata.name and
message without full escaping; create/use an escapeHtml helper and replace all
special chars (& < > " ' /` ) in metadata.name and message (instead of only
replacing `"` and `<`) before embedding them in the value string so the error
marker cannot introduce executable HTML; update the children.push call to use
escapedMetadataName and escapedMessage variables (referencing metadata.name and
message) and ensure the data-plugin and data-error attributes and the element
text all use the escaped values.

Comment on lines +4 to +5
const defaultMeta = { name: 'test', version: '1.0.0', priority: 100 };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Preserve coverage for registrations that omit metadata.

All updated cases in this file now pass metadata explicitly. Since the new API is supposed to keep existing plugins working unchanged, add one test that registers without metadata and verifies the default values/order.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/plugin-api/tests/rehypePluginStore.test.ts` around lines 4 - 5, Add
a test that registers a plugin without passing metadata to ensure backward
compatibility: create a registration call using the existing register function
(the same one used by other tests) with no metadata argument, then assert that
the stored plugin metadata equals the defaultMeta object (name:'test' or
appropriate default fields) and that the plugin ordering respects the default
priority/order; reference the existing defaultMeta constant in the test to
compare expected values and place the test alongside the other registration
tests in rehypePluginStore.test.ts.

Comment on lines +4 to +5
const defaultMeta = { name: 'test', version: '1.0.0', priority: 100 };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Keep one metadata-free registration path under test.

Every registration in this suite now supplies metadata, so the backward-compatible path is no longer exercised here. Please add one case that omits metadata and asserts the default name/version/priority so that existing-plugin compatibility cannot regress silently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/plugin-api/tests/remarkPluginStore.test.ts` around lines 4 - 5, Add
a test that covers the metadata-free registration path: register a plugin
without passing the metadata argument and assert that the store fills in the
defaults (name 'test'/'1.0.0'/priority 100 as represented by defaultMeta).
Locate the registration call used in this file (the function that currently
takes a metadata parameter) and add one case that omits that parameter, then
assert the stored plugin's name, version and priority match the defaults (use
the defaultMeta constant to compare). Ensure the test name clearly indicates
"metadata-free registration" so this backward-compatible path remains exercised.

tomymaritano and others added 8 commits March 11, 2026 20:06
- Replace unsafe `Function` type casts with explicit signatures in safePluginWrapper
- Add escapeHtml helper to prevent XSS in error marker HTML injection
- Change console.debug to console.warn in plugin stores (ESLint no-console)
- Use CSS custom properties with fallbacks for error block colors
- Add overflow-wrap: anywhere to prevent horizontal overflow on long errors
- Add default metadata backward-compatibility tests to both plugin stores

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…try store

Introduce ThemeDefinition type, CORE_THEME_TOKENS whitelist, and
validation helpers that reject unknown CSS tokens. Add ThemeRegistry
Zustand vanilla store for registering/unregistering themes with
active theme management.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…t method

Wire theme system into the app: useThemeOverrides applies active theme
tokens to document root, and registerTheme lets plugins register full
themes through the PluginContext with proper lifecycle cleanup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire up Electron's nativeTheme API so the renderer can set the theme
source (dark/light/system) and receive system theme change notifications
via IPC instead of relying solely on window.matchMedia.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add activeThemeId to AppearanceSettings schema, plugin theme selector
in AppearanceSection (shown only when plugin themes are registered),
and startup restoration of the saved theme in App.tsx.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover isValidThemeToken, validateThemeTokens, and full themeRegistryStore
lifecycle (register, unregister, unregisterAll, setActive, getActiveTheme).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tomymaritano

Copy link
Copy Markdown
Collaborator Author

CodeRabbit Review Comments — All Resolved ✅

All 12 CodeRabbit comments have been reviewed. Here's what was addressed:

Fixed (commit 38a5170):

  1. 🟠 MAJOR — ESLint Function type (safePluginWrapper.ts:25,40): Replaced (plugin as Function) with explicit function signatures (...args: unknown[]) => unknown and (tree: unknown, file: unknown) => unknown

  2. 🟠 MAJOR — XSS in error marker (safePluginWrapper.ts:60): Added escapeHtml() helper that escapes & < > " characters. All interpolations in the error marker HTML now use it.

  3. 🟡 Minor — console.debug (rehypePluginStore.ts:41, remarkPluginStore.ts:41): Changed console.debug to console.warn to satisfy ESLint no-console rule.

  4. 🟡 Minor — CSS hardcoded colors (preview.css:559,562): Replaced hardcoded red colors with CSS custom properties (--error-bg, --error-border, --error-fg) with fallback values. Added overflow-wrap: anywhere.

  5. 🟡 Minor — Test coverage (rehypePluginStore.test.ts:5, remarkPluginStore.test.ts:5): Added backward-compatibility tests for default metadata values.

Skipped (documentation only):

  • 4 comments on docs/plans/2026-03-11-remark-rehype-hooks-enhancement.md — plan doc, not code.

Verification: 169 tests pass, typecheck clean across all 3 phases.

tomymaritano and others added 3 commits March 11, 2026 20:57
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ype-hooks-enhancement

# Conflicts:
#	apps/desktop/src/preload/index.ts
#	apps/desktop/src/renderer/App.tsx
#	apps/desktop/src/renderer/hooks/useAppearanceSettings.ts
#	apps/desktop/src/renderer/pages/settings/sections/AppearanceSection.tsx
#	packages/plugin-api/src/lifecycle/PluginRegistry.ts
#	packages/plugin-api/src/theme/themeRegistryStore.ts
#	packages/plugin-api/src/theme/useThemeOverrides.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@tomymaritano tomymaritano merged commit 3d41f25 into develop Mar 12, 2026
@tomymaritano tomymaritano deleted the feature/remark-rehype-hooks-enhancement branch March 12, 2026 00:29
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