Skip to content

The patch lock is scoped to CLODEX_HOME while the backup directory is shared #232

Description

@bman654

The patch lock is CLODEX_HOME/patch.lock (getPatchLockPath), but the pristine-backup directory is
~/.tweakcc (or TWEAKCC_CONFIG_DIR) and is shared across every CLODEX_HOME. Two clodex
profiles therefore take different locks while writing the same files. Separately, --restore takes no
lock at all, and clodex patch reads the manifest and the live binary's state before acquiring one.

Three consequences, all found while reviewing #204:

  1. A provenance record's confidence can be replaced by a weaker one. recordBackupProvenance
    reads the existing record and then publishes by temp + rename. The rename is atomic for readers,
    but the read-decide-write is not, so two processes recording the SAME install can race and the last
    rename wins — an established record can be overwritten with assumed: true. Measured directly: two
    writers forced to observe the same absent target produced assumed: false or assumed: true
    purely by rename order. Distinct installs do not race (one file each — 100 concurrent writers
    produced 100 correct records); only the same install's confidence does.
  2. A stale --restore can unlink a manifest a concurrent patch just wrote. Restore copies, a patch
    renames and writes a new manifest, and the restore then clears it. This behaviour predates clodex patch --restore still picks a backup by version tag when no manifest exists #204.
  3. clodex patch decides on state it read before locking, so its plan can be based on a binary
    another process has since replaced.

Reachability

Needs two overlapping clodex processes. That is supported but unusual — two CLODEX_HOME profiles
sharing one TWEAKCC_CONFIG_DIR, or a --restore run against a patch in flight. Consequence for (1)
is a later false refusal that a subsequent patch usually self-heals, not wrong bytes; (2) leaves a
patched binary with no manifest, which is recoverable.

Fix

One lock keyed by the shared resource rather than by CLODEX_HOME — the resolved binary path, or
the backup directory — acquired by BOTH clodex patch and clodex patch --restore, and acquired
before reading mutable manifest/live/backup state. Atomic no-replace creation (openSync(…, 'wx'))
handles a record's FIRST write, but a confidence update also needs either that lock or a
compare-and-swap retry, since "never downgrade established" is a read-then-write decision.

Documented as a stated limitation under "Pristine backups are content-addressed" in
.claude/docs/patcher.md.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions