feat(sccm): analyze client policy transactions - #391
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a public SCCM client-policy workflow analyzer. It parses and correlates policy evidence, reduces workflow transactions, reports coverage and findings, handles chronology and rotation cases, and adds catalog and integration-test coverage. ChangesSCCM client-policy analysis
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SccmNormalizedBundle
participant analyze_client_policy
participant PolicyFactParser
participant PolicyTransactionReducer
participant SccmWorkflowAnalysis
SccmNormalizedBundle->>analyze_client_policy: provide normalized artifacts and evidence
analyze_client_policy->>PolicyFactParser: validate and parse admissible evidence
PolicyFactParser->>PolicyTransactionReducer: provide policy facts and provenance
PolicyTransactionReducer->>SccmWorkflowAnalysis: construct workflow transactions
SccmWorkflowAnalysis-->>analyze_client_policy: return analysis results
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
#321 production policy reducer — exact-head implementation gateOwner/worktree: root agent — Exact head: Dependency state: #318 shared spine is merged at Implemented scope: pure-Rust/wasm-compatible TDD evidence:
Fixture/test matrix: 14 frozen sanitized scenarios ( Verification at exact head:
Review/acceptance state: draft/open. Local CodeRabbit 0.7.1 exact-range review is rate-limited for 13 minutes and is explicitly not approval. A hosted exact-head full review and a different independent review are being requested. Synthetic fixtures only; no native Windows/live SCCM acceptance is claimed. |
|
@coderabbitai full review Please review the full exact-head range |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 3 minutes. |
|
Independent exact-head review active in fresh detached worktree |
adamgell
left a comment
adamgell
left a comment
There was a problem hiding this comment.
Independent exact-head review: BLOCK
Reviewed the full a8a367c..1e5a317 range in a fresh detached, read-only worktree. I made no edits or pushes.
Committed gates
- Focused policy: 13/13 pass.
- Shared spine: 136/136 pass.
- Full cmtraceopen-parser suite: pass.
- Strict parser Clippy: pass.
- Rust 1.88 wasm32 check: pass.
- TypeScript noEmit: pass.
- Scoped Rustfmt and git diff --check: pass; worktree clean.
- Workspace-wide cargo fmt remains red only on inherited files outside all eight PR-owned paths; every changed Rust file passes rustfmt.
Independent adversarial evidence
A disposable external harness imported all 13 committed policy tests, then ran six additional contract probes. Command: cargo test --test probe -- --nocapture. Result: 13 committed tests passed and all 6 adversarial probes failed against this exact SHA.
- Duplicate exact-key labels fail open: adding a second conflicting occurrence of each of AssignmentId, PolicyId, RequestId, ClientHandle, SiteCode, and SelectedManagementPointHostHandle still emits a high-confidence successful exact transaction.
- Compound markers fail open: not-Request succeeded-ish and an embedded NotRequestId label both emit the same high-confidence exact transaction.
- Conflicting facts with one evidence identity are input-order dependent: cloning the Report reference with the opposite terminal result yields high ConfirmedFailure in one order and high Success after reversing input, while the exported evidence references are identical after deduplication.
- Unusable chronology still proves a complete sequence: clearing utc_millis and marking every cross-artifact timestamp OffsetInvalid still emits high Succeeded at Report.
- Coverage provenance is collapsed: StateMessage AccessDenied plus captured CIAgent becomes a synthetic client-policy-state Partial gap, losing the explicit access-denied source state.
- Latest local state is not honored: a same-artifact Schedule Deferred followed by Schedule Succeeded resolves to Contradictory rather than the later recovered state.
Required before re-review
- Route policy key admission through the frozen versioned extraction-profile contract, including full token boundaries and exactly one unambiguous occurrence per required label. A hard-coded profile string plus ad hoc first-match extraction is not an exact validated key.
- Reject or conservatively quarantine conflicting facts that share one logical evidence identity before sorting/deduplication, and add a permanent reversal test.
- Resolve coherent same-source latest state by source-local order, including Deferred to Succeeded recovery. Do not let missing or invalid cross-artifact time provenance prove a high-confidence request-to-report sequence.
- Preserve each unavailable source state and citation; a captured sibling cannot erase an AccessDenied, Capped, Missing, or other explicit gap.
- Move or freeze shared intake/model additions such as SccmNormalizedBundle and the shared Partial coverage variant in the upstream #318/#319 contract before merging #321. This workflow PR must consume stable shared interfaces rather than become the dependency base for unrelated reducers.
- Promote all six probes into repository tests and show focused red-to-green evidence.
This is a substantive merge block, not a compile failure. The reducer remains pure Rust/wasm-compatible and makes no explicit MP causal claim, but its exact-key, evidence-identity, chronology, latest-state, and coverage guarantees are not yet conservative enough for counterpart-ready facts or downstream #333 correlation.
CodeRabbit note: the hosted exact-head attempt currently contains only a fair-usage/rate-limit response and a green status context, not substantive review findings. It must remain in the loop, but that status is not approval.
| fn extract_label_token<'a>(message: &'a str, label: &str) -> Option<&'a str> { | ||
| let normalized = message.to_ascii_lowercase(); | ||
| let marker = format!("{}=", label.to_ascii_lowercase()); | ||
| let start = normalized.find(&marker)? + marker.len(); |
There was a problem hiding this comment.
[P1] Exact-key extraction must fail closed on ambiguous or embedded labels. This first-match search accepts both a conflicting second RequestId and NotRequestId=... as the required RequestId, and equivalent duplicate mutations pass for every required key label while retaining high confidence. Please use the frozen versioned key-profile extractor or equivalent exact-boundary, exactly-once validation, with permanent duplicate/embedded-label tests.
| if same_artifact { | ||
| return facts | ||
| .iter() | ||
| .max_by(|left, right| compare_evidence_refs(&left.reference, &right.reference)) |
There was a problem hiding this comment.
[P1] Validate logical evidence identity before choosing a latest outcome. Two opposite terminal facts with the same artifactId, entryId, and line range compare equal here; max_by then selects according to input order. Reversing that input flips the public result between high ConfirmedFailure and high Success even though deduplicated evidence is identical. Reject/quarantine identity collisions before reduction and add the reversal regression. The same latest-state path should also allow later same-artifact Deferred to Succeeded recovery.
| for later in facts.iter().filter(|fact| fact.phase <= current_phase) { | ||
| if earlier.phase >= later.phase | ||
| || earlier.reference.artifact_id == later.reference.artifact_id | ||
| || !earlier.time_comparable |
There was a problem hiding this comment.
[P1] Skipping non-comparable cross-artifact phases removes the only chronology guard, after which a complete set of markers is still emitted as high Succeeded at Report. A probe with utcMillis absent and OffsetInvalid on every phase reproduces this. Missing or invalid provenance must cap the sequence or make it explicitly incomplete; it cannot prove an ordered request-to-report path.
| .filter(|artifact| policy_group(&artifact.display_name) == Some(logical_id)) | ||
| .map(|artifact| artifact.coverage.clone()) | ||
| .collect::<Vec<_>>(); | ||
| if states.is_empty() || states.contains(&SccmCoverageState::Captured) { |
There was a problem hiding this comment.
[P1] A captured sibling must not erase an explicit coverage gap. With CIAgent captured and StateMessage AccessDenied, this early return discards the access-denied state and the caller synthesizes client-policy-state Partial. Preserve source-specific coverage/citations so AccessDenied, Capped, Skipped, Unsupported, malformed, and missing remain observable non-outcomes.
| /// The bundle owns no raw file handles or collection behavior. Its evidence has | ||
| /// already passed through the shared CCM logical-record scanner. | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| pub struct SccmNormalizedBundle { |
There was a problem hiding this comment.
[P1] This is an intake/shared interface, not policy workflow behavior. The execution contract makes #321 a consumer of frozen #318/#319 contracts; introducing SccmNormalizedBundle here forces unrelated reducers to stack on the policy branch. Land and review this bundle contract upstream, together with any shared coverage-model addition, then rebase this issue-scoped reducer onto that public API.
|
@coderabbitai full review Please perform a substantive exact-head review of a8a367c..1e5a317 after the prior fair-usage response. An independent review has now posted six reproducible blocker classes and five inline anchors. Please evaluate the exact-key uniqueness/boundaries, logical evidence-identity collisions, input-order determinism, latest same-source recovery, invalid cross-artifact time provenance, source-specific coverage preservation, and the #318/#319 shared-interface dependency. A green status context without findings is not approval. |
|
I will assess exact-key boundaries, evidence identity, deterministic ordering, recovery state, timestamp provenance, source-specific coverage, the ✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 31 minutes. |
adamgell
added
feature
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 38 minutes. |
adamgell
left a comment
adamgell
left a comment
There was a problem hiding this comment.
BLOCK at exact head 1fb7a86.
Battery at 1fb7a86: sccm_client_policy 22 passed; sccm_spine_contract 136
passed; cargo test --locked -p cmtraceopen-parser 751 passed, 0 failed;
cargo clippy --locked --workspace --all-targets -- -D warnings exit 0;
cargo check --target wasm32-unknown-unknown exit 0; rustfmt --check clean on
all nine changed .rs files. cargo test --locked --workspace is green apart
from src-tauri/tests/cmtlog_parser.rs, a pre-existing shared-temp-dir race
that passes with --test-threads=1 and on three reruns; this PR touches no
file under src-tauri.
RED discipline verified for all six pairs by replaying each RED at its own
commit: 3e24f79 3/16, 246bef2 2/18, c8ca95b 1/20, d0d5153 1/21,
f2c9e70 1/22, 7e4ef4d 1/22. Every failure is an assertion whose message
matches the reason its test name claims.
Per-closure adjudication:
- Exact-key admission: PARTIAL. See gaps 3 and 6.
- Identity collisions: CLOSED. Equal claims are not quarantined, a
three-way collision is, and both are order independent. - Chronology: CLOSED. All-OffsetInvalid no longer yields high Succeeded,
partial comparability caps in every single-source variant with the right
group and reason, and the anti-over-reach case holds. - Coverage gaps: CLOSED for the scenario the thread named. See gap 5.
- Scope split: NOT CLOSED as stated. Detail below.
- Cross-role: PARTIAL. See gaps 1 and 2.
- Recapture reason: PARTIAL. See gap 4.
Gaps, each reproduced:
-
[P1] group_has_no_captured_source (policy.rs:1185-1194) is not filtered
to SccmRole::Client. Adding a ManagementPoint artifact named
LocationServices.log with Captured coverage to request-auth-failure
erases the client-location coverage gap and its artifact request and
raises the transaction from confirmedFailure/medium to
confirmedFailure/high. The inverse leaks too: an MP artifact at
AccessDenied creates a client-location gap and a client artifact request
that the client-only bundle does not have. coverage_gaps_for_group
(line 1142) does filter on role, so this is inconsistent within the
same feature. -
[P1] The last-wins artifact_by_id collect (policy.rs:205-210) is still
decided by artifact vector order when two Client artifacts share one
artifact_id. With a second client artifact reusing
policy-complete-agent-current as CIDownloader.log/AccessDenied, the
forward bundle yields zero transactions plus a malformed symptom and the
reversed bundle yields high-confidence succeeded. f2c9e70 passes only
because it shadows with a different role. -
[P1] is_label_boundary_start (line 927) uses is_label_token_boundary
(line 944), the value terminator set, as its left word-boundary test. A
conflicting occurrence preceded by [ ( = / # | { or * is neither
admitted nor counted as ambiguity. Appending
context=[RequestId={99999999-9999-9999-9999-999999999999} retry=1] to
the complete-scenario request record still produces one transaction,
state succeeded, confidence high, keyed on the first RequestId.
Terminator-prefixed duplicates all fail closed, which shows the
asymmetry is unintended. -
[P2] request_for_group (line 1096) keys the reason on the phase alone,
so a MISSING Schedule phase is requested with "Recapture bounded
scheduler evidence for the deferred policy retry." Removing the Schedule
record from the complete scenario reproduces it. Same class as closure 7. -
[P2] A non-captured sibling is invisible once the chain succeeds. With
StateMessage.log captured and a StatusAgent.log sibling at AccessDenied,
Capped, Skipped, Unsupported, or ParseFailed, coverageGaps is empty in
all five cases. Closure 4 works only because losing the source also
loses the phase. -
[P3] is_marker_boundary (line 758) tests raw bytes, so a UTF-8
continuation byte reads as a separator and "cafeRequest succeeded" (with
an e-acute) is admitted as the exact "Request succeeded" marker. Same
class as the not-Request case.
On closure 5: sccm/ingest.rs is genuinely pre-existing in a8a367c,
0df7f42 is the only commit in this PR that touches it, the public path
sccm::SccmNormalizedBundle is unchanged, and nothing outside the PR
referenced the type. The objection is still open on its own terms.
0df7f42 also deletes from client/mod.rs and edits policy.rs, both files
this PR creates, so it cannot be applied to the base and no other reducer
can consume the contract without stacking on this branch, which is the harm
the thread named. The thread also asked for any shared coverage-model
addition to land upstream, and SccmCoverageState::Partial in models.rs plus
the coverage_state_order renumbering in findings.rs remain in this PR.
Separately, health.rs is not present in this tree, so the pattern cited by
closures 4 and 6 could not be cross-checked here.
Privacy: clean. A canary sweep injecting a hostname, a C:\Users path, and a
message payload across six scenarios leaked nothing into the serialized
analysis, and the diff sweep over 1e5a317..HEAD shows no PII, host, path,
or credential material.
Non-blocking note: opposite claims at the same artifact and line range but
different entry ids are resolved by a lexicographic entry-id tiebreak. It is
deterministic and lands conservative in the case probed, but the winner is
chosen by string sort rather than by evidence.
Recommend gaps 1, 2, and 3 be closed RED-first before merge, since PR #392
and the #333 correlation work depend on these facts being stable. Gaps 1 and
2 in particular are the same defect classes closures 6 and 2 were opened
for, at sites the frozen tests do not reach.
1fb7a86 to
d6c2bc3
Compare
There was a problem hiding this comment.
Pull request overview
Adds an SCCM client-side policy workflow analyzer to cmtraceopen-parser, along with a new synthetic-corpus contract test suite and supporting plumbing (normalized bundle type, coverage state expansion, and catalog entries). This extends the SCCM diagnostics layer to model policy request→download→persist→schedule→evaluate→report phases and emit transactions, observations, findings, coverage gaps, and artifact requests.
Changes:
- Introduce
sccm::client::policyreducer (analyze_client_policy) and its public analysis model types. - Add a comprehensive
sccm_client_policyintegration test using the 14-scenario synthetic policy fixture corpus. - Expand SCCM coverage/state/catalog support for the policy workflow (new
Partialcoverage state + new policy-related catalog tuples).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/cmtraceopen-parser/tests/sccm_spine_contract.rs | Extends spine contract expectations for the new Partial coverage state and additional policy-related catalog tuples. |
| crates/cmtraceopen-parser/tests/sccm_client_policy.rs | New integration test suite defining the frozen public behavior contract for client policy analysis across 14 scenarios. |
| crates/cmtraceopen-parser/src/sccm/models.rs | Adds SccmCoverageState::Partial to the shared SCCM model. |
| crates/cmtraceopen-parser/src/sccm/mod.rs | Exposes the new sccm::client module and re-exports it from sccm. |
| crates/cmtraceopen-parser/src/sccm/ingest.rs | Introduces SccmNormalizedBundle as the normalized SCCM analyzer input contract. |
| crates/cmtraceopen-parser/src/sccm/findings.rs | Updates coverage-gap ordering to account for the new Partial state. |
| crates/cmtraceopen-parser/src/sccm/client/policy.rs | New client policy reducer implementation (transaction reduction, evidence/key extraction, findings, coverage gaps, and artifact requests). |
| crates/cmtraceopen-parser/src/sccm/client/mod.rs | New client module wrapper re-exporting the policy analyzer. |
| crates/cmtraceopen-parser/src/sccm/catalog.rs | Adds CIAgent/CIDownloader/StateMessage/StatusAgent to the declared SCCM source catalog under ClientPolicy. |
| for earlier in facts.iter().filter(|fact| fact.phase <= current_phase) { | ||
| for later in facts.iter().filter(|fact| fact.phase <= current_phase) { | ||
| if earlier.phase >= later.phase | ||
| || earlier.reference.artifact_id == later.reference.artifact_id |
| pub mod catalog; | ||
| pub mod client; | ||
| mod evidence; |
Code reviewFive independent reviewers examined this change from different angles (CLAUDE.md compliance, focused bug scan, git-blame history, prior-PR feedback, and code-comment guidance). Each candidate finding was then independently verified and confidence-scored; anything below 80 is reported separately as unconfirmed. 1. Cross-phase time inversion emits one finding spanning two unrelated logical groups, violating the suite's own frozen invariant
Verified by probe against head. 2. An entirely uncollected client-location group yields HIGHER confidence than one recorded as absent
3. Ambiguous-artifact-id quarantine runs before the policy-scope check, so non-policy evidence becomes policy findings
4. Repair requests are derived from the phase, not from the source that holds it: Scheduler.log and PolicyEvaluator.log can never be requested
The frozen tests project requests back to their group in 5. "Name the source that broke chronology" names it by phase, so a CIDownloader or PolicyEvaluator break points at the wrong log family
6. Ambiguous-artifact-id rejection runs before policy scoping, so unrelated client logs become policy findings
7. Success markers are never checked against an explicit terminal result code (one-sided check; the exact class fixed in PR #393)
8. Evidence quarantine only catches identical line ranges, not overlapping physical intervals (accepted P1 class from PR #393)
The same hole lets one physical line prove two different phases, since nothing prevents e.g. PolicyAgent 1-2 Request and 2-3 Persist from both being admitted. 9. Cross-artifact time inversion still fans out to every inverted group, so one finding requests two unrelated log families
Verified full suite (34 tests) green at head, so the invariant is genuinely unenforced on this branch. 10. Ambiguous client artifact ids are rejected before the policy-scope gate, so duplicated non-policy client logs become
|
Freeze that any non-word character starts a countable label occurrence, that a word character never does, and that phase markers judge neighbours as characters rather than raw bytes. Refs #321
Define a boundary as the absence of a word character instead of an enumerated separator list, so punctuation nobody listed can no longer hide a duplicate label. Compare characters rather than raw bytes so a multibyte letter cannot pose as a separator, and stop reusing the value-terminator set as a left boundary. Refs #321
Freeze that an out-of-scope artifact sharing a policy basename neither answers a client coverage question nor invents a client gap, in both the captured and unavailable directions. Refs #321
Route every logical-group coverage query through one client-role selector so an out-of-scope artifact sharing a policy basename can neither satisfy a client coverage question nor invent a client gap. Refs #321
Freeze that two client artifacts sharing one id are ambiguous rather than order dependent, stay observable, and cannot carry a high-confidence verdict, while an exact repeat is harmless. Refs #321
Detect duplicate ids within the client role and withhold every id no artifact can speak for, so admission is never decided by artifact vector order. Ambiguous records stay observable as a local symptom. An exact repeat is not a conflict. Refs #321
Freeze that an unavailable client policy source stays a cited non-outcome even when the transaction proves a complete chain, without vetoing it, and that a fully captured bundle stays gap free. Refs #321
Report every client policy source that is not fully captured as a coverage gap regardless of the transaction verdict, so a sibling that happened to carry the chain can no longer hide an explicit non-outcome. Refs #321
Freeze that a missing phase is not asked for as a deferred retry and that a contradiction caused by unusable time asks for usable time, while a genuine deferral keeps its own wording. Refs #321
Key every artifact request on the state that caused it instead of on the phase, so a missing phase is no longer asked for as a deferred retry. Carry the contradiction cause out of phase resolution so an unorderable phase asks for usable time. Refs #321
Two probes the reducer currently answers the wrong way round. Removing the only client-location record raises confidence from medium to high and deletes the repair request, and a confirmed terminal failure past the first missing phase leaves no trace in any output field. Refs #321
An empty client-location group answered "a captured source exists", so deleting the only record of an absence raised confidence from medium to high and dropped the repair request. The guard now agrees with its citation twin, which already maps the empty group to absent. A confirmed terminal failure recorded past the first missing phase is no longer discarded. It leaves the reduction as a source-local observation and an error-severity finding instead of vanishing behind a moderate evidence gap. Refs #321
The success side of the terminal result check and the overlapping half of evidence quarantine are both open here. A success or deferral marker carrying an explicit failure code still proves its phase, and a wider overlapping line range still outranks the record it overlaps. Refs #321
A marker and an explicit terminal result that disagree can no longer be read either way, so a success or deferral carrying a failure code stops proving its phase, mirroring the existing failure-side check. Evidence quarantine now compares physical line extent instead of an identity tuple, so a wider overlapping range no longer outranks the record it overlaps and one physical line can no longer prove two phases. Refs #321
Six probes for requests derived from the phase rather than the artifact holding the evidence. Scheduler.log and CIDownloader.log can never be named, a deferral asks for PolicyAgent, an inversion between two valid offsets claims the offset is invalid and fans out across two families, and an ambiguous non-policy client id becomes a policy finding. Refs #321
Every repair now carries the declared sources that would supply it, read off the artifact rather than guessed from the phase. Scheduler.log and CIDownloader.log become requestable, a deferral asks for the file that deferred, and a chronology break names the family that broke it. A cross-phase inversion names one broken link with its own cause, so it no longer claims a valid normalized offset is unusable and no longer spans two log families in one finding. A break also replaces the missing phase request instead of joining it, since a phase can only be called missing once the surrounding phases can be sequenced. Ambiguous client artifact ids are only escalated when the clash costs this reducer an admissible policy source, so a duplicated AppEnforce id no longer becomes a policy finding. Refs #321
Scheduler.log and PolicyEvaluator.lo_ are not fragments of one another, yet the reducer reports them as a split PolicyAgent record because the candidate set is the whole logical group. Refs #321
Split candidates are grouped by their declared basename, so only files that could actually be rotations of the same record are compared. An unrelated agent-group sibling no longer turns into half a torn PolicyAgent record. Refs #321
The rebase onto codex/parser-family-skeleton applied cleanly and did not build. Three independent collisions, each from both sides adding the same contract separately: SccmCoverageState::Partial is introduced by this lane, and the skeleton independently added seven exhaustive matches over that enum. Place Partial semantically in each rather than widening any match to a wildcard: it carries real evidence, so it outranks every noncaptured state and loses only to a complete capture, matching findings::coverage_state_order, which is the canonical ordering. Rank maps, sort keys, rejection reasons, and operator wording each get the placement their own function means. SOURCE_CATALOG and its frozen expected-tuple list each carried a duplicated run of CIAgent, CIDownloader, StateMessage, and StatusAgent: the skeleton already declares every client-policy entry this lane wanted to add, in a different order, so git appended rather than merged them. Drop the duplicate run from both sides of the contract. SccmClientWorkflow was defined in both client/intake.rs and client/policy.rs, so the two glob re-exports in client/mod.rs were ambiguous. The intake definition is the shared four-variant client contract and the policy one was a single-variant stub predating it; policy now imports the shared type. Every client reducer must agree on one set of workflow names or identical evidence sorts differently per lane. Refs #321
1bdcd9c to
486e982
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
crates/cmtraceopen-parser/src/sccm/client/policy.rs:443
policy:source-local:malformedalways requestsclient-policy-agent, butrejected_policy_evidencecan include policy-state artifacts too (e.g., invalid profile/version or ambiguous parsing from CIAgent/StateMessage). This can produce a remediation hint that names the wrong group and even says “policy-agent evidence” when the rejected evidence is policy-state-only.
let request = workflow_request(
POLICY_AGENT_GROUP,
"Capture bounded policy-agent evidence under a validated ConfigMgr version profile with a complete exact key.",
);
crates/cmtraceopen-parser/src/sccm/client/policy.rs:321
- The PR description says this commit adds only the RED test contract and that
analyze_client_policy/SccmNormalizedBundle“do not exist yet”, but this PR adds both the production reducer (analyze_client_policy) and the normalized bundle type. Please update the PR description (and/or remove the outdated CodeRabbit summary) so reviewers understand the true scope of the changes.
pub fn analyze_client_policy(bundle: &SccmNormalizedBundle) -> SccmWorkflowAnalysis {
crates/cmtraceopen-parser/src/sccm/client/intake.rs:1520
- The comment above
SccmCoverageState::Partialis misleading here:group_coverage()usesmax_by_key(coverage_rank), so higher rank values represent worse coverage. Also, this ranking does not fully matchfindings::coverage_state_order(e.g.,Unsupported/Skippedare ordered differently), so calling that “canonical” is inaccurate.
// Partial carries real evidence, so it outranks every noncaptured
// state while still losing to a complete capture. This mirrors
// `findings::coverage_state_order`, which is the canonical ordering.
Execution gate — rebuild requiredThis PR is now draft and must not merge from its current head Compared with the current SCCM integration head Two additional reducer/test gaps remain reproducible: unrelated URL Resume gate: create a fresh #321 branch from the then-current |
Implements the production reducer slice of #321 against the already-merged 14-scenario synthetic policy corpus.
Current TDD state: RED by design at
394dc17.cargo test --locked -p cmtraceopen-parser --test sccm_client_policyfails only becauseanalyze_client_policyandSccmNormalizedBundledo not exist yet.Next commit will add the smallest pure-Rust/wasm-compatible keyed reducer and the four exact policy-state source catalog tuples identified in #321. This PR will remain draft until focused/aggregate/Clippy/wasm/format gates pass and exact-head CodeRabbit plus independent false-causality review clear.
Summary by CodeRabbit
New Features
Bug Fixes