feat: AT1 rejected-value tombstone (migration v41, guard, reject verbs) - #142
Merged
Conversation
…reject verbs) A human who rejects a value now gets a durable say: a digest-keyed rejected_values tombstone (per store, per tenant) refuses byte-stable re-assertion of that value across every write surface. - Migration v41: rejected_values (PK tenant_id+digest, WITHOUT ROWID, no FK, no content stored - reason is the identity; no min_compatible_binary bump, tradeoff documented in MEMORY_ENVELOPE.md). - Guard at the single INSERT choke point (upsertEntryRow), content- introduction semantics: refuses new rows AND same-id edits TO a rejected value; unchanged re-persists exempt. Sole bypass: batchWriteAndDelete (consolidation paraphrase rollups). Recovery paths (bootstrapLegacyStore, rebuildIndex) run the guard with per-row skip, closing mirror-resurrection structurally. - Refusal audit (reject_refusal) written post-rollback by the transaction owners (writeEntry, api.supersede). - hippo reject <id>|--value + rejections + unreject (required --reason); api.reject/unreject/listRejections; shared reject-flow.ts transaction + post-commit mirror purge reusing the raw_archive mirror_cleaned_at reaper. - resolveConflict: additive opts (rejectLoserValue), kind-aware loser removal (fixes pre-existing raw-loser crash), and the previously MISSING audit - every resolution now lands conflict_resolve. - deleteEntryCore split (db-scoped, composable); cmdSupersede reordered successor-first so a refusal cannot leave a dangling superseded_by pointer. - Containment: multi-item write loops (capture, importers, extract, learn, watch, dag, sync, connectors) skip-and-count refusals; single-item surfaces fail loud. Connector refusals ack idempotency permanently (no DLQ retry). - AuditOps reject_value/reject_refusal/unreject_value/conflict_resolve in 3-site lockstep. 24 historical schema-pin test assertions bumped 40->41. Tests: 25 new across guard/verbs/acceptance incl. the roadmap-verbatim acceptance case and the AT5 paired recall case. ROADMAP.md Part V AT1 [critical, next]. Plan: docs/plans/2026-08-15-at1-rejected-value-tombstone.md (3-round plan gate, 2-round code gate, trail in episode 01M025CW434ZAPVSFC61BGFGCT). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NUZVRXZRWfGiztx9JHYQt
… hardening) Review-stage fixes for PR #142 (2 P1, 6 P2, 3 low + 8 regression tests): - P1: applyRebuildResult wrote rebuilt summary content via direct UPDATE, bypassing the rejection guard; now tombstone-checked, refusal falls through to the metadata-only branch (dirty cleared so the DAG cannot loop on a doomed rebuild) + reject_refusal audit. - P1: resolveConflict now purges mirrors + stamps the reaper for EVERY removed raw loser, not only the reject path (pre-AT1 raw forget-loser crashed outright, so there was no legacy behavior to preserve). - Consolidation resurrection loop closed at the producer: merge assembly tombstone-checks mergeContents output before pushing to the batch writer; a hit skips the whole cluster (sources stay unmerged and undeleted). Bypass comments + plan doc corrected: merges are deterministic concatenation, not LLM paraphrase. - P2s: tenant-change on a same-id upsert now counts as content introduction; legacy bootstrap gated by a completion meta flag (an all-rejected mirror set no longer re-runs bootstrap every open); api.reject rejects ambiguous id+value; api.unreject rejects blank prefixes; dry-run imports probe tombstones so rejected counts are accurate without writing; MCP hippo_resolve passes the resolved actor. - Lows: JSDoc on the five new exports; deleteEntryCore docstring names both suppressForgetAudit callers. Tests: 33 rejection-suite tests green (8 new); dag/consolidate/importer suites 223/223; build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NUZVRXZRWfGiztx9JHYQt
…ners) Four sleep-pipeline corner interactions with the rejection tombstone: - Auto-promoted traces: producer-side tombstone check before the batch bypass (a rejected trace no longer regenerates every sleep); one consolidate-wide db handle now spans the auto-promote and merge passes. - Merge tombstone check now runs under the DESTINATION entry's tenant (createMemory stamps 'default' when tenantId is omitted; checking the source cluster's tenant missed default-tenant rejections and could falsely block). The pre-existing cross-tenant landing behavior itself is untouched and filed as a follow-up. - autoShare: per-candidate RejectedValueError containment (one rejected candidate no longer aborts the whole sleep phase); rejectedSkipped stat threaded parallel to secretSkipped through SleepResult. - Mirror purge honesty: shared purgeMirrorBestEffort helper (retry once; raw ids keep the true reaper message; non-raw ids print the leftover path with a delete-manually notice) wired into reject-flow and resolveConflict; MEMORY_ENVELOPE documents the no-reaper reality for non-raw mirrors. Tests: +4 acceptance cases (trace resurrection, destination-tenant both directions, autoShare containment); rejection suites 37/37; touched consolidate/dag/shared/api-sleep/trace suites all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NUZVRXZRWfGiztx9JHYQt
…ep, compat) - Trace-layer mirrors included in removeEntryMirrors + getExistingEntryMirrorPaths (rejected trace content no longer survives on disk; also closes the pre-existing plain-forget trace-mirror gap). - Batch rejection race closed at root: batchWriteAndDelete now point-probes tombstones INSIDE its transaction (same connection) before each bypassed upsert - a reject that lands mid-consolidate can no longer be undone by a queued re-persist; skips audited as sleep-batch and excluded from post-commit mirror writes. - resolveConflict rejectLoserValue now removes ALL same-tenant same-digest duplicates kind-aware (matching the reject verb), strictly tenant-scoped, with keepId excluded so the human's keep choice cannot be undone in the same transaction. - capture --dry-run probes tombstones read-only so previews match real runs; consolidate db handle try/finally now spans every phase; ImportResult.rejected made optional (published-surface compat). Tests: +4 (trace mirror purge, batch-race pin, tenant-scoped dup sweep, capture dry-run parity); one stale test asserting the old blind-bypass behavior rewritten to the corrected contract. 298/298 across 31 touched suites + resolve-conflict 14/14; build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NUZVRXZRWfGiztx9JHYQt
…EDIATE Codex delta-review P2s: the in-transaction batch probe now reuses checkRejectionGuard (content-introduction classification) so an unchanged same-id re-persist of a legitimately-live same-content row (keepId case) keeps its decay/replay updates; and batchWriteAndDelete opens with BEGIN IMMEDIATE so the probe cannot pin a stale WAL snapshot and fail the whole batch on a concurrent write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NUZVRXZRWfGiztx9JHYQt
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e486f5a450
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+125
to
+129
| const rows = db | ||
| .prepare(`SELECT id, kind, content FROM memories WHERE tenant_id = ?`) | ||
| .all(opts.tenantId) as Array<{ id: string; kind: string; content: string }>; | ||
| for (const row of rows) { | ||
| if (rejectionDigest(row.content) !== digest) continue; |
There was a problem hiding this comment.
--value from deleting live memories
When the pre-emptive opts.value form happens to match content already stored in the tenant, this unconditional scan removes every matching live row, including irreversibly archiving raw rows. That contradicts the documented zero-removal contract for reject --value and can cause unexpected data loss; only the memoryId form should perform the removal sweep.
AGENTS.md reference: AGENTS.md:L24-L29
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ROADMAP.md Part V AT1 [critical, next]: durable value-keyed suppression on human rejection. A digest-keyed, per-store/per-tenant
rejected_valuestombstone refuses byte-stable re-assertion of a rejected value across every write surface, with audited refusals and anunrejectescape hatch.Full design + 3-round plan-gate trail:
docs/plans/2026-08-15-at1-rejected-value-tombstone.md(dev-framework-rl episode 01M025CW434ZAPVSFC61BGFGCT).rejected_values, no content stored, no min_compatible_binary bump: tradeoff documented)hippo reject <id>|--value/rejections/unreject(+ api trio); post-commit mirror purge reusing the mirror_cleaned_at reaperresolveConflict: kind-aware loser removal (fixes pre-existing raw-loser crash) + previously-missing audit (conflict_resolveon every path) + opt-in--reject-losercmdSupersedereordered successor-first (no danglingsuperseded_byon refusal)Test plan
🤖 Generated with Claude Code
https://claude.ai/code/session_016NUZVRXZRWfGiztx9JHYQt