From 76d4ee45cf8e8856df50e552f6535ea128b7a6d9 Mon Sep 17 00:00:00 2001 From: ProtocolWarden <32967198+ProtocolWarden@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:05:36 -0400 Subject: [PATCH] fix(reconcile): repair a pointer section instead of appending a second one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_ensure_pointer` recognised the pointer by heading AND `_POINTER_PREFIX`, so a section already headed `Archived` whose body lacked the prefix matched neither branch: the loop fell through and appended a SECOND `## Archived` beside it. #53 flagged this and left it. It is not hypothetical — that PR nearly shipped it. Its first end-to-end fixture wrote the legacy pointer with an ASCII `->` instead of the prefix's Unicode `→`, produced two `## Archived` sections, and passed the whole unit suite. Two ways in: an operator writes the heading by hand, or an older run's wording drifts from the constant. Matching on heading alone and branching on the body inside. A section under the pointer heading with no pointer line is now repaired rather than ignored. Repaired, not replaced. Replacing wholesale would discard whatever prose is already under that heading, and this module goes out of its way elsewhere to preserve an operator's annotations on a portable pointer — so the pointer line is inserted under the heading and the remaining body kept verbatim. That also keeps the invariant the caller actually needs: after a prune, exactly one `## Archived` section, carrying a correct portable ref. 6 tests. Each fails against the pre-fix predicate — checked by reverting it, which is worth stating because two of them did NOT fail on the first draft: the pre-fix code also preserves the prose and is also stable on re-run, just at two sections rather than one. Both now assert the section count, which is the thing that actually distinguishes the behaviours. Suite 488 -> 494 passed; the 16 failures are unchanged and identical, and `test_committed.py` / `test_signing.py` still fail to import for lack of `cryptography` in this environment. No narrative log entry, following #53: this repo's log is at exactly 400/400, so adding one would trip RC1 and force a prune to land a commit about pruning. Custodian ADR 0001 argues rationale belongs in the commit message instead. Co-Authored-By: Claude Opus 5 --- src/context_lifecycle/reconcile/prune.py | 47 +++++++++++++-- tests/test_archive_pointer.py | 74 ++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/src/context_lifecycle/reconcile/prune.py b/src/context_lifecycle/reconcile/prune.py index 0ad4ef0..786a1ca 100644 --- a/src/context_lifecycle/reconcile/prune.py +++ b/src/context_lifecycle/reconcile/prune.py @@ -340,20 +340,57 @@ def _pointer_section(archive_file: Path, *, private_root: Path | None = None) -> _ABSOLUTE_REF_RE = re.compile(r"`(?:[A-Za-z]:[\\/]|/)") +def _pointer_line(pointer: Section) -> str: + """The `_Archived … → ``_` line on its own, without the heading.""" + for line in pointer.body.splitlines(keepends=True): + if _POINTER_PREFIX in line: + return line + return "" + + +def _merge_pointer_into(sec: Section, pointer: Section) -> Section: + """Insert the pointer line into an existing section, keeping its prose. + + Used when a section already carries the pointer heading but no pointer + line. Replacing it wholesale would discard whatever the operator wrote + there, and this module elsewhere goes out of its way to preserve + annotations — so the pointer is inserted under the heading and the rest of + the body is kept verbatim. + """ + lines = sec.body.splitlines(keepends=True) + heading_line = lines[0] if lines else f"## {sec.heading}\n" + remainder = "".join(lines[1:]).strip("\n") + body = f"{heading_line}\n{_pointer_line(pointer)}" + if remainder: + body += f"\n{remainder}\n" + return Section(heading=sec.heading, body=body.rstrip("\n") + "\n\n") + + def _ensure_pointer(sections: list[Section], pointer: Section) -> list[Section]: - """Append the pointer section, or upgrade a legacy absolute one in place. + """Append the pointer section, or repair an existing one in place. Idempotent for any pointer already in the portable form — including one an operator has annotated, which is left untouched. A pointer carrying an ABSOLUTE path is replaced: older runs embedded the operator's home directory (and often their name) into tracked files, and leaving that in place would keep leaking it every time the file is read. + + A section under the pointer heading that carries no pointer line at all — + hand-written, or an older run whose wording drifted from ``_POINTER_PREFIX`` + — is repaired rather than ignored. Matching on heading *and* prefix meant + such a section was not recognised, so a second ``## Archived`` was appended + beside it and the file ended up with two. That very nearly shipped: a test + fixture written with an ASCII ``->`` instead of the prefix's ``→`` produced + exactly this and passed the unit suite. """ for i, sec in enumerate(sections): - if sec.heading == pointer.heading and _POINTER_PREFIX in sec.body: - if _ABSOLUTE_REF_RE.search(sec.body): - return sections[:i] + [pointer] + sections[i + 1 :] - return sections # already portable — leave it, annotations and all + if sec.heading != pointer.heading: + continue + if _POINTER_PREFIX not in sec.body: + return sections[:i] + [_merge_pointer_into(sec, pointer)] + sections[i + 1 :] + if _ABSOLUTE_REF_RE.search(sec.body): + return sections[:i] + [pointer] + sections[i + 1 :] + return sections # already portable — leave it, annotations and all return sections + [pointer] diff --git a/tests/test_archive_pointer.py b/tests/test_archive_pointer.py index f533bc4..816a4c7 100644 --- a/tests/test_archive_pointer.py +++ b/tests/test_archive_pointer.py @@ -137,6 +137,80 @@ def test_upgrades_a_legacy_absolute_pointer_in_place(self, tmp_path, legacy_raw) assert PRIVATE_ROOT_PLACEHOLDER in out[1].body +class TestPointerHeadingWithoutPrefix: + """A section under the pointer heading that carries no pointer line. + + Matching on heading AND prefix meant such a section went unrecognised, so a + second ``## Archived`` was appended beside it. Reachable two ways: an + operator writes the heading by hand, or an older run's wording drifts from + ``_POINTER_PREFIX`` — which is exactly how the ASCII-arrow fixture slipped + through the suite that shipped this module. + """ + + def _pointer(self, tmp_path) -> Section: + root = tmp_path / "P" + return _pointer_section( + root / "archive" / "console" / "Custodian" / "log-2026-08-03.md", + private_root=root, + ) + + def test_does_not_append_a_second_section(self, tmp_path): + bare = Section(heading="Archived", body="## Archived\n\nSee the private manifest.\n\n") + out = _ensure_pointer([bare], self._pointer(tmp_path)) + assert len(out) == 1 + assert out[0].body.count("## Archived") == 1 + + def test_inserts_the_pointer_line(self, tmp_path): + bare = Section(heading="Archived", body="## Archived\n\nSee the private manifest.\n\n") + out = _ensure_pointer([bare], self._pointer(tmp_path)) + assert _POINTER_PREFIX in out[0].body + assert PRIVATE_ROOT_PLACEHOLDER in out[0].body + + def test_keeps_the_existing_prose_in_the_repaired_section(self, tmp_path): + """Replacing wholesale would discard whatever the operator wrote. + + Asserts the prose and the pointer land in the SAME single section — the + pre-fix code also kept the prose, just in a section sitting beside a + second ``## Archived`` heading, so a laxer assertion passes either way. + """ + bare = Section(heading="Archived", body="## Archived\n\nSee the private manifest.\n\n") + out = _ensure_pointer([bare], self._pointer(tmp_path)) + assert len(out) == 1 + assert "See the private manifest." in out[0].body + assert _POINTER_PREFIX in out[0].body + + def test_ascii_arrow_variant_is_repaired_not_duplicated(self, tmp_path): + """The near-miss made concrete: prefix wording drifts, heading does not.""" + drifted = Section( + heading="Archived", + body="## Archived\n\n_Archived completed history -> `/x.md`_\n\n", + ) + out = _ensure_pointer([drifted], self._pointer(tmp_path)) + assert len(out) == 1 + assert out[0].body.count("## Archived") == 1 + assert _POINTER_PREFIX in out[0].body + + def test_result_is_idempotent(self, tmp_path): + """A second prune over the repaired section must change nothing. + + The length assertion is what makes this bite: the pre-fix code is also + stable on re-run, but stable at two sections rather than one. + """ + p = self._pointer(tmp_path) + bare = Section(heading="Archived", body="## Archived\n\nSee the private manifest.\n\n") + once = _ensure_pointer([bare], p) + assert len(once) == 1 + assert _ensure_pointer(once, p) == once + + def test_other_sections_are_untouched(self, tmp_path): + bare = Section(heading="Archived", body="## Archived\n\nnote\n\n") + before = Section(heading="Before", body="## Before\n\nb\n\n") + after = Section(heading="After", body="## After\n\na\n\n") + out = _ensure_pointer([before, bare, after], self._pointer(tmp_path)) + assert len(out) == 3 + assert out[0] is before and out[2] is after + + class TestPruneEndToEnd: """Unit tests cover the pointer in isolation; this drives a real prune.