Repository navigation
Conversation
TDD assessment: codereview-cli PR #634FindingsP1 — The passing checks do not exercise the release path that must render the URLs
The only repository assertion added in Before this package can publish, merge shared PR open-cli-collective/.github#48, update this caller to its merged immutable SHA, and retain its packed- Evidence reviewed
|
![rianjs-bot[bot]](https://avatars.githubusercontent.com/u/7378790?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: d6e117c85f16
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]
Summary
| Reviewer | Findings |
|---|---|
| automation:ci-release | 0 |
| policies:conventions | 0 |
| structure:repo-health | 1 |
structure:repo-health (1 finding)
Major - packaging/chocolatey/tools/chocolateyInstall.ps1:5
The package-release contract is changed to inject both URL placeholders, but
docs/development.mdstill says the shared Chocolatey job replaces only the two checksum placeholders. That repo-local guide is the durable source developers use for release mechanics, so it will send the next maintainer to an incomplete contract and make shared-workflow updates/error diagnosis harder. Update the Chocolatey section to nameURL_AMD64_PLACEHOLDERandURL_ARM64_PLACEHOLDERalongside the checksum placeholders.
Reviewer Coverage
automation:ci-release— complete (broad); skipped: none; constraints: Review limited to the assigned CI/release, Chocolatey installer, and package-render verifier changes.policies:conventions— complete (broad); skipped: none; constraints: Shared CLI standards and shared workflow source were not available locally; review used the repository-local development guide and assigned diff.structure:repo-health— complete (broad); skipped: none; constraints: Reviewed the repository-side integration only; the pinned shared reusable workflow implementation is not present in this checkout.
Inspected files (3)
.github/workflows/release.ymlpackaging/chocolatey/tools/chocolateyInstall.ps1scripts/verify-package-render.sh
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 1m 30s | gpt-5.6-terra | cr 0.10.314
| Field | Value |
|---|---|
| Model | gpt-5.6-terra |
| Reviewers | automation:ci-release, policies:conventions, structure:repo-health |
| Engine | codex_cli · gpt-5.6-terra |
| Reviewed by | cr · rianjs-bot[bot] |
| Duration | 1m 30s wall · 2m 37s compute |
| Cost | unavailable |
| Tokens | 411.3k in / 6.8k out |
Per-workstream usage
orchestrator-selection— gpt-5.6-terra- In: 16.3k
- Out: 298
- Cache read: 11.0k
- Cache create: unavailable
- Cost: unavailable
- Duration: 9s
automation:ci-release— gpt-5.6-terra- In: 149.6k
- Out: 2.3k
- Cache read: 123.4k
- Cache create: unavailable
- Cost: unavailable
- Duration: 52s
policies:conventions— gpt-5.6-terra- In: 88.7k
- Out: 1.6k
- Cache read: 70.7k
- Cache create: unavailable
- Cost: unavailable
- Duration: 38s
structure:repo-health— gpt-5.6-terra- In: 121.0k
- Out: 2.1k
- Cache read: 100.1k
- Cache create: unavailable
- Cost: unavailable
- Duration: 49s
orchestrator-rollup— gpt-5.6-terra- In: 35.8k
- Out: 482
- Cache read: 27.1k
- Cache create: unavailable
- Cost: unavailable
- Duration: 7s
| $toolsDir = Split-Path -Parent $MyInvocation.MyCommand.Definition | ||
|
|
||
| # Checksums injected by release workflow - DO NOT EDIT MANUALLY | ||
| # URLs and checksums injected by release workflow - DO NOT EDIT MANUALLY |
There was a problem hiding this comment.
The package-release contract is changed to inject both URL placeholders, but docs/development.md still says the shared Chocolatey job replaces only the two checksum placeholders. That repo-local guide is the durable source developers use for release mechanics, so it will send the next maintainer to an incomplete contract and make shared-workflow updates/error diagnosis harder. Update the Chocolatey section to name URL_AMD64_PLACEHOLDER and URL_ARM64_PLACEHOLDER alongside the checksum placeholders.
Reply inline to this comment.
There was a problem hiding this comment.
Fixed in ffb4fa3: the development guide now names both URL placeholders and both checksum placeholders in the shared release rendering contract.
|
These are low-value, please approve the PR |
## Summary Update only the reusable release workflow pin to `776a11fe7a86dccb73bccb5dbf3824fe2ec8d4af`, which includes the merged canonical Windows archive resolver in open-cli-collective/.github#49. The failed v0.10.320 release lists `cr.exe` in each Windows archive's `extra.Binaries`, while the identity manifest declares `cr`. The old resolver returned zero matches and emitted empty matrices, preventing the Windows package jobs from starting. The updated resolver matches both architectures and fails loudly for missing assets. ## Evidence Using the actual `dist` artifact from [the failed release](https://github.com/open-cli-collective/codereview-cli/actions/runs/36931823778): | Resolver | AMD64 | ARM64 | |---|---|---| | Old pinned workflow | zero matches | matrix not produced | | Updated shared action | `cr_v0.10.320_windows_amd64.zip` | `cr_v0.10.320_windows_arm64.zip` | Both Chocolatey and WinGet matrices contain those exact published asset names. The shared resolver regression also verifies missing AMD64 assets fail loudly. Validation: consumer workflow `actionlint` passes; shared resolver, Homebrew modernization/idempotency and alias tests pass; all 28 Chocolatey-push tests pass. The Windows PowerShell renderer was inspected but has not yet run here. The intervening shared changes were inspected: action dependency pins, Homebrew install-step modernization and staged-path correction, and backward-compatible Chocolatey rendering. Existing package templates without URL placeholders keep their legacy path; this does not apply the static-URL migration staged in #634. Package identity, secrets, triggers, production code, and CI pins remain unchanged. The fixture/artifact replay proves matrix resolution, not live Windows package publication. A successful subsequent release is still required. No published tag or asset is changed by this PR; workflow-only changes do not automatically mint a new release.
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.