Skip to content

fix: render literal Chocolatey download URLs - #34

Open
rianjs wants to merge 1 commit into
mainfrom
fix/choco-static-urls
Open

rianjs wants to merge 1 commit into
mainfrom
fix/choco-static-urls

Conversation

@rianjs

@rianjs rianjs commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Render both architecture download URLs as literals in Chocolatey packages so Business Internalizer can resolve them. Keep this staged until the confluence-cli balloon passes licensed internalization and offline install. The shared release workflow must ship first.

@rianjs

rianjs commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

TDD coverage assessment: google-cli PR #34

Findings

P1 — The static-package check can pass when either Google package has unusable or wrong architecture URLs

The PR adds the two URL placeholders, but it has no test. Its required release-path companion, .github PR #48, only rejects known placeholders and runtime-version expressions after choco pack. Its acceptance test treats arbitrary literal strings as valid URLs and does not check URL count, GitHub release shape, final tag, the gro/grw archive names, architecture pairing, or that each selected checksum is the SHA256 for that same archive. Consequently, all current checks can pass if rendering writes the AMD64 asset into both variables, swaps gro and grw, writes malformed literal URLs, or leaves a non-SHA256 checksum. Chocolatey Business internalization would then ingest the wrong artifact or fail later.

Add one small pre-push rendered-nupkg test in the shared release workflow/action, using the actual google-readonly and google-readwrite templates (or a fixture with their manifest asset names). Render a fixed version/tag and two distinct 64-hex hashes, pack, then assert chocolateyInstall.ps1 contains exactly these literal URLs:

  • .../v2.0.123/gro_v2.0.123_windows_amd64.zip and .../gro_v2.0.123_windows_arm64.zip for google-readonly;
  • .../v2.0.123/grw_v2.0.123_windows_amd64.zip and .../grw_v2.0.123_windows_arm64.zip for google-readwrite.

Also assert ARM64 selects its ARM64 URL/checksum and the 64-bit fallback selects its AMD64 pair. This is the smallest automated proof of the ticket contract; the licensed Chocolatey Business internalization plus GitHub-blocked install remains the plan's post-publication balloon acceptance check.

Evidence reviewed

  • Approved internalization plan, especially its requirement to inspect each generated nupkg and fail on placeholders or runtime version expressions.
  • google-cli PR fix: render literal Chocolatey download URLs #34 complete diff (c7799cb): only the two PowerShell templates changed; CI reports 11 checks passing but has no Chocolatey/template test.
  • google-cli packaging/identity.yml and .goreleaser.yaml: expected archive names are gro_v{{ .Version }}_windows_{amd64,arm64}.zip and grw_v{{ .Version }}_windows_{amd64,arm64}.zip.
  • Dependent shared-workflow PR #48: its validate_static_package tests prove only rejection of placeholders/runtime expressions; its passing fixture accepts arbitrary literal URL and checksum content.

@rianjs
rianjs marked this pull request as ready for review September 25, 2026 15:39

@rianjs-bot rianjs-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated PR Review

Reviewed commit: c7799cb6a64e
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer Findings
automation:ci-release 1
policies:conventions 0
automation:ci-release (1 finding)

Blocking - packaging/chocolatey/google-readwrite/tools/chocolateyInstall.ps1:5

The active shared release workflow at open-cli-collective/.github/.github/workflows/release.yml@v1 only verifies and replaces the checksum placeholders; it has no replacement or assertion for URL_AMD64_PLACEHOLDER/URL_ARM64_PLACEHOLDER. Consequently the published google-readwrite package will retain this literal non-URL and fail to download on installation. Ship the corresponding URL rendering/assertion in the shared workflow and consume that released revision before this template can be released.

Reviewer Coverage

  • automation:ci-release — complete (broad); skipped: none; constraints: Reviewed only the assigned Chocolatey installer scripts and their active shared release-workflow rendering contract.
  • policies:conventions — complete (broad); skipped: none; constraints: Review limited to the two assigned Chocolatey installer scripts; no local cli-common convenience copy was available to inspect.
Inspected files (2)
  • packaging/chocolatey/google-readonly/tools/chocolateyInstall.ps1
  • packaging/chocolatey/google-readwrite/tools/chocolateyInstall.ps1

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 6m 56s | gpt-5.6-terra | cr 0.10.314
Field Value
Model gpt-5.6-terra
Reviewers automation:ci-release, policies:conventions
Engine codex_cli · gpt-5.6-terra
Reviewed by cr · rianjs-bot[bot]
Duration 6m 56s wall · 6m 55s compute
Cost unavailable
Tokens 338.2k in / 4.6k out

Per-workstream usage

  • orchestrator-selection — gpt-5.6-terra
    • In: 16.4k
    • Out: 168
    • Cache read: 16.1k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 5m 10s
  • automation:ci-release — gpt-5.6-terra
    • In: 201.9k
    • Out: 3.1k
    • Cache read: 172.0k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 1m 13s
  • policies:conventions — gpt-5.6-terra
    • In: 84.1k
    • Out: 921
    • Cache read: 69.6k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 24s
  • orchestrator-rollup — gpt-5.6-terra
    • In: 35.8k
    • Out: 400
    • Cache read: 32.3k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 7s

$version = $env:ChocolateyPackageVersion
$toolsDir = Split-Path -Parent $MyInvocation.MyCommand.Definition

$urlAmd64 = 'URL_AMD64_PLACEHOLDER'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The active shared release workflow at open-cli-collective/.github/.github/workflows/release.yml@v1 only verifies and replaces the checksum placeholders; it has no replacement or assertion for URL_AMD64_PLACEHOLDER/URL_ARM64_PLACEHOLDER. Consequently the published google-readwrite package will retain this literal non-URL and fail to download on installation. Ship the corresponding URL rendering/assertion in the shared workflow and consume that released revision before this template can be released.

Reply inline to this comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The shared v1 tag was updated to 86df1fd5 before this review. That release workflow now calls actions/chocolatey-render, which replaces both URL placeholders and checks the packed package for leftovers. Its Google gro/grw fixtures passed CI in .github#48. The package remains gated on the cfl Business internalization check.

@rianjs

rianjs commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

These are low-value, please approve the PR

@rianjs-bot rianjs-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approving after an explicit PR author override request following a prior codereview pass.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant