Add immutable web export - #34
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR adds ChangesImmutable Web export
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Host
participant StaticBundle
participant Browser
CLI->>Host: export checked Editor/Play generation
Host->>StaticBundle: provide static files
CLI->>StaticBundle: materialize mount and write manifest
Browser->>StaticBundle: load mounted index and runtime artifacts
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/uhura-cli/src/cmd/play.rs (1)
307-350: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared logic between
locate_web_assetsandlocate_export_web_assets.The new
locate_export_web_assetsduplicates almost the entire body oflocate_web_assets(candidate search order, attempt tracking,symlink_metadataloop, wasm-root resolution), differing only in the env var name, packaged subdirectory, dev-mode subdirectory, and error text. Extracting a shared helper avoids the two copies drifting apart on discovery-order/error-format changes.♻️ Proposed refactor
-pub(super) fn locate_web_assets() -> Result<WebAssets, String> { - let mut candidates = Vec::new(); - if let Some(explicit) = std::env::var_os("UHURA_WEB_DIST") { - candidates.push(PathBuf::from(explicit)); - } - if let Ok(executable) = std::env::current_exe() - && let Some(bin) = executable.parent() - { - candidates.push(bin.join("../share/uhura/web")); - } - candidates.push(tool_root().join("web/dist")); - - let mut attempted = Vec::new(); - for root in candidates { - ... - } - ... - Err(format!( - "browser application is not built (looked for {locations}); set \ - UHURA_WEB_DIST or build web/ before starting a browser surface" - )) -} - -pub(super) fn locate_export_web_assets() -> Result<WebAssets, String> { - let mut candidates = Vec::new(); - if let Some(explicit) = std::env::var_os("UHURA_EXPORT_WEB_DIST") { - candidates.push(PathBuf::from(explicit)); - } - if let Ok(executable) = std::env::current_exe() - && let Some(bin) = executable.parent() - { - candidates.push(bin.join("../share/uhura/web-export")); - } - candidates.push(tool_root().join("web/dist-export")); - - let mut attempted = Vec::new(); - for root in candidates { - ... - } - ... - Err(format!( - "export browser application is not built (looked for {locations}); set \ - UHURA_EXPORT_WEB_DIST or build the export Web profile into web/dist-export before \ - running `uhura export`" - )) -} +fn locate_frontend_assets( + env_var: &str, + packaged_subdir: &str, + dev_subdir: &str, + build_hint: &str, +) -> Result<WebAssets, String> { + let mut candidates = Vec::new(); + if let Some(explicit) = std::env::var_os(env_var) { + candidates.push(PathBuf::from(explicit)); + } + if let Ok(executable) = std::env::current_exe() + && let Some(bin) = executable.parent() + { + candidates.push(bin.join(packaged_subdir)); + } + candidates.push(tool_root().join(dev_subdir)); + + let mut attempted = Vec::new(); + for root in candidates { + ... + } + ... + Err(format!( + "browser application is not built (looked for {locations}); {build_hint}" + )) +} + +pub(super) fn locate_web_assets() -> Result<WebAssets, String> { + locate_frontend_assets( + "UHURA_WEB_DIST", + "../share/uhura/web", + "web/dist", + "set UHURA_WEB_DIST or build web/ before starting a browser surface", + ) +} + +pub(super) fn locate_export_web_assets() -> Result<WebAssets, String> { + locate_frontend_assets( + "UHURA_EXPORT_WEB_DIST", + "../share/uhura/web-export", + "web/dist-export", + "set UHURA_EXPORT_WEB_DIST or build the export Web profile into web/dist-export \ + before running `uhura export`", + ) +}Also applies to: 352-397
🤖 Prompt for 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. In `@crates/uhura-cli/src/cmd/play.rs` around lines 307 - 350, Extract the common candidate-search and asset-discovery flow from locate_web_assets and locate_export_web_assets into a shared helper. Parameterize the helper with each command’s environment variable, packaged path, development path, and error text, while preserving candidate ordering, deduplication, symlink_metadata handling, wasm-root resolution, and each function’s existing return behavior.
🤖 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/uhura-cli/src/cmd/export.rs`:
- Around line 888-895: Update the post-publication cleanup in run() so failure
of remove_dir_all(&backup) is downgraded to a warning rather than propagated as
an error. Preserve successful export status after the staging-to-out rename,
while still reporting the backup path and cleanup error for the orphaned backup.
In `@crates/uhura-port/src/route.rs`:
- Around line 894-895: Update decode_query_value and the path-component decoding
flow to reject decoded C0 and C1 control characters before performing
canonicality checks. Ensure values such as percent-encoded NUL are rejected
rather than accepted or re-encoded, matching Rust host runtime and browser
export behavior. Keep valid decoded components and existing canonical validation
unchanged.
---
Nitpick comments:
In `@crates/uhura-cli/src/cmd/play.rs`:
- Around line 307-350: Extract the common candidate-search and asset-discovery
flow from locate_web_assets and locate_export_web_assets into a shared helper.
Parameterize the helper with each command’s environment variable, packaged path,
development path, and error text, while preserving candidate ordering,
deduplication, symlink_metadata handling, wasm-root resolution, and each
function’s existing return behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6356ed43-cfde-408e-9bdd-4555e5de9e3a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
.gitignoreREADME.mdcrates/uhura-cli/Cargo.tomlcrates/uhura-cli/src/cmd/export.rscrates/uhura-cli/src/cmd/mod.rscrates/uhura-cli/src/cmd/play.rscrates/uhura-cli/src/main.rscrates/uhura-cli/tests/export.rscrates/uhura-host/src/lib.rscrates/uhura-port/src/route.rsdocs/README.mddocs/rfcs/0006-immutable-web-export.mddocs/rfcs/README.mdscripts/package.shweb/README.mdweb/package.jsonweb/src/app/host.test.tsweb/src/app/host.tsweb/src/app/index.htmlweb/src/app/router.test.tsweb/src/app/router.tsweb/src/editor/editor.tsweb/src/play/application-location.tsweb/src/play/browser-adapters.test.tsweb/src/play/browser-adapters.tsweb/src/play/main.tsweb/src/play/shell.tsweb/src/renderer/assets.tsweb/vite.config.ts
| pub fn encode_query_value(value: &str) -> String { | ||
| encode_url_component(value) | ||
| encode_url_component(value, safe_query_component_byte) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'route.rs|host.ts' . | sed 's#^\./##'
echo "== route.rs relevant snippets =="
if [ -f crates/uhura-port/src/route.rs ]; then
wc -l crates/uhura-port/src/route.rs
sed -n '840,935p' crates/uhura-port/src/route.rs
sed -n '1260,1335p' crates/uhura-port/src/route.rs
fi
echo "== host.ts relevant snippets =="
if [ -f web/src/app/host.ts ]; then
wc -l web/src/app/host.ts
sed -n '1,170p' web/src/app/host.ts
fiRepository: gridaco/uhura
Length of output: 11435
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all normalizeUrlComponent usages =="
rg -n "normalizeUrlComponent\\(|normalizeQuery|query" web/src/app/host.ts -C 3
echo "== read host.ts query parsing block =="
sed -n '170,260p' web/src/app/host.ts
echo "== read route.rs full encode/decode/validators and tests around route parser =="
sed -n '760,1090p' crates/uhura-port/src/route.rs
sed -n '1335,1455p' crates/uhura-port/src/route.rs
echo "== deterministic JS behavior probe for %00 and query normalizer =="
node - <<'JS'
const source = "%00";
const hexValue = (byte) => {
if (byte >= 0x30 && byte <= 0x39) return byte - 0x30;
if (byte >= 0x41 && byte <= 0x46) return byte - 0x41 + 10;
if (byte >= 0x61 && byte >= 0x61 && byte <= 0x66) return byte - 0x61 + 10;
return null;
};
const isRouteQueryComponentCharacter = (byte) => {
const pathCond =
(byte >= 0x30 && byte <= 0x39)
|| (byte >= 0x41 && byte <= 0x5a)
|| (byte >= 0x61 && byte <= 0x7a)
|| "-._!~*()".includes(String.fromCharCode(byte));
return pathCond && byte !== 0x27;
};
let i = 0;
let decodedByte = null;
if (source.charCodeAt(i) === 0x25) {
const high = hexValue(source.charCodeAt(i+1));
const low = hexValue(source.charCodeAt(i+2));
decodedByte = (high << 4) | low;
}
console.log(JSON.stringify({decodedByte, unreservedAllowed: (decodedByte >= 0x30 && decodedByte <= 0x39) || (decodedByte >= 0x41 && decodedByte <= 0x5a) || (decodedByte >= 0x61 && decodedByte <= 0x66) || "-._~".includes(String.fromCharCode(decodedByte)), queryAllowed: isRouteQueryComponentCharacter(decodedByte)}));
try {
[...String.fromCodePoint(decodedByte)].some((character) => {
const point = character.codePointAt(0);
return point <= 0x1f || (point >= 0x7f && point <= 0x9f);
});
console.log("unsafe control check passes");
} catch (e) {
console.log("unsafe control check throws", String(e));
}
JS
echo "== simple Rust source-level evidence for %00 roundtrip =="
python3 - <<'PY'
from pathlib import Path
p = Path("crates/uhura-port/src/route.rs")
s = p.read_text()
needle = '("\0")'
print("contains roundtrip assertion for null:", needle in s)
print("encode_query_value lines:", [i+1 for i,l in enumerate(s.splitlines()) if "pub fn encode_query_value" in l or "safe_query_component_byte" in l])
PYRepository: gridaco/uhura
Length of output: 23033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== JS probe exact JS from host.ts behavior for %00 route query component =="
node - <<'JS'
const value = "%00";
const label = "query";
const hexValue = byte => {
if (byte >= 0x30 && byte <= 0x39) return byte - 0x30;
if (byte >= 0x41 && byte <= 0x46) return byte - 0x41 + 10;
if (byte >= 0x61 && byte <= 0x66) return byte - 0x61 + 10;
return null;
};
const isRouteQueryComponentCharacter = byte =>
((byte >= 0x30 && byte <= 0x39)
|| (byte >= 0x41 && byte <= 0x5a)
|| (byte >= 0x61 && byte <= 0x7a)
|| "-._!~*()".includes(String.fromCharCode(byte)))
&& byte !== 0x27;
let index = 0;
let decodedByte = null;
const decodedBytes = [];
const canonical = [];
if (value.charCodeAt(index) === 0x25) {
const high = hexValue(value.charCodeAt(index + 1));
const low = hexValue(value.charCodeAt(index + 2));
decodedByte = (high << 4) | low;
decodedBytes.push(decodedByte);
canonical.push(
isRouteQueryComponentCharacter(decodedByte)
? String.fromCodePoint(decodedByte)
: `%${decodedByte.toString(16).toUpperCase().padStart(2, "0")}`
);
index += 3;
}
console.log({
decodedByte: descriptor => String.fromCodePoint(descriptor.value),
canonical: canonical.join(""),
unsafeControl: [...String.fromCodePoint(decodedByte)].some(character => {
const point = character.codePointAt(0);
return point <= 0x1f || (point >= 0x7f && point <= 0x9f);
}),
});
try {
for (const character of value) {
const point = character.codePointAt(0);
if (point <= 0x1f || (point >= 0x7f && point <= 0x9f))
throw new TypeError(`${label} contains an unsafe control character`);
}
console.log("raw input control check passes");
} catch (e) {
console.log("raw input control check throws:", e.message);
}
JS
echo "== search for route-value validation and decode_query_value usages =="
rg -n "decode_query_value|encode_query_value|safe_query_component_byte|NonCanonicalComponent|unsafe control|unsafe.*control|C0|C1|point <= 0x1f" crates web/src/app/host.ts -C 2Repository: gridaco/uhura
Length of output: 8119
Reject control characters in decoded route components before canonicality checks.
decode_query_value accepts %00 for \0, while Rust host runtime decoding and the browser export path reject decoded unsafe controls. Also check decoded C0/C1 in path components before final canonical checks so Rust cannot emit or accept routes the browser host refuses.
🤖 Prompt for 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.
In `@crates/uhura-port/src/route.rs` around lines 894 - 895, Update
decode_query_value and the path-component decoding flow to reject decoded C0 and
C1 control characters before performing canonicality checks. Ensure values such
as percent-encoded NUL are rejected rather than accepted or re-encoded, matching
Rust host runtime and browser export behavior. Keep valid decoded components and
existing canonical validation unchanged.
Summary
uhura exportfor one checked, immutable Editor/Play Web bundleScope
The product surface in this PR is export. Embedding the result in a containing site is downstream publication proof, not an Uhura feature dependency or the goal of this change. The artifact does not select a hosting vendor, require a dedicated Uhura server, or include Spock changes.
Export output must be disjoint from the captured project tree, preventing a previous bundle from entering source identity and preventing destructive source/output overlap.
Validation
cargo fmt --all -- --checkcargo test --locked --workspacecargo clippy --locked --workspace --all-targets -- -D warningscorepack pnpm check(46 files, 328 passed, 2 skipped)cargo test --locked -p uhura-port -p uhura-host -p uhura-cliscripts/package.sh /private/tmp/uhura-package-pr.nd2f4k