From 75507b4984c04c8dd704bd48a2661581531586f8 Mon Sep 17 00:00:00 2001 From: Adrian Bonpin Date: Sun, 12 Jul 2026 09:21:27 +0800 Subject: [PATCH] docs: design spec for plugin bugs + library-injection features --- ...6-07-12-plugin-bugs-and-features-design.md | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-12-plugin-bugs-and-features-design.md diff --git a/docs/superpowers/specs/2026-07-12-plugin-bugs-and-features-design.md b/docs/superpowers/specs/2026-07-12-plugin-bugs-and-features-design.md new file mode 100644 index 0000000..f1ca915 --- /dev/null +++ b/docs/superpowers/specs/2026-07-12-plugin-bugs-and-features-design.md @@ -0,0 +1,225 @@ +# Plugin Bugs + Features — Design + +**Date:** 2026-07-12 +**Scope:** Decky Loader plugin (`plugins/decky-vault`) + DeckyVault web API (`apps/web/lib/api`). Three bug fixes and two features, unified in one spec. +**No DB migrations required** — all changes reuse existing schema columns and routes. + +--- + +## 1. Background & decisions (from brainstorm) + +### Bugs +1. **Stop/restart recording can crash the game.** Root cause: `handleStart` calls `write_mangohud_config()` (rewrites `~/.config/MangoHud/MangoHud.conf`) and `clear_mangohud_log()` (deletes `/tmp/*MangoHud*`, `/tmp/*.csv`, **and `/tmp/*.log`**) on **every** start, even while MangoHud is running as the game's wrapper. Rewriting a live config + deleting a file the wrapper has open can crash MangoHud, which takes the game with it. +2. **"FPS must be from 0 to 500" on resubmit.** Root causes: (a) `buildImportPayload` sends `fpsAvg: sess.fpsAvg ?? 0` — a failed/short recording with no parsed FPS submits `0`, failing the server's `fpsAvg < 1` check; (b) the server caps **all** FPS fields at 500 — `fpsHigh` (and even `fpsAvg`) can legitimately exceed 500 in menus/2D games; (c) the game-not-found (404) check runs *before* FPS validation in `performance-import.ts`, so the first submit masks the real FPS error and it only surfaces after the user adds the game to the DB. +3. **Photos does not pull screenshots.** Root cause: `list_screenshots` only scans `~/Pictures/Screenshots/` + `Steam Client/` (the Desktop Mode export path). Game Mode (Steam+R1) saves to `~/.local/share/Steam/userdata//760/remote//screenshots/*.jpg`, which is never discovered. + +### Features (unified into one injection surface) +4. **Game-open → show game entry, top entries, est FPS.** +5. **Library focus → inject device-filtered metrics.** + +**Feasibility finding:** `definePlugin` (in our installed `@decky/api@1.1.3` / `@decky/ui@4.11.6`) only exposes a single QAM content panel — there is no `tabs`/`gameTabs` field. However, Decky's **documented** `routerHook.addPatch('/library/app/:appid', patch)` + `createReactTreePatcher` / `afterPatch` / `findInReactTree` / `appDetailsClasses` (all exported by our installed packages; verified) enable injecting a React section into the library/app-details page — the same canonical pattern used by the maintained `HLTB for Deck` plugin. This is **not** raw fragile monkey-patching; it is the supported route-patch API. + +**Unified design:** Both features become a single DeckyVault section injected onto the game's library/app-details page. No new in-QAM tab. The section shows: game entry status, device-scoped est FPS, and top recent/pinned/most-positive entries (3 cards). + +**Decisions locked during brainstorm:** +- Feature 5 surface: `A` — library app-details section via `routerHook.addPatch` (HLTB pattern), guarded so a Steam UI change degrades silently. +- Feature 4 (no in-panel tab): fold into the same injection. +- "Est FPS (based on global)": **device-scoped** to the detected hardware slug (matches Feature 5). +- Device scope control: **D1** — default to detected device, with a small in-section dropdown to switch device or view "All devices". +- Bug 2 handling: **A** — plugin clamps/guards before submit; server raises FPS caps to 1000; NaN still rejected. + +--- + +## 2. Bug fixes — detailed design + +### 2.1 Bug 1 — safe recording start + +**Plugin frontend (`src/index.tsx` → `handleStart`):** +- Remove `await writeMangohudConfig()`. Config is a one-time setup step already exposed via the "Write Config" button in the MangoHud Setup panel. The hot start path must never rewrite a config a live game is using. + +**Plugin backend (`main.py` → `clear_mangohud_log`):** +Rework to be MangoHud-specific and non-destructive of live/foreign files: +- Globs to delete: `/tmp/*MangoHud*` and `/tmp/*MangoHud*.csv` only. **Never** `/tmp/*.log` (system/foreign logs). **Never** bare `/tmp/*.csv` (could belong to other tools). +- Skip any candidate whose `mtime` is within the last 3 seconds (an active session may still have it open). +- Return `{ success, deleted: [{name}], skipped: [{name, reason}] }` for observability. + +**Recording-specific log tracking (frontend + backend):** +- On `handleStop`, after `readAndParseMangohudLog`, store the resolved log path in `session.lastLogPath`. +- On next `handleStart`, if `session.lastLogPath` is known, delete (or rename to `*.bak`) **that specific file** only; otherwise fall back to the safe `clear_mangohud_log` above. +- Add a new lightweight RPC `delete_log_file(path)` that strictly validates the path is under `/tmp` and matches `*MangoHud*` before removal (defence in depth). + +**Tests (`tests/test_clear_log.py`, new):** +- Only `*MangoHud*` files removed; a `/tmp/system.log` and `/tmp/other.csv` untouched. +- A file with `mtime` < 3s ago is skipped. +- `delete_log_file` rejects paths outside `/tmp` or not matching `*MangoHud*`. + +### 2.2 Bug 2 — FPS validation/caps + ordering + +**Plugin frontend (`src/lib/store.ts` → `buildImportPayload`):** +- If `sess.fpsAvg == null` or `<= 0`: the caller (`session-form.tsx` `handleUpload`) must block upload with `setError("No FPS data captured — re-record or export only.")` and **not** call `uploadToDeckyvault`. (Export-to-file remains allowed.) +- Clamp: negatives → 0; `fpsHigh`/`fpsAvg`/`fpsLow`/`fpsOnePercentLow` capped at 1000 before sending (server-side cap matches). +- Remove the `?? 0` fallback for `fpsAvg`; use `null` and let the guard above handle it. New helper `sanitizeFps(value)`. + +**Server (`apps/web/lib/api/performance-import.ts`):** +- Reorder validation: validate **FPS shape first** (non-NaN, range), then hardware, then game-not-found. Rationale: the user should always see the *real* first error regardless of DB state; a 404 must not mask a 400. +- Raise caps to 1000 for `fpsAvg`, `fpsLow`, `fpsOnePercentLow`, `fpsHigh`. Keep rejecting `NaN`. Lower bounds unchanged (`fpsAvg` ≥ 1, others ≥ 0). +- Apply the same changes to `apps/web/lib/api/performance-submit.ts` (the website submit path) for parity. + +**Tests (`apps/web/lib/api/__tests__/performance-import.test.ts`, new/extended):** +- No-data payload (`fpsAvg` missing/null) → 400 with a clear message, never a silent 0. +- `fpsHigh: 750` → 201 (accepted), not 400. +- `fpsAvg: NaN` → 400. +- Validation ordering: a payload that is *both* game-not-found *and* has invalid FPS returns the FPS error (400) first. + +### 2.3 Bug 3 — screenshot discovery + +**Plugin backend (`main.py` → `list_screenshots`):** +Add the Game Mode path and keep existing paths (Desktop Mode export): +- New globs: `~/.local/share/Steam/userdata/*/760/remote/*/screenshots/*.{jpg,png}` (all Steam accounts). +- Existing globs retained: `~/Pictures/Screenshots/*.{jpg,png}` + `Steam Client/*.{jpg,png}`; skip `most_recent.jpg` duplicate as today. +- Merge all, de-duplicate by real path (not just basename), sort by `mtime` desc, slice to `limit`. +- Optional new param `app_id: int | None`: when provided, prefer that app's folder (still include Desktop exports) and tag each screenshot with `{ appId }` in the response so the picker can group/filter. + +**`read_screenshot` (`main.py`):** +- Existing Pillow downscale to `max_width` stays. +- Add a pure-Python fallback when Pillow is absent: if raw size < 1 MB, return the data URL; if ≥ 1 MB, return `{ dataUrl: "", error: "Preview unavailable (too large, no Pillow)" }` so CEF is never handed a giant buffer. The picker will show the placeholder thumbnail + name; upload still works (upload reads the file directly, not the preview). + +**Tests (`tests/test_screenshots.py`, new):** +- A fixture tree with `~/Pictures/Screenshots/x.jpg`, `~/Pictures/Screenshots/Steam Client/y.jpg`, and `~/.local/share/Steam/userdata//760/remote//screenshots/z.jpg` → all three returned, deduped, sorted by mtime. +- `app_id` filter returns only that app's folder screenshots + Desktop exports. +- Fallback `read_screenshot` behaviour under a `PIL ImportError` mock: under 1 MB returns data URL, over 1 MB returns preview-unavailable but no exception. + +--- + +## 3. Features 4 & 5 — library app-details DeckyVault section + +### 3.1 Injection mechanism + +A new module `plugins/decky-vault/src/patches/LibraryApp.tsx` registers a route patch on plugin load and unpatches on dismount, mirroring `HLTB for Deck`'s `patchAppPage`: + +```ts +// sketch (real impl in the plan) +routerHook.addPatch('/library/app/:appid', (routerTree) => { + const routeProps = findInReactTree(routerTree, (x) => x?.renderFunc); + if (!routeProps) return routerTree; // guard: unexpected tree → no-op + afterPatch(routeProps, 'renderFunc', createReactTreePatcher( + [(tree) => { + const child = findInReactTree(tree, (x) => x?.props?.children?.props?.overview); + if (!child) return null; // guard: not a game page + const overview = child.props.children.props.overview; + if (!isSteamGameType(overview.app_type)) return null; // only real games (1/8) + return child.props.children; + }], + (_, ret) => { + const container = findInReactTree(ret, + (x) => Array.isArray(x?.props?.children) && + x?.props?.className?.includes(appDetailsClasses.InnerContainer)); + const arr = container?.props?.children; + const idx = arr?.findIndex(/* locate the splicable anchor, HLTB-style */); + if (idx > -1) arr.splice(idx, 0, ); + return ret; + } + )); + return routerTree; +}); +``` + +**Guardrails (critical):** +- Every `findInReactTree` / array access is null-checked; a missing anchor logs a `console.debug` and returns the tree unmodified (no crash, no splice). Non-game pages (soundtracks, tools) are skipped via `app_type`. +- The injected component is async and self-contained — fetch failures render a tiny "DeckyVault: unavailable" line or nothing, never a thrown error in the tree. +- `onDismount` calls `routerHook.removePatch('/library/app/:appid', patch)`. + +### 3.2 Injected component — `LibraryAppPanel.tsx` + +Props: `{ appId: number, title: string }`. Behaviour: +- Reads the plugin's detected `hardwareSlug` (via a shared store accessor) for the default device scope. +- Fetches `GET /api/plugin/game/:steamAppId?hardware=&limit=3` (see 3.4). +- Caches the response per `appId` in an in-memory LRU (1h TTL) to avoid refetch on every page re-render. +- Renders, inside a Steam-styled container (uses `appDetailsClasses` + `staticClasses`): + 1. **Game entry status** — "In DeckyVault" with a link button (decky `Router.NavigateToURL` gloss) or "Not in DeckyVault — open this game on deckyvault.xyz to add it." CTA when the API returns 404 / `game == null`. + 2. **Est FPS** — device-scoped by default (the detected slug), with a compact device **dropdown** (D1): options = detected device + "All devices" + other devices returned by the optional `/devices` endpoint. Switching the dropdown re-fetches with the new `hardware` query. + 3. **Top 3 entry cards** — pinned / top-upvoted / most-recent, each expandable to show `settingsJson` summary, upscaler/frame-gen, Proton/OS, TDP, and the contributor. Reuses fields already returned by the existing performance endpoint shape. + 4. **Empty state** — "No entries for your device yet — be the first: open the DeckyVault plugin and record." when the game exists but the device scope has no data. + +### 3.3 Plugin data fetching + +`plugins/decky-vault/src/lib/plugin-api.ts` (new) provides typed fetch helpers. Since CEF fetch from Steam's context to `deckyvault.xyz` can be CORS-restricted, we proxy through the Python backend as a reliable fallback: +- Python (`main.py`) gains a small generic RPC `plugin_get(path: str) -> dict` that does an authenticated (none needed — reads are public) `urllib` GET to `${base_url}/api${path}` with the existing SSL fallback, returning parsed JSON or `{ error, status }`. Keeps all network in one place, matches the existing upload pattern. +- TS helper wraps `callable("plugin_get")`, caches in the same LRU. + +### 3.4 New web API — read-only, public + +New module `apps/web/lib/api/plugin-public.ts`, mounted in `apps/web/lib/api/app.ts`: + +- `GET /api/plugin/game/:steamAppId?hardware=&limit=` + - Resolves the game by `steamAppId` (reuses the existing `games-lookup` resolution). Returns `{ game: {...}|null, estFps: {avg, low, onePct, high, count}|null, topEntries: [...], recentEntries: [...] }`. + - `hardware` optional: when a valid slug, scope `estFps` + entries to that device; when omitted/null, all devices. + - `topEntries` = entries ordered by `isPinned desc, upvotes desc` (existing behaviour), sliced to `limit`. + - `recentEntries` = entries ordered by `createdAt desc`, sliced to `limit`. + - `estFps` = aggregates over the (device-scoped) non-removed entries using `avg()`/`min()`/`max()`/`count()` on `performanceEntries` (patterns already used in `dashboard-public.ts`, `hardware-stats.ts`, `compare.ts`). + - Entry shape reuses the `games-performance.ts` row projection (trimmed: id, hardware, fpsAvg/Low/OnePct/High, upscalerType, frameGenMethod, protonVersion, osVersion, tdpWatts, settingsJson, upvotes, isPinned, createdAt, userName, userImage). + - Public (no API key) — community data is already public on the site. Same rate-limit middleware as other public routes. + +- `GET /api/plugin/game/:steamAppId/devices` (optional, drives the dropdown) + - Returns `[{ slug, name, count }]` for hardware slugs with ≥1 non-removed entry for the game. + +**Tests (`apps/web/lib/api/__tests__/plugin-public.test.ts`, new):** +- Game in DB, `hardware=steamdeck-oled` → device-scoped est FPS + entries. +- `hardware` omitted → all-device scope. +- Game not in DB → 404 with `{ game: null }` plus an `error` string (so the panel shows its CTA). +- `limit` honoured; `devices` endpoint returns only slugs with data. + +### 3.5 Activation wiring + +`plugins/decky-vault/src/index.tsx`: +- On `definePlugin` body: create the patch (`patchAppPage()`) and keep its handle. +- `onDismount`: `routerHook.removePatch('/library/app/:appid', handle)` (plus existing cleanup). +- The existing QAM panel (`Content`) stays as-is (Recording → Session Results flow unchanged except Bug 1/2 fixes). + +--- + +## 4. Cross-cutting: error handling, caching, testing + +**Error handling:** +- Library patch: never throws into Steam's tree; every external call is wrapped; render falls back to null/CTA. +- API: standard Elysia error shapes (`{ error }`); 404 for unknown game; 400 only for malformed query. +- Plugin uploads: Bug 2 guard blocks before network call. + +**Caching:** +- Plugin-side per-appid LRU (1h) for library panel reads. +- API: `Cache-Control: public, max-age=60` on the read endpoints (community data, acceptable staleness). + +**Testing summary:** +- Python pytest: Bug 1 safe-clear + `delete_log_file`; Bug 3 screenshot discovery + fallback. +- Vitest (web API): `performance-import` validation ordering + 1000 caps; `plugin-public` contract + device scope + 404. +- Plugin TS: `buildImportPayload` clamp/null guard (unit if harness present, else build + manual smoke). No DB migrations. + +--- + +## 5. Files touched + +**Plugin:** +- `plugins/decky-vault/main.py` — `clear_mangohud_log`, new `delete_log_file`, `list_screenshots`, `read_screenshot`, new `plugin_get`. +- `plugins/decky-vault/src/index.tsx` — `handleStart` cleanup; wire patch + `onDismount`. +- `plugins/decky-vault/src/lib/store.ts` — `buildImportPayload` clamp/null guard + `sanitizeFps`. +- `plugins/decky-vault/src/lib/api.ts` — new `pluginGet` RPC wrapper. +- `plugins/decky-vault/src/lib/plugin-api.ts` (new) — typed library-panel fetch helpers + cache. +- `plugins/decky-vault/src/patches/LibraryApp.tsx` (new) — `routerHook.addPatch` splicer. +- `plugins/decky-vault/src/components/LibraryAppPanel.tsx` (new) — injected section UI. +- `plugins/decky-vault/tests/test_clear_log.py`, `tests/test_screenshots.py` (new) + fixtures. + +**Web API:** +- `apps/web/lib/api/performance-import.ts`, `performance-submit.ts` — caps (1000) + validation order. +- `apps/web/lib/api/plugin-public.ts` (new) — mounted in `app.ts`. +- `apps/web/lib/api/__tests__/performance-import.test.ts`, `plugin-public.test.ts` (new/extended). + +**No schema changes, no migrations.** + +--- + +## 6. Out of scope + +- Any Decky store submission / release packaging. +- The in-QAM "new tab" concept (dropped per user decision — folded into library injection). +- Toast / grid-focus overlays (feature 5 option B, dropped). +- Per-entry rich-text comments rendering inside the library panel (future enhancement). \ No newline at end of file