feat: memory policy enforcement — #6 Phase 2 (adapter side, off by default)#50
Merged
Conversation
Adds the optional memory_used result field (enabled + read/proposed/rejected) and its schema, the receiving half of the memory governance contract: the adapter tolerates and can inspect runtime-reported memory activity. Optional and skip-serialized, so memory-free results are unchanged and the golden fixtures still round-trip. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
New memory module implementing the contract's enforcement core: compute_policy is deny-by-default and trust-gated (fork/external never get a write scope), and validate_memory_used re-checks every runtime-reported read/write against the policy — scope grants, mandatory owner/repo|tenant key prefixes, secret scrubbing, and approval gating. Pure logic with exhaustive tests; wiring into the brief and worker follows. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
…t validation (#6) Adds the [memory] config policy (off by default, per-repo overrides, doctor warning when writes are ungated), an optional memory_policy brief field + schema, and worker wiring: compute a deny-by-default trust-gated policy, stamp it into the brief only when opted in, and re-validate the runtime's reported memory_used against it, refusing out-of-scope activity. Emission stays off by default, so default deployments are unchanged pending the coven-code side. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances the hosted memory governance contract (#6) on the coven-github adapter side, adding optional schema fields and adapter-side computation/validation of memory policy and runtime-reported memory activity. Memory emission is intended to remain off by default, with policy stamped only when enabled per config.
Changes:
- Add optional contract fields:
memory_policyonSessionBriefandmemory_usedonSessionResult, plus matching JSON schema updates. - Introduce adapter-side memory policy computation and runtime self-report validation (
crates/worker/src/memory.rs) and wire it into the worker run path. - Add
[memory]configuration (off by default) with doctor warnings and example config documentation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/contracts/session-brief.schema.json | Adds optional memory_policy contract block for hosted memory governance. |
| docs/contracts/result.schema.json | Adds optional memory_used metadata to capture runtime-reported memory activity. |
| crates/worker/src/redact.rs | Updates result-sanitization test struct initialization for the new memory_used field. |
| crates/worker/src/memory.rs | Implements policy computation and validation of reported memory reads/writes against granted scopes/prefix rules. |
| crates/worker/src/lib.rs | Wires policy stamping into the brief and performs post-run revalidation of result.memory_used. |
| crates/worker/src/brief.rs | Extends SessionBrief to optionally include memory_policy and tests stamping/omission behavior. |
| crates/webhook/src/routes.rs | Updates test config initialization to include the new MemoryConfig default. |
| crates/github/src/lib.rs | Extends SessionResult with memory_used and adds serde round-trip tests. |
| crates/config/src/lib.rs | Adds [memory] config types, per-repo overrides, and doctor warnings for unsafe approval settings. |
| config/example.toml | Documents the new [memory] configuration block and intended defaults/constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+733
to
+743
| if let (Some(policy), Some(used)) = (&memory_policy, &result.memory_used) { | ||
| let rejections = | ||
| memory::validate_memory_used(policy, used, |text| redact::redact(text, &[]) != text); | ||
| if !rejections.is_empty() { | ||
| warn!( | ||
| task_id = %task.id, | ||
| rejected = rejections.len(), | ||
| "refused out-of-policy memory activity — not persisting those entries" | ||
| ); | ||
| } | ||
| } |
Comment on lines
+153
to
+155
| if !used.enabled { | ||
| return rejections; | ||
| } |
Comment on lines
+400
to
+404
| let memory_on = self.memory.enabled || self.memory.repos.values().any(|o| o.enabled == Some(true)); | ||
| if memory_on { | ||
| let gated = self.memory.approval_required | ||
| && self.memory.repos.values().all(|o| o.approval_required != Some(false)); | ||
| if !gated { |
Comment on lines
+27
to
+41
| #[derive(Debug, Clone, Default, Deserialize, Serialize)] | ||
| pub struct MemoryConfig { | ||
| /// Master opt-in. `false` (or section absent) → the adapter emits no memory | ||
| /// policy and the runtime does no memory work. | ||
| #[serde(default)] | ||
| pub enabled: bool, | ||
| /// Written memory stays `pending` until a maintainer approves it. | ||
| #[serde(default = "default_true")] | ||
| pub approval_required: bool, | ||
| /// Optional retention horizon for durable memory. | ||
| pub retention_days: Option<u32>, | ||
| /// Per-repo overrides keyed "owner/name". | ||
| #[serde(default)] | ||
| pub repos: std::collections::HashMap<String, RepoMemoryOverride>, | ||
| } |
BunsDev
added a commit
that referenced
this pull request
Jul 7, 2026
) Detect cross-repo (fork) PRs from the PR metadata (head repo id != base, or a deleted head repo) and thread head_is_fork through ResolvedTargets. The memory trust derivation now maps a fork PR to ForkPr — which grants no write scope — overriding even a maintainer trigger, since the risk is the untrusted content, not the actor. Closes the follow-up flagged in #50: auto and command reviews of fork content can no longer write durable memory even with approval. Signed-off-by: Val Alexander <bunsthedev@gmail.com>
This was referenced Jul 7, 2026
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.
Phase 2 of the memory governance contract (
docs/memory-contract.md, #48), coven-github side. Advances #6; does not close it. Emission is gated off by default, so default deployments are unchanged pending the coven-code counterpart (coven-code#98–104).What this delivers
memory_usedonSessionResult(+read/proposed/rejected) and the matchingresult.schema.json— the adapter tolerates and can inspect runtime-reported memory activity. Optional + skip-serialized, so memory-free results are unchanged and the golden fixtures still round-trip.crates/worker/src/memory.rs):compute_policyis deny-by-default and trust-gated —fork_prandexternalactors get no write scope, so a hostile fork PR can never write durable memory.validate_memory_usedre-checks every reported read/write against the policy: scope grants, mandatoryrepo/<owner>/<name>/…|tenant/<install>/…key prefixes (what makes inspect/revoke-by-repo possible), secret scrubbing viaredact, and approval gating. 10 unit tests cover the doc's test plan (fork-can't-write, cross-repo/tenant rejection, ungranted scope, secret, approval, memory-disabled).[memory](off by default; per-repo overrides; doctor warns when memory is enabled without approval gating).memory_policyonSessionBrief+session-brief.schema.json, stamped bybuild()only when the repo has opted in.result.memory_usedafter the run, refusing out-of-policy activity before it is persisted.Scope boundaries
Off-by-default emission keeps the bilateral contract safe: no brief carries
memory_policyunless an operator opts in (by which point the coven-code side is coordinated). Deferred, and noted inline: fork-PR hard-detection (currently auto-triggered reviews default tocollaborator, which already forbids durable writes without approval — the refinement tightens genuine fork PRs tofork_prnever-write), the inspect/revoke API surface (needs #3), and retention expiry / delete-on-uninstall (Phase 3).Local gates:
cargo check --all-targets+cargo clippy --all-targets -- -D warnings+cargo test --all(167 passed, 0 failed).