Skip to content

Commit 0491621

Browse files
BunsDevCompleteDotTechCopilot
authored
feat(hosted): close the hosted review hardening gaps from the #119 track (#134)
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>
1 parent 0cce5ef commit 0491621

15 files changed

Lines changed: 1337 additions & 29 deletions

File tree

docs/advanced.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,31 @@ Direct hosted auto-persistence requires an explicit trusted policy:
584584
`hostedReview.memorySourceTrust` must meet or exceed
585585
`hostedReview.memoryTrustThreshold`.
586586

587+
Additional hosted invariants:
588+
589+
- Every hosted scope component (tenant, installation, repo id, canonical repo
590+
identity) is validated non-empty before any hosted namespace, transcript
591+
path, or team-sync key is derived; an empty component fails closed instead
592+
of collapsing isolation.
593+
- Hosted loads floor the trust of memory entries that carry no `source`
594+
provenance, so an unattributed entry cannot self-attest a trusted level.
595+
Durable auto-extracted entries record their session/source provenance next
596+
to the trust label.
597+
- Memory deletion and redaction write frontmatter tombstones
598+
(`deleted_at`/`redacted_at`) that propagate through team-memory sync: a
599+
remote tombstone always applies over local content, a local tombstone is
600+
never resurrected by a pull, and a file deleted locally after a sync is not
601+
silently re-created.
602+
- A key with an unresolved team-memory pull conflict is blocked from further
603+
sync until the persisted conflict record under `.conflicts/` is resolved.
604+
- `/review` injects the ids and trust labels of every loaded memory entry into
605+
the review prompt and validates the returned review's memory citations
606+
against that set; unknown citations and memory-dependent findings without
607+
`memory_refs` are surfaced as warnings.
608+
- Headless review results carry a `review.memory` report listing the loaded
609+
memory domains and entries (id, effective trust, visibility, scope) — see
610+
the [headless contract](headless-contract).
611+
587612
---
588613

589614
## Security and permissions

docs/headless-contract.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,18 @@ the file MAY be absent.
149149
"findings": [],
150150
"tests_run": [],
151151
"no_findings_reason": "Reviewed the supplied PR file and found no blocking issues.",
152-
"limitations": []
152+
"limitations": [],
153+
"memory": {
154+
"domains_loaded": ["default-branch"],
155+
"entries": [
156+
{
157+
"id": "mem_review_policy",
158+
"trust": "maintainer-approved",
159+
"visibility": "public_review",
160+
"scope": "managed"
161+
}
162+
]
163+
}
153164
},
154165
"exit_reason": null
155166
}
@@ -191,6 +202,7 @@ the intended code. It is required on every result. Non-review tasks MUST set
191202
| `tests_run` | array | Commands run while reviewing, with `passed`, `failed`, `not_run`, or `unknown` status. |
192203
| `no_findings_reason` | string \| null | File-backed explanation for a clean review. MAY be `null` for degraded/partial output when `evidence_status` and `limitations` explain why a substantive clean-review conclusion was not possible. |
193204
| `limitations` | string[] | Evidence gaps, skipped checks, or other caveats. |
205+
| `memory` | object | Memory audit report: `domains_loaded` (hosted memory domains eligible for this review, e.g. `default-branch`; empty for local runs) and `entries` (every loaded memory entry with its stable `id`, effective `trust` label after hosted caps/floors, optional `visibility`, and load `scope`). Lets the consumer audit which memory inputs could have influenced findings and cross-check `memory_refs` citations. |
194206

195207
Each finding carries `severity`, `file`, optional `line`, `title`, `body`, and
196208
optional `recommendation`. Valid severities are `info`, `low`, `medium`, `high`,

src-rust/crates/cli/src/headless.rs

Lines changed: 183 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,74 @@ pub struct ReviewResult {
561561
pub tests_run: Vec<ReviewTestRun>,
562562
pub no_findings_reason: Option<String>,
563563
pub limitations: Vec<String>,
564+
/// Memory entries and domains that were loaded for this review, so the
565+
/// artifact records the trust level and provenance scope of every memory
566+
/// input that could have influenced findings.
567+
pub memory: ReviewMemoryUse,
568+
}
569+
570+
/// Memory usage report attached to a review artifact.
571+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Default)]
572+
pub struct ReviewMemoryUse {
573+
/// Hosted memory domains that were eligible for this review (e.g.
574+
/// `default-branch`). Empty for local, non-hosted runs.
575+
pub domains_loaded: Vec<String>,
576+
/// Every memory entry loaded into the review context.
577+
pub entries: Vec<ReviewMemoryEntry>,
578+
}
579+
580+
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
581+
pub struct ReviewMemoryEntry {
582+
/// Stable memory id (frontmatter `id` or content hash).
583+
pub id: String,
584+
/// Effective trust after hosted caps/floors (kebab-case label).
585+
pub trust: String,
586+
/// Declared visibility, when present.
587+
pub visibility: Option<String>,
588+
/// Memory scope the entry was loaded from (managed/user/project/local).
589+
pub scope: String,
590+
}
591+
592+
/// Enumerate the memory entries and domains the current configuration loads
593+
/// for a review of `workspace_root`. Uses the same load options as the live
594+
/// context build, so the report matches what the model actually saw.
595+
pub fn collect_review_memory(
596+
workspace_root: &Path,
597+
config: &claurst_core::config::Config,
598+
) -> ReviewMemoryUse {
599+
let options = config.memory_load_options();
600+
let files =
601+
claurst_core::claudemd::load_all_memory_files_with_options(workspace_root, &options);
602+
let entries = files
603+
.iter()
604+
.map(|file| ReviewMemoryEntry {
605+
id: claurst_core::claudemd::memory_id(file),
606+
trust: serde_enum_label(&claurst_core::claudemd::effective_memory_trust(
607+
file, &options,
608+
)),
609+
visibility: file.frontmatter.visibility.map(|v| serde_enum_label(&v)),
610+
scope: serde_enum_label(&file.scope),
611+
})
612+
.collect();
613+
let domains_loaded = if config.hosted_review_enabled() {
614+
// Hosted review currently loads only the default-branch domain;
615+
// security-private and branch domains are excluded by policy.
616+
vec![claurst_core::hosted_review::MemoryDomain::DefaultBranch.path_component()]
617+
} else {
618+
Vec::new()
619+
};
620+
ReviewMemoryUse {
621+
domains_loaded,
622+
entries,
623+
}
624+
}
625+
626+
/// Render a unit enum's serde label (kebab/snake-case string form).
627+
fn serde_enum_label<T: Serialize>(value: &T) -> String {
628+
match serde_json::to_value(value) {
629+
Ok(serde_json::Value::String(label)) => label,
630+
_ => "unknown".to_string(),
631+
}
564632
}
565633

566634
impl ReviewResult {
@@ -574,13 +642,23 @@ impl ReviewResult {
574642
tests_run: Vec::new(),
575643
no_findings_reason: None,
576644
limitations: Vec::new(),
645+
memory: ReviewMemoryUse::default(),
577646
}
578647
}
579648

580649
pub fn from_brief(
581650
brief: Option<&SessionBrief>,
582651
trace: Option<&ReviewTrace>,
583652
final_text: &str,
653+
) -> Self {
654+
Self::from_brief_with_memory(brief, trace, final_text, ReviewMemoryUse::default())
655+
}
656+
657+
pub fn from_brief_with_memory(
658+
brief: Option<&SessionBrief>,
659+
trace: Option<&ReviewTrace>,
660+
final_text: &str,
661+
memory: ReviewMemoryUse,
584662
) -> Self {
585663
let Some(brief) = brief else {
586664
return Self::none();
@@ -648,6 +726,7 @@ impl ReviewResult {
648726
tests_run: parsed.tests_run,
649727
no_findings_reason: parsed.no_findings_reason,
650728
limitations,
729+
memory,
651730
}
652731
}
653732
}
@@ -1322,19 +1401,40 @@ fn classify(
13221401
}
13231402
}
13241403

1325-
/// Build the `result.json` envelope and the process exit code from the run.
1326-
pub fn build_result(
1404+
/// Test convenience: build the result envelope with an empty memory report.
1405+
#[cfg(test)]
1406+
fn build_result(
1407+
brief: Option<&SessionBrief>,
1408+
git: &GitSummary,
1409+
outcome: RunOutcome,
1410+
final_text: &str,
1411+
review_trace: Option<&ReviewTrace>,
1412+
) -> (ResultEnvelope, i32) {
1413+
build_result_with_memory(
1414+
brief,
1415+
git,
1416+
outcome,
1417+
final_text,
1418+
review_trace,
1419+
ReviewMemoryUse::default(),
1420+
)
1421+
}
1422+
1423+
/// Build the result envelope with an explicit memory-usage report attached to
1424+
/// the review artifact.
1425+
pub fn build_result_with_memory(
13271426
brief: Option<&SessionBrief>,
13281427
git: &GitSummary,
13291428
outcome: RunOutcome,
13301429
final_text: &str,
13311430
review_trace: Option<&ReviewTrace>,
1431+
memory: ReviewMemoryUse,
13321432
) -> (ResultEnvelope, i32) {
13331433
let comment_only = brief.map(SessionBrief::is_comment_only).unwrap_or(false);
13341434
let (mut status, mut exit_reason, code) =
13351435
classify(outcome, !git.commits.is_empty(), comment_only);
13361436

1337-
let review = ReviewResult::from_brief(brief, review_trace, final_text);
1437+
let review = ReviewResult::from_brief_with_memory(brief, review_trace, final_text, memory);
13381438
if review.mode != ReviewMode::None
13391439
&& status == Status::Success
13401440
&& review.evidence_status != ReviewEvidenceStatus::Complete
@@ -1543,6 +1643,86 @@ mod tests {
15431643
trace.record_tool_end("Read", "", false);
15441644
}
15451645

1646+
// ── Review memory report ────────────────────────────────────────────────
1647+
1648+
#[test]
1649+
fn collect_review_memory_local_lists_project_entries() {
1650+
let ws = tempfile::tempdir().unwrap();
1651+
std::fs::write(
1652+
ws.path().join("AGENTS.md"),
1653+
"---\nid: mem_local_fact\ntrust: maintainer_approved\nsource: unit-test\n---\nLocal fact.",
1654+
)
1655+
.unwrap();
1656+
let config = claurst_core::config::Config::default();
1657+
1658+
let memory = collect_review_memory(ws.path(), &config);
1659+
1660+
assert!(memory.domains_loaded.is_empty());
1661+
let entry = memory
1662+
.entries
1663+
.iter()
1664+
.find(|entry| entry.id == "mem_local_fact")
1665+
.expect("project memory entry is reported");
1666+
assert_eq!(entry.scope, "project");
1667+
assert_eq!(entry.trust, "maintainer-approved");
1668+
}
1669+
1670+
#[test]
1671+
fn collect_review_memory_hosted_reports_domain_and_excludes_untrusted() {
1672+
let ws = tempfile::tempdir().unwrap();
1673+
// A repo file self-attesting high trust must not survive hosted caps.
1674+
std::fs::write(
1675+
ws.path().join("AGENTS.md"),
1676+
"---\nid: mem_attacker\ntrust: maintainer_approved\nsource: repo\n---\nAttacker fact.",
1677+
)
1678+
.unwrap();
1679+
let mut config = claurst_core::config::Config::default();
1680+
config.hosted_review.enabled = true;
1681+
1682+
let memory = collect_review_memory(ws.path(), &config);
1683+
1684+
assert_eq!(memory.domains_loaded, vec!["default-branch".to_string()]);
1685+
assert!(
1686+
memory.entries.is_empty(),
1687+
"hosted review must not report untrusted repo memory as loaded: {:?}",
1688+
memory.entries
1689+
);
1690+
}
1691+
1692+
#[test]
1693+
fn result_envelope_serializes_review_memory_report() {
1694+
let (dir, mut trace) = review_workspace();
1695+
record_successful_read(
1696+
&mut trace,
1697+
dir.path().join("src/support.rs").to_str().unwrap(),
1698+
);
1699+
let memory = ReviewMemoryUse {
1700+
domains_loaded: vec!["default-branch".to_string()],
1701+
entries: vec![ReviewMemoryEntry {
1702+
id: "mem_policy".to_string(),
1703+
trust: "maintainer-approved".to_string(),
1704+
visibility: Some("public_review".to_string()),
1705+
scope: "managed".to_string(),
1706+
}],
1707+
};
1708+
1709+
let (envelope, _) = build_result_with_memory(
1710+
Some(&sample_review_brief()),
1711+
&GitSummary::default(),
1712+
RunOutcome::Completed,
1713+
"## Findings\n- [low] src/lib.rs:1 — fine\n\n## Supporting Context Used\n- src/support.rs: checked",
1714+
Some(&trace),
1715+
memory,
1716+
);
1717+
1718+
let value = serde_json::to_value(&envelope).unwrap();
1719+
let memory_value = &value["review"]["memory"];
1720+
assert_eq!(memory_value["domains_loaded"][0], "default-branch");
1721+
assert_eq!(memory_value["entries"][0]["id"], "mem_policy");
1722+
assert_eq!(memory_value["entries"][0]["trust"], "maintainer-approved");
1723+
assert_eq!(memory_value["entries"][0]["scope"], "managed");
1724+
}
1725+
15461726
// ── Input conformance ───────────────────────────────────────────────────
15471727

15481728
#[test]

src-rust/crates/cli/src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,12 +919,13 @@ async fn main() -> anyhow::Result<()> {
919919
.unwrap_or_else(|| "main".to_string());
920920
let git_summary = headless::collect_git_summary(&cwd, &default_branch);
921921
let (envelope, exit_code) = match &run {
922-
Ok(r) => headless::build_result(
922+
Ok(r) => headless::build_result_with_memory(
923923
github_context.as_ref(),
924924
&git_summary,
925925
r.outcome,
926926
&r.final_text,
927927
Some(&r.review_trace),
928+
headless::collect_review_memory(&cwd, &config),
928929
),
929930
Err(e) => headless::infra_error_result(
930931
github_context.as_ref(),

src-rust/crates/cli/tests/headless_contract/result.example.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@
2222
}
2323
],
2424
"no_findings_reason": null,
25-
"limitations": []
25+
"limitations": [],
26+
"memory": {
27+
"domains_loaded": [],
28+
"entries": []
29+
}
2630
},
2731
"exit_reason": null
2832
}

src-rust/crates/cli/tests/headless_contract/result.schema.json

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,31 @@
105105
"limitations": {
106106
"type": "array",
107107
"items": { "type": "string" }
108+
},
109+
"memory": {
110+
"type": "object",
111+
"additionalProperties": false,
112+
"required": ["domains_loaded", "entries"],
113+
"properties": {
114+
"domains_loaded": {
115+
"type": "array",
116+
"items": { "type": "string" }
117+
},
118+
"entries": {
119+
"type": "array",
120+
"items": {
121+
"type": "object",
122+
"additionalProperties": false,
123+
"required": ["id", "trust", "visibility", "scope"],
124+
"properties": {
125+
"id": { "type": "string" },
126+
"trust": { "type": "string" },
127+
"visibility": { "type": ["string", "null"] },
128+
"scope": { "type": "string" }
129+
}
130+
}
131+
}
132+
}
108133
}
109134
},
110135
"allOf": [

0 commit comments

Comments
 (0)