From 3a474f750c12c5de9837a555b482429f15339b1f Mon Sep 17 00:00:00 2001 From: Anto01 Date: Tue, 28 Apr 2026 21:53:39 -0400 Subject: [PATCH] fix(memory): close Codex Wave 1 audit conditions (auto_triage + supersede guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex's formal audit of fb4d55c said GO WITH CONDITIONS. Two P2 findings to fold in before merge: 1. auto_triage.py:417 still PUT {"content": cand["content"]} — the suggested-project correction was unreachable even with MemoryUpdateRequest.project in place. Changed body to {"project": suggested} so misattribution flags actually retarget the memory. Added a regression test that asserts the script source contains the new PUT shape, so a future "optimization" can't silently undo this. 2. POST /memory/{id}/supersede had no status guard — calling supersede_memory() delegated to update_memory(status="superseded"), which would silently flip a candidate to superseded. Mirrored the invalidate route: get_memory(id) lookup, 404 unknown / 200 already_superseded / 409 wrong-status / 200 superseded. Plus a P3 from the same audit: covered the "retarget to project='' when a global active duplicate exists" case via test_update_memory_to_empty_project_detects_global_duplicate. Tests: 581 -> 586 (+5: 3 supersede route + 1 project-empty duplicate + 1 auto_triage caller invariant). Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/auto_triage.py | 12 +++++++---- src/atocore/api/routes.py | 26 ++++++++++++++++++++---- tests/test_invalidate_supersede.py | 29 +++++++++++++++++++++++++++ tests/test_memory.py | 32 ++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 8 deletions(-) diff --git a/scripts/auto_triage.py b/scripts/auto_triage.py index 81665bb..6a12584 100644 --- a/scripts/auto_triage.py +++ b/scripts/auto_triage.py @@ -404,19 +404,23 @@ def process_candidate(cand, base_url, active_cache, state_cache, known_projects, known_projects, TIER1_MODEL, DEFAULT_TIMEOUT_S, ) - # Project misattribution fix: suggested_project surfaces from tier 1 + # Project misattribution fix: suggested_project surfaces from tier 1. + # Earlier code POSTed only {"content": cand["content"]}, which left + # the project field unchanged because MemoryUpdateRequest had no + # project key and the service signature didn't accept one. Wave 1 + # added project to MemoryUpdateRequest and update_memory(); this + # caller now actually applies the suggested project. suggested = (v1.get("suggested_project") or "").strip() if suggested and suggested != project and suggested in known_projects: - # Try to re-canonicalize the memory's project if not dry_run: try: import urllib.request as _ur req = _ur.Request( f"{base_url}/memory/{mid}", method="PUT", headers={"Content-Type": "application/json"}, - data=json.dumps({"content": cand["content"]}).encode("utf-8"), + data=json.dumps({"project": suggested}).encode("utf-8"), ) - _ur.urlopen(req, timeout=10).read() # triggers canonicalization via update + _ur.urlopen(req, timeout=10).read() except Exception: pass print(f" ↺ misattribution flagged: {project!r} → {suggested!r}") diff --git a/src/atocore/api/routes.py b/src/atocore/api/routes.py index 946c138..906d6b4 100644 --- a/src/atocore/api/routes.py +++ b/src/atocore/api/routes.py @@ -827,15 +827,33 @@ def api_supersede_memory( memory_id: str, req: MemorySupersedeRequest | None = None, ) -> dict: - """Supersede an active memory (Issue E — active → superseded).""" - from atocore.memory.service import supersede_memory + """Supersede an active memory (Issue E — active → superseded). + + Mirrors the invalidate route's status guard: candidates and other + non-active rows must not silently flip to superseded. + """ + from atocore.memory.service import get_memory, supersede_memory reason = req.reason if req else "" + target = get_memory(memory_id) + if target is None: + raise HTTPException(status_code=404, detail=f"Memory not found: {memory_id}") + if target.status == "superseded": + return {"status": "already_superseded", "id": memory_id} + if target.status != "active": + raise HTTPException( + status_code=409, + detail=( + f"Memory {memory_id} is {target.status}; " + "only active memories can be superseded" + ), + ) + success = supersede_memory(memory_id, actor="api-http", reason=reason) if not success: raise HTTPException( - status_code=404, - detail=f"Memory not found or not active: {memory_id}", + status_code=409, + detail=f"Memory {memory_id} could not be superseded", ) return {"status": "superseded", "id": memory_id} diff --git a/tests/test_invalidate_supersede.py b/tests/test_invalidate_supersede.py index 2c18117..9b2f853 100644 --- a/tests/test_invalidate_supersede.py +++ b/tests/test_invalidate_supersede.py @@ -249,6 +249,35 @@ def test_api_invalidate_unknown_id_is_404(env): assert r.status_code == 404 +def test_api_supersede_candidate_returns_409(env): + """Mirror of the invalidate guard: candidates must not silently flip + to superseded via the active-only supersede route.""" + m = create_memory( + memory_type="knowledge", content="candidate target", status="candidate" + ) + client = TestClient(app) + r = client.post(f"/memory/{m.id}/supersede", json={"reason": "wrong route"}) + assert r.status_code == 409 + # Row should still be a candidate + assert _get_memory(m.id).status == "candidate" + + +def test_api_supersede_already_superseded_is_idempotent(env): + m = create_memory(memory_type="knowledge", content="will be superseded") + client = TestClient(app) + r1 = client.post(f"/memory/{m.id}/supersede", json={"reason": "first"}) + assert r1.status_code == 200 + r2 = client.post(f"/memory/{m.id}/supersede", json={"reason": "again"}) + assert r2.status_code == 200 + assert r2.json()["status"] == "already_superseded" + + +def test_api_supersede_unknown_id_is_404(env): + client = TestClient(app) + r = client.post("/memory/no-such-id/supersede", json={"reason": "ghost"}) + assert r.status_code == 404 + + def test_admin_dashboard_active_count_matches_full_table(env): """/admin/dashboard memories.active must match the SQL aggregate even when there are more active memories than the legacy sample limit (500). diff --git a/tests/test_memory.py b/tests/test_memory.py index e72effd..f512990 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -661,3 +661,35 @@ def test_update_memory_project_unchanged_when_not_passed(isolated_db): mem = create_memory("knowledge", "untouched project", project="p06-polisher") update_memory(mem.id, content="edited content") assert get_memory(mem.id).project == "p06-polisher" + + +def test_update_memory_to_empty_project_detects_global_duplicate(isolated_db): + """Codex P3: when retargeting to project='' (global), the duplicate + check must scope to the new project. If a global active memory with + the same content already exists, the update must raise.""" + import pytest as _pytest + from atocore.memory.service import create_memory, update_memory + + create_memory("knowledge", "shared global fact", project="") + scoped = create_memory("knowledge", "shared global fact", project="p04-gigabit") + + with _pytest.raises(ValueError, match="duplicate active memory"): + update_memory(scoped.id, project="") + + +def test_auto_triage_suggested_project_put_body_uses_project_key(): + """Regression: the auto_triage caller used to PUT {"content": ...} + which silently dropped the suggested project change. The fix sends + {"project": suggested}. Inspect the script source so we don't have + to spin up a live triage run.""" + from pathlib import Path + + src = Path(__file__).resolve().parents[1] / "scripts" / "auto_triage.py" + text = src.read_text(encoding="utf-8") + # The block that PUTs to /memory/{mid} for a suggested_project fix + assert 'json.dumps({"project": suggested})' in text, ( + "auto_triage.py must PUT {\"project\": suggested} so the " + "suggested-project correction actually applies. See Wave 1." + ) + # And must not be back to the old shape + assert 'json.dumps({"content": cand["content"]})' not in text