fix(sccm): canonicalize server intake fixture contracts - #345
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
Comment |
|
@coderabbitai full review Please review exact head |
|
✅ 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 13 minutes. |
There was a problem hiding this comment.
Pull request overview
This PR canonicalizes the synthetic SCCM server-intake fixture corpus for Issue #335 so it matches documented ConfigMgr contracts (3-character site codes and .lo_ rollover naming), and adds an automated contract test to prevent future fixture drift.
Changes:
- Replaces invalid
CONTOSOsite codes with a synthetic 3-characterLABacross the server-intake manifests. - Canonicalizes
.lo_rollover naming (MP_GetPolicy.lo_) and updates the rotations fixture path accordingly. - Adds a focused Rust fixture-contract test to enforce scenario-matrix size, site-code format,
.lo_basename/path existence, and byte-count integrity; updates related documentation.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/sccm/preparation/issue-335-server-intake.md | Updates documentation to describe canonical .lo_ rollover spelling as replacement-extension. |
| crates/cmtraceopen-parser/tests/sccm_server_intake_fixture_contract.rs | Adds a contract test validating server-intake fixture invariants (site code, .lo_ artifact identity/path, bytes). |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/README.md | Updates fixture README to reflect the new contract test and canonical site-code expectations. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/unsupported-db-supplement/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/unsorted-manifest/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/skipped-iis/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/rotations/manifest.json | Updates siteCode and canonicalizes .lo_ basename/relativePath. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/rotations/evidence/sccm/server/management-point/server-mp-policy/lo_/MP_GetPolicy.lo_ | Adds/updates the .lo_ rollover fixture file with canonical basename. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/multiline/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/configured-nondefault-path/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/complete-multi-role/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/collision-same-basename-configured-roots/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/capped-sup/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/access-denied-mp/manifest.json | Updates siteCode to canonical 3-character LAB. |
| crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake/absent-dp/manifest.json | Updates siteCode to canonical 3-character LAB. |
adamgell
left a comment
adamgell
left a comment
There was a problem hiding this comment.
Independent exact-head review: BLOCKED at 58fc0908
This review is anchored to 58fc0908daf529985c434f10a3d8efdd91001672 (base 262a18dedff5934868ed2f89d82795c412935733). The PR advanced to a newer head while this review was concluding, so the fixes require a fresh exact-head review.
Blocking findings
-
The exact-byte test is not cross-platform.
.gitattributesprotects onlysrc-tauri/tests/fixtures/**; the byte-sensitivecrates/cmtraceopen-parser/tests/fixtures/**tree has no attribute. A simulated Git-for-Windows checkout withcore.autocrlf=truechangedMP_GetPolicy.lo_from the manifest's 177 bytes to 178 bytes. The newmetadata().len()assertion therefore fails because of checkout normalization, not fixture content. Add a byte-preserving attribute such as an appropriately scoped-textrule. -
The #335 execution-contract plan remains noncanonical.
docs/superpowers/plans/2026-07-30-sccm-server-intake-and-core.md:24and:103still prescribeCONTOSO, contradicting the correctedLABfixtures and the PR statement that no #335-owned documentation residual remains. Because this plan is the execution contract, it can reintroduce the invalid six-character site code.
Verified at the reviewed SHA
The remaining requested scope passed:
- all 11 scenarios and JSON files are present and internally consistent
MP_GetPolicy.lo_is a physical 100% rename; the old.log.lo_path is absent; manifest/path/provenance and 177 LF bytes agree on this host- role/topology producer and workflow-subject facts remain distinct
- configured non-default provenance is retained and default-path absence does not infer a missing role
- absent/access-denied/capped/skipped/unsupported states remain coverage-only with conservative expected results
- all manifest/expected/evidence paths, IDs, limits, SHA-256, and byte relationships pass the exact-byte and independent consistency audits
- fixtures are synthetic/privacy-safe and make no native Windows/live acceptance claim
- focused contract: 1 passed; full parser: 594 passed; strict Clippy, wasm32,
npx tsc --noEmit, Rust 1.88 formatting, JSON parse, andgit diff --check: PASS
Copilot's EOL-normalization comment is confirmed blocking. CodeRabbit's response remains rate-limited and is not approval.
|
Review corrections are pushed at exact head
Focused test, exact-byte validator, full parser, strict Clippy, wasm32, TypeScript, owned rustfmt, and diff check are green. Fresh independent exact-head review and substantive CodeRabbit review remain required. @coderabbitai review Please review exact head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
✏️ Learnings added
✅ 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 10 minutes. |
adamgell
left a comment
adamgell
left a comment
There was a problem hiding this comment.
Independent exact-head re-review: PASS
No actionable findings at f30e40cd12407243fb4f75c272a9acc4dde3e2fc against base 262a18dedff5934868ed2f89d82795c412935733.
The blockers from the superseded 58fc0908 review are resolved:
.gitattributesnow markscrates/cmtraceopen-parser/tests/fixtures/** -text;git check-attrreportstext: unset.- A controlled
core.autocrlf=truecheckout now preservesMP_GetPolicy.lo_byte-for-byte at 177 bytes (177 == 177 andcmpsucceeds), fixing the Windows checkout failure. - the #335 execution plan, preparation docs, intake corpus, README, and focused test contain no residual
CONTOSOor.log.lo_ - topology host labels and the
LABsite code are documented separately - manifest read/parse failures now include scenario names and underlying errors
Fresh exact-head verification:
- focused server-intake contract: 1 passed
- exact-byte corpus validator: PASS across all 11 scenarios
- JSON parse and independent manifest↔expected↔evidence audit: PASS
- role/topology/configured-path provenance, collision identities, rotation lineage, cap SHA/bytes, and conservative absent/access/skipped/unsupported semantics: PASS
- full parser: 594 passed (349 unit + 222 ESP + 1 client-intake + 1 server-intake + 21 SCCM)
- strict parser Clippy: PASS
- wasm32 parser check: PASS
npx tsc --noEmit: PASS- Rust 1.88 formatting and
git diff --check: PASS - synthetic/privacy-only and no-native/live-acceptance boundaries remain explicit
CodeRabbit is visibly in the exact-head loop and GitHub records an APPROVED review at this SHA. Its accompanying command response also reports the fair-usage limit, so under the program's stricter rule that a rate-limited response is not substantive approval, the coordinator should keep that separate gate pending unless it accepts the actual review object as sufficient.
|
Substantive CodeRabbit gate is now satisfied at exact head Local CodeRabbit CLI 0.7.1 exact committed-range review:
This is the substantive review result; the earlier GitHub fair-usage response was not counted as approval. Independent exact-head review |
Canonicalize the synthetic server-intake site and rollover contract, preserve parser fixture bytes across Windows-style checkouts, and add the focused cross-platform corpus guard. Refs #335.
Summary
Corrects the already-merged #335 preparation corpus to match documented ConfigMgr contracts:
CONTOSOsite codes with exact synthetic three-characterLABacross all 11 server-intake manifestsMP_GetPolicy.log.lo_to replacement-extensionMP_GetPolicy.lo_, updating manifest identity and physical path^[A-Z0-9]{3}$, canonical.lo_identity, manifest-to-file path existence, and byte-count integrityMicrosoft references:
TDD proof
After correcting the test’s expected matrix count to the observed 11 planned scenarios, the focused test failed with 13 fixture defects:
CONTOSOsite codesMP_GetPolicy.log.lo_original basename.log.lo_relative pathIt passes after the smallest fixture/document corrections.
Verification
cargo test --locked -p cmtraceopen-parser --test sccm_server_intake_fixture_contract— 1 passedcargo test --locked -p cmtraceopen-parser --quiet— 594 passed (349 unit + 222 ESP + 1 client-intake + 1 server-intake + 21 SCCM)npx tsc --noEmit— passgit diff --check— passCONTOSOor.log.lo_remains in SCCM Server: add role-aware intake, coverage, and corpus foundation #335-owned intake/docs pathsBoundaries
This is an issue-scoped synthetic-corpus correction only. It does not implement native configured-path discovery/capture, workflow reducers, Windows I/O, or live acceptance. Issue #335 remains open for production native implementation and native Windows/lab validation.
Part of #335.