Skip to content

fix(plugin): bind component cleanup to the exact package manifest - #2107

Merged
Eli Bosley (elibosley) merged 5 commits into
mainfrom
fix/txz-current-package-manifest
Oct 6, 2026
Merged

Eli Bosley (elibosley) merged 5 commits into
mainfrom
fix/txz-current-package-manifest

Conversation

@elibosley

@elibosley Eli Bosley (elibosley) commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Prevent Connect TXZ upgrades from deleting current WebGUI components by selecting the exact package manifest for the version, architecture, and build being installed.

The first wildcard match can select an old -upgraded- manifest. Excluding archived entries alone also leaves a latent failure when a lone outdated active manifest remains. The TXZ builder now packages its exact filename identity, and doinst.sh uses only that identity in the primary or legacy package database. Missing or invalid identity, or a missing exact manifest, preserves components.

The plugin build workflow now validates the final TXZ using checksum-pinned Slackware installpkg/upgradepkg/removepkg scripts. It exercises upgrade and reinstall with an old upgraded manifest, compares every current component byte-for-byte, and checks the installed API entrypoints, environment, CLI links, and packaged API CLI execution.

The date/time coverage tests now use a fixed clock and unmount components to stop intervals. Regression assertions cover interval updates and calendar boundaries, addressing clock-dependent coverage variation without changing coverage thresholds.

Validation:

  • Full Web type check and ESLint passed after building the UI dependency; the test translator uses explicit schema and locale types without casts.
  • All 8 date/time tests passed twice with identical 97.8% line coverage for the composable.
  • Built a 66 MB TXZ locally in Linux using the existing exact-head API/Web/UI CI artifacts; application payloads were unchanged. Builder used Node 24; packaged API runtime is Node 22.18.0. CI uses the repository's configured Node 22.
  • Real Slackware pkgtools upgrade and reinstall passed: all 150 component files remained byte-identical, stale files were removed, API payload/environment/CLI links were preserved, and the packaged CLI executed successfully.
  • All 12 cleanup regression cases passed, including old-only active metadata, a wrong build for the same version, multiple active manifests, primary/legacy selection, and missing/invalid metadata.
  • ShellCheck passed; existing shell detection tests passed (4) and extractor tests passed (62).
  • Current head c8e6f9f2a2c3e7326f66f3a7bfb26343e1378938: API tests, API/Web/UI/plugin builds, final-TXZ pkgtools validation, CodeQL, and Codecov project/patch passed. Dependency audit remains advisory; findings were not suppressed.
  • Live Unraid 7.3.2 smoke test passed using the exact CI TXZ (run 37435328608, SHA-256 afda743519e6da3732213cdb9e5d1fb7978fe4c2246fcf06b395b0b48fa43bd9): all 150 components were byte-identical after install and reinstall, stale files were removed, API restart and temporary VIEWER API-key creation passed, authenticated GraphQL worked through the Unix socket and nginx, the internal CLI report succeeded, and a served WebGUI component matched the archive byte-for-byte. Managed QA restored and released the guest.
  • This smoke test does not cover browser visual journeys or Connect cloud registration/callbacks. The historical upgraded-manifest coexistence case is covered by CI regression and final-TXZ pkgtools checks; the clean live guest had no prior upgraded manifest.

Fixes #2106.

Summary by CodeRabbit

  • Bug Fixes
    • Improved package cleanup during upgrades by checking the installed package identity against an exact package record before removing stale component files.
    • Component files are now preserved when the package identity is missing or invalid, or when a matching package record cannot be identified unambiguously. This also avoids removing files based on records for a different package.

Fixes #2106. Preserve components when multiple active package manifests make cleanup ambiguous.
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bf02d12a-33f5-45f0-8954-d66efcaa2c8b
📥 Commits

Reviewing files that changed from the base of the PR and between 45b5db2 and c8e6f9f.

📒 Files selected for processing (1)
  • web/__test__/composables/dateTime.test.ts
 _______________________________________________________________________________________________________________________________________________________________
< English is just a programming language. Write documents as you would write code: honor the DRY principle, use metadata, MVC, automatic generation, and so on. >
 ---------------------------------------------------------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 36b481ef-8f69-4f1e-b4ac-14c115283a97
📥 Commits

Reviewing files that changed from the base of the PR and between 0884339 and 45b5db2.

📒 Files selected for processing (2)
  • plugin/builder/build-txz.ts
  • plugin/builder/utils/consts.ts

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


Walkthrough

The TXZ builder now records the package identity. Install cleanup validates that identity and selects an exact matching package manifest before removing stale component files. Tests cover manifest selection and built archive installation, and the build workflow runs the integration test for generated archives.

Changes

TXZ package installation

Layer / File(s) Summary
Package identity and stale component cleanup
plugin/builder/utils/consts.ts, plugin/builder/build-txz.ts, plugin/.gitignore, plugin/source/dynamix.unraid.net/install/doinst.sh, plugin/tests/test-txz-install.sh
The builder derives and writes the package identity to the package config. Cleanup rejects missing or invalid identities and checks for the exact identity-named manifest. Tests cover upgraded manifests, directory fallback, competing manifests, wrong builds, and invalid or missing identities.
Built archive installation validation
plugin/tests/test-built-txz-install.sh, .github/workflows/build-plugin.yml
The integration test validates archive upgrades and reinstalls, including component files, stale-file removal, payloads, command links, the manifest, and API CLI output. The build workflow runs the test for each generated archive after checking and installing Slackware pkgtools scripts.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 45b5d

No actionable merge-blocking issue is established in the reviewed changes; normal checks remain appropriate.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0894e

The change protects current components without adding a production entrypoint or broader deletion authority. No security regression is demonstrated, but stale-component loading and real installation ordering remain incompletely established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The changed cleanup governs files beneath the installed plugin's unraid-components directory. The PR does not expand its configured deletion root or add another destructive sink.

Trust Boundaries and Controls

  • observed — Manifest authority still derives from the package-database location, filename pattern, and regular-file check. The new controls reject upgraded filenames and ambiguous eligible manifests; they do not introduce package-content identity validation or establish database permissions.

Resilience and Maintainability Implications

  • observed — Deletion remains nontransactional and reads the live manifest separately for each file. A manifest read error can be treated as a missing entry, and interruption can leave partial cleanup. These behaviors predate this PR; production serialization and recovery guarantees remain unverified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #2106 requires cleanup to select the manifest for the installed package and preserve current components when an older -upgraded- manifest exists. build-txz.ts writes the exact version, archi…
Out of Scope Changes check ✅ Passed The package identity, cleanup logic, regression tests, and pkgtools-based TXZ validation all support issue #2106 by verifying manifest selection and component preservation. The reviewed changes contai…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: binding component cleanup to the exact package manifest.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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

I’m a rabbit with a package to inspect,
I check its name before I hop,
I leave old manifests in their place,
And test each archive from tip to top,
The fresh files stay right where they belong,
Then off I bound through fields of logs!

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

@linear-code
linear-code Bot marked this pull request as ready for review October 5, 2026 20:28
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@elibosley
Eli Bosley (elibosley) marked this pull request as draft October 5, 2026 20:57
@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.43%. Comparing base (b638d29) to head (c8e6f9f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2107      +/-   ##
==========================================
+ Coverage   53.39%   53.43%   +0.03%     
==========================================
  Files        1044     1044              
  Lines       72705    72705              
  Branches     8396     8416      +20     
==========================================
+ Hits        38821    38850      +29     
+ Misses      33757    33728      -29     
  Partials      127      127              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR2107/dynamix.unraid.net.plg

@elibosley
Eli Bosley (elibosley) marked this pull request as ready for review October 6, 2026 06:59
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T10:04:08.356010Z c8e6f9f Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@elibosley Eli Bosley (elibosley) changed the title fix(plugin): exclude upgraded manifests from component cleanup fix(plugin): bind component cleanup to the exact package manifest Oct 6, 2026
@elibosley
Eli Bosley (elibosley) marked this pull request as draft October 6, 2026 07:29
@elibosley
Eli Bosley (elibosley) marked this pull request as ready for review October 6, 2026 10:01
@elibosley
Eli Bosley (elibosley) merged commit 89b1395 into main Oct 6, 2026
@elibosley
Eli Bosley (elibosley) deleted the fix/txz-current-package-manifest branch October 6, 2026 10:01
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

🔄 PR Merged - Plugin Redirected to Staging

This PR has been merged and the preview plugin has been updated to redirect to the staging version.

For users testing this PR:

  • Your plugin will automatically update to the staging version on the next update check
  • The staging version includes all merged changes from this PR
  • No manual intervention required

Staging URL:

https://preview.dl.unraid.net/unraid-api/dynamix.unraid.net.plg

Thank you for testing! 🚀

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8e6f9f2a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment on lines +135 to +137
curl --fail --silent --show-error --location --max-time 60 \
"https://mirrors.slackware.com/slackware/slackware64-current/source/a/pkgtools/scripts/$script" \
--output "$pkgtools_dir/$script"

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 Badge Pin pkgtools downloads to an immutable source

When Slackware updates any script under the rolling slackware64-current URL, the downloaded content will no longer match the hard-coded hashes on lines 142–144, so every invocation of the inspected build-plugin.yml workflow will fail at sha256sum --check and block plugin artifacts and releases. Fetch these scripts from an immutable release or source revision while retaining checksum verification.

Useful? React with 👍 / 👎.

SimonFair pushed a commit that referenced this pull request Oct 20, 2026
🤖 I have created a release *beep* *boop*
---


## [4.37.6](v4.37.5...v4.37.6)
(2026-10-20)


### Bug Fixes

* **api:** stop temperature metrics from spinning up array disks
([#2091](#2091))
([b15af83](b15af83))
* **callback:** validate callback requests
([#2097](#2097))
([1bda4d2](1bda4d2))
* **connect:** avoid restarting active static remote access
([#2110](#2110))
([d19a6f4](d19a6f4))
* **deps:** refresh vulnerable API and web packages
([#2098](#2098))
([64eac12](64eac12))
* **plugin:** bind component cleanup to the exact package manifest
([#2107](#2107))
([89b1395](89b1395))
* **web:** align mobile header identity and update actions
([#2111](#2111))
([6582cc1](6582cc1))
* **web:** align registration license action buttons
([#2101](#2101))
([b638d29](b638d29))
* **web:** polish combined header on mobile and desktop
([#2109](#2109))
([64f1e81](64f1e81))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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.

doinst.sh stale-component cleanup may select previous -upgraded- package manifest

1 participant