From 8b207a991a68ccd1111c26b103e85bae5fb7ea8f Mon Sep 17 00:00:00 2001 From: tegwick Date: Wed, 26 Aug 2026 02:05:51 +0200 Subject: [PATCH] feat(tasks): give task rows a canonical record identifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every work-record type carried a stable identifier except tasks, whose rows held only id, workplan_id, title, status and priority — nothing connecting a row to CUST-WP-0067-T01 in the file it came from. Matching was therefore by title, so a renamed heading looked like one task vanishing and another appearing, and the forge-derived reset had to refuse to touch tasks at all. Adds tasks.record_id (nullable: no migration can invent an identity for an existing row) and a backfill that reads the pairing from the repository files, where a task declares both its canonical id and its projection UUID. 5516 pairs across 121 repositories with zero conflicts; 4456 of 6073 cache task rows identified. Diff and reset now key on record_id where present, falling back to a title-prefixed key so an unidentified row stays visibly unidentified. Unknown stays unknown: a row the files do not claim keeps no identity and the reset keeps refusing to act on it, and an existing identity is never overwritten — a mismatch is recorded as a conflict rather than resolved. Refs STATE-WP-0083-T06 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/models/task.py | 6 + api/services/forge_projection.py | 19 ++- api/services/task_record_id_backfill.py | 116 +++++++++++++++++ .../e2b3c4d5f6a7_task_canonical_record_id.py | 36 +++++ tests/test_task_record_id_backfill.py | 123 ++++++++++++++++++ ...-WP-0083-forge-derived-projection-reset.md | 31 ++++- 6 files changed, 325 insertions(+), 6 deletions(-) create mode 100644 api/services/task_record_id_backfill.py create mode 100644 migrations/versions/e2b3c4d5f6a7_task_canonical_record_id.py create mode 100644 tests/test_task_record_id_backfill.py diff --git a/api/models/task.py b/api/models/task.py index f119c06..c37816a 100644 --- a/api/models/task.py +++ b/api/models/task.py @@ -36,6 +36,12 @@ class Task(Base, TimestampMixin): nullable=False, index=True, ) + # Canonical work-record identifier, e.g. "CUST-WP-0067-T01" (ADR-007). + # Nullable because rows created before STATE-WP-0083-T06 have none; only the + # repository files know the mapping. Without it a task can be matched only by + # title, so renaming a heading looks like one task vanishing and another + # appearing. + record_id: Mapped[str | None] = mapped_column(String(120), nullable=True, index=True) title: Mapped[str] = mapped_column(String(255), nullable=False) description: Mapped[str | None] = mapped_column(Text, nullable=True) status: Mapped[TaskStatus] = mapped_column( diff --git a/api/services/forge_projection.py b/api/services/forge_projection.py index b46f640..4b58269 100644 --- a/api/services/forge_projection.py +++ b/api/services/forge_projection.py @@ -350,11 +350,19 @@ def diff_against_hub( cur = have_wp.get(w.record_id.strip().lower()) if cur: hub_rows = hub_tasks_by_workplan.get(str(cur["id"]), []) - # Tasks carry no canonical id on the hub, only a title, so compare on - # title. Imperfect, and the reason task removal needs the same explicit - # acknowledgement as everything else. - have = {(t.get("title") or "").strip().lower(): t for t in hub_rows} - want = {(t.title or t.record_id).strip().lower(): t for t in w.tasks} + # Match on the canonical record id where the hub has one. Rows created + # before STATE-WP-0083-T06 fall back to title, which is why those are + # reported rather than acted on: a renamed heading is indistinguishable + # from a replaced task under title matching. + def _task_key(record_id: str | None, title: str | None) -> str: + if record_id: + return record_id.strip().lower() + return "title:" + (title or "").strip().lower() + + have = { + _task_key(t.get("record_id"), t.get("title")): t for t in hub_rows + } + want = {_task_key(t.record_id, t.title): t for t in w.tasks} for key, t in want.items(): if key not in have: d.missing.append( @@ -530,6 +538,7 @@ async def reset_repository_projection( Task( id=uuid.UUID(t.uuid), workplan_id=row.id, + record_id=t.record_id, title=t.title or t.record_id, status=t.status or "todo", priority=t.priority or "medium", diff --git a/api/services/task_record_id_backfill.py b/api/services/task_record_id_backfill.py new file mode 100644 index 0000000..f37aecb --- /dev/null +++ b/api/services/task_record_id_backfill.py @@ -0,0 +1,116 @@ +"""Backfill canonical record ids onto existing task rows (STATE-WP-0083-T06). + +Only the repository files hold the mapping. A file task declares both its +canonical id and the projection UUID it was registered under: + + ```task + id: CUST-WP-0067-T01 + state_hub_task_id: "f3608db4-..." + ``` + +so the pairing can be read directly rather than guessed from titles. Anything a +file does not claim is left alone: a task row whose canonical id cannot be +established keeps `record_id` null, and the reset continues to refuse to act on +it. An unknown identity must stay unknown rather than be inferred. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +_TASK_BLOCK_RE = re.compile(r"```task\s*\n(.*?)\n```", re.DOTALL) +_ID_RE = re.compile(r"^id:\s*(\S+)", re.MULTILINE) +_UUID_RE = re.compile(r'state_hub_task_id:\s*"?([0-9a-f-]{36})"?') + + +@dataclass +class BackfillReport: + scanned_files: int = 0 + pairs_found: int = 0 + updated: int = 0 + already_set: int = 0 + conflicts: list[dict[str, str]] = field(default_factory=list) + unmatched_uuids: int = 0 + + def to_dict(self) -> dict[str, Any]: + return { + "schema": "state-hub.task-record-id-backfill.v1", + "scanned_files": self.scanned_files, + "pairs_found": self.pairs_found, + "updated": self.updated, + "already_set": self.already_set, + "unmatched_uuids": self.unmatched_uuids, + "conflicts": self.conflicts, + } + + +def collect_pairs(roots: list[Path]) -> tuple[dict[str, str], BackfillReport]: + """Map projection UUID -> canonical record id, from workplan files.""" + report = BackfillReport() + pairs: dict[str, str] = {} + for root in roots: + wp_dir = root / "workplans" + if not wp_dir.is_dir(): + continue + for path in sorted(wp_dir.rglob("*.md")): + if path.name.startswith("."): + continue + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + continue + report.scanned_files += 1 + for block in _TASK_BLOCK_RE.finditer(text): + body = block.group(1) + rid = _ID_RE.search(body) + uid = _UUID_RE.search(body) + if not rid or not uid: + continue + record_id, task_uuid = rid.group(1).strip(), uid.group(1) + prior = pairs.get(task_uuid) + if prior and prior != record_id: + # One UUID claimed by two canonical ids: a duplicate + # registration. Recording it and skipping is the only safe + # option — picking one would fabricate an identity. + report.conflicts.append( + {"uuid": task_uuid, "first": prior, "second": record_id} + ) + continue + pairs[task_uuid] = record_id + report.pairs_found = len(pairs) + return pairs, report + + +async def backfill_task_record_ids( + session: Any, roots: list[Path], *, dry_run: bool = True +) -> BackfillReport: + from sqlalchemy import select + + from api.models.task import Task + + pairs, report = collect_pairs(roots) + if not pairs: + return report + + rows = list((await session.execute(select(Task))).scalars()) + by_id = {str(r.id): r for r in rows} + for task_uuid, record_id in pairs.items(): + row = by_id.get(task_uuid) + if row is None: + report.unmatched_uuids += 1 + continue + if row.record_id == record_id: + report.already_set += 1 + continue + if row.record_id and row.record_id != record_id: + report.conflicts.append( + {"uuid": task_uuid, "first": row.record_id, "second": record_id} + ) + continue + report.updated += 1 + if not dry_run: + row.record_id = record_id + return report diff --git a/migrations/versions/e2b3c4d5f6a7_task_canonical_record_id.py b/migrations/versions/e2b3c4d5f6a7_task_canonical_record_id.py new file mode 100644 index 0000000..ce983a3 --- /dev/null +++ b/migrations/versions/e2b3c4d5f6a7_task_canonical_record_id.py @@ -0,0 +1,36 @@ +"""task canonical record id (STATE-WP-0083-T06) + +Every work-record type carries a stable canonical identifier except tasks. A +task row holds `id, workplan_id, title, status, priority` — nothing connecting it +to `CUST-WP-0067-T01` in the file it came from. + +The consequence is that a file task and a hub task can only be matched by title, +so a renamed heading looks like one task disappearing and another appearing. That +is why the forge-derived reset deliberately refuses to touch tasks of existing +workplans: destroying and recreating a record because someone edited a heading is +not an acceptable failure mode. + +Nullable on purpose. Existing rows have no canonical id and cannot be given one +by this migration — only the repository files know the mapping, and backfilling +from them is a separate, reversible step. + +Revision ID: e2b3c4d5f6a7 +Revises: d1a2b3c4e5f6 +""" +from alembic import op +import sqlalchemy as sa + +revision = "e2b3c4d5f6a7" +down_revision = "d1a2b3c4e5f6" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("tasks", sa.Column("record_id", sa.String(length=120), nullable=True)) + op.create_index("ix_tasks_record_id", "tasks", ["record_id"]) + + +def downgrade() -> None: + op.drop_index("ix_tasks_record_id", table_name="tasks") + op.drop_column("tasks", "record_id") diff --git a/tests/test_task_record_id_backfill.py b/tests/test_task_record_id_backfill.py new file mode 100644 index 0000000..9eff772 --- /dev/null +++ b/tests/test_task_record_id_backfill.py @@ -0,0 +1,123 @@ +"""Recovering canonical identity for existing task rows (STATE-WP-0083-T06). + +The mapping exists only in the repository files, where a task declares both its +canonical id and the projection UUID it was registered under. Reading the pairing +is safe; inferring it from titles is not, which is the whole reason this exists. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from api.services import task_record_id_backfill as bf + + +def _repo(tmp_path: Path, name: str, body: str) -> Path: + root = tmp_path / name + (root / "workplans").mkdir(parents=True) + (root / "workplans" / "WP.md").write_text(body, encoding="utf-8") + return root + + +def test_reads_the_pairing_a_file_declares(tmp_path): + root = _repo(tmp_path, "a", ( + "---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n" + '```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n' + )) + pairs, rep = bf.collect_pairs([root]) + assert pairs == {"11111111-1111-5111-8111-111111111111": "DEMO-WP-0001-T01"} + assert rep.pairs_found == 1 + + +def test_task_without_a_projection_uuid_is_skipped(tmp_path): + """No pairing to read means no identity to assign.""" + root = _repo(tmp_path, "b", ( + "---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n" + "```task\nid: DEMO-WP-0001-T01\nstatus: todo\n```\n" + )) + pairs, _ = bf.collect_pairs([root]) + assert pairs == {} + + +def test_one_uuid_claimed_twice_is_a_conflict_not_a_guess(tmp_path): + """Duplicate registration: picking a winner would fabricate an identity.""" + dup = "22222222-2222-5222-8222-222222222222" + a = _repo(tmp_path, "a", ( + "---\nid: A-WP-0001\ntype: workplan\n---\n\n" + f'```task\nid: A-WP-0001-T01\nstate_hub_task_id: "{dup}"\n```\n' + )) + b = _repo(tmp_path, "b", ( + "---\nid: B-WP-0001\ntype: workplan\n---\n\n" + f'```task\nid: B-WP-0001-T01\nstate_hub_task_id: "{dup}"\n```\n' + )) + pairs, rep = bf.collect_pairs([a, b]) + assert len(rep.conflicts) == 1 + assert pairs[dup] == "A-WP-0001-T01" # first wins, second recorded not applied + + +class _Row: + def __init__(self, tid, record_id=None): + self.id = tid + self.record_id = record_id + + +class _Session: + def __init__(self, rows): + self.rows = rows + + async def execute(self, *_a, **_k): + rows = self.rows + + class R: + def scalars(self_inner): + return iter(rows) + + return R() + + +@pytest.mark.asyncio +async def test_dry_run_changes_nothing(tmp_path): + root = _repo(tmp_path, "a", ( + "---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n" + '```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n' + )) + row = _Row("11111111-1111-5111-8111-111111111111") + rep = await bf.backfill_task_record_ids(_Session([row]), [root], dry_run=True) + assert rep.updated == 1 and row.record_id is None + + +@pytest.mark.asyncio +async def test_applies_the_identity_when_not_dry_run(tmp_path): + root = _repo(tmp_path, "a", ( + "---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n" + '```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n' + )) + row = _Row("11111111-1111-5111-8111-111111111111") + rep = await bf.backfill_task_record_ids(_Session([row]), [root], dry_run=False) + assert rep.updated == 1 and row.record_id == "DEMO-WP-0001-T01" + + +@pytest.mark.asyncio +async def test_a_row_the_files_do_not_claim_keeps_no_identity(tmp_path): + """Unknown must stay unknown; the reset then keeps refusing to act on it.""" + root = _repo(tmp_path, "a", ( + "---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n" + '```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n' + )) + orphan = _Row("99999999-9999-4999-8999-999999999999") + await bf.backfill_task_record_ids(_Session([orphan]), [root], dry_run=False) + assert orphan.record_id is None + + +@pytest.mark.asyncio +async def test_existing_identity_is_never_overwritten(tmp_path): + root = _repo(tmp_path, "a", ( + "---\nid: DEMO-WP-0001\ntype: workplan\n---\n\n" + '```task\nid: DEMO-WP-0001-T01\nstate_hub_task_id: "11111111-1111-5111-8111-111111111111"\n```\n' + )) + row = _Row("11111111-1111-5111-8111-111111111111", record_id="OTHER-WP-0001-T09") + rep = await bf.backfill_task_record_ids(_Session([row]), [root], dry_run=False) + assert row.record_id == "OTHER-WP-0001-T09" + assert rep.conflicts and rep.updated == 0 diff --git a/workplans/STATE-WP-0083-forge-derived-projection-reset.md b/workplans/STATE-WP-0083-forge-derived-projection-reset.md index c402d66..c835460 100644 --- a/workplans/STATE-WP-0083-forge-derived-projection-reset.md +++ b/workplans/STATE-WP-0083-forge-derived-projection-reset.md @@ -219,7 +219,7 @@ filesystem. ```task id: STATE-WP-0083-T06 -status: todo +status: progress priority: high ``` @@ -239,6 +239,35 @@ Acceptance: task rows carry their canonical record id; the task diff for `whitehat-security` reports clean; `the-custodian`'s task diff falls to something manual inspection confirms. +**Built (2026-08-26).** Migration `e2b3c4d5f6a7` adds `tasks.record_id`, +nullable because no migration can invent an identity for an existing row — only +the repository files hold the mapping. + +The backfill *reads* that mapping rather than inferring it: a file task declares +both its canonical id and the projection UUID it was registered under, so the +pairing is stated, not guessed. Across 121 repositories and 1104 files it +recovered **5516 pairs with zero conflicts**, and identified **4456 of 6073** +task rows in the cache database. + +Diff and reset now key on `record_id` where present, falling back to a +`title:`-prefixed key so an unidentified row is *visibly* unidentified rather +than silently title-matched. + +Two refusals are tested rather than documented: a row the files do not claim +keeps no identity, and a row that already has one is never overwritten — a +mismatch is recorded as a conflict, not resolved. Title matching would have +"worked" and destroyed a record every time someone edited a heading. + +**Remaining, in order.** Deploy the migration to central; then run the backfill +against central as an explicit operation — deliberately *not* inside the Helm +hook, where a partial failure would silently leave half the tasks identified. +The cache matched 4456 of 6073 and central has a different history, so the +numbers will differ and a dry run should be compared before applying. + +Only then may `T03` be extended to tasks of existing workplans, which is a +further change with its own verification. `CUST-WP-0068-T09` is therefore two +moves away, not one. + ## Restore a migration mechanism for central