Add opt-in telemetry consent with an administrative policy ceiling

[RamonArjona4]

📖 Description

Adds an opt-in telemetry consent system, so consumers of the MXC SDKs can offer
their users a genuine choice about telemetry, plus an administrative policy that
Intune, another MDM, or Group Policy can use to restrict collection.

This is a proof of concept for the consent and policy plumbing. The
underlying TraceLogging/ETW telemetry remains an experimental feature, so
collection still additionally requires --experimental and an
experimental.telemetry config block.

Telemetry is Windows-only and is never collected without explicit consent.
MXC keeps its own consent state and never reads, or infers from, the Windows
system telemetry consent. Non-Windows platforms report telemetry as
not-applicable and expose no consent surface at all.

How collection is gated

Three independent conditions must all hold before a single event is emitted.
They are combined by one conjunction in wxc_common::telemetry::is_enabled, so
there is exactly one place in the codebase where "is telemetry on?" is decided.

Condition	Source	Default
The user granted consent	per-user JSON store under %LOCALAPPDATA%	undetermined (off)
Administrative policy permits it	HKLM\SOFTWARE\Policies\Mxc\AllowTelemetry	absent (unrestricted)
The config kill-switch is unset	experimental.telemetry	off

Every error path fails closed. An unreadable consent store, an unreadable,
malformed, or wrongly-typed policy value, or a missing experimental.telemetry
block all resolve to "telemetry off".

Consent

• The user is prompted the first time they run the sandbox.
• They can opt in or opt out at any time afterwards, through whichever SDK the
  consuming agent already uses. Consent is persisted per user, so the prompt is
  not repeated once answered.

Administrative policy

HKLM\SOFTWARE\Policies\Mxc\AllowTelemetry (REG_DWORD) mirrors the numeric
scale Windows already uses for its own AllowTelemetry policy:

Value	Meaning for MXC
0 (Security)	MXC emits nothing
1 (Required)	MXC emits nothing — MXC data is optional-tier
3 (Optional)	MXC may emit, if the user consented
absent	unrestricted — consent alone decides
any other value, wrong type, or unreadable	blocked

Two deliberate design points are worth calling out for review:

• The policy is deny-only; it can never opt a user in. If the user opted out
  and policy permits telemetry, the result is opt-out. A policy may only
  restrict what the user already permitted.
• This is an MXC-specific policy. We deliberately do not read the Windows
  AllowTelemetry policy. That matches its documented scope (it "doesn't apply
  to any additional apps installed by your organization") and the pattern used
  by Office, VS Code, Visual Studio, and WinGet.

The key sits at SOFTWARE\Policies\Mxc rather than the more conventional
SOFTWARE\Policies\Microsoft\Mxc on purpose: Windows forbids ADMX-ingested
policies from writing under Software\Policies\Microsoft outside a fixed
allowlist that MXC cannot join. Staying out from under that prefix is what makes
the shipped ADMX ingestible by Intune.

One implementation, five surfaces

The consent and policy logic is defined once, in Rust, and surfaced
identically through wxc-exec (CLI, JSON output), the Rust mxc-sdk, the
mxc_ffi C ABI, the C# SDK, and the Node/TypeScript SDK. No consent or policy
decision is re-implemented in any binding, and a parity script guards the state
enums against drift.

Read-only queries never throw and can never crash the hosting application.
Failures that must be swallowed are reported once per process rather than hidden.

🔗 References

No existing issue is resolved by this PR. Follow-up work filed from it, which
this PR intentionally does not close:

• Share the telemetry-consent test harness between wxc_common and mxc_ffi #690 — share the telemetry-consent test harness between wxc_common and mxc_ffi
• Telemetry consent path has no automated coverage in release builds #691 — the consent/policy paths have automated coverage only in debug builds

Background: #492 added the TraceLogging telemetry that this change gates.

Docs added here:

• docs/telemetry/telemetry-consent-design.md — design, rationale, and the resolved decisions behind the behaviour above
• docs/telemetry/telemetry-policy.md — administrator-facing reference, ADMX sample, and Intune import steps

🔍 Validation

Automated:

• cargo test -p wxc_common -- telemetry — 100 passed, covering consent
  persistence, every policy value, all fail-closed paths, and the full
  consent × policy matrix
• cargo test --workspace — no failures
• cargo clippy --workspace --all-targets -- -D warnings and
  cargo fmt --all -- --check — clean
• npm test (Node SDK) — 226 tests, 0 failures, including the new
  telemetry.test.ts
• New script scripts/check-telemetry-policy-parity.js — asserts the four
  policy states match across Rust, C#, and TypeScript
• scripts/check-dotnet-bindings-codegen.js now also asserts the four new
  telemetry FFI entry points

Both scripts pass locally and were negative-tested, but neither is wired into
a workflow by this PR — see the note below.

Manual:

• tests/scripts/run_telemetry_consent_smoke_test.ps1 drives first-run prompt,
  opt-in, opt-out, and policy override end to end against wxc-exec.exe.
• Each new check was negative-tested by deliberately breaking the invariant it
  guards and confirming it fails.

CI wiring is deliberately not included. Workflow files are owned by the
build maintainers, so this PR touches no .github/workflows/ file. The two
parity/codegen checks above therefore run only on demand until someone with
build ownership wires them in. Recommendations for that follow-up have been
written up and will be shared with the build owners separately.

Known and intentional: dotnet test -c Release fails, because the test-only
overrides are compiled out of release builds so a release binary can never be
spoofed into believing policy permits collection. This is documented in
sdk/dotnet/README.md and tracked by #691.

Cargo.lock changes by exactly one line — mxc_ffi gaining a dependency on the
in-workspace wxc_common crate. No third-party dependency is added, so
dependency-feed-check has nothing new to resolve.

✅ Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• Updated documentation (if applicable)
• Updated Copilot instructions (if build, architecture, or conventions changed)
• If this PR changes Cargo.lock, the dependency-feed-check check passes (see docs/pull-requests.md)

📋 Issue Type

• Bug fix
• Feature
• Task

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Pull request overview

Adds Windows-only, opt-in telemetry consent with a deny-only administrative policy ceiling, distributed through MXC’s CLI, Rust, FFI, Node, and .NET surfaces.

Changes:

• Adds persisted consent and administrative policy enforcement to wxc_common.
• Exposes fail-closed consent APIs across all supported SDK surfaces.
• Adds tests, documentation, parity checks, and build integration.

Reviewed changes

Copilot reviewed 33 out of 34 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
.github/copilot-instructions.md	Documents telemetry conventions and testing.
docs/telemetry/telemetry-consent-design.md	Describes the consent architecture.
docs/telemetry/telemetry-policy.md	Documents administrator policy deployment.
docs/telemetry/telemetry.md	Adds consent and policy gating details.
scripts/check-dotnet-bindings-codegen.js	Checks new telemetry FFI exports.
scripts/check-telemetry-policy-parity.js	Validates policy-state parity.
sdk/dotnet/README.md	Documents the .NET consent API.
sdk/dotnet/Microsoft.Mxc.Sdk.Tests/MxcTelemetryTests.cs	Tests .NET consent and policy behavior.
sdk/dotnet/Microsoft.Mxc.Sdk/ErrorCode.cs	Adds consent-write failure status.
sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj	Builds matching native profiles.
sdk/dotnet/Microsoft.Mxc.Sdk/MxcException.cs	Preserves wrapped native failures.
sdk/dotnet/Microsoft.Mxc.Sdk/MxcTelemetry.cs	Implements the .NET telemetry API.
sdk/dotnet/Microsoft.Mxc.Sdk/Native/NativeLibraryResolver.cs	Aligns native resolution with build profile.
sdk/dotnet/Microsoft.Mxc.Sdk/TelemetryConsentState.cs	Defines .NET consent states.
sdk/dotnet/Microsoft.Mxc.Sdk/TelemetryPolicyState.cs	Defines .NET policy states.
sdk/node/README.md	Documents Node telemetry consent.
sdk/node/package.json	Adds telemetry unit tests.
sdk/node/src/index.ts	Exports telemetry APIs and types.
sdk/node/src/telemetry.ts	Implements Node consent operations.
sdk/node/tests/unit/telemetry.test.ts	Tests Node fail-closed behavior.
src/Cargo.lock	Records the FFI foundation dependency.
src/core/lxc/src/main.rs	Adds Linux consent CLI parity.
src/core/mxc_darwin/src/main.rs	Adds macOS consent CLI parity.
src/core/mxc-sdk/README.md	Documents Rust SDK consent.
src/core/mxc-sdk/src/lib.rs	Exposes Rust telemetry APIs.
src/core/wxc/src/main.rs	Adds Windows consent CLI commands.
src/core/wxc_common/Cargo.toml	Adds telemetry test support.
src/core/wxc_common/src/telemetry/consent.rs	Implements persisted consent storage.
src/core/wxc_common/src/telemetry/consent_cli.rs	Shares consent CLI handling.
src/core/wxc_common/src/telemetry/mod.rs	Combines consent, policy, and config gates.
src/core/wxc_common/src/telemetry/policy.rs	Implements the Windows policy ceiling.
src/ffi/mxc_ffi/Cargo.toml	Adds telemetry dependencies and test support.
src/ffi/mxc_ffi/src/lib.rs	Adds telemetry C ABI exports.
tests/scripts/run_telemetry_consent_smoke_test.ps1	Adds Windows CLI smoke coverage.

Comments suppressed due to low confidence (1)

src/ffi/mxc_ffi/src/lib.rs:464

• This eprintln! is outside the catch_unwind. If persisting consent fails while the embedding host has a closed/broken stderr pipe, the diagnostic write can panic across the extern "C" boundary and abort the host instead of returning MXC_STATUS_CONSENT_WRITE_FAILED. Emit this diagnostic with a fallible write whose error is ignored.

[Copilot]

Pull request overview

Copilot reviewed 34 out of 35 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (8)

sdk/dotnet/Microsoft.Mxc.Sdk/MxcTelemetry.cs:264

• An unknown native policy string is silently collapsed to Blocked here. That is fail-closed, but it hides the mismatched/corrupt native response despite this PR's requirement to report swallowed failures once. Make parsing expose recognition failure and invoke ReportFailClosed before returning Blocked.

                    var value = outUtf8 is null ? null : Marshal.PtrToStringUTF8((IntPtr)outUtf8);
                    return ParsePolicyState(value);

src/ffi/mxc_ffi/src/lib.rs:719

• The telemetry tests only set this debug-only override; they never prove that the native code honors it. On Windows, cargo test -p mxc_ffi --release compiles the override lookup out, after which the tests at lines 769, 795, and 821 write to the developer's real %LOCALAPPDATA% consent record. Gate the writing tests to debug builds or add the same read-only two-state verification used by the C# fixture/smoke test and abort before any native write.
  src/core/wxc_common/src/telemetry/mod.rs:177
• This changes enabled: None into an enabled state once consent and policy permit it, but the wire contract still says omission is disabled (wire.rs:571-573, also emitted into the dev schema), and models.rs:730-732 says the same. Thus experimental.telemetry: {} can now collect contrary to the published schema. Update the wire/domain documentation and regenerate the dev schema so the config contract matches this intended kill-switch behavior.
  sdk/dotnet/Microsoft.Mxc.Sdk/Native/NativeLibraryResolver.cs:59
• This logging call can itself throw (for example after a host replaces or closes Console.Error). Because Initialize runs in a static constructor, that exception permanently poisons MxcTelemetry, defeating the documented never-throw behavior that this catch is meant to preserve. Make this best-effort with a nested try/catch, as ReportFailClosed already does.

            Console.Error.WriteLine(
                $"mxc: could not register the native library resolver ({ex.GetType().Name}: {ex.Message}). " +
                "Falling back to the default loader.");

sdk/dotnet/Microsoft.Mxc.Sdk/MxcTelemetry.cs:41

• The summary says this method “Always succeeds,” but the implementation deliberately throws MxcException for a genuine non-success native status and wraps unexpected marshalling failures. Document that distinction here so callers do not rely on a false no-throw contract.

    /// <summary>
    /// Read the persisted telemetry consent state. Always succeeds — never
    /// throws for "no decision yet" or "not on Windows"; both are ordinary
    /// return values (<see cref="TelemetryConsentState.Undetermined"/> and
    /// <see cref="TelemetryConsentState.NotApplicable"/> respectively).

sdk/dotnet/Microsoft.Mxc.Sdk/MxcTelemetry.cs:72

• ParseConsentState silently maps any unknown or malformed native string to Undetermined, so a mismatched/corrupt binding is indistinguishable from a genuine undecided user and no deduplicated diagnostic is emitted. Have parsing report whether the value was recognized, then call ReportFailClosed before returning the safe fallback.

This issue also appears on line 263 of the same file.

                    var value = outUtf8 is null ? null : Marshal.PtrToStringUTF8((IntPtr)outUtf8);
                    return ParseConsentState(value);

src/ffi/mxc_ffi/Cargo.toml:17

• This new production dependency bypasses the public mxc-sdk facade even though mxc-sdk::telemetry exposes the same operations. The rest of this crate consistently wraps mxc_sdk (lib.rs:54, streaming.rs:57, state_aware.rs:25), and mxc-sdk explicitly owns facade types so consumers remain decoupled from wxc_common. Route production calls through mxc_sdk::telemetry; keep the feature-enabled wxc_common entry only as a dev-dependency for the test redirector.
  src/core/wxc_common/src/telemetry/policy.rs:177
• This non-NotFound registry failure is collapsed to Unreadable and then Blocked with no diagnostic. That keeps collection off, but an ACL failure becomes indistinguishable from an intentional block and contradicts the documented “every swallowed failure is reported once” behavior; bindings only receive the valid string blocked, so they cannot report it later. Emit a deduplicated, non-throwing diagnostic here while preserving the fail-closed state (and do the same for value-read failures below).

[Copilot]

Pull request overview

Copilot reviewed 37 out of 39 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (11)

docs/telemetry/telemetry-consent-design.md:375

• This section says the Rust SDK is a pure re-export of wxc_common types, but the implementation deliberately defines crate-owned ConsentState and PolicyState facades. Update the description and example so maintainers do not bypass the SDK boundary based on stale architecture documentation.

`mxc-sdk` — the public Rust SDK — re-exports the consent and policy API
verbatim from `wxc_common::telemetry`, so a Rust consumer gets the same
operations as a Node or C# consumer:

```rust
pub mod telemetry {
    pub use wxc_common::telemetry::consent::{
        get_consent, needs_consent_prompt, set_consent, ConsentState,

src/core/wxc_common/src/telemetry/consent.rs:199

• Caching Option<PathBuf> permanently caches a transient resolution failure. If the token/registry/Shell call fails once in a long-lived SDK host, every later consent read and write remains unavailable until process restart even after the system recovers. Cache only successful paths and retry after None.
  src/core/wxc_common/src/telemetry/consent.rs:220
• SHGetKnownFolderPath requires a non-null token to have TOKEN_QUERY | TOKEN_IMPERSONATE, but this handle requests only TOKEN_QUERY. On systems enforcing that documented contract, LocalAppData resolution fails, so consent always reads as Undetermined and writes fail. Open the process token with the required rights.
  src/core/wxc_common/src/telemetry/consent.rs:358
• These read and parse failures are silently collapsed into Undetermined. That contradicts the PR's guarantee that swallowed fail-closed failures are reported once, and downstream SDKs cannot distinguish this from a genuinely fresh store, so no later layer can report it. Add a deduplicated, non-panicking diagnostic for non-NotFound I/O and parse failures.
  src/core/wxc_common/src/telemetry/policy.rs:159
• PolicyValue::Unreadable discards the registry error and silently returns Blocked. This violates the stated once-per-process reporting guarantee, and bindings see an ordinary blocked policy so they cannot diagnose the failure later. Preserve and report the underlying open/read error through a deduplicated, non-panicking diagnostic path.
  src/core/wxc_common/src/telemetry/consent_cli.rs:119
• --telemetry-consent-source is ignored unless another consent action is present: source-only execution returns None and can continue into a normal sandbox run, while status+source silently discards the value. Since this option is documented only as provenance for grant/revoke, reject it with EX_USAGE when neither write action is set.
  src/core/wxc/src/main.rs:836
• This is not the cheapest fast path: recover_orphaned_state() already ran above. Because the Node SDK invokes this CLI for each consent query, a read-only status call can unexpectedly mutate DACL state and can exceed the SDK's 5-second timeout when recovery is slow, yielding a false fail-closed result. Move consent handling immediately after CLI parsing, before force-reclaim and DACL recovery.

    // --telemetry-consent-{status,grant,revoke}: administer the persisted
    // consent flag and exit. Run before --probe (cheapest possible fast
    // path — no config parsing, no policy defaults needed).
    if handle_telemetry_consent_flags(&cli) {

src/core/mxc-sdk/src/telemetry.rs:188

• This new public Rust SDK operation exposes Result<(), String>, unlike the SDK's typed Error/ErrorCode facade (src/core/mxc_engine/src/error.rs:72-88). Consumers cannot reliably match consent-write failures, and changing this after release would break the API. Return an SDK-owned error type or add a stable consent-write error variant before shipping.

/// Record the user's decision. Returns `Err` on a non-Windows host, and on
/// Windows if the decision could not be persisted — a caller must not treat a
/// failed write as consent.
pub fn set_consent(granted: bool, source: &str) -> Result<(), String> {
    inner_consent::set_consent(granted, source)

src/ffi/mxc_ffi/Cargo.toml:17

• The runtime dependency bypasses the repository's stated boundary: mxc_ffi is a C ABI over mxc_sdk (src/ffi/mxc_ffi/src/lib.rs:4), and this PR already adds crate-owned telemetry facades in mxc_sdk. Route the four exports through mxc_sdk::telemetry and keep wxc_common only as a dev-dependency for the test guard; otherwise the FFI layer is coupled directly to foundation internals.
  scripts/check-telemetry-policy-parity.js:89
• This check rejects the implementation in this PR: ParsePolicyState uses _ => UnrecognizedPolicyState(value), not _ => TelemetryPolicyState.Blocked, so csharpDefault is always false and the supposedly passing parity script exits with an error. Accept the helper form only after verifying that helper returns Blocked.

const csharpDefault = /_\s*=>\s*TelemetryPolicyState\.Blocked/.test(
  parseBody[1]
);
if (!csharpDefault) {
  errors.push(
    "C# ParsePolicyState must fail closed with `_ => TelemetryPolicyState.Blocked`"
  );
}
// The default arm covers "blocked", so treat it as handled.
csharpStates.add("blocked");

docs/telemetry/telemetry-consent-design.md:139

• The formula says an omitted enabled field permits collection (None != Some(false)), but the implementation and surrounding text require an explicit Some(true). Correct the formula so this privacy gate is documented unambiguously.

This issue also appears on line 368 of the same file.

         && request.experimental.telemetry.enabled != Some(false)

[Copilot]

Pull request overview

Copilot reviewed 37 out of 39 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (11)

docs/telemetry/telemetry-consent-design.md:419

• The implementation checklist repeats the obsolete “pure re-export” architecture even though the SDK now owns facade enums in src/core/mxc-sdk/src/telemetry.rs. Update this row to match the implemented boundary.

| `src/core/mxc-sdk/src/lib.rs` | `pub mod telemetry` re-exporting `get_consent` / `set_consent` / `needs_consent_prompt` / `ConsentState` from `wxc_common`, so the public **Rust** SDK offers the same consent surface as the Node and C# SDKs. Pure re-export — no Rust-SDK-specific logic. | ✅ Done |

scripts/check-telemetry-policy-parity.js:89

• This check currently fails against the C# implementation it is meant to validate: ParsePolicyState delegates its default arm to UnrecognizedPolicyState(value), while this regex only accepts a literal TelemetryPolicyState.Blocked. The script therefore reports an error even though that helper returns Blocked. Validate both the delegating arm and its fail-closed helper (while retaining the explicit "blocked" state check).

const csharpDefault = /_\s*=>\s*TelemetryPolicyState\.Blocked/.test(
  parseBody[1]
);

src/core/wxc_common/src/telemetry/consent.rs:373

• These branches silently collapse both an unreadable store and malformed JSON into an ordinary Undetermined result. Because the FFI/SDK receives a successful state, no outer reporter can diagnose the failure. This contradicts the PR's guarantee that swallowed failures are reported once per process (and the design document's statement that corrupt records are logged). Keep a missing file quiet, but report and deduplicate other read/parse failures before failing closed.
  src/core/wxc_common/src/telemetry/policy.rs:178
• This discarded error (and the analogous get_value error below) makes an unreadable policy indistinguishable from a legitimate administrative block to every outer surface, so the failure is never reported. The PR promises that fail-closed failures are diagnosed once per process; preserve Blocked, but pass the error to a deduplicated, non-throwing reporter.
  src/core/wxc_common/src/telemetry/consent_cli.rs:119
• --telemetry-consent-source by itself is accepted by clap but reaches this branch and returns None, so the executor continues into normal config parsing and can even run a sandbox if config was also supplied. Since this option is documented only as provenance for grant/revoke, treat the source-only form as EX_USAGE instead of silently ignoring it.
  src/core/mxc-sdk/src/telemetry.rs:189
• This new public SDK operation exposes a raw String error, unlike the crate's other public fallible operations, which return the typed Error/ErrorCode facade (src/core/mxc-sdk/src/lib.rs:100,118,136,146). Callers cannot reliably distinguish a consent persistence failure without parsing text, even though the C# surface already exposes ConsentWriteFailed. Return a crate-owned typed error (or a dedicated consent error) and map the internal message into it.

/// Record the user's decision. Returns `Err` on a non-Windows host, and on
/// Windows if the decision could not be persisted — a caller must not treat a
/// failed write as consent.
pub fn set_consent(granted: bool, source: &str) -> Result<(), String> {
    inner_consent::set_consent(granted, source)
}

docs/telemetry/telemetry-consent-design.md:385

• This section still describes the Rust SDK as a pure re-export, but the current implementation deliberately added SDK-owned ConsentState/PolicyState facades in src/core/mxc-sdk/src/telemetry.rs. Update the design record so it no longer documents the architecture that the earlier review fix removed.

This issue also appears on line 419 of the same file.

This is a **pure re-export** — there is deliberately no Rust-SDK-specific
consent logic to keep in sync. A Rust host calls `needs_consent_prompt()`
at first sandbox run, shows its own UI, then calls `set_consent(..)`, and
can call `get_consent()`/`set_consent(..)` from a settings surface later —
exactly the flow described in §8.

src/ffi/mxc_ffi/Cargo.toml:17

• The repository architecture defines mxc_ffi as a C ABI over mxc-sdk, but this production dependency lets the new exports bypass the SDK telemetry facade and call the foundation crate directly. Use mxc_sdk::telemetry in the FFI exports and keep wxc_common only as a dev-dependency for the test redirector; this preserves the established layer boundary documented in .github/copilot-instructions.md.
  docs/telemetry/telemetry-consent-design.md:429
• This checklist says three entry points were added, but the script and FFI surface now require four: get consent, set consent, needs prompt, and get policy. Correct the count so the design record accurately describes the codegen gate.

| `scripts/check-dotnet-bindings-codegen.js` | Extended `REQUIRED_ENTRY_POINTS` with the three new FFI exports. | ✅ Done |

src/core/wxc/src/main.rs:838

• The consent handler is not actually the first fast path here: recover_orphaned_state() runs beforehand and can modify filesystem ACLs and emit unrelated stderr. Thus a Node/C# “read-only” consent status query performs DACL recovery, contrary to the design document's guarantee that these flags run before any other startup work. Move this handler ahead of recovery, or explicitly document and test the side effect if it is intentional.

    // --telemetry-consent-{status,grant,revoke}: administer the persisted
    // consent flag and exit. Run before --probe (cheapest possible fast
    // path — no config parsing, no policy defaults needed).
    if handle_telemetry_consent_flags(&cli) {
        return;
    }

sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj:75

• This duplicates the identical InternalsVisibleTo item already present at lines 38–40, causing the SDK to generate the same friend-assembly attribute twice. Keep only one item group.

  <ItemGroup>
    <InternalsVisibleTo Include="Microsoft.Mxc.Sdk.Tests" />
  </ItemGroup>

[Copilot]

Pull request overview

Copilot reviewed 40 out of 42 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/ffi/mxc_ffi/Cargo.toml:17

• This adds a production dependency from mxc_ffi directly to the foundation crate even though the new mxc_sdk::telemetry facade exposes all four operations. That bypasses the established FFI → mxc-sdk boundary (.github/copilot-instructions.md:211) and defeats the facade introduced in src/core/mxc-sdk/src/telemetry.rs:11-19. Call the SDK facade from the FFI entry points and keep wxc_common only as the dev-dependency needed by the test harness.
  src/core/wxc/src/main.rs:140
• source is documented and typed as free-form, but clap will treat a hyphen-leading value as another option by default. For example, setTelemetryConsent(true, "--telemetry-consent-revoke") becomes grant + revoke and exits with the mutual-exclusion error instead of persisting that source. Allow hyphen values (or explicitly narrow/validate the advertised source contract).

    #[arg(long = "telemetry-consent-source")]
    telemetry_consent_source: Option<String>,

[Copilot]

Pull request overview

Copilot reviewed 40 out of 42 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj:75

• This duplicates the existing InternalsVisibleTo item at lines 38–40, causing the same assembly attribute to be generated twice. Keep a single declaration.

  <ItemGroup>
    <InternalsVisibleTo Include="Microsoft.Mxc.Sdk.Tests" />
  </ItemGroup>

src/core/mxc-sdk/src/telemetry.rs:188

• This is the only fallible public mxc-sdk operation that returns an unstructured String; the rest of the SDK returns its crate-owned Error/ErrorCode. Consumers cannot reliably distinguish a non-Windows refusal from persistence failure without parsing text, and the error wording becomes part of the API contract. Return a typed SDK-owned consent error (or add ConsentWriteFailed to the existing facade) and map the internal string into it.

/// Record the user's decision. Returns `Err` on a non-Windows host, and on
/// Windows if the decision could not be persisted — a caller must not treat a
/// failed write as consent.
pub fn set_consent(granted: bool, source: &str) -> Result<(), String> {
    inner_consent::set_consent(granted, source)

scripts/check-telemetry-policy-parity.js:116

• The parity check validates only the exported TypeScript union, not isPolicyState, which is the actual runtime parser (sdk/node/src/telemetry.ts:90). Adding a Rust state and updating the union while forgetting that guard would pass this check but still make Node reject the state as blocked. Compare the guard’s accepted literals with the union too.

// --- TypeScript: the exported union ---------------------------------------
const tsSrc = readFileSync(tsPath, "utf8");
const unionMatch = tsSrc.match(
  /export type TelemetryPolicyState\s*=\s*([^;]+);/

[Copilot]

Pull request overview

Copilot reviewed 40 out of 42 changed files in this pull request and generated no new comments.

Suppressed comments (3)

sdk/node/src/telemetry.ts:166

• A missing or non-boolean needsPrompt is silently coerced to false. Because queryTelemetryConsent() only reports failures when error is set, an outdated or malformed native payload becomes indistinguishable from a genuine “do not prompt” result and violates the documented once-per-failure diagnostic behavior. Validate this field and return a fail-closed result with error so the existing reporter runs; update the query/error documentation and test accordingly.

    needsPrompt: record.needsPrompt === true && policy !== 'blocked',

src/core/wxc_common/src/telemetry/consent_cli.rs:213

• The stated reason for pinning JSON field order is no longer true: the smoke test parses output with ConvertFrom-Json and compares properties. Keeping this test unnecessarily makes harmless field reordering a failure; remove it unless field order is an explicit CLI contract.
  sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj:75
• InternalsVisibleTo is already declared at lines 38–40, so this second item emits duplicate friend-assembly metadata. Remove the duplicate block.

  <ItemGroup>
    <InternalsVisibleTo Include="Microsoft.Mxc.Sdk.Tests" />
  </ItemGroup>