feat(hosted): close the hosted review hardening gaps from the #119 track#134
Conversation
Scope validation, provenance, sync lifecycle, conflict gating, and citation enforcement follow-ups to the hosted review stack landed in PR #132. - Validate hosted scope components everywhere a hosted namespace is derived (memory paths, transcripts, team sync, settings sync); empty components fail closed instead of collapsing tenant/repo isolation. - Verify caller-claimed local project ids against the identity derived from the origin remote; a missing or unparseable remote fails closed. - Persist source/session provenance on durable auto-extracted memories and floor hosted trust for entries that carry no provenance. - Report loaded memory domains and per-entry trust/visibility/scope in ReviewResult and the v2 result envelope (schema, example, and docs). - Propagate deletion/redaction tombstones through team-memory sync; never resurrect locally deleted or tombstoned files on pull. - Block keys with unresolved pull conflicts from sync until the persisted conflict record is resolved; exercise the RemoteOnly path. - Inject loaded memory ids into the live /review prompt and validate the returned citations against the loaded set. closes #98 closes #99 closes #103 closes #104 closes #106 closes #107 closes #109 closes #110 closes #111 closes #112 closes #119 Co-authored-by: Timothy Wayne Gregg <5861166+romgenie@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Closes the remaining hardening gaps for Hosted Review Mode by tightening hosted scope/project identity validation, improving memory provenance/trust handling, hardening team-memory sync deletion/conflict semantics, and enforcing/recording memory usage and citations in both /review and headless artifacts.
Changes:
- Fail-closed hosted scoping: validate non-empty tenant/installation/repo/canonical identity before deriving hosted namespaces (memory + transcripts) and before hosted settings/team sync keying.
- Memory hardening: persist durable memories with provenance attribution, floor trust for hosted loads missing provenance, and add tombstone-based deletion semantics (propagated through sync).
- Review/audit surfaces: headless results now include a
review.memoryaudit report;/reviewinjects loaded memory IDs and validates returned review citations against what was loaded; schema + docs updated.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src-rust/crates/core/src/hosted_review.rs | Adds validate() for hosted scope and canonical repo identity; extends tests for namespace distinctness. |
| src-rust/crates/core/src/memdir.rs | Validates hosted scope before deriving hosted memory paths; adds tombstone-based delete_memory_file(); adds isolation/leak-prevention tests. |
| src-rust/crates/core/src/session_storage.rs | Validates hosted scope before deriving hosted transcript paths. |
| src-rust/crates/core/src/claudemd.rs | Floors trust for hosted loads when provenance is missing; ensures managed rules carry source. |
| src-rust/crates/core/src/team_memory_sync.rs | Makes hosted constructor fallible + scope-validating; adds tombstone precedence and conflict gating on pull; exposes conflict inspection helpers. |
| src-rust/crates/core/src/git_utils.rs | Adds fail-closed verified_local_project_id() derived from origin remote; includes tests. |
| src-rust/crates/core/src/settings_sync.rs | Verifies claimed project id before applying/collecting project-scoped settings/memory; hosted collection validates scope and is now fallible. |
| src-rust/crates/query/src/session_memory.rs | Adds persist_with_provenance() and routes approvals/hosted persistence through it; adds tests proving provenance persistence. |
| src-rust/crates/commands/src/lib.rs | /review now lists loaded memory IDs/trust in-prompt and validates output citations (mem_...) against the loaded set. |
| src-rust/crates/cli/src/headless.rs | Adds ReviewResult.memory report; collects loaded memory entries/domains; introduces build_result_with_memory(). |
| src-rust/crates/cli/src/main.rs | Switches to build_result_with_memory() and includes collected review memory in v2 envelope. |
| src-rust/crates/cli/tests/headless_contract/result.schema.json | Extends schema with optional review.memory object (domains_loaded + entries). |
| src-rust/crates/cli/tests/headless_contract/result.example.json | Updates example to include empty review.memory. |
| docs/headless-contract.md | Documents review.memory fields and intent (audit + citation cross-check). |
| docs/advanced.md | Documents additional hosted invariants (scope validation, trust flooring, tombstones, conflict gating, citation enforcement, memory reporting). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| block.push_str(&format!( | ||
| "- {} (trust: {}, from {})\n", | ||
| claurst_core::claudemd::memory_id(file), | ||
| trust, | ||
| file.path.display() | ||
| )); |
| /// Extract `mem_...` citation tokens from free-form review text. | ||
| pub fn extract_memory_citations(text: &str) -> Vec<String> { | ||
| let mut cited: Vec<String> = Vec::new(); | ||
| for token in text.split(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) { |
| let provenance_label = provenance | ||
| .filter(|p| !p.trim().is_empty()) | ||
| .map(|p| format!(", provenance: {}", p)) | ||
| .unwrap_or_default(); |
| fn conflict_record_path(team_dir: &Path, key: &str) -> PathBuf { | ||
| let safe_key = key.replace('/', "__"); | ||
| team_dir.join(".conflicts").join(format!("{safe_key}.json")) | ||
| } |
|
Reviewed the full diff plus a security pass on the isolation boundaries. Two findings, both fixed in f1f259d:
Remaining caveats for the issue track (not blockers):
Verified locally at f1f259d: fmt, |
Summary
Context
/reviewcitation enforcement.Changes
core/hosted_review.rs:HostedReviewScope::validate()andCanonicalRepoIdentity::validate()reject empty/whitespace identity components; namespace-distinctness tests for two repos under one installation.core/memdir.rs,core/session_storage.rs: hosted memory/transcript path derivation validates the scope before keying anything;delete_memory_file()writes adeleted_attombstone instead of removing the file; tests prove the same repo at two paths maps to one hosted namespace, different repos at one path map to different namespaces, two repos under one installation cannot see each other's memory, and branch-domain content cannot leak into a default-branch load.core/claudemd.rs: hosted loads floor the trust of entries missingsourceprovenance (publiceffective_memory_trust); unattributed managed rules can no longer self-attestsystem_policy.core/team_memory_sync.rs:TeamMemorySync::hosted()is fallible and validates the full scope; remote tombstones always apply, local tombstones are never resurrected, locally deleted files are not re-created (RemoteOnlynow emitted and tested); keys with unresolved conflict records are blocked from pull (UnresolvedConflictPending) untilresolve_conflict();pending_conflicts()exposes the gate.core/settings_sync.rs,core/git_utils.rs:verified_local_project_id()derives the project id from the origin remote and fails closed on missing/ambiguous remotes or mismatched claimed ids;collect_local_entries/apply_to_localverify caller-claimed project ids;collect_hosted_entriesvalidates the scope.query/session_memory.rs: durable persists carryprovenance: session:<id>;source:<origin>next to the trust label (persist_with_provenance); candidate approval keeps its recorded provenance.cli/headless.rs,cli/main.rs:ReviewResultgains amemoryreport (domains_loaded+ per-entryid/trust/visibility/scope), collected from the same load options as the live context build and emitted in the v2 result envelope.commands/lib.rs:/reviewnow injects the loaded memory ids and trust labels into the review prompt and validates the returned review's citations (validate_review_memory_citations) — unknown ids and memory-dependent findings withoutmemory_refsare surfaced as warnings in the output.result.schema.jsondeclares the optionalreview.memoryobject,result.example.jsonupdated,docs/headless-contract.md§3.2 documents it;docs/advanced.mddocuments the new hosted invariants.Validation
git diff --check— cleancargo fmt --all— cleancargo check --workspace— clean, no warningscargo clippy --workspace --all-targets -- -D warnings— cleancargo test --workspace— all green (568 core, 102 commands, 101 query, 57+4+1 cli incl. ACP smoke, 653 tui, plus integration suites)npx ajv-cli validate --spec=draft2020 -s result.schema.json -d result.example.json—result.example.json validPR Readiness
mem_...test id, annotated inline).Risks / follow-ups
review.memoryis a new always-serialized property on the v2 result envelope. The schema copy in this repo declares it (optional,additionalProperties: falseinner objects); the coven-github adapter's schema copy needs the same additive declaration before it strictly validates results produced by this build.pending_conflicts(); there is still no live command path that drives team sync, so no UI surfaces the gate yet.