Skip to content

fix(reconciler): suppress false-positive stale-config banner on fresh installs - #351

Merged
ChuckBuilds merged 1 commit into
mainfrom
fix/reconciler-false-positive-system-keys
May 25, 2026
Merged

fix(reconciler): suppress false-positive stale-config banner on fresh installs#351
ChuckBuilds merged 1 commit into
mainfrom
fix/reconciler-false-positive-system-keys

Conversation

@ChuckBuilds

@ChuckBuilds ChuckBuilds commented May 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • config_manager.load_config() deep-merges config_secrets.json into the main config before returning. This causes secrets top-level keys (github, youtube) to appear alongside structural config keys (sync) in the dict that _get_config_state() iterates.
  • _SYSTEM_CONFIG_KEYS was missing all three, so the reconciler flagged them as PLUGIN_MISSING_ON_DISK on every startup — showing the "Stale plugin config entries found" warning banner to users with a fresh install where those plugins have never existed.
  • Found via a fresh install on devpi: the banner appeared immediately after first_time_install.sh completed with no plugins ever installed.

Test plan

  • Fresh install (first_time_install.sh -y): confirm the stale-config banner no longer appears after reboot
  • GET /api/v3/plugins/reconciliation-status: confirm unresolved list is empty (or contains only genuinely missing plugins, not sync/github/youtube)
  • Existing installs with real stale plugin entries (e.g. a plugin was deleted from disk): confirm the banner still fires correctly for those

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved plugin system configuration state reconciliation to properly exclude additional system configuration keys from plugin detection.

Review Change Stack

config_manager.load_config() deep-merges config_secrets.json into the
main config before returning it. This means secrets top-level keys
(github, youtube) appear alongside structural config keys (sync) in the
dict that _get_config_state() iterates.

_SYSTEM_CONFIG_KEYS was missing all three, so the reconciler treated them
as plugin IDs and flagged them as PLUGIN_MISSING_ON_DISK on every startup,
showing the "Stale plugin config entries found" warning banner to users on
a fresh install where those plugins have never existed.

Add the three keys with brief comments explaining which file each comes
from so the distinction is clear when the list grows.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e9399eb0-fa51-4075-8b93-c883042fa4f8

📥 Commits

Reviewing files that changed from the base of the PR and between 0c7d03a and c9668bb.

📒 Files selected for processing (1)
  • src/plugin_system/state_reconciliation.py

📝 Walkthrough

Walkthrough

StateReconciliation._SYSTEM_CONFIG_KEYS is expanded to exclude additional top-level config keys (sync, github, youtube) from being treated as plugin entries during state reconciliation, narrowing what _get_config_state() returns as plugin state candidates.

Changes

System Config Keys Exclusion Expansion

Layer / File(s) Summary
System config keys exclusion update
src/plugin_system/state_reconciliation.py
_SYSTEM_CONFIG_KEYS set expanded to exclude sync structural config and secrets-file keys github, youtube from plugin-state extraction.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • ChuckBuilds/LEDMatrix#293: Further expands _SYSTEM_CONFIG_KEYS to exclude additional config keys from plugin-state candidate filtering.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: preventing false-positive stale-config warnings on fresh installs by excluding system config keys from plugin reconciliation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/reconciler-false-positive-system-keys

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.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@ChuckBuilds
ChuckBuilds merged commit ea95f37 into main May 25, 2026
5 checks passed
@ChuckBuilds
ChuckBuilds deleted the fix/reconciler-false-positive-system-keys branch May 25, 2026 21:09
ChuckBuilds added a commit that referenced this pull request Sep 2, 2026
This PR was opened to say B6 was deliberately held. It has since run, so the
document said the opposite of the truth -- which is the exact failure it was
written to fix: "a plan that misreports which phase it is in is worse than no
plan".

Merges current main first (53 commits), which brings in #508's corrections to
this same file, then replaces the hold with what happened.

**Why the hold lifted is worth recording, because the stated gate was never
met.** It asked for evidence of 3.2.0 uptake, and that evidence could not
arrive: the core updates by `git pull --rebase`, so release-asset counts cannot
measure it, and no store-side telemetry exists. What changed is that the risk
the gate protected against was closed directly -- the store now refuses a
plugin whose floor exceeds the running core on all three routes in:
install_plugin (#431/#433), update_plugin's git branch (#508), and
install_from_url (#510). A pre-3.2.0 user cannot receive a sunset plugin at
all, so they keep the version they run. Refusal replaced the bundled copy,
which is what the copy stood in for.

Records what shipped (eight plugins, ~5,800 lines, plugins #346/#349/#350/#351)
and the two findings worth carrying to the next module: baseball's fallback was
the only one holding orchestration logic the core lacked, and two tests had
been leaning on the guard -- soccer's stubbed `src` in a way that shadowed the
core, so it had been exercising the frozen copy rather than the shipping class
since B5.

The remaining-work list is replaced too. Its first item was "nothing on the
critical path, B6 is waiting on calendar time", which is no longer true. What
remains: hardware soaks (with a note to check the rig's display_mode first, or
a board in switch mode tells you nothing about the scroll code), cutting 3.3.0
-- not required by B6, whose floors are 3.2.0, but calendar 1.2.3 floors at
3.3.0 and is un-installable until it exists -- and reconsidering the modules
held back during the sunset.

Keeps the pre-B6 sections as history. The reasoning still applies to the next
module; it is just no longer in force for this one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
ChuckBuilds added a commit that referenced this pull request Sep 2, 2026
…511)

#510 shows as merged, but into fix/gate-git-pull-updates -- #508's branch --
rather than main. #508 reached main first, so the sideload gate was left behind
on a branch. Same failure as plugins #350/#351, which merged into each other's
bases; worth knowing the pattern, because GitHub reports these as MERGED and
`gh pr list` shows nothing outstanding.

main today has two of the three routes gated: install_plugin (#431/#433) and
update_plugin's git branch (#508). install_from_url validates required manifest
fields and then installs whatever it found, never comparing the core version.

Cherry-picked unchanged from the orphaned branch -- it applies to main with no
conflict. TestSideloadGate pins the three cases the other routes pin: refuses a
floor above this core leaving nothing behind, still allows a compatible plugin
(the guard against a gate that refuses everything), and does not block a 2.0.0
floor on a core reporting an untrustworthy version.

Full suite 3725 passed, 6 skipped.


Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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