build(webui-v2): remove generated dist from source control#5244
build(webui-v2): remove generated dist from source control#5244hanakannzashi wants to merge 8 commits into
Conversation
github-actions
Bot
added
scope: ci
📝 WalkthroughWalkthroughWebUI v2 now builds its bundle into Cargo output, and the frontend bundler honors an injected dist directory. CI, Docker, ignore rules, and docs now match the generated asset flow. ChangesWebUI v2 bundle output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request automates the WebUI v2 frontend build process during Cargo builds when the webui-v2-beta feature is enabled. The build script (build.rs) now runs npm ci and node build.mjs to generate the SPA bundle directly into OUT_DIR and embeds it, allowing previously committed build artifacts to be removed. Feedback on these changes highlights a performance bottleneck in the build script, where running npm ci on every build is inefficient; it is recommended to conditionally run the installation only when node_modules is missing or outdated relative to package-lock.json.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_webui_v2_static/build.rs`:
- Around line 85-90: The build script unconditionally runs npm ci in the WebUI
rebuild path, causing dependency reinstalls on every static edit. Update
build.rs so the dependency install step in the frontend setup only runs when
package-lock.json changes or when frontend/node_modules is missing, and keep the
normal rebundle path in the same build flow driven by the existing run_command
helper and frontend_dir logic.
- Around line 91-102: The build step in `build.rs` leaves stale files in
`generated_dist_dir`, so `collect()` can embed removed or renamed assets from
prior runs. Update the bundling flow around `run_command` and the `app_js` check
to clear or recreate `generated_dist_dir` before invoking the frontend build,
using the existing `generated_dist_dir`, `dist_dir`, and `collect()` path so
only the current `static/js/**` output is shipped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 522b7d11-5048-4d8a-adf2-6f67515ceec5
⛔ Files ignored due to path filters (12)
crates/ironclaw_webui_v2_static/static/dist/app.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/ar-SZP35X44.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/chunk-IGTNS7XG.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/de-4TCP3DQM.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/es-D5ZIOK7J.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/fr-XBDTUGK7.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/hi-663X6SQY.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/ja-4MS55UBZ.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/ko-52L6WTKU.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/pt-BR-V3GY6XKG.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/uk-O6OITDBG.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/zh-CN-RO5ML42R.jsis excluded by!**/dist/**
📒 Files selected for processing (9)
.github/workflows/reborn-e2e.yml.github/workflows/reborn-tests.yml.gitignoreREADME.mdcrates/ironclaw_webui_v2_static/build.rscrates/ironclaw_webui_v2_static/frontend/README.mdcrates/ironclaw_webui_v2_static/frontend/build.mjscrates/ironclaw_webui_v2_static/frontend/build.shcrates/ironclaw_webui_v2_static/frontend/package.json
hanakannzashi
force-pushed
the
codex/webui-v2-build-dist-artifacts
branch
from
069da66 to
2849fc4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/reborn-tests.yml:
- Around line 205-211: The Node.js setup condition is using a hardcoded package
allowlist, while the actual Cargo feature selection already comes from
scripts/ci/package-feature-flags.sh, creating two sources of truth. Update the
Install Node.js for WebUI bundle builds step in reborn-tests.yml to derive its
if condition from the same feature-flag source used later in the job, so
packages like ironclaw_webui_v2_beta are handled automatically. Keep the change
aligned with the existing package-feature-flags.sh logic and the matrix.package
checks used by the job.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a30a4a7-2f38-46ff-8744-49baeb98dcda
⛔ Files ignored due to path filters (12)
crates/ironclaw_webui_v2_static/static/dist/app.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/ar-SZP35X44.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/chunk-IGTNS7XG.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/de-4TCP3DQM.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/es-D5ZIOK7J.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/fr-XBDTUGK7.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/hi-663X6SQY.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/ja-4MS55UBZ.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/ko-52L6WTKU.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/pt-BR-V3GY6XKG.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/uk-O6OITDBG.jsis excluded by!**/dist/**crates/ironclaw_webui_v2_static/static/dist/chunks/zh-CN-RO5ML42R.jsis excluded by!**/dist/**
📒 Files selected for processing (9)
.github/workflows/reborn-e2e.yml.github/workflows/reborn-tests.yml.gitignoreREADME.mdcrates/ironclaw_webui_v2_static/build.rscrates/ironclaw_webui_v2_static/frontend/README.mdcrates/ironclaw_webui_v2_static/frontend/build.mjscrates/ironclaw_webui_v2_static/frontend/build.shcrates/ironclaw_webui_v2_static/frontend/package.json
|
🚅 Deployed to the ironclaw-pr-5244 environment in ironclaw-ci-preview
|
…cts' into codex/webui-v2-build-dist-artifacts
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5244
…dist-artifacts # Conflicts: # crates/ironclaw_webui_v2_static/static/dist/app.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile.reborn`:
- Around line 17-27: The build stage in Dockerfile.reborn is using mutable apt
inputs for apt-get install and the NodeSource repository setup, so pin the
package versions or replace this stage with a pinned Node base/runtime. Update
the RUN block that installs ca-certificates, curl, gnupg, and nodejs so the apt
inputs are version-locked and reproducible, and keep the change localized to the
Dockerfile.reborn build stage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3b10a339-defb-4053-b410-6d19c69126a3
📒 Files selected for processing (2)
.dockerignoreDockerfile.reborn
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5244
|
@claude review |
|
Found 5 issues: [CRITICAL:95] Missing index.html from generated dist bundle (lines 40-76): index.html becomes 'dist/index.html' but validation checks for 'index.html'. Build will panic. [CRITICAL:90] Process invocation with no timeout (lines 134-161): npm ci and node build.mjs can hang indefinitely, blocking entire build. [HIGH:80] Missing rerun-if-changed for frontend/ (lines 22-26): Source file changes won't trigger rebuild. Add println!("cargo:rerun-if-changed=frontend"); [HIGH:85] Excessive metadata syscalls (lines 104-116): fs::metadata() called 3 times each build even when unnecessary. [MEDIUM:75] Error recovery incomplete state (lines 82-101): Directory deleted before build validation; inconsistent state on failure. See full review at: https://github.com/anthropics/ironclaw/pull/5244 |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5244
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5244
|
Follow-up on body comments:
|
|
[BLOCK] This PR is not ready for human final review yet. Blocking status:
No additional code findings were added in this pass beyond mergeability. Guidance for human follow-up:
|
|
@claude review |
|
Mergeability check: is (). Please rebase or merge the current base branch to resolve conflicts before continuing review. |
|
Mergeability check: mergeStateStatus is DIRTY and mergeable is CONFLICTING. Please rebase or merge the current base branch to resolve conflicts before review can proceed. |
hanakannzashi
changed the title
2 participants
Summary
Fixes #5236.
static/distbundle artifactsOUT_DIRwhenwebui-v2-betais enabled, then embed it as/v2/dist/*frontend/build.shavailable for ignoredstatic/distpreviews and vendor refreshesChange Type
Build/CI/docs change for WebUI v2 beta static asset generation. Runtime behavior and served asset URLs stay the same.
Linked Issue
Fixes #5236.
Security Impact
No runtime auth, listener, CORS, secret, sandbox, or outbound network behavior changes. The main security tradeoff is build-time npm access for
webui-v2-betabuilds; package resolution remains lockfile-based, and Docker uses a pinned Node image digest for the build toolchain.Trust-Boundary
No product trust boundary changes. Host/runtime request handling is untouched. The only new boundary is build-time frontend dependency installation when a WebUI-enabled Rust build runs without existing
frontend/node_modules.Database
No database schema, persistence, migration, PostgreSQL, or libSQL changes.
Blast Radius
Scoped to WebUI v2 beta static asset generation and the CI/Docker paths that compile crates with
webui-v2-beta./v2/index.htmland/v2/dist/*remain the served paths; committed generatedstatic/distfiles are removed from review diffs.Rollback
Revert this PR to restore the previous committed
static/distartifact flow. If the Node toolchain path causes deployment issues, the Docker/CI Node setup can be reverted independently while restoring committed dist output.Review Follow-Through
index.htmlstill comes fromstatic/index.html, and WebUI source changes are already covered bycargo:rerun-if-changed=static; addingfrontend/recursively would watchnode_modules.Review track
Build/CI review, with WebUI v2 beta asset-serving awareness.
Verification
cargo fmt --package ironclaw_webui_v2_staticcargo test -p ironclaw_webui_v2_static --features webui-v2-betacargo check -p ironclaw_reborn_cli --features webui-v2-betacargo test -p ironclaw_webui_v2_staticcargo check -p ironclaw_webui_v2_static --no-default-features --libgit diff --checkDOCKER_BUILDKIT=1 docker build -f Dockerfile.reborn --target chef -t ironclaw-reborn-chef-node-test .DOCKER_BUILDKIT=1 docker build -f Dockerfile.reborn --target builder -t ironclaw-reborn-builder-test .