path: root/.ai/scripts/tests/test_cj_remove_block.py

"""Tests for cj-remove-block.py — idempotent removal of cj annotations by line range.

The script removes lines [start, end] (1-indexed, inclusive) from an org file but
validates first that those lines actually look like a cj annotation. Refusing on
mismatch protects against accidentally trimming the wrong block when line numbers
drift between scan and remove calls.
"""

import subprocess
from pathlib import Path

import pytest

SCRIPT = Path(__file__).parent.parent / "cj-remove-block.py"


@pytest.fixture(autouse=True)
def isolated_tmpdir(tmp_path, monkeypatch):
    """Give every test in this module a private TMPDIR.

    The script backs up to the system temp dir under a name derived from the
    edited file's BASENAME. The real todo.org shares that basename, so any test
    operating on a fixture named todo.org writes something indistinguishable
    from a production backup — and an earlier version of this file globbed the
    shared /tmp and unlinked every match, so a routine `make test` destroyed
    Craig's real backups (found in review, 2026-07-24).

    Isolating at module scope rather than per-test is deliberate: the same bug
    was fixed once in the elisp sibling and left here, so relying on each new
    test to remember is exactly how it recurred. Autouse makes it structural.
    """
    d = tmp_path / "_tmpdir"
    d.mkdir()
    # TMPDIR covers subprocess invocations of the script.
    monkeypatch.setenv("TMPDIR", str(d))
    # tempfile.gettempdir() caches its answer on first call, so a test that
    # loads the module in-process would keep writing to the real /tmp no matter
    # what TMPDIR says. Override the cache too — this is the gap that made the
    # env-var-only version still leak one backup per suite run.
    import tempfile as _tempfile
    monkeypatch.setattr(_tempfile, "tempdir", str(d))
    return d


@pytest.fixture
def run_remove(tmp_path):
    """Write content to a temp org file, run cj-remove-block, return new contents."""
    def _run(content: str, start: int, end: int) -> str:
        f = tmp_path / "test.org"
        f.write_text(content)
        subprocess.run(
            ["python3", str(SCRIPT),
             "--file", str(f),
             "--start", str(start),
             "--end", str(end)],
            check=True,
            capture_output=True,
        )
        return f.read_text()
    return _run


@pytest.fixture
def run_remove_expecting_failure(tmp_path):
    """Write content, run cj-remove-block expecting non-zero exit; return CalledProcessError."""
    def _run(content: str, start: int, end: int):
        f = tmp_path / "test.org"
        f.write_text(content)
        with pytest.raises(subprocess.CalledProcessError) as excinfo:
            subprocess.run(
                ["python3", str(SCRIPT),
                 "--file", str(f),
                 "--start", str(start),
                 "--end", str(end)],
                check=True,
                capture_output=True,
            )
        return excinfo.value, f.read_text()  # file should be unchanged on failure
    return _run


# ----------------------------------------------------------------------
# Source-block removal
# ----------------------------------------------------------------------

class TestCjRemoveBlockSourceBlock:
    """Removing #+begin_src cj: ... #+end_src blocks."""

    def test_cj_remove_block_minimal_three_line_source_block(self, run_remove):
        """Normal: the three lines of a minimal source-block are removed."""
        content = "* S\n#+begin_src cj: comment\nbody\n#+end_src\nafter\n"
        result = run_remove(content, start=2, end=4)
        assert result == "* S\nafter\n"

    def test_cj_remove_block_source_block_multiline_body(self, run_remove):
        """Normal: source-block with multi-line body removed cleanly."""
        content = "* S\n#+begin_src cj: comment\nline 1\nline 2\nline 3\n#+end_src\nafter\n"
        result = run_remove(content, start=2, end=6)
        assert result == "* S\nafter\n"

    def test_cj_remove_block_preserves_lines_before_and_after(self, run_remove):
        """Normal: surrounding lines outside the range stay intact."""
        content = "before\n#+begin_src cj: comment\nx\n#+end_src\nafter\n"
        result = run_remove(content, start=2, end=4)
        assert result == "before\nafter\n"

    def test_cj_remove_block_source_block_with_label_variant(self, run_remove):
        """Boundary: source-block with no trailing label (#+begin_src cj:) also removable."""
        content = "* S\n#+begin_src cj:\nbody\n#+end_src\nafter\n"
        result = run_remove(content, start=2, end=4)
        assert result == "* S\nafter\n"

    def test_cj_remove_block_case_insensitive_fence(self, run_remove):
        """Boundary: case-variant fences (#+BEGIN_SRC / #+END_SRC) also removable."""
        content = "* S\n#+BEGIN_SRC cj: comment\nbody\n#+END_SRC\nafter\n"
        result = run_remove(content, start=2, end=4)
        assert result == "* S\nafter\n"


# ----------------------------------------------------------------------
# Legacy-inline removal
# ----------------------------------------------------------------------

class TestCjRemoveBlockLegacyInline:
    """Removing single-line legacy `cj: ...` annotations."""

    def test_cj_remove_block_legacy_inline_single_line(self, run_remove):
        """Normal: single legacy-inline cj line removed."""
        content = "* S\ncj: legacy note\nafter\n"
        result = run_remove(content, start=2, end=2)
        assert result == "* S\nafter\n"

    def test_cj_remove_block_legacy_inline_at_eof(self, run_remove):
        """Boundary: legacy-inline cj at last line; file ends cleanly."""
        content = "* S\ncj: at end\n"
        result = run_remove(content, start=2, end=2)
        assert result == "* S\n"


# ----------------------------------------------------------------------
# Refusal-on-mismatch safety
# ----------------------------------------------------------------------

class TestCjRemoveBlockSafety:
    """Refuses to remove if the specified range doesn't look like a cj annotation."""

    def test_cj_remove_block_refuses_non_cj_single_line(self, run_remove_expecting_failure):
        """Error: a single non-cj line is rejected."""
        err, post_content = run_remove_expecting_failure(
            "* S\nthis is not a cj line\nafter\n", start=2, end=2,
        )
        assert err.returncode != 0
        # File must be unchanged
        assert post_content == "* S\nthis is not a cj line\nafter\n"

    def test_cj_remove_block_refuses_mismatched_fence(self, run_remove_expecting_failure):
        """Error: multi-line range where line N isn't an opening fence is rejected."""
        err, post_content = run_remove_expecting_failure(
            "* S\nbody1\nbody2\n#+end_src\nafter\n", start=2, end=4,
        )
        assert err.returncode != 0
        assert "body1" in post_content  # file unchanged

    def test_cj_remove_block_refuses_missing_closing_fence(self, run_remove_expecting_failure):
        """Error: multi-line range where line M isn't a closing fence is rejected."""
        err, post_content = run_remove_expecting_failure(
            "* S\n#+begin_src cj: comment\nbody\nnot-a-close\nafter\n", start=2, end=4,
        )
        assert err.returncode != 0
        assert "not-a-close" in post_content

    def test_cj_remove_block_refuses_out_of_bounds(self, run_remove_expecting_failure):
        """Error: range outside the file is rejected, file unchanged."""
        err, post_content = run_remove_expecting_failure(
            "* S\nafter\n", start=5, end=7,
        )
        assert err.returncode != 0
        assert post_content == "* S\nafter\n"

    def test_cj_remove_block_refuses_inverted_range(self, run_remove_expecting_failure):
        """Error: end < start is rejected, file unchanged."""
        original = "* S\n#+begin_src cj: comment\nbody\n#+end_src\n"
        err, post_content = run_remove_expecting_failure(original, start=4, end=2)
        assert err.returncode != 0
        assert post_content == original


class TestMultiBlockRangeRefused:
    """The validation exists to catch a drifted range, but it only checked the
    first and last lines of that range. A span from one block's opening fence to
    a LATER block's closing fence passed, and the removal silently deleted every
    line between — real prose, headings, whole tasks — with a zero exit. Drift is
    the skill's normal operating mode (respond-to-cj-comments edits the file as it
    processes, and a file under cj review usually holds several blocks), so this
    is the exact scenario the check was written for. Reproduced 2026-07-24."""

    TWO_BLOCKS = (
        "* Alpha\n"
        "#+begin_src cj:\n"
        "note A\n"
        "#+end_src\n"
        "KEEP THIS LINE\n"
        "* Beta\n"
        "#+begin_src cj:\n"
        "note B\n"
        "#+end_src\n"
    )

    def test_range_spanning_two_blocks_is_refused(self, run_remove_expecting_failure):
        # Lines 2..9: block one's opener through block two's closer.
        err, content = run_remove_expecting_failure(self.TWO_BLOCKS, 2, 9)
        assert err.returncode == 1
        assert "KEEP THIS LINE" in content, "content between the blocks was destroyed"
        assert "* Beta" in content, "a heading between the blocks was destroyed"

    def test_refusal_names_the_reason(self, run_remove_expecting_failure):
        err, _ = run_remove_expecting_failure(self.TWO_BLOCKS, 2, 9)
        assert "more than one" in err.stderr.decode().lower()

    def test_a_correct_single_block_range_still_removes(self, run_remove):
        # The fix must not over-tighten: the legitimate range still works.
        out = run_remove(self.TWO_BLOCKS, 2, 4)
        assert "note A" not in out
        assert "KEEP THIS LINE" in out
        assert "note B" in out, "the second block must be untouched"

    def test_a_nested_end_src_inside_the_range_is_refused(self, run_remove_expecting_failure):
        # Any #+end_src before the final line means the range covers >1 block.
        content = (
            "#+begin_src cj:\n"
            "a\n"
            "#+end_src\n"
            "middle\n"
            "#+begin_src cj:\n"
            "b\n"
            "#+end_src\n"
        )
        err, after = run_remove_expecting_failure(content, 1, 7)
        assert err.returncode == 1
        assert "middle" in after


class TestSafeMutation:
    """The script rewrites Craig's org files (todo.org, notes.org). It wrote with
    a bare write_text, which truncates the target on open, and took no backup —
    so a mid-write failure left the file truncated with no copy to recover from.
    lint-org.el, the other tool that mutates these files, backs up to a temp dir
    first. Match that, and make the write atomic.

    Every test here redirects TMPDIR to a private directory. The backup name
    derives from the file's basename, and the real todo.org shares it, so a test
    globbing the shared temp dir cannot tell its own artifact from a genuine
    backup — and an earlier version of this class globbed /tmp and unlinked every
    match, so a routine `make test` destroyed real backups (found in review,
    2026-07-24). Never glob or delete across the shared temp dir."""

    ONE_BLOCK = "* T\n#+begin_src cj:\nnote\n#+end_src\nkeep\n"

    def test_a_backup_is_written_before_mutating(self, tmp_path):
        import subprocess, glob, os
        bdir = tmp_path / "bk"
        bdir.mkdir()
        f = tmp_path / "todo.org"
        f.write_text(self.ONE_BLOCK)
        subprocess.run(
            ["python3", str(SCRIPT), "--file", str(f), "--start", "2", "--end", "4"],
            check=True, capture_output=True,
            env={**os.environ, "TMPDIR": str(bdir)},
        )
        backups = glob.glob(str(bdir / "todo.org.before-cj-remove.*"))
        assert backups, "no backup was written before mutating the org file"
        assert "note" in Path(max(backups)).read_text()

    def test_no_partial_file_when_the_write_fails(self, tmp_path, monkeypatch):
        import importlib.util
        spec = importlib.util.spec_from_file_location("crb", SCRIPT)
        mod = importlib.util.module_from_spec(spec)
        spec.loader.exec_module(mod)
        bdir = tmp_path / "bk"
        bdir.mkdir()
        monkeypatch.setenv("TMPDIR", str(bdir))
        f = tmp_path / "todo.org"
        f.write_text(self.ONE_BLOCK)
        def boom(*a, **k):
            raise OSError("disk full")
        monkeypatch.setattr(mod.os, "replace", boom)
        with pytest.raises(OSError):
            mod.remove_range(f, 2, 4)
        # The original survives intact — no truncation, no partial.
        assert f.read_text() == self.ONE_BLOCK


class TestBackupNeverOverwrites:
    """Same defect class as todo-cleanup's, and more reachable here: the
    respond-to-cj-comments skill removes several annotations in quick
    succession, so a second-resolution stamp collides and the later backup
    overwrote the earlier one with already-mutated content."""

    TWO_BLOCKS = (
        "* A\n#+begin_src cj:\nfirst\n#+end_src\n"
        "* B\n#+begin_src cj:\nsecond\n#+end_src\n"
    )

    def test_consecutive_removals_each_keep_a_backup(self, tmp_path, monkeypatch):
        import subprocess, glob
        bdir = tmp_path / "bk"
        bdir.mkdir()
        monkeypatch.setenv("TMPDIR", str(bdir))
        f = tmp_path / "todo.org"
        f.write_text(self.TWO_BLOCKS)
        original = f.read_text()
        # Remove the second block, then the first — back to back, same second.
        subprocess.run(["python3", str(SCRIPT), "--file", str(f),
                        "--start", "6", "--end", "8"],
                       check=True, capture_output=True,
                       env={**__import__("os").environ, "TMPDIR": str(bdir)})
        subprocess.run(["python3", str(SCRIPT), "--file", str(f),
                        "--start", "2", "--end", "4"],
                       check=True, capture_output=True,
                       env={**__import__("os").environ, "TMPDIR": str(bdir)})
        backups = glob.glob(str(bdir / "todo.org.before-cj-remove.*"))
        assert len(backups) == 2, f"expected 2 backups, got {len(backups)}"
        contents = [Path(b).read_text() for b in backups]
        assert original in contents, "no backup holds the true original"