refactor(tweak-system): fix snapshot loss on failed rollback, remove v1 profile system, add CI (#31)

* chore(repo): enforce CRLF line endings project-wide

This is a Windows-only application that is never built or run on another
platform, so the working tree should be CRLF unconditionally.

Previously .gitattributes was "* text=auto", which normalizes blobs to LF and
leaves the working-tree ending up to each developer's core.autocrlf setting.
That made CRLF a property of one machine rather than of the repository, and
eight tracked files had already drifted to LF in the working tree, including
build.rs, hosts_service.rs, firewall_service.rs and Cargo.toml.

- .gitattributes: "* text=auto eol=crlf", plus explicit binary declarations
  for fonts and images so they are never converted
- .prettierrc: endOfLine auto -> crlf
- rustfmt.toml: newline_style Auto -> Windows ("Auto" preserves whatever a
  file already has, which is how LF crept in)
- .editorconfig: end_of_line = crlf

Blobs remain LF-normalized in the object database, which keeps diffs, merges
and blame readable.

* docs: add tweak system audit, remediation plan, ADRs and glossary

Planning artifacts for the tweak system remediation work.

- docs/TWEAK_SYSTEM_REVIEW.md: audit of the Rust backend, build-time validator
  and YAML corpus against the docs-as-spec. 47 findings (41 confirmed by an
  independent adversarial pass, 5 plausible, 1 refuted), each with a failing
  scenario, evidence and a fix sketch. Refuted findings are retained and
  labelled rather than dropped.
- docs/TWEAK_SYSTEM_PLAN.md: phased remediation plan. Records the execution
  order and, importantly, the three places where two independent research
  passes contradicted each other and how each was resolved -- notably the
  rejection of the windows-service crate, whose change_config() omits
  SERVICE_NO_CHANGE and would wipe service account passwords.
- docs/adr/0001-0004: the design decisions taken deliberately, with their
  rejected alternatives, so they are not re-litigated later.
- CONTEXT.md: project glossary. Separates "Stock Default" (what Windows ships)
  from "System Default" (what this machine was on before we touched it) --
  conflating those two is the root of several doc contradictions.

* fix(tweaks): keep the snapshot when a failed apply cannot be rolled back

apply_tweak discarded the rollback result and then deleted the snapshot
unconditionally:

    let _ = backup_service::restore_from_snapshot(&snapshot);
    backup_service::delete_snapshot(&tweak_id)?;

restore_from_snapshot can fail in two distinct ways. It returns
Ok(RestoreResult { success: false, .. }) when service, scheduler, hosts or
firewall restores fail and are collected, and a hard Err when the registry
phase aborts -- in which case the remaining phases never run at all. Both were
thrown away.

So an apply that failed after mutating services, and whose rollback then could
not restore them, left the machine half-changed, deleted the only record of the
user's original state, and reported nothing beyond the apply error. revert_tweak
afterwards answers "No snapshot found for this tweak". This also violated the
project's own rule in .github/copilot-instructions.md: do not silently ignore
privileged operation failures.

The snapshot is now released only when the rollback is verifiably complete.
Otherwise it is kept and every unrestored resource is reported, mirroring the
partial-failure shape revert_tweak already uses so the UI can surface detail
rather than a bare error string.

Secondary fix: `delete_snapshot(..)?` replaced the real apply error with a
file-deletion error. It is now logged instead of propagated.

The classification is extracted into `classify_rollback` so the decision that
was wrong -- when is a rollback good enough to release a snapshot -- is pure and
testable without Windows state. Six tests cover it; all four negative cases were
confirmed to fail against the reintroduced bug before the fix landed.

Implements docs/adr/0001 and docs/adr/0002.

* fix(build): emit tweaks.json deterministically

build.rs collected tweaks into a HashMap and serialized it straight to
tweaks.json. Rust's HashMap uses a randomly-seeded hasher, so the keys came out
in a different order on every build: three build directories on this machine
held three different 436,243-byte files, all from identical YAML input.

That defeats build caching, makes the artifact useless as a diffable record, and
makes it impossible to verify that a change to the YAML or to the parser left
the output untouched -- which is exactly the check needed before swapping the
YAML parser.

Switching to BTreeMap makes the key order the tweak IDs' sort order. The runtime
still deserializes into a HashMap; JSON object order is irrelevant there, so
nothing downstream changes.

Verified: two consecutive forced rebuilds now produce byte-identical output, and
a deep comparison against the pre-change artifact confirms all 189 tweaks are
semantically unchanged -- only the key order moved.

* fix(build): replace serde_yml with serde_yaml_bw (RUSTSEC-2025-0068)

serde_yml is affected by RUSTSEC-2025-0068: unsound, every version affected, no
patched release, and the upstream repository is archived. Not exploitable here --
the parser runs at build time over nine trusted in-repo files -- but there is no
reason to carry a known-unsound unmaintained dependency.

serde_yaml_bw over the other serde_yaml forks because it is the only one still
alive. Last releases: serde_yaml_bw 2.5.6 (2026-05-02, six releases since Nov
2025); serde_yaml_ng 0.10.0 (2024-05-26); serde_norway 0.9.42 (2024-12-21);
serde_yaml 0.9.34 (deprecated upstream 2024-03-25). serde_yml's own 0.0.13
release is its deprecation notice, pointing at noyalib -- a new crate by the same
maintainer whose previous crate was archived for unsoundness, so not followed.

Verified before adopting, made possible by the preceding determinism fix:

- tweaks.json is byte-identical across all 189 tweaks, so no authored value
  parses differently under the new parser.
- The deny_unknown_fields diagnostic still enumerates every valid field, so the
  build-time typo message an author sees is unchanged.
- "value: null" still deserializes to None, producing the same "'set' action
  requires value" error. ADR-0004 is therefore unaffected, now confirmed against
  the parser actually in use rather than inferred from serde_yml's source.

* docs: mark the 2026-05-29 remediation plan superseded

The plan was largely executed (08fa2e6, 522d9e1, 5131569, 2621dda) but every
checkbox still reads "- [ ]", so a finished plan looks pending.

The boxes are deliberately left unticked rather than marked done: ticking them
without re-verifying each task individually would replace honest ambiguity with
a false record. Instead the header states what was carried forward and what no
longer applies.

Carried forward: Task 1 is genuinely incomplete. Its acceptance criterion
required REG_BINARY values to detect consistently, but the unification reached
detection.rs and stopped before inspection.rs, which still compares with raw JSON
equality. A value authored in the supported "00,A0,FF" hex form matches in one
and mismatches in the other.

No longer applicable: Tasks 2, 3 and 6 concern the profile system, which is being
deleted and rebuilt.

* docs: record the profile v1 archive and hash spec before deleting it

The v1 profile system is about to be deleted and rebuilt. The .mgx archive is
self-describing -- a Deflate ZIP with a format_version field and plain JSON
payloads -- with exactly one exception: the option content hash, which is
embedded in stored profiles to resolve options that moved index, and which
cannot be reconstructed from an archive alone.

Records the three details that would silently break a reimplementation: the
b"profile-option-v2" domain separator is hashed before the payload, the payload
is compact JSON while the archive checksums use pretty-printed JSON, and the
digest is truncated to the first 32 hex characters. Also records the legacy
field-order variant that import still accepted.

Also notes for the rebuild: system_state.json was exported and never read,
per-tweak progress events were modelled and never emitted, and the hash's
sensitivity to TweakOption's serde representation argues for a structural hash
over meaningful fields rather than over serde output.

* refactor(profile): delete the v1 profile system, keep the UI disabled

The profile system was incomplete -- system_state was exported and never read,
per-tweak progress events were modelled and never emitted -- and it is going to
be rebuilt rather than repaired. Carrying a half-built subsystem through the
remaining refactor stages would mean keeping a second apply path working for
code that is going to be thrown away.

Backend deleted (2,169 lines): models/profile.rs, commands/profile.rs, and
services/profile/{mod,archive,export,import,migration,validation}.rs, along with
their 7 Tauri commands, their module wiring, and 6 tests.

Dependencies dropped: zip, sha2, hex and hostname. All four were used only by
profile code -- notably hex, which looked like it was used by registry_value.rs
but is not: the apparent matches there are a local variable named `hex`, error
string prose, and test names.

Also removes RuntimeContext::windows_build, which the deletion orphaned.

Frontend kept, deliberately disabled rather than removed, so the feature reads
as "coming back" rather than "gone":
- The 6 invoke() bodies in src/lib/api/profile.ts are neutralized at that single
  choke point; the ~195 lines of type definitions have no backend dependency and
  still compile. Parameters are underscore-prefixed to preserve the signatures
  for the rebuild.
- ProfileManager's onMount no longer calls loadSavedProfiles(); leaving it would
  have produced an error toast every time the tab is opened.
- A banner explains the state, and the Export/Import entry points in
  SnapshotsView and SettingsModal are disabled.

The archive format and the option content hash are recorded in
docs/spec/profile-v1.md, committed beforehand, since the hash cannot be
reconstructed from a .mgx file alone.

* refactor: delete dead code and add a guard against build/runtime type drift

Removes code with no remaining callers, verified by grep across the Rust source
and the TypeScript frontend rather than by compiler warnings -- cargo check does
not flag most of it, because a pub item in an rlib is API surface, not dead code.
Only unregistering the Tauri commands made the compiler agree.

Deleted:
- models/backup.rs in full (RegistryKeyId, make_key_id, one test); nothing
  referenced it.
- The second, unused YAML pipeline in models/tweak.rs: TweakDefinitionRaw,
  from_raw, TweakFile, validate, all_registry_keys, all_service_names,
  get_registry_changes_for_version, applicable_versions, and the four tests that
  existed only to exercise them. from_raw held a duplicate of the
  ti > system > admin inference rule; only the build.rs copy that actually runs
  remains, so the two can no longer disagree.
- The `aliases` field. Its sole reader was the profile system's validation, so it
  became write-only when that was deleted. Removed from both models/tweak.rs and
  the build.rs mirror -- removing only one side would emit a field that
  TweakDefinition's deny_unknown_fields rejects, panicking on first tweak lookup.
- Eight registered-but-never-invoked Tauri commands and their bodies:
  get_available_tweaks_for_version, get_tweaks_by_category, get_tweak,
  get_backup_system_status, cleanup_old_backups, apply_registry_as_system,
  delete_registry_as_system, get_debug_mode. None appears in any frontend
  invoke() call. cleanup_old_backups migrated away from a legacy
  exe_dir/backups layout and had never been wired to anything.
- Error variants never constructed: UnsupportedWindowsVersion, StateLock,
  Timeout, and ProfileError, plus their code() arms.
- The generated TWEAK_COUNT constant, which nothing read. CATEGORY_COUNT is kept;
  tweak_loader uses it.

Added, in tweak_loader.rs: a test that forces the embedded tweak data to
deserialize into the runtime types. build.rs parses YAML with a hand-maintained
copy of these types and the runtime deserializes the result, so drift between
them surfaces as a panic inside a LazyLock on a user's machine. This turns it
into a test failure instead, and it earned its place immediately: it caught the
aliases mismatch above when a careless `git checkout` reverted one side of it.

* ci: add a Windows CI workflow

The project had no CI: .github/ contained only copilot-instructions.md and
hooks, so the gate documented there ("ALWAYS run bun run validate before
committing") was enforced by discipline alone.

Windows-only by design -- the app is never built or run elsewhere and the Rust
calls Win32 directly.

Four jobs:

- validate: runs the project's own `bun run validate` (prettier, tsc,
  svelte-check, cargo fmt, cargo clippy -D warnings, eslint) plus
  `cargo test --all-targets`.
- line-endings: asserts a fresh checkout really is CRLF. .gitattributes pins
  this, but a violation otherwise surfaces as a confusing `cargo fmt --check`
  failure against newline_style = "Windows" rather than as the line-ending
  problem it is. This job caught two files on its first local run
  (docs/spec/profile-v1.md and Cargo.lock), both fixed here.
- reproducible-build: builds twice from identical YAML and compares the sha256
  of the generated tweaks.json. Guards the property established when build.rs
  moved from HashMap to BTreeMap; without it the regression is invisible until
  someone tries to diff the artifact.
- audit: rustsec/audit-check, which would have flagged RUSTSEC-2025-0068 in
  serde_yml on its own.

RUSTFLAGS: -D warnings is deliberately not set globally -- it applies to
dependencies too, so an upstream warning would fail our build. The clippy step
inside `bun run validate` already scopes -D warnings to this crate.

Author: ehsan18t

.editorconfig

@@ -4,8 +4,10 @@
 root = true
 
 # All files
+# Windows-only application: CRLF everywhere, matching .gitattributes.
 [*]
 charset = utf-8
+end_of_line = crlf
 insert_final_newline = true
 trim_trailing_whitespace = true
 

.gitattributes

@@ -1,2 +1,22 @@
-# Auto detect text files and perform LF normalization
-* text=auto
+# MagicX Toolbox is a Windows-only application. It is never built or run on
+# any other platform, so the working tree is CRLF everywhere, unconditionally.
+#
+# `eol=crlf` makes this a property of the REPOSITORY rather than of each
+# developer's `core.autocrlf` setting: every checkout gets CRLF regardless of
+# local git config. Blobs are still stored LF-normalized in the object database,
+# which is what keeps diffs, merges and blame readable.
+* text=auto eol=crlf
+
+# Binary assets — never touch these.
+*.woff2 binary
+*.png   binary
+*.ico   binary
+*.jpg   binary
+*.jpeg  binary
+*.gif   binary
+*.webp  binary
+*.pdf   binary
+*.zip   binary
+*.mgx   binary
+*.exe   binary
+*.dll   binary

.github/workflows/ci.yml

@@ -0,0 +1,119 @@
+name: CI
+
+# Windows-only by design: this application is never built or run on another
+# platform, and the Rust code calls Win32 APIs directly.
+on:
+  push:
+    branches: [main]
+  pull_request:
+  workflow_dispatch:
+
+concurrency:
+  group: ci-${{ github.ref }}
+  cancel-in-progress: true
+
+env:
+  CARGO_TERM_COLOR: always
+  # Deliberately NOT setting `RUSTFLAGS: -D warnings`: it applies to dependencies
+  # too, so an upstream crate emitting a warning would fail our build. The
+  # `lint:rust` step inside `bun run validate` already passes -D warnings to
+  # clippy for this crate only, which is the behaviour we actually want.
+
+jobs:
+  validate:
+    name: Validate
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: oven-sh/setup-bun@v2
+        with:
+          bun-version: latest
+
+      - uses: dtolnay/rust-toolchain@stable
+        with:
+          components: rustfmt, clippy
+
+      - uses: Swatinem/rust-cache@v2
+        with:
+          workspaces: src-tauri
+
+      - name: Install frontend dependencies
+        run: bun install --frozen-lockfile
+
+      # The project's own gate, per .github/copilot-instructions.md:
+      # prettier, tsc, svelte-check, cargo fmt, cargo clippy, eslint.
+      - name: bun run validate
+        run: bun run validate
+
+      - name: Rust tests
+        working-directory: src-tauri
+        run: cargo test --all-targets
+
+  line-endings:
+    name: Line endings
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      # .gitattributes pins the working tree to CRLF. If a file is committed such
+      # that checkout produces something else, catch it here rather than letting
+      # `cargo fmt --check` fail confusingly on newline_style = "Windows".
+      - name: Assert the checkout is CRLF
+        shell: bash
+        run: |
+          fail=0
+          while IFS= read -r f; do
+            case "$f" in *.woff2|*.png|*.ico|*.jpg|*.jpeg|*.gif|*.webp|*.pdf|LICENSE) continue;; esac
+            [ -f "$f" ] || continue
+            # A tracked text file with no CRLF at all is suspicious; empty and
+            # single-line-without-newline files are not, so require at least one LF.
+            if grep -qU $'\n' "$f" && ! grep -qU $'\r$' "$f"; then
+              echo "::error file=$f::expected CRLF line endings"
+              fail=1
+            fi
+          done < <(git ls-files)
+          exit $fail
+
+  reproducible-build:
+    name: Reproducible tweak data
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: dtolnay/rust-toolchain@stable
+
+      - uses: Swatinem/rust-cache@v2
+        with:
+          workspaces: src-tauri
+
+      # build.rs compiles tweaks/*.yaml into tweaks.json. That artifact must be a
+      # pure function of the YAML: it previously used a HashMap, so key order --
+      # and therefore the bytes -- changed on every build. Guard the property.
+      - name: Same input produces identical output
+        shell: bash
+        working-directory: src-tauri
+        run: |
+          find_artifact() { find target/debug/build -name tweaks.json | head -1; }
+          touch tweaks/privacy.yaml && cargo build --quiet
+          first=$(sha256sum "$(find_artifact)" | cut -d' ' -f1)
+          touch tweaks/privacy.yaml && cargo build --quiet
+          second=$(sha256sum "$(find_artifact)" | cut -d' ' -f1)
+          echo "build 1: $first"
+          echo "build 2: $second"
+          if [ "$first" != "$second" ]; then
+            echo "::error::tweaks.json is not reproducible across builds"
+            exit 1
+          fi
+
+  audit:
+    name: Dependency audit
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: dtolnay/rust-toolchain@stable
+
+      - uses: rustsec/audit-check@v2
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}

.prettierrc

@@ -6,7 +6,7 @@
   "trailingComma": "all",
   "arrowParens": "always",
   "bracketSpacing": true,
-  "endOfLine": "auto",
+  "endOfLine": "crlf",
   "printWidth": 120,
   "plugins": ["prettier-plugin-svelte", "prettier-plugin-tailwindcss"],
   "overrides": [

CONTEXT.md

@@ -0,0 +1,70 @@
+# MagicX Toolbox
+
+A Windows tweaking tool. The user picks a target state for a Windows setting; the app captures what was there before, applies the change, and can put it back.
+
+This file is the glossary — the words we use and the ones we deliberately don't. For what the app *is*, see [docs/APP_CONTEXT.md](./docs/APP_CONTEXT.md). For how to author tweaks, see [docs/TWEAK_AUTHORING.md](./docs/TWEAK_AUTHORING.md).
+
+## Language
+
+### The two meanings of "default"
+
+These are different concepts and conflating them has caused real doc contradictions. Never write bare "Default" without qualifying which one you mean.
+
+**Stock Default**:
+The value Windows itself ships with, before anyone touched the machine. A property of Windows, identical on every clean install. Appears in option labels like "500 KB (Stock Default)".
+_Avoid_: Default, Windows Default, factory setting
+
+**System Default**:
+The state one particular machine's setting was in before this app first touched it. A property of *that machine*, and frequently not the Stock Default — a prior tool, a policy, or the user may have changed it. Selectable whenever a Snapshot exists.
+_Avoid_: Default, original, unmatched state, unknown state
+
+### Tweaks
+
+**Tweak**:
+One configurable Windows setting the app can change, expressed as a set of mutually exclusive target states.
+_Avoid_: setting, hack, mod
+
+**Option**:
+One complete target state of a Tweak, including every change needed to reach it. A Tweak has at least two.
+_Avoid_: value, choice, mode, state
+
+**Applied Option**:
+The Option the app most recently put the machine into. Distinct from what the machine currently *matches*, which can drift when something outside the app changes the same setting.
+_Avoid_: current option, selected option
+
+**Effect Phase**:
+One of the five categories of change an Option can make — registry, services, scheduled tasks, hosts entries, firewall rules. Named as a group because they succeed or fail together.
+_Avoid_: change type, action group
+
+### Safety
+
+**Snapshot**:
+The captured Original State of everything one Tweak touches, taken before the app first changed it. One per Tweak, not one per Option — switching between Options never replaces it.
+_Avoid_: backup, restore point, save state
+
+**Original State**:
+What the machine looked like before this app first applied a given Tweak. What a Snapshot holds, and what System Default returns you to.
+_Avoid_: initial state, baseline, before state
+
+**Apply**:
+Putting the machine into a chosen Option.
+
+**Option Switch**:
+Applying a different Option to a Tweak that already has a Snapshot. Preserves the existing Snapshot rather than taking a new one.
+_Avoid_: re-apply, change option
+
+**Revert**:
+Returning a Tweak to its Original State and releasing the Snapshot. Selecting System Default is a Revert.
+_Avoid_: undo, restore, rollback — *rollback* specifically means something else here
+
+**Rollback**:
+The automatic attempt to undo a *failed* Apply. Distinct from Revert, which is a deliberate user action on a Tweak that applied successfully.
+_Avoid_: revert, undo
+
+**Needs Attention**:
+The state of a Tweak whose Rollback did not fully succeed, leaving the machine partly changed. Retains its Snapshot and names the resources that could not be restored.
+_Avoid_: failed, broken, partial, error state
+
+**Inferred Status**:
+A Tweak status determined by something's *absence* rather than by reading its value — used where a resource does not exist on every Windows edition. Surfaced to the user rather than presented as a confirmed reading.
+_Avoid_: assumed, guessed, implied