Repository navigation
fix: final Codex findings on the 2.0.0 merge PR (#460) - #462
Conversation
- build.zig.zon .version 1.63.0 -> 2.0.0, so untagged builds (dev, CI, plain `zig build`) identify as 2.0.0 and `labelle init` stamps it. compatibility tests updated for the CLI's new MAJOR. - An invalid `.provider_config` (duplicate mapping, undeclared package, empty package, malformed record, invalid path) no longer makes `build`/`generate`/`run` exit 1 silently: the verbose project read prints the project path, the error and the offending entry. - `wasm serve --watch`: a replan installs the re-read `.prebuild` steps on the rebuild context along with the hook config, so removed or changed steps stop running from the startup config. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a34669463d
ℹ️ 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".
| // (Codex P2 on #460). They live on `next`'s arena, like `cfg`; the | ||
| // slice they replace (the startup parse, or `previous`'s arena) is | ||
| // not read again once this assignment lands. | ||
| ctx.prebuild_steps = cfg.prebuild; |
There was a problem hiding this comment.
Refresh watch ignores with the installed prebuild steps
When a watched project.labelle changes or removes a prebuild step's .outputs, this updates the steps but leaves the watcher's ignore_files unchanged; that list is built once from startup parsed.prebuild in src/cli/pipeline/run.zig:290-305 and then reused for every signature. In particular, after removing a step, its former outputs remain permanently ignored, so subsequent external edits to those now-ordinary source files do not trigger a rebuild and the server can keep serving stale results. The ignore set needs to be updated atomically with the new prebuild configuration.
Useful? React with 👍 / 👎.
Fixes the three Codex findings on #460 (commit 5b939df).
P1: CLI version (
src/cli/init.zig:34).build.zig.zon.versiongoes from1.63.0to2.0.0. Before this, untagged builds (dev, CI, a plainzig build) reported 1.63.0, andlabelle initstamped 1.63.0 into new projects. A plain build now printslabelle v2.0.0. Twocompatibility.zigtests assumed a 1.x CLI, so they're updated: the same-line fixtures now pinlabelle_version = "2.0.0", and the all-1.x project now warns 4 times (core, engine, gfx and cli) instead of 3. No other test or fixture hardcodes 1.63.0.P2: silent
.provider_configfailures (src/cli/config.zig:97).provider_settings.validateProjectnow takes the project path when the read is verbose. On a failure it printslabelle: invalid .provider_config in '<path>/project.labelle': …with a message for each error class:unexpected field 'typo')The quiet read (provider dispatch) is unchanged.
parseandvalidatekeep their signatures. The new e2e intest/provider_config_e2e.py(reject_pipeline) runs bothgenerateandbuildfor each error class. It asserts exit code 1, the quoted absolute project path, the error name and the specific message.P2: stale watched prebuild steps (
src/cli/pipeline/watch_replan.zig:189). When a replan commits a new generation, it now setsctx.prebuild_steps = cfg.prebuild. It does this in the same place it setshooks.cfg, and before the previous generation is destroyed. The new slice is on the new generation's arena, and the context never reads the old one again (it came from the startup parse or the previous arena). A failed replan leaves the installed steps unchanged. New unit test:watch replan installs the re-read prebuild stepscovers old → changed (2 steps with args) → failed replan (steps kept) → removed (0 steps).Known remaining gap:
precheckruns before the prebuild steps, and it doesn't install anything. So in the one rebuild triggered by theproject.labelleedit itself, the previous steps still run once. Every rebuild after that uses the new steps.Checks (each exit code checked separately):
zig build: passzig build -Dtarget=x86_64-windows: passzig build test: passzig build test-guard: passzig build test-provider-dispatch: passzig fmt --checkon the touched files: passprovider_android_like(18),provider_config(37),provider_dispatch(28),provider_doctor(16),provider_github(93),provider_hooks(55),provider_output(2),provider_targets(45),prebuild_stdio(7)https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.