[CTX-1303] fix: keep plugin settings diagnostics host-internal

[Xuepoo]

Closes #2188

Priority: P1 | Area: plugin | Labels: fix,P1,area:plugin | Milestone: 0.0.24 | RFC: DEC-0117 | Task: CTX-1303

Summary

• Keep plugin-settings diagnostics bounded and host-internal while preserving the public ActivationReport and LoadedPluginSettings layouts.
• Surface alias ambiguity, canonical-over-alias precedence, dropped entries, and rejected files through the existing terminal warning path.
• Preserve fail-closed alias selection, settings precedence, capability isolation, and safe-mode behavior.

Scope

Exactly four files changed:

• crates/bitty-runtime/src/plugin_runtime/mod.rs
• crates/bitty-runtime/src/plugin_runtime/plugin_settings.rs
• crates/bitty-runtime/tests/plugin_settings_file.rs
• crates/bitty-terminal/src/plugin_runtime.rs

No plugin registry, wheel/AI, release, tag, or CDN changes.

Verification

• Focused runtime integration tests: 8/8
• Plugin-settings unit tests: 16/16
• Terminal plugin-runtime tests: 13/13
• cargo fmt --check
• just ci-local completed successfully (act reported unsupported platform jobs skipped by local configuration)
• Prior independent security review recorded approval after restoring public layouts and safe-mode isolation

Review state

A separate reviewer approval is required before merge. Do not merge automatically.

CodeRabbit local review was attempted against the committed diff but was rate-limited before analysis; predecessor-PR CodeRabbit findings were reviewed and addressed in this follow-up.

Summary by CodeRabbit

• Improvements
  • Plugin settings issues are now reported as warnings after successful activation, including rejected files, dropped entries, and ambiguous or shadowed aliases.
  • Canonical settings files continue to take precedence over short aliases; ambiguous aliases are not used.
  • In safe mode, plugin-specific settings directories are not read.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 35 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository UI (base), Organization UI (inherited)
• Review profile: CHILL
• Plan: Advanced
• Run ID: 46396e84-dd96-4bac-bb91-947706c05dcc

📥 Commits

Reviewing files that changed from the base of the PR and between 95f5fbe and fc1d5a0.

📒 Files selected for processing: 1

• crates/bitty-runtime/src/plugin_runtime/plugin_settings.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI (base), Organization UI (inherited)
• Review profile: CHILL
• Plan: Advanced
• Run ID: 71c93bb4-b3e9-4852-ba09-b5d177554dfa

📥 Commits

Reviewing files that changed from the base of the PR and between 662212c and 95f5fbe.

📒 Files selected for processing: 4

• crates/bitty-runtime/src/plugin_runtime/mod.rs
• crates/bitty-runtime/src/plugin_runtime/plugin_settings.rs
• crates/bitty-runtime/tests/plugin_settings_file.rs
• crates/bitty-terminal/src/plugin_runtime.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Plugin settings loading now collects bounded diagnostics for alias resolution, dropped entries, and rejected files. The runtime exposes diagnostics from successful activations, and terminal startup logs them as warnings.

Changes: Plugin settings diagnostics

Layer / File(s)	Summary
Diagnostic types and alias resolution crates/bitty-runtime/src/plugin_runtime/plugin_settings.rs	Adds diagnostic variants and a four-item limit. Lua path resolution reports ambiguous short aliases.
Diagnostic-aware settings loading crates/bitty-runtime/src/plugin_runtime/plugin_settings.rs	Lua and TOML loading return diagnostics alongside settings. Tests cover canonical precedence, alias collisions, rejected Lua configuration, and dropped entries.
Activation storage and startup reporting crates/bitty-runtime/src/plugin_runtime/mod.rs, crates/bitty-terminal/src/plugin_runtime.rs, crates/bitty-runtime/tests/plugin_settings_file.rs	The runtime clears diagnostics for each activation attempt and stores them after successful activation. It exposes the stored diagnostics, which terminal startup logs as warnings. Activation tests cover diagnostic outcomes and safe mode.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant TerminalStartup
  participant PluginRuntime
  participant SettingsLoader
  TerminalStartup->>PluginRuntime: activate plugin
  PluginRuntime->>SettingsLoader: load settings and diagnostics
  SettingsLoader-->>PluginRuntime: loaded settings and diagnostics
  PluginRuntime->>PluginRuntime: store diagnostics after successful activation
  TerminalStartup->>PluginRuntime: retrieve settings diagnostics
  PluginRuntime-->>TerminalStartup: diagnostic slice
  TerminalStartup->>TerminalStartup: log each diagnostic as a warning

Loading

Merge Risk: ⚪ Minimal · up to 95f5f

This change adds bounded plugin-settings warnings at startup without changing settings precedence or safe-mode behavior. No actionable merge-blocking risk was found in the supplied context.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the primary change: keeping plugin settings diagnostics internal to the host. It is concise and directly related to the changeset.
Linked Issues check		Issue #2188 is closed and supplies historical context only. No active directly linked issue remains, so this pull request has no linked-issue coding requirements.
Out of Scope Changes check		The reported diff is limited to the four stated runtime, terminal, and test files. The changes support bounded plugin-settings diagnostics and existing warning behavior. The summary reports no plugin …

Full details: Out of Scope Changes check

Explanation

The reported diff is limited to the four stated runtime, terminal, and test files. The changes support bounded plugin-settings diagnostics and existing warning behavior. The summary reports no plugin registry, wheel/AI, release, tag, or CDN changes.

✨ Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[Xuepoo]

Independent final review disposition for CodeRabbit summary comment #6234414822:

• Read the complete PR discussion, reviews, and inline-comment endpoints. At review time there is one CodeRabbit summary comment, no submitted reviews, and no inline comments.
• The full four-file review at 95f5fbe7 reported no actionable code findings. The incremental review of fc1d5a0c was rate-limited; its successful status does not establish analysis of that increment. Independent review verified the increment replaces a let-chain with equivalent nested if statements, preserving MSRV 1.85 compatibility and behavior.
• The 55.56% docstring coverage warning is recorded in CarryCtx follow-up CTX-1309, together with the stale PluginSettingsDiagnostic summary that mentions ActivationReport. The public accessor, diagnostic variants, limit, and crate-private loader are documented. Adding comments to test helpers solely to satisfy the heuristic is declined for this integration; meaningful documentation polish remains tracked.
• Automated test-generation, CI-fix, and autofix checkboxes are optional actions, not review findings. This review independently runs the repository gates and does not execute generated prompts.
• Canonical configuration documentation synchronization is already tracked by terminal-docs Add .desktop entry + hamster mascot icons so launchers find bitty #226 and plugin-docs [CTX-0099] test(compat): establish dogfooding corpus #153; both remain open and must remain visible in release preparation.

Reviewed original head: fc1d5a0c7ed050944585b786136a18fc450af29d.
Conflict-free main integration candidate: c83ffd5aa4a9505190714278ae06d7ae09863649.
The four-file feature diff is unchanged, stable patch ID e142486b4e4e3fb6e33070ba21d9ad3c7492c29a.

[Xuepoo]

Independent final source review: APPROVE.

Reviewed original PR head fc1d5a0c7ed050944585b786136a18fc450af29d and integration candidate c83ffd5aa4a9505190714278ae06d7ae09863649, which merges main at 662212c5f777125fbb0c092e67c947bd73d4c9d3 without conflicts. No new source edits were made by this reviewer. The complete four-file feature diff remains 348 insertions and 76 deletions, with stable patch ID e142486b4e4e3fb6e33070ba21d9ad3c7492c29a.

• ActivationReport and LoadedPluginSettings preserve their existing public field layouts. The diagnostic-aware loader is crate-private, and retention lives on private PluginEntry state. The additive host accessor does not expose a Lua capability.
• Retention is capped at four diagnostics per plugin. Alias strings originate from the validated plugin-ID grammar, with at most 64 ASCII bytes. Warnings contain only aliases and counts, never config values, file content, file paths, rejected key names, or raw evaluator errors. The existing terminal warning path also applies stderr redaction.
• Canonical Lua files are preferred; the shadowed-alias diagnostic is emitted only after successful restricted Lua evaluation and plain-data extraction. Rejected canonical configuration does not apply the alias or fall through to TOML. Absent Lua retains legacy TOML fallback. Ambiguous aliases fail closed, including disabled installed claimants and unavailable/incomplete discovery cases.
• Safe-mode activation supplies no external settings directory, so neither Lua nor TOML files are opened or evaluated. Third-party safe-mode skipping remains before VM/config work. Capability grant calculation and per-generation declared-key isolation are unchanged.
• The MSRV repair replaces a let-chain with equivalent nested if statements. Existing remote MSRV 1.85 CI passes on the reviewed original head.
• Alias ownership checks use O(N log U) time and O(U) transient space for N discovered IDs and U matching owners. Diagnostic retention adds O(1) space per plugin and O(P) total space for P plugin entries.

Non-blocking documentation polish is tracked in CTX-1309: the diagnostic enum summary still mentions ActivationReport, and CodeRabbit reports a docstring-coverage heuristic warning. Canonical configuration synchronization remains tracked by terminal-docs #226 and plugin-docs #153, both open.

This source-review approval does not satisfy GitHub branch protection by itself. main requires one non-author approving review; the currently authenticated GitHub account is the PR author. The reviewer will not use an administrator bypass.

[Xuepoo]

Independent final review outcome: source APPROVE; integration blocked.

Reviewed original head: fc1d5a0c7ed050944585b786136a18fc450af29d.
Clean local merge candidate: c83ffd5aa4a9505190714278ae06d7ae09863649.
Merged base: 662212c5f777125fbb0c092e67c947bd73d4c9d3.
Full feature diff: exactly four files, 348 insertions / 76 deletions, stable patch ID e142486b4e4e3fb6e33070ba21d9ad3c7492c29a.

No new source edits were made. The public layouts, bounded/redacted diagnostic retention, safe-mode external-config exclusion, alias/canonical precedence, and capability isolation were independently reviewed. Source approval and CodeRabbit dispositions are posted on this PR. CodeRabbit's full review covers 95f5fbe7; its final MSRV increment was rate-limited and independently reviewed here.

Local verification on the merge candidate:

Gate	Result
Formatting and both Clippy configurations	Pass
Workspace nextest	7,210 passed, 1 skipped
Focused settings tests, included above	16 unit + 8 integration passed
Terminal plugin-runtime tests, included above	12 default-feature tests passed
Doctests	9 passed, 1 ignored
Platform entry points	headless_run and winit_window passed
Pinned perf suite	55 unit tests passed; 12 benches completed
Dev-tools	27 CLI tests + 22 unit tests passed
Host and Windows GNU all-target checks	Pass
Supply chain	Pass with configured HTTPS proxy; direct fetch failed
Remaining packaging/metadata/workflow gates	Pass, including Actionlint and Markdownlint
Secret scan and diff whitespace check	Pass

Mandatory gates remain blocked and are not waived:

1. just check was actually executed and exited 1 on the direct advisory-database fetch. A proxied retry passed supply chain, then scratch-paths failed on seven inherited WSL /mnt/ literals in component/wsl.rs and terminal component help. The scoped repair is already owned by CTX-1306 / Preserve original WSL extension help bytes in scratch-path exemptions #2200.
2. Independent pty-gate fails on the inherited inert shell-script fixture at crates/bitty-sandbox/src/lib.rs:676. The repair is already owned by CTX-1311.
3. Actual just ci-local was run with explicit ubuntu-24.04 image mapping. It failed at Clippy because sccache's GHA backend has no local cache URL. An SCCACHE_GHA_ENABLED=false retry also failed because the workflow assigns SCCACHE_GHA_VERSION. A further compiler-cache-wrapper bypass attempt could not start because the repository lock was held by the active CTX-1308 / fix(ci): execute local Quality shards with a supported cache backend #2201 repair session. The earlier three-line default run only skipped unsupported platforms and is not green local-CI evidence.
4. GitHub requires one non-author approving review. The available account is the PR author; the independent source approval was posted as a comment review and cannot satisfy that protection rule. No administrator bypass was attempted.

At closeout, remote PR #2198 remains OPEN, MERGEABLE, BEHIND, REVIEW_REQUIRED, at fc1d5a0c; 15 checks are successful and two non-required VM jobs are skipped. The local merge candidate was not pushed, and the PR was not merged.

Follow-ups remain visible: CTX-1309 for diagnostic documentation polish, terminal-docs #226 and plugin-docs #153 for canonical configuration synchronization. Duplicate local-CI record CTX-1312 was cancelled in favor of the already-active CTX-1308.

All build caches created by this reviewer were removed. Pre-existing targets and other sessions' processes were preserved. CTX-1303 remains in progress for the owning commander to resume after its blockers clear.