You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds glossary.
Terminology
In consumer-targeted docs, don't use weird-vague terms. (In internal developer docs - OK.)
Stop using,
- "closed" (to mean 'predefined') and "pinned" (to mean 'restricted')
- "exact contract"
- "one-shot"
- "state-aware"
- "wire-format"
Use specific format names, not "raw JSON".
Don't use garbled mojibake characters in .rs files (e.g. "â€").
Avoid limiting helper codes to native-runtime errors
docs/api-reference/node/v1/api.md:101
This public helper is also used to create SDK-side validation errors (for example, malformed container IDs in sdk/node/src/state-aware-helper.ts:56-72), and its string parameter intentionally accepts unknown codes. Calling the input a native-runtime code is therefore too narrow.
Include SDK-side validation errors in the error code scope
docs/api-reference/node/v1/types.md:240
These codes are not limited to native-runtime failures. The Node SDK also uses this union for SDK-side request and option validation before calling native code (for example, sdk/node/src/v1/container.ts:336-341).
Document MxcError for SDK validation and binding failures
docs/api-reference/node/v1/types.md:410
MxcError is also thrown directly for SDK-side validation and binding failures, so describing it only as a response to native JSON excludes common documented behavior. For example, containerConfig constructs it before native execution (sdk/node/src/v1/container.ts:336-341).
Distinguish policies from backend configuration
docs/glossary.md:12
Policy and config are distinct concepts in the SDK: policies are cross-backend restrictions, while backend configuration contains settings such as a WSLc image (sdk/node/README.md:190-192). Combining them here incorrectly defines all configuration as access rules.
This issue also appears on line 22 of the same file.
Clarify the explanation of failure classification and error handling in WSLc.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Updated comment to clarify usage of camelCase in result field.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Avoid claiming all MxcError values originate from native JSON
docs/api-reference/node/v1/types.md:410
Not every MxcError is produced from JSON: local SDK validation constructs this type directly, and most native calls map the FFI status plus MxcErrorDetail. Describing all instances as responses to a native JSON object is therefore inaccurate.
This issue also appears on line 428 of the same file.
Clarify foreground exit and descendant cleanup guarantees
src/mxc-sdk/src/sandbox.rs:226
This sentence is incomplete: “confirms that process” does not say what is confirmed, and the singular descendant wording obscures the cleanup guarantee. State explicitly that the foreground process is confirmed gone and that backgrounded descendants are reclaimed later.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The new consumer glossary contains inaccurate lifecycle, streaming, opaque-ID, and host-loopback definitions.
0 open findings
Previously missed (4)
In code that hasn't changed since last review
Distinguish configuration from access policy
docs/glossary.md:12
config is broader than policy: MXC configuration also carries the workload, containment selection, runtime settings, and other non-access fields. Treating the terms as synonyms gives consumers an incorrect definition; split them into separate entries.
Clarify valid handling of persisted container identifiers
docs/glossary.md:17
“Never construct it” conflicts with the public SDKs: Rust exposes ContainerId::parse, and .NET exposes new ContainerId(string) specifically to restore a persisted identifier. Consumers must not fabricate or interpret the value, but they may need to wrap the unchanged returned string after persistence.
Distinguish live streams from captured output
docs/glossary.md:20
The streaming definition conflates captured output (“collected after completion”) with live output. spawn returns readable streams that must be accessed before wait; captured execution instead returns stdout/stderr after completion. Define these as distinct delivery models.
Document host loopback as bidirectional access
docs/glossary.md:22
This defines host loopback as container-to-host only, but ingress.hostLoopback is explicitly bidirectional (docs/backends/process-container/networking.md:105 and docs/schema.md:113). Consumers could otherwise underestimate the access granted by "allow".
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The new glossary contains inaccurate I/O definitions and retains ambiguous terminology the PR intends to remove.
0 open findings
Previously missed (2)
In code that hasn't changed since last review
Split policy and config glossary definitions
docs/glossary.md:12
Policy and config are not synonyms: a ContainerRequest configuration also carries the workload command, containment/backend settings, working directory, and environment. Defining both as access rules makes the new glossary misleading; split the terms so config retains its broader meaning.
Use the defined MXC request JSON terminology
sdk/node/README.md:74
executor JSON configuration introduces another undefined name for the format and prevents readers from connecting this statement to the glossary and schema guide. Use the defined “MXC request JSON” name, as the PR description requires for consumer documentation.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The new glossary and Node reference contain factual inaccuracies about lifecycle cleanup, streaming output, host-loopback directionality, and error origins.
The reason will be displayed to describe this comment to others. Learn more.
note: can you tell the bot to update all links in the sdk README.md's to the full link from github main and to have it remove any stale links? I think this one will go to a 404 page if clicked from nuget.org.
The rejection is performed by the version-specific contract parser, not by the schema artifact. Calling this “schema 0.9.0-alpha” conflicts with the terminology rule introduced in this PR to reserve “schema” for a contract's JSON description.
Describe contract parser and backend validation behavior
docs/backends/wslc/wslc-state-aware.md:209
This is contract behavior, not schema behavior: the contract parser and backend validation determine which networking combinations are accepted. The new developer glossary explicitly reserves “schema” for the JSON description of a contract.
Name the specific JSON interface format
docs/container-lifecycle.md:5
“Native JSON format” is not a defined format name and leaves readers guessing which interface the link describes. Name the owning interface, consistent with the new wording guide's requirement to use explicit format names.
This issue also appears on line 11 of the same file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds glossary.
Terminology
In consumer-targeted docs, don't use weird-vague terms. (In internal developer docs - OK.)
Stop using,
Use specific format names, not "raw JSON".
Don't use garbled mojibake characters in .rs files (e.g. "â€").
Microsoft Reviewers: Open in CodeFlow