Skip to content

Retention routes: forget, purge, retention-class (CL-6288) - #36

Merged
TheGreatAxios merged 16 commits into
mainfrom
cl-6288-retention-routes
Aug 19, 2026
Merged

Retention routes: forget, purge, retention-class (CL-6288)#36
TheGreatAxios merged 16 commits into
mainfrom
cl-6288-retention-routes

Conversation

@TheGreatAxios

@TheGreatAxios TheGreatAxios commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

tombstoneDocument, hardDeleteDocument, and setRetentionClass were
exported, tested, and completely unreachable over HTTP — registerMemoryRoutes
only mounted add/search/list/feed. This closes CL-6288.

Stacked on #35 (CL-6286, machine-caller identity resolution) — target branch
is cl-6286-machine-auth, not main. Rebased onto its current tip
(bb0a31c); src/routes/deps.test.ts shows zero diff from that tip, and
src/routes/deps.ts shows only the intentional NonBlankId-sharing refactor
from review item 4 below (no behavior change — all of #35's own tests pass
unmodified).

Route surface

Route Grant action Plane verb
POST /api/tenants/:tenantId/memory/documents/:documentId/forget memory:forget tombstoneDocument
POST /api/tenants/:tenantId/memory/documents/:documentId/purge memory:purge hardDeleteDocument
POST /api/tenants/:tenantId/memory/versions/:versionId/retention-class memory:forget setRetentionClass

All three run behind resolveCaller → requirePrincipal → grantGuard —
identical guard chain to add/search/list, so a machine caller from
CL-6286's callerResolver seam works here too (now covered explicitly, see
review item 2).

Tombstone vs. hard delete stay distinct — and forget is NOT reversible

Separate routes, separate grant actions, no boolean flag. forget (tombstone)
stops a document appearing in search/feed and overwrites its chunk text with
[redacted] — there is no history table and no un-tombstone verb, so the
original content does not survive; only version metadata remains for audit.
purge (hard delete) removes the row entirely and is refused while a
durable-class version is untombstoned — that one really is irreversible. An
earlier draft of this PR called forget "reversible in principle" — that was
wrong and has been corrected everywhere it appeared (route summaries,
docs/RETENTION.md).

Grant design — two authorization mechanisms, not one

Added memory:forget / memory:purge to MEMORY_GRANT_REQUIREMENTS.
memory:search never authorizes any retention write — tested explicitly.

Review correction: the requirement shape originally carried
source: "creator" sitting next to source: "tenant", which read like an
enforcement switch the grant store resolves. It is not — nothing reads that
field. Renamed it to installHint (type MemoryGrantInstallHint) with a
comment stating plainly it is advisory-only sizing metadata for install
tooling, and added a test asserting the requirement shape carries no
enforcement field. ARCHITECTURE.md's Boundaries section now states the split
explicitly: grant tags decide capability + visibility (a share grant can
legitimately widen who can search/see a document); a separate imperative
ownership check (services/retention-ownership.ts, wired into
memory.ts) is the sole source of truth for who may forget/purge a specific
document, and a share grant never satisfies it.

Also fixed a latent surface leak: the resident distiller was spreading
every registered capability id into its agent definition, which would have
silently handed it memory:forget/memory:purge. Added
capabilityIdsForSurface(surface) and switched the distiller to
capabilityIdsForSurface("distiller").

Ownership — the security property of this ticket

New src/services/retention-ownership.ts resolves the actual creator
(created_by_principal_id — a document's first version for forget/purge, a
version's own row for retention-class) independent of accessTags, and the
plane wiring in memory.ts refuses the call (403) for anyone but that
creator, 404 for an unknown document/version, before ever calling into
services/retention.ts.

  • src/routes/routes.test.ts has a document owned by "alice" that PRINCIPAL
    can be granted forget/purge for (capability) but is refused on
    (ownership) — the mirror of CL-6286's cross-tenant test. Tests assert the
    destructive call did not happen, not just the status code.
  • Review addition: the most likely real-world caller of forget/purge is a
    workflow-run child resolved through callerResolver, retiring memory it
    wrote itself — previously untested. Added coverage for a resolved machine
    caller succeeding on its own document/version and still being refused on
    one it doesn't own.
  • Review addition: the ownership-gate unit tests never varied tenantId.
    Added a scoped fake sql that filters by both bound values and asserts
    exactly two are bound (tenantId, id) — verified by temporarily stripping
    the tenant_id predicate from the real query and confirming the new tests
    go red for that reason, then restoring it.

sweepEphemeral: no route

Left off the HTTP surface — a tenant-wide TTL sweep with no principalId
and no natural ownership check. A host schedules its own cron calling
memory.sweepEphemeral({ tenantId }) in-process. Documented in
docs/RETENTION.md, which also had a pre-existing inaccuracy (fixed in this
PR since already editing the file): the ephemeral sweeper only ever
deprecates past valid_until, it never hard-deletes.

Other review fixes

  • DocumentIdParam/VersionIdParam used "string >= 1" — a length
    constraint that lets " " through — for the same bug Machine-auth: caller resolver for workflow-run children (CL-6286) #35 had already hit
    and fixed for the resolved-caller boundary (NonBlankId). Extracted
    NonBlankId to core/schemas/non-blank-id.ts (shared by both deps.ts
    and http-bodies.ts now) and reused it for the path params. Added
    red-then-green tests for whitespace-only documentId/versionId.
  • IMPLEMENTATION.md's route table and "register the three HTTP routes"
    sentence predated feed and this PR's three routes — the package mounts
    seven. Fixed.
  • A fake-sql branch in src/memory.test.ts looked like it distinguished the
    document-owner vs version-owner query but both arms returned an identical
    row. Dropped the dead branch.

Test plan

  • bun run typecheck — clean
  • bun test ./src — 465 pass, 0 fail (410 pre-CL-6288 baseline; +18 from
    CL-6286's rebased commits; +37 from this PR across both review rounds)
  • Grant-guard coverage: memory:search/no-grant is 403 on forget/purge/retention-class
  • Ownership coverage: creator succeeds, non-creator (even with the
    capability grant) is 403, unknown id is 404 — status code AND that the
    destructive call never happened
  • Cross-tenant coverage on the ownership gate itself (not just inferred
    from the destructive queries)
  • Machine-caller (callerResolver) coverage: resolved run succeeds on its
    own document/version, refused on one it doesn't own
  • Forget/purge distinctness: calling forget never hard-deletes
  • Invalid retention_class value and whitespace-only path params are 400
  • Rebased onto cl-6286-machine-auth@bb0a31c; src/routes/deps.test.ts
    diff against that tip is empty; src/routes/deps.ts diff is only the
    NonBlankId-sharing refactor

@TheGreatAxios
TheGreatAxios force-pushed the cl-6288-retention-routes branch 2 times, most recently from 2179b31 to f1b9881 Compare August 19, 2026 00:32
@TheGreatAxios
Add tests for memory:forget/purge grant requirements (CL-6288)
54fbd52 
Retention routes need capability grants distinct from search, and a
distiller/tools install must not inherit routes-only capabilities. Assert
the forget/purge shape and a surface-filtered capability id helper ahead
of adding either.
@TheGreatAxios
Grant requirements: add memory:forget/purge; scope distiller capabili…
38346ae 
…ties by surface (CL-6288)

Retention writes get their own capability grants (source: "creator", pairing
with the ownership check the routes enforce) instead of riding on
memory:search. Add capabilityIdsForSurface() and use it in the resident
distiller so a routes-only capability never leaks into its agent
definition — a bug the previous unconditional MEMORY_CAPABILITY_IDS spread
would have introduced the moment forget/purge existed.
@TheGreatAxios
Add tests for the retention ownership gate (CL-6288)
e1daf20 
canAccessDocument (grant-tags.ts) answers "can this principal see the
document" — including via a share grant — which is the wrong question for
forget/purge/retention-class. Exercise the creator-lookup helpers those
routes will use, against a mocked raw sql client (same pattern as the
search ACL post-filter), ahead of adding the module.
@TheGreatAxios
Add the retention ownership gate (CL-6288)
62c47f5 
resolveDocumentOwner / resolveVersionOwner load a document's or version's
creator directly (created_by_principal_id — first version for a document,
own row for a version), independent of accessTags. isOwner() is the single
place "may this caller forget/purge this" gets decided, so the memory.ts
plane wiring and the HTTP routes both call through it rather than
re-deriving ownership.
@TheGreatAxios
Add tests for the plane's retention ownership wiring (CL-6288)
3c9fa1a 
tombstoneDocument/hardDeleteDocument/setRetentionClass must refuse a
non-creator caller with 403, and an unknown document/version with 404,
before ever touching services/retention.ts — a share grant that lets a
peer search a document must not let them forget or purge it. Mock the raw
sql client and services/retention.ts the same way the existing search ACL
tests do.
@TheGreatAxios
Plane: gate tombstone/hard-delete/set-retention-class on document own…
be868cf 
…ership (CL-6288)

Memory.tombstoneDocument / hardDeleteDocument / setRetentionClass now take
the caller's principalId and check it against resolveDocumentOwner /
resolveVersionOwner before delegating to services/retention.ts — 404 for
an unknown document/version, 403 for anyone but its creator. deprecateVersion
and sweepEphemeral are unchanged (not HTTP-routed; see docs/RETENTION.md).
@TheGreatAxios
Add tests for the retention HTTP routes (CL-6288)
168306d 
Extend the stub plane with the same creator/documentId fixtures the routes
will call through: grant-guard coverage for forget/purge/retention-class
(memory:search must not authorize any of them), the cross-principal
ownership refusal (mirrors CL-6286's cross-tenant test — a share grant
that lets a peer see a document must not let them forget or purge it),
404s for unknown documents/versions, and that forget and purge stay
distinct routes rather than one call with a boolean flag.
@TheGreatAxios
Mount retention routes: forget, purge, retention-class (CL-6288)
dbec4c0 
POST …/memory/documents/:documentId/forget (grant memory:forget) tombstones;
POST …/memory/documents/:documentId/purge (grant memory:purge) hard-deletes;
POST …/memory/versions/:versionId/retention-class (grant memory:forget) sets
retention class. Separate routes and separate grant actions for tombstone vs
hard delete — never a single route toggled by a boolean a client could get
wrong. Path/body validated with arktype (DocumentIdParam, VersionIdParam,
ForgetRequest, SetRetentionClassRequest); ownership refusal from the plane
surfaces as 403, an unknown document/version as 404.

sweepEphemeral gets no route — it is a tenant-wide maintenance sweep, not a
per-caller action, and has no natural ownership check; a host schedules it
on its own cron against the in-process Memory (docs/RETENTION.md).
@TheGreatAxios
Update docs: retention HTTP routes (CL-6288)
d19df1f 
AGENTS.md/ARCHITECTURE.md route lists, docs/RETENTION.md (route table,
grant actions, ownership gate, sweepEphemeral decision), and CHANGELOG.
@TheGreatAxios
Fix: tombstone is not reversible; stop claiming otherwise (CL-6288)
4d8bf1e 
tombstoneDocument overwrites chunk.text with '[redacted]' — there is no
history/audit table and no un-tombstone verb, so the original content does
not survive a forget request; only version metadata does. The route summary
and docs/RETENTION.md previously said "reversible in principle," which would
lead a host to build an "undo forget" button with nothing to undo to.
Describe what forget actually does (stops appearing in search, content
redacted, row kept for audit) instead. purge remains genuinely irreversible
(the row itself is removed) — that claim was already correct.

Drive-by: docs/RETENTION.md said the ephemeral sweeper "hard-deletes" past
valid_until; sweepEphemeral only sets status=deprecated and never deletes.
Pre-existing inaccuracy, fixed while already editing this file.
@TheGreatAxios
Rename grant "source" to "installHint"; document the two authorizatio…
39ee05b 
…n mechanisms (CL-6288 review)

`source: "creator"` sat next to `source: "tenant"` and read like an
enforcement switch the grant store resolves — it is not. Nothing reads this
field; it is advisory sizing metadata for install tooling. Renamed the field
to `installHint` and the type to `MemoryGrantInstallHint`, with a doc comment
that says plainly this is not enforced and points at
services/retention-ownership.ts as the actual mechanism. Added a test
asserting the requirement shape carries no enforcement field, so a future
tightening of grant-requirements.ts trips on this comment rather than
assuming installHint is load-bearing.

ARCHITECTURE.md's Boundaries section previously described one authorization
model ("document access is grant tags + creator"). The library now runs two:
grant tags for capability + visibility, and a separate imperative ownership
check for forget/purge. Stated the split plainly, including which mechanism
is the source of truth for "whose document is it" (ownership — never grant
tags; a share grant never satisfies it).
@TheGreatAxios
Add tests for whitespace-only retention path params (CL-6288 review)
0733cfd 
DocumentIdParam/VersionIdParam use "string >= 1" — a length constraint, not
a content one, so " " (length 1) passes and reaches the plane instead of
being rejected as invalid input. #35 already hit and fixed this exact bug
for the resolved-caller trust boundary (NonBlankId, routes/deps.ts); it
regrew here because the fix lived in a comment instead of a shared schema.
Extract NonBlankId to core/schemas/non-blank-id.ts (deps.ts now imports it
too, unchanged behavior) ahead of reusing it for the path params.
@TheGreatAxios
Reuse NonBlankId for retention path params (CL-6288 review)
3fa7d60 
DocumentIdParam/VersionIdParam now validate with the shared NonBlankId
schema instead of "string >= 1", so a whitespace-only documentId/versionId
is rejected at the trust boundary (400) instead of reaching the plane as a
normal-looking (nonexistent) id. routes/deps.ts imports the same schema for
the resolved-caller boundary it originally shipped with — behavior there is
unchanged (src/routes/deps.test.ts has zero diff against origin's tip).
@TheGreatAxios
Add cross-tenant tests for the ownership gate (CL-6288 review)
bcd1c03 
resolveDocumentOwner/resolveVersionOwner tests never varied tenantId, so
tenant scoping was inferred from the destructive queries rather than proven
on the ownership lookup itself. Add a scoped fake sql that filters by both
bound values and asserts exactly two are bound (tenantId, id) — a dropped
tenant_id predicate fails loudly here instead of silently returning
whatever a coincidental positional match produces. Verified by temporarily
stripping the tenant_id filter from the real query and confirming these
tests go red for that reason, then restoring it.
@TheGreatAxios
Add tests for machine-caller forget/purge on its own memory (CL-6288 …
2c4d626 
…review)

The most likely real-world caller of forget/purge is a workflow-run child
resolved through #35's callerResolver, retiring memory it wrote itself. If a
resolver's principalId ever drifted from created_by_principal_id (different
derivation, casing, run-address vs principal-address), every automated
retention call would 403 in production with nothing catching it first.
Extend stubMachinePlane with the same creator-check fixtures stubPlane uses
and cover: forget/purge/retention-class succeeding for the resolved run's
own document/version, and still refused for one it does not own.
@TheGreatAxios
Fix stale route table and a decorative test branch (CL-6288 review)
96d4a70 
IMPLEMENTATION.md's route table and "register the three HTTP routes"
sentence predated feed and now this PR's three retention routes — the
package mounts seven. Added feed, forget, purge, and retention-class rows
and corrected the sentence; also listed the optional retention plane
methods alongside the existing transform ones.

src/memory.test.ts's ownership-wiring fake sql branched on
`text.includes("document_id")` with both arms returning an identical row —
it read as distinguishing the document-owner and version-owner queries but
did not. Dropped the branch; a comment explains real per-query scoping is
covered separately in retention-ownership.test.ts.
@TheGreatAxios
TheGreatAxios force-pushed the cl-6288-retention-routes branch from f1b9881 to 96d4a70 Compare August 19, 2026 00:42
@TheGreatAxios
TheGreatAxios changed the base branch from cl-6286-machine-auth to main August 19, 2026 00:42
@TheGreatAxios
TheGreatAxios merged commit 7fcbd00 into main Aug 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@TheGreatAxios