Skip to content

apply_patch move: a crash between the two renames leaves a .<name>.<pid>.<ts>.backup file that nothing sweeps (follow-up to #1404) #2371

Open

Description

@easonLiangWorldedtech

Leftover .<name>.<pid>.<ts>.backup files from a failed apply_patch move have nothing that sweeps them

Measured at 8415547c9c4af2752b1d09d5bd2a43478999814e (feat/per-write-checkpoints-b1, PR #1404). Raised while clearing review threads on #1404: thread PRRT_...7p-u concerns the window between two renames, and CodeRabbit itself offered to file this follow-up. The unit that owns the write path answered, and the answer is that no code in the repository looks for these files.

The shape

src/core/tools/ApplyPatchTool.ts stages a move as two renames:

  • :573 names the staging file .${moveBase}.${moveStamp}.staging and :574 names the backup .${moveBase}.${moveStamp}.backup, where moveStamp is ${'$'}{process.pid}.${'$'}{Date.now()} (:570).
  • :588 renames the existing destination onto the backup name, :590 renames the staged file onto the destination.
  • The catch cleans up what it can see: :608 removes the staging file, :617 renames the backup back and sets destinationMissing when that rename itself fails, and :632-634 puts the backup path in the error message so a person can find it.

If the process dies between :588 and :590, nothing observes anything: the destination is absent, its content exists only under .<name>.<pid>.<ts>.backup, and no startup path, task-start path or recovery path enumerates that name. git grep at this head for the backup suffix outside tests returns exactly two producers — this line :574 and the .bak_<ts>_<rand>.tmp name in src/utils/safeWriteJson.ts — and src/services/json-file-recovery/jsonFileRecovery.ts recovers the .new_ and .bak_ JSON shapes, not this one.

Why #1406 does not already cover it

#1406 is feat(checkpoints): per-task change journal with torn-tail repair (B2). Its body is 16,710 characters and the word sweep appears once in it, in the context of removing carried Stryker disable comments. A torn-tail repair of the journal and a sweep of stray workspace backup files are different repairs over different data, and the journal does not record the backup path today.

What a fix has to decide

  • Whether the sweep runs at extension start, at task start, or at the first write into that directory, and what it does with a backup whose destination is missing (restore it, or report it and leave it, since restoring silently can overwrite a file the user recreated in the meantime).
  • Whether the tool records the backup path in the change journal so a later repair can pair a stray backup with the mutation that produced it, which is the one place where feat(checkpoints): per-task change journal with torn-tail repair (B2, #1375) #1406 genuinely joins this.
  • Whether the staging name gets the same treatment: :573 is removed by the in-process catch, and a crash between :581 and :590 leaves it behind for the same reason.

No code change is proposed here and nothing is asserted about which of those is right. This issue exists so the gap is owned somewhere other than a resolved thread.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions