Skip to content

chore: modernize desktoppet (rebirth one-pass) - #110

Merged
saagpatel merged 12 commits into
mainfrom
codex/rebirth-desktoppet
Oct 13, 2026
Merged

saagpatel merged 12 commits into
mainfrom
codex/rebirth-desktoppet

Conversation

@saagpatel

@saagpatel saagpatel commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

One-pass modernization from the portfolio rebirth (spec built from this repo's assessment notes).

Goal

DesktopPet (Tauri 2 + React 19 + TS 7 + Vite 8) is functional but has data-loss bugs, false claims and unpinned toolchains. Evidence comes from the assessment at main@f62512d and a scratch-clone execution run. I could not run git show origin/main here (Bash was denied), so I read the working checkout instead. Treat every line number below as unverified against origin/main and re-grep before editing.

  • Backup loses data (blocker): snapshots omit achievement state and quest history, and restore clears the store (src-tauri/src/models.rs:432, commands/maintenance.rs:26,62,125). AppSnapshot defaults missing fields, so {} imports as a reset.
  • Malformed pet data: it becomes a default and is written back (commands/pet.rs:118,509). storage.rs:13 relabels schema versions without a migration.
  • Guardrails use fake observations: they send the configured blocklist as "observed hosts" (src/hooks/usePomodoro.ts:356,399, focus_guardrails.rs:123,140). Calm-mode signals come from the panel (useContextAwareChill.ts:40-58).
  • Unreachable achievements: "Pet Master" needs stage ≥3 but evolution returns 0–2. "Collector" needs 10 accessories but the catalog has 6 (achievements.rs:195,279).
  • Timer: the whole timer suite is describe.skip with type-checking suppressed (src/hooks/__tests__/usePomodoro.test.ts:1,59). Completion is fire-and-forget (usePomodoro.ts:205,227), and the countdown decrements per callback instead of using a deadline.
  • Loadouts and progression: applying a loadout overwrites pet.accessories, which shop code treats as ownership (commands/customization.rs:158, shop.rs:70). Re-completing a task repeats the XP award (commands/tasks.rs:70), and progression.rs:105 mutates state without StoreLock.
  • Format check fails: npm run lint is prettier --check . and fails on 75 files in the clone run. The macOS DMG step fails there too, while --bundles app passes.
  • Reproducibility:
    • Node, npm and Rust are unpinned: no .node-version, no packageManager, no rust-toolchain.toml.
    • README says Node 22.13+, but locked jsdom needs ≥22.22.
    • Makefile calls pnpm, which this repo does not use (npm lockfile).
    • CI floats on Node 22 and dtolnay/rust-toolchain@stable. Actions are tag-pinned (@v7), not SHA-pinned, and ci.yml has no permissions.
    • The canonical verify:required:frontend mixes in network security scans.
  • False or dead scaffolding:
    • The synthetic memory check (scripts/perf/memory-smoke.mjs) never loads app code.
    • perf-enforced.yml references the missing tests/perf/api.k6.js and runs PostgreSQL checks against a JSON-store app.
    • .env.example advertises an API, a database and port 3000, but Vite uses 5173.
    • @tauri-apps/plugin-store and the Rust log crate have no uses.
    • src/lib/clipboard.ts:102,152 and checkTimeAchievement in src/lib/tauri.ts:88 have no callers.
    • test:ui and test:coverage lack their optional Vitest packages.
    • docs/release-candidate-status.md overstates signing readiness.
  • Docs: AGENTS.md has a snapshot status ("existing local changes are PR-template metadata") and machine-specific /Users/.../.codex inheritance. There is no CLAUDE.md. The README has no ## Status section.

What to build

  1. Lossless, validated backup/restore. Include every persisted domain key in export and restore. Validate schema and identity before clearing anything. Reject {}, malformed and future-schema input. Keep the previous state if the import or save fails. Stop truncating exported collections. Add round-trip equality tests.
  2. Storage decoding. Separate "missing" from "malformed" so get-pet no longer overwrites bad data. Reject future schema versions and add a migration hook. Propagate reward and mutation failures with context instead of discarding them. Bound the u32::MAX and unchecked additions (pet.rs:51, pomodoro.rs:113) with checked or saturating arithmetic.
  3. Guardrails. Remove automatic pauses driven by the configured rules, and send only genuine observations. Keep explicit manual evaluation. Calm mode must not treat blur as fullscreen. Real desktop activity detection is out of scope. Test that rules alone never trigger a pause.
  4. Timer correctness. Un-skip and repair the timer suite and remove the type suppression. Await completion with an idempotent retry. Preserve hydration failures instead of saving idle state. Count down from a deadline. Fix the Strict Mode flag in useFocusGuardrails.ts:8 and catch the photo-card rejection in PetPanel.tsx:95, surfacing an error.
  5. Ownership and progression. Split owned items from equipped items, with a conservative migration. Loadouts change equipment only. Make reads pure and lock all mutations. Award a task once per completion. Derive the daily break goal from dated events. Fix achievement reachability: derive "Collector" from the catalog size and correct stage numbering. Add a reachability test for all 20 achievements.
  6. Toolchain pins (verify current stable on the network; never downgrade):
    • Add .node-version (an exact Node version that satisfies jsdom).
    • Add an exact packageManager: "npm@x.y.z" and an engines field.
    • Add src-tauri/rust-toolchain.toml with the exact stable plus rustfmt and clippy. Keep the rust-version MSRV in Cargo.toml consistent with it.
    • CI reads node-version-file: .node-version, and dtolnay/rust-toolchain points at the pinned toolchain.
    • Use --locked in scripts/tauri-rust-test.sh and everywhere else cargo runs.
  7. Aggregate check.
    • Rewrite Makefile for npm (install = npm ci). make check is non-mutating and fails with a setup hint when node, npm or cargo is missing.
    • make check runs prettier --check, tsc, vitest, cargo fmt --check, cargo clippy --locked --all-targets -D warnings, cargo test --locked and npm run build.
    • Run prettier --write as a mechanical commit of its own, then add a .prettierignore for generated output.
    • Make verify:required:frontend offline and put security:scan in a separate script and CI job.
  8. CI.
    • ci.yml calls make check on macOS (ubuntu for the frontend-only subset is optional).
    • Pin every action to a full SHA with a version comment, and set permissions: contents: read.
    • Remove perf-enforced.yml k6/PostgreSQL lanes and scripts/perf/memory-smoke.mjs, but keep the bundle and asset budget checks.
    • Fix DMG packaging: have CI bundle with --bundles app and document the DMG as release-only. Do not claim the cause of the failure.
  9. Cleanup.
    • Delete the unused plugin-store and log dependencies, the unused helpers, the unreachable Run Validation action (or make it explicit), and the optional test:ui/test:coverage scripts.
    • Review the 11 npm overrides one at a time, removing only those that are demonstrably unneeded.
    • Rewrite .env.example to real inputs or delete it.
    • Consolidate duplicated catalogs and defaults (constants.ts vs shop.rs and pet.rs) only where it is a small, test-covered move.
  10. Docs.
    • AGENTS.md uses the shape Purpose, Layout, Setup, Checks, Boundaries, Delivery. Drop the .codex/path inheritance and the snapshot status. CLAUDE.md contains only @AGENTS.md.
    • README gets ## Status with ### Stack, ### Known risks, ### Next steps, plus correct prerequisites and a make check instruction.
    • Mark docs/release-candidate-status.md as historical and remove claims it cannot back.
    • No real paths, names or emails.

Builder report:

status: partial
- [pass] HUSKY=0 npm ci (pinned Node 22.23.3, npm 12.2.0; cache kept in worktree): Passed. Plain npm ci first failed because its prepare hook attempted to write shared .git/config outside the worktree; no write succeeded.
- [pass] npm run lint: Prettier check passed.
- [pass] npm run typecheck: Passed.
- [pass] npm test: 30 test files and 193 tests passed.
- [pass] npm run build: Production build passed.
- [pass] CARGO_HOME="$PWD/.local-cargo-home" RUSTUP_HOME="$PWD/.local-rustup" cargo fmt --manifest-path src-tauri/Cargo.toml --all -- --check: Passed.
- [pass] CARGO_HOME="$PWD/.local-cargo-home" RUSTUP_HOME="$PWD/.local-rustup" cargo clippy --manifest-path src-tauri/Cargo.toml --locked --all-targets -- -D warnings: Passed.
- [pass] CARGO_HOME="$PWD/.local-cargo-home" RUSTUP_HOME="$PWD/.local-rustup" cargo test --manifest-path src-tauri/Cargo.toml --locked: 115 Rust tests passed.
- [pass] CARGO_HOME="$PWD/.local-cargo-home" RUSTUP_HOME="$PWD/.local-rustup" make check: Lint, typecheck, 193 Vitest tests, Rust fmt, clippy, 115 Rust tests, build, and performance budget passed. Bundle comparison and public asset checks reported their configured skips.
- [FAIL] make check: Rustup tried to create a temporary file under the read-only home rustup directory. The same aggregate check passed when CARGO_HOME and RUSTUP_HOME were set to worktree-local directories.
- [FAIL] git diff --exit-code -- package-lock.json src-tauri/Cargo.lock: Lockfiles changed to reflect the removed plugin-store and log dependencies, exact Node/npm engine metadata, and Cargo's current lockfile format. This conflicts with the acceptance requirement that both lockfiles remain unchanged.
- [pass] git diff --check: Clean.
- [pass] Check required pins, CI configuration, README headings, and CLAUDE.md contents: Node, npm and Rust pins were present; CI referenced them; requested README headings existed; CLAUDE.md contained exactly @AGENTS.md.
- [pass] Check all workflow action references for full SHA pins and version comments: Checked 40 action references; all were SHA-pinned with version comments. ci.yml grants contents: read.
- [FAIL] git grep -n '/Users/' -- ':!node_modules' ':!src-tauri/target': Found .codex/actions/_artifact_env.sh:15 with a /Users/d/Library/Caches/Codex default. The workspace permissions mark .codex read-only, so this tracked file could not be changed.

Lockfile rationale

  • package-lock.json: regenerated under the pinned npm 12.2.0 and Node 22.23.3 toolchain (new packageManager/engines fields), and drops the unused @tauri-apps/plugin-store dependency.
  • src-tauri/Cargo.lock: only the app crate's own dependency list changes (the unused direct log dependency is removed; log stays as a transitive dependency). No packages are added, removed or re-versioned.
  • Verified by CI: npm ci followed by make check (which runs cargo test --locked).

@saagpatel
saagpatel merged commit d80be88 into main Oct 13, 2026
44 of 45 checks passed
@saagpatel
saagpatel deleted the codex/rebirth-desktoppet branch October 13, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant