Skip to content

fix: final Codex findings on the 2.0.0 merge PR (#460) - #462

Merged
apotema merged 1 commit into
developmentfrom
fix/460-final-review
Sep 27, 2026
Merged

apotema merged 1 commit into
developmentfrom
fix/460-final-review

Conversation

@apotema

@apotema apotema commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Fixes the three Codex findings on #460 (commit 5b939df).

P1: CLI version (src/cli/init.zig:34). build.zig.zon .version goes from 1.63.0 to 2.0.0. Before this, untagged builds (dev, CI, a plain zig build) reported 1.63.0, and labelle init stamped 1.63.0 into new projects. A plain build now prints labelle v2.0.0. Two compatibility.zig tests assumed a 1.x CLI, so they're updated: the same-line fixtures now pin labelle_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_config failures (src/cli/config.zig:97). provider_settings.validateProject now takes the project path when the read is verbose. On a failure it prints labelle: invalid .provider_config in '<path>/project.labelle': … with a message for each error class:

  • duplicate mapping: the package name
  • undeclared package: the package name
  • empty package: the entry number
  • invalid path: the package, the file and the rule it breaks
  • malformed record: the expected record shape, plus the ZON diagnostic with line and column (e.g. unexpected field 'typo')

The quiet read (provider dispatch) is unchanged. parse and validate keep their signatures. The new e2e in test/provider_config_e2e.py (reject_pipeline) runs both generate and build for 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 sets ctx.prebuild_steps = cfg.prebuild. It does this in the same place it sets hooks.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 steps covers old → changed (2 steps with args) → failed replan (steps kept) → removed (0 steps).

Known remaining gap: precheck runs before the prebuild steps, and it doesn't install anything. So in the one rebuild triggered by the project.labelle edit itself, the previous steps still run once. Every rebuild after that uses the new steps.

Checks (each exit code checked separately):

  • zig build: pass
  • zig build -Dtarget=x86_64-windows: pass
  • zig build test: pass
  • zig build test-guard: pass
  • zig build test-provider-dispatch: pass
  • zig fmt --check on the touched files: pass
  • e2e scripts, all pass: provider_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


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

- 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
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-27T11:53:49.424604Z a346694 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 7c2755a9-9dc2-44b5-8850-a6d523a51a0f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@apotema
apotema merged commit 1f1c48f into development Sep 27, 2026
10 checks passed
@apotema
apotema deleted the fix/460-final-review branch September 27, 2026 12:06
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