Repository navigation
feat(#271): labelle add pack / labelle add feature subcommand - #272
Conversation
Thin CLI forwarder for the Packs scaffold (RFC §7): parses `add ...` off argv and hands it verbatim to the assembler's `add` subcommand, same split as `labelle init`. Adds `add` to the Command enum + dispatch + arg parsing, a new `cli/add.zig`, and help text/examples. Bumps REQUIRED_PROTOCOL to 4 (the `add` subcommand needs the protocol-4 assembler). Closes #271. Part of #651.
|
Warning Review limit reached
Next review available in: 9 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 (4)
📝 WalkthroughWalkthroughThis PR adds a new ChangesLabelle add subcommand
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the add subcommand to the CLI, which delegates to the assembler binary to scaffold packs and feature-units. The changes include registering the new command in src/cli.zig, implementing the forwarding logic in src/cli/add.zig, updating the minimum required assembler protocol version to 4 in src/cli/assembler_proc.zig, and updating the help documentation. I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 239f6961f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// subcommands the CLI depends on. `resolve` checks this and fails early | ||
| /// with a clear message instead of letting a stale binary reject a | ||
| /// subcommand. | ||
| pub const REQUIRED_PROTOCOL: u32 = 4; |
There was a problem hiding this comment.
When no project pin or LABELLE_ASSEMBLER override is set, resolveDefault still downloads assembler.DEFAULT_ASSEMBLER_VERSION (0.40.0), whose tagged assembler declares protocol 3, so raising the global minimum here makes fresh installs reject the auto-downloaded default before generate, build, run, or the new add can execute. Please bump the default assembler release to a protocol-4 build or require protocol 4 only for the add subcommand.
Useful? React with 👍 / 👎.
| .version => return help.printVersion(), | ||
| .targets => return help.printTargets(), | ||
| .init_cmd => return init.cmdInit(allocator, parsed_args.extra_args[0..parsed_args.extra_count]), | ||
| .add_cmd => return add.cmdAdd(allocator, parsed_args.extra_args[0..parsed_args.extra_count]), |
There was a problem hiding this comment.
Because add is returned from the standalone-command switch, it skips the existing config.readProjectConfig guard below and will invoke the assembler even when the current directory is not a Labelle project. In that situation a mistyped cwd such as a parent directory or home directory creates packs/, components/, or scripts/ there instead of producing the same “No project.labelle found” error as project-scoped commands; validate the manifest or route through the normal project-dir path before forwarding.
Useful? React with 👍 / 👎.
|
/gemini review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Address two codex review findings on the `labelle add` PR. Finding 1 (protocol floor): reverting the global protocol minimum back to 3 and gating `add` at 4 per-subcommand. The prior global bump 3->4 rejected the auto-downloaded DEFAULT_ASSEMBLER_VERSION (0.40.0, protocol 3) before any command could run — a breaking change for every fresh install with no project pin. `minProtocolFor(subcommand)` is now a small lookup (floor = MIN_PROTOCOL = 3, `add` = 4) that `resolve`/`checkProtocol` consult; new gated subcommands add one line (e.g. `check` -> 5, #273) without raising the floor. Error message names the subcommand. Finding 2 (require a project): `add` is dispatched from the standalone switch, skipping the readProjectConfig guard project-scoped commands use, so `labelle add ...` in a non-project cwd would scaffold packs/components/ scripts there. `cmdAdd` now checks `config.projectExists(".")` and emits the shared "No project.labelle found" error (exit 1) before forwarding. Tests: minProtocolFor map (add=4, floor cmds=3, unknown=floor, gate>floor) and config.projectExists (present/absent/nonexistent). 301/301 pass.
|
Addressed both codex review findings in 498126f. Finding 1 — per-subcommand protocol minimum (no global bump).
Finding 2 — Tests (added, 292 → 301, all green):
|
|
/gemini review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
…ng (#273) Sync `packs/check-lint` past main (#272 per-subcommand protocol floor, `add` subcommand) and finish the `labelle check` wiring: - cli.zig: resolve the Command-enum merge conflict keeping BOTH `add_cmd` and `check_cmd`; surface `check.ParseCheckArgsSpec` in the test aggregator. - assembler_proc.zig: un-stub the gate table entry `check` -> 5 so `runSubcommand(.., "check", ..)` rejects a protocol-<5 assembler up front (clear "needs protocol >= 5" error) instead of it failing with "unknown subcommand"; add a MinProtocolForSpec case for it. - check.zig: rewrite arg parsing into a pure, testable `parseCheckArgs`. Reserve `--project-root` (space + `=value` forms) so the CLI's own injection is never double-added and its value is not mistaken for the positional dir; accept at most one positional (TooManyArguments otherwise); forward all other flags untouched. Add ParseCheckArgsSpec unit tests (dir extraction, flag forwarding, project-root reservation, too-many-args error). zig build + zig build test green (312/312). Claude-Session: https://claude.ai/code/session_01P7B7UzgrWEbBYLT3YBrAog
1 participant
Closes #271. Part of the Packs initiative (umbrella labelle-toolkit/labelle-engine#651, RFC-packs §7).
What
labelle addscaffolds the two authoring units the Packs RFC defines:Following the exact split
labelle inituses, the CLI is a thin forwarder: it parsesadd ...off argv and hands it verbatim to the assembler'saddsubcommand, which owns the templating (thepack.labelleschema and the game-root convention layout are the assembler's). See the assembler PR for the templates + logic: labelle-toolkit/labelle-assembler#485.Where the logic lives
src/cli/add.zig(one-linerunSubcommand(".", "add", args)), wired into theCommandenum + dispatch + arg parsing incli.zig, plus help text/examples. BumpsREQUIRED_PROTOCOL3 → 4 (theaddsubcommand needs the protocol-4 assembler).addsubcommand,pack.labelle/feature templates, validation, and tests.Verified
zig build+zig build testgreen.LABELLE_ASSEMBLERoverride):labelle add pack citizensandlabelle add feature need boredomscaffold the expected trees;labelle add feature bogus xpropagates the assembler's exit code 2.Deferred / notes
Part of labelle-toolkit/labelle-engine#651.