pleb
44 lines
4.5 KiB
Markdown
44 lines
4.5 KiB
Markdown
**Findings**
|
||
|
||
- High: install state is still keyed by instance name, not a concrete installation. [state.py](/home/pleb/ops/beatsaber/plugin-helper/src/plugin_helper/state.py:10) writes everything under `instances/<instance>`, and the TUI gives every discovered root the same `runtime.state_root` in [cli.py](/home/pleb/ops/beatsaber/plugin-helper/src/plugin_helper/cli.py:279). That is fine only if Linux and Windows installs never share a state dir. For package-manager ambitions, introduce an `install_id` derived from profile/root/path, and split reusable asset cache from per-install state.
|
||
|
||
- High: `apply_plan` is not transactional. It backs up and writes files one by one, then saves state at the end in [installer.py](/home/pleb/ops/beatsaber/plugin-helper/src/plugin_helper/installer.py:39). If extraction/copy fails mid-plan, the game tree can be half-mutated without matching install state. A future package manager wants transactions/generations: stage, apply, record transaction, and have a first-class rollback command.
|
||
|
||
- Medium-high: installed state records files but not the installed package identity. [installer.py](/home/pleb/ops/beatsaber/plugin-helper/src/plugin_helper/installer.py:63) stores `path`, `sha256`, and `size`, but not repo/tag/asset/source URL/source hash/reason. The report then uses the current lockfile to describe installed versions, which can drift after a lock update. Record the lock identity into install state at apply time.
|
||
|
||
- Medium-high: dependency handling is intentionally thin. `Dependency.constraint` exists in [models.py](/home/pleb/ops/beatsaber/plugin-helper/src/plugin_helper/models.py:10), but [planner.py](/home/pleb/ops/beatsaber/plugin-helper/src/plugin_helper/planner.py:68) only expands required dependency IDs and does not enforce constraints, conflicts, `loadAfter`, `loadBefore`, BeatMods version-id closures, or “provides” relationships. That is the next big unlock for package-manager behavior.
|
||
|
||
- Medium: the agent skills have a few stale or conflicting operational rules. The manager skill still tells agents to inspect/edit `registry/plugins.toml` in [.agents/skills/beatsaber-plugin-manager/SKILL.md](/home/pleb/ops/beatsaber/plugin-helper/.agents/skills/beatsaber-plugin-manager/SKILL.md:54), while the project has moved to `registry/plugins/*.toml`. The builder skill says PR checkouts go under `<state_dir>/build` in [.agents/skills/beatsaber-plugin-builder/SKILL.md](/home/pleb/ops/beatsaber/plugin-helper/.agents/skills/beatsaber-plugin-builder/SKILL.md:24), while `AGENTS.md` says GitHub plugin source checkouts should live under `~/src/<owner>/<repo>`. That will make agents inconsistent.
|
||
|
||
**Skills**
|
||
|
||
I would not collapse the three skills into one. They are conceptually different:
|
||
|
||
- `beatsaber-plugin-manager`: mutating install/update/bootstrap workflow.
|
||
- `beatsaber-plugin-update-auditor`: read-mostly audit/report workflow.
|
||
- `beatsaber-plugin-builder`: source build workflow.
|
||
|
||
But I would consolidate shared policy into references, then have each skill import that mental model:
|
||
|
||
- `references/repo-workflow.md`: repo root, `.venv`, `PYTHONPATH=src`, dirty worktree rules, validation commands.
|
||
- `references/state-and-profiles.md`: `.state`, profiles, install identity, source checkout location.
|
||
- `references/artifact-policy.md`: GitHub first, BeatMods metadata/fallback, private sources, checksum policy.
|
||
- `references/live-validation.md`: smoketest and process cleanup.
|
||
|
||
That removes duplicated drift while preserving good trigger boundaries.
|
||
|
||
**Design Direction**
|
||
|
||
Yes, the script design is useful. The best parts are exactly the right bones for a real package manager: registry, locks, dry-run plans, hash checks, managed install state, bootstrap as its own phase, update audit, and known-good sets.
|
||
|
||
The next shape I’d aim for is:
|
||
|
||
- `packages/registry`: identity, source aliases, install strategy, metadata extraction rules.
|
||
- `versions/locks`: selected package versions/artifacts/evidence for each Beat Saber version.
|
||
- `cache/downloads`: content-addressed artifacts reusable across installs.
|
||
- `installs/<install_id>`: installed packages, transactions, bootstrap, known-good generations, backups.
|
||
- commands like `resolve`, `fetch`, `plan`, `apply`, `rollback`, `audit`, `verify`.
|
||
|
||
Beat Saber plugins are heterogeneous in packaging, but homogeneous enough in runtime shape (`Plugins`, `Libs`, `IPA/Pending`, root BSIPA files) that this can become a nice small package manager. The trick is to normalize artifacts into a planned file tree before touching the game.
|
||
|
||
Validation: `compileall` passed, and `unittest discover` passed: 75 tests, 1 skipped. |