fix(memory): close Codex Wave 1 audit conditions (auto_triage + supersede guard)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user