feat: memory revoke, delete-on-uninstall, and citation — completes #6#56
Merged
Conversation
MemoryPolicy gains a denied list of revoked key prefixes; validate_memory_used refuses any read or write matching one — the adapter's own guarantee, not the runtime's cooperation — and the list serializes into the brief so the runtime can stop surfacing them. The store gains a memory_revocations table with record/query and delete_memory_for_installation (delete-on-uninstall); the worker loads a repo's revocations into the policy each run. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
…#6) The marker-backed status comment now discloses which memory entries the review read and the adapter accepted (revoked/refused reads are excluded), satisfying the contract's requirement that reviews cite the memory that influenced them. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
) POST /api/github/memory/revoke records a revocation behind the #3 tenant boundary — a tenant may only revoke within its own installation and repos, the service token may name any installation, every call audited. An installation 'deleted' webhook purges that tenant's memory activity and revocations (delete-on-uninstall), idempotent via the delivery id. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
Records that the adapter side of the memory governance contract is done — policy/enforcement, inspect, revoke, delete-on-uninstall, citation — with retention expiry (a periodic sweep) as the remaining operational follow-up and the runtime-side denial-list honoring as the coven-code counterpart. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the adapter-side completion of the hosted memory governance contract (issue #6) by adding revocation enforcement and persistence, delete-on-uninstall purging, and review-surface citation of accepted memory reads.
Changes:
- Add a revocation denial-list (
denied) toMemoryPolicyand refuse revoked reads/writes during result validation. - Persist revocations in the store (schema v5), expose a tenant-scoped
POST /api/github/memory/revoke, and purge memory data oninstallation.deletedwebhooks. - Cite adapter-accepted memory reads on the marker-backed status comment for transparency.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/memory-contract.md | Updates the phase plan to reflect completion of inspect/revoke/citation and notes retention expiry follow-up. |
| docs/contracts/session-brief.schema.json | Extends the session brief schema with memory_policy.denied for runtime denial-list propagation. |
| crates/worker/src/memory.rs | Adds denied to policy and enforces revocations during validate_memory_used. |
| crates/worker/src/lib.rs | Loads revocations into policy, computes cited memory reads, and appends citations to the final status surface. |
| crates/webhook/src/routes.rs | Adds POST /api/github/memory/revoke and purges memory on uninstall (installation.deleted). |
| crates/store/src/lib.rs | Bumps schema to v5 and adds memory_revocations persistence + uninstall purge helper. |
| crates/server/src/main.rs | Wires the new revoke route into the server router. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+155
to
+157
| fn is_revoked(policy: &MemoryPolicy, target: &str) -> bool { | ||
| policy.denied.iter().any(|prefix| target.starts_with(prefix)) | ||
| } |
Comment on lines
+763
to
+767
| .revocations_for(task.installation_id, &repo_full) | ||
| .await | ||
| .unwrap_or_else(|e| { | ||
| warn!(task_id = %task.id, "failed to load memory revocations: {e:#}"); | ||
| Vec::new() |
Comment on lines
+1366
to
1377
| // Disclose which memory entries influenced this review (issue #6). | ||
| if cited_memory.is_empty() { | ||
| body | ||
| } else { | ||
| let cited = cited_memory | ||
| .iter() | ||
| .map(|id| format!("`{id}`")) | ||
| .collect::<Vec<_>>() | ||
| .join(", "); | ||
| format!("{body}\n\nMemory used: {cited}") | ||
| } | ||
| } |
Comment on lines
+244
to
+254
| // Operator-wide callers must name the installation explicitly. | ||
| ApiCaller::Service | ApiCaller::Open => match req.installation_id { | ||
| Some(id) => ("service".to_string(), id), | ||
| None => { | ||
| return ( | ||
| StatusCode::BAD_REQUEST, | ||
| Json(json!({"ok": false, "error": "installation_id required"})), | ||
| ) | ||
| .into_response() | ||
| } | ||
| }, |
Comment on lines
+257
to
+261
| match state | ||
| .store | ||
| .record_revocation(installation_id, &req.repo, &req.key) | ||
| .await | ||
| { |
Comment on lines
+806
to
+816
| r#" | ||
| CREATE TABLE memory_revocations ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| at TEXT NOT NULL, | ||
| installation_id INTEGER NOT NULL, | ||
| repo TEXT NOT NULL, | ||
| target TEXT NOT NULL | ||
| ); | ||
| CREATE INDEX memory_revocations_scope | ||
| ON memory_revocations(installation_id, repo); | ||
| "#, |
Comment on lines
+580
to
+582
| "INSERT INTO memory_revocations (at, installation_id, repo, target) | ||
| VALUES (?1, ?2, ?3, ?4)", | ||
| params![now_rfc3339(), installation_id, repo, target], |
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
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.
Completes issue #6 (adapter side): revoke + citation — the last acceptance criteria — plus delete-on-uninstall. Builds on the merged inspect (#54), enforcement (#50), and fork hardening (#52).
What this delivers
memory.rs):MemoryPolicycarries adeniedlist of revoked key prefixes;validate_memory_usedrefuses any read or write matching one. This is the adapter's own guarantee — a revoked memory can never influence a review regardless of what the runtime does — and the list also serializes into the brief so the runtime can stop surfacing it.memory_revocationstable (schema v5) withrecord/query;POST /api/github/memory/revokebehind the Add tenant-scoped task API authentication for CovenCave and hosted clients #3 tenant boundary — a tenant may only revoke within its own installation and repos, the service token may name any installation, every call audited.installationdeletedwebhook purges that tenant's memory activity and revocations (idempotent via the delivery id).#6 acceptance criteria — now all met
explicit policy per invocation ✅ · fail-closed on missing scope ✅ · fork/untrusted content can't write ✅ · reviews cite influencing memory ✅ (this PR) · customers can inspect and revoke by installation/repo ✅ (#54 + this PR) · cross-install/cross-repo/fork/disabled tests ✅.
The one honest boundary
Retention expiry (time-based deletion) needs a periodic sweep/scheduler — an operational enhancement, not a contract requirement — and is a noted follow-up. And the inherently bilateral parts (the runtime honoring the
deniedlist; physically deleting revoked bytes in coven-code) are the coven-code counterpart — but the adapter's refusal stands alone, so revocation is a real guarantee here today.Closes #6.
Local gates:
cargo check --all-targets+cargo clippy --all-targets -- -D warnings+cargo test --all(198 passed, 0 failed).🤖 Generated with Claude Code