Skip to content

[codex] Pin WebUI v2 frontend Node tooling - #5384

Merged
serrrfirat merged 1 commit into
mainfrom
codex/reborn-webui-v2-toolchain-main
Jun 28, 2026
Merged

serrrfirat merged 1 commit into
mainfrom
codex/reborn-webui-v2-toolchain-main

Conversation

@ilblackdragon

Copy link
Copy Markdown
Member

Summary

  • Replays the WebUI v2 Node-tooling pin from [codex] Pin WebUI v2 frontend Node tooling #5370 onto main.
  • Adds .node-version and .nvmrc for Node 22.
  • Updates the frontend package engine metadata and lockfile metadata.
  • Adds a static build-script rerun hint so cargo rebuilds when embedded WebUI files change.

Context

#5370 was accidentally merged into the stacked branch codex/reborn-openai-responses after #5347 had already landed on main. This PR cherry-picks only that five-file tooling change onto a fresh branch from origin/main.

Validation

  • node --version && npm --version && npm install --package-lock-only --prefix crates/ironclaw_webui_v2_static/frontend
  • cargo fmt --all -- --check
  • git diff --check origin/main...HEAD

Copilot AI review requested due to automatic review settings June 27, 2026 22:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@railway-app
railway-app Bot temporarily deployed to ironclaw-ci-preview / ironclaw-pr-5384 June 27, 2026 22:59 Destroyed
@github-actions github-actions Bot added size: S 10-49 changed lines risk: low Changes to docs, tests, or low-risk modules labels Jun 27, 2026
@coderabbitai

coderabbitai Bot commented Jun 27, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ee017c97-b2e8-4747-9097-d0022334f3dd

📥 Commits

Reviewing files that changed from the base of the PR and between 128e744 and 196b955.

⛔ Files ignored due to path filters (1)
  • crates/ironclaw_webui_v2_static/frontend/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • .node-version
  • .nvmrc
  • crates/ironclaw_webui_v2_static/build.rs
  • crates/ironclaw_webui_v2_static/frontend/package.json

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated the project’s Node.js version requirements to Node 22 and aligned local tooling guidance.
  • Bug Fixes
    • Improved rebuild detection so static asset changes are picked up more reliably during development and builds.

Walkthrough

Pins Node.js to version 22 in .node-version, .nvmrc, and frontend/package.json (engines field). Adds a per-file cargo:rerun-if-changed directive in build.rs for each processed static asset during the directory walk.

Changes

Node.js 22 Pin and Build Improvements

Layer / File(s) Summary
Node.js version config pins
.node-version, .nvmrc, crates/ironclaw_webui_v2_static/frontend/package.json
Pins Node.js to 22 in .node-version and .nvmrc; adds engines block (>=22 <23, npm >=10) to package.json.
Per-file cargo:rerun-if-changed in build.rs
crates/ironclaw_webui_v2_static/build.rs
Emits a cargo:rerun-if-changed=<path> directive for each non-test static asset file processed during the tree walk.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

Node twenty-two now rules the land,
.nvmrc and .node-version hand in hand.
Each static file now tells Cargo its name,
rerun-if-changed — never miss a flame.
🦀 Small delta, tight ship, well done! 🎉

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description omits most required template sections, including Change Type, Linked Issue, Security Impact, Rollback Plan, and Review track. Fill in the full template: 2-5 summary bullets, change type, linked issue, security/db/blast radius/rollback sections, and review track.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the change set, describing the WebUI v2 Node tooling pin.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the contributor: core 20+ merged PRs label Jun 27, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Node.js version requirements to Node 22 across configuration files and frontend package manifests. It also updates the Rust build script to trigger rebuilds when static files change. The feedback recommends using relative paths instead of absolute paths in the cargo:rerun-if-changed instruction to ensure build caching and reproducibility are not broken.

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.

Comment on lines +84 to 85
println!("cargo:rerun-if-changed={}", path.display());
let rel = path.strip_prefix(root).expect("strip prefix"); // safety: build script — strip_prefix only fails on a logic bug

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Emitting absolute paths via cargo:rerun-if-changed can break build caching and reproducibility (e.g., when using distributed compilation tools like sccache or sandboxed environments like Nix/Bazel) because the paths contain local workspace directory structures.

Instead, we should emit paths relative to the crate root (CARGO_MANIFEST_DIR). Since root represents the static directory, we can strip the prefix and construct a relative path starting with static/.

Suggested change
println!("cargo:rerun-if-changed={}", path.display());
let rel = path.strip_prefix(root).expect("strip prefix"); // safety: build script — strip_prefix only fails on a logic bug
let rel = path.strip_prefix(root).expect("strip prefix"); // safety: build script — strip_prefix only fails on a logic bug
println!("cargo:rerun-if-changed={}", Path::new("static").join(rel).display());
References
  1. When relativizing paths, use std::path::Path::strip_prefix on Path objects rather than string manipulation, unless the string prefix is guaranteed to be exact.

@railway-app

railway-app Bot commented Jun 27, 2026

Copy link
Copy Markdown

🚅 Deployed to the ironclaw-pr-5384 environment in ironclaw-ci-preview

Service Status Web Updated (UTC)
ironclaw ✅ Success (View Logs) Web Jun 27, 2026 at 11:05 pm

@serrrfirat
serrrfirat merged commit 79f9be3 into main Jun 28, 2026
106 checks passed
@serrrfirat
serrrfirat deleted the codex/reborn-webui-v2-toolchain-main branch June 28, 2026 09:09

This branch was successfully deployed

No deployments
ironclaw-ci-preview / ironclaw-pr-5384 — 196b9559 Deployed Jun 27, 2026 by railway-app[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: S 10-49 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants