fix(sccm): map server intake artifact requests - #431
Conversation
📝 WalkthroughWalkthroughSCCM server intake now classifies eligible incomplete artifacts and creates shared catalog requests. Tests cover request metadata, finding serialization, distribution-point identifiers, unsupported sources, and identity-scoped suppression. ChangesSCCM artifact-gap requests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested_head="850c82d0bbbd42ee7f264107d7821c8b344e0f92"
stack_base="4a330621fdb69303ba77d9137d4127a7ba628854"
echo "== checked-out commit =="
git rev-parse HEAD
echo "== requested commit =="
git rev-parse "$requested_head"
echo "== ancestry =="
git merge-base --is-ancestor "$stack_base" "$requested_head" && echo "base-is-ancestor=yes" || echo "base-is-ancestor=no"
echo "== changed paths, stack base..requested head =="
git diff --name-status "$stack_base" "$requested_head"
echo "== diff summary =="
git diff --stat "$stack_base" "$requested_head"
echo "== changed Rust file outlines =="
git diff --name-only "$stack_base" "$requested_head" -- '*.rs' |
while IFS= read -r file; do
echo "--- $file"
ast-grep outline "$file" --items all --view expanded || true
done
echo "== relevant contract symbols and test names =="
rg -n -C 3 -g '*.rs' \
'UndeclaredArtifactRequest|ArtifactRequest|originalBasename|producer.role|producer_role|server_intake|sccm_server_intake' \
.Length of output: 213873 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="4a330621fdb69303ba77d9137d4127a7ba628854"
head="850c82d0bbbd42ee7f264107d7821c8b344e0f92"
echo "== exact diff =="
git diff --find-renames --find-copies --unified=80 "$base" "$head" -- \
crates/cmtraceopen-parser/src/sccm/server/windows/intake.rs \
crates/cmtraceopen-parser/tests/sccm_server_intake.rs
echo "== intake assessment request collection and request_for_gap =="
sed -n '200,305p' crates/cmtraceopen-parser/src/sccm/server/windows/intake.rs
sed -n '860,930p' crates/cmtraceopen-parser/src/sccm/server/windows/intake.rs
echo "== server source classification =="
sed -n '1,160p' crates/cmtraceopen-parser/src/sccm/server/windows/catalog.rs
echo "== shared catalog classifier and declared entries =="
rg -n -C 4 -g '*.rs' \
'fn classify_artifact_name|pub fn classify_artifact_name|struct SccmArtifactClassification|declared_source_catalog' \
crates/cmtraceopen-parser/src/sccmLength of output: 40648 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== shared classifier implementation and relevant catalog entries =="
sed -n '401,460p' crates/cmtraceopen-parser/src/sccm/catalog.rs
rg -n -C 5 -i \
'distmgr\.log|mp_getpolicy\.log|wsyncmgr\.log|mpcontrol\.log|smsdpprov\.log|pullDp' \
crates/cmtraceopen-parser/src/sccm/catalog.rs
echo "== server intake fixture gap declarations =="
fd --type f --glob 'manifest.json' crates/cmtraceopen-parser/tests/fixtures/sccm/server/intake \
--exec sh -c '
for file do
if grep -Eq "\"captureState\"[[:space:]]*:[[:space:]]*\"(absent|accessDenied|capped|parseFailed)\"" "$file"; then
echo "--- $file"
jq -r ".artifacts[] | select(.captureState == \"absent\" or .captureState == \"accessDenied\" or .captureState == \"capped\" or .captureState == \"parseFailed\") | [.artifactId, .sourceId, .sourceKind, .producerRole, (.workflowSubject.role // \"-\"), .originalBasename, .captureState] | `@tsv`" "$file"
fi
done
' sh {} +
echo "== request_for_gap references =="
rg -n -C 3 '\brequest_for_gap\s*\(' crates/cmtraceopen-parser/src crates/cmtraceopen-parser/tests
echo "== whitespace and working tree =="
git diff --check 4a330621fdb69303ba77d9137d4127a7ba628854 850c82d0bbbd42ee7f264107d7821c8b344e0f92
git status --shortLength of output: 6273
I found no actionable issues. The change maps each coverage gap to the shared catalog artifact selected by ✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR refines SCCM server-intake “coverage gap” artifact requests so they map from server-group coverage entries to the exact shared SCCM source-catalog artifact (selected by originalBasename + producer role), while preserving producer-role provenance and ensuring requests/reasons survive finding validation plus JSON serialization boundaries.
Changes:
- Update server-intake gap request generation to derive
logical_id,role, and a bounded catalog-scopedreasonfromclassify_artifact_name(...), and suppress requests for unknown/non-CCM sources. - Add focused tests proving DP/MP/SUP gap requests now emit declared shared-catalog logical IDs and reasons that pass finding build + serialize/deserialize.
- Adjust existing DP absent-case expectation from server-group ID to the shared catalog logical name (
distmgr).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/cmtraceopen-parser/tests/sccm_server_intake.rs | Adds regression tests and a helper to ensure emitted next-artifact requests validate through finding + JSON boundaries; updates expected logical ID for DP absent case. |
| crates/cmtraceopen-parser/src/sccm/server/windows/intake.rs | Reworks request_for_gap to emit catalog-declared artifact requests (logical name + producer role) with bounded per-artifact reasons and to suppress non-CCM/unknown requests. |
|
Independent root review at exact head 850c82d found no additional issue in the two-file feature diff. Focused server intake is 18/18 green in a clean detached review worktree. I also overlaid blocked PR #404 with a no-commit merge to test the actual dependency boundary: the merge was conflict-free, server intake stayed 18/18, and the hardened spine suite was 151/151 green. The review worktree was then returned clean to exact 850c82d. This is not a merge approval yet: PR #431 remains stacked on draft PR #430, exact-head Copilot completion and hosted checks are still pending, and neither PR is being treated as native Windows acceptance. |
|
Superseded: the dedicated requested-reviewers API is now being used for review-only. The coding-agent change prompted by the earlier mention was reverted because it touched files outside this PR scope. |
Reviewed the two-file diff at 850c82d. The One issue found: |
…le_match" This reverts commit 58d0feb.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
crates/cmtraceopen-parser/src/sccm/server/windows/intake.rs:899
usable_compatible_candidateis derived fromlogical_source_key, but that key currently ignoresproducer_host_handleandworkflow_subject_handle. With the provenance scoping work (stacked on #430), a captured artifact on one host/subject can incorrectly suppress the default-candidate request for an absent artifact on a different host/subject that happens to share the same(producer_role, source_id, workflow_subject_role).
Consider including the host/subject handles in logical_source_key so request suppression stays scoped to the same provenance dimensions as artifact identity.
&& usable_compatible_candidate
{
return None;
}
if !matches!(
|
@coderabbitai full review |
|
Exact-head review checkpoint for |
✅ Action performedFull review finished. |
Scope
Dependency state
PR #430 is merged into
codex/parser-family-skeletonas2d3c2b0e9ad21152ff76c5251066f7fb4e3b17b3; this PR is retargeted to that integration branch.A Copilot coding-agent commit that touched unrelated #331/#332 fixture tests was independently inspected and reverted. Review-only Copilot was subsequently requested through the requested-reviewers API.
Refs #335
Epic #317
TDD evidence
Observed RED before implementation:
server-dp-distribution;SccmFindingBuilderrejected it as an undeclared artifact request;GREEN at exact head
46a7d06efd66cf989756c5a9989e896f78f70079:Verification
cmtraceopen-parser: passedwasm32-unknown-unknowncheck: passedgit diff --check: passedSynthetic fixtures only. Native server discovery/capture and live Windows acceptance are not included or claimed; issue #335 remains open.
Summary by CodeRabbit
Bug Fixes
Tests