Repository navigation
Add verbose Learning Mode diagnostics - #1083
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The SDK path regresses non-blocking polling and can suppress or silently discard verbose telemetry failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds bounded, sanitized Learning Mode diagnostics, publishes them beside actionable denials, and transports them through consent-gated telemetry. It also updates telemetry consent wording and renews consent via resource version 3.
Changes:
- Adds verbose denial aggregation, paired artifact handling, and ETW chunking.
- Extends Windows ETL decoding, filtering, and capture finalization.
- Updates consent resources, SDK documentation, and tests.
File summaries
| File | Description |
|---|---|
tests/scripts/run_telemetry_etw_smoke_test.ps1 |
Updates consent expectations. |
tests/scripts/run_telemetry_consent_smoke_test.ps1 |
Updates consent smoke assertions. |
src/testing/wxc_e2e_tests/tests/e2e_telemetry_etw.rs |
Seeds consent version 3. |
src/mxc_telemetry/src/lib.rs |
Adds the verbose-denials ETW event. |
src/host/plm/src/stop.rs |
Handles verbose-output path collisions. |
src/host/plm/src/log.rs |
Updates analysis fixture. |
src/host/plm/src/elevated.rs |
Updates actionable terminology. |
src/host/plm/src/analysis.rs |
Writes paired denial artifacts. |
src/host/plm/readme.md |
Updates PLM guidance. |
src/core/wxc/src/main.rs |
Emits verbose telemetry and cleanup warnings. |
src/core/wxc/src/audit.rs |
Relocates and validates artifact pairs. |
src/core/wxc_common/src/telemetry/mod.rs |
Adds authorized verbose emission. |
src/core/wxc_common/src/telemetry/events.rs |
Defines verbose telemetry records. |
src/core/wxc_common/src/telemetry/consent.rs |
Uses the canonical resource version. |
src/core/wxc_common/src/telemetry/consent_prompt.rs |
Updates consent prompt tests. |
src/core/wxc_common/resources/telemetry/consent/en-US.json |
Introduces consent resource version 3. |
src/core/mxc_engine/src/verbose_telemetry.rs |
Validates, chunks, and emits verbose documents. |
src/core/mxc_engine/src/run.rs |
Adds audit runner resolution. |
src/core/mxc_engine/src/lib.rs |
Integrates verbose SDK telemetry. |
src/core/mxc_engine/Cargo.toml |
Adds hashing and randomness dependencies. |
src/core/learning_mode_core/src/verbose_logging.rs |
Defines bounded verbose diagnostics. |
src/core/learning_mode_core/src/model.rs |
Makes classifications orderable. |
src/core/learning_mode_core/src/lib.rs |
Exports verbose and paired-output APIs. |
src/core/learning_mode_core/src/analyze.rs |
Adds bounded verbose results. |
src/core/learning_mode_core/Cargo.toml |
Adds output-management dependencies. |
src/Cargo.toml |
Adds the shared same-file dependency. |
src/Cargo.lock |
Locks new dependencies. |
src/backends/learning_mode/windows/src/ui.rs |
Updates actionable terminology. |
src/backends/learning_mode/windows/src/tdh_decode.rs |
Adds typed and partial property decoding. |
src/backends/learning_mode/windows/src/etl_filter.rs |
Preserves known-provider events safely. |
src/backends/learning_mode/windows/src/capability_dacl.rs |
Adds sanitized verbose metadata. |
src/backends/learning_mode/windows/Cargo.toml |
Adds hashing support. |
src/backends/appcontainer/common/src/guarded_capture.rs |
Updates paired-output terminology. |
src/backends/appcontainer/common/src/capture_output.rs |
Publishes actionable and verbose artifacts together. |
sdk/node/tests/unit/telemetry.test.ts |
Updates consent title fixture. |
sdk/node/tests/unit/default-consent-protocol-runner.test.ts |
Updates protocol fixture title. |
sdk/node/README.md |
Documents verbose telemetry. |
sdk/dotnet/README.md |
Documents verbose telemetry. |
sdk/dotnet/Microsoft.Mxc.Sdk.Tests/MxcTelemetryTests.cs |
Updates consent expectations. |
README.md |
Documents audit and telemetry changes. |
docs/telemetry/telemetry.md |
Documents the new event and inventory. |
docs/telemetry/telemetry-consent-design.md |
Updates consent disclosure and version. |
docs/process-container/os-version-support.md |
Updates actionable terminology. |
docs/learning-mode/capabilities.md |
Documents verbose artifacts and classifications. |
.github/copilot-instructions.md |
Updates Learning Mode architecture guidance. |
Review details
Suppressed comments (1)
docs/telemetry/telemetry.md:309
- This second review-status statement still calls the consent wording version 2 and says its privacy review is pending, although the resource is version 3 and the PR identifies the disclosure as privacy-reviewed. Remove the consent wording from the pending list while retaining any genuinely pending verbose-event reviews.
The version 2 consent wording, verbose event inventory, WinExt
classification, retention, access, regional processing, deletion, and
localization/accessibility decisions require explicit privacy review.
- Files reviewed: 47/48 changed files
- Comments generated: 8
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Cross-platform compilation is broken and the telemetry payload can expose unreviewed workload-controlled values.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 48/49 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The telemetry projection can emit unvalidated strings, and some new failure diagnostics remain inaccessible or silently discarded.
Review details
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
src/core/learning_mode_core/src/verbose_logging.rs:155
- When
total_occurrencesis alreadyu64::MAX,record_overflowsaturates it atMAXand the following subtraction reduces it toMAX - 1. Recording another event must never decrease the total; update only the overflow counters here because the total was already incremented before the match.
src/core/mxc-sdk/src/sandbox.rs:178 wait()can both return an error and append the new verbose-telemetry warning (the added engine test constructs exactly this case), but?consumes and dropsselfbefore either warnings or finalized metadata are returned. Consequentlymxc_sdk::runand FFI/SDK callers still cannot diagnose an ETW/artifact failure when backend teardown also fails. Preserve these diagnostics in the error path or return a result type that carries them.
src/core/wxc/src/audit.rs:225- If the JSON pair commits but source cleanup produces warnings, and the subsequent ETL move fails, this
?discards those warnings. The operator then sees only the ETL error and is not told that stale actionable/verbose source files may remain. Include accumulated cleanup warnings in the returned error on this branch.
src/core/mxc_engine/src/verbose_telemetry.rs:153
- Only property values are redacted. This artifact is reopened from a caller-selected path and deserialized without any producer identity/integrity check, so a concurrent replacement can place customer content in
propertiesnames orprovider_guid; both strings survive this projection and are uploaded inmxc.content. Build a closed telemetry projection (for example, derive the GUID from the provider enum and allowlist/drop property names), or bind the reopened file to the artifact produced during teardown.
- Files reviewed: 48/49 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Copilot's suppressed review also identified three issues in inherited base-PR code that are intentionally not addressed by this stacked PR:
These findings belong to the underlying telemetry work in #822 (or a follow-up from that branch). This PR remains scoped to the new |
e4311f4 to
7954302
Compare
Emit sanitized verbose capture artifacts as bounded, independently parseable MXC.Verbose chunks across executor and SDK completion paths. Version the consent disclosure and document the reconstruction and privacy contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e408b60b-e267-416c-806d-0a7e119fe53f
Propagate EventWriteTransfer status codes through the verbose telemetry path so rejected chunks produce a bounded local diagnostic instead of failing silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e32b4c9-eb47-4231-91de-0da5fde17ab7
Use MXC.VerboseDenials as the ETW identity so the event's Learning Mode denial payload is clear to telemetry consumers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9e32b4c9-eb47-4231-91de-0da5fde17ab7
Apply the privacy-reviewed Microsoft Products disclosure and bump the consent resource version so existing grants require renewed consent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ba4f8739-616b-4b42-8eba-24a762602400
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ba4f8739-616b-4b42-8eba-24a762602400
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ba4f8739-616b-4b42-8eba-24a762602400
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ba4f8739-616b-4b42-8eba-24a762602400
Preserve the nonblocking try_wait contract and ensure the verbose ETW test uses the current schema and effective consent override. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e408b60b-e267-416c-806d-0a7e119fe53f
86050ca to
121ae2e
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e408b60b-e267-416c-806d-0a7e119fe53f
📖 Description
Adds bounded, sanitized verbose Learning Mode diagnostics alongside the
actionable denial artifact. Non-actionable and unsupported events retain a
closed exclusion reason for investigation without changing policy
recommendations. Telemetry reports verbose-artifact production and surfaces ETW
write failures locally without changing the sandbox result.
Also applies the privacy-reviewed Microsoft Products consent disclosure and
bumps the consent resource version to require renewed consent for the material
wording change.
This branch has been rebuilt directly on
main. It no longer carries ordepends on the open SDK telemetry changes in #821 and #822.
Deferred integration gaps
MxcTelemetryTests.csconsent-resource assertion remains in Expose telemetry through Rust SDK, C ABI, and .NET binding #821/Add Node.js and .NET telemetry SDKs #822because that .NET telemetry test/API surface is not on
main.default-consent-protocol-runner.test.tsandtelemetry.test.tswording snapshots remain in Add Node.js and .NET telemetry SDKs #822 for the same reason.
try_waitemitsnormal completion telemetry but cannot emit
MXC.VerboseDenials, becausecapture metadata is finalized during
wait. This limitation is documentedin
docs/telemetry/telemetry.md.🔗 References
Related PRs:
🔍 Validation
cargo fmt --all -- --checkcargo test -p wxc_common telemetrycargo test -p mxc_telemetrycargo test -p mxc-sdk sandbox_and_output_expose_security_warningscargo test -p mxc_enginecargo test -p wxc_e2e_tests --test e2e_telemetry_etw test_verbose_denials_etw_payload_honors_consent -- --ignored --nocapturecargo clippy -p mxc_engine -p wxc_common -p mxc_telemetry -p mxc-sdk -p wxc_e2e_tests --all-targets -- -D warningsMXC.VerboseDenialsevents confirmed in TRTT✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(
MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.