diff --git a/.gh-pr-body.md b/.gh-pr-body.md deleted file mode 100644 index 91c2e76..0000000 --- a/.gh-pr-body.md +++ /dev/null @@ -1,81 +0,0 @@ -## Summary - -This PR adds **`moddin-agent/`** — an MCP server + agent skill that lets an AI assistant add new mods to Moddin Desktop by authoring capability recipes the user then previews and applies inside the app — and ships the rest of the working copy alongside it. - -The user talks to their AI assistant in natural language (*"add this FPS unlocker to Elden Ring"*); the agent asks the few questions it can't guess, drafts a capability YAML, validates it against the JSON Schema mirror of the Rust `CapabilitySpec`, shows the predicted diff, asks for explicit confirmation, and writes the file to `%LOCALAPPDATA%\Moddin\capabilities\`. The user opens Moddin, clicks **Rescan**, the new capability appears, and the normal preview / transaction / rollback pipeline takes over. - -## Highlights — `moddin-agent/` (new) - -- **`src/mcp-server.mjs`** — single-file Node.js MCP server over stdio. **11 tools**: - - `moddin_status`, `list_supported_games`, `get_game_info` - - `list_capabilities`, `get_capability` - - `get_step_kinds`, `get_check_kinds` - - `get_capability_template` - - `validate_capability_yaml` - - `preview_capability_plan` - - `save_capability_yaml` -- **`schema/capability.schema.json`** — JSON Schema mirror of `src-tauri/src/capability.rs`. Source of truth for `validate_capability_yaml`. -- **`templates/`** — starter recipes for four patterns: `extract-zip`, `proxy-dll`, `registry-override`, `vr-runtime-overlay`. -- **`SKILL.md`** — system prompt the agent loads alongside the MCP server. Walks it through the workflow (intent → template → compose → validate → preview → confirm → save). -- **`scripts/validate-templates.mjs`** + **`scripts/smoke-test.mjs`** — CI guards. Both wired into `.github/workflows/ci.yml`. -- **`README.md`** — install + how to register the MCP server with Claude Desktop / Claude Code / Codex / Cursor. - -## Hard safety boundaries (the non-obvious bit) - -The agent **cannot** invoke install / uninstall / rollback. The MCP server has no such tool. The only filesystem write is `save_capability_yaml`, which is restricted to `%LOCALAPPDATA%\Moddin\capabilities\` (the same folder Moddin Desktop already reads). Every save is schema-validated first. Moddin stays the source of truth for every destructive action — the agent is purely an authoring assistant. Full threat model in `docs/AGENT-RECIPES.md`. - -## Highlights — the rest of the working copy - -While this PR was being put together, the working copy accumulated the next increment of the catalog / backend / frontend work. It lands in this PR as well: - -- **Catalog** — `enginePreset` + `pcgwSlug` wired into every game (`cyberpunk-2077`, `dawnwalker`, `dead-island-2`, `doom-2016`, `elden-ring`, `stalker-2`); new `idtech` and `unity` engine presets so every supported game resolves a non-empty default module list. -- **Tauri backend** — `compat_report.rs` (read-only per-module snapshot of installed + upstream versions), `gog.rs` (GOG.com registry discovery), `library_state.rs` (in-memory library cache the UI polls), `pcgw_cache.rs` (PCGamingWiki enrichment cache with TTL). `transaction.rs` gains named `Snapshot` rollback points. `bepinex.rs`, `reshade.rs`, `steam.rs` hardened and extended. -- **Frontend** — `services/catalog.ts`, `services/pcgw.ts`, `services/profile.ts` plus the matching `types/*` mirror the new Rust commands. -- **Community catalog** — `catalog.json` (initial snapshot), `moddin-community-capabilities/` as a proper submodule pointing at `petonexus/moddin-community-capabilities.git` (with `.gitmodules`). - -## Validations - -Run locally before push: - -- `node scripts/validate-templates.mjs` — **4/4 templates valid** against the schema -- `node scripts/smoke-test.mjs` — **16/16 MCP checks** pass (initialize, every tool advertised, list 5 games, list 6 built-in capabilities, end-to-end `template → compose → validate → preview`, malformed spec correctly rejected) -- Full file list (15 files in `moddin-agent/`) verified by `git diff --stat HEAD~7..HEAD` - -CI on this PR will additionally run: -- `npm ci && npm run check:architecture && npm run build` -- `cargo check --manifest-path src-tauri/Cargo.toml` -- `cargo test --manifest-path src-tauri/Cargo.toml` - -## Commits - -- `6b76847` — `chore(agent): track moddin-agent package-lock.json` -- `7785eae` — `feat(community): community catalog scaffold + PR body draft` -- `756203f` — `feat(frontend): catalog / PCGW / profile services + types` -- `5e32847` — `feat(tauri): GOG discovery + library cache + PCGW enrichment + snapshots + module work` -- `78c8e18` — `feat(catalog): add idtech and unity engine presets` -- `272600e` — `feat(catalog): add enginePreset + pcgwSlug to all supported games` -- `bef870c` — `feat(agent): moddin-agent MCP server + skill for AI-driven mod authoring` - -## Post-merge checklist - -```powershell -npm ci -cd moddin-agent && npm ci && node scripts/validate-templates.mjs && node scripts/smoke-test.mjs -cargo test --manifest-path src-tauri/Cargo.toml -``` - -To register the MCP server with an agent (Claude Desktop / Claude Code / Codex / Cursor), add to its `mcpServers`: - -```jsonc -{ - "mcpServers": { - "moddin": { - "command": "node", - "args": ["/moddin-agent/src/mcp-server.mjs"], - "env": { "MODDIN_PROJECT_ROOT": "" } - } - } -} -``` - -Then load `moddin-agent/SKILL.md` as a skill in the agent and you're done. \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af63525..b9b7698 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,18 @@ name: CI on: push: - branches: [main, 'feat/**', 'refactor/**'] + # Every branch prefix CONTRIBUTING.md documents gets feedback. The + # list used to be main/feat/**/refactor/** only, so a fix/, + # hardening/, docs/ or chore/ branch shipped unverified until + # someone opened a PR against it. + branches: + - main + - 'feat/**' + - 'refactor/**' + - 'fix/**' + - 'hardening/**' + - 'docs/**' + - 'chore/**' pull_request: branches: [main] @@ -18,6 +29,12 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + # The community capabilities repo is a submodule and the + # capability-kind parity check reads its validator. Checking + # out without it made that check impossible, which is why the + # drift it exists to catch went unnoticed. + submodules: recursive - name: Setup Node.js uses: actions/setup-node@v4 @@ -25,40 +42,266 @@ jobs: node-version: 22 cache: npm + # The `tauri` dependency needs its `custom-protocol` Cargo feature, and + # without it the release build still compiles, still bundles and still + # installs — the window just loads http://127.0.0.1:1420 and renders + # nothing. Nothing in `cargo check`, `cargo test` or the installer build + # can see it; the symptom is a blank app on a user's machine, so the + # manifest is all a cheap gate can check. Zero dependencies, and run + # through `node` directly so it really does execute before the install + # below, on a bare checkout, with nothing able to fail first. + - name: Check bundled-build features + run: node scripts/check-bundle-features.mjs + - name: Install frontend dependencies run: npm ci + # Before the build on purpose. The catalogue is data with no + # other gate, and it is not validatable by eyeball: a game shipped + # declaring RE Engine while offering UEVR, OFXR and Cheeky, and + # nothing noticed. The check declares no dependencies, so it runs + # on a bare checkout, and it gets its own step so a red run names + # the data problem instead of a build error. + - name: Validate the game catalogue + run: npm run check:catalog + + - name: Frontend tests + # ROADMAP F-12. The four ConfirmDialog prompts and the + # unsigned-consent gate shipped with nothing asserting they + # appear; this is the gate that stops the next one from doing + # the same. Kept as its own step so a red run names the cause. + run: npm run test + - name: Check and build frontend run: | npm run check:architecture + npm run check:capability-kinds npm run build + # The community repo lives in a submodule, so this only runs when + # it is actually checked out. The parity guard inside the desktop + # repo is the one that always runs. - name: Validate moddin-agent capability templates working-directory: moddin-agent run: | npm ci node scripts/validate-templates.mjs + - name: Validate community capabilities + working-directory: moddin-community-capabilities + run: | + python -m pip install --quiet -r requirements.txt + python scripts/validate_capability.py capabilities/ + - name: Smoke test moddin-agent MCP server working-directory: moddin-agent run: node scripts/smoke-test.mjs - - name: Setup Rust stable MSVC + - name: Setup Rust toolchain shell: pwsh + # rust-toolchain.toml at the repo root pins the exact release, + # the profile and the components (rustfmt, clippy). `rustup show` + # is what installs and selects it; `rustup default stable-msvc` + # was the moving target this replaces. run: | - rustup default stable-msvc + rustup show rustc --version cargo --version + rustfmt --version + cargo clippy --version + # ------------------------------------------------------------------ # tauri.conf.json bundles ../moddin-runtime/{node.exe,moddin-agent}, # and tauri-build validates those resource paths during cargo's # build script — so cargo test/check fails without them. The folder # is gitignored and generated; this is the documented step that # creates it (same one developers run before `npm run tauri build`). + # + # It is also the slowest thing in this workflow: a ~28 MB + # digest-verified Node download plus a full `npm install` of the + # staged agent runtime, on every push, to produce output that is a + # pure function of its inputs. So it is cached. + - name: Resolve runtime bundle cache key + id: runtime-key + shell: pwsh + env: + # Everything the bundle is derived from: the script itself + # (which pins the Node version and the staged manifest), and + # the agent sources it copies and installs from. + INPUT_HASH: >- + ${{ hashFiles('moddin-runtime/bundle-runtime.ps1', + 'moddin-agent/package.json', + 'moddin-agent/package-lock.json', + 'moddin-agent/src/**', + 'moddin-agent/scripts/**', + 'moddin-agent/templates/**', + 'moddin-agent/schema/**') }} + run: | + # hashFiles returns the empty string when a pattern matches + # nothing. Left unchecked, that collapses the key to one entry + # shared by every commit in history, and the first job to fill + # it would hand the rest a bundle built from inputs nobody + # looked at. Fail the run instead. + if (-not $env:INPUT_HASH) { + throw "hashFiles matched no inputs, so the runtime cache key would be a single shared entry: '$env:INPUT_HASH'" + } + "hash=$env:INPUT_HASH" >> $env:GITHUB_OUTPUT + Write-Host "runtime bundle key: $env:INPUT_HASH" + + - name: Restore runtime bundle + id: runtime-cache + uses: actions/cache@v4 + with: + # No `restore-keys`. A prefix fallback would let a job whose + # inputs differ from the cached one run `cargo test` against + # a bundle staged from a different commit — the exact "stale + # bundle" this step exists to rule out. + path: | + moddin-runtime/node.exe + moddin-runtime/moddin-agent + moddin-runtime/BUNDLE_OK + # v2: the bundle no longer carries node.zip, tmp/ or + # node_modules.old, and it installs with `npm ci` from + # moddin-agent/package-lock.json. A v1 cache entry holds a + # staged tree with a 56 MB dev node_modules.old in it, and + # caching it back out would keep shipping that. + key: moddin-runtime-v2-${{ runner.os }}-${{ steps.runtime-key.outputs.hash }} + - name: Bundle moddin-runtime + if: steps.runtime-cache.outputs.cache-hit != 'true' shell: pwsh run: pwsh -NoProfile -File ./moddin-runtime/bundle-runtime.ps1 + - name: Verify runtime bundle + shell: pwsh + # A cache hit is a claim that the bundle on disk was built from + # exactly these inputs. This step is the claim's audit, and it + # fails the job rather than warning: cargo test and cargo check + # below are about to treat this folder as the shipped runtime. + run: | + $ErrorActionPreference = 'Stop' + $out = 'moddin-runtime' + $agent = Join-Path $out 'moddin-agent' + + $required = @( + (Join-Path $out 'node.exe'), + (Join-Path $out 'BUNDLE_OK'), + (Join-Path $agent 'package.json'), + (Join-Path $agent 'package-lock.json'), + (Join-Path $agent 'node_modules'), + (Join-Path $agent 'scripts\smoke-test.mjs') + ) + $missing = $required | Where-Object { -not (Test-Path $_) } + if ($missing) { + throw "Runtime bundle is incomplete (cache hit: '${{ steps.runtime-cache.outputs.cache-hit }}'). Missing: $($missing -join ', ')" + } + + # The staged manifest is generated by bundle-runtime.ps1, not + # copied. A cache that restored the agent's dev manifest + # instead would carry devDependencies into the installer. + $staged = Get-Content (Join-Path $agent 'package.json') -Raw | ConvertFrom-Json + if ($staged.name -ne 'moddin-agent-runtime') { + throw "Staged runtime manifest is '$($staged.name)', not 'moddin-agent-runtime'. The cache is holding a dev copy." + } + foreach ($dev in @('typescript', '@yao-pkg/pkg', 'vitest')) { + if ($null -ne $staged.dependencies.$dev) { + throw "Staged runtime manifest still declares the dev dependency '$dev'." + } + } + foreach ($prod in @('@modelcontextprotocol/sdk', 'ajv', 'ajv-formats', 'yaml')) { + $pkg = $prod.Replace('/', '\') + if ($null -eq $staged.dependencies.$prod) { throw "Staged manifest is missing '$prod'." } + if (-not (Test-Path (Join-Path $agent "node_modules\$pkg"))) { throw "Staged runtime has no node_modules\$pkg." } + } + + # The install is `npm ci` against this file, so it is the one + # thing deciding which versions ship (F-13 / O-06). If the + # staged copy is not the committed one, the install was + # resolved from something else and the guarantee is gone. + $srcLock = (Get-FileHash 'moddin-agent\package-lock.json' -Algorithm SHA256).Hash + $stagedLock = (Get-FileHash (Join-Path $agent 'package-lock.json') -Algorithm SHA256).Hash + if ($srcLock -ne $stagedLock) { + throw "Staged package-lock.json is not moddin-agent\package-lock.json byte for byte." + } + + # Every shipped version is exact, because the install is + # `npm ci` against moddin-agent/package-lock.json (F-13 / O-06). + # A caret here would mean the install and the manifest had + # drifted apart, which is the state this replaced. + foreach ($prop in $staged.dependencies.PSObject.Properties) { + if ($prop.Value -notmatch '^\d+\.\d+\.\d+([-+].*)?$') { + throw "Staged manifest pins '$($prop.Name)' as '$($prop.Value)', which is a range, not a version." + } + } + + # Build scratch and the previous script's staging leftover. + # tauri.conf.json lists the whole moddin-agent directory as a + # bundle resource, so anything left inside it ships. These + # were 28.5 MB and 56 MB respectively, and a cached bundle + # that grew them is how they stayed invisible. + foreach ($scratch in @( + (Join-Path $out 'node.zip'), + (Join-Path $out 'tmp'), + (Join-Path $agent 'node_modules.old') + )) { + if (Test-Path $scratch) { throw "Build scratch '$scratch' survived the bundler and would be bundled." } + } + + # The bundle ships its own pinned Node; a cache that restored + # the runner's own would still build, just not the artifact. + $nodeVersion = (& (Join-Path $out 'node.exe') --version).Trim() + if ($nodeVersion -ne 'v20.19.5') { + throw "Bundled node.exe reports $nodeVersion, expected v20.19.5." + } + + # The same smoke test the bundler runs before writing + # BUNDLE_OK: a cached bundle is re-proven, not assumed. + $env:MODDIN_PROJECT_ROOT = (Resolve-Path '.').Path + & (Join-Path $out 'node.exe') (Join-Path $agent 'scripts\smoke-test.mjs') + if ($LASTEXITCODE -ne 0) { throw "Runtime smoke test failed on the '${{ steps.runtime-cache.outputs.cache-hit }}' bundle." } + $env:MODDIN_PROJECT_ROOT = $null + Write-Host "runtime bundle verified (cache hit: '${{ steps.runtime-cache.outputs.cache-hit }}', node $nodeVersion)" + + # ------------------------------------------------------------------ + # Both gates are blocking. They were added reported-only while the + # backlog stood at 3,653 `cargo fmt` diff lines across 34 files and + # 144 clippy diagnostics across 30 lints (53 of them `dead_code` + # from the nine unwired module implementations in docs/ + # BACKEND-MODULES.md). That backlog is now cleared, so there is + # nothing left to report and nothing left to justify `continue-on-error`. + # + # `cargo fmt --check` needs no flags to block: it exits non-zero on + # any diff. `cargo clippy` does — without `-- -D warnings` it exits + # 0 and only prints its findings, which is precisely why the step + # looked green while the lints were outstanding. + # + # Deliberately no `[lints]` allow-list in src-tauri/Cargo.toml: the + # tree is clean without one, and a blanket allow would turn the gate + # into a guard that has been switched off. The only suppressions + # left are three item-scoped `#[allow(dead_code, reason = ...)]` on + # the un-adopted `Module` contract in src-tauri/src/module.rs, each + # stating why the code exists and why it is not compiled into the + # shipped binary. + # + # These two sit BELOW the runtime bundle on purpose, and the ordering + # is load-bearing rather than cosmetic. `cargo fmt` does not compile, + # so it passed wherever it was put — but `cargo clippy` compiles the + # crate, which runs tauri's build script, and that script copies + # every path in `bundle.resources` unconditionally (tauri-build + # `lib.rs`: `copy_resources(ResourcePaths::new(res, true), …)`), erroring + # with `resource path '..\moddin-runtime\node.exe' doesn't exist` on a + # checkout where the gitignored bundle has not been staged. The first + # run with these gates above the bundle failed in the Clippy step for + # exactly that reason, with the gates' own history never printed. + # `main` had the bundle before every Rust compile and was green; these + # two were inserted above it and broke it. **Any new step that compiles + # Rust belongs after `Verify runtime bundle`, not before it.** + - name: Format check + run: cargo fmt --manifest-path src-tauri/Cargo.toml --check + + - name: Clippy + run: cargo clippy --manifest-path src-tauri/Cargo.toml --all-targets -- -D warnings + - name: Run Rust tests run: cargo test --manifest-path src-tauri/Cargo.toml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d67f6cf..4c1e2d4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,9 +1,17 @@ name: Release # Publishes a GitHub release when a version tag is pushed. -# The release body comes from `.release-notes-.md` at the repo root, and -# its first line (a Markdown H1) becomes the release title. Tags with a -# pre-release suffix (e.g. v0.1.0-beta.2) are published as pre-releases. +# +# The installer is BUILT here, on windows-latest, and the SHA-256 of +# whatever came out is written back into the release body. That is the +# whole point: the release notes already tell users to check a digest, +# and a digest computed by hand on a maintainer's laptop is not +# evidence of anything. Every installer in this release's history was +# built outside CI. +# +# The body comes from `docs/release-notes/.release-notes-.md`; its +# first line (a Markdown H1) becomes the release title. Tags with a +# pre-release suffix (e.g. v0.1.0-beta.2) publish as pre-releases. on: push: @@ -13,20 +21,169 @@ permissions: contents: write jobs: + build: + name: Build Windows installer + runs-on: windows-latest + outputs: + installer: ${{ steps.package.outputs.installer }} + sha256: ${{ steps.package.outputs.sha256 }} + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - name: Install frontend dependencies + run: npm ci + + - name: Check and build frontend + run: | + npm run check:architecture + npm run build + + - name: Setup Rust toolchain + shell: pwsh + # Same override ci.yml uses: rust-toolchain.toml at the repo root + # pins the release, so a tagged build cannot be produced by a + # different compiler than the one CI verified. + run: | + rustup show + rustc --version + cargo --version + + # tauri.conf.json bundles ../moddin-runtime/{node.exe,moddin-agent}, + # and tauri-build validates those resource paths during cargo's + # build script, so the Tauri build fails without them. The script + # downloads a digest-verified portable Node and stages the + # production agent runtime. + - name: Bundle moddin-runtime + shell: pwsh + run: pwsh -NoProfile -File ./moddin-runtime/bundle-runtime.ps1 + + - name: Build installer + run: npm run tauri build -- --bundles nsis + + # The updater needs three more things than the installer: the + # versioned NSIS zip, its minisign signature, and the latest.json + # manifest that names both. This step throws rather than skipping, + # deliberately. + # + # `tauri build` produces the zip whenever the updater plugin is + # configured, and the .sig only when TAURI_SIGNING_PRIVATE_KEY is + # present. So a release without the secrets yields a zip with no + # signature beside it — an updater payload the app would refuse. + # Publishing that would be the same failure the audit records from + # the last time a release went out wrong, one layer up. The job + # stops instead, and the error names the document. + # + # Note the desktop build already refuses to compile while + # tauri.conf.json holds the pubkey placeholder, so a release + # cannot reach this step with an unconfigured key anyway. The two + # guards are independent on purpose: one stops the build, the other + # stops a signed-but-unpublished payload. + # + # The secrets are the maintainer's to add; see docs/UPDATER.md. + - name: Collect updater artifacts + id: updater + shell: pwsh + env: + TAURI_SIGNING_PRIVATE_KEY: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY }} + TAURI_SIGNING_PRIVATE_KEY_PASSWORD: ${{ secrets.TAURI_SIGNING_PRIVATE_KEY_PASSWORD }} + run: | + $bundle = 'src-tauri\target\release\bundle\nsis' + $zip = Get-ChildItem -Path $bundle -Filter '*.nsis.zip' -ErrorAction SilentlyContinue | + Sort-Object LastWriteTime -Descending | Select-Object -First 1 + + if (-not $zip) { + throw "No *.nsis.zip was produced at $bundle. The updater plugin must be configured; see docs/UPDATER.md." + } + if (-not (Test-Path "$($zip.FullName).sig")) { + throw "Found $($zip.Name) with no .sig beside it. A release that ships an unsigned updater payload is the failure the updater was built to prevent; see docs/UPDATER.md for the signing secrets." + } + if (-not (Test-Path "$bundle\latest.json")) { + throw "Found the signed zip but no latest.json at $bundle. Tauri writes it only when the signing secrets are present; check the repository secrets." + } + + "zip=$($zip.FullName)" >> $env:GITHUB_OUTPUT + "sig=$($zip.FullName).sig" >> $env:GITHUB_OUTPUT + "manifest=$bundle\latest.json" >> $env:GITHUB_OUTPUT + # The release job gates its download on this, so a build with + # no updater payload simply skips the step rather than failing + # on a missing artifact. + "updater_zip=$($zip.Name)" >> $env:GITHUB_OUTPUT + Write-Host "updater payload: $($zip.Name) + .sig + latest.json" + + - name: Upload updater artifacts + uses: actions/upload-artifact@v4 + with: + name: moddin-updater + path: | + ${{ steps.updater.outputs.zip }} + ${{ steps.updater.outputs.sig }} + ${{ steps.updater.outputs.manifest }} + retention-days: 7 + + # Pin the exact artifact and its digest so the release job uploads + # what it hashed, with no glob left to match something else. + - name: Collect and hash the installer + id: package + shell: pwsh + run: | + $installer = Get-ChildItem -Path 'src-tauri\target\release\bundle\nsis' -Filter '*-setup.exe' -ErrorAction SilentlyContinue | + Sort-Object LastWriteTime -Descending | + Select-Object -First 1 + if (-not $installer) { + throw 'No NSIS installer was produced at src-tauri\target\release\bundle\nsis\' + } + $digest = (Get-FileHash -Path $installer.FullName -Algorithm SHA256).Hash.ToLowerInvariant() + Write-Host "installer: $($installer.Name)" + Write-Host "sha256: $digest" + "installer=$($installer.FullName)" >> $env:GITHUB_OUTPUT + "sha256=$digest" >> $env:GITHUB_OUTPUT + Add-Content -Path "$env:GITHUB_STEP_SUMMARY" -Value "| file | sha256 |`n| --- | --- |`n| $($installer.Name) | ``$digest`` |" + + - name: Upload installer + uses: actions/upload-artifact@v4 + with: + name: moddin-installer + path: ${{ steps.package.outputs.installer }} + if-no-files-found: error + retention-days: 7 + release: name: Publish GitHub release runs-on: ubuntu-latest + needs: build steps: - name: Checkout uses: actions/checkout@v4 + - name: Download installer + uses: actions/download-artifact@v4 + with: + name: moddin-installer + + # The updater payload is always present now — the build step above + # throws without it — so this download is unconditional. The `if:` + # would only hide a real failure. + - name: Download updater payload + uses: actions/download-artifact@v4 + with: + name: moddin-updater + - name: Create release from notes file env: GH_TOKEN: ${{ github.token }} TAG: ${{ github.ref_name }} + SHA256: ${{ needs.build.outputs.sha256 }} run: | - notes=".release-notes-${TAG}.md" + notes="docs/release-notes/.release-notes-${TAG}.md" if [ ! -f "$notes" ]; then echo "::error::Missing $notes. Commit the release notes before pushing the tag." exit 1 @@ -36,8 +193,28 @@ jobs: prerelease=() case "$TAG" in *-*) prerelease=(--prerelease) ;; esac + # Append the CI-computed digest so the notes and the artifact + # can never disagree about what was published. + printf '\n---\n\nBuilt by CI on windows-latest. SHA-256 of the installer:\n\n```\n%s\n```\n' "$SHA256" >> "$notes" + + # `latest.json` is the file the app fetches, so it must sit + # at the path the plugin's endpoint names. A prerelease keeps + # it out of `releases/latest/download/`, which is why the + # panel treats a prerelease as unavailable rather than + # offering it — the stable channel is what that endpoint + # serves. + updater=() + if [ -f ./latest.json ] && [ -f ./*.nsis.zip ] && [ -f ./*.nsis.zip.sig ]; then + updater=(./latest.json ./*.nsis.zip ./*.nsis.zip.sig) + echo "attaching updater payload: $(ls -1 ./*.nsis.zip)" + else + echo "::notice::No signed updater payload in this build. The installer release is unaffected; the app's update check will report the manifest as missing. See docs/UPDATER.md." + fi + gh release create "$TAG" \ --verify-tag \ --title "$title" \ --notes-file "$notes" \ + ./*.exe \ + "${updater[@]}" \ "${prerelease[@]}" diff --git a/.gitignore b/.gitignore index 1cbde26..7426077 100644 --- a/.gitignore +++ b/.gitignore @@ -98,3 +98,30 @@ moddin-runtime/moddin-mcp.mjs moddin-runtime/BUNDLE_OK moddin-runtime/stderr.txt moddin-runtime/*.tmp +# Downloaded by the bundler and re-read on every run to pin the Node +# archive. It is the checksum list for a 28 MB download, not a project +# input, and a fresh bundle used to leave it untracked in the tree. +moddin-runtime/SHASUMS256.txt + +# Generated and wrong-platform weight (roadmap O-11). None of it is read +# at runtime or by the build — see the notes per entry. + +# Stale copy of the community catalog, generated by +# moddin-community-capabilities/scripts/regenerate_catalog.py from the +# capability YAMLs. The app downloads its own signed copy into +# %LOCALAPPDATA%/Moddin at runtime; nothing in this repo opens this file. +/catalog.json + +# Release pack whose entire contents are already tracked, byte for +# byte, under tools/elden-ring-ervr-ofxr-baseline/. Ship the directory. +/EldenRing-ERVR-OFXR-Baseline-Moddin.zip + +# macOS / iOS icons. Moddin is Windows-only (docs/SCOPE.md) and +# tauri.conf.json lists only icons/icon.ico under bundle.icon. +/src-tauri/icons/icon.icns +/src-tauri/icons/ios/ + +# tauri-build regenerates these on every build. The only reference is +# the "$schema" editor hint in src-tauri/capabilities/default.json, +# which resolves again the moment cargo runs. +/src-tauri/gen/schemas/ diff --git a/EldenRing-ERVR-OFXR-Baseline-Moddin.zip b/EldenRing-ERVR-OFXR-Baseline-Moddin.zip deleted file mode 100644 index dfa3543..0000000 Binary files a/EldenRing-ERVR-OFXR-Baseline-Moddin.zip and /dev/null differ diff --git a/README.md b/README.md index 5cd0554..fd6a4f5 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ You can also drop your own YAML capability into `%LOCALAPPDATA%\Moddin\capabilit ## Getting Moddin 1. **Download** the latest release from [Releases](https://github.com/petonexus/moddin-desktop/releases). Beta releases are tagged `vX.Y.Z-beta.N`. -2. **Install** by running the `.msi` (Windows 10/11 x64). +2. **Install** by running the downloaded `Moddin.Desktop_*_x64-setup.exe` (Windows 10/11 x64). 3. **Open** the app. Moddin scans your Steam + Epic libraries automatically. 4. **Pick a game**, see which mods apply, and install what you need. diff --git a/ROADMAP.md b/ROADMAP.md index 79fe582..dacac42 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,55 +1,791 @@ # Roadmap -## v0.1 — Library bootstrap - -- [x] Tauri + Vue + TypeScript skeleton -- [x] Steam library discovery -- [x] Windows Registry fallback for non-standard Steam installs -- [x] installed-game scan -- [x] YAML catalog validation -- [x] Elden Ring catalog entry -- [x] Cyberpunk 2077 catalog entry -- [x] installed-games UI -- [x] transaction model and local transaction store -- [x] first reusable module: OBS VR capture - -## v0.2 — Safe actions - -- [ ] generic action engine primitives -- [x] backup before mutation -- [x] rollback / undo -- [x] dry-run / preview screen -- [x] inspect executable directory for common proxy DLLs -- [x] VR-ready launch profiles with OpenXR preflight and recommended graphics settings -- [x] OFXR Bridge FrameGen install/start/configure/arm flow before VR launch -- [x] per-game OpenXR runtime override without changing the Windows global runtime -- [x] module verification checklist and post-action validation -- [x] reapply/reinstall and transaction-backed removal flows -- [x] structured per-action logs -- [x] conflict resolution for common proxy DLL names - -## v0.3 — Graphics & VR modules - -- [x] OptiScaler -- [x] OFXR Bridge FrameGen -- [x] Cheeky Foveated DLSS ReShade add-on -- [x] OpenXR helpers -- [x] ReShade -- [ ] per-game VR / flat profiles -- [x] module state detection and version reporting - -## v0.4 — Mod frameworks - -- [ ] UE4SS -- [x] BepInEx -- [ ] REFramework -- [x] mod loaders / framework dependency graph -- [ ] compatibility metadata by game build - -## Later - -- [x] GitHub Releases updater -- [ ] signed/versioned remote catalog -- [ ] Nexus integration where permitted -- [x] Epic / GOG detection -- [ ] import/export profiles +Last rewritten on 2026-09-30, after a fourth pass. The previous three are preserved in +git history; the evidence lives in [docs/AUDIT-2026-09-29.md](docs/AUDIT-2026-09-29.md), and +every item below cites the audit ID it comes from. + +The passes after the audit have one finding in common worth stating once, at the top, +because it is the thing that kept happening: **the guards this project writes are usually +right about the class and wrong about the inventory.** The step-kind guard compared four of +five lists and the fifth had already drifted. The engine vocabulary now lives in five places +and the guard compares five. A baseline file in a validator is a sixth place a defect can be +named. Each time, the omission was found by a human reading the source, not by a tool. + +The fourth pass is the sharpest instance, and it is worth reading as a method rather than an +anecdote. Generalising two spec invariants — "a step may not reference an undeclared config +field" and "a check may only inspect what its recipe writes" — from one hand-picked recipe to +all nine cost three false positives before the guard was right, and every one of them was the +same mistake: + +- `ofxr-bridge` was reported as declaring `sha256` for nothing, while its `archive-sha256` + **check** reads exactly that field. The first version scanned steps and not checks. +- `bepinex` was reported as checking a file no step writes, when `extract-zip` is what puts + it there. An extracted archive is the one artifact whose whole tree is the product. +- The sibling rule, "every declared field is consumed", turned out to be **unstatable from + where the guard sits**: `configSchema` is the union of three consumers in three languages — + the chain, `src/features/modules/module-registry.ts` for the typed module commands, and + `build_metadata`, which records `version` on the transaction. Six of six available recipes + looked broken. + +That last one is the general form. **A guard that names a whole category as broken on the day +you write it does not get read the second time**, so the rule that can be stated honestly is +the one that ships, and the rule that cannot is written down as a known gap instead. The +lesson generalises to the three previous cases: find the whole inventory before writing the +check, not after it fails. + +## How to read this + +- **Now** — blocks the next beta. One item needs a maintainer's key; the rest are closed, and one of them was found by auditing a branch this roadmap had repeatedly deferred closing. +- **Next** — the current cycle. +- **Later** — real, but not yet earned. +- Every item has a **Done when** line. An item is not done until that line is true and reviewable. +- Effort is rough: **XS** < 1 h · **S** half a day · **M** 1–2 days. + +--- + +# Now — release blockers + +Every P0 and every F-03 remainder is closed. **One thing needs a key rather than a +change**, and one thing that was a change is now closed too — it was found by +cross-checking the branch this roadmap keeps postponing, which is the argument for +closing that item earlier than it wanted. + +- [x] **The bundled app had no frontend in it.** — new, found while auditing + `feat/agent-mcp` + `src-tauri/Cargo.toml` declared `tauri = { version = "2", features = [] }`. + Without the `custom-protocol` Cargo feature, `tauri`'s build script takes + `let dev = !custom_protocol` and emits `cargo:dev`; `tauri-build` then runs + `cfg_alias("dev", is_dev())` for the whole crate; and `Manager::get_app_url` + resolves the app URL to `build.devUrl` — `http://127.0.0.1:1420` — instead + of the embedded `frontendDist`. + + *This is not a defect that announces itself, which is why it survived + forty-seven commits of hardening.* `cargo check` passes, `cargo build + --release` passes, `beforeBuildCommand` runs, `dist/` is produced, the + installer builds, installs and launches. The only symptom is a blank + WebView2 on a user's machine. And it was not a theory: the release build + directory on this machine, dated earlier today, records + `cargo:rustc-cfg=dev` and has **no `tauri-codegen-assets` directory at + all** — the exact state the mechanism predicts. The sibling build from the + `feat/agent-mcp` branch, taken a week earlier, records `dev=false` and + fifty-six embedded asset files. + + It arrived on `feat/agent-mcp` as `ef9553c` and was never merged, because + the roadmap read that branch as "partly cherry-picked" product work rather + than as a fix that belonged on this one. **A merge is not the only way a + commit reaches a release, and "not in main" is not the same as "not + needed".** + + `scripts/check-bundle-features.mjs` now gates the manifest, and it walks + for every `Cargo.toml` rather than naming `src-tauri/Cargo.toml`, for the + reason at the top of this file: a guard is only as good as its inventory. + It declares no dependencies and runs through `node` directly, so it is the + first step in CI, before `npm ci`. *The limit is written into the guard + itself:* it reads the manifest, not the artifact, and cargo does not + re-run a dependency's build script when a feature flips — so a stale + target directory can hold the old binary. **The artifact-level proof is + the dry-run tag below**, and nothing short of it should be called + verification. + +- [ ] **Re-sign the community catalogue.** — `F-11`, XS, **by hand only** + `catalog.json` changed twice today — the revocations moved inside it, and the engine + vocabulary is now enforced against it — so it must be regenerated and signed once, with + the final format. `regenerate_catalog.py` refuses to sign without `MODDIN_SIGNING_KEY`, + which is not in this environment and must not be pasted into a chat or a commit. + Until it is signed the desktop app fails signature verification on the community fetch. + That is correct fail-closed behaviour, and it is release-blocking by hand. + *Also worth doing before announcing a release:* a dry-run tag. It is the first + installer built by CI with a digest computed from the artifact rather than typed beside + it, and the updater's signing setup is not complete either. + +Everything below was closed in this pass. It is kept rather than deleted, because the +reasoning is what the next person needs: + +- [x] **Capability engine integrity.** — `P0-1`, `P0-2`, `P0-3`, `F-02` + `download-file` in the built-in specs, the transaction opened before the step loop, + `bepinex`/`ue4ss`/`reframework` registered, steps constrained to the install root, and + `SKILL.md` corrected to say what is actually enforced — that a step cannot traverse out + of the root, and that the store will not back anything up outside it. Absolute paths + from `path`-typed config fields stay honoured, because those fields exist to name a + location outside the game folder; both the refusal and the exception are stated now. + +- [x] **The four step kinds the last two recipes needed.** — `P0-1` follow-ups + `extract-zip` gained a target subdirectory, `move-file` gained directory support, + `git-checkout` and `build-project` are new. `ofxr-bridge` is `available`; `uevr` stays + `planned` and its `safetyNotes` say why, which is not the reason the roadmap assumed — + the chain is correct, but UEVR publishes prebuilt archives from three repositories, so + its `backend` enum is a matrix one checkout and one build cannot express. + +- [x] **Trust: the pin, the cache, the fetch host, the revocations.** — `F-03`, all four + The key pin is checked against a compiled keyring. The cached catalogue is re-verified + against the signature on **each read** rather than trusting a `meta.json` flag. The YAML + fetch has a host allow-list. And the revocations now travel **inside the signed + `catalog.json`**, so one signature covers both halves and they cannot disagree — + before, `revoked-ids.json` was honoured over a connection the app verified and trusted + anyway, which meant an intercepted GET could suppress a revocation until the TTL + expired. The empty case is a table in `SECURITY.md`, not a paragraph. + +- [x] **The id-space bug, the missing descriptions, the dead commands.** — `P0-5`, `P0-6`, `F-14` + *F-14 gained a rule, and it is a rule rather than a fact:* the snapshot commands were + deleted for having no caller and then restored. Deleting a user-facing feature because + the wiring was outside an agent's file ownership trades a visible problem for an + invisible one. An unreachable command is a question — wire it, or say why it is not + wanted. Two deletions were the opposite call and both were right: ReShade's Rust module + was unreachable while the declarative path was live, and GOG discovery shipped in the + build with no caller at all, so its scan now runs where the UI actually calls. + +- [x] **The release job builds what it signs.** — `P0-8`, `O-06` + Plus a follow-up the notes move exposed: the job read its body from the repo root, where + the notes no longer are, and the next tag would have failed. + + *Done when:* `ofxr-bridge` is no longer `planned` because the engine lacked a step + kind. It is now `available`: download, extract to staging, promote the manifest, write + the tray, start it. Its `safetyNotes` name the one element this repository cannot + verify — the manifest filename inside the archive — and say that a wrong name fails + loudly and rolls back rather than installing nothing. + + `uevr` stays `planned`, and the reason is not the one the roadmap assumed. The chain + is correct; the recipe is not yet self-contained. UEVR publishes prebuilt archives + from three different repositories, so its `backend` enum is a repository matrix that + one checkout and one build command cannot express. Its `safetyNotes` say so. + +- [x] **Add a registry test that parses every `include_str!` spec** and asserts each install + chain's first step resolves to a local archive source. — `P0-1` guard + +### Trust and safety + +- [x] **Pin the community key against a compiled keyring.** — `F-03` (key pinning) +- [x] **Re-verify the cached catalogue before using its `downloadUrl`.** — `F-03` remainder + The cache stored a pretty-printed re-serialisation and no signature, and `fetch()` + copied a `signatureVerified` flag out of a local `meta.json` any writer could set. The + cache now keeps the raw bytes plus the signature and verifies against every + non-revoked key in the keyring on **each read**; the flag is derived from an actual + verification. Missing or corrupt signature is a cache miss, not a fallback. +- [x] **Allow-list the YAML fetch host.** — `F-03` remainder + The default adds `raw.githubusercontent.com`, because every `downloadUrl` in the + catalogue points there and matching `download-file`'s list byte for byte would have + made community installs impossible rather than safer. +- [x] **Honour `revoked-ids.json` in the app.** — `F-03` remainder + Fail-closed: no list, unreadable list, non-2xx and unparseable content all refuse. A + *network* failure falls back to the cached list, which is what `SECURITY.md` promises; a + *corrupt* list does not, because a bad upstream file is not masked by a stale one. The + kill switch is consulted before the signature, deliberately — a tampered list can only + add refusals, never remove one. A revoked capability stays visible with the maintainers' + reason rather than silently vanishing. +- [x] **Sign `revoked-ids.json`, or fold it into the signed catalogue.** — `F-03`, new + The revocation list was fetched over a verified connection but was **not itself signed**: + an attacker who can intercept that one GET could suppress a revocation until the TTL + expired, and the cache re-verification added in this pass does not help, because the list + is trusted precisely when it is stale. The revocations now live **inside** the + already-signed `catalog.json`, which closes it without a second signature to manage. The + "kill switch before the signature" ordering was deliberately kept even though both are + verified in the same read now: a tampered catalogue can only add refusals, never remove + one, and a revocation that is visible to an attacker who can suppress it is not a + revocation. *Consequence, by hand:* the published `catalog.json` now fails the drift + check until the maintainer regenerates and signs it — see `docs/UPDATER.md`. + +### Two broken user paths + +- [x] **Fix the id-space bug in the AI install flow.** — `P0-5` +- [x] **Render a human description on capability cards.** — `P0-6` +- [x] **Wire or delete the unused backend commands** — `F-14` + `capability_reload` is wired, so a locally dropped recipe shows up without a restart. + `get_compat_report`, `reshade`, `reshade_module`, `library_state` and seven orphaned + snapshot helpers are gone. `validate_recommendations_yaml` is **kept** — the audit was + wrong, it is called from the AI assistant. + *The snapshot commands were deleted for having no caller, then restored.* That is the + rule this item produced: an unreachable command is a question — wire it, or say why it + is not wanted. Deleting a user-facing feature because the wiring was outside the + current file ownership trades a visible problem for an invisible one. + *Two of the deletions were the opposite call and both were right.* ReShade's Rust module + was unreachable while the declarative path was live and registered, so it was + redundancy. GOG was the reverse: `refresh_installed_games_async` was the only caller of + `detect_gog_installed_games_blocking` and had no caller itself, so GOG discovery shipped + in the build and was unreachable in the app. The scan now runs where the UI calls. + +- [x] **Fix `create_snapshot`, which was not per-game.** — new, found while wiring the UI + The filter compared `record.game_id` against a helper returning `record.game_id.clone()` + — a tautology, under a `placeholder for future per-game filtering` note the original + author left. A snapshot of Elden Ring captured every applied transaction across every + installed game, and rolling it back undid mods in titles the user never named. It also + made the most destructive button in the app a silent no-op after one use, because + `rollback_snapshot` skips spent transactions and returns nothing. Both fixed; a spent + snapshot now disables its own button, and restore is deliberately *not* gated on + revocation — refusing it would strand files the user cannot remove. + +### Delivery + +- [x] **Make `release.yml` build the installer.** — `P0-8` + *Follow-up, found when the release notes moved:* the job read its body from + `.release-notes-.md` at the repo root, and the notes now live in + `docs/release-notes/`. The next tag would have failed. Fixed. +- [x] **Verify the bundled runtime.** — `O-06` +- [x] **Give the staged runtime a lockfile.** — `O-06`, second half + The blocker was that the generated manifest lives under a gitignored path — but the + lockfile that matters already existed and was already tracked: + `moddin-agent/package-lock.json`. The staged folder copies it in and installs with + `npm ci --omit=dev` against exact-pinned versions. Two consecutive runs produce an + identical `node_modules`: 3,869 files, same tree hash. Pinning the manifest alone would + not have been enough — it covers four direct dependencies and leaves ninety-one + transitives to the registry. + *Also found:* the script threw on Windows PowerShell 5.1, because `ConvertFrom-Json` + rejects the lockfile's `packages[""]` key. CI runs `pwsh` 7, so it would have shipped + green and broken for every maintainer on 5.1. + +--- + +# Next — current cycle + +## Quality gates + +- [x] **Add the missing quality gates.** — `F-12`, `O-07` + A pinned `rust-toolchain.toml` (1.98.1, the version `stable-msvc` resolved to, so the + build is unchanged and no longer floats) and `vitest`, now at **195 tests** over the + files where behaviour has broken before. +- [x] **Turn `fmt` and `clippy` into blocking gates.** — `O-07` + Done in the order that makes it meaningful: delete the dead adapters, fix the real + lints, *then* turn the gates on. The CI count of "144 / 30" was raw emissions including + duplicates across the lib, lib-test and example targets; deduplicated it was **81 unique + diagnostics, 23 lints, 34 of them `dead_code`**. The tree is now **zero** and both gates + block. + Six files went: the five `*_module.rs` adapters and `updater.rs`, each confirmed + unreferenced by a repo-wide search before deletion. The cascade was accounted for rather + than tolerated, and it found three real things — `archive.rs`'s SHA-256 layer could never + verify anything because `with_expected_sha256` was never called; `StepContext::spec` was + never read; and `ai_assistant_setup::restore_from_backup` is never called, so setup + writes a backup nothing restores. The third is a genuine gap in the config-write safety + net, left flagged rather than fixed here, because wiring it is a behaviour change. + `too_many_arguments` got the real fix: adjacent same-typed `&Path` parameters that + could be swapped silently and point the backup scope at the wrong root are one + `InstallScope` now. Four `#[allow]`s remain, each item-scoped and each with a `reason`; + no `[lints]` block was added, because the tree is clean without one and a blanket allow + would turn the gate into a switched-off guard. + +- [x] **Migrate the `ofxr-framegen` module cards.** — consequence of the deletion, new + Deleting `ofxr_module.rs` removed the crate's only `"ofxr-framegen"` string, which is the + pattern `validate-catalog.mjs` builds its module inventory from. Five catalogues were + advertising a card whose only backing was a file nothing dispatched to — the same dead + card `reshade` was. The module is now `ofxr-bridge` in all five, its dead config block + dropped, and it joins the capability-backed list in `App.vue`. No Rust shim was added to + satisfy the check; that would be creating dead code to turn a number green. + +## Recipe-kind parity + +- [x] **Sync the recipe-kind lists across every repo that declares one** — `F-05`, `F-06` +- [x] **Add a guard so the lists cannot drift again** — `F-05` + `scripts/check-capability-kind-parity.mjs` reads the lists and fails the build on any + disagreement. +- [x] **Cover the fifth source, and check params, not just names.** — `F-05`, new + `moddin-agent/src/mcp-server.mjs` keeps its own `STEP_KINDS` map, feeds it to the agent + through `get_step_kinds`, and was not in the guard. It had **already drifted**: two + step kinds and one check kind missing, and its `registry-write` entry documented + `keyField`/`nameField`/`typeField`/`dataField` while the Rust step reads + `key`/`value`/`type`. An agent authoring a recipe through MCP would have produced a + step that passed every kind check and failed at install time with "key is required". + + Kind parity turned out to be necessary but not sufficient, so the guard now also + verifies that every param name the map advertises is a name the runner reads. That + check was written per-function first and produced five false positives, because several + steps resolve their path through a shared helper whose literals live in another + function's body. It is file-global instead: weaker, but it cannot cry wolf, and a guard + people ignore protects nothing. + +- [x] **Guard the engine vocabulary too, and the gate's verdicts.** — new + `supportedEngines` is now enforced, which made the vocabulary load-bearing in the + community validator, which made it a **fifth** list. The guard compares four + vocabularies across five sources — step kinds, check kinds, engine ids, engine-match + verdicts — and every one of the four was found by a human reading the source. + +## Catalogue integrity + +- [x] **Gate capabilities on the engine.** — `F-09` + In Rust, because it is the only layer holding the spec while the list is built. A game + whose `enginePreset` no recipe names gets everything — `doom-2016` is `idtech` and + nothing Moddin ships targets it, so a strict gate there empties the page. An empty + `supportedEngines` means every engine, on the evidence of the repository's own template + and the only shipped community recipe; the opposite reading would have made that recipe + uninstallable everywhere. `elden-ring` declares no engine at all and is the flagship VR + title, so gating on an undeclared engine would hide UEVR from the game people most use + it with. Both are written down rather than left to the next reader. + +- [x] **Validate the catalogue in CI.** — new + `scripts/validate-catalog.mjs` checks a wrong `executable`, an `enginePreset` no preset + defines, a module whose id nothing implements, a game pointing at another game's + directory, and a game offering a module its own preset's description excludes — the + `dead-island-2` shape, read from the preset rather than matched by hand. It found eight + real defects and reports them by name; see **Later**. + +## Product surface + +- [x] **Profile import/export.** — Later item, closed + A versioned `moddin-profile` document, schema version checked *before* the parse because + a future-version file is well-formed JSON and would otherwise be read as v1 with fields + missing. Applying uses `capability_install` — the same command the cards use — so + preflight, transaction and rollback are the existing ones and there is no second install + path. Community capabilities are refused from a profile, because the signature check and + the revocation kill switch live in `community_capability_install`. The secrets rule is + one function called on both sides, so a hand-edited file cannot widen it. + *The roadmap's premise was wrong twice:* the storage layer is not a profile store, it is + a YAML shape over the catalogue module list with zero callers; and there is no file + dialog in this app at all, so export writes a named file under `%LOCALAPPDATA%` and + reveals it in Explorer. A real Save-As needs `tauri-plugin-dialog`. + +- [x] **Collections and Contribute.** — `O-04` + Ported from `feat/agent-mcp` and mounted. The branch drove a backend session with its + own install/resume/abort — 1,127 lines that would have been a second install path — so + the frontend is a loop over the existing `community_capability_install` instead, with + ordered steps, a stop-and-ask on failure, newest-first revert and a transaction record + per step. Two branch files were deliberately not ported: its `TransactionsView` is a + worse rendering of data the History panel already owns, and its `LibraryEmptyState` has + nothing left to own. *The loader has no content* — see **Later**. + +- [x] **A real CSP, and 23% off the installer.** — `F-13` + Verified rather than assumed: the app was launched and the header read out of WebView2 + over CDP with zero console entries. The interesting part is what is *absent* — the MCP + agent is not loaded into the webview at all, it is written into an external tool's + config and launched over stdio, so `node://` would have been decoration. The frontend + makes no `fetch`; every GitHub, catalogue and UEVR request is Rust-side `reqwest`. + *The size claim was pointing at the wrong file.* The 28.5 MB `node.zip` is not in the + installer — `bundle.resources` is an explicit list and 7-Zip finds zero zip entries. + What was shipping is **56 MB**: the bundler copied `moddin-agent/node_modules`, the + full dev install, renamed it `node_modules.old`, and never deleted the rename. 6,296 + entries, in every build, hidden behind a "best-effort" cleanup that had been failing + silently. Installer 31,705,095 → 24,403,351; unpacked payload 160,073,106 → 103,975,929. + *Costs accepted and written down:* `strip` removes the shipped symbol table, so a + user-reported crash yields no symbols, and `npm ci` now fails where `npm install` would + have limped. + +## UI/UX consistency + +- [x] **Tier 1 (XS).** — `UX-30`, `UX-13`, `UX-05`, `UX-04`, `UX-01` + Design tokens, `pre-line` on error paragraphs, verification passed to `ModuleCard` with + the duplicated checklist deleted, card state derived from verification and busy flags, + two labelled sidebar groups and a distinct glyph for IA local. +- [x] **Tier 2 (S).** — `UX-32`, `UX-09`, `UX-16`, `UX-07`, `UX-02` + One error path (`useFriendlyError` + `ErrorCallout`) replacing three classifiers and ten + raw `err.message` dumps; one `.empty-state` contract; `BaseDialog` as the only dialog + implementation, with `useDialogLifecycle` as its only focus entry point; the five AI + verbs collapsed into one. +- [x] **Tier 3: the localization boundary.** — `UX-21` + The four backend-authored identifiers now map to locale keys at the service layer, and + the capability-card description that was English in every locale is the thing it fixes. + The decision that mattered is the one about ids the table does not declare: they render + **the backend's own text, verbatim** — the command name, the recipe's own sentence, the + real I/O message. Not blank, not refused. A log row with no action is untriageable. + + The enforcement is the other half: every table is checked by set equality against the + source the ids come from — the persistent-action list, the literal `warnings.push` + strings in `openxr.rs`, the `Some("…")` values in `inspect_agent`, the recipe YAMLs. So + the fallback is reachable only by a string the app does not know about, and a renamed + recipe becomes a build failure rather than a stale entry. + + *What it does not cover, stated rather than implied:* a pt-BR card still shows English + in `safetyNotes`, `configSchema[].description`, `displayName` and each check's `detail`. + None is one of the four named identifiers; the config-field half is a separate item. + A community or AI-authored recipe keeps its own English — it is the author's copy. + +- [x] **Tier 3: the accessibility sweep.** — `UX-26`, `UX-27`, `UX-28`, `UX-29` + Per-item `aria-label`s on every repeated icon-only action, built from the visible text so + label-in-name holds for voice control; `.sr-only` labels on the activity-log controls; + `aria-required` / `aria-invalid` on config fields, wired to the real state rather than to + first paint, because a required field technically reads as missing before the user has + done anything; and `aria-busy` on six regions. The `ToastStack` was announcing every + message twice — a polite container plus alert children — and the container is now silent. + +- [-] **Tier 3: `UX-25`, closed as written.** — its subject no longer exists + There is no `role="menu"` anywhere in the tree. Tier 2 collapsed the five AI verbs into + one button and took the menu with it. Inventing a menu to give it a keyboard model would + be building the opposite of what Tier 2 did. + +- [-] **Tier 3: `UX-06`, closed as written, with a real gap found instead.** — its premise is stale + The three call sites do not share a spec-driven config form. The game page is the only + one that has one, and it is inline in the card rather than a dialog; the Community panel + installs with an empty config and has no form; the AI flow shows the field names + read-only beside a YAML editor. Extracting a shared dialog would have had one caller. + + *The gap it exposed is real and is now a Later item:* the Community panel cannot install + a recipe that declares required config fields, because it never collects them. That is a + feature, not a refactor, and it needs field labels first. + + +## Organization and delivery hygiene + +- [x] **Correct the docs that actively mislead** — `O-02`, `O-03`, `O-05` +- [x] **Add a docs index, delete the dead files, rehome the release notes** — `O-12` +- [x] **Write the conventions the history already follows** — `O-13` + Counted from the real history, not asserted. Two things it surfaced rather than + smoothed over: `chore/` has never been used for a branch, and `ci.yml` only triggered on + `main`, `feat/**` and `refactor/**` — so this `fix/` branch, and every `hardening/`, + `docs/` and `chore/` branch, shipped unverified until someone opened a PR. Fixed. +- [x] **Cache the `Bundle moddin-runtime` CI step** — `O-08` +- [x] **Stop tracking generated and wrong-platform weight** — `O-11` +- [x] **Document which of each `src-tauri/src/*` vs `*_module.rs` pair is authoritative** — `O-09` + [docs/BACKEND-MODULES.md](docs/BACKEND-MODULES.md). Five of the adapters are + unreferenced and `updater.rs` has never had a `mod` declaration in any commit, so it has + never been compiled. *Still open:* whether `module.rs` should survive once the adapters + go, and the Rust-command/YAML-capability duplication for `ofxr-bridge`, `optiscaler`, + `openxr-helpers` and `uevr` — both are live, and which one is right is a product call. +- [x] **Decide `App.vue`'s fate, and do it.** — `O-10` + Decomposed, not the budget raised. **110,278 → 73,213 bytes** on disk; the guard + measures 107,739 → 71,488 after CRLF→LF normalisation, so headroom went from ~7 KB to + ~43.5 KB with the 115,000 budget untouched. + + The seams were found before the sizes, and the largest one was the per-module action + table — the `if (module.id === ...)` chain running through preview, remove, update and + verify. That became a module the app *imports* rather than a branch it runs, and it now + has 23 tests holding it against the three things it claims to mirror: the catalogue, the + backend's `invoke_handler`, and the services that actually invoke each command. Two + module ids have already been dead cards in this repo's history — `reshade` and + `ofxr-framegen` — and both were found by a catalogue validator rather than by the guard. + That test is what makes the next one a build failure instead of a support ticket. + + It was proven by mutation, not asserted: dropping `ofxr-bridge` from the + capability-backed list fails with all five games that would have rendered a dead card. + And it caught a case while being written — `kharvox-vr` and `cheeky-foveated-dlss-uevr` + are `planned`, so their cards are disabled rather than dead, which is *why* the + assertion is on `available` modules and not on every id. + + Four behaviour bugs were found and left alone, which is the right call for a refactor + whose value is being reviewable: the hero progress bar counts modules the grid never + verifies so it cannot reach its total; UEVR never self-verifies; the update path writes + an untranslated detail string; and `updateModule` sets a `configured` flag nothing reads. + +- [ ] **Extract `feat/agent-mcp`, then delete the merged branches.** — `O-04`, corrected + The roadmap said the branch was "partly cherry-picked". It is not. `feat/agent-mcp` + holds **~1,500 lines that never reached main**: Collections (tab, install dialog, + `useCollectionInstall`, `types/collection.ts`), Contribute (dialog and trigger) and a + `TransactionsView`. The automated release code on it *is* superseded by `P0-8`; the + product line is not, and it renders nowhere. The decision taken is to port the product + line, not to discard it. Porting needs 60 locale keys across three languages, and the + components must be re-mounted — copying the files alone would leave dead components, + which is the same mistake as the ReShade card. + + The other two branches are safe: `feat/optiscaler-safe-install` (annotated `[ahead 82]`) + and `feat/capability-deps-and-build-compat` (`[ahead 2]`) have **zero** commits not in + `main`. Those annotations are stale upstreams, not unpublished work, exactly as the + roadmap guessed. `backup/pre-unify-20260912` and `feat/bootstrap-mvp` are both at a + commit fully merged into main. + +## Bugs found by the new test suite + +These were not on the roadmap. All are fixed, each with a test that fails without the fix. + +- [x] **`getMissingDependenciesForModule` returned the module itself.** `configureModule` + treats a non-empty result as "open the prerequisite dialog", so clicking Configure on + any uninstalled module showed a dialog listing the very module being configured and + never reached the install flow. Its own doc said the opposite. +- [x] **Escape closed a ref no template read.** A keyboard user could not dismiss a + destructive-action prompt at all — capability removal, the Local AI disconnect, history + undo. Those prompts are the ones this pass added. +- [x] **`actionCancel` existed in no locale file.** `ConfirmDialog`'s own default label and + the capability removal call site both rendered the literal text `actionCancel`. +- [x] **`ModuleCard`'s Check button stayed live during an install.** It disabled on + `verifyBusy` and `blockedReason` but not on the busy flag that disables every other + control beside it; the click was then swallowed silently. +- [x] **The topbar Contribute button dispatched a window event nothing in the repo listens + to.** A dead button, before this pass touched it. + +--- + +# Later + +- [-] **App self-updater** — `F-04`, `P0-4` · wired, waiting on one key + The code is done and it **refuses to install anything** while the signing key is a + placeholder. Verified by running it: `cargo check --release --lib` fails with E0080 and a + message naming the command and the document. The guard is gated on + `not(debug_assertions)` on purpose — an always-on version would leave the two blocking + gates permanently red, and a permanently red guard gets deleted. + + Three more refusals back it: both commands return `not-configured` before any network + call, the install re-checks and refuses if the release moved from the version the user + confirmed, and the error mapping is keyed on the plugin's error *variants* rather than + their strings, so upstream rewording cannot reclassify a signature failure into + something that looks recoverable. + + `release.yml` throws rather than skipping when the payload is incomplete. That step was + written to skip gracefully first, on the reasoning that a release before the updater is + configured should still produce an installer — the reasoning was wrong, and the skip + branch would have published a release whose update check could never succeed. + + **Left for the maintainer, in `docs/UPDATER.md`:** the pubkey in `tauri.conf.json`, and + the `TAURI_SIGNING_PRIVATE_KEY` / `TAURI_SIGNING_PRIVATE_KEY_PASSWORD` repository + secrets. A valid-but-wrong key is undetectable at build time, so the dry-run tag is the + guard. There is deliberately **no** build check against reusing the community keyring — + duplicating those private consts would create exactly the drifting list this repo keeps + getting bitten by. +- [-] **A second community capability, signed** — `F-11` + `community-specialk-tweak-profile` is authored, validated by the community validator and + exercises a transactional path (`write-text-file` + `move-file`) no first-party recipe + touches. It is **not** signed, and cannot be: signing is the maintainer's key. The recipe + is therefore refused at load time inside a collection, and installable only from the + Community panel with the signature gate in front of it. That is the intended shape for an + unsigned recipe, not a defect — but the trust chain is still not exercised in production + until a signed entry exists. *Blocked on the same key as the catalogue.* +- [-] **UE4SS and REFramework specs** — `F-10` + Split, and the split is the finding. **UE4SS is `available`**: its old blocker was the + "engine-generation matrix", which was wrong — v3.0.1 ships one runtime archive, verified by + downloading and listing the real release rather than trusting the docs. **REFramework is + still `planned`, now for a specific reason**: its layout is verified, but v1.5.9.1's + release note requires non-VR users to extract *only* `dinput8.dll`, and `extract-zip` + writes every member with no include/exclude param and no conditional step. Staging plus + `move-file` installs the flat-screen case correctly and silently gives a VR user no OpenXR + support and no autorun scripts. *Done when:* `extract-zip` can filter its members — + see the runner gap below. + + **The engine can now filter, and REFramework still does not ship.** The blocker moved + from "the runner cannot express this" to authoring, and the remaining work is specific + enough to name: a VR/no-VR config field the *user* answers, because the choice is + per-user and not per-game, so a `boolean` is the honest shape. Then the `include` + patterns have to be verified against a real v1.5.9.1 archive rather than against this + roadmap's summary of it — the flat case must be provably `dinput8.dll` alone, and the + VR case provably everything including the `reframework/autorun/**` tree, which the + release note warns can crash a non-VR game. + + *The shape the engine offers is a flat list, and that is the interesting part: the + recipe cannot branch on the answer itself.* It takes one step per case with a + conditional, or the collection picks the variant. Choosing between those is a product + call, and it is why this is still `planned` rather than a half-built recipe. +- [x] **A member filter on `extract-zip`.** — new, found while closing `F-10` + `include` and `exclude`, arrays of glob patterns over archive members. `include`, + when non-empty, selects; `exclude` then removes. `*` stays inside one segment, `**` + crosses any number, a pattern with no `/` matches the basename at any depth, and + matching is ASCII case-insensitive because the filesystem is. `?` and `[` are + **literals**: a Windows member name cannot contain them, so reading them as + wildcards could only ever silently drop a member the recipe asked for. + + *The filter runs before `payload` and the proxy rename, not after.* The filtering is + the point — a recipe that filters to one DLL still needs `payload` to name it, and the + two compose in that order. Both the zip and the 7z branch consume the same selection, + one by archive ordinal and one by member name, because a 7z reader hands its entries + over in block order. A filter that selects nothing is **refused**, naming both pattern + lists and the archive's member count, and nothing is written — a filter that silently + installed nothing is the exact failure this item exists to prevent. + + Seven tests, including one that asserts `plan_step_targets` and the run agree + member-for-member. **Preflight and the run sharing a function is the part that matters:** + they are two code paths, and a transaction that backs up different files than the step + writes is a rollback that restores nothing. + +- [x] **Environment-variable expansion in `render_template` / `resolve_path`.** — new + `%NAME%` for `[A-Za-z_][A-Za-z0-9_]*`, no `${VAR}` — this is a Windows app and + `%LOCALAPPDATA%` is the case that has to work. **A variable the process does not have + is left literal**, because a template is also prose and a file's content has `%` for + reasons that have nothing to do with the environment. `%%`, `%1`, `%PATH:1%` and a + trailing `%` all pass through untouched. + + *No new exemption in the install-root rule.* An expanded path lands in + `resolve_write_target` and gets exactly the treatment a literal one gets: absolute is + honoured, which is the pre-existing `path`-typed-field exception, and a `..` walk out + of the root is still refused — pinned by a test whose variable *is* `..`. The obvious + wrong fix here was to let expansion bypass the scope check because "the user put it in + a config field", and that would have turned the missing capability into a hole. + + `write-text-file` gains `format: json`: each `{param}` is inserted as escaped JSON + string **content** and the recipe keeps its own quotes and punctuation, and the + rendered result is parsed with `serde_json` **before the parent directory is created**. + That last part is the fix, not a nicety — the failure this item exists to end is a + Windows path rendered unescaped into a preference document that `read_game_preference` + then silently answers `None` for. A refusal beats a silent no-op. An unknown `format` + is refused rather than falling back to text. + + **The four stuck `ofxr-bridge` fields are unblocked, and `openxr-helpers`' two named + gaps are closed in the runner.** Neither recipe is flipped — see the two items above; + the remaining obstacle in both is authoring and a safety call, not a missing step. + + *A behaviour change beyond the item, flagged because the roadmap's own rule asks for + it:* `render_template` no longer consumes a `{…}` run it cannot resolve. The old + scanner took everything up to the next `}` and put it back, which for a JSON template + means the object's opening brace swallows the first placeholder — `{"runtimePath": + "{preferredRuntime}"}` rendered the whole `{"runtimePath": "{preferredRuntime` as one + unresolvable name. `format: json` is not implementable without the fix. The output is + byte-identical for a template that is nothing but placeholders; only the resume point + differs. The same reasoning forced a second fix in the recipe-wide field scanner, which + would otherwise have reported `{"runtimePath": "{preferredRuntime` as an undeclared + field and failed every recipe the JSON format exists to enable. + +- [x] **The default proxy rename collides.** — new, found while fixing `extract-zip` + With no `payload` declared, every archive member basenamed `reshade64.dll` or `dxgi.dll` + is renamed onto the same single proxy path, and the **last one wins silently**. Now + refused: `archive_member_targets` accumulates a destination → claimants map and returns + an error naming the destination, the members claiming it, and the fix (`payload`, or + `include`/`exclude` for the rest of the archive). + + *Determinism is the whole point, so it is tested as a property rather than an + outcome:* two archives with the same members in opposite order produce a byte-equal + message. The map is a `BTreeMap` keyed on the lowercased destination — because Windows + compares paths case-insensitively, so `DXGI.dll` and `dxgi.dll` are one destination — + with a `BTreeSet` of claimants inside it, and the lowest-spelled spelling kept for the + message. It also catches a rename landing on a member already named like the proxy, + which is the same defect arriving by a different route. A declared `payload` stays + immune, as it was. + +- [ ] **`chain_artifacts` does not know about the filter.** — new, found while closing the + member filter + The invariant that says "a check may only inspect what its recipe writes" reads + `extract-zip` params in `capability_runner.rs` and sets `extracts = true` for any + `extract-zip` step, recording only `target` and `targetSubdir`. So a recipe that + filters, plus a `file-exists` check on a member the filter removed, still passes the + guard and then fails its own check at install time. Nothing ships this way today — no + recipe uses a filter yet — so it is a latent hole, not a live one. + + Left open deliberately: fixing it means deciding what a *filtered* archive's artifact + set is, and that is an authoring-policy question, not a mechanical one. It is the + roadmap's own recurring shape — a guard that is right about the class and does not + know the inventory — arriving one level up from where the previous ones were found. +- [ ] **Per-game flat profiles.** No `flat` key exists anywhere in the catalogue yet. · M +- [ ] **Nexus integration** where permitted. Still genuinely open — no code, no URL. +- [x] **Collections need content.** Two shipped collections with real members, and a test that + reads the published files rather than a fixture — `every_built_in_collection_fits_the_game_it_targets`. + `feat/agent-mcp` had shipped the loader with zero collection YAML. The branch's 1,127-line + session runner stays unported on purpose: the frontend drives the existing + `capability_install`, so porting it would reintroduce the second install path this pass + removed. +- [x] **Let the Community panel install a recipe with required config fields.** — found + while closing `UX-06`. The panel renders the entry's own `configSchema` as a form and + installs with resolved values. Resolving went further than the item asked: the catalogue + `config:` block is now the source for both the panel and collections, through one shared + precedence rule in `src/features/capability-modules/config.ts`, and a required field with + no value is refused **before** any install call instead of failing inside the first step + the run had already begun. +- [-] **Translate `safetyNotes`, `configSchema[].description` and `displayName`.** — new + The localization boundary covers the four backend-authored identifiers and the card + description. These three were not. **The decision, and the part worth arguing about:** + `displayName` is in scope, and the other two are not. + + `displayName` is a short label behind a stable id, and the app already rewrites + `description` the same way, so it gets a `CAPABILITY_NAMES` table beside + `capabilityDescriptionKey` — same shape, same exhaustiveness guard, three locales written + by hand. + + `safetyNotes` and `configSchema[].description` stay in the recipe's own language, for the + reason this codebase already wrote down about community recipes: *"that text is the + author's, not the backend's, and the app does not get to rewrite what a person wrote."* + That was written to protect an outside author from being paraphrased. It applies with more + force to first-party prose that is mostly **warnings** — a note saying a registry write + needs UAC, or that a transaction rollback is partial, is text the reader is relying on. + A machine rendering of a safety warning is worse than an English one, because the reader + has no way to check it. Translating them is not a localization gap; it is a safety + regression wearing one. + + If that call is wrong, the honest way to reverse it is per-field opt-in with a human + reviewer on the copy, not a blanket pass. `check.detail` stays as-is either way: it is + data with an embedded version, not prose. +- [x] **A real Save-As in the profile import/export.** — new + `tauri-plugin-dialog`, pinned to the 2.7 line because 2.8 requires + `tauri = "^2.12"` and the graph is deliberately on 2.11.5. Export + opens a native save dialog and reports the path the file really + landed at, replacing a writer that picked + `%LOCALAPPDATA%\Moddin\profiles\exports\` on the user's behalf. Import + gains a Browse button that opens a native open dialog and drops the + answer into the path box. A dismissed dialog is not a failure: it + resolves to "nothing happened" and leaves the box and the preview + untouched. The webview gets no dialog permission, so a profile can + only be written to a path the user picked. + *The correction this item carried was the useful part:* the previous + wording claimed the storage layer "already exists" and implied only + a dialog was missing. The layer existed and had zero callers. +- [x] **Resolve the eight baselined catalogue defects.** — new + `scripts/validate-catalog.mjs` found them; they were reported by name rather than + suppressed, and all eight are now fixed. The validator reports **zero** open data defects + and `KNOWN_DEFECTS` is an empty list — with the record of the eight kept in the file above + the array, because an empty baseline otherwise reads as "nothing was ever wrong". + + They were one mistake in five costumes: the catalogue listing a module id nothing + implements. All eight were `status: planned`, so each rendered as a disabled "Planned" + card — nothing crashed, which is exactly why they survived. The `cyberpunk-2077` `uevr` + pair was the only real product call: the comment there had framed it as "teach the recipe + about REDengine, or drop the module" and left it open, and it is the second one, because + UEVR injects Unreal Engine and Cyberpunk 2077 is REDengine. Each removal leaves a comment + saying what would bring it back. *A baseline is a sixth place a defect can be named. Read + every line in it as a bug.* +- [ ] **OpenXR per-game runtime binding** — new, and deliberately not faked + The `openxr-helpers` recipe was `available` and wrote + `HKCU\...\OpenXR\1\per-game\{gameExecutable}\ActiveRuntime`, described as binding a runtime + for one game. The OpenXR loader specification puts `ActiveRuntime` under a single + machine-level key and states that runtime selection is handled external to the loader; + **there is no per-game registry key in it.** The recipe was writing a key no loader reads, + so it is `planned` with an empty chain and the evidence cited in its safety notes. The + two mechanisms that really do scope a runtime to one game — `XR_RUNTIME_JSON` per process, + and Moddin's own `%LOCALAPPDATA%\Moddin\profiles\openxr\.json` — are both outside + what a step can express: no step expands environment variables, and `write-text-file` has + no JSON escaping, so a Windows path rendered into the preference document is invalid JSON + and `read_game_preference` silently returns `None`. *Done when:* a step can expand an + environment variable and write JSON with escaping. **This is the second time a recipe + claimed an effect with no mechanism behind it, which is why it is an item and not a + footnote.** + + **Both named gaps are now closed in the runner, and the recipe still cannot ship.** Its + *Done when* line was written as though the capability were the whole obstacle, and it + was not: a step can now write a preference document at `%LOCALAPPDATA%` with a path + escaped into valid JSON, and the document still cannot be written. What is missing is + `gameId` — the recipe has to know which game's document it is writing — and + `run_uninstall` resolves no config, so the uninstall half cannot be expressed in a step + at all. Underneath both sits the question the roadmap already refused to guess at: + **what Undo does for a file outside the install root**, which the transaction store + will not back up. The empty chain is still the honest shape of a `planned` recipe, and + that is a better answer than a half-written one. + + *The generalisation is the reason this was worth an item rather than a note:* a recipe + that writes outside the install root is the whole difficulty, and until now the engine + could not even address such a path. It still cannot offer one for undo. + +--- + +# Shipped + +The accurate baseline, so the next person does not re-derive it. A full table lives in +`docs/SCOPE.md:83-103`. + +**Solid:** Steam + Epic + GOG discovery (GOG only became reachable this pass), game scan and +catalogue matching, the capability install pipeline with transactional undo, the community +catalogue fetch with Ed25519 verification and revocation travelling *inside* the signed +catalogue, the AI assistant and MCP agent shell, profile import/export, collections and +contribute, snapshot create/list/rollback/delete, engine-gated capability lists, a frontend +architecture guard, a parity guard comparing four vocabularies across five sources including +param names, a catalogue validator, a frontend test runner, a real CSP, a guard on the Cargo +feature a bundled build silently depends on, and compile-enforced +locale parity across pt-BR / en / es. + +**Known not solid:** the trust chain is not exercised in production at all right now, because +the catalogue has not been re-signed and the app is correctly refusing the community fetch; +the bundled app's frontend embedding was broken until this pass and its guard reads the +manifest rather than the artifact; catalogue breadth is still six games, one of which +(`doom-2016`) is a single-module near-duplicate of the `desktop-shortcut` every other game +carries; and the four UX-20 items the audit raised that this pass closed as *written* left +real gaps behind them, recorded at each item rather than here. + +*This paragraph was four separate lies a moment ago, and every one of them was true when it +was written.* It said `fmt` and `clippy` were reported-only — they have blocked since the +quality-gate pass. It said the collection loader had no content — it ships two collections +with a test that reads the published files. It said the catalogue carried eight known data +defects — `KNOWN_DEFECTS` is an empty list with the record of the eight kept above it. It +said there was no file dialog — `tauri-plugin-dialog` landed with a real Save-As this pass. +**A summary of what is not solid is the first thing to go stale and the last thing anyone +re-reads**, because nothing makes it false: a defect that gets fixed leaves the sentence +behind, still true-looking, describing a repository that no longer exists. The items it +contradicted are all `[x]` above, and the honest rule is the one already written as standing +constraint 5 — the summary has to be rewritten in the change that moves the code, and the +change that moves the code is the only moment anyone is looking at it. + +--- + +# Standing constraints + +Recurring sources of drift, recorded so they stay visible: + +1. **A list of valid step kinds lived in five places across three repositories, and nothing + kept them in sync.** It drifted twice. The first guard covered four of the five; the + unguarded one had already gone stale on its own. *A guard is only as good as its inventory + of sources* — the fix for the next one is to find the fifth list before writing the + check, not after it fires. +2. **The signing key is a Rust constant**, so rotation requires shipping a binary — and the + one time it was needed, users downloaded a knowingly broken installer. +3. **`App.vue` has ~5 KB of headroom** against its own CI budget. +4. **The bundle step gates `cargo test`**, so every Rust check depends on nodejs.org and + registry.npmjs.org being reachable. Now cached, but still a hard dependency. +5. **Docs drift silently.** Six documents made claims the code contradicted; they are + corrected, and `docs/README.md` indexes what remains. The standing fix is still to update + docs in the same change that moves the code. +6. **Deleting a command is not the same as resolving one.** An unreachable command is a + question: wire it, or say why it is not wanted. Deleting it because wiring it was outside + the current file ownership trades a visible problem for an invisible one. diff --git a/catalog.json b/catalog.json deleted file mode 100644 index 7d27a7c..0000000 --- a/catalog.json +++ /dev/null @@ -1,40 +0,0 @@ -{ - "version": 1, - "generatedAt": "2026-09-22T17:38:00Z", - "generator": "scripts/regenerate-catalog.py", - "capabilities": [ - { - "id": "community-fps-unlocker", - "version": "1.0.0", - "homepage": "https://github.com/petonexus/moddin-community-capabilities/tree/main/capabilities/community-fps-unlocker", - "downloadUrl": "https://raw.githubusercontent.com/petonexus/moddin-community-capabilities/main/capabilities/community-fps-unlocker/capability.yaml", - "configSchema": [ - { - "name": "downloadUrl", - "type": "url", - "required": true - }, - { - "name": "sha256", - "type": "sha256", - "required": true - }, - { - "name": "version", - "type": "string", - "required": true - }, - { - "name": "extractedDir", - "type": "path", - "required": true - } - ], - "safetyNotes": [ - "FPS Unlocker modifies the game\u0027s rendering loop; some games\u0027 anti-cheat will flag it.", - "Always close the game AND any overlay before applying or removing." - ], - "signed": false - } - ] -} diff --git a/docs/AUDIT-2026-09-29.md b/docs/AUDIT-2026-09-29.md new file mode 100644 index 0000000..e38b5c3 --- /dev/null +++ b/docs/AUDIT-2026-09-29.md @@ -0,0 +1,781 @@ +# Moddin Desktop — UI/UX, Functionality & Organization Audit + +**Date:** 2026-09-29 · **Commit:** `5168ee1` (`main`, in sync with `origin/main`) · **Tree:** clean +**Scope:** whole repo — Vue frontend, Rust backend, catalog, docs, CI/delivery. + +This audit exists to correct [ROADMAP.md](../ROADMAP.md). It found that the roadmap currently +disagrees with the code in both directions: several items marked done are not done, several items +marked open are shipped, and a large amount of shipped work (the AI assistant, the MCP agent, the +community catalog) appears nowhere. + +--- + +## How to read this + +| Severity | Meaning | +| --- | --- | +| **P0** | Ships a broken promise to the user, or loses data. Blocks the next beta. | +| **P1** | Real friction or a real risk. Should land in the current cycle. | +| **P2** | Polish and consistency. Batch it; do not let it starve P0/P1. | + +Evidence: every finding carries `path:line`. Claims marked **[verified]** were confirmed by +reading the code during this audit. Claims marked **[risk]** are inferred from a code path that was +not executed. + +--- + +## Summary + +Three things dominate everything else: + +1. **The declarative engine — the product's entire thesis — cannot install anything.** + 5 of 6 registered built-in specs fail at their first install step. The one spec written + correctly (`bepinex`) is not registered. "Adding a new mod is one YAML file" is currently false. +2. **The transactional-undo promise does not hold on the capability path.** The transaction record + is created *after* the files are written, so overwritten game files are backed up in their + already-modified state, and a mid-install failure leaves orphaned writes with no rollback record. +3. **The UI and the docs are considerably healthier than the pipeline.** Locale parity is + compile-enforced, the architecture guard is real, 208 Rust tests exist, and the design system is + coherent. The damage is concentrated in the Rust capability runner and in delivery. + +| Area | P0 | P1 | P2 | +| --- | --- | --- | --- | +| Capability engine / backend | 4 | 6 | 3 | +| UI / UX | 2 | 14 | 11 | +| Docs / delivery / process | 2 | 9 | 7 | +| **Total** | **8** | **29** | **21** | + +--- + +# Fix pass — 2026-09-29 (block 3: recipe authoring, and the catalogue's own claims) + +Same branch. **235 Rust tests, `vue-tsc` clean, both guards passing, `npm run build` +green, the community validator green.** + +| Audit ID | Status | What changed | +| --- | --- | --- | +| **F-06** | Done | `src/types/capability.ts` declared **5** of the 11 step kinds the runner implements. The union is the contract the AI/MCP authoring path types recipes against, so six step kinds — including `download-file`, the one every corrected recipe now depends on — were untypable there. Now 11, with a comment pointing at the parity guard. | +| **F-05** | Done, and wider than the audit said | The community validator was missing `download-file` and `exe-version`. But the **agent-side JSON schema already had all 11** — so the drift was three-way, not two-way, and the TypeScript union was the worst offender. All four sources now agree, enforced by a new check. | +| **New** | `scripts/check-capability-kind-parity.mjs` | The durable fix. Reads the four lists — `builtin_steps.rs`, `builtin_checks.rs`, `capability.ts`, the agent's `capability.schema.json`, and the Python validator — and fails the build if any disagree. Zero dependencies, like the existing architecture guard. **Verified by deliberately removing `download-file` from the Python set: the guard failed, and passed again once restored.** | +| **F-07** | Already resolved in block 1 | `uninstallSkipsFiles` existed only in `cheeky-foveated-dlss.yaml`, which block 1 rewrote. Nothing in the repo uses it now, and the agent schema never declared it. | +| **F-08** | Done | `archive-sha256` is a **blocker** that ran while the UI rendered a module card, and it downloaded the entire archive to answer "should this button be enabled". Added `RemoteArchive::content_length()` (a HEAD request) and a 64 MB preflight ceiling: above it the check reports "verified during install" and skips the download. That is not a weakening — the `download-file` step re-verifies the same `sha256` config field, with the archive already on its way to disk. | +| **F-11** | Done, and it was four bugs, not one | The shipped `community-fps-unlocker` capability had: a `kill-process` with `processName: '{{gameExecutableBaseName}}'` (no engine has `{{...}}` syntax — the literal would never match a process); an `extract-zip` pointed at a **URL** (the P0-1 bug, reproduced in the community catalogue); a `move-file` with identical source and destination; and an `uninstall` that `file-delete`s a **directory**. The validator passed all four. | +| **New** | Validator rules, not just a fix | Three new checks in `validate_capability.py`, each written for the class rather than the symptom: a `{placeholder}` in any string param must name a declared config field; `extract-zip` must not read a field declared `type: url`; `move-file` must not have the same source and destination. All three now fail on the capability as it was, and pass after the fix. | +| **F-10** | Done | `dead-island-2.yaml` declared `enginePreset: re-engine` while shipping UEVR, OFXR and Cheeky modules that the `re-engine` preset's own description says it does not target. Dead Island 2 is Unreal 5. Corrected. | +| **F-10 (docs)** | Done | `SCOPE.md` claimed VR launch profiles for five games (two have one), listed ReShade host and Auto-update as shipped (neither is reachable from the UI; there is no self-updater), and called catalog breadth "In progress" without saying six games include a one-module stub. `references/game-support-matrix.md` listed **four** games and omitted Elden Ring and Cyberpunk 2077 — the two flagship titles. Both rewritten against the YAML. | + +### Two things a maintainer has to do by hand + +1. **`catalog.json.sig` is now stale.** The community catalogue changed, and + `regenerate_catalog.py` refuses to sign without `MODDIN_SIGNING_KEY`. Until + it is re-signed, the desktop app will fail signature verification on the + community fetch. That is the correct fail-closed behaviour and it is + *visible* — but it is a release-blocking manual step, not something this + change can finish. +2. **The community capability is now `status: planned`.** Its install chain is + structurally correct (download before extract, digest checked before + anything is written), but the archive layout of an FPS Unlocker build is not + something this repository can assert — and the previous version of the file + asserted it and shipped an install that could not run. It stays `planned` + until someone verifies it against a real game. + +### What this block is really about + +Blocks 1 and 2 fixed the *runtime*. This block fixed the *authoring side*, and +the pattern is the same both times: a list of valid step or check kinds lives +in four places across three repositories, and nothing kept them in sync. The +drift was found by a human auditing the source, twice. The parity check makes +the third occurrence a red build instead. + +# Fix pass — 2026-09-29 (block 2: trust, safety and delivery) + +Same branch. Final state: **235 Rust tests passing, `vue-tsc` clean, architecture +guard passing (21 Vue files), `npm run build` green, 393 locale keys identical +across pt-BR / en / es.** Still uncommitted. + +| Audit ID | Status | What changed | +| --- | --- | --- | +| **P0-8** | Done | `release.yml` gained a `windows-latest` build job: checkout → `npm ci` → `npm run build` → `bundle-runtime.ps1` → `npm run tauri build -- --bundles nsis` → hash → upload-artifact. The `release` job now `needs: build`, downloads the artifact, and appends the CI-computed SHA-256 to the release body. The digest and the artifact can no longer disagree, because they come from the same run. | +| **O-06** | Done | `bundle-runtime.ps1` now fetches Node's published `SHASUMS256.txt`, refuses to install when the archive is not listed, and aborts on a digest mismatch instead of extracting it. The script's header also no longer claims Node is optional — step 3 shells out to `npm install`. | +| **F-03** | Done, partially | The on-disk `pinned-public-key.bin` is no longer trusted by itself. It is now validated against a compiled **keyring** (`TRUSTED_PUBLIC_KEYS_B64`), which holds the active key plus the one retired in the 2026-09-24 rotation. A pin that does not match is discarded, logged, and replaced by the anchor. A `REVOKED_KEY_FINGERPRINTS` list is in place for the case where a key in the ring is compromised. Four tests cover it, including one that would be vacuous if the "attacker" key failed to decode. | +| **UX-12** | Done | The AI install path no longer hardcodes `acceptUnsigned: true`. It fetches the verified community catalog, splits the batch into signed and unsigned, and **fails closed** on the unsigned ones with a message naming them and pointing at the Community panel. Consent is now persisted per capability id (`recordUnsignedConsent`) and written by the same checkbox the Community panel already showed — so agreeing there also unblocks the AI path, and only that. | +| **UX-11** | Done | New `ConfirmDialog` built on `BaseDialog` (so focus trap, Escape, focus restore and `aria-modal` are inherited, not re-implemented). Wired into all four unconfirmed destructive actions: capability removal, the Windows-wide OpenXR runtime switch, Local AI disconnect, and History undo. Added a `.btn-danger-solid` variant, which the design system did not have, plus `.confirm-details` / `.confirm-footnote`. | + +### What is deliberately still open + +- **`F-03` is only half of F-03.** Two of its four sub-items are not done: + `community_candidate_install` still resolves `downloadUrl` from the **cached, + unsigned** catalog, and the YAML fetch still has no host allow-list. Both are + now bounded by the keyring fix (a cached catalog is only re-accepted when the + key that verified it is in the ring), but neither is the real fix. +- **`revoked-ids.json` is still unread by the app.** The key-level revocation + list (`REVOKED_KEY_FINGERPRINTS`) is wired; *capability*-level revocation is + not, and the community repo already maintains the file. +- **The OpenXR and Local AI confirmations are new UI paths that no screenshot + test covers** — this project has no frontend test runner at all (`F-12`). + They are typechecked and the locale keys are compile-enforced, but nothing + asserts the dialog actually appears. +- **The `description` lines on capability cards are English in every locale.** + That was already true of the rest of the backend-authored strings; it is part + of `UX-21`, not a regression from this work. + +### One thing worth knowing about the delivery fix + +`release.yml` now builds, which means the **first tag pushed after this lands +will produce a real CI-built installer** — and it will be the first one whose +SHA-256 in the release body was not typed by hand. The maintenance step in +`.release-notes-*.md` that documents comparing digests becomes checkable for the +first time. Worth pushing a dry-run tag before announcing a release. + +# Fix pass — 2026-09-29 (block 1: capability engine) (block 1: capability engine) + +Work landed on branch `fix/capability-engine-integrity`. Verified with +`cargo test --lib`, `npm run typecheck` and `npm run check:architecture`, all green. +Nothing is committed yet. + +| Audit ID | Status | What changed | +| --- | --- | --- | +| **P0-1** | Done, partially by design | `optiscaler`, `reshade` and `cheeky-foveated-dlss` now fetch into the download cache before extracting, and use `extract-zip`'s native `proxyField` instead of a `move-file` that pointed a URL at a rename. `ofxr-bridge` and `uevr` are `status: planned` with the missing step capability documented — see below. | +| **P0-2** | Done | `execute_install` now plans each step's targets, extends the transaction **before** the step runs, and rolls back on failure. New `transaction::add_files_to_transaction` lets the record grow as the install progresses (an archive cannot be inspected until the previous `download-file` step has fetched it). Covered by three new tests. | +| **P0-3** | Done | `bepinex`, `ue4ss` and `reframework` are registered. The six-spec list became one `BUILT_IN_YAML` const used by both `load()` and `load_with_local_dir()`, which had drifted. | +| **F-02** | Done, narrower than proposed | `..` traversal out of the install directory is now refused outright, with a test. **The audit's suggestion to also require `allowOutsideRoot: true` for absolute paths was wrong** — `path`-typed config fields exist precisely to name a location outside the game folder, and an existing dependency test relied on it. Absolute paths stay allowed; the transaction store still refuses to back up anything outside the install root, and the runner reports those paths as not covered by Undo. | +| **P0-4** | **Retracted** | See the correction in place. The commands existed all along. | +| **P0-5** | Done | Both AI entry points already passed a *catalog* id; `resolveInstallTarget` matched it against a *store* app id. Added `findInstalledGameForCatalogGame` (the inverse of the existing `findCatalogGameByInstalledGame`) and renamed the parameter to `catalogGameId` so the contract is explicit. | +| **P0-6** | Done, and the prescription was wrong | The `description` field did not exist anywhere. Added it to `CapabilitySpec`, `CapabilitySummary`, the TypeScript interfaces and all nine YAMLs. | + +### Two decisions worth reviewing + +**`ofxr-bridge` and `uevr` are now `status: planned` rather than fixed.** The +audit treated P0-1 as a YAML-only fix, but their install chains need step +kinds the engine does not have: `ofxr-bridge` extracts to a staging directory +and promotes one named file into the OpenXR `ApiLayers` folder (`extract-zip` +has no target-subdirectory option and `move-file` cannot move a directory), +and `uevr` ships as a git tag that has to be built, so no download or extract +step can consume `pinnedTag`. Marking them `planned` stops them advertising an +install that cannot work. It removes nothing the user can do today — the +library grid still installs both through the working `ofxr.rs` / `uevr.rs` +modules. *(Corrected after the fact: `uevr_module.rs` is one of five +`src-tauri/src/*_module.rs` adapters that nothing references; the live +path is `uevr.rs`. See [BACKEND-MODULES.md](BACKEND-MODULES.md).)* +Each spec carries a `safetyNotes` entry naming the exact step +capability it needs. + +**The `uninstall:` file steps were removed from three specs.** `run_uninstall` +executes them with `ResolvedConfig::default()`, so any step naming a config +field failed with "config field is missing" — `optiscaler`'s uninstall could +never have run. With the transaction now recording every file a step touches, +rollback is both complete and correct, so the partial file-delete steps were +redundant as well as broken. + +### Also fixed in passing + +- `run_spawn_process` was pushing `pid:NNN` into `affected_paths`, making a PID + look like a file to anything consuming that list. Removed. +- `run_move_file` silently renamed a path onto itself. Now rejected. +- `test_support::env_lock` used `.expect()`, so one failing env test poisoned + the mutex and turned into twelve unrelated failures. Now recovers from + poisoning, so a real failure stays a single failure. +- A YAML scalar containing `": "` silently becomes a mapping. This bit twice + during this pass, and it is the most likely authoring mistake for anyone + writing a `safetyNotes` entry. The parse panic now names the offending + recipe. + +# P0 — Release blockers + +## P0-1 · The capability install pipeline fails on its first step for 5 of 6 built-in specs + +**[verified]** `run_extract_zip` resolves the archive with `resolve_download_path` +(`src-tauri/src/builtin_steps.rs:711`), which only special-cases a **bare filename** and otherwise +falls through to `resolve_path` (`:686`) — that is, `base.join(candidate)`. Five registered specs +point `archivePathField` at a **URL**: + +| Spec | Registered | First install step | Result | +| --- | --- | --- | --- | +| `ofxr-bridge.yaml:84-87` | yes | `extract-zip archivePathField: downloadUrl` | fails | +| `optiscaler.yaml:50-54` | yes | `extract-zip archivePathField: downloadUrl` | fails | +| `cheeky-foveated-dlss.yaml:44-48` | yes | `extract-zip archivePathField: downloadUrl` | fails | +| `reshade.yaml:50-54` | yes | `extract-zip archivePathField: downloadUrl` | fails | +| `uevr.yaml:48-51` | yes | `extract-zip archivePathField: pinnedTag` (a git tag) | fails | +| `openxr-helpers.yaml:40-46` | yes | `registry-write` | works | +| `bepinex.yaml:38-48` | **no** | `download-file` + `extract-zip` | correct but unreachable | + +The maintainer already knew: the `run_download_file` doc comment in `builtin_steps.rs` says +*"Until this kind existed, `extract-zip` could only read a path from config, so every recipe that +pointed that field at a URL failed with a file-not-found at install time."* The `download-file` step +was then added to exactly one spec — the one that is not registered. + +`move-file` is misused the same way in four specs (`optiscaler.yaml:58`, `reshade.yaml:58`, +`uevr.yaml:56`, `ofxr-bridge.yaml:91`), and `ofxr-bridge.yaml:91` moves from `extractedDir`, a +staging directory that is never created. + +**Why it matters:** `docs/SCOPE.md:11` and `docs/MODULES.md:16-18` promise that adding a mod is one +YAML file. It is not. This is the single most damaging finding in the audit. + +**Fix:** add the `download-file` step to the five specs (copy the shape from `bepinex.yaml:39-44`), +correct the `move-file` fields, and create the `extractedDir` staging directory in `extract-zip`. +Estimate: < 2 h. + +## P0-2 · Backup is taken *after* mutation — Undo cannot recover overwritten game files + +**[verified]** `src-tauri/src/capability_runner.rs:266-314`: + +```rust +for step in &spec.install { + let result = builtin_steps::execute_step(step, &step_context)?; // mutates the game folder + affected.extend(result.affected_paths.clone()); +} +let transaction = if !affected.is_empty() { + let record = transaction::begin_file_set_transaction(...)?; // snapshot AFTER +``` + +Consequences: + +- Any pre-existing game file the archive overwrote is snapshotted **in its new state**. `restore_record` + then restores the mod's file — the original is gone permanently. +- If a step fails mid-loop, `?` aborts **before** any transaction is created. Files already written stay + in the game folder with **zero rollback**. +- The legacy modules get this right: `optiscaler.rs:575` opens the transaction before the apply loop + at `:587`. The capability path is a regression against the system it was meant to replace. + +`docs/SCOPE.md:98` claims "Transactional Undo | Stable" and the app's own store description claims +*"Every change is previewed, backed up, and reversible"*. Both are false on this path. + +**Fix:** open the transaction before the step loop, precomputing the target set from the spec rather +than from post-hoc `affected_paths`. Add a test — there is currently **no** test covering the +step→transaction ordering. Estimate: ~2 h incl. test. + +## P0-3 · BepInEx, UE4SS and REFramework are invisible and uninstallable + +**[verified]** `src/App.vue:296` removes them from the library grid: + +```ts +const CAPABILITY_BACKED_MODULE_IDS = new Set(['bepinex', 'ue4ss', 'reframework']) +``` + +expecting a capability card to take over (`App.vue:306` filters them out of `libraryOwnedModules()`). +But `CapabilityRegistry::load()` (`capability_runner.rs:37-43`) registers only six `include_str!` +specs, none of them those three. The comment in `useCapabilityModules.ts:45` claiming built-ins like +BepInEx "surface as a card" is false. + +Net result: these three render **nowhere**. `ROADMAP.md:44` marks BepInEx `[x]`. `ue4ss.yaml` and +`reframework.yaml` are `status: planned` with no `install:` block, so `[ ]` is correct for them. + +**Fix:** register the three specs in `capability_runner.rs:37-43` (and finish `ue4ss`/`reframework` +specs, or leave them `planned`). Also note `unity.yaml` — the only engine preset declaring +`bepinex` — is referenced by zero games, so BepInEx is not applicable to any catalogued title yet. +Estimate: < 30 min for BepInEx. + +## P0-4 · **RETRACTED — false alarm.** The commands exist. + +> **Corrected 2026-09-29 during the fix pass.** This finding was wrong. +> `open_external_url` and `open_web_url` both exist in `src-tauri/src/lib.rs:84` +> and `:96`, are registered in `generate_handler!` at `:168-169`, validate the +> scheme and reject credential-embedded URLs, and are covered by tests at +> `:237-246`. The audit agent reported them missing because it looked for a +> command module instead of reading `lib.rs`. The fix pass initially +> reimplemented them in a new `open_link.rs`, which collided with the real +> ones and was removed. +> +> What survives from this finding: those commands, plus `get_compat_report` +> (529 lines), the four snapshot commands, `install_reshade` / +> `preview_reshade` / `uninstall_reshade`, `detect_gog_installed_games`, +> `get_cached_installed_games`, `refresh_installed_games_async` and +> `validate_recommendations_yaml`, are **registered but never called from the +> frontend**. That is dead code to wire or delete, not broken links. Moved to +> **F-14**. + +Also real, and still open: `updates.rs` + `updater.rs` are a **per-module +version check**, not app self-update. There is no `tauri-plugin-updater` in +`Cargo.toml` and no `plugins.updater` in `tauri.conf.json`, so +`ROADMAP.md`'s "GitHub Releases updater | [x]" is misclassified. `capability_reload` +*is* registered (`lib.rs:120`); the issue there is only that no UI calls it. + +## P0-5 · The AI "install recommendation" flow cannot work from the per-game banner + +**[verified]** Two different id spaces are both called `gameId`: + +- `App.vue:1952` passes `selectedGame.catalog.id` (e.g. `elden-ring`) to `AiGameSuggestions`. +- `App.vue:121` passes the real `selectedAppId` (e.g. `1245620`) to the topbar path. + +`AiGameSuggestions.vue:24` forwards it into `useAiAssistant.ts:393` → `:448` calls +`resolveInstallTarget(context.value.gameId)` → `capability-modules/service.ts:47` does +`games.find((game) => game.appId === appId)`. A catalog id never equals a store app id, so it +returns `null`, and every install ends at `useAiAssistant.ts:460` with the hard-coded **English** +constant `NO_GAME_SELECTED`. + +The flagship flow — "ask the AI what to install, then install it" — fails 100% of the time from the +most prominent entry point, with an untranslated message. + +**Fix:** one typed id. A branded `StoreAppId` / `CatalogGameId` pair plus a single resolver closes +this and makes the related `acceptUnsigned: true` below structurally impossible to forget. Estimate: ~2 h. + +## P0-6 · Community and AI mod cards show their machine id as the description + +**[verified]** `src/features/capability-modules/CapabilityModulesSection.vue:224`: + +```vue +:description="capability.id" +``` + +`ModuleCard.vue:67` renders this as `

` — the most prominent string +on the card. Built-in modules get a real benefit line from the locale (`pt-BR.ts:203`); every +community or AI-authored mod instead displays e.g. `cheeky-dlss-foveated`. + +This directly violates the documented voice rule in `docs/FRONTEND.md:237` ("outcome before +mechanism") and is the exact opposite of what the rest of the app does. + +**Fix:** ~~render `spec.description`~~ — **corrected during the fix pass: the +field did not exist.** Neither `CapabilitySpec` nor `CapabilitySummary` had a +`description`, no built-in YAML declared one, and the locale only covers +built-in modules. The field had to be *created* (Rust struct + `#[serde(default)]`, +TypeScript interface, all nine YAMLs) before the card could use it. That makes +the original diagnosis right and the original prescription wrong: a community or +AI-authored mod had no way to write a human line at all, which is why the id +was the only option available. Now implemented. + +## P0-7 · README and CONTRIBUTING tell users to download a file that does not exist + +**[verified]** `README.md:44` and `CONTRIBUTING.md:181` both say "run the `.msi`". +`src-tauri/tauri.conf.json:30` declares `"targets": ["nsis"]`, and +`.release-notes-v0.1.0-beta.5.md:57` ships `Moddin.Desktop_0.1.0_x64-setup.exe` (NSIS). + +A new user following the README looks for a file that was never published. + +**Fix:** three line edits. Estimate: 5 min. + +## P0-8 · `release.yml` builds nothing — every shipped installer was hand-built + +**[verified]** `.github/workflows/release.yml:18-43` runs on `ubuntu-latest` and does exactly one +thing: `gh release create "$TAG" --notes-file "$notes"`. No build step, no artifact, no checksum. + +The installer users are told to download was produced on a maintainer's machine. This is visible in +the release notes themselves: `.release-notes-v0.1.0-beta.5.md:64` documents that the shipped binary +had to be **replaced after first upload** because of a broken Codex integration, with users told to +re-download and compare digests. + +The automated build already exists but was abandoned on the unmerged `feat/agent-mcp` branch +(`7f82b8f`, MSI-based, since superseded by the NSIS switch). + +**Why it matters:** every other fix in this audit only reaches users if the pipeline ships. Right now +it does not. This is a trust problem before it is a tooling problem. + +**Fix:** add a `windows-latest` build job (`bundle-runtime.ps1` → `npm run tauri build` → +`gh release upload`), with the produced SHA-256 written back into the release body. Estimate: M. + +--- + +# P1 — UI / UX + +Grouped by theme. Full inventory and evidence in the companion notes; these are the ones that change +the roadmap. + +## Information architecture + +- **UX-01 (P1) — ten entry points into six modals, with no grouping.** + `GlobalTools.vue:15-19` renders five undifferentiated sidebar rows (AI assistant, VR system, + Local AI, Community, Activity) plus a topbar split button and a per-game banner. "Assistente de IA" + and "IA local" share the **same icon** (`AppIcon.vue:10` has one `ai` glyph), so the two most + similar entries are the hardest to tell apart. *Fix: distinct icon, and group the five under two + labelled sections (Gerenciar mods / Ajuda e diagnóstico).* + +- **UX-02 (P1) — the AI surface has five different verbs for one dialog.** + `AiAssistantTrigger.vue:7` opens `author`; `AiTopbarMenu.vue:69-94` offers ask/recommend/audit/ + diagnose/contribute; `AiGameSuggestions.vue:22-36` offers recommend/audit; `useAiModuleActions.ts` + adds improve/diagnose. The sidebar item defaults to the least discoverable mode. *Fix: one verb + ("Ask the AI") at every entry point; the mode picker inside the dialog is the only place modes are named.* + +- **UX-03 (P2) — the per-game AI banner is a nag that reappears on every game.** + `AiGameSuggestions.vue:17` keeps `dismissed` in component-local state, and `App.vue:1950` remounts + it per game. *Fix: persist the dismissal through `services/storage`.* + +## Consistency + +- **UX-04 (P1) — capability cards can only be in 2 of the 6 states the shared card supports.** + `CapabilityModulesSection.vue:225` passes `isInstalled(...) ? 'active' : 'available'`, while + `ModuleCard.vue:44-53` also supports `attention`, `checking`, `planned`, `unknown`. A community mod + whose checks fail looks identical to a healthy one. + +- **UX-05 (P1) — the checklist block is implemented twice; the capability copy bypasses the card.** + `ModuleCard.vue:130-144` renders `verification.checks`, but the capability section never passes + `verification` and re-implements the list at `:292-313` with a byte-equivalent `.capability-checks` + block. *Fix: pass the prop, delete the duplicate.* + +- **UX-06 (P1) — the same capability has three different install UIs.** + Game page (full config form, `CapabilityModulesSection.vue:239-290`), Community modal (bare + "Install", `config: {}` hard-coded, `CommunityPanel.vue:139`), and the AI path + (`useAiAssistant.ts:476`). A mod with required config fields installs blank or fails from two of + the three. *Fix: one spec-driven install dialog that all three call.* + +- **UX-07 (P1) — four dialogs re-implement the shell that `BaseDialog` already provides.** + `CommunityPanel.vue:198-215`, `ActivityLogPanel.vue:100-117`, `OpenXrManager.vue:52-69` and + `LocalAiPanel.vue:109-126` each rebuild the backdrop, `role="dialog"`, `aria-label` and close button — + and `useDialogLifecycle.ts:35-36` focuses the **close X** rather than the title, so keyboard users + land in a different place in each of six dialogs. This is the largest single copy-paste mass in `src/`. + *Fix: render all four through `BaseDialog`.* + +- **UX-08 (P2) — refresh buttons behave three different ways** (disabled + spinner / disabled with no + indicator / icon swap + spin). *Fix: one `.btn` loading convention with `aria-busy`.* + +- **UX-09 (P2) — empty states have three different shapes**, and `CapabilityModulesSection.vue:147` + has none at all — the section simply does not exist until the list arrives. + *Fix: one `.empty-state` contract in `base.css`.* + +- **UX-10 (P2) — three pluralization strategies in one app** (`GameList.vue:74` uses vue-i18n's + native `|` form; `community/copy.ts:27` uses `{count}`; `pt-BR.ts:178` uses a hand-rolled + `{count,plural,...}`). *Fix: standardize on the `|` form.* + +## Feedback, trust and state + +- **UX-11 (P1) — four destructive actions with no confirmation.** Capability removal + (`CapabilityModulesSection.vue:117-121`), the **Windows-wide** OpenXR runtime switch + (`OpenXrManager.vue:142-150`, which writes `HKLM` via a UAC prompt and only shows the warning as a + passive callout), Local AI disconnect (`LocalAiPanel.vue:191-199`, rewrites the user's editor config) + and History undo (`HistoryView.vue:82-93`). Every one is undoable only through a different surface, + which undercuts the app's central promise. *Fix: one `ConfirmDialog` primitive that lists the + affected paths and links to "Desfazer no Histórico".* + +- **UX-12 (P1) — the AI install path bypasses the unsigned-consent gate.** + `useAiAssistant.ts:477` passes `acceptUnsigned: true`. The Community panel makes the user tick an + explicit box reading "Entendo que este mod não foi verificado…" (`CommunityPanel.vue:279-286`). + The AI path installs the same unsigned artifacts with no prompt at all. *Fix: route all three + through the same consent component.* + +- **UX-13 (P1) — multi-line errors are collapsed onto one line.** + `useAiAssistant.ts:299,516` joins errors with `\n`, but every render site is `

{{ error }}

` + (`AiAssistantDialog.vue:287,305,371,446`) and `.callout p` sets no `white-space: pre-line`. A + 3-item validation list becomes one run-on sentence. *Fix: `white-space: pre-line` or a real list.* + +- **UX-14 (P1) — a broken state with no repair action.** `LocalAiPanel.vue:25-49` renders + `configError` as a red badge, but the footer only offers a button for `detectedNotConfigured` or + `configured` — so this state shows a badge and **no action at all**. + +- **UX-15 (P1) — silent failure hides the game list in the VR panel.** + `useOpenXrManager.ts:63-65` swallows the error with `catch { installedGames.value = [] }`; the + select then offers only "Todos os jogos" with no explanation, on a screen that has a full + friendly-error system for everything else. + +- **UX-16 (P1) — a loading state exists in the composable and not in the UI.** + `useCapabilityModules.ts:55,152-164` maintains `loading`; `CapabilityModulesSection.vue:38-49` + never destructures it, so the mods area silently pops in on a slow disk. + +- **UX-17 (P2) — disabled controls whose reason exists but is mouse-only** (`title` attribute + only) in `ModuleCard.vue:94,104,114`, `HistoryView.vue:88`, `DesktopShortcutDialog.vue:34` and + `AiTopbarMenu.vue:145`. *Fix: visible text tied with `aria-describedby`.* + +- **UX-18 (P2) — `window.confirm` for the log wipe** (`useActivityLogPanel.ts:53`) in an app that has + a full dialog system, and no toast offers an "Abrir Histórico" action even when the message says + the change can be undone there. + +## Copy, terminology and localization + +- **UX-19 (P1) — "recipe" leaks into user-facing copy** (`community/copy.ts:6,27`) while every other + surface calls the same thing a "mod". + +- **UX-20 (P1) — config field labels are raw YAML keys** (`CapabilityModulesSection.vue:253,287`), + violating the project's own rule that technical names appear only next to a plain label. + *Fix: allow an optional `label` in the config schema.* + +- **UX-21 (P1) — backend-generated strings surface untranslated.** `ActivityLogPanel.vue:153,156` + renders `entry.action` and `entry.message` verbatim; likewise `check.label`/`check.detail`, + `state.warnings` and `agent.detail`. `docs/FRONTEND.md:363` already flags this as known debt. + *Fix: map known backend ids to locale keys at the service boundary.* + +- **UX-22 (P1) — internal storage paths in normal UI.** `activity/copy.ts:21` prints + `Salvo em %LOCALAPPDATA%\Moddin\logs` in the dialog footer, and `local-ai/copy.ts:18-20` shows raw + binary/config/MCP paths for every agent in a default-open list. *Fix: fold into "Detalhes técnicos".* + +- **UX-23 (P2) — dead copy marks half-built features.** `local-ai/copy.ts:10-11` (`intro`) and + `openxr/copy.ts:33` (`uacHint`) are defined in all three locales and referenced nowhere. They read + like onboarding text that was never wired. + +- **UX-24 (P2) — the "install Cursor" hint is shown to everyone** (`LocalAiPanel.vue:204-206` fires + whenever *no* agent has a binary path), so a Claude Desktop or Codex user is told to install a + competing product. + +## Accessibility + +- **UX-25 (P1) — `role="menu"` with no menu keyboard model** (`AiTopbarMenu.vue:120-155`): no arrow / + Home / End handling, `:aria-haspopup="true"` instead of `"menu"`, focus never moved into the + dropdown. A keyboard user must Tab blindly to reach items visually attached to the button. + +- **UX-26 (P1) — repeated, context-free button labels.** `HistoryView.vue:92` renders "Desfazer" on + every row and `ModuleCard.vue:107` renders "Conferir" on every card. *Fix: per-item `aria-label`.* + +- **UX-27 (P1) — unlabelled form controls.** `ActivityLogPanel.vue:121-122` has a search input and a + select with no label — an outlier, since Community and OpenXR wrap theirs in `