From 3c884dd3f9aad6942ef9953bdaea4c0cae4f4fe0 Mon Sep 17 00:00:00 2001 From: tegwick Date: Fri, 28 Aug 2026 11:16:12 +0200 Subject: [PATCH] fix(projection): a retirement must survive the next pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five rows came back tombstoned but not retired — a zombie beside the correct record derived from the same file. Two of my own changes combined to produce it. Releasing the identifier destroys the evidence that made the row a re-key: the tombstoned slug is no longer a work-record identifier, and a legacy row is not UUID-derived, so both guards fall through to path matching — which matches the very file the row was retired for and resurrects it. And the release loop ran before matching, so it also stamped rows that were about to derive again, leaving them unable to match their own file and never un-retired. Two changes. A retired row is now matched by UUID or by its own identifier, and never by the path heuristics; a record that genuinely returns still un-retires, because its identifier is unchanged. And the release exempts any row the forge still derives. 746 pass. Co-Authored-By: Claude Opus 5 Assistant: claude-code Assistant-Model: opus Assistant-Process: 2583210@bnt-lap001 Assistant-Session: f2bff2d5-e9b2-4338-92ca-10282a927006 --- api/services/forge_projection.py | 48 ++++++++++++++++++++++++-------- tests/test_forge_projection.py | 43 ++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/api/services/forge_projection.py b/api/services/forge_projection.py index 96cc24d..7773269 100644 --- a/api/services/forge_projection.py +++ b/api/services/forge_projection.py @@ -692,17 +692,6 @@ async def reset_repository_projection( ).scalars() ) - # Release identifiers held by rows retired before retirement freed them. - # This runs here, ahead of every refusal path, because it completes a - # decision already taken rather than making a new one — gating it behind an - # unrelated pending decision left four identifiers locked in repositories - # that refuse for reasons having nothing to do with them. - for r in rows: - if r.projection_retired_at is not None and RETIRED_SLUG_MARK not in (r.slug or ""): - before = r.slug - r.slug = _tombstone_slug(r.slug or str(r.id), r.projection_retired_at) - outcome.released.append(before) - want = {w.record_id.strip().lower(): w for w in derived.workplans} want_paths = {_path_key(w.relative_path): k for k, w in want.items()} @@ -713,6 +702,24 @@ async def reset_repository_projection( # from being read as a re-key and proposed for retirement. want_by_uuid = {w.uuid: k for k, w in want.items()} + # Release identifiers held by rows retired before retirement freed them. + # This runs ahead of every refusal path, because it completes a decision + # already taken rather than making a new one — gating it behind an unrelated + # pending decision left four identifiers locked in repositories refusing for + # reasons having nothing to do with them. + # + # A row the forge still derives is exempt: stamping it would change the slug + # out from under the matching below, so the record would fail to match its + # own file and never be un-retired. + for r in rows: + if r.projection_retired_at is None or RETIRED_SLUG_MARK in (r.slug or ""): + continue + if (r.slug or "").strip().lower() in want or str(r.id) in want_by_uuid: + continue + before = r.slug + r.slug = _tombstone_slug(r.slug or str(r.id), r.projection_retired_at) + outcome.released.append(before) + matched: dict[str, Any] = {} for row in rows: key = (row.slug or "").strip().lower() @@ -720,6 +727,25 @@ async def reset_repository_projection( if by_uuid is not None: matched[by_uuid] = row continue + if key in want: + # The identifier itself derives again — including for a retired row, + # which is then deliberately un-retired below. + matched[key] = row + continue + if row.projection_retired_at is not None: + # Past this point only the heuristics remain, and a retired row must + # not be matched by those. Retirement is a + # decision, and re-deriving the same file must not silently undo it. + # + # Releasing the identifier makes this necessary: the tombstoned slug + # is no longer an identifier and a legacy row is not UUID-derived, so + # both re-key guards fall through to path matching — which matches + # the file the row was retired *for* and resurrects it, alongside the + # correct record already created from that same file. + # + # A record that genuinely returns matches by UUID or by its own + # identifier above, and is un-retired there. + continue if key not in want: if _identity_is_derived(row) or _slug_is_identifier(row.slug or ""): # ADR-007: a record identified by an identifier *is* that diff --git a/tests/test_forge_projection.py b/tests/test_forge_projection.py index 51c9e0e..3787c31 100644 --- a/tests/test_forge_projection.py +++ b/tests/test_forge_projection.py @@ -887,3 +887,46 @@ status: todo def test_uuid_follows_the_qualified_id(self): t = fp._parse_tasks(self.BODY, "LLM-WP-0001")[0] assert t.uuid == fp.derived_record_uuid("LLM-WP-0001-T01") + + +class TestRetirementIsNotUndoneByPath: + """A retired row is matched by UUID or not at all. + + Releasing the identifier removes the evidence that made the row a re-key: + the tombstoned slug is no longer an identifier, and a legacy row is not + UUID-derived, so both guards fall through to path matching — which matches + the very file the row was retired for and resurrects it beside the correct + record created from that same file. + """ + + def test_a_tombstoned_slug_is_not_an_identifier(self): + """This is the signal the tombstone destroys.""" + assert fp._slug_is_identifier("adhoc-2026-06-01") + assert not fp._slug_is_identifier("adhoc-2026-06-01@retired-20260827") + + def test_retired_rows_are_skipped_before_path_matching(self): + import inspect + src = inspect.getsource(fp.reset_repository_projection) + skip_at = src.index("if row.projection_retired_at is not None:") + path_at = src.index("if bp and _path_key(bp) in want_paths:") + assert skip_at < path_at, "retired rows must be skipped before path matching" + + def test_uuid_match_still_precedes_the_skip(self): + """A record that genuinely returns must still be un-retired.""" + import inspect + src = inspect.getsource(fp.reset_repository_projection) + assert src.index("want_by_uuid.get(str(row.id))") < src.index( + "if row.projection_retired_at is not None:" + ) + + def test_a_row_that_derives_again_is_not_tombstoned(self): + """Releasing it would change the slug out from under the matching. + + The row would then fail to match its own file and never be un-retired — + which is how five rows ended up tombstoned but not retired. + """ + import inspect + src = inspect.getsource(fp.reset_repository_projection) + rel = src.index("outcome.released.append(before)") + guard = src.index('in want or str(r.id) in want_by_uuid') + assert guard < rel, "the release must exempt records the forge still derives"