Repository navigation
feat(providers): phase 3b provider-declared targets and --platform resolution (RFC #406) - #421
Merged
Merged
Conversation
…RFC #406 phase 3a) Manifest: a `replace` hook must name a target its own package declares (ReplaceRequiresOwnedTarget); `after_hooks` entries are `<package>/<id>` references, never self (InvalidHookReference); `ownsTarget`, `splitHookRef`, `qualifiedId`. Planner (`provider_hooks.zig`): `validateAll` checks the cross-provider graph once at discovery — DuplicateReplaceHook, MissingHookReference, HookReferenceMismatch, HookPhaseOrder, HookCycle — and `plan` orders each (step, target) as before → replace → after, Kahn's algorithm on the `after_hooks` edges with ties by qualified id, so the plan is independent of manifest read order. `stepOutputDir` is the step output-layout contract; `runPhase`/`finishRun` execute a phase and propagate a failing hook's exit code. Dispatch: `Provider`/`discover` are pub and `discover` validates the hook graph; `execute` is split into `resolveHost` + `runTool` over a `ToolRun` (command dispatch is a thin wrapper, its e2e unchanged); the lock read, pin check and integrity gate move into `requirePinned`; `resolveSettings` and `canonicalDir` are pub. Progress: `Reporter.beginPhaseOrStep` renames the sub-step inside the live phase, since the machine rejects a phase re-entering itself and hooks share the phase of the core step they wrap. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…efault (RFC #406 phase 3a) Discovery runs once per pipeline invocation (only when `.plugins` is non-empty), before the reporter, the assembler or any compiler, and a failed discovery exits 1 with `provider discovery failed: <error>`. The four plans are computed up front; a project without hooks never resolves the host compiler. Hook points, each `before` → core or unique `replace` → `after`, stopping at the first failure and exiting with the hook's own code: - generate: around assembler generate + fingerprints + emsdk activation, output_dir = the generated tree; - build: around docker/host `zig build` + DLL staging, output_dir = zig-out; - bundle: around `bundle.createFromBuild`, output_dir = zig-out/bundle/<target> or the resolved --output; - run: `before` once ahead of every branch, `after` at each success exit via `finishRun` (only on a clean game exit, before the terminal `done` record; `wasm serve` runs them once the server returns). `labelle.lock` is written right after `validatePluginCoreCompat`, before generation, so a `before generate` hook already sees it. Core steps report through `beginPhaseOrStep` so a hook and the core step can share a phase. The desktop `.app` default moves to `<target_dir>/zig-out/bundle/desktop/`, the step output directory bundle hooks receive, so hooks and the core packager agree; `--output` is unchanged. Help text names the hooks doc. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
#406 phase 3a) `docs/provider-hooks.md` covers attach rules, discovery-time graph errors, ordering (after_hooks edges only, ties by qualified id), the output-layout table, the hook context, failure semantics and the limitations (no JSON relay, legacy platform handlers, bundle still macOS-only). The dispatch, contract and GitHub-pins docs point at it. `test/provider_hooks_e2e.py` drives the real CLI with a fake assembler (the plugin_compat pattern; a `.cmd` shim on Windows) and two fixture providers declared in reverse order: order regardless of declaration order, an `after_hooks` edge inverting the tie, context fields, a failing before hook's exit code with no core build and no after hook, a failing core build skipping after, ReplaceRequiresOwnedTarget / DuplicateReplaceHook / MissingHookReference / HookPhaseOrder at `labelle help` before any compiler, generate hooks seeing the lock, run hooks around the game, bundle hooks and the .app location on macOS, the refusal elsewhere. The fixture tool appends one JSON line per invocation to `<output_dir>/hooks.log` and fails on `PROVIDER_PROBE_FAIL=<hook id>`. Wired into CI next to the other provider suites on all three hosts. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
… (RFC #406 phase 3b) `--platform=<t>` is a target NAME resolved by the pipeline: `desktop` is core, every other name must be declared in a pinned provider's `.targets` (`provider_targets.resolve`), and an undeclared one fails before the assembler, a compiler or the cache is touched with `labelle: no provider for target '<t>' in this project; add and pin the package that declares target '<t>'`. The `(registry: <package>)` line is added only when the cached registry document — kept by `providers resolve --accept` — lists a package whose verified cached archive declares the target (`provider_github.cachedRegistryOwner`: no network, no extraction, no package code); the registry record carries no targets, so a name is never invented. The resolved string names the target dir, is passed to the assembler and to every hook context. `project_config.Platform` stays the strict schema type (documented as the legacy codegen platform). The labelle-assembler#378 boundary is explicit: a provider target outside that enum is generated for only by its provider's `replace` hook on `generate`; without one the command stops with `... but the pinned assembler cannot generate for it yet (labelle-assembler#378)`. Schema-named provider targets reach the assembler as `--platform <name>` unchanged. Parsers: `ParsedArgs.platform_override` is a string; `parseTargetValue` checks identifier shape only (`invalid target '<t>' (targets are lowercase identifiers; run 'labelle targets')`); `astc --platform` passes the name through and rejects one without a legacy capability table. `labelle bundle --platform=<t>`: the macOS host gate moves from cli.zig into the pipeline after resolution — refusing only the core desktop packager off macOS, still before any build — the "bundling desktop instead" override is gone, and a provider target needs a `replace` hook on `bundle` (`NoBundleReplacement`), which then runs on any host into `zig-out/bundle/<t>/`. Breaking (RFC "Migration", #410): `labelle wasm serve|export`, `labelle android …` and `labelle ios …` set the same override and go through the resolver, so without the pinned provider they fail with the no-provider error; the words stay reserved. `labelle targets` lists `desktop (core)` then `<t> provided by <package>` from discovery, replacing the hardcoded backend table. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…(RFC #406 phase 3b) `docs/provider-targets.md` covers the resolution rules, the registry hint, the labelle-assembler#378 boundary, `labelle bundle --platform=<t>`, `labelle targets` and the migration note (web/mobile projects add and pin their provider; `--platform=wasm` is unchanged in name). The hooks, dispatch, contract and RFC docs point at it. `test/provider_targets_e2e.py` drives the real CLI with a fake assembler that reports the target it was asked for: the no-provider error for `--platform=wasm`, the declared `.platform` and every legacy subcommand with no assembler invocation and nothing created; malformed values; the #378 message before the assembler runs; `NoBundleReplacement`; a provider replacing generate/build/bundle for `probe-target`, including `labelle bundle --platform=probe-target` running the replacement on every host with `output_dir == …/zig-out/bundle/probe-target` (and `--output`); a provider owning `wasm` making the assembler receive `--platform wasm`; and the `labelle targets` listing under a dead compiler and assembler. Wired into CI next to the hooks suite. The hooks suite's off-macOS bundle refusal now also asserts the cache stays untouched, since the gate runs after discovery. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
|
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 |
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. |
![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: 68dd0cd673
ℹ️ 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".
Comment on lines
+42
to
+43
| if (provider.meta.ownsTarget(requested)) | ||
| return .{ .name = requested, .provider = provider, .legacy = std.meta.stringToEnum(project.Platform, requested) }; |
There was a problem hiding this comment.
When a remote plugin has no matching labelle.providers.lock pin, provider_dispatch.discover can fall back to the legacy package cache and return it with Provider.verified = false, but this resolver accepts its target declaration anyway. For a schema-named target such as wasm with no hooks, nothing subsequently calls requirePinned, so generation and building can proceed based entirely on an unverified cached manifest, bypassing the provider integrity boundary. Require provider.verified before using a remote provider as a target owner.
Useful? React with 👍 / 👎.
| for (doc.providers) |pin| { | ||
| // Only a verified cached archive can say what a package declares; a | ||
| // release that is not cached, or whose bytes do not verify, says nothing. | ||
| const meta = (cachedManifest(a, pin) catch continue) orelse continue; |
There was a problem hiding this comment.
When an unresolved target triggers the registry hint lookup and several registry releases have cached archives, this loop repeatedly calls cachedManifest with the pipeline's arena allocator. Each call can allocate a 128 MiB compressed archive and up to 512 MiB of decompressed tar data, while its defer a.free(...) calls do not reclaim arena allocations, so buffers from every inspected release remain live until the command exits and the diagnostic can exhaust memory. Use a resettable scratch allocator per pin or stream only the manifest.
Useful? React with 👍 / 👎.
| else | ||
| null; | ||
| const bundle_out = try provider_hooks.stepOutputDir(hook_arena, target_dir, .bundle, hook_target, bundle_override); | ||
| const bundle_out = try provider_hooks.stepOutputDir(hook_arena, target_dir, .bundle, target.name, bundle_override); |
There was a problem hiding this comment.
When a provider declares an otherwise valid identifier that is a Windows device name, such as con, nul, or com1, the default provider bundle output uses that target verbatim as its final directory component. Windows cannot create such a directory, so labelle bundle --platform=<target> completes generation and building but fails before the replacement hook runs unless the user supplies --output. Either encode the target in filesystem paths or reject these names during target validation.
Useful? React with 👍 / 👎.
An `after build` hook reports under the build's phase (`compile`), but when Zig's progress stream has already advanced the reporter to `link` that request was a backward transition the machine silently rejected: the phase stayed `link`, the detail kept the linker's name, and no record was written — JSON/status consumers never saw the advertised `hook <package>/<id>` sub-step (Codex P2 on #420). `beginPhaseOrStep(.compile, …)` while in `link` is now a detail update of the live `link` phase (never a backward move); a genuinely earlier phase is still refused so the fold cannot mask a programmer error. The reporter spec drives `compileUpdate(saw_link)` into `link` and asserts the status file record carries `phase: link` with the hook detail. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
A PR stacked on a feature branch (#421 on feat/provider-hooks) has that branch as its base, so `branches: [main, development]` gave it no pull_request run at all. Add 'feat/**'; `push` is unchanged. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…e lock `provider_github.resolve --accept` parsed every selected manifest and validated ownership, but the cross-provider hook validation only ran through `provider_dispatch.discover`, so an accept could write `labelle.providers.lock` for providers with missing references, phase-order violations, duplicate replacements or cycles, and the freshly accepted project then failed on its next help/build (Codex P2 on #420). The accepted set — remote pins plus local providers, as `discover` would see it — is now run through `provider_hooks.validateAll` after the ownership check and before the lock is written. The GitHub e2e adds a registry provider whose `after_hooks` names a hook nobody declares: `--accept` fails with `MissingHookReference` (naming both hooks) and the working lock is byte-identical afterwards. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
The generate/build plans were used only for the initial `wasm serve --watch` build; `WasmRebuildCtx.rebuildStaged` called the assembler, the fingerprint pass and Zig directly, so `before`, `replace` and `after` hooks were all skipped on every subsequent file change — projects whose hooks generate inputs or post-process the WASM output reloaded stale or incomplete artifacts after the first watched edit, and a `replace` hook's step silently fell back to the core operation (Codex P2 on #420). The context now carries the cold pipeline's hook site, the `generate` and `build` plans and their step output directories, and wraps the watched core steps in the same before / core-or-replace / after phases. A failing hook is a new `HookFailed` stage: reported, server kept alive, browser not reloaded — like a failing core step. The feed is already terminal during the loop, so the hooks' sub-step records are no-ops there. The serve loop is interactive, so the subprocess e2e cannot drive it; the phase runner is an injectable field (production default asserted to be `provider_hooks.runPhase`) and a unit test on the context proves the plumbing: phases run in contract order with the right output directories, a `replace` plan stands in for the core step, and a failing or unstartable hook stops the rebuild before the next stage. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
The desktop run path ran an unconditional `zig build` right before spawning the game, AFTER the `after build` hooks had run. Those hooks were therefore not after the final build: a hook that signs, strips or patches `zig-out/` had its work overwritten by the warm build's install steps before launch, and a failure in that second build surfaced only after publishing-style after hooks had already run (Codex P2 on #420). Decision: remove the build rather than move the hook site. The core `build` step above (`core_build`, or its `replace` hook) always runs for `labelle run`, so the second build was truly redundant — a leftover of translating `zig build run` into build-then-exec in cli#265, kept only as a "warm-cache no-op". It was not a no-op for the install steps (Zig's `updateFile` re-copies any destination whose size/mtime differ, i.e. exactly a hook-patched file), it re-ran `zig build` even after a `replace` hook had stood in for the core build, and neither `enforceCliNotStale` nor the prebuild path depended on it. Nothing built in the run branch could be newer than the core build's output, so there is nothing for a warm build to pick up. Moving the hooks around it instead would have kept a second build whose only observable effect was undoing the hooks. The `ok()` doc no longer describes the removed build's exit code path. E2E: the fixture game's build now INSTALLS `data.txt` to `zig-out/bin/` and the game prints it at launch; with `PROVIDER_PROBE_PATCH=a-post` the `after build` hook overwrites that file and the launched game must print the patched contents. The mechanism is asserted too: a manual `zig build` on the same tree afterwards DOES revert the file, so the launch proving it intact means no build ran between the hook and the game, and "build ok" appears exactly once. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
On a clean checkout the pipeline discovered providers BEFORE the assembler's `install`, and `provider_dispatch.discover` treated a declared remote package with no cached `plugin.labelle` as runtime-only. The installer then fetched the provider and the build proceeded without its declared lifecycle hooks or integrity requirement, while the same command on a warm cache discovered the provider and refused it as unpinned (Codex P1 on #420). Discovery (and the hook plans) now run right after `gateThenInstall`, still before the plugin/core compatibility check, the lock write, generation and any compiler; a failure marks the progress feed `failed` ("provider discovery failed") since the reporter is live at that point. `discover` takes a `CacheState`: the pipeline passes `.populated`, under which a non-local package that is neither pinned nor in the ordinary cache is `ProviderPackageMissing` — a broken install, never runtime-only. A package DIRECTORY without a manifest stays runtime-only, because the assembler's light packs legitimately ship none (the stricter "no manifest = error" would refuse every light pack). `labelle help` and command dispatch, which run before any installer, pass `.unknown` and keep listing what exists. E2E (test/provider_hooks_e2e.py): the discovery-error cases now assert the install line IS present and the generate line is NOT (LABELLE_ZIG still points nowhere), plus the `failed` status record. New cold-cache cases with a fresh LABELLE_HOME and a remote `example/fixture-c`: (1) the installer delivers nothing → `ProviderPackageMissing` after install, before generation; (2) the installer populates the cache → the provider is discovered and its unpinned hook is refused with `RemoteProviderIntegrityRequired`, no core build; (3) a warm cache gives the identical outcome; (4) pinned via `labelle.providers.lock` + a seeded archive with a cold ordinary cache → the hook runs from the verified extraction. Never a silent skip. test/plugin_compat_e2e.py: the "absent manifest" fixture now lands the package directory at install (as the real assembler does) and omits only the manifest, which is what the case name means; an absent directory is the new fail-closed path. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Discovery timing (after the package cache is populated, before generation), the fail-closed `ProviderPackageMissing` rule and the light-pack exception, `--accept` validating the hook graph, the single build on the run path, the `link`-phase sub-step, watched wasm rebuilds running the hook phases, and the e2e coverage list. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…argets Forward-merges the #420 round-1 fixes (ff5e789..bb9f3d2) and resolves the discovery-order tension between the two PRs. #420 moved provider discovery to AFTER `gateThenInstall` (cache populated, `CacheState.populated`, fail closed on a declared non-local package missing from both caches). #421 had moved it EARLIER, because target resolution needs the providers before the reporter and target dir are set up. Resolution: split target resolution into two halves. - Name half, before the install (`provider_targets.provisional`): the requested string alone settles the name — `desktop` is core, any other well-formed name is provisionally a provider target. That is enough for the target dir, the progress feed and `parsed.platform` (legacy enum) that every pre-install step keys off. Two verdicts that need no provider land here: a project with no `.plugins` cannot own a provider target (refused before anything is read, written or built — the targets e2e's no-plugins cases stay `untouched`), and `labelle bundle` of the core target is refused off macOS before any install. - Ownership half, after the install (`confirmTarget` → `provider_targets.resolve`): right after `discover(.populated)`, the provisional target must be owned by a discovered provider, else `reportNoProvider` + a `failed` feed record ("no provider for target"). The hook plans, the labelle-assembler#378 gate and the provider bundle-replacement check follow at the same point — after the install, before the lock, generation and any compiler. - `wasm serve --no-build` installs nothing, so it confirms the name against a `.unknown` discovery (like `labelle targets`). - `labelle targets` passes `.unknown` (new `discover` signature). Conflicts: src/cli/pipeline.zig, docs/provider-hooks.md. Tests: targets e2e gains `installed_only` (install ran; no generate, no lock, nothing but the `.build-progress` record in the target dir) for the verdicts that need a declared package's manifest, a status record check, and the `--no-build` refusal; `reset()` clears the lock. A unit test covers `provisional` (enum-iterated, guard-clean). Verified: zig build, zig build test (1006/1011, 5 skipped), test-provider-dispatch (44), and the six Python suites (hooks 26, targets 28, dispatch 24, github 46, config 21, plugin_compat 8/8). Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Contributor
Author
|
Forward-merged Discovery-order resolution. #420 moved provider discovery to after
Consequence, accepted: with a package declared, "no provider for Verified on the merge commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d95a396338
ℹ️ 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".
| // refused when none declares it. This is the only place a provider | ||
| // target becomes a resolved one; nothing has been generated, locked or | ||
| // compiled yet, and the `failed` progress record names the reason. | ||
| const target = try confirmTarget(hook_arena, providers, requested_target) orelse { |
There was a problem hiding this comment.
When a project has any .plugins, an identifier-shaped typo such as --platform=waasm reaches this ownership check only after .prebuild commands, assembler resolution, ASTC conversion, and install have already run. This regresses the previous parser-level rejection: an invalid invocation can now execute arbitrary side-effecting prebuild steps, modify ASTC siblings, or fail while locating/downloading the assembler instead of reporting that no provider owns the target. Resolve or reject the target before those pre-install actions, while retaining a separate post-install verification if cold-cache discovery requires it.
Useful? React with 👍 / 👎.
| // replace, which treat it as the generic host baseline. `parsed.platform` | ||
| // is derived from the NAME only where the pinned assembler and the | ||
| // legacy sites still need the schema enum. | ||
| parsed.platform = provisional.legacy orelse .desktop; |
There was a problem hiding this comment.
For a valid provider target outside project_config.Platform, this fallback sets parsed.platform to .desktop. Consequently, when desktop asset compression is ASTC, the prepass at parsed.asset_compression.formatFor(parsed.platform) invokes cmdAstc with --platform desktop even though the resolved target is something like probe-target. That bypasses the new cmdAstc rejection for targets without a capability table and can leave the provider's generate replacement consuming ASTC siblings encoded according to desktop capabilities. Key the ASTC prepass on the requested target/provisional.legacy, and skip or reject conversion when the target has no ASTC table.
Useful? React with 👍 / 👎.
…ata-only discovery Under CacheState.unknown (labelle help, command dispatch) a declared remote package absent from both caches was skipped on purpose, but validateAll then reported MissingHookReference for any reference into it — hiding every provider's commands from help and refusing dispatch until the package happened to enter the cache (Codex P2 on #420). discover now records such packages as unresolved and validateAll defers only references into them; a reference into an undeclared package or into a present package without the hook is still missing, and .populated still fails closed on the absence. Unit-tested at the graph and at discover against a real cache layout in both states; e2e covers help staying intact, the pipeline failing on the absence rather than the reference, and the typo still reported. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
… are reachable serveAndOpen blocked in an unconditional accept loop and the process died on Ctrl+C, so the after run provider hooks that follow it never ran (Codex P2 on #420). A signal handler (POSIX) or console control handler (Windows, SetConsoleCtrlHandler, best-effort: compile-checked only) sets an atomic flag; a waker thread pokes the listener with one loopback connection so the blocked accept returns; the loop observes the flag and returns cleanly. A repeat Ctrl+C forces the exit. The connection-based wake is the one mechanism that behaves alike on every platform (std.posix.poll is a compile error on Windows; shutdown of a listener wakes accept on Linux but not macOS). The loop's flag and the waker's poke are unit-tested; the handler itself is interactive and not driven end to end. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
A wasm serve --watch session runs the generate and build hook phases on every saved edit through one long-lived Site whose allocator was the pipeline arena, so every hook's lock parse, settings, workspace paths, environment and serialised context stayed live until the server exited (Codex P2 on #420). Site gains a backing allocator; runPhase carves a scratch arena from it and frees it when the phase returns. Only the resolved host outlives a phase, on the long-lived arena; plans were already there. A counting allocator wrapper proves two phases on one site each allocate through the scratch and leave nothing live. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
A zero status was not a clean exit: runZigInheritWithEnv returns 0 after the --timeout watchdog kills the game (cli#390), and the iOS/Android deploy paths return as soon as simctl launch / am start does, with the app still running — so publishing or cleanup after run hooks ran after a forced timeout or right after a mobile launch (Codex P2 on #420). runner gains runInheritTerm returning an explicit Termination (exited / timed_out); runZigInheritWithEnv keeps its 0-on-timeout contract on top of it. provider_hooks gains RunOutcome (exited_clean, exited_error, timed_out, launched_detached); finishRun runs the after hooks only on exited_clean and otherwise prints one "labelle: after-run hooks skipped: <reason>" line, with the CLI's exit status unchanged. E2E: a --timeout kill of a hanging fixture game runs no after hook and prints the skip line; a clean exit still runs them. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…rovider-targets # Conflicts: # docs/provider-hooks.md # src/cli/pipeline.zig
…piler validateAll ordered the whole (step, target, phase) group once per hook, so N hooks in one phase ran the quadratic sort N times with every temporary left on the discovery arena. Track visited groups and order each distinct group once; a test seam counts orderPhase invocations. runPhase resolved the host compiler (running `zig version`, creating cache dirs) before requirePinned, so an unpinned remote hook on a machine without the pinned compiler reported ProviderCompilerMissing. Check every hook's pin first, as the command path does; a resolve_host seam proves the resolver is never reached for an unpinned provider. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…e-passes Both pre-passes read the declared PNGs, so a `before generate` hook that produces one ran too late: `generate --bake` failed on the missing file and a hook-generated PNG never got its `.astc` sibling. Move both pre-passes into the core generate step, after the before hooks; a `replace generate` hook now stands in for them too (documented). The fixture tool gains PROVIDER_PROBE_COPY so a hook can produce a generation input; the e2e drives `generate --bake` with the hook writing the declared PNG (baked, then generated) and proves the mechanism by running the same command without the hook's file (bake fails first). Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
`labelle build` finalizes the compiled tree (the Linux .desktop entry and the installable package on the platform that produces one) AFTER the `after build` hooks ran, so a signing/inspecting/publishing hook never saw the final artifact and reported success even when packaging then failed. Move the finalization inside the core build step, ahead of the after hooks; a `replace build` hook now owns it (documented). The fixture tool snapshots the step output directory in each hooks.log line; the e2e drives `build --linux-desktop` and asserts the after hooks list the entry while the before hooks do not. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…refusal The targets slice added `assert not home.exists()` to the off-macOS `bundle` case, but by that point the cold-cache section has legitimately populated the cache (the pinned-provider archive, the remote package the fake install delivered), so the assertion failed on ubuntu-latest and cancelled the other hosts. The pipeline's core-desktop gate already sits before the install; what the test means is that the refusal changes nothing in the cache — so snapshot it before and compare after. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…can be With `.plugins` present, an identifier-shaped typo (`--platform=waasm`) reached the ownership check only after `.prebuild`, the assembler resolution, the ASTC prepass and `install` had run (Codex on #421). Restore an early verdict: for a provider target the pipeline first runs the same metadata-only discovery `labelle targets` does (`.unknown`: cached manifests, no installer). `discoverAll` now also reports the declared remote packages it could not read; when that list is empty every declared package was readable and the verdict — no owner, or an unpinned owner — is the one the post-install check would reach, so it lands here with the same diagnostics and nothing run. A manifest that fails discovery fails closed here too: the install cannot mend a manifest it can already read. Only a partial view (cold cache, no pin) defers to the post-install discovery, which stays the authoritative check with its `failed` record. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…table A provider target outside the `Platform` enum derives `parsed.platform = .desktop` for the legacy sites, so with desktop atlases set to ASTC the prepass ran `cmdAstc --platform desktop` for e.g. `probe-target`, encoding siblings by desktop capabilities for a provider's own `generate` to pick up and bypassing `cmdAstc`'s rejection of such a name (Codex on #421). Key the prepass on `provisional.legacy`: only `desktop` and the schema-named provider targets have a table; a provider owns its target's asset pipeline. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
The targets e2e now checks: a local provider's non-ownership refused before anything runs; an identifier-shaped typo leaving a `.prebuild` marker step unrun while a resolving target runs it; a remote package on a cold cache deferring the verdict to after the install (the `failed` record stays the authoritative path) and, unpinned, refused as an owner both after the install and — warm — before it, with the cache untouched; the `(unpinned)` mark in `labelle targets`; and the ASTC prepass running for `desktop` but not for `probe-target`. The fake assembler's `install` can populate the cache (the hooks-suite pattern). docs/provider-targets.md describes the early verdict, the pinned-owner rule, the reserved-device-name rule, the bounded registry-hint scan and the Docker fixture; the contract's name rule gains the device-name clause. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Contributor
Author
Round-1 fixes — d70dcb0 (merged
|
The --no-build branch returned before provider discovery, so every declared before/replace/after run hook was silently skipped although serving or exporting the existing artifact is the same run step the building path hooks. Discover (metadata-only, nothing is installed on this path) and run the run plan around the export or the serve loop, skipping only generate and build. E2E: `wasm export --no-build` over a pre-made web dir runs the wasm run hooks around the export with no install/generate/build, and a failing before hook stops the export with the hook's exit code. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
There was a problem hiding this comment.
💡 Codex Review
labelle-cli/src/cli/pipeline.zig
Lines 1497 to 1499 in d70dcb0
Reject build numbers for provider bundle replacements
When bundle --platform=<provider-target> --build-number=<n> is used, this replacement branch skips bundle.createFromBuild, which is the only consumer of parsed_args.bundle_build_number. Bundle hooks receive no trailing arguments and the provider context has no build-number field, so the command succeeds while silently ignoring the explicitly requested version. Either expose the value to the provider contract or reject --build-number for provider targets before generation and build.
ℹ️ 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".
| inspected += 1; | ||
| continue; | ||
| }, | ||
| } orelse continue; |
There was a problem hiding this comment.
When the cached registry references valid cached archives that contain no plugin.labelle, cachedManifest returns null and this orelse continue skips incrementing inspected. Such archives have already been fully read, verified, validated, and decompressed, so a registry containing many of them bypasses registry_hint_scan_limit and can perform an effectively unbounded number of up-to-512 MiB decompressions merely to print a missing-target diagnostic. Increment the counter for every cached archive inspected, including archives with no manifest.
Useful? React with 👍 / 👎.
Comment on lines
+1119
to
+1121
| const target = try confirmTarget(hook_arena, providers, requested_target) orelse { | ||
| if (reporter) |r| r.finishFailed(1, "no provider for target"); | ||
| return 1; |
There was a problem hiding this comment.
When post-install resolution rejects an unverified remote owner, confirmTarget prints the correct unpinned-owner diagnostic but returns the same null used for a genuinely absent owner, so build/run/bundle then persist detail: "no provider for target". Consumers of labelle status --json therefore receive the wrong failure reason and may advise adding a provider rather than pinning the declared one. Preserve the resolution failure kind and write a distinct unpinned-owner status detail.
Useful? React with 👍 / 👎.
`wasm serve --watch` captured the hook plans and the site's config once at startup and reused them for every rebuild, so a watched edit to project.labelle, a provider manifest or provider_config never reached the lifecycle: removed hooks kept running and added ones were skipped until the server restarted. WasmRebuildCtx gains a per-rebuild `replan` seam, run after the prebuild steps and before any phase; production installs WatchReplan, which re-reads the project, rediscovers with the cache populated and replans both phases on a fresh arena, installing the result only on success (a failed replan stops that rebuild and keeps the last good plans). Unit tests cover the seam (invoked once per rebuild, the fresh plans are what run, failure stops before any phase) and the production replan against a real temp project (manifest edits, a broken manifest, a dropped plugin). Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
… fixes fixture-c gains a `before generate` hook so the cold-cache unpinned build refuses it before any compiler: with LABELLE_ZIG pointing nowhere the error is RemoteProviderIntegrityRequired, never ProviderCompilerMissing, and nothing is generated; once pinned, the same dead compiler is the failure (the mechanism). docs/provider-hooks.md covers the once-per-group validation, the pin order and the new e2e/unit coverage. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…al in status Round-2 Codex findings on #421: - provider_github.cachedOwner: a cached archive that verifies but carries no `plugin.labelle` was read, verified and decompressed whole yet not counted against `registry_hint_scan_limit`, so a registry of such releases could read past the bound. Count every archive read; unit test seeds `limit` manifest-less releases ahead of the owner and shows the owner is not read (and is, one release fewer). - pipeline.confirmTarget: an unverified (unpinned) remote owner returned the same null as an absent owner, so build/run/bundle persisted `detail: "no provider for target"` for both. The refusal kind now survives to the `failed` progress record as `"unpinned provider for target"`; the e2e reads both records. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Contributor
Author
|
Round-2 Codex findings addressed in a351851:
bbd5189 merges the base branch's round-3 fixes ( Verified locally on the merged tree: |
resolveBuiltExe looked for '<name>' and 'game' in zig-out/bin, but Zig installs '<name>.exe' on a Windows host, so 'build --linux-desktop' (and bundle) failed with BinaryNotFound there. Append the host exe suffix to the declared name and both fallbacks. Fixes the Windows CI job on #420. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
…argets Conflicts in src/cli/pipeline.zig and docs/provider-hooks.md, resolved keeping both sides: - The hooks side moved the ASTC and --bake pre-passes inside the core `generate` step (after the `before generate` hooks) and the build finalization (Linux .desktop entry, APK) inside the core build. The targets side's guards follow the code: the ASTC pre-pass runs only for a target the capability tables know (`target.legacy != null`) and the Linux entry only for core desktop (`target.provider == null`). - `wasm serve|export --no-build` plans its run hooks from the discovery the served target was just confirmed against (`known`, `served.name`) instead of a second discovery keyed on the legacy platform name. - The hooks e2e's `wasm export --no-build` section declares `wasm` on fixture-a for its span (a provider target needs an owner) and asserts the undeclared refusal first; both sides' cases run. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
0004dc8 put the suffix into ResolvedExe.name, so the Linux desktop entry became 'game.exe.desktop' on Windows. The name stays '<name>'. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbd5189e66
ℹ️ 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".
| .asm_bin = asm_bin, | ||
| .project_dir = project_dir, | ||
| .platform_tag = @tagName(parsed.platform), | ||
| .platform_tag = target.name, |
There was a problem hiding this comment.
When wasm serve --watch observes a project.labelle edit that removes the target-owning plugin—or makes its remote owner unpinned—the WatchReplan.run path rediscovers providers and replans hooks but never calls provider_targets.resolve again. It then installs empty plans and the rebuild continues with assembler generate --platform wasm, bypassing the new requirement that every non-desktop target have a pinned owner. Revalidate ctx.hooks.target against the newly discovered providers before installing the replanned state.
Useful? React with 👍 / 👎.
| // cannot mend a manifest it can already read. Never for the core | ||
| // target, which needs no provider. | ||
| if (!provisional.is_core) { | ||
| var early_sources: provider_github.Sources = .{ .a = hook_arena }; |
There was a problem hiding this comment.
When a non-core target is used with pinned remote providers, this early discovery passes the pipeline's long-lived arena to Sources.fromPin; reading one archive can allocate up to 128 MiB of compressed data and 512 MiB of decompressed tar data, and its free calls do not reclaim arena allocations. The authoritative post-install discovery then extracts the same providers again on the same arena, so valid large providers can consume twice their archive footprint (and wasm serve --no-build retains both copies for the server's lifetime). Run this metadata-only pass on a disposable arena that is deinitialized when the early check finishes.
Useful? React with 👍 / 👎.
Comment on lines
+262
to
+268
| switch (parseBundleValueFlag(arg, args, "platform", "--platform=<target>") orelse return null) { | ||
| .value => |v| { | ||
| result.platform = parseTargetValue(v) orelse { | ||
| printInvalidTarget("bundle", v); | ||
| return null; | ||
| }; | ||
| continue; |
There was a problem hiding this comment.
When bundle --platform=<provider-target> is combined with --build-number, parsing and validation succeed, but the replacement hook receives neither that value in LABELLE_CONTEXT nor as an argument. The only consumer of bundle_build_number is bundle.createFromBuild, which is skipped whenever the required provider replacement runs, so release automation can report success while silently producing an artifact without the requested build number. Either expose the value to provider bundle hooks or reject this option combination.
Useful? React with 👍 / 👎.
…ery, hand bundle hooks --build-number - WatchReplan.run confirms ctx.hooks.target against the rediscovered providers before installing anything: an edit that drops the owning package or unpins a remote owner fails that rebuild with NoProviderForTarget / UnverifiedTargetOwner (and the cold pipeline's diagnostic) and keeps the previous generation (Codex P1 on #421). - The pre-install ownership check runs in earlyTargetCheck on a scratch arena freed before it returns; only the verdict leaves, so pinned archives are no longer held twice or for a --no-build server's lifetime (Codex P2 on #421). Tested by counting the backing allocator. - LABELLE_CONTEXT gains an optional build_number, set only for bundle-step hooks when `bundle --build-number` was given and omitted (not null) otherwise; documented in provider-contract-v1.md and provider-hooks.md; e2e asserts the bundle replacement's captured context carries it and the build hook's does not (Codex P2 on #421). Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Contributor
Author
|
Round 3 (final) fixes, pushed as Windows CI (run 36258640392). This was not the #420 Codex P1: revalidate the target on watched rebuilds ( Codex P2: free the early discovery. A new Codex P2: Verified: |
There was a problem hiding this comment.
💡 Codex Review
Lines 281 to 287 in cad4979
Defer build-number validation to provider bundles
When bundling a provider target, this parser still applies the core macOS CFBundleVersion grammar before target resolution, even though the value is now forwarded to a provider-defined packager. This rejects legitimate provider-specific build numbers—for example Android version code 10000 is refused because Apple's first component is limited to four digits—so the provider never gets a chance to enforce its own platform rules. Apply bundle.validateBuildNumber only for the core desktop packager, while retaining the context contract's non-empty check for provider targets.
ℹ️ 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".
Comment on lines
+35
to
+38
| /// The build number `labelle bundle --build-number=N` was given, for the | ||
| /// provider that packages the target (its `bundle` hooks); the core | ||
| /// packager stamps it itself. Optional on the wire (see above). | ||
| build_number: ?[]const u8 = null, |
There was a problem hiding this comment.
When bundle --build-number invokes a provider built against the existing 1.0.0 wire contract, the CLI now emits build_number while still advertising contract_version: "1.0.0". The v1 contract requires unknown fields to be rejected, and the previous Context decoder does exactly that, so already-published bundle hooks can fail parsing instead of packaging. Introduce a negotiated contract version for this field, or otherwise preserve compatibility with strict 1.0.0 consumers.
Useful? React with 👍 / 👎.
Comment on lines
+639
to
+645
| switch (try confirmTarget(a, providers, ctx.hooks.target)) { | ||
| .resolved => {}, | ||
| .refused => |kind| return switch (kind) { | ||
| .no_provider => error.NoProviderForTarget, | ||
| .unpinned_owner => error.UnverifiedTargetOwner, | ||
| }, | ||
| } |
There was a problem hiding this comment.
When a wasm serve --watch edit removes or unpins the target owner, this new check is reached through rebuildStaged only after that function has already run the startup .prebuild commands. Thus an invalid target configuration can still execute arbitrary side-effecting prebuild work once before the rebuild is refused. Fresh evidence beyond the earlier ownership finding is the current ordering: prebuild.runAll is at lines 168–171, while this confirmTarget call runs later during replan; perform the re-read and ownership check before prebuild execution.
Useful? React with 👍 / 👎.
apotema
added a commit
that referenced
this pull request
Sep 26, 2026
* feat(providers): hook manifest rules, planner and dispatch refactor (RFC #406 phase 3a) Manifest: a `replace` hook must name a target its own package declares (ReplaceRequiresOwnedTarget); `after_hooks` entries are `<package>/<id>` references, never self (InvalidHookReference); `ownsTarget`, `splitHookRef`, `qualifiedId`. Planner (`provider_hooks.zig`): `validateAll` checks the cross-provider graph once at discovery — DuplicateReplaceHook, MissingHookReference, HookReferenceMismatch, HookPhaseOrder, HookCycle — and `plan` orders each (step, target) as before → replace → after, Kahn's algorithm on the `after_hooks` edges with ties by qualified id, so the plan is independent of manifest read order. `stepOutputDir` is the step output-layout contract; `runPhase`/`finishRun` execute a phase and propagate a failing hook's exit code. Dispatch: `Provider`/`discover` are pub and `discover` validates the hook graph; `execute` is split into `resolveHost` + `runTool` over a `ToolRun` (command dispatch is a thin wrapper, its e2e unchanged); the lock read, pin check and integrity gate move into `requirePinned`; `resolveSettings` and `canonicalDir` are pub. Progress: `Reporter.beginPhaseOrStep` renames the sub-step inside the live phase, since the machine rejects a phase re-entering itself and hooks share the phase of the core step they wrap. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * feat(providers): run lifecycle hooks in the pipeline; move the .app default (RFC #406 phase 3a) Discovery runs once per pipeline invocation (only when `.plugins` is non-empty), before the reporter, the assembler or any compiler, and a failed discovery exits 1 with `provider discovery failed: <error>`. The four plans are computed up front; a project without hooks never resolves the host compiler. Hook points, each `before` → core or unique `replace` → `after`, stopping at the first failure and exiting with the hook's own code: - generate: around assembler generate + fingerprints + emsdk activation, output_dir = the generated tree; - build: around docker/host `zig build` + DLL staging, output_dir = zig-out; - bundle: around `bundle.createFromBuild`, output_dir = zig-out/bundle/<target> or the resolved --output; - run: `before` once ahead of every branch, `after` at each success exit via `finishRun` (only on a clean game exit, before the terminal `done` record; `wasm serve` runs them once the server returns). `labelle.lock` is written right after `validatePluginCoreCompat`, before generation, so a `before generate` hook already sees it. Core steps report through `beginPhaseOrStep` so a hook and the core step can share a phase. The desktop `.app` default moves to `<target_dir>/zig-out/bundle/desktop/`, the step output directory bundle hooks receive, so hooks and the core packager agree; `--output` is unchanged. Help text names the hooks doc. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * docs+test(providers): hooks doc and hermetic subprocess regression (RFC #406 phase 3a) `docs/provider-hooks.md` covers attach rules, discovery-time graph errors, ordering (after_hooks edges only, ties by qualified id), the output-layout table, the hook context, failure semantics and the limitations (no JSON relay, legacy platform handlers, bundle still macOS-only). The dispatch, contract and GitHub-pins docs point at it. `test/provider_hooks_e2e.py` drives the real CLI with a fake assembler (the plugin_compat pattern; a `.cmd` shim on Windows) and two fixture providers declared in reverse order: order regardless of declaration order, an `after_hooks` edge inverting the tie, context fields, a failing before hook's exit code with no core build and no after hook, a failing core build skipping after, ReplaceRequiresOwnedTarget / DuplicateReplaceHook / MissingHookReference / HookPhaseOrder at `labelle help` before any compiler, generate hooks seeing the lock, run hooks around the game, bundle hooks and the .app location on macOS, the refusal elsewhere. The fixture tool appends one JSON line per invocation to `<output_dir>/hooks.log` and fails on `PROVIDER_PROBE_FAIL=<hook id>`. Wired into CI next to the other provider suites on all three hosts. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(progress): report a compile-phase hook as a sub-step while in link An `after build` hook reports under the build's phase (`compile`), but when Zig's progress stream has already advanced the reporter to `link` that request was a backward transition the machine silently rejected: the phase stayed `link`, the detail kept the linker's name, and no record was written — JSON/status consumers never saw the advertised `hook <package>/<id>` sub-step (Codex P2 on #420). `beginPhaseOrStep(.compile, …)` while in `link` is now a detail update of the live `link` phase (never a backward move); a genuinely earlier phase is still refused so the fold cannot mask a programmer error. The reporter spec drives `compileUpdate(saw_link)` into `link` and asserts the status file record carries `phase: link` with the hook detail. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * ci: run pull_request CI for PRs based on feat/** branches A PR stacked on a feature branch (#421 on feat/provider-hooks) has that branch as its base, so `branches: [main, development]` gave it no pull_request run at all. Add 'feat/**'; `push` is unchanged. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(providers): validate the hook graph at resolve --accept before the lock `provider_github.resolve --accept` parsed every selected manifest and validated ownership, but the cross-provider hook validation only ran through `provider_dispatch.discover`, so an accept could write `labelle.providers.lock` for providers with missing references, phase-order violations, duplicate replacements or cycles, and the freshly accepted project then failed on its next help/build (Codex P2 on #420). The accepted set — remote pins plus local providers, as `discover` would see it — is now run through `provider_hooks.validateAll` after the ownership check and before the lock is written. The GitHub e2e adds a registry provider whose `after_hooks` names a hook nobody declares: `--accept` fails with `MissingHookReference` (naming both hooks) and the working lock is byte-identical afterwards. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(wasm): run the generate/build hook phases on every watched rebuild The generate/build plans were used only for the initial `wasm serve --watch` build; `WasmRebuildCtx.rebuildStaged` called the assembler, the fingerprint pass and Zig directly, so `before`, `replace` and `after` hooks were all skipped on every subsequent file change — projects whose hooks generate inputs or post-process the WASM output reloaded stale or incomplete artifacts after the first watched edit, and a `replace` hook's step silently fell back to the core operation (Codex P2 on #420). The context now carries the cold pipeline's hook site, the `generate` and `build` plans and their step output directories, and wraps the watched core steps in the same before / core-or-replace / after phases. A failing hook is a new `HookFailed` stage: reported, server kept alive, browser not reloaded — like a failing core step. The feed is already terminal during the loop, so the hooks' sub-step records are no-ops there. The serve loop is interactive, so the subprocess e2e cannot drive it; the phase runner is an injectable field (production default asserted to be `provider_hooks.runPhase`) and a unit test on the context proves the plumbing: phases run in contract order with the right output directories, a `replace` plan stands in for the core step, and a failing or unstartable hook stops the rebuild before the next stage. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(run): launch the built binary without a second warm `zig build` The desktop run path ran an unconditional `zig build` right before spawning the game, AFTER the `after build` hooks had run. Those hooks were therefore not after the final build: a hook that signs, strips or patches `zig-out/` had its work overwritten by the warm build's install steps before launch, and a failure in that second build surfaced only after publishing-style after hooks had already run (Codex P2 on #420). Decision: remove the build rather than move the hook site. The core `build` step above (`core_build`, or its `replace` hook) always runs for `labelle run`, so the second build was truly redundant — a leftover of translating `zig build run` into build-then-exec in cli#265, kept only as a "warm-cache no-op". It was not a no-op for the install steps (Zig's `updateFile` re-copies any destination whose size/mtime differ, i.e. exactly a hook-patched file), it re-ran `zig build` even after a `replace` hook had stood in for the core build, and neither `enforceCliNotStale` nor the prebuild path depended on it. Nothing built in the run branch could be newer than the core build's output, so there is nothing for a warm build to pick up. Moving the hooks around it instead would have kept a second build whose only observable effect was undoing the hooks. The `ok()` doc no longer describes the removed build's exit code path. E2E: the fixture game's build now INSTALLS `data.txt` to `zig-out/bin/` and the game prints it at launch; with `PROVIDER_PROBE_PATCH=a-post` the `after build` hook overwrites that file and the launched game must print the patched contents. The mechanism is asserted too: a manual `zig build` on the same tree afterwards DOES revert the file, so the launch proving it intact means no build ran between the hook and the game, and "build ok" appears exactly once. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(providers): discover providers after the package cache is populated On a clean checkout the pipeline discovered providers BEFORE the assembler's `install`, and `provider_dispatch.discover` treated a declared remote package with no cached `plugin.labelle` as runtime-only. The installer then fetched the provider and the build proceeded without its declared lifecycle hooks or integrity requirement, while the same command on a warm cache discovered the provider and refused it as unpinned (Codex P1 on #420). Discovery (and the hook plans) now run right after `gateThenInstall`, still before the plugin/core compatibility check, the lock write, generation and any compiler; a failure marks the progress feed `failed` ("provider discovery failed") since the reporter is live at that point. `discover` takes a `CacheState`: the pipeline passes `.populated`, under which a non-local package that is neither pinned nor in the ordinary cache is `ProviderPackageMissing` — a broken install, never runtime-only. A package DIRECTORY without a manifest stays runtime-only, because the assembler's light packs legitimately ship none (the stricter "no manifest = error" would refuse every light pack). `labelle help` and command dispatch, which run before any installer, pass `.unknown` and keep listing what exists. E2E (test/provider_hooks_e2e.py): the discovery-error cases now assert the install line IS present and the generate line is NOT (LABELLE_ZIG still points nowhere), plus the `failed` status record. New cold-cache cases with a fresh LABELLE_HOME and a remote `example/fixture-c`: (1) the installer delivers nothing → `ProviderPackageMissing` after install, before generation; (2) the installer populates the cache → the provider is discovered and its unpinned hook is refused with `RemoteProviderIntegrityRequired`, no core build; (3) a warm cache gives the identical outcome; (4) pinned via `labelle.providers.lock` + a seeded archive with a cold ordinary cache → the hook runs from the verified extraction. Never a silent skip. test/plugin_compat_e2e.py: the "absent manifest" fixture now lands the package directory at install (as the real assembler does) and omits only the manifest, which is what the case name means; an absent directory is the new fail-closed path. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * docs(providers): document the round-1 review fixes on hooks Discovery timing (after the package cache is populated, before generation), the fail-closed `ProviderPackageMissing` rule and the light-pack exception, `--accept` validating the hook graph, the single build on the run path, the `link`-phase sub-step, watched wasm rebuilds running the hook phases, and the e2e coverage list. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(providers): defer hook references into an unread package at metadata-only discovery Under CacheState.unknown (labelle help, command dispatch) a declared remote package absent from both caches was skipped on purpose, but validateAll then reported MissingHookReference for any reference into it — hiding every provider's commands from help and refusing dispatch until the package happened to enter the cache (Codex P2 on #420). discover now records such packages as unresolved and validateAll defers only references into them; a reference into an undeclared package or into a present package without the hook is still missing, and .populated still fails closed on the absence. Unit-tested at the graph and at discover against a real cache layout in both states; e2e covers help staying intact, the pipeline failing on the absence rather than the reference, and the typo still reported. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(wasm): stop the serve loop on Ctrl+C / SIGTERM so after-run hooks are reachable serveAndOpen blocked in an unconditional accept loop and the process died on Ctrl+C, so the after run provider hooks that follow it never ran (Codex P2 on #420). A signal handler (POSIX) or console control handler (Windows, SetConsoleCtrlHandler, best-effort: compile-checked only) sets an atomic flag; a waker thread pokes the listener with one loopback connection so the blocked accept returns; the loop observes the flag and returns cleanly. A repeat Ctrl+C forces the exit. The connection-based wake is the one mechanism that behaves alike on every platform (std.posix.poll is a compile error on Windows; shutdown of a listener wakes accept on Linux but not macOS). The loop's flag and the waker's poke are unit-tested; the handler itself is interactive and not driven end to end. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(hooks): run each phase on a scratch arena freed on return A wasm serve --watch session runs the generate and build hook phases on every saved edit through one long-lived Site whose allocator was the pipeline arena, so every hook's lock parse, settings, workspace paths, environment and serialised context stayed live until the server exited (Codex P2 on #420). Site gains a backing allocator; runPhase carves a scratch arena from it and frees it when the phase returns. Only the resolved host outlives a phase, on the long-lived arena; plans were already there. A counting allocator wrapper proves two phases on one site each allocate through the scratch and leave nothing live. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(run): run after-run hooks only when the game itself exited clean A zero status was not a clean exit: runZigInheritWithEnv returns 0 after the --timeout watchdog kills the game (cli#390), and the iOS/Android deploy paths return as soon as simctl launch / am start does, with the app still running — so publishing or cleanup after run hooks ran after a forced timeout or right after a mobile launch (Codex P2 on #420). runner gains runInheritTerm returning an explicit Termination (exited / timed_out); runZigInheritWithEnv keeps its 0-on-timeout contract on top of it. provider_hooks gains RunOutcome (exited_clean, exited_error, timed_out, launched_detached); finishRun runs the after hooks only on exited_clean and otherwise prints one "labelle: after-run hooks skipped: <reason>" line, with the CLI's exit status unchanged. E2E: a --timeout kill of a hanging fixture game runs no after hook and prints the skip line; a clean exit still runs them. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * docs(providers): document the round-2 review fixes on hooks Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(hooks): validate each phase group once; check pins before the compiler validateAll ordered the whole (step, target, phase) group once per hook, so N hooks in one phase ran the quadratic sort N times with every temporary left on the discovery arena. Track visited groups and order each distinct group once; a test seam counts orderPhase invocations. runPhase resolved the host compiler (running `zig version`, creating cache dirs) before requirePinned, so an unpinned remote hook on a machine without the pinned compiler reported ProviderCompilerMissing. Check every hook's pin first, as the command path does; a resolve_host seam proves the resolver is never reached for an unpinned provider. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(hooks): run before-generate hooks ahead of the ASTC and --bake pre-passes Both pre-passes read the declared PNGs, so a `before generate` hook that produces one ran too late: `generate --bake` failed on the missing file and a hook-generated PNG never got its `.astc` sibling. Move both pre-passes into the core generate step, after the before hooks; a `replace generate` hook now stands in for them too (documented). The fixture tool gains PROVIDER_PROBE_COPY so a hook can produce a generation input; the e2e drives `generate --bake` with the hook writing the declared PNG (baked, then generated) and proves the mechanism by running the same command without the hook's file (bake fails first). Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(hooks): run after-build hooks once the build's finalization is done `labelle build` finalizes the compiled tree (the Linux .desktop entry and the installable package on the platform that produces one) AFTER the `after build` hooks ran, so a signing/inspecting/publishing hook never saw the final artifact and reported success even when packaging then failed. Move the finalization inside the core build step, ahead of the after hooks; a `replace build` hook now owns it (documented). The fixture tool snapshots the step output directory in each hooks.log line; the e2e drives `build --linux-desktop` and asserts the after hooks list the entry while the before hooks do not. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(hooks): keep the run hooks on wasm serve|export --no-build The --no-build branch returned before provider discovery, so every declared before/replace/after run hook was silently skipped although serving or exporting the existing artifact is the same run step the building path hooks. Discover (metadata-only, nothing is installed on this path) and run the run plan around the export or the serve loop, skipping only generate and build. E2E: `wasm export --no-build` over a pre-made web dir runs the wasm run hooks around the export with no install/generate/build, and a failing before hook stops the export with the hook's exit code. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(hooks): replan the provider hooks on every watched rebuild `wasm serve --watch` captured the hook plans and the site's config once at startup and reused them for every rebuild, so a watched edit to project.labelle, a provider manifest or provider_config never reached the lifecycle: removed hooks kept running and added ones were skipped until the server restarted. WasmRebuildCtx gains a per-rebuild `replan` seam, run after the prebuild steps and before any phase; production installs WatchReplan, which re-reads the project, rediscovers with the cache populated and replans both phases on a fresh arena, installing the result only on success (a failed replan stops that rebuild and keeps the last good plans). Unit tests cover the seam (invoked once per rebuild, the fresh plans are what run, failure stops before any phase) and the production replan against a real temp project (manifest edits, a broken manifest, a dropped plugin). Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * test(hooks): e2e for the pin-before-compiler order; document rounds 3 fixes fixture-c gains a `before generate` hook so the cold-cache unpinned build refuses it before any compiler: with LABELLE_ZIG pointing nowhere the error is RemoteProviderIntegrityRequired, never ProviderCompilerMissing, and nothing is generated; once pinned, the same dead compiler is the failure (the mechanism). docs/provider-hooks.md covers the once-per-group validation, the pin order and the new e2e/unit coverage. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(bundle): find the host-built game as <name>.exe on Windows resolveBuiltExe looked for '<name>' and 'game' in zig-out/bin, but Zig installs '<name>.exe' on a Windows host, so 'build --linux-desktop' (and bundle) failed with BinaryNotFound there. Append the host exe suffix to the declared name and both fallbacks. Fixes the Windows CI job on #420. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(bundle): keep the resolved exe name bare; only the path carries .exe 0004dc8 put the suffix into ResolvedExe.name, so the Linux desktop entry became 'game.exe.desktop' on Windows. The name stays '<name>'. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * style(bundle): keep ResolvedExe's doc comment on the struct Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(bundle): find the built game as <name> or <name>.exe 25d5faa required the host suffix, so Windows unit tests that stage a bare 'game' fixture failed with BinaryNotFound (18 tests). existingExe tries the bare name first, then '<name><exe_ext>' (what Zig installs on a Windows host); the resolved name stays bare either way. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Brings in 1f1b6e2 (resolveBuiltExe accepts <name> or <name>.exe), the only #420 change this branch did not already carry; fixes the 18 Windows BinaryNotFound test failures. All other conflicts were the squash re-adding #420 content already merged here, resolved to this branch's version. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Contributor
Author
|
Merged into |
apotema
added a commit
that referenced
this pull request
Sep 26, 2026
* fix(providers): negotiate the wire version; build_number is contract 1.1.0 (#424) The CLI emitted `build_number` while still advertising contract_version "1.0.0", whose strict decoders reject unknown fields. The CLI now implements contract 1.1.0 (1.0.0 plus the optional build_number key) and still speaks 1.0.0: each invocation negotiates the newest wire the provider's `command_contract` range admits, and a 1.0.0 context never carries the key (the drop is reported once on stderr). Documented as an additive minor. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(progress): clear step counters on sub-steps; let a serve revise its done (#423) A same-phase sub-step (a hook after a compile update) kept the compiler's step/total, so hook records and the following terminal record reported a stale percentage. `beginPhaseOrStep` now resets them as `beginPhaseLocked` does. `reviseDoneAsFailed` is the one sanctioned exception to terminal absorption, for the interactive serve whose `done` lands before its loop. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(hooks): restore the core detail after before phases; serve after-hook failures fail the feed (#423) `runBefore` re-enters the core step's progress detail once a before phase succeeds, so the feed stops naming the last hook through the core work. `finishServe` runs the serve's after-run hooks and revises the provisional `done` to `failed` when one fails or cannot start. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(serve): bound a self-writing rebuild to one follow-up in --watch (#423) Lifecycle hooks declare no outputs, so a hook writing into the watched tree re-triggered the watcher forever. The loop now also snapshots the tree after each callback; a rebuild fired for exactly that snapshot is a follow-up, and whatever its own callback writes is taken as built. Edits saved during an ordinary rebuild still fire the next one. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(pipeline): hooks and watch follow-ups from #420/#421 review (#423, #424) - watched rebuild: replan (re-read, ownership check) runs before prebuild - watched replan: re-install and rewrite labelle.lock when project.labelle changed, before discovery / after the plans are good - shader override re-gated after the before-generate hooks (cold + watched) - replan storage kept alive through the shutdown after-run hooks, and the site restored to startup storage on release - serve after-run hook failures revise the feed; before-generate/bundle phases hand the progress detail back to the core step - a cross-target `--docker --target` run is skipped before any run hook Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(serve,pipeline): keep edits saved during a follow-up rebuild; replan after prebuild (#427 review) serve: each rebuild is bracketed by per-path snapshots (start/post); a follow-up settles only when the paths it changed are a subset of the previous rebuild's, so a self-writing hook still ends after one extra rebuild while an edit saved during the follow-up stays pending. pipeline: the watched rebuild now pre-checks target ownership (metadata only, no install/lock/plan) before prebuild, and runs the full rediscover + replan after prebuild so generated provider manifests are seen by the same rebuild. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP * fix(serve,pipeline): shutdown runs the replanned after-run hooks; cap varying-path follow-ups (#427 review) Codex P2 on #427 (pipeline.zig:2963): a watched serve's shutdown ran the startup `hook_plans.run.after`, so after a replan bumped a provider's version it failed with StaleProviderPin against the rewritten lock, and added/removed after-run hooks were ignored. Each WatchReplan generation now plans the `run` phase too, and the shutdown runs the current generation's after-run hooks (`shutdownRunAfter`), falling back to the startup plan when no rebuild replanned. Codex P2 on #427 (serve.zig:720): a writer that changes a different path on every run (a timestamp-named report) never satisfied the subset rule and rebuilt forever. WatchBaseline now tracks each follow-up chain's delta union and the writers' per-run new-path footprint: from the 2nd follow-up on, a follow-up adding no more new-to-the-chain paths than the footprint settles, logging "labelle: watch: settled after N follow-up rebuilds triggered by build outputs". A follow-up that also carries a user edit exceeds the footprint and still fires one more rebuild; an edit saved between rebuilds always rebuilds and restarts the chain. An 8-follow-up ceiling settles unconditionally so no writer can loop. Claude-Session: https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
This was referenced Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 3b of RFC #406, stacked on #420 (lifecycle hooks).
--platform=<t>is now resolved against the core target and the targets the project's pinned providers declare; the CLI keeps no list of platforms. Doc:docs/provider-targets.md.Resolution rules
--platform=<t>(parsers check identifier shape only:invalid target '<t>' (targets are lowercase identifiers; run 'labelle targets')), else the project's declared.platform.desktopis the core target and needs no provider..targets(ownership already validated at discovery: one owner per name, nobody ownsdesktop).providers resolve --accept, newprovider_github.cachedRegistryOwner) lists a package whose verified cached archive declares the target — read out of the archive without extracting or running anything. The registry record itself has no target field (contract §4), so a name is never invented..labelle/<backend>_<target>/, is passed to the assembler'sgenerate, selects the hook plans and is thetargetin every hook context.project_config.Platformstays the strict schema type (doc comment added);parsed.platformis derived from the result for the legacy sites.The labelle-assembler#378 boundary
Schema-named provider targets (
wasm,android,ios) reach the assembler as--platform <name>exactly as before. A provider target outside the enum stops before the assembler runs unless its provider replacesgenerate:Deviation from the plan's decision 5 wording, called out for review: the gate is "no
replacehook ongenerate", not "always". With a generate replacement the assembler is never the generator, so the #378 limitation does not apply — and this is the only reading under which the plan's ownlabelle bundle --platform=probe-targetrequirement (replace hook running on every host) is reachable. Core steps a provider does not replace treat such a target as the generic host baseline (parsed.platform = .desktop).Breaking: legacy subcommands
labelle wasm serve|export,labelle android …andlabelle ios …set the same target override ("wasm"/"android"/"ios") and go through the resolver. Without a pinned provider declaring that target they fail with the no-provider error — no shim, no alias (RFC "Migration", #410). Same for a project whose.platformis one of them.--platform=wasmis unchanged in name. The three words stay inprovider_dispatch.reserved. Studio's web preview must add and pinlabelle-web(migration note in the doc).labelle bundleparseBundleArgsaccepts--platform=<t>/--platform <t>; the "project platform is X; bundling desktop instead" override is deleted — bundle bundles the resolved target.cli.ziginto the pipeline, after resolution and before the reporter/assembler/any build: it refuses only the coredesktoppackager off macOS (no hook can replace it since nobody ownsdesktop).replacehook onbundle→labelle: target '<t>' has no bundle replacement; package '<pkg>' must declare a.when = .replacehook onbundle`` (NoBundleReplacement). With one, it runs on any host with `output_dir = zig-out/bundle//` (or the resolved `--output`).labelle targetsReplaces the hardcoded backend/ECS table:
desktop (core)then<t> provided by <package>for each pinned provider, metadata-only viadiscover(a broken manifest is a warning, likelabelle help); outside a project only the core line.Also
astc/cmd.zigtakes--platformas a string; a target outside the legacy capability table is a usage error naming chore: pin labelle-assembler 0.105.0 #378.provider_targets.zigcontains none of the guard's forbidden words; every touched file still on the migration allowlist keeps at least one legacy mention, so the allowlist is unchanged.Tests
zig build test,zig build test-provider-dispatch):provider_targets.resolve(core with no providers, provider target, undeclared →NoProviderForTarget, schema-name mapping only through a provider, diagnostic with/without registry hint),provider_github.cachedRegistryOwner(hint only from a verified cached archive; no document / uncached / corrupted → null),parseTargetValue(identifier shape; rejectsWindows, empty,a b),parseBundleArgs --platform(both spellings, malformed, missing value, following flag, composition).test/provider_targets_e2e.py(fake assembler, wired into CI next to the hooks suite, all three hosts, 27 real CLI invocations): no-provider error for--platform=wasm, the declared.platformand every legacy subcommand with no assembler invocation and nothing created; malformed values; the chore: pin labelle-assembler 0.105.0 #378 message before the assembler runs;NoBundleReplacement; a provider replacinggenerate/build/bundleforprobe-targetincludinglabelle bundle --platform=probe-targetrunning the replacement on every host withoutput_dir == …/zig-out/bundle/probe-target(and--output); a provider owningwasmmaking the assembler receive--platform wasm(stopping afterlabelle generate); thelabelle targetslisting under a dead compiler/assembler.test/provider_hooks_e2e.py: the off-macOS bundle refusal assertion now also checks the cache is untouched (the gate runs after discovery).https://claude.ai/code/session_01R2K5sJDL5Msn1uRTkybiDP
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.