aboutsummaryrefslogtreecommitdiff
path: root/.ai/scripts/tests/test-todo-cleanup.el
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-24 10:56:50 -0500
committerCraig Jennings <c@cjennings.net>2026-07-24 10:56:50 -0500
commit267d1de7b8a8e7fd22b156433567c88216ec3d0f (patch)
tree4289b12c333ea3a0c0c027c4de2c38224a5aced5 /.ai/scripts/tests/test-todo-cleanup.el
parentb34fb4e413eea994f2492669d64d2e5ee9aa8110 (diff)
downloadrulesets-267d1de7b8a8e7fd22b156433567c88216ec3d0f.tar.gz
rulesets-267d1de7b8a8e7fd22b156433567c88216ec3d0f.zip
fix: harden the org-file mutators and close four verified bugs
An overnight hygiene run over this repo, reviewed adversarially afterward. Every problem below was reproduced before it was fixed, and every fix was re-checked by a skeptic who was told to refute it. cj-remove-block could silently destroy content. Its range check validated only the first and last lines, so a span from one cj block's opener to a later block's closer passed and the removal deleted everything between — prose, headings, whole tasks, with a zero exit. That is the failure the check exists to prevent, and drift is its normal case, since the calling skill edits the file while processing. It now refuses a range holding more than one block, backs up first, and writes atomically while preserving the file's mode and following symlinks to the real target. todo-cleanup rewrote todo.org with no backup, alone among the three tools that mutate these files. It now copies to the temp dir like lint-org does. Both backup helpers suffix rather than overwrite, because a second-resolution stamp collapsed two back-to-back invocations into one backup holding already-mutated content — and running todo-cleanup twice in a row is the shipped path. audit.bats failed about one run in eight, in teardown rather than in its assertions, so a passing test reported as a false red. git commit spawns a detached maintenance process that is still writing packs when the commit returns, racing the cleanup. The fixture disables it. route_recommend downgraded a correct strong match to weak when two projects shared a basename. lint.sh now sweeps the scripts that get symlinked onto PATH, which were the only shell in the repo with no gate over them. Two test defects found in review and fixed here: a suite that deleted real backups from the shared temp dir, and one that depended on that directory's contents. Isolation now lives in an autouse fixture rather than in each test's memory. Known and filed, not closed: the range check still cannot prove the range is the block that was scanned, so drift by exactly one whole block deletes the wrong annotation. That needs a content assertion and a CLI contract change.
Diffstat (limited to '.ai/scripts/tests/test-todo-cleanup.el')
-rw-r--r--.ai/scripts/tests/test-todo-cleanup.el103
1 files changed, 102 insertions, 1 deletions
diff --git a/.ai/scripts/tests/test-todo-cleanup.el b/.ai/scripts/tests/test-todo-cleanup.el
index 838913f..a92d238 100644
--- a/.ai/scripts/tests/test-todo-cleanup.el
+++ b/.ai/scripts/tests/test-todo-cleanup.el
@@ -516,6 +516,12 @@ gitignore todo.org, then run `--archive-done' aging with the DEFAULT archive pat
.gitignore contents or nil), :archive-ignored (whether git ignores the archive),
:archive-exists."
(let* ((root (make-temp-file "tc-git-" t))
+ ;; Private backup dir: this helper writes a file literally named
+ ;; todo.org and runs a real (non-check) pass, so without this its
+ ;; backup lands in the shared temp dir under the exact production
+ ;; name and is indistinguishable from a real one.
+ (temporary-file-directory
+ (file-name-as-directory (make-temp-file "tc-git-bk-" t)))
(todo (expand-file-name "todo.org" root))
(archive (expand-file-name "archive/task-archive.org" root))
(gi (expand-file-name ".gitignore" root)))
@@ -536,7 +542,8 @@ gitignore todo.org, then run `--archive-done' aging with the DEFAULT archive pat
:archive-ignored
(eq 0 (call-process "git" nil nil nil "check-ignore" "-q" archive))
:archive-exists (file-readable-p archive)))
- (delete-directory root t))))
+ (delete-directory root t)
+ (delete-directory temporary-file-directory t))))
(ert-deftest tc-age-self-protect-gitignores-archive-when-todo-ignored ()
"When the todo file is gitignored, the aged-out archive is added to .gitignore
@@ -1073,3 +1080,97 @@ line) is left untouched — the strip stops at the first non-planning line."
(provide 'test-todo-cleanup)
;;; test-todo-cleanup.el ends here
+
+;;; ---------------------------------------------------------------------------
+;;; Backup before mutating (parity with lint-org.el / wrap-org-table.el)
+;;
+;; todo-cleanup rewrites todo.org in place and left no copy behind, while both
+;; sibling org-mutators back up to /tmp first. It is also the one that runs most
+;; often (every wrap, every sentry fire). Emacs's own backup does not fire under
+;; --batch -q, so there was genuinely no undo short of git.
+
+(ert-deftest tc-backup-written-before-a-real-mutation ()
+ "A real (non-check) run leaves a copy holding the pre-edit content.
+
+`temporary-file-directory' is rebound to a private dir for the duration: the
+backup name derives from the *file's* basename, and the real todo.org shares
+that basename, so a live sentry run writing /tmp/todo.org.before-todo-cleanup.*
+would otherwise be indistinguishable from this test's own artifact. The first
+version of this test globbed the shared /tmp and passed only until a real run
+created one (2026-07-24)."
+ (let* ((dir (make-temp-file "tc-backup-" t))
+ (bdir (file-name-as-directory (make-temp-file "tc-bk-" t)))
+ (file (expand-file-name "todo.org" dir))
+ (before "* P Open Work\n** TODO [#B] parent\n*** DONE a subtask\nCLOSED: [2026-07-01 Tue]\n"))
+ (unwind-protect
+ (progn
+ (with-temp-file file (insert before))
+ (let ((tc-check-only nil)
+ (tc-convert-subtasks t)
+ (temporary-file-directory bdir))
+ (tc-process-file file))
+ (let ((backups (file-expand-wildcards
+ (concat bdir "todo.org.before-todo-cleanup.*"))))
+ (should backups)
+ (should (string-match-p
+ "a subtask"
+ (with-temp-buffer (insert-file-contents (car backups))
+ (buffer-string))))))
+ (delete-directory dir t)
+ (delete-directory bdir t))))
+
+(ert-deftest tc-no-backup-in-check-mode ()
+ "--check writes nothing, so it must not leave a backup either.
+Uses a private `temporary-file-directory' for the same isolation reason."
+ (let* ((dir (make-temp-file "tc-backup-" t))
+ (bdir (file-name-as-directory (make-temp-file "tc-bk-" t)))
+ (file (expand-file-name "todo.org" dir)))
+ (unwind-protect
+ (progn
+ (with-temp-file file
+ (insert "* P Open Work\n** TODO [#B] parent\n*** DONE sub\nCLOSED: [2026-07-01 Tue]\n"))
+ (let ((tc-check-only t)
+ (tc-convert-subtasks t)
+ (temporary-file-directory bdir))
+ (tc-process-file file))
+ (should-not (file-expand-wildcards
+ (concat bdir "todo.org.before-todo-cleanup.*"))))
+ (delete-directory dir t)
+ (delete-directory bdir t))))
+
+(ert-deftest tc-backup-never-overwrites-an-earlier-one ()
+ "Two invocations in the same second must not collapse to one backup.
+
+open-tasks.org runs --convert-subtasks then --archive-done back to back, each
+a sub-second batch run. With a second-resolution stamp and copy-file's
+OK-IF-ALREADY-EXISTS, the second invocation overwrote the first's backup with
+already-mutated content, so the true pre-session original was unrecoverable —
+the exact state the backup exists to preserve (found 2026-07-24 in review)."
+ (let* ((dir (make-temp-file "tc-collide-" t))
+ (bdir (file-name-as-directory (make-temp-file "tc-cbk-" t)))
+ (file (expand-file-name "todo.org" dir))
+ (original (concat "* P Open Work\n** TODO [#B] parent\n*** DONE sub\n"
+ "CLOSED: [2026-07-01 Tue]\n"
+ "* P Resolved\n** DONE [#C] old\nCLOSED: [2025-01-01 Wed]\n")))
+ (unwind-protect
+ (progn
+ (with-temp-file file (insert original))
+ ;; Two back-to-back invocations, as the shipped workflow does.
+ (let ((temporary-file-directory bdir))
+ (let ((tc-check-only nil) (tc-convert-subtasks t))
+ (tc-process-file file))
+ (let ((tc-check-only nil) (tc-convert-subtasks nil) (tc-archive-done t)
+ (tc-archive-retain-days nil))
+ (tc-process-file file)))
+ (let ((backups (file-expand-wildcards
+ (concat bdir "todo.org.before-todo-cleanup.*"))))
+ ;; Both invocations kept their own backup.
+ (should (= (length backups) 2))
+ ;; And one of them still holds the true original.
+ (should (cl-some (lambda (b)
+ (string= original
+ (with-temp-buffer (insert-file-contents b)
+ (buffer-string))))
+ backups))))
+ (delete-directory dir t)
+ (delete-directory bdir t))))