Skip to content

feat(plugin): add dynamic config UI with schema-driven form#109

Merged
mpiton merged 8 commits intomainfrom
feat/task-15-plugin-config-ui
Apr 26, 2026
Merged

feat(plugin): add dynamic config UI with schema-driven form#109
mpiton merged 8 commits intomainfrom
feat/task-15-plugin-config-ui

Conversation

@mpiton
Copy link
Copy Markdown
Owner

@mpiton mpiton commented Apr 25, 2026

Summary

• Plugin config schema parsing from plugin.toml with dual validation (UI + backend)
• SQLite persistence via new plugin_configs table and PluginConfigStore port
• CQRS handlers: GetPluginConfigQuery (schema + values) and UpdatePluginConfigCommand (validate + persist)
• TypeScript React form dispatcher rendering all field types (string, integer, float, boolean, url, enum, array)
• Tauri IPC: plugin_config_get(name) and plugin_config_update(name, key, value) commands
• Runtime config sync: persisted values replayed to SharedHostResources::plugin_configs at startup
• i18n support (en/fr) with 11 new translation keys

Type

feat


Summary by cubic

Adds a schema‑driven plugin configuration UI powered by [config] in plugin.toml. Values are validated in the UI and backend, stored in SQLite, exposed via IPC, applied live to running plugins, and validated at plugin load to avoid stale values; fulfills task 15.

  • New Features

    • Parse [config] schema with defaults and constraints (string, integer, float, boolean, url, enum, array with min/max/regex/options); new domain types (ConfigField, ConfigFieldType, PluginConfigSchema).
    • Persist values via plugin_configs (SQLite) and PluginConfigStore; replay at startup; live sync via loader set_runtime_config; get_manifest exposes schema.
    • CQRS + IPC: GetPluginConfigQuery, UpdatePluginConfigCommand, and Tauri plugin_config_get(name) / plugin_config_update(name, key, value).
    • Frontend PluginConfigDialog with dynamic fields and client‑side validation; “Configure” only shows when a plugin declares fields; i18n (en/fr).
  • Bug Fixes

    • Validation now runs at plugin-load time: startup replay inserts raw values, and the loader drops schema‑invalid persisted entries before the plugin can read them via get_config.
    • Validation hardening: validate manifest defaults; integer bounds with ceil/floor; reject empty floats and NaN/±Infinity; strict JSON array parser with element‑count min/max and RFC‑8259 checks; regex checks reject unsupported features and malformed patterns at parse and validate.
    • Safer apply flow: set runtime first, then persist; roll back runtime to prior effective/default on persist failure.
    • UI polish: validate arrays (shape + count) before submit; avoid clobbering drafts while saving; correct aria-describedby; stable query key; single invalidate after save; float validator uses Number.isFinite.
    • Tests expanded for JSON array parsing/count bounds, invalid escapes and leading zeros, NaN/Infinity rejection, regex feature/malformed detection, and manifest rejection of unsupported/malformed patterns.

Written for commit 6f56f48. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Dynamic plugin configuration: typed schemas, per-field validation, Configure dialog with sequential per-field saves, persisted runtime plugin settings restored at startup
    • History retention setting with presets and unlimited mode (scheduled purge)
    • Move downloads: single & bulk relocation with progress and partial‑failure reporting
    • Queue reordering: drag‑and‑drop, move‑to‑top/bottom, and persisted manual ordering
    • English & French UI copy for configuration flows and toasts
  • Documentation

    • CHANGELOG updated with the above features
  • Tests

    • New unit tests for plugin config UI and validation

Plugins declaring a `[config]` block in `plugin.toml` now expose their
schema at runtime. Backend extends `PluginManifest` with
`PluginConfigSchema`, adds the `PluginConfigStore` port (SQLite-backed
via the new `plugin_configs` table), persists user values across
restarts, and surfaces them through new `plugin_config_get` /
`plugin_config_update` IPC commands. The `ExtismPluginLoader` exposes
`get_manifest` and `set_runtime_config` so the loader's in-memory
plugin_configs map is kept in sync with the persisted store, ensuring
`get_config(key)` calls inside WASM plugins observe the latest value
without a reload.

Frontend ships a `PluginConfigField` dispatcher (string → input,
boolean → switch, integer/float → numeric input with bounds, url →
url input, enum / string-with-options → select) and a
`PluginConfigDialog` that loads the schema, validates every field
against type / min / max / regex on the UI side and persists changed
values sequentially. The Plugins view fetches each installed plugin's
schema once and only renders the *Configure* button when the manifest
declares fields.

Validation runs both UI-side (optimistic feedback) and backend-side
(authoritative — `UpdatePluginConfigCommand` re-validates against the
schema before persisting), matching the existing `update_config`
pattern.

i18n: `plugins.action.configure`, `plugins.config.{title,description,
loading,error,noFields,toast.{saveSuccess,validationFailed}}` (en/fr).
@github-actions github-actions Bot added documentation Improvements or additions to documentation rust frontend labels Apr 25, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds dynamic per-plugin runtime configuration: schema extraction from manifests, typed validation, SQLite-backed persisted store with migrations/repo, CQRS commands/queries and Tauri IPC, frontend modal editor and field components, and startup replay of persisted configs into the plugin loader.

Changes

Cohort / File(s) Summary
Domain: plugin schema & validation
src-tauri/src/domain/model/plugin.rs, src-tauri/src/adapters/driven/plugin/manifest.rs
Introduce config schema types (ConfigFieldType, ConfigField, PluginConfigSchema), robust validation (types, bounds, arrays, URL, regex handling), and manifest parsing to build schema + defaults.
Plugin loader & registry
src-tauri/src/domain/ports/driven/plugin_loader.rs, src-tauri/src/adapters/driven/plugin/extism_loader.rs, src-tauri/src/adapters/driven/plugin/registry.rs
Add get_manifest and set_runtime_config to PluginLoader; Extism loader exposes manifests and updates per-plugin runtime config map; registry exposes manifest() accessor.
Persistence: entity, migration, repo
src-tauri/src/adapters/driven/sqlite/entities/plugin_config.rs, src-tauri/src/adapters/driven/sqlite/migrations/m20260425_000005_create_plugin_configs.rs, src-tauri/src/adapters/driven/sqlite/plugin_config_repo.rs, src-tauri/src/adapters/driven/sqlite/migrations/mod.rs, src-tauri/src/adapters/driven/sqlite/mod.rs, src-tauri/src/adapters/driven/sqlite/entities/mod.rs
Add plugin_configs SeaORM entity (composite PK plugin_name,key), migration, and SqlitePluginConfigRepo implementing PluginConfigStore with upsert, listing, deletion and tests.
Ports & wiring
src-tauri/src/domain/ports/driven/plugin_config_store.rs, src-tauri/src/domain/ports/driven/mod.rs, src-tauri/src/application/command_bus.rs, src-tauri/src/application/query_bus.rs, src-tauri/src/lib.rs
Introduce PluginConfigStore trait; add optional plugin loader/config store fields to CommandBus/QueryBus with builder setters; seed persisted configs into shared host resources at startup; register new IPC handlers.
CQRS: commands & queries
src-tauri/src/application/commands/update_plugin_config.rs, src-tauri/src/application/commands/mod.rs, src-tauri/src/application/queries/get_plugin_config.rs, src-tauri/src/application/queries/mod.rs
Add UpdatePluginConfigCommand and GetPluginConfigQuery handlers: manifest lookup, schema validation, runtime map update, persistence with upsert and rollback on error, and schema+values merging for reads.
Read models & DTOs
src-tauri/src/application/read_models/plugin_config_view.rs, src-tauri/src/application/read_models/mod.rs
Add PluginConfigView and ConfigFieldDto to convert domain schema into camelCase, sorted DTOs for frontend consumption.
Tauri IPC & frontend wiring
src-tauri/src/adapters/driving/tauri_ipc.rs, src/views/PluginsView.tsx, src/views/PluginsView/PluginConfigDialog.tsx, src/views/PluginsView/PluginConfigField.tsx, src/views/PluginsView/PluginStoreRow.tsx, src/views/PluginsView/__tests__/*
Add plugin_config_get / plugin_config_update IPCs; frontend modal and field components with client-side validation, sequential per-key persistence, conditional Configure button, and tests.
Types & i18n
src/types/plugin-config.ts, src/i18n/locales/en.json, src/i18n/locales/fr.json, CHANGELOG.md
Add TS types for plugin config schema, localization keys for Configure UI and toasts, and changelog entry.
Other features included
(various backend + frontend files)
Also includes history retention pruning, change-directory download moves with engine pause/restart, and queue reordering with persisted queue_position and DnD support (covered elsewhere in diff).

Sequence Diagram

sequenceDiagram
    participant FE as Frontend UI
    participant IPC as Tauri IPC
    participant QB as QueryBus
    participant PL as PluginLoader
    participant CS as PluginConfigStore
    participant DB as SQLite
    participant WASM as WASM Plugin

    FE->>IPC: plugin_config_get(name)
    IPC->>QB: GetPluginConfigQuery(name)
    QB->>PL: get_manifest(name)
    PL-->>QB: PluginManifest + schema
    QB->>CS: get_values(name)
    CS->>DB: SELECT * FROM plugin_configs
    DB-->>CS: persisted values
    CS-->>QB: values map
    QB->>QB: merge schema + defaults + persisted
    QB-->>IPC: PluginConfigView
    IPC-->>FE: schema & values

    FE->>IPC: plugin_config_update(name,key,value)
    IPC->>QB: UpdatePluginConfigCommand(name,key,value)
    QB->>PL: get_manifest(name)
    PL-->>QB: PluginManifest + schema
    QB->>QB: validate(key,value)
    QB->>CS: set_value(name,key,value)
    CS->>DB: INSERT/UPDATE plugin_configs
    DB-->>CS: success
    CS-->>QB: Ok
    QB->>PL: set_runtime_config(name,key,value)
    PL->>WASM: update shared host config
    WASM-->>PL: Ok
    PL-->>QB: Ok
    QB-->>IPC: Ok
    IPC-->>FE: success
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

ui

Poem

🐇 I hopped through manifests by moonlit light,
I checked each field and kept defaults tight.
I saved to SQLite with a cheerful hum,
Plugins wake configured — now let them run! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.01% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(plugin): add dynamic config UI with schema-driven form' accurately captures the main objective of the PR: introducing dynamic plugin configuration with a schema-driven UI form system.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/task-15-plugin-config-ui

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

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 25, 2026

Greptile Summary

This PR adds a complete schema-driven plugin configuration system: [config] sections in plugin.toml define typed fields that are validated on both the frontend and backend, stored in a new plugin_configs SQLite table, replayed at startup, and exposed through a PluginConfigDialog with per-field inline errors. The implementation is thorough — runtime rollback on persist failure, GetPluginConfigQuery filtering stale persisted values against the current schema, and i18n for all new strings — and most issues raised in earlier review rounds have been addressed (empty-float, array element-count checks, Number.isFinite for Infinity).

Confidence Score: 4/5

Safe to merge with low risk; two previously-flagged P1s (escaped \$ anchor and stale config rows after uninstall) remain open but don't block core functionality.

No new P0/P1 issues found in this review pass. The P1 ceiling comes from the unresolved \$ end-anchor stripping bug in match_regex and PluginConfigStore::delete_all not being called in handle_uninstall_plugin, both identified in earlier review rounds. All previously-raised validation defects (empty-float, array element-count, Infinity) are correctly fixed. The rest of the code is well-structured with solid test coverage.

src-tauri/src/domain/model/plugin.rs (regex anchor stripping) and src-tauri/src/application/commands/uninstall_plugin.rs (missing delete_all call)

Important Files Changed

Filename Overview
src-tauri/src/domain/model/plugin.rs Adds ConfigField, ConfigFieldType, PluginConfigSchema, a hand-rolled JSON array length counter, and a minimal regex engine; the $ end-anchor stripping in match_regex does not handle escaped \$ (flagged in previous review and still present)
src-tauri/src/application/commands/update_plugin_config.rs Validates value against schema, sets runtime first, persists, rolls back runtime on persist failure; runtime-only inconsistency when there is no prior value and persist fails is documented and minor
src-tauri/src/application/queries/get_plugin_config.rs Joins schema with persisted values, prunes stale entries via schema validation, fills manifest defaults; clean and well-tested
src-tauri/src/application/commands/uninstall_plugin.rs Never calls PluginConfigStore::delete_all on uninstall; config rows for the uninstalled plugin remain indefinitely (flagged in previous review and still unaddressed)
src-tauri/src/adapters/driven/sqlite/plugin_config_repo.rs SQLite repo with upsert, per-plugin isolation, delete_all; well-tested with in-process tests
src/views/PluginsView/PluginConfigDialog.tsx Validate function now properly uses Number.isFinite and checks array element counts; draft-seeding race fixed; sequential saves with single post-loop invalidation is correct
src/views/PluginsView.tsx Batch-fetches all installed plugins' configs to show/hide Configure button; query key includes installedNames so any status change triggers a full refetch; misleading cache-sharing comment (flagged previously)
src-tauri/src/lib.rs Startup replay inserts all persisted values raw; relies on per-plugin load-time pruning to discard stale values (documented in code comment); unresolved concern from previous review

Sequence Diagram

sequenceDiagram
    participant UI as PluginConfigDialog
    participant IPC as Tauri IPC
    participant QB as QueryBus
    participant CB as CommandBus
    participant Loader as ExtismPluginLoader
    participant Store as SqlitePluginConfigRepo

    UI->>IPC: plugin_config_get(name)
    IPC->>QB: handle_get_plugin_config
    QB->>Loader: get_manifest(name)
    QB->>Store: get_values(name)
    QB-->>UI: PluginConfigView {fields, values}

    UI->>IPC: plugin_config_update(name, key, value)
    IPC->>CB: handle_update_plugin_config
    CB->>Loader: get_manifest(name) — validate schema
    CB->>Store: get_values(name) — capture prior value
    CB->>Loader: set_runtime_config(name, key, value)
    CB->>Store: set_value(name, key, value)
    alt persist fails
        CB->>Loader: set_runtime_config(name, key, prior) — rollback
        CB-->>UI: Err(AppError)
    else persist ok
        CB-->>UI: Ok(())
    end
Loading

Fix All in Claude Code

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/views/PluginsView.tsx
Line: 128-149

Comment:
**Batch config refetch on every plugin status change**

The query key `["plugin_config_get_all", installedNames]` is an array that is rebuilt whenever `entries` or `locallyDisabled` changes. Any plugin install, uninstall, enable, or disable mutates `installedNames`, invalidates the whole cache entry, and triggers a fresh `Promise.all` call for every installed plugin — including the ones whose schemas haven't changed. For users with many plugins this causes unnecessary IPC traffic on every status change.

Individual queries per plugin (keyed by `["plugin_config_get", { name }]` — the same key `PluginConfigDialog` uses) would let TanStack Query serve cached responses for unchanged plugins and only fetch the newly installed one.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (8): Last reviewed commit: "fix(plugin): validate persisted configs ..." | Re-trigger Greptile

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (2)
src/views/PluginsView/__tests__/PluginConfigField.test.tsx (1)

20-127: Cover the float and array branches too.

This suite exercises most of the new dispatcher, but the PR also introduces fieldType: "float" and fieldType: "array". Those paths currently have no regression coverage.

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

In `@src/views/PluginsView/__tests__/PluginConfigField.test.tsx` around lines 20 -
127, Add two tests to PluginConfigField.test.tsx that exercise the new "float"
and "array" branches: for fieldType: "float" (use makeField({ fieldType: "float"
})) render PluginConfigField and assert the labeled control is an
HTMLInputElement with type "number" and an appropriate step attribute (e.g.,
"any" or a numeric step) to allow decimals; for fieldType: "array" (use
makeField({ fieldType: "array" })) render PluginConfigField with an array value
and assert the component renders the expected array control (e.g., a textarea or
list of inputs) and that the displayed value matches the provided array
(serialized/visible), using the same patterns as the existing tests
(screen.getByLabelText / getByText) to locate controls.
src-tauri/src/domain/model/plugin.rs (1)

353-360: Array validation only checks brackets, not JSON validity.

The validation accepts strings like "[not, valid, json{" as long as they start with [ and end with ]. If strict JSON array validation is needed, consider parsing with serde_json.

This is acceptable if the consuming code (plugins) handles malformed arrays gracefully.

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

In `@src-tauri/src/domain/model/plugin.rs` around lines 353 - 360, The Array
branch in ConfigFieldType::Array currently only checks leading/trailing brackets
on value, which allows malformed JSON arrays; update the validation in the
ConfigFieldType::Array match arm (where value.trim() is used) to parse the
string with serde_json::from_str::<serde_json::Value>(trimmed) (or
from_str::<Vec<_>> if you want to enforce array type) and return a
DomainError::ValidationError including the serde_json parse error if parsing
fails, instead of only checking starts_with/ends_with.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Line 21: The changelog entry mixes French and English—replace all French terms
with English equivalents and ensure consistent wording: e.g., change "UI
dynamique" to "Dynamic UI", "plugins déclarant" to "plugins declaring", "input
texte" to "text input", and translate other fragments (keep all symbol names
unchanged: ConfigField / ConfigFieldType / PluginConfigSchema,
PluginConfigStore, SqlitePluginConfigRepo, migration
m20260425_000005_create_plugin_configs, adapters/driven/plugin/manifest.rs,
UpdatePluginConfigCommand, GetPluginConfigQuery,
PluginLoader.get_manifest()/set_runtime_config(), ExtismPluginLoader,
PluginRegistry, SharedHostResources::plugin_configs, lib.rs,
PluginConfigField.tsx, PluginConfigDialog.tsx, PluginsView, IPC commands
plugin_config_get/plugin_config_update, and i18n keys plugins.action.configure
and plugins.config.*) so the entire paragraph is in English and reads naturally.

In `@src-tauri/src/adapters/driven/plugin/manifest.rs`:
- Around line 166-189: The code currently copies entry.default into ConfigField
without validating it; update the block that handles entry.default (and where
options/min/max/regex are applied) to validate the encoded default produced by
encode_config_default(default)? against the declared field_type and constraints:
ensure its type matches field_type, that it is one of the encoded options if
entry.options is present, that numeric values respect entry.min and entry.max,
and that string values match entry.regex; if validation fails return an error
(or propagate one) instead of inserting into schema. Implement this either by
calling an existing validator (e.g. validate_config_value or similar) or by
adding a validate_default helper used before field.with_default(...), and keep
using ConfigField, encode_config_default, entry.options, entry.min/entry.max,
entry.regex, and schema.insert to locate the changes.

In
`@src-tauri/src/adapters/driven/sqlite/migrations/m20260425_000005_create_plugin_configs.rs`:
- Around line 11-14: The migration uses Table::create().if_not_exists() in the
up() for PluginConfigs::Table which masks unexpected pre-existing tables; remove
the if_not_exists() call so the create will error loudly if the table already
exists, and make the down() symmetric by adding an if_exists() guard to the
Table::drop() for PluginConfigs::Table so rollback is safe but not silent.
Locate the up() function and the Table::create() invocation for
PluginConfigs::Table and delete the if_not_exists() call, then locate the down()
function and add the corresponding if_exists() option to the Table::drop() call
for PluginConfigs::Table.

In `@src-tauri/src/application/commands/update_plugin_config.rs`:
- Around line 39-45: The code currently calls store.set_value(...) before
plugin_loader().set_runtime_config(...), which can leave persisted state ahead
of the live plugin; change the sequence or add atomic behavior: either (A)
implement a transactional adapter method on the store/loader that applies
runtime and persistent changes together, or (B) capture the previous stored
value via store.get_value(&cmd.plugin_name, &cmd.key), call
plugin_loader().set_runtime_config(&cmd.plugin_name, &cmd.key, &cmd.value)
first, and only on success call store.set_value(...); if the subsequent
store.set_value fails roll back the runtime by calling
plugin_loader().set_runtime_config(..., &previous_value) and map errors to
AppError::Domain so update_plugin_config returns a consistent state.

In `@src-tauri/src/application/queries/get_plugin_config.rs`:
- Around line 29-46: The handler currently trusts get_values(&query.plugin_name)
and only fills missing keys with field.default_value(), causing stale/invalid
persisted values to be returned; after fetching values, revalidate each
persisted entry against manifest.config_schema() (for each (key, field) in
manifest.config_schema().fields()) and if a value is missing or fails validation
(enum/regex/type constraints) replace it with field.default_value() when present
or remove/normalize it to a safe default, then pass the cleaned map into
values_to_view(...) and PluginConfigView::new(...) so the UI always sees
schema-valid values rather than stale invalid persisted ones.

In `@src-tauri/src/domain/model/plugin.rs`:
- Around line 320-325: The integer branch in ConfigFieldType::Integer parses
value into parsed: i64 and then casts to f64 before calling
self.check_numeric_bounds(parsed as f64), which can lose precision for large
integers; fix by avoiding the f64 cast—either add/use an integer-aware bounds
check (e.g., a new method check_numeric_bounds_i64 or extend
check_numeric_bounds to accept i64) that compares parsed directly against
integer min/max, or convert the stored min/max/max to i64 when they represent
integral bounds and then perform i64 comparisons; update the
ConfigFieldType::Integer branch to call this integer-safe check (referencing
parsed and check_numeric_bounds/check_numeric_bounds_i64) so large integers are
validated without precision loss.

In `@src/views/PluginsView/PluginConfigDialog.tsx`:
- Around line 39-45: The float validator in PluginConfigDialog (inside the
branch checking field.fieldType === "float") accepts empty strings because
Number("") === 0; update the validation to treat empty/whitespace-only input as
invalid before converting to Number: check that value (or value.trim()) is
non-empty and return "Must be a number" if it is empty, then proceed to parse
into n and perform the existing Number.isNaN and min/max checks (refer to the
variables field.fieldType, value, and n in that function).
- Around line 103-145: The useEffect that resets draft when `data` changes can
overwrite the in-memory draft mid-save because each `updateMutation.mutateAsync`
triggers `queryClient.invalidateQueries`; to fix, guard that effect so it does
nothing while saves are in flight: update the effect that resets `draft` (the
one reading `data`) to return early if `updateMutation.isPending` (or
`updateMutation.isLoading`) is true (and add
`updateMutation`/`updateMutation.isPending` to the effect deps), so
`handleSave`'s sequential `updateMutation.mutateAsync` calls won't have the
draft clobbered by a refetch.

In `@src/views/PluginsView/PluginConfigField.tsx`:
- Around line 46-55: The error element currently sets aria-describedby on itself
and the computed describedBy is never applied to the input control; remove
aria-describedby from the <p> error element (keep its id like
`${labelId}-error`) and pass the describedBy variable into the renderControl
call so the actual form control components (Switch, SelectTrigger, Input)
receive aria-describedby={describedBy}; update the renderControl
implementation/call sites to accept the describedBy prop and add
aria-describedby to those components so screen readers reference the error from
the input.

---

Nitpick comments:
In `@src-tauri/src/domain/model/plugin.rs`:
- Around line 353-360: The Array branch in ConfigFieldType::Array currently only
checks leading/trailing brackets on value, which allows malformed JSON arrays;
update the validation in the ConfigFieldType::Array match arm (where
value.trim() is used) to parse the string with
serde_json::from_str::<serde_json::Value>(trimmed) (or from_str::<Vec<_>> if you
want to enforce array type) and return a DomainError::ValidationError including
the serde_json parse error if parsing fails, instead of only checking
starts_with/ends_with.

In `@src/views/PluginsView/__tests__/PluginConfigField.test.tsx`:
- Around line 20-127: Add two tests to PluginConfigField.test.tsx that exercise
the new "float" and "array" branches: for fieldType: "float" (use makeField({
fieldType: "float" })) render PluginConfigField and assert the labeled control
is an HTMLInputElement with type "number" and an appropriate step attribute
(e.g., "any" or a numeric step) to allow decimals; for fieldType: "array" (use
makeField({ fieldType: "array" })) render PluginConfigField with an array value
and assert the component renders the expected array control (e.g., a textarea or
list of inputs) and that the displayed value matches the provided array
(serialized/visible), using the same patterns as the existing tests
(screen.getByLabelText / getByText) to locate controls.
🪄 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: f11eff2a-66fb-4d4f-b758-bdd0b9ef29b1

📥 Commits

Reviewing files that changed from the base of the PR and between 1f981dc and ee54277.

📒 Files selected for processing (32)
  • CHANGELOG.md
  • src-tauri/src/adapters/driven/plugin/extism_loader.rs
  • src-tauri/src/adapters/driven/plugin/manifest.rs
  • src-tauri/src/adapters/driven/plugin/registry.rs
  • src-tauri/src/adapters/driven/sqlite/entities/mod.rs
  • src-tauri/src/adapters/driven/sqlite/entities/plugin_config.rs
  • src-tauri/src/adapters/driven/sqlite/migrations/m20260425_000005_create_plugin_configs.rs
  • src-tauri/src/adapters/driven/sqlite/migrations/mod.rs
  • src-tauri/src/adapters/driven/sqlite/mod.rs
  • src-tauri/src/adapters/driven/sqlite/plugin_config_repo.rs
  • src-tauri/src/adapters/driving/tauri_ipc.rs
  • src-tauri/src/application/command_bus.rs
  • src-tauri/src/application/commands/mod.rs
  • src-tauri/src/application/commands/update_plugin_config.rs
  • src-tauri/src/application/queries/get_plugin_config.rs
  • src-tauri/src/application/queries/mod.rs
  • src-tauri/src/application/query_bus.rs
  • src-tauri/src/application/read_models/mod.rs
  • src-tauri/src/application/read_models/plugin_config_view.rs
  • src-tauri/src/domain/model/plugin.rs
  • src-tauri/src/domain/ports/driven/mod.rs
  • src-tauri/src/domain/ports/driven/plugin_config_store.rs
  • src-tauri/src/domain/ports/driven/plugin_loader.rs
  • src-tauri/src/lib.rs
  • src/i18n/locales/en.json
  • src/i18n/locales/fr.json
  • src/types/plugin-config.ts
  • src/views/PluginsView.tsx
  • src/views/PluginsView/PluginConfigDialog.tsx
  • src/views/PluginsView/PluginConfigField.tsx
  • src/views/PluginsView/PluginStoreRow.tsx
  • src/views/PluginsView/__tests__/PluginConfigField.test.tsx

Comment thread CHANGELOG.md Outdated
Comment thread src-tauri/src/adapters/driven/plugin/manifest.rs
Comment thread src-tauri/src/application/commands/update_plugin_config.rs Outdated
Comment thread src-tauri/src/application/queries/get_plugin_config.rs
Comment thread src-tauri/src/domain/model/plugin.rs
Comment thread src/views/PluginsView/PluginConfigDialog.tsx
Comment thread src/views/PluginsView/PluginConfigDialog.tsx Outdated
Comment thread src/views/PluginsView/PluginConfigField.tsx
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

6 issues found across 32 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src-tauri/src/application/queries/get_plugin_config.rs">

<violation number="1" location="src-tauri/src/application/queries/get_plugin_config.rs:76">
P3: This test module duplicates large shared query-test scaffolding instead of reusing `application/test_support.rs`, increasing maintenance cost and drift risk.</violation>
</file>

<file name="src/views/PluginsView.tsx">

<violation number="1" location="src/views/PluginsView.tsx:122">
P2: The config query key is tied to filtered UI results, so typing in search or changing category triggers unnecessary refetches and cache churn.</violation>
</file>

<file name="src/types/plugin-config.ts">

<violation number="1" location="src/types/plugin-config.ts:8">
P2: `array` is declared as a field type, but UI rendering/validation does not handle it, so array config values are treated as unvalidated plain text.</violation>
</file>

<file name="src-tauri/src/domain/model/plugin.rs">

<violation number="1" location="src-tauri/src/domain/model/plugin.rs:355">
P2: Array field validation is too weak: it accepts invalid JSON arrays by only checking `[` and `]`.</violation>

<violation number="2" location="src-tauri/src/domain/model/plugin.rs:576">
P2: Malformed character classes are not rejected, so invalid regex patterns can still match and pass validation.</violation>
</file>

<file name="src/views/PluginsView/PluginConfigDialog.tsx">

<violation number="1" location="src/views/PluginsView/PluginConfigDialog.tsx:34">
P2: Reject empty input before numeric coercion in float validation; otherwise an empty field is treated as `0` in the UI and can pass client-side checks incorrectly.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/views/PluginsView.tsx Outdated
| "float"
| "url"
| "enum"
| "array";
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 25, 2026

Choose a reason for hiding this comment

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

P2: array is declared as a field type, but UI rendering/validation does not handle it, so array config values are treated as unvalidated plain text.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/types/plugin-config.ts, line 8:

<comment>`array` is declared as a field type, but UI rendering/validation does not handle it, so array config values are treated as unvalidated plain text.</comment>

<file context>
@@ -0,0 +1,24 @@
+  | "float"
+  | "url"
+  | "enum"
+  | "array";
+
+export interface PluginConfigField {
</file context>
Fix with Cubic

Comment thread src-tauri/src/domain/model/plugin.rs
Comment thread src-tauri/src/domain/model/plugin.rs Outdated
Comment thread src/views/PluginsView/PluginConfigDialog.tsx
PluginLoader, PluginReadRepository, StatsRepository,
};

struct FakeArchiveExtractor;
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 25, 2026

Choose a reason for hiding this comment

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

P3: This test module duplicates large shared query-test scaffolding instead of reusing application/test_support.rs, increasing maintenance cost and drift risk.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/src/application/queries/get_plugin_config.rs, line 76:

<comment>This test module duplicates large shared query-test scaffolding instead of reusing `application/test_support.rs`, increasing maintenance cost and drift risk.</comment>

<file context>
@@ -0,0 +1,350 @@
+        PluginLoader, PluginReadRepository, StatsRepository,
+    };
+
+    struct FakeArchiveExtractor;
+    impl ArchiveExtractor for FakeArchiveExtractor {
+        fn detect_format(
</file context>
Fix with Cubic

- Translate task 15 changelog entry from FR to EN
- Validate plugin manifest defaults against their declared schema
- Drop if_not_exists on plugin_configs migration; add if_exists on rollback
- Apply runtime config first, persist second, roll back runtime on persist failure
- Drop persisted config values that no longer match the schema before returning
- Use i64 bounds for integer config validation (avoid f64 precision loss)
- Validate JSON array structure (balanced brackets, escaped strings) instead of just '[' and ']'
- Reject unterminated regex character classes by returning a never-matching empty class
- Reject empty input on the float validator instead of treating it as 0
- Skip schema-reset effect while a save is in flight to avoid clobbering the draft
- Validate JSON arrays on the frontend before submitting
- Move aria-describedby from the error <p> to the input control (correct a11y semantics)
- Key the per-plugin config query off the unfiltered installed list so search/category churn doesn't refetch
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src-tauri/src/domain/model/plugin.rs (1)

532-593: Consider adding recursion depth limit.

The recursive regex_match_from function could stack overflow on deeply nested patterns with many */+ quantifiers. While plugin config regexes are typically simple, a malicious manifest could trigger this.

💡 Optional: Add depth parameter
-fn regex_match_from(pat: &[char], val: &[char], start: usize, anchor_end: bool) -> bool {
+fn regex_match_from(pat: &[char], val: &[char], start: usize, anchor_end: bool) -> bool {
+    regex_match_from_depth(pat, val, start, anchor_end, 0)
+}
+
+fn regex_match_from_depth(pat: &[char], val: &[char], start: usize, anchor_end: bool, depth: usize) -> bool {
+    if depth > 100 {
+        return false; // Bail on deeply nested patterns
+    }
     // ... rest of implementation, passing depth + 1 to recursive calls
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src-tauri/src/domain/model/plugin.rs` around lines 532 - 593, The recursive
regex_match_from can overflow on deeply nested/ambiguous patterns; add a
recursion depth parameter (e.g., depth: usize) with a sensible max constant
(e.g., MAX_REGEX_RECURSION_DEPTH) and check at the start of regex_match_from to
return false (or an error sentinel) when depth >= MAX_REGEX_RECURSION_DEPTH, and
then pass depth + 1 on every recursive call (calls inside the '*', '+', '?'
branches and any other recursive invocation); update any callers to start with
depth = 0 and adjust unit tests/uses accordingly so the function bounds
recursion safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src-tauri/src/application/commands/update_plugin_config.rs`:
- Around line 55-62: When store.set_value(&cmd.plugin_name, &cmd.key,
&cmd.value) fails we currently only roll back to a previous value if Some(prev)
exists, leaving the new value in the runtime when persistence failed; change the
error path so that if previous is None you remove the runtime config entry
instead of leaving it set by calling the PluginLoader removal API (e.g.
plugin_loader().remove_runtime_config(&cmd.plugin_name, &cmd.key)) before
returning Err(AppError::Domain(e)); if PluginLoader does not expose a removal
method, add one or document this as a known limitation and ensure the code
attempts removal where supported.

---

Nitpick comments:
In `@src-tauri/src/domain/model/plugin.rs`:
- Around line 532-593: The recursive regex_match_from can overflow on deeply
nested/ambiguous patterns; add a recursion depth parameter (e.g., depth: usize)
with a sensible max constant (e.g., MAX_REGEX_RECURSION_DEPTH) and check at the
start of regex_match_from to return false (or an error sentinel) when depth >=
MAX_REGEX_RECURSION_DEPTH, and then pass depth + 1 on every recursive call
(calls inside the '*', '+', '?' branches and any other recursive invocation);
update any callers to start with depth = 0 and adjust unit tests/uses
accordingly so the function bounds recursion safely.
🪄 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: 2bc9a08f-887b-4634-a01b-41a28a3e04f0

📥 Commits

Reviewing files that changed from the base of the PR and between ee54277 and e9b0515.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • src-tauri/src/adapters/driven/plugin/manifest.rs
  • src-tauri/src/adapters/driven/sqlite/migrations/m20260425_000005_create_plugin_configs.rs
  • src-tauri/src/application/commands/update_plugin_config.rs
  • src-tauri/src/application/queries/get_plugin_config.rs
  • src-tauri/src/domain/model/plugin.rs
  • src/views/PluginsView.tsx
  • src/views/PluginsView/PluginConfigDialog.tsx
  • src/views/PluginsView/PluginConfigField.tsx
✅ Files skipped from review due to trivial changes (1)
  • src-tauri/src/adapters/driven/plugin/manifest.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • src-tauri/src/adapters/driven/sqlite/migrations/m20260425_000005_create_plugin_configs.rs
  • src-tauri/src/application/queries/get_plugin_config.rs
  • CHANGELOG.md

Comment on lines +55 to +62
if let Err(e) = store.set_value(&cmd.plugin_name, &cmd.key, &cmd.value) {
if let Some(prev) = previous {
let _ = self
.plugin_loader()
.set_runtime_config(&cmd.plugin_name, &cmd.key, &prev);
}
return Err(AppError::Domain(e));
}
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

Incomplete rollback when no previous value exists.

If this is the first time a key is being set (no previous value) and persistence fails, the runtime will have the new value while disk does not. Consider removing the runtime config entry in this case.

🔧 Proposed fix
         if let Err(e) = store.set_value(&cmd.plugin_name, &cmd.key, &cmd.value) {
             if let Some(prev) = previous {
                 let _ = self
                     .plugin_loader()
                     .set_runtime_config(&cmd.plugin_name, &cmd.key, &prev);
+            } else {
+                // No previous value existed; ideally remove the key from runtime.
+                // If PluginLoader doesn't support removal, this is a known limitation.
             }
             return Err(AppError::Domain(e));
         }

Note: This requires PluginLoader to support removing a runtime config key, which may not be implemented. If not, document this as a known limitation.

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

In `@src-tauri/src/application/commands/update_plugin_config.rs` around lines 55 -
62, When store.set_value(&cmd.plugin_name, &cmd.key, &cmd.value) fails we
currently only roll back to a previous value if Some(prev) exists, leaving the
new value in the runtime when persistence failed; change the error path so that
if previous is None you remove the runtime config entry instead of leaving it
set by calling the PluginLoader removal API (e.g.
plugin_loader().remove_runtime_config(&cmd.plugin_name, &cmd.key)) before
returning Err(AppError::Domain(e)); if PluginLoader does not expose a removal
method, add one or document this as a known limitation and ensure the code
attempts removal where supported.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 9 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/views/PluginsView/PluginConfigDialog.tsx">

<violation number="1" location="src/views/PluginsView/PluginConfigDialog.tsx:115">
P2: Including `updateMutation.isPending` in this reset effect causes draft values to be overwritten when a mutation settles, which can discard user edits after a failed save.</violation>
</file>

<file name="src-tauri/src/application/commands/update_plugin_config.rs">

<violation number="1" location="src-tauri/src/application/commands/update_plugin_config.rs:43">
P2: Rollback only uses persisted values, so a failed save can leave runtime config mutated when the prior effective value came from manifest defaults.</violation>
</file>

<file name="src-tauri/src/domain/model/plugin.rs">

<violation number="1" location="src-tauri/src/domain/model/plugin.rs:324">
P2: Integer bound checks truncate `min`/`max` to `i64`, which can incorrectly accept out-of-range values when bounds are fractional.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/views/PluginsView/PluginConfigDialog.tsx Outdated
Comment thread src-tauri/src/application/commands/update_plugin_config.rs Outdated
Comment thread src-tauri/src/domain/model/plugin.rs
- Use ceil/floor on integer bound checks so fractional min/max (e.g. min=1.5) reject the integer 1 instead of widening the range via i64 truncation
- Roll back runtime config to the manifest default when no persisted value exists, so a failed save no longer leaves the runtime mutated past the prior effective value
- Replace the isPending-based draft reset with a one-shot ref keyed on pluginName, so a failed save no longer overwrites the user's edits when the mutation settles; collapse the per-iteration invalidate into a single invalidate after the save loop
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src-tauri/src/domain/model/plugin.rs (1)

801-819: Add a fractional-bound regression case here.

This test still only covers integer min/max, so it won't catch the ceil/floor behavior that motivated the latest fix. A case like min = 1.5 rejecting "1" and accepting "2" would lock that in.

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

In `@src-tauri/src/domain/model/plugin.rs` around lines 801 - 819, Extend the
existing test_config_field_validate_integer_parses_and_checks_bounds test to
include a fractional-bound regression case: create a
ConfigField::new(ConfigFieldType::Integer) with with_min(1.5) (and optionally
with_max(2.5)), then assert that f.validate("1") returns a ValidationError
(rejected), f.validate("2") is Ok (accepted), and if you add max 2.5 assert
f.validate("3") returns a ValidationError while "2" remains Ok; use the same
f.validate(...) and DomainError::ValidationError pattern used in the existing
assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src-tauri/src/domain/model/plugin.rs`:
- Around line 371-377: The Array branch in ConfigFieldType::Array currently
trusts is_json_array_structure and accepts malformed arrays and never enforces
min/max element counts; update the validation in the ConfigFieldType::Array
handling (the match arm using is_json_array_structure and returning
DomainError::ValidationError) to parse `value` with a strict JSON parser (e.g.,
serde_json::from_str::<Vec<...>>) and reject parse errors, then apply the
field's min/max element constraints to the parsed Vec length and return a
ValidationError with a clear message if out of range; ensure the same rigorous
parsing-and-count-check logic is applied to the other Array handling locations
noted (lines ~451-499).
- Around line 501-649: The current mini-regex in
match_regex/regex_match_from/parse_atom/Atom silently treats
group/alternation/counted-quantifier syntax as literals; update parse_atom (or
add a validate_pattern function called from match_regex) to detect and reject
unsupported tokens like '(', ')', '|', '{', '}', and other constructs you don't
implement (returning a clear malformed result/false or an explicit validation
error) so manifests using real regex features fail early, or alternatively move
matching out of this domain layer into a higher layer that can depend on a full
regex engine and replace calls to match_regex with that engine there.

---

Nitpick comments:
In `@src-tauri/src/domain/model/plugin.rs`:
- Around line 801-819: Extend the existing
test_config_field_validate_integer_parses_and_checks_bounds test to include a
fractional-bound regression case: create a
ConfigField::new(ConfigFieldType::Integer) with with_min(1.5) (and optionally
with_max(2.5)), then assert that f.validate("1") returns a ValidationError
(rejected), f.validate("2") is Ok (accepted), and if you add max 2.5 assert
f.validate("3") returns a ValidationError while "2" remains Ok; use the same
f.validate(...) and DomainError::ValidationError pattern used in the existing
assertions.
🪄 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: 93e2050a-ba55-40d3-afc0-047aa6321984

📥 Commits

Reviewing files that changed from the base of the PR and between e9b0515 and 1426cf2.

📒 Files selected for processing (3)
  • src-tauri/src/application/commands/update_plugin_config.rs
  • src-tauri/src/domain/model/plugin.rs
  • src/views/PluginsView/PluginConfigDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/views/PluginsView/PluginConfigDialog.tsx
  • src-tauri/src/application/commands/update_plugin_config.rs

Comment thread src-tauri/src/domain/model/plugin.rs
Comment thread src-tauri/src/domain/model/plugin.rs
- Replace the structural array validator with a strict recursive-descent JSON parser so [1 2], [1,], [tru] now reject; apply min/max to the array element count
- Reject regex patterns with unsupported features (alternation, groups, counted quantifiers) at manifest parse time and from validate(), so plugin authors get a clear error instead of a silent match failure
- Add tests covering the new array parser, array count bounds, regex feature detector, and the manifest parser rejecting unsupported regex patterns
Comment thread src/views/PluginsView/PluginConfigDialog.tsx
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

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

Inline comments:
In `@src-tauri/src/domain/model/plugin.rs`:
- Around line 392-399: The current check using unsupported_regex_feature()
misses malformed patterns (e.g., unclosed '[' or trailing '\'), so update the
regex-handling in the code path that sees self.regex (inside the block using
unsupported_regex_feature and match_regex in plugin.rs) to attempt a syntax
validation by compiling the pattern (e.g., try Regex::new(pattern)) and if
compilation fails return a DomainError::ValidationError that includes the
compilation error message; alternatively perform the same compile-and-fail-fast
validation during manifest parsing in build_config_schema (adapter driven plugin
manifest parsing) so invalid manifests are rejected up-front rather than later
producing generic “does not match regex” errors.
- Around line 595-665: parse_string currently accepts any escaped char and
control characters; update parse_string to validate JSON string escapes: allow
only the standard escapes (\" \\ \/ \b \f \n \r \t) and \u followed by exactly
four hex digits, and reject raw control characters (U+0000..U+001F); also ensure
it fails if EOF is reached before the closing quote. For parse_number, enforce
JSON numeric syntax: disallow leading zeros for integers (only "0" is valid),
require at least one digit in integer and fractional/exponent parts as already
done, and reject a leading plus sign; keep using the same position updates and
return semantics. Use the existing function names parse_string and parse_number
to locate where to add these validations.
- Around line 344-349: The ConfigFieldType::Float branch currently parses floats
but allows NaN/inf through; after parsing the f64 in the ConfigFieldType::Float
match arm (the parsed variable), explicitly reject non-finite values by checking
parsed.is_finite() and returning a DomainError::ValidationError with a clear
message (e.g. "expected finite float, got '{value}'") if it is not finite,
before calling self.check_numeric_bounds(parsed)?; this uses the existing
DomainError::ValidationError and check_numeric_bounds symbols to locate where to
add the is_finite() guard.
🪄 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: 45db62fb-ac57-4c09-bf96-ce777090c9ef

📥 Commits

Reviewing files that changed from the base of the PR and between 1426cf2 and 1a9f230.

📒 Files selected for processing (2)
  • src-tauri/src/adapters/driven/plugin/manifest.rs
  • src-tauri/src/domain/model/plugin.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src-tauri/src/adapters/driven/plugin/manifest.rs

Comment thread src-tauri/src/domain/model/plugin.rs
Comment thread src-tauri/src/domain/model/plugin.rs
Comment thread src-tauri/src/domain/model/plugin.rs
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src-tauri/src/domain/model/plugin.rs">

<violation number="1" location="src-tauri/src/domain/model/plugin.rs:602">
P2: String escape handling is too permissive, so malformed JSON arrays can pass validation as valid.</violation>

<violation number="2" location="src-tauri/src/domain/model/plugin.rs:623">
P2: Number parsing allows leading-zero integers, which makes malformed JSON pass array validation.</violation>

<violation number="3" location="src-tauri/src/domain/model/plugin.rs:675">
P2: Add regex syntax validation during schema construction; the current unsupported-feature scan allows malformed patterns (such as an unclosed `[` or trailing `\`) to be accepted and then fail all runtime matches.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src-tauri/src/domain/model/plugin.rs
Comment thread src-tauri/src/domain/model/plugin.rs Outdated
Comment thread src-tauri/src/domain/model/plugin.rs Outdated
- JSON parser now rejects invalid string escapes (\q etc.) and unescaped control characters per RFC 8259
- JSON parser now rejects leading-zero integers ([01], [001], [-01]) per RFC 8259
- Reject NaN, +/-Infinity in float validation: f64::from_str accepts them and NaN compares false against any bound, silently bypassing min/max
- Reject malformed regex patterns (unclosed [, trailing \) at manifest parse time and from validate(), so plugin authors get a clear error instead of silent never-matches
- Frontend float validator uses Number.isFinite to reject 'Infinity' inline, matching the backend's rejection
- Tests for invalid escapes, leading zeros, NaN/inf rejection, malformed-regex detection, and the manifest parser rejecting malformed patterns
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/views/PluginsView/PluginConfigDialog.tsx (2)

32-37: Integer validation accepts values outside safe integer range.

The regex /^-?\d+$/ accepts arbitrarily large integers, but Number(value) loses precision for integers outside ±2⁵³. This can cause the bounds check to behave incorrectly for very large values. The backend uses i64::parse() which has a different range.

This is unlikely to matter for typical plugin configs, but worth noting for completeness.

♻️ Optional: Add BigInt check for precision safety
   if (field.fieldType === "integer") {
     if (!/^-?\d+$/.test(value)) return "Must be an integer";
     const n = Number(value);
+    // Check if value exceeds safe integer range
+    if (!Number.isSafeInteger(n)) return "Integer too large";
     if (field.min !== null && n < field.min) return `Min ${field.min}`;
     if (field.max !== null && n > field.max) return `Max ${field.max}`;
     return null;
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/views/PluginsView/PluginConfigDialog.tsx` around lines 32 - 37, The
integer validation currently uses Number(value) which loses precision for values
outside the safe integer range; change the check to parse with BigInt to
preserve precision: try to construct BigInt(value) (catch and return "Must be an
integer" on error), then compare the BigInt against bounds by converting
field.min/field.max to BigInt (or if those can be null leave comparisons
conditional) instead of using Number(value); optionally also clamp/validate
against the backend i64 range by comparing to BigInt("-9223372036854775808") and
BigInt("9223372036854775807") so the behavior matches i64::parse(), all inside
the same block that tests field.fieldType === "integer" and using value,
field.min, field.max.

59-67: Frontend array validation is more lenient than backend.

The frontend uses JSON.parse() which accepts payloads like [01] (leading zeros) and ["\q"] (invalid escapes) that the backend's strict RFC 8259 parser rejects. This mismatch means the UI shows no error, but the backend returns a validation failure toast.

Consider using stricter validation or accepting the backend as authoritative (current behavior is acceptable since backend validates).

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

In `@src/views/PluginsView/PluginConfigDialog.tsx` around lines 59 - 67, The
current client-side array validation in PluginConfigDialog.tsx (inside the block
checking field.fieldType === "array") uses JSON.parse which accepts non-RFC8259
payloads (see JSON.parse usage and parsed variable); replace this with a
stricter RFC 8259-compliant check or remove the eager parse and defer to backend
validation: either integrate a strict JSON parser/validator library that
enforces RFC8259 for the value before accepting it, or change the validation
path to only check for non-empty input and let the backend return the
authoritative error (update the error messages accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/views/PluginsView/PluginConfigDialog.tsx`:
- Around line 32-37: The integer validation currently uses Number(value) which
loses precision for values outside the safe integer range; change the check to
parse with BigInt to preserve precision: try to construct BigInt(value) (catch
and return "Must be an integer" on error), then compare the BigInt against
bounds by converting field.min/field.max to BigInt (or if those can be null
leave comparisons conditional) instead of using Number(value); optionally also
clamp/validate against the backend i64 range by comparing to
BigInt("-9223372036854775808") and BigInt("9223372036854775807") so the behavior
matches i64::parse(), all inside the same block that tests field.fieldType ===
"integer" and using value, field.min, field.max.
- Around line 59-67: The current client-side array validation in
PluginConfigDialog.tsx (inside the block checking field.fieldType === "array")
uses JSON.parse which accepts non-RFC8259 payloads (see JSON.parse usage and
parsed variable); replace this with a stricter RFC 8259-compliant check or
remove the eager parse and defer to backend validation: either integrate a
strict JSON parser/validator library that enforces RFC8259 for the value before
accepting it, or change the validation path to only check for non-empty input
and let the backend return the authoritative error (update the error messages
accordingly).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1218f64e-eb11-4bc0-b2af-4f23d317f1ca

📥 Commits

Reviewing files that changed from the base of the PR and between 1a9f230 and 3476743.

📒 Files selected for processing (3)
  • src-tauri/src/adapters/driven/plugin/manifest.rs
  • src-tauri/src/domain/model/plugin.rs
  • src/views/PluginsView/PluginConfigDialog.tsx

Comment thread src/views/PluginsView/PluginConfigDialog.tsx
Backend already bounds array length, but the dialog only validated the JSON shape, so a schema declaring min=1 still let the user save []  and surfaced a generic backend toast instead of an inline field error.
Comment thread src-tauri/src/lib.rs
… replay

Replay now runs after plugin loading so each persisted (key, value) is checked against the current schema before being inserted into the runtime map. Entries that no longer match (e.g. after a plugin update narrowed an enum, tightened a regex, or dropped a key) are skipped with a warning, so WASM plugins never observe a stale schema-invalid value via get_config.
Comment on lines +788 to +792
let (anchor_end, body) = if body.last() == Some(&'$') {
(true, &body[..body.len() - 1])
} else {
(false, body)
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 End-anchor stripping does not check for an escaped \$

body.last() == Some(&'$') strips the $ as a positional anchor without verifying that the preceding character is not a backslash. A pattern like ^[A-Z]+\$ ends with the two characters \ and $. The anchor-end check fires, anchor_end becomes true, and the body shrinks to [A-Z]+\ — meaning the backslash becomes a dangling escape, causing match_regex to return incorrect results for any value.

let (anchor_end, body) = {
    let escaped = body.len() >= 2 && body[body.len() - 2] == '\\';
    if !escaped && body.last() == Some(&'$') {
        (true, &body[..body.len() - 1])
    } else {
        (false, body)
    }
};
Prompt To Fix With AI
This is a comment left during a code review.
Path: src-tauri/src/domain/model/plugin.rs
Line: 788-792

Comment:
**End-anchor stripping does not check for an escaped `\$`**

`body.last() == Some(&'$')` strips the `$` as a positional anchor without verifying that the preceding character is not a backslash. A pattern like `^[A-Z]+\$` ends with the two characters `\` and `$`. The anchor-end check fires, `anchor_end` becomes `true`, and the body shrinks to `[A-Z]+\` — meaning the backslash becomes a dangling escape, causing `match_regex` to return incorrect results for any value.

```rust
let (anchor_end, body) = {
    let escaped = body.len() >= 2 && body[body.len() - 2] == '\\';
    if !escaped && body.last() == Some(&'$') {
        (true, &body[..body.len() - 1])
    } else {
        (false, body)
    }
};
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src-tauri/src/lib.rs">

<violation number="1" location="src-tauri/src/lib.rs:203">
P2: Skipping replay for not-yet-loaded plugins drops their persisted overrides for later runtime loads, so those plugins fall back to manifest defaults in the current session.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src-tauri/src/lib.rs Outdated
Move the schema-validation pass from lib.rs into build_host_functions so it runs every time a plugin loads (startup or hot-load via the file watcher). Replay in lib.rs is back to inserting raw values, so plugins that load later in the session keep their persisted overrides instead of falling back to manifest defaults.
@mpiton mpiton merged commit 60f85b3 into main Apr 26, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation frontend rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant