feat(update): machine-readable update checks — update/upgrade --check [--json] (#276) - #299
Conversation
… [--json] (#276) Read-only, non-mutating version checks so labelle-studio (#7) can surface "CLI x.y.z available" and per-project pin-vs-latest without parsing human output or mutating anything. - `labelle update --check [--json]`: running CLI vs newest published release (R2 latest.txt); never installs a binary. - `labelle upgrade [dir] --check [--json]`: project.labelle pins (core/engine/gfx/labelle/assembler + plugins) vs the bundled compatible set this CLI targets; never rewrites project.labelle. Fully offline. - `--json` implies `--check` (a tool asking for JSON never triggers a mutation). Exit 2 = updates available, 0 = up to date (cheap scripting). - Stable schema `{cli, packages}` adopting the #278 sketch the issue endorses; per-entry checked/error distinguishes up-to-date from offline / no-known-latest / local-override. New shared, pure, unit-tested reporting module src/cli/update_check.zig (JSON shape + status computation + exit codes). Also fixes a latent dispatch bug where a leading `--check`/`--force` was captured as the project dir. Claude-Session: https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds read-only ChangesUpdate/Upgrade check reporting
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as cli.zig
participant Update as update.zig
participant Upgrade as upgrade.zig
participant Report as update_check.zig
participant Server as release server
CLI->>Update: cmdUpdateCheck(--check/--json)
Update->>Server: fetch latest.txt
Server-->>Update: version string
Update->>Report: cliStatus(installed, latest)
Report-->>Update: CliStatus
Update->>Report: writeJson/writeHumanCli
Update->>Update: exitCode(report)
CLI->>Upgrade: cmdUpgrade(--check/--json)
Upgrade->>Upgrade: reportOnly() true
Upgrade->>Report: packageStatus(pins, latest)
Report-->>Upgrade: PackageStatus[]
Upgrade->>Report: writeJson/writeHumanPackages
Upgrade->>Upgrade: exitCode(report)
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/upgrade.zig (1)
50-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSame unknown-flag pass-through gap as
update.zig'sparseUpdateArgs.An unrecognized
--token (e.g. a mistyped--jso) is appended topositionalsinstead of being rejected. Since it's not"assembler"/"all",cmdUpgradewould delegate it as a "subcommand" toassembler_proc.runSubcommand(..., "upgrade", ...)— a mutating path — rather than failing fast. Lower direct risk than the update.zig case (the assembler binary gets a chance to reject it), but the same defensive gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/upgrade.zig` around lines 50 - 68, `parseUpgradeArgs` is currently treating unknown `--` options as positionals, which can leak mistyped flags into `cmdUpgrade` and then `assembler_proc.runSubcommand`. Update `parseUpgradeArgs` to explicitly reject unrecognized arguments that look like flags (for example any token starting with `--` or `-` that is not one of the supported options) instead of appending them to `positionals`, while still allowing valid subcommand positionals like `assembler` and `all` to pass through.
🧹 Nitpick comments (1)
src/cli.zig (1)
752-783: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the upgrade dispatch loop into a standalone testable function.
This inline loop is exactly the fix location for the leading-dash-as-dir bug described in the comment, but unlike
parseRunArgs/parseWasmServeArgs/parseDirAndScene(which are extracted functions with dedicatedtestIter-based specs), it lives directly inmain()and has no unit test exercising it — the new tests only coverupgrade.zig'sparseUpgradeArgs, which operates on the already-splitcmd_argsslice, not on this dir/subcommand-splitting logic itself.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli.zig` around lines 752 - 783, The upgrade argument splitting loop in main() should be extracted into a standalone helper so it can be unit tested like parseRunArgs, parseWasmServeArgs, and parseDirAndScene. Move the dir/subcommand parsing logic currently handling seen_subcommand, dir_set, and appendExtraArg into a dedicated function with a testIter-based spec, and keep main() only responsible for wiring it into parseUpgradeArgs. This ensures the leading-dash-as-dir behavior is covered directly by tests instead of only being indirectly exercised through already-split args.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/update.zig`:
- Around line 27-41: `parseUpdateArgs` is treating any unknown token, including
mistyped `--` flags, as `version_arg` instead of rejecting it. Update
`parseUpdateArgs` to explicitly detect unrecognized `--`-prefixed arguments and
fail fast rather than assigning them to `out.version_arg`, and make sure
`cmdUpdate` only receives a validated version/option set. Follow the pattern
used by the other flag parsers in this CLI so `--check`/`--json` behavior in
`reportOnly()` cannot be bypassed by typos.
---
Outside diff comments:
In `@src/cli/upgrade.zig`:
- Around line 50-68: `parseUpgradeArgs` is currently treating unknown `--`
options as positionals, which can leak mistyped flags into `cmdUpgrade` and then
`assembler_proc.runSubcommand`. Update `parseUpgradeArgs` to explicitly reject
unrecognized arguments that look like flags (for example any token starting with
`--` or `-` that is not one of the supported options) instead of appending them
to `positionals`, while still allowing valid subcommand positionals like
`assembler` and `all` to pass through.
---
Nitpick comments:
In `@src/cli.zig`:
- Around line 752-783: The upgrade argument splitting loop in main() should be
extracted into a standalone helper so it can be unit tested like parseRunArgs,
parseWasmServeArgs, and parseDirAndScene. Move the dir/subcommand parsing logic
currently handling seen_subcommand, dir_set, and appendExtraArg into a dedicated
function with a testIter-based spec, and keep main() only responsible for wiring
it into parseUpgradeArgs. This ensures the leading-dash-as-dir behavior is
covered directly by tests instead of only being indirectly exercised through
already-split args.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bb547b0d-9570-44ea-94f0-50614e7da1eb
📒 Files selected for processing (5)
src/cli.zigsrc/cli/help.zigsrc/cli/update.zigsrc/cli/update_check.zigsrc/cli/upgrade.zig
…Rabbit #299) `parseUpdateArgs` treated any unrecognized token as the target version, so a typo'd flag bypassed the read-only `--check` guard and fell into the binary-replacing install path. The exploit: `labelle update --chek 1.60.0` — the dropped `--chek` leaves `1.60.0` as version_arg, `reportOnly()` is false, and the CLI INSTALLS 1.60.0 instead of doing a read-only check. Both parsers now reject unknown `--`-prefixed tokens with a clear message + `error.InvalidArguments` (mirrors the `labelle status` unknown-flag convention; distinct nonzero exit, so it never collides with the 0/2 up-to-date/updates-available codes). Bare non-dash tokens stay valid version positionals. `parseUpgradeArgs` had the same class of bug (a typo'd flag became a positional and reached the mutating path) and is fixed symmetrically. Regression tests for both parsers (verified they fail without the fix). Claude-Session: https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j
Closes #276. Unblocks labelle-studio#7 — studio can now ask "what's outdated?" without parsing human output and without mutating anything.
What
Two read-only, non-mutating reporting modes over the versions the CLI already knows:
labelle update --check [--json]— running CLI binary vs the newest published release (releases.labelle.games/cli/latest.txt). Never downloads/installs.labelle upgrade [dir] --check [--json]—project.labellepins (core/engine/gfx/labelle/assembler+ plugins) vs the bundled compatible set this CLI targets (the same setupgrade allwould apply). Never rewritesproject.labelle. Fully offline — a "read-only mode over whatupgradealready queries," as the issue asks.Contract:
--jsonimplies--check— a tool asking for machine output can never accidentally trigger a mutating install/upgrade.0= up to date,2= at least one update available. Offline/unknown is not counted as an update (exit0); thechecked/errorfields carry that nuance.JSON schema (stable — studio consumes this)
Adopts the concrete sketch from #278 that @apotema endorsed in the issue thread:
{cli:{installed,latest,update_available}, packages:[{name,pinned,latest,update_available}]}, plus a per-entrychecked/errorto distinguish "up to date" from "offline / no-known-latest". Both commands emit the same top-level shape; each fills only its half. All keys are always present (nullwhen unknown) for a stable typed shape — the same convention as the--progress=jsonNDJSON feed.{ "cli": { // update --check → populated; upgrade --check → null "installed": "1.55.1", "latest": "1.56.0", // null when the fetch failed "update_available": true, "checked": true, // false = couldn't determine (offline) "error": null // string when checked=false }, "packages": [ // upgrade --check → populated; update --check → [] { "name": "core", "pinned": "1.20.0", "latest": "1.21.0", "update_available": true, "checked": true, "error": null }, { "name": "engine", "pinned": "1.65.0", "latest": "1.65.0", "update_available": false, "checked": true, "error": null }, { "name": "assembler", "pinned": null, "latest": "0.40.0", "update_available": false, "checked": true, "error": null }, // unpinned → on CLI default { "name": "my-plugin", "pinned": "4.0.1", "latest": null, "update_available": false, "checked": false, "error": "no known latest version for this package" }, { "name": "local-dep", "pinned": "0.1.0", "latest": null, "update_available": false, "checked": false, "error": "local path override — not version-comparable" } ] }update --check --json→{"cli":{…},"packages":[]}upgrade --check --json→{"cli":null,"packages":[…]}Studio composes the two:
update --checkanswers "is the installed CLI stale?" (network);upgrade --checkanswers "are this project's pins behind?" (offline). The CLI has no plugin-version registry, so pluginlatestisnull/checked:false— the pin is still surfaced so studio can display it.Real output
Implementation
src/cli/update_check.zig— status model, JSON/human serialization, exit-code logic (no network, no fs; all inputs supplied by the callers).update.zig/upgrade.zigare thin wrappers: the CLI-latest fetch and theproject.labelleread live there and feed the pure core.upgradedispatch bug where a leading--check/--forcetoken was captured as the project dir (would have sent--checktoreadProjectConfig).Tests
zig build test→ 425/425 passed. New coverage: status computation (up-to-date / outdated / offline / unpinned / unknown-latest / local-override), exit codes, and JSON shape assertions (both report variants, null-key stability); flag parsing for both commands (incl.--json-implies---check). Non-mutation verified by hashingproject.labellebefore/after--check.https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j