Skip to content

build: WinGet manifest drafts for Wheels.Wheels + Wheels.WheelsBE#2557

Merged
bpamiri merged 5 commits into
developfrom
peter/winget-manifest-drafts
May 11, 2026
Merged

build: WinGet manifest drafts for Wheels.Wheels + Wheels.WheelsBE#2557
bpamiri merged 5 commits into
developfrom
peter/winget-manifest-drafts

Conversation

@bpamiri
Copy link
Copy Markdown
Collaborator

@bpamiri bpamiri commented May 11, 2026

Summary

Scaffolds the 3-file YAML triplet per channel for eventual submission to microsoft/winget-pkgs. Includes a dependency-free Python validator that catches schema-level mistakes without needing the Windows-only winget validate tool.

These are drafts, not ready-to-submit:

  • InstallerUrl + InstallerSha256 are zero-placeholders
  • The actual installer artifact (WiX MSI vs NSIS self-extracting EXE) is a separate build job — see the README for the strategy decision

Why both channels are drafted, but only stable may ship

WinGet's manual-merge cadence (~24-48hr per PR through microsoft/winget-pkgs) is incompatible with per-develop-merge snapshot publish frequency. Daily snapshot PRs to the WinGet registry would be noise.

Plan:

  • Wheels.Wheels → ships on WinGet, slow stable cadence fits
  • Wheels.WheelsBE → kept on Scoop only (hourly autoupdate via Excavator)

The BE manifest in this PR is kept for symmetry + as a record of metadata conventions if we ever change our minds.

What lands

Path Purpose
tools/distribution-drafts/winget/Wheels.Wheels/4.0.0/*.yaml Stable channel triplet (top + locale + installer)
tools/distribution-drafts/winget/Wheels.WheelsBE/4.0.0-snapshot.1789/*.yaml BE channel triplet (kept for symmetry)
tools/distribution-drafts/winget/validate.py Schema + cross-file consistency checker
tools/distribution-drafts/winget/README.md Submission flow + installer-strategy decision pending

Test plan

  • python3 tools/distribution-drafts/winget/validate.py — 0 errors, 4 warnings (all placeholder-related, expected)
  • Post-GA: build installer artifact, fill InstallerUrl + InstallerSha256, re-run validate, submit PR to microsoft/winget-pkgs

Scaffolds the 3-file YAML triplet per channel that we will eventually
submit to microsoft/winget-pkgs. Includes a dependency-free Python
validator that catches schema-level mistakes (identifier/version
mismatches across the triplet, wrong ManifestType tags, non-https
installer URLs) without needing the Windows winget validate tool.

DRAFT status: InstallerUrl and InstallerSha256 are zero-filled
placeholders. The actual installer artifact (WiX MSI vs NSIS
self-extracting EXE) is a separate build job — see
tools/distribution-drafts/winget/README.md for the strategy decision
still pending.

The BE channel manifest is kept for symmetry + documentation but will
likely never ship — WinGet's manual-merge cadence is incompatible with
per-develop-merge snapshot publish frequency. Scoop's hourly
bucket-update bot is the right surface for BE; WinGet for stable.

Validator output: 0 errors, 4 warnings (all placeholder-related,
expected pre-build).
@github-actions github-actions Bot added the docs label May 11, 2026
Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This PR adds WinGet manifest drafts (two 3-file YAML triplets) plus a dependency-free Python validator and a thorough README explaining the installer strategy decision. The manifests themselves are well-formed and the overall approach is sound. One concrete correctness bug exists in validate.py — a false-positive condition that will permanently warn even after the real installer artifact is built and the placeholders replaced. Everything else is clean. Verdict: comment (no blocking issues; the validator bug should be fixed before the submission workflow is exercised for real).


Correctness

validate.py line 122 — "wheels-windows-x64" in url is a false positive

if "PLACEHOLDER" in line or "wheels-windows-x64" in url:
    warn(f"{installer}: InstallerUrl is a PLACEHOLDER (real artifact does not yet exist)")

The second condition ("wheels-windows-x64" in url) will fire for the real artifact URL too. The placeholder URL is:

https://github.com/wheels-dev/wheels/releases/download/v4.0.0/wheels-windows-x64-4.0.0.zip

Once the installer is built, the hash filled in, and the # PLACEHOLDER inline comment removed, the line in the YAML becomes:

  InstallerUrl: https://github.com/wheels-dev/wheels/releases/download/v4.0.0/wheels-windows-x64-4.0.0.zip

At that point "PLACEHOLDER" in line → False, but "wheels-windows-x64" in url → True (the artifact filename is literally wheels-windows-x64-4.0.0.zip). The validator will still emit a WARN after the real artifact is shipped. Someone running validate.py before opening the microsoft/winget-pkgs PR will see a spurious warning that makes it look like the submission isn't ready.

Fix — drop the artifact-name heuristic; "PLACEHOLDER" in line alone is sufficient to catch the current draft state:

if "PLACEHOLDER" in line:
    warn(f"{installer}: InstallerUrl is a PLACEHOLDER (real artifact does not yet exist)")

The sha256 check below (h == "0" * 64) independently catches an unfilled hash, so there is no scenario where the PLACEHOLDER comment is gone but the manifest is still unready that this change would miss.


Commits

build: WinGet manifest drafts for Wheels.Wheels + Wheels.WheelsBE — valid type, no scope (correct; no allowlisted scope maps cleanly to distribution tooling), subject is within 100 chars and not ALL-CAPS. Clean.


Docs

The README is thorough: installer-type decision rationale, submission flow, autoupdate comparison, identifier rationale. No framework .ai/ docs are affected (no CFML changes). No CHANGELOG.md entry — acceptable for a draft artifact that hasn't shipped a user-facing feature.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Reviewer B (round 1)

A's review is well-grounded. The one finding — a false-positive condition in validate.py that will misfire on the real artifact URL — is verified correct, the proposed fix is complete, and the rest of the diff is clean. No sycophancy, no false positives in A's analysis, no significant missed issues. Converging on converged-changes to apply the one-line fix.

Sycophancy

None detected. A's positive framing ("well-formed", "sound approach") is backed by concrete inspection of the YAML triplets and the commit message — not blanket approval.

False positives

None detected. A's claim about validate.py line 122 is accurate. Tracing the logic:

url = s.split(":", 1)[1].strip().split("#")[0].strip()
# For:  InstallerUrl: https://.../wheels-windows-x64-4.0.0.zip  # PLACEHOLDER
# url = "https://.../wheels-windows-x64-4.0.0.zip"  (comment stripped)

When the author removes the # PLACEHOLDER comment and fills in the real sha256, "PLACEHOLDER" in line becomes False but "wheels-windows-x64" in url stays True (the artifact filename literally contains that string). The validator fires a spurious warning at exactly the moment it matters most — pre-submission validation. A's fix (drop the artifact-name heuristic, rely solely on the PLACEHOLDER comment check) is sufficient: the sha256 check h == "0" * 64 provides independent coverage for an unfilled hash, so no scenario slips through.

The same false positive applies to the Wheels.WheelsBE installer URL (wheels-windows-x64-4.0.0-snapshot.1789.zip), which A's analysis also covers implicitly.

Missed issues

None material. One minor observation not worth blocking on: validate.py uses license as a local variable name (line ~110), shadowing the Python built-in of the same name. This is cosmetic and has no runtime impact here — not a missed bug.

Verdict alignment

A chose comment (non-blocking). Given the PR is explicitly scoped as draft scaffolding with zero-placeholder artifact data, this is defensible. request-changes would also be justified since the validator is being shipped in this PR and the bug surfaces precisely when someone exercises it for a real submission, but the distinction is minor for draft tooling.

Convergence

Aligned. A found one real, actionable bug with a specific fix; I have no remaining disputes or additional findings. A's recommendation to fix validate.py before the submission workflow is exercised is correct. Emitting converged-changes to apply the one-line fix.

- tools/distribution-drafts/winget/validate.py:122 — drop the
  `"wheels-windows-x64" in url` heuristic so the validator does not
  emit a false-positive PLACEHOLDER warning once the real artifact
  URL is in place. The `"PLACEHOLDER" in line` comment check plus the
  zero-sha256 check below already cover all unfilled-manifest cases.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):

  • tools/distribution-drafts/winget/validate.py:122 — dropped the "wheels-windows-x64" in url heuristic. The "PLACEHOLDER" in line comment check, combined with the zero-sha256 check below it (line 126), already catches every unfilled-manifest case. The artifact-name heuristic would have fired a spurious WARN once the real artifact URL was in place — precisely the pre-submission validation moment when a clean run matters most. Same false positive applied to the Wheels.WheelsBE installer URL; this fix covers both.

Skipped (not in consensus):

  • Reviewer B's minor cosmetic note about license shadowing the Python built-in (line 110) — explicitly flagged by B as "not worth blocking on" and "cosmetic with no runtime impact." Not part of the converged-changes set.

Test verification: the change is a Python-only edit to a standalone draft-tooling script; no Wheels test layer is affected. The validator's logic is self-contained and the diff is a strict subset of the original condition, so the behavior change is bounded to removing the spurious warning.

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on `approve` or the outer-loop cap (5 rounds) is reached.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This PR adds WinGet manifest drafts (two 3-file YAML triplets), a dependency-free Python validator, and a thorough README explaining the installer strategy. The single correctness bug flagged in the previous review — a false-positive "wheels-windows-x64" in url heuristic — has been fixed in the follow-up commit (65fbdda5a). The current state of the PR is clean. Verdict: comment (one minor docstring inaccuracy; no blocking issues).


Correctness

Previous bug: fixed. The "wheels-windows-x64" in url condition that would have fired as a false positive on the real artifact URL has been correctly dropped. The check is now:

if "PLACEHOLDER" in line:
    warn(f"{installer}: InstallerUrl is a PLACEHOLDER (real artifact does not yet exist)")

The inline # PLACEHOLDER comment in both installer YAMLs triggers the warning now; removing that comment when the real artifact is built silences it. The independent zero-sha256 check covers the case where the URL is real but the hash has not been filled in yet.


Docs

Minor — validate.py line 15: docstring path reference is inaccurate

# - PackageIdentifier matches the directory path (manifests/w/Wheels/<Name>/)

The script is invoked from tools/distribution-drafts/winget/, not from a WinGet-style manifests/w/ tree. The actual check validates that each file's PackageIdentifier field matches the expected_id parameter from the hardcoded pkgs list in main() — not a directory path. Suggested fix:

# - PackageIdentifier in all three files matches the package directory name (e.g. Wheels.Wheels)

This is cosmetic and does not affect validation correctness.


Commits

  • e29ebc5a6 build: WinGet manifest drafts for Wheels.Wheels + Wheels.WheelsBE — valid type, no scope (correct; distribution is not an allowlisted scope), subject within 100 chars, not ALL-CAPS. Clean.
  • 65fbdda5a fix: address Reviewer A/B consensus findings (round 1) — valid type, no scope, subject within 100 chars. Clean.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Reviewer B (round 1)

A's round 2 review is substantively accurate. One minor citation error in A's finding (wrong line number, wrong syntax prefix) but the underlying finding is real and the suggested fix is correct. No sycophancy, no false positives in substance, no missed issues. Converging on converged-changes to apply the one-line docstring fix.

Sycophancy

None detected. A's confirmation that the previous fix is clean is backed by direct inspection of the changed logic, not blanket approval.

False positives

None in substance. One citation inaccuracy worth noting:

A attributed the inaccurate docstring line to "line 15" and quoted it with a # prefix:

# - PackageIdentifier matches the directory path (manifests/w/Wheels/<Name>/)

The line is actually at line 12 in validate.py, and it lives inside the module-level triple-quoted docstring — no # prefix. Line 15 is - Installer URLs use https, a different docstring entry. The citation is wrong but the finding is real: line 12 describes the check as validating a manifests/w/Wheels/<Name>/ directory path, when the actual implementation uses a hardcoded pkgs list of (pkg_dir, expected_id) tuples with no path-matching logic. A's suggested replacement is accurate:

- PackageIdentifier in all three files matches the package directory name (e.g. Wheels.Wheels)

Missed issues

None material. The validator logic is correct: the InstallerUrl PLACEHOLDER check operates on the original line (before strip), so the inline # PLACEHOLDER comment triggers the warning correctly. The sha256 zero-check correctly strips the trailing # PLACEHOLDER comment before comparing to "0" * 64. No edge cases are unhandled for the current draft content.

Verdict alignment

A used comment (non-blocking). Appropriate — the sole remaining issue is a docstring line that describes what the validator checks, not a correctness or submission-blocker. The PR is draft tooling with known placeholder data; the cosmetic inaccuracy does not affect validator runtime behavior.

Convergence

Aligned. A found one real, actionable finding with a specific fix. I have no disputes with A's analysis and no additional findings. Emitting converged-changes to apply the one-line docstring correction (line 12 of validate.py, update the path-reference description to reflect the actual identifier-matching logic).

- validate.py:12 — corrected docstring entry to describe the actual
  PackageIdentifier check (identifier vs. expected package dir name like
  Wheels.Wheels) rather than a non-existent manifests/w/Wheels/<Name>/
  directory-path check.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Address Review (round 2)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 2):

  • tools/distribution-drafts/winget/validate.py:12 — corrected the docstring entry that described the PackageIdentifier check as validating a manifests/w/Wheels/<Name>/ directory path. The actual implementation in main() validates each file's PackageIdentifier field against the expected_id from the hardcoded pkgs list (e.g. Wheels.Wheels). Replaced with A's suggested wording: - PackageIdentifier in all three files matches the package directory name (e.g. Wheels.Wheels). Per B's note, the line is at 12 (in the module docstring), not 15 — A's citation was off by three lines and incorrectly prefixed #, but the underlying finding and fix were correct.

No findings were skipped — the single consensus item was applied verbatim.

Test verification: the change is a Python docstring edit only; no executable logic changed, and no Wheels test layer is affected. The validator's runtime behavior is unchanged.

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on approve or the outer-loop cap (5 rounds) is reached.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: This PR adds WinGet manifest drafts (two 3-file YAML triplets), a Python validator, and a README explaining the distribution strategy. Both prior correctness bugs (false-positive URL heuristic, inaccurate docstring path reference) have been fixed in commits 65fbdda5a and 4b849c931. What remains are three minor docstring inaccuracies in validate.py — none are blocking. Verdict: comment.


Correctness

validate.py line 11: "exactly" in docstring does not match the implementation

- Each version dir contains exactly the 3 files: <Id>.yaml, <Id>.locale.en-US.yaml, <Id>.installer.yaml

The code at lines 77-80 checks that the three required files exist, but it never enumerates the directory to verify no additional files are present:

for f in (top, locale, installer):
    if not f.exists():
        fail(f"{version_dir}: missing {f.name}")
        return

If a user accidentally leaves a .yaml.bak or temp editor file in a version directory, no warning fires. The word "exactly" implies an exclusivity check that is not there. Either add the check or soften the docstring to "contains at least the 3 required files".

validate.py line 4: "missing required fields" overstates coverage

mistakes (missing required fields, identifier/version mismatches across the

The validator checks file presence and specific field consistency (identifier, version, ManifestType, DefaultLocale, License, InstallerUrl, InstallerSha256). It does not verify that WinGet-required metadata fields like Publisher, PackageName, or ShortDescription are non-empty. "Missing required fields" implies broader coverage than exists. Suggested wording: "missing required files, identifier/version mismatches across the 3-file triplet".

validate.py lines 97-102: ManifestType validation is implemented but not listed in the Checks docstring

# Manifest type tags
if top_m.get("ManifestType") != "version":
    fail(f"{top}: ManifestType must be 'version'")
if locale_m.get("ManifestType") != "defaultLocale":
    ...
if installer_m.get("ManifestType") != "installer":
    ...

The module-level Checks list (lines 10-17) omits this validation. Should add: - ManifestType in each file matches its role (version / defaultLocale / installer).


Commits

  • e29ebc5a6 build: WinGet manifest drafts for Wheels.Wheels + Wheels.WheelsBE - valid type, no scope (correct; no allowlisted scope covers distribution tooling), subject within 100 chars, not ALL-CAPS. Clean.
  • 65fbdda5a fix: address Reviewer A/B consensus findings (round 1) - valid type, no scope, within 100 chars. Clean.
  • 4b849c931 fix: address Reviewer A/B consensus findings (round 2) - valid type, no scope, within 100 chars. Clean.

Docs

The README is thorough and the round-2 fix landed correctly - the docstring now accurately describes the PackageIdentifier check as matching against the package directory name rather than a non-existent manifests/w/Wheels/ path. No framework .ai/ docs affected (no CFML changes). No CHANGELOG.md entry - acceptable for draft scaffolding that has not yet shipped a user-facing feature.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Reviewer B (round 1)

A's round 3 review is clean. All three findings are verified against the diff, the verdict is consistent with the severity, and there is no sycophancy. Converging on converged-changes to apply the three docstring corrections.

Sycophancy

None detected. A's positive framing ("what remains are three minor docstring inaccuracies") is grounded in direct line-by-line inspection, not blanket approval.

False positives

None detected. All three claims check out against the actual code:

  • Line 11 "exactly": validate_version at lines 77-82 only checks that the three required files exist; it never enumerates the directory to catch extra files. The word "exactly" implies exclusivity — the check is not there.
  • Line 4 "missing required fields": The validator verifies file presence and specific field consistency (identifier, version, ManifestType, DefaultLocale, License, InstallerUrl, InstallerSha256). WinGet-required metadata fields such as Publisher, PackageName, and ShortDescription are not validated. "Missing required fields" overstates coverage.
  • Lines 97-102 ManifestType: The Checks list in the module docstring (lines 10-17) has seven bullets. ManifestType validation is implemented but absent from the list. A's suggested addition is accurate and matches the actual check.

Missed issues

None material. One edge case worth noting for completeness: parse_yaml_lite uses line.partition(":") to split key/value, which would truncate URL-containing values (e.g. PackageUrl: https://wheels.dev would parse val as //wheels.dev). However, none of the fields accessed through parse_yaml_lite in validation logic contain URLs — InstallerUrl is validated via raw text scan instead. No correctness impact; not a missed bug.

Verdict alignment

A chose comment (non-blocking). Correct — all three issues are docstring inaccuracies with no runtime impact on the validator. Per the command instructions, comment with concrete actionable findings maps to converged-changes.

Convergence

Aligned. A found three real, actionable findings with specific fixes. I have no disputes and no additional findings beyond a noted-but-non-material edge case. The three docstring corrections are well-scoped: soften "exactly" to "at least", tighten "missing required fields" to "missing required files", and add ManifestType to the Checks list. Emitting converged-changes.

- tools/distribution-drafts/winget/validate.py: three docstring corrections in the module docstring so the documented Checks list matches what the validator actually does.
  - Line 4: "missing required fields" -> "missing required files". The validator checks file presence and specific field consistency (identifier, version, ManifestType, DefaultLocale, License, InstallerUrl, InstallerSha256), but does not verify all WinGet-required metadata fields (Publisher, PackageName, ShortDescription) are non-empty. "Missing required fields" overstated coverage.
  - Line 11: "exactly the 3 files" -> "at least the 3 required files". validate_version() at lines 77-80 checks the three required files exist but never enumerates the directory to reject extras. The word "exactly" implied an exclusivity check that is not present.
  - Lines 10-17 (Checks list): added "ManifestType in each file matches its role (version / defaultLocale / installer)" between PackageVersion and DefaultLocale, mirroring the implementation order at lines 97-102. The check is implemented but was previously absent from the documented Checks list.

Docstring-only edits; no executable logic changed. The Reviewer B convergence call (round 3 of the outer loop) noted these are docstring inaccuracies with no runtime impact on the validator.
@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Address Review (round 3)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 3) — all three are docstring inaccuracies in tools/distribution-drafts/winget/validate.py that drifted away from the actual implementation:

  • validate.py:4missing required fieldsmissing required files. The validator checks file presence and a fixed set of field consistencies (identifier, version, ManifestType, DefaultLocale, License, InstallerUrl, InstallerSha256); it does not verify WinGet-required metadata fields like Publisher, PackageName, or ShortDescription are non-empty. The original wording overstated coverage.
  • validate.py:11Each version dir contains exactly the 3 filesEach version dir contains at least the 3 required files. validate_version() at validate.py:77-80 checks for the three required files but never enumerates the directory to reject extras (a stray .yaml.bak or editor temp file would not fire a warning). The word "exactly" implied an exclusivity check that is not present.
  • validate.py:13 (Checks list) — added - ManifestType in each file matches its role (version / defaultLocale / installer) between PackageVersion and DefaultLocale, mirroring the actual implementation order at validate.py:97-102. The check is implemented but the documented Checks list omitted it.

No findings were skipped — the converged-changes set contained exactly these three docstring corrections, all applied verbatim.

Test verification: docstring-only edits to a standalone Python draft-tooling script. No executable logic changed; no Wheels test layer is affected. Confirmed via git diff tools/distribution-drafts/winget/validate.py that the diff is a strict subset of the module-level docstring (3 insertions / 2 deletions, all inside the triple-quoted string).

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on `approve` or the outer-loop cap (5 rounds) is reached.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: All three findings from the previous review rounds have been addressed: the false-positive URL heuristic (round 1), the inaccurate docstring path reference (round 2), and the three module-docstring inaccuracies (round 3). One minor latent gap remains in parse_yaml_lite — it does not strip inline YAML comments from values — but none of the fields the validator currently checks carry inline comments in the manifests, so it causes no present bug. Manifests and README are accurate. Verdict: comment (no blocking issues).


Correctness

validate.py lines 52-58 — parse_yaml_lite does not strip inline YAML comments from values

key, _, val = line.partition(":")
out[key.strip()] = val.strip()

For the two fields in Wheels.Wheels.installer.yaml that carry inline comments:

InstallerType: zip  # TODO: change to nullsoft|inno|msi|wix once installer built
NestedInstallerType: portable  # placeholder; revisit when installer chosen

parse_yaml_lite would store zip # TODO: change to nullsoft|inno|msi|wix once installer built as the value for InstallerType, not zip. This is safe today because neither field is checked by the validator. However, if a future InstallerType allowlist check were added (a natural follow-on once the installer type is decided), it would compare against the polluted value and always fail.

Suggested one-line fix inside the loop body:

out[key.strip()] = val.split("#")[0].strip()

This is the same pattern already used correctly on lines 113 and 120 when grepping the raw installer text for InstallerUrl and InstallerSha256. Keeping the same idiom in parse_yaml_lite would make the function consistent and safe against future check additions.


Commits

  • e29ebc5a6 build: WinGet manifest drafts for Wheels.Wheels + Wheels.WheelsBE — valid type, no scope (correct; no allowlisted scope maps to distribution tooling), subject within 100 chars, not ALL-CAPS. Clean.
  • 65fbdda5a fix: address Reviewer A/B consensus findings (round 1) — valid type, no scope, within 100 chars. Clean.
  • 4b849c931 fix: address Reviewer A/B consensus findings (round 2) — valid type, no scope, within 100 chars. Clean.
  • e5a915890 fix: address Reviewer A/B consensus findings (round 3) — valid type, no scope, within 100 chars. Clean.

Docs

README is accurate and thorough: installer-type decision rationale, submission flow, autoupdate comparison, identifier rationale, and the Scoop-vs-WinGet cadence argument are all present and correctly stated. No framework .ai/ docs are affected (no CFML changes). No CHANGELOG.md entry — acceptable for draft scaffolding that has not shipped a user-facing feature.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Reviewer B (round 1)

A's round 4 review is accurate. One minor undercounting in A's analysis (three fields with inline comments in the installer YAML, not two), but the finding, fix, and verdict are all correct. No sycophancy, no false positives, no missed issues beyond that footnote. Converging on converged-changes to apply the one-line parse_yaml_lite fix.

Sycophancy

None detected. A confirmed the round 3 fixes are clean with explicit line citations and did not simply approve without inspection.

False positives

None detected. A's claim about parse_yaml_lite is accurate. Traced against the actual diff:

  • InstallerType: zip # TODO: change to nullsoft|inno|msi|wix once installer builtpartition(":") splits on the first colon, giving val = " zip # TODO: change to nullsoft|inno|msi|wix once installer built"; after .strip() the inline comment remains in the stored value.
  • Same applies to NestedInstallerType: portable # placeholder; revisit when installer chosen.

A says "two fields" but there are actually three: MinimumOSVersion: 10.0.17763.0 # Windows 10 1809 — required by openjdk21 also carries an inline comment and would parse dirty. This is a minor undercounting in A's characterization, not a false positive — the finding is still real and the suggested fix (val.split("#")[0].strip()) covers all three fields.

None of the three fields are currently validated through installer_m.get(). The fields that are checked (PackageIdentifier, PackageVersion, ManifestType) carry no inline comments. A's "no current bug" claim is verified.

A's fix is consistent with the existing pattern at lines 113 and 120:

url = s.split(":", 1)[1].strip().split("#")[0].strip()
h   = s.split(":", 1)[1].strip().split("#")[0].strip()

Applying the same idiom inside parse_yaml_lite makes the function consistent and future-safe.

Missed issues

None material. A's note about the latent gap is the only substantive observation remaining, and the MinimumOSVersion third field noted above is covered by the same one-line fix. No CFML changes, no .ai/ doc impact, no commit message violations.

Verdict alignment

A chose comment (non-blocking). Appropriate — the finding is latent with zero current impact on the validator's runtime behavior and the PR is explicitly draft scaffolding. request-changes would also be defensible since the fix is trivial and the validator is shipped in this PR, but comment is within bounds.

Convergence

Aligned. A found one real, actionable finding: replace val.strip() with val.split("#")[0].strip() in parse_yaml_lite. I have no disputes with A's analysis and no additional findings. Emitting converged-changes to apply the fix.

- tools/distribution-drafts/winget/validate.py:60 — strip inline `# ...`
  comments from values in parse_yaml_lite. Replaces `val.strip()` with
  `val.split("#")[0].strip()`, matching the idiom already used at lines
  120 and 126 for InstallerUrl/InstallerSha256. Three top-level fields
  in the installer YAML currently carry inline comments
  (`InstallerType`, `NestedInstallerType`, `MinimumOSVersion`) — none
  are read via `installer_m.get()` today, so the impact is latent, but
  the fix prevents a future field check from silently comparing a
  comment-contaminated value.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Address Review (round 4)

Applied the consensus finding from Reviewer A and Reviewer B's convergence (round 4):

  • tools/distribution-drafts/winget/validate.py:60parse_yaml_lite now strips inline # ... comments from each value. Replaced out[key.strip()] = val.strip() with out[key.strip()] = val.split("#")[0].strip(). This matches the idiom already used at validate.py:120 and validate.py:126 for the raw-text InstallerUrl/InstallerSha256 scans. Three top-level fields in the installer YAML currently carry inline comments — InstallerType (# TODO: change to nullsoft|inno|msi|wix once installer built), NestedInstallerType (# placeholder; revisit when installer chosen), and MinimumOSVersion (# Windows 10 1809 — required by openjdk21). None of these are read via installer_m.get() in current validation logic, so the impact is latent today, but the fix forecloses a future field check from silently comparing a comment-contaminated value. Per B's footnote, A's "two fields" characterization undercounted by one (MinimumOSVersion also has an inline comment), but the single one-line fix covers all three — no extra change needed.

No findings were skipped — the converged-changes set contained exactly this one finding, applied verbatim.

Test verification: a one-line edit to a standalone Python draft-tooling script; no Wheels test layer is affected and no executable behavior changes for values currently consumed by the validator (none of those values carried # characters). The smoke test (python3 tools/distribution-drafts/winget/validate.py) wasn't run in this sandbox — the harness denied the python3 invocation — but the change is a strict literal extension of an idiom already exercised at lines 120 and 126 in the same file.

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on `approve` or the outer-loop cap (5 rounds) is reached.

Copy link
Copy Markdown
Contributor

@wheels-bot wheels-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer A

TL;DR: The round 4 fix correctly applies the val.split(\"#\")[0].strip() change to parse_yaml_lite (line 60), matching the idiom already used at lines 120 and 126 for the raw installer-text checks and resolving the latent comment-contamination risk flagged in the previous round. All prior findings across rounds 1-4 have been addressed. Two minor nits below — neither is a blocker for the draft. Verdict: comment.


Correctness

Round 4 fix: correctly applied.

# before (round 3 state)
out[key.strip()] = val.strip()

# after (round 4, line 60)
out[key.strip()] = val.split("#")[0].strip()

The three top-level fields in Wheels.Wheels.installer.yaml that carry inline comments (InstallerType, NestedInstallerType, MinimumOSVersion) are now parsed cleanly. None of those fields are currently checked via installer_m.get(), so the fix is purely preventive — exactly as described in the commit message. The logic is consistent with s.split(":", 1)[1].strip().split("#")[0].strip() used in the installer-text grep at lines 120 and 126.


Docs

Minor — Wheels.Wheels.installer.yaml line 21: ReleaseDate is one day in the future

ReleaseDate: 2026-05-12

Current date is 2026-05-11. Not a blocker for a draft, and the field will be regenerated when the real installer is built. Worth a note so the submitter remembers to verify the date against the actual GA tag timestamp.

Minor — Wheels.Wheels.installer.yaml line 28: verify PortableCommandAlias works with a .cmd file before submission

NestedInstallerFiles:
- RelativeFilePath: wheels\bin\wheels.cmd
  PortableCommandAlias: wheels

WinGet's PortableCommandAlias creates a shim on %LOCALAPPDATA%\Microsoft\WinGet\Links\. For .exe nested files this is straightforward. For .cmd files, WinGet does support it (creates a .cmd forwarding shim), but it is worth running winget install in a Windows sandbox against a real artifact before opening the microsoft/winget-pkgs PR — the WinGet bot validates via actual install, and a non-functional PortableCommandAlias on a .cmd file would fail that automated review. The draft comment already flags the installer type as a placeholder; this is a companion reminder for the .cmd PATH-shim path specifically.


Commits

All five commits are clean:

  • e29ebc5a6 build: WinGet manifest drafts for Wheels.Wheels + Wheels.WheelsBE — valid type, no scope (correct; no allowlisted scope maps to distribution tooling), ≤ 100 chars, not ALL-CAPS.
  • 65fbdda5a fix: address Reviewer A/B consensus findings (round 1) — clean.
  • 4b849c931 fix: address Reviewer A/B consensus findings (round 2) — clean.
  • e5a915890 fix: address Reviewer A/B consensus findings (round 3) — clean.
  • 461ab9476 fix: address Reviewer A/B consensus findings (round 4) — clean.

@wheels-bot
Copy link
Copy Markdown
Contributor

wheels-bot Bot commented May 11, 2026

Wheels Bot — Reviewer B (round 1)

A's round 5 review is accurate and the verdict is appropriate. Both findings are verified against the diff; neither requires a code change in the draft. No sycophancy, no false positives, no missed issues. Converging on converged-approve.

Sycophancy

None detected. A confirmed the round 4 fix with specific line citations and did not issue blanket approval.

False positives

None detected. Both nits check out:

  • ReleaseDate: 2026-05-12: Confirmed in the diff — the value is one day ahead of today (2026-05-11). A's observation is accurate. As A notes, this is minor for a draft and will be regenerated from the actual GA tag timestamp before submission.
  • PortableCommandAlias: wheels on wheels\bin\wheels.cmd: Confirmed in the installer YAML. WinGet does support .cmd nested files with PortableCommandAlias, but the forwarding shim behavior warrants a sandbox install before the microsoft/winget-pkgs PR is opened. A's note is accurate and proportionate.

Missed issues

None material. The parse_yaml_lite fix from round 4 (val.split("#")[0].strip()) is confirmed present in the diff. The license variable shadowing the Python built-in (noted by B in round 1 as cosmetic) remains but is non-blocking. No CFML changes, no cross-engine concerns, no .ai/ doc impact.

Verdict alignment

A chose comment (non-blocking). Consistent with the findings — both nits are pre-submission reminders for the human author, not defects in the draft scaffolding. No code change is appropriate here.

Convergence

Aligned. A's review is substantively correct, the two nits are verified, and neither warrants a code change for bot-address-review.yml to apply. This PR is review-clean for the current SHA. Emitting converged-approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant