feat(reborn): frictionless local-dev onboarding (auto-provision serve, REPL model wizard, onboard launcher) - #6285
loopstring wants to merge 1 commit into
Conversation
|
This PR was not deployed automatically as @loopstring does not have access to the Railway project. In order to get automatic PR deploys, please add @loopstring to your workspace on Railway. |
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
size: XL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe CLI enables the WebUI beta by default, adds interactive onboarding and model setup controls, provisions local-development WebUI credentials, and improves WebUI build errors and onboarding presentation. ChangesCLI and WebUI flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant OnboardCommand
participant ReplCommand
participant ServeCommand
participant RebornProfile
participant RebornHome
participant Browser
Operator->>OnboardCommand: choose launch surface
OnboardCommand->>ReplCommand: re-exec repl
ReplCommand->>Operator: configure provider and API key
OnboardCommand->>ServeCommand: re-exec serve
ServeCommand->>RebornProfile: check profile credential policy
ServeCommand->>RebornHome: load or persist local-dev token
ServeCommand->>Operator: print tokenized sign-in URL
ServeCommand->>Browser: optionally open URL
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
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 |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
⏭️ IronLoop Review Declined: reviewer
Review at a glance
| Disposition | Head |
|---|---|
| ⏭️ Review declined | ac1349b2a752 |
Head: ac1349b2a752ab53672dc4d38116c02cbe33c5ca
Reason: The comparison spans 1,177 files with 31,633 additions and 93,809 deletions, including 359 full renames plus hundreds of unrelated modifications/deletions. A reliable complete review is not feasible within scope.
Next: Split the onboarding implementation from the WebUI migration and unrelated repository-wide changes, or provide a narrowed comparison against the immediate parent of the restructuring commit; then request review of each focused diff.
Run details
Status: Current
Trustworthy review produced: no
Summary
Skipped: the supplied base-to-head comparison is a mega diff dominated by broad repository restructuring, not a reviewable focused onboarding change.
![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 introduces first-run onboarding improvements, including interactive prompts to launch the REPL or Web UI, automatic model setup, and auto-provisioning of local-dev WebUI credentials with automatic browser launching. Feedback on these changes highlights a security race condition when writing the dev token to disk without atomic permissions, a usability issue where manual setup instructions are skipped if the launch prompt is declined, and a bug where using port 0 results in an invalid auto-open URL.
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.
| fn write_dev_webui_token(path: &std::path::Path, token: &str) -> std::io::Result<()> { | ||
| if let Some(parent) = path.parent() { | ||
| std::fs::create_dir_all(parent)?; | ||
| } | ||
| std::fs::write(path, token)?; | ||
| #[cfg(unix)] | ||
| { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
The current implementation of write_dev_webui_token writes the token to disk first with default permissions (which may be world- or group-readable depending on the system's umask) and only restricts them to 0o600 afterwards. This creates a brief race condition where another local user could read the sensitive session-signing token.
On Unix platforms, you can use std::os::unix::fs::OpenOptionsExt to set the file permissions to 0o600 atomically at creation time.
fn write_dev_webui_token(path: &std::path::Path, token: &str) -> std::io::Result<()> {
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)?;
}
#[cfg(unix)]
{
use std::io::Write;
use std::os::unix::fs::OpenOptionsExt;
let mut file = std::fs::OpenOptions::new()
.create(true)
.write(true)
.truncate(true)
.mode(0o600)
.open(path)?;
file.write_all(token.as_bytes())?;
}
#[cfg(not(unix))]
{
std::fs::write(path, token)?;
}
Ok(())
}| if !self.no_launch && io::stdin().is_terminal() { | ||
| if let Some(surface) = prompt_launch_surface()? { | ||
| return launch_surface(surface); | ||
| } | ||
| } else { | ||
| println!(); | ||
| println!("next: run `ironclaw-reborn repl` or `ironclaw-reborn serve` to finish setup"); | ||
| } |
There was a problem hiding this comment.
When onboarding is run interactively on a TTY and the user chooses to skip the launch prompt (or enters an invalid choice), the command exits silently without printing the next steps. The instructions on how to manually start the REPL or Web UI are only printed in the else block when the prompt is skipped entirely (e.g., via --no-launch or non-interactive stdin).
Removing the else constraint ensures that the manual setup instructions are always printed if the automatic launch is skipped or not executed.
if !self.no_launch && io::stdin().is_terminal() {
if let Some(surface) = prompt_launch_surface()? {
return launch_surface(surface);
}
}
println!();
println!("next: run `ironclaw-reborn repl` or `ironclaw-reborn serve` to finish setup");| if let Some(dev_token) = dev_token_for_url.as_deref() { | ||
| // The SPA auto-signs-in from `?token=`, so a local-dev operator can | ||
| // click straight through without pasting the generated dev token. | ||
| let sign_in_url = format!("http://{listen_addr}/v2/?token={dev_token}"); | ||
| eprintln!( | ||
| "ironclaw serve: open this URL to sign in automatically (dev token embedded):\n \ | ||
| {sign_in_url}" | ||
| ); | ||
| if !self.no_browser { | ||
| spawn_browser_open(sign_in_url); | ||
| } | ||
| } |
There was a problem hiding this comment.
There is an issue with the browser auto-open implementation when port 0 is used. If the operator starts the server with --port 0 (which is explicitly supported to let the OS pick a free port), listen_addr will contain port 0. The printed URL and the browser will attempt to open http://127.0.0.1:0/..., which is invalid.
While a fixed-duration sleep is a pragmatic tradeoff for waiting on resource initialization in non-critical paths, we should still leverage the bound_addr_tx oneshot channel in RebornWebuiServeOptions to resolve the actual bound address and print/open the correct URL with the dynamically allocated port.
References
- A fixed-duration sleep (e.g.,
tokio::time::sleep) can be a pragmatic tradeoff for waiting on resource initialization in non-critical paths, especially if a fallback mechanism (like logging) exists and a more robust solution is out of scope.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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_reborn_cli/Cargo.toml`:
- Line 22: Update the default feature list in the CLI crate’s Cargo
configuration to remove webui-v2-beta, leaving root-llm-provider enabled.
Preserve webui-v2-beta as an opt-in feature so default builds do not include the
web UI stack.
In `@crates/ironclaw_reborn_cli/src/commands/serve.rs`:
- Around line 426-437: Map unspecified listen hosts to loopback when composing
the sign-in URL in the serve command, reusing the existing mapping from
webui_oauth_callback_origin. Use the mapped host for both the printed URL and
spawn_browser_open while preserving the embedded dev token and port.
In `@crates/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx`:
- Around line 244-246: Update the subtitle paragraph’s text color utility in the
onboarding page to use Tailwind v4’s `text-(--v2-accent)` shorthand instead of
the bracketed `text-[var(--v2-accent)]` form, leaving the surrounding markup and
styling unchanged.
🪄 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: 912d5b68-4003-4bd5-97d3-c7bb73cec54c
📒 Files selected for processing (8)
crates/ironclaw_reborn_cli/Cargo.tomlcrates/ironclaw_reborn_cli/src/commands/config/init.rscrates/ironclaw_reborn_cli/src/commands/onboard.rscrates/ironclaw_reborn_cli/src/commands/repl.rscrates/ironclaw_reborn_cli/src/commands/serve.rscrates/ironclaw_reborn_config/src/profile.rscrates/ironclaw_webui_v2/build.rscrates/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx
|
|
||
| [features] | ||
| default = ["root-llm-provider"] | ||
| default = ["root-llm-provider", "webui-v2-beta"] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== crates/ironclaw_reborn_cli/Cargo.toml ==\n'
nl -ba crates/ironclaw_reborn_cli/Cargo.toml | sed -n '1,120p'
printf '\n== search for webui-v2-beta usage ==\n'
rg -n --hidden --glob '!target' --glob '!**/node_modules/**' 'webui-v2-beta|root-llm-provider|serve' crates/ironclaw_reborn_cli .claude AGENTS.md CLAUDE.md 2>/dev/null || trueRepository: nearai/ironclaw
Length of output: 238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== list likely repo policy files ==\n'
fd -a 'CLAUDE.md|AGENTS.md|\.claude' . 2>/dev/null || true
printf '\n== read top-level policy files if present ==\n'
for f in CLAUDE.md AGENTS.md .claude/rules .claude/rules.md .claude/CLAUDE.md; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
nl -ba "$f" | sed -n '1,240p'
fi
doneRepository: nearai/ironclaw
Length of output: 7040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== Cargo.toml around feature defaults =='
sed -n '1,80p' crates/ironclaw_reborn_cli/Cargo.toml | nl -ba
echo
echo '== all references in CLI crate =='
rg -n 'webui-v2-beta|root-llm-provider|serve' crates/ironclaw_reborn_cliRepository: nearai/ironclaw
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== crates/ironclaw_reborn_cli/Cargo.toml =='
sed -n '1,140p' crates/ironclaw_reborn_cli/Cargo.toml | awk '{printf "%4d %s\n", NR, $0}'
echo
echo '== root policy files =='
for f in CLAUDE.md AGENTS.md crates/ironclaw_reborn_cli/AGENTS.md crates/ironclaw_webui_v2/CLAUDE.md crates/ironclaw_webui_v2/AGENTS.md; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,220p' "$f" | awk '{printf "%4d %s\n", NR, $0}'
echo
fi
doneRepository: nearai/ironclaw
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== feature references in CLI crate =='
grep -RIn --exclude-dir=target --exclude-dir=node_modules -E 'webui-v2-beta|root-llm-provider|serve' crates/ironclaw_reborn_cli || true
echo
echo '== likely gating in workspace =='
grep -RIn --exclude-dir=target --exclude-dir=node_modules -E 'webui-v2-beta|webui.*beta|WebChat v2|HTTP gateway|axum|SSE|WS' crates | head -n 200 || trueRepository: nearai/ironclaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== crate-specific policy =='
if [ -f crates/ironclaw_reborn_cli/AGENTS.md ]; then
sed -n '1,220p' crates/ironclaw_reborn_cli/AGENTS.md | awk '{printf "%4d %s\n", NR, $0}'
fi
echo
echo '== workspace policy mentions of webui-v2-beta =='
grep -RIn --exclude-dir=target --exclude-dir=node_modules -E 'webui-v2-beta must be off by default|must be off by default|explicitly gate the WebChat v2 HTTP gateway' . || trueRepository: nearai/ironclaw
Length of output: 4596
Remove webui-v2-beta from default
crates/ironclaw_reborn_cli/AGENTS.md:35-58 says this feature is off by default and keeps serve out of default builds; crates/ironclaw_reborn_cli/Cargo.toml:22 flips it on, pulling the axum/auth/HTTP/SSE/WS stack into every default binary. Drop it from default unless that crate contract changes in the same PR.
🤖 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/ironclaw_reborn_cli/Cargo.toml` at line 22, Update the default feature
list in the CLI crate’s Cargo configuration to remove webui-v2-beta, leaving
root-llm-provider enabled. Preserve webui-v2-beta as an opt-in feature so
default builds do not include the web UI stack.
Source: Coding guidelines
| if let Some(dev_token) = dev_token_for_url.as_deref() { | ||
| // The SPA auto-signs-in from `?token=`, so a local-dev operator can | ||
| // click straight through without pasting the generated dev token. | ||
| let sign_in_url = format!("http://{listen_addr}/v2/?token={dev_token}"); | ||
| eprintln!( | ||
| "ironclaw serve: open this URL to sign in automatically (dev token embedded):\n \ | ||
| {sign_in_url}" | ||
| ); | ||
| if !self.no_browser { | ||
| spawn_browser_open(sign_in_url); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Sign-in URL embeds the raw bind address. When [webui].listen_host is 0.0.0.0/::, the printed and browser-opened URL is http://0.0.0.0:PORT/..., which most browsers (macOS/Windows) refuse to connect to. Reuse the same unspecified→loopback mapping already applied in webui_oauth_callback_origin when composing sign_in_url.
🤖 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/ironclaw_reborn_cli/src/commands/serve.rs` around lines 426 - 437, Map
unspecified listen hosts to loopback when composing the sign-in URL in the serve
command, reusing the existing mapping from webui_oauth_callback_origin. Use the
mapped host for both the printed URL and spawn_browser_open while preserving the
embedded dev token and port.
| <p className="mx-auto mt-3 max-w-md text-balance text-lg font-medium text-[var(--v2-accent)]"> | ||
| {t("onboarding.subtitle")} | ||
| </p> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '\[var\(--' crates/ironclaw_webui_v2/frontend/srcRepository: nearai/ironclaw
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tailwind deps ==\n'
rg -n '"tailwindcss"|`@tailwindcss`|tailwind' crates/ironclaw_webui_v2/frontend/package.json crates/ironclaw_webui_v2/frontend/pnpm-lock.yaml crates/ironclaw_webui_v2/frontend/yarn.lock crates/ironclaw_webui_v2/frontend/package-lock.json 2>/dev/null || true
printf '\n== tailwind config / docs pointers ==\n'
git ls-files 'crates/ironclaw_webui_v2/frontend/**' | rg 'tailwind|postcss|README|md$|css$'
printf '\n== onboarding snippet context ==\n'
sed -n '236,250p' crates/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsxRepository: nearai/ironclaw
Length of output: 5867
🌐 Web query:
Tailwind CSS v4 arbitrary values CSS variables parentheses brackets documentation
💡 Result:
In Tailwind CSS v4, the syntax for using CSS variables as arbitrary values has been updated to use parentheses instead of the square brackets [] used in v3 [1][2]. This change was made to resolve ambiguities in modern CSS [1]. Key changes and usage in v4: - Syntax Update: You can now use a CSS variable directly by wrapping it in parentheses, such as bg-(--brand-color) instead of the old bg-[var(--brand-color)] [1][3]. This syntax is a shorthand that automatically adds the var function for you [3]. - Resolving Ambiguities: If Tailwind cannot determine the utility type automatically, you can provide a "hint" using a CSS data type before the variable inside the parentheses [3]. For example: -
@utility directive, you can support arbitrary CSS variables by using the --value([*]) syntax, which acts as a wildcard for variables [3][4]. - Arbitrary Properties: For completely arbitrary CSS properties that are not standard Tailwind utilities, square bracket notation remains the standard for defining properties and their values, such as [mask-type:luminance] or [--scroll-offset:56px] [3]. Note that in v4, commas in arbitrary values are no longer automatically replaced with spaces; you must use underscores to represent spaces (e.g., grid-cols-[max-content_auto]) [1].
Citations:
- 1: https://tailwindcss.com/docs/upgrade-guide
- 2: https://github.com/tailwindlabs/tailwindcss/blob/14b13378cb9966af11dce6e37a22c1fc5cead16a/CHANGELOG.md
- 3: https://tailwindcss.com/docs/adding-custom-styles
- 4: [v4] How to support arbitrary CSS variables in custom utilties? tailwindlabs/tailwindcss#18792
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== frontend README tailwind notes ==\n'
sed -n '1,220p' crates/ironclaw_webui_v2/frontend/README.md
printf '\n== app.css tailwind entrypoint ==\n'
sed -n '1,220p' crates/ironclaw_webui_v2/frontend/src/styles/app.cssRepository: nearai/ironclaw
Length of output: 9541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'text-\(--|bg-\(--|border-\(--|accent-\(--|ring-\(--|fill-\(--|stroke-\(--|from-\(--|to-\(--|via-\(--' crates/ironclaw_webui_v2/frontend/src crates/ironclaw_webui_v2/frontend/public || trueRepository: nearai/ironclaw
Length of output: 153
🌐 Web query:
Tailwind v4 bg-[var(--brand-color)] still valid or deprecated arbitrary value CSS variable brackets
💡 Result:
In Tailwind CSS v4, the previous syntax for using CSS variables in arbitrary values has changed to resolve ambiguity [1][2]. While bg-[var(--brand-color)] remains technically valid in that it will be parsed, the recommended, modern syntax for referencing a CSS variable as an arbitrary value is bg-(--brand-color) [1][3]. Here is a breakdown of the current state of this syntax: 1. Recommended Syntax: Use bg-(--brand-color) [1][3]. This is treated as syntactic sugar that automatically injects the var() function for you [4][2]. 2. Previous Syntax: The older bg-[var(--brand-color)] is still supported by the parser [2], but you should migrate to the parenthetical syntax for better consistency and to avoid potential issues with newer CSS properties that allow dashed-idents (where var() is explicitly required) [2]. 3. Why it changed: In earlier versions, bg-[--brand-color] was often used as a shorthand, but it became ambiguous because some modern CSS properties accept raw dashed-idents (like --brand-color) without a var() wrapper [2]. To resolve this, Tailwind v4 introduced the (...) syntax to clearly distinguish when you are referencing a CSS variable [2]. If you are using Tailwind v4, you can safely update your classes to the ( ) notation [1][5]. If you need to perform calculations or combine variables, you can still use the [ ] notation with the full var() syntax if necessary (e.g., bg-[calc(var(--brand-color)/2)]) [6][7].
Citations:
- 1: https://tailwindcss.com/docs/upgrade-guide
- 2: Re-introduce automatic var injection shorthand tailwindlabs/tailwindcss#15020
- 3: https://tailwindcss.com/docs/background-color
- 4: https://tailwindcss.com/docs/adding-custom-styles
- 5: https://www.codewithseb.com/blog/tailwind-css-4-whats-new-migration-guide
- 6: https://tailwindcss.com/docs/theme
- 7: https://tailwindcss.com/docs/functions-and-directives
Prefer the Tailwind v4 shorthand here. text-(--v2-accent) is the current form; text-[var(--v2-accent)] still parses, so this is a cleanup rather than a syntax fix.
🤖 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/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx`
around lines 244 - 246, Update the subtitle paragraph’s text color utility in
the onboarding page to use Tailwind v4’s `text-(--v2-accent)` shorthand instead
of the bracketed `text-[var(--v2-accent)]` form, leaving the surrounding markup
and styling unchanged.
cac6623 to
da8a89c
Compare
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_reborn_cli/src/commands/serve.rs`:
- Around line 262-302: Update resolve_webui_credentials to return a
WebuiCredentialSource alongside the token and user ID, setting it when the
resolver auto-provisions credentials and otherwise marking operator-supplied
credentials. In the serve caller, destructure the source and derive
token_autoprovisioned with a source match instead of recomputing
token_env.is_none() && profile.allows_dev_credential_autoprovision(). Preserve
the existing sign-in URL and notice behavior based on the returned source.
- Around line 130-141: Update write_dev_webui_token to create the token file
with restrictive permissions before writing, using platform-appropriate
file-open options and 0o600 on Unix. Replace the
std::fs::write-then-set_permissions sequence with an initially secured file
creation/write flow, while preserving parent-directory creation and the
function’s existing Result 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 082b0253-9eca-4eaa-8108-d0323c215ee0
📒 Files selected for processing (8)
crates/ironclaw_reborn_cli/Cargo.tomlcrates/ironclaw_reborn_cli/src/commands/config/init.rscrates/ironclaw_reborn_cli/src/commands/onboard.rscrates/ironclaw_reborn_cli/src/commands/repl.rscrates/ironclaw_reborn_cli/src/commands/serve.rscrates/ironclaw_reborn_config/src/profile.rscrates/ironclaw_webui_v2/build.rscrates/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx
| fn write_dev_webui_token(path: &std::path::Path, token: &str) -> std::io::Result<()> { | ||
| if let Some(parent) = path.parent() { | ||
| std::fs::create_dir_all(parent)?; | ||
| } | ||
| std::fs::write(path, token)?; | ||
| #[cfg(unix)] | ||
| { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Dev token briefly written with default (world-readable) permissions before chmod.
std::fs::write creates the file with the process umask (commonly 0o644) and only afterward is it restricted to 0o600. Since this file doubles as the session-signing key, another local user on a shared box/CI runner could read it during that window. Open the file with mode 0o600 up front instead of writing-then-chmod.
🔒 Proposed fix — open with restrictive mode atomically
fn write_dev_webui_token(path: &std::path::Path, token: &str) -> std::io::Result<()> {
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)?;
}
- std::fs::write(path, token)?;
- #[cfg(unix)]
- {
- use std::os::unix::fs::PermissionsExt;
- std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?;
- }
+ #[cfg(unix)]
+ {
+ use std::io::Write;
+ use std::os::unix::fs::OpenOptionsExt;
+ std::fs::OpenOptions::new()
+ .write(true)
+ .create(true)
+ .truncate(true)
+ .mode(0o600)
+ .open(path)?
+ .write_all(token.as_bytes())?;
+ }
+ #[cfg(not(unix))]
+ std::fs::write(path, token)?;
Ok(())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn write_dev_webui_token(path: &std::path::Path, token: &str) -> std::io::Result<()> { | |
| if let Some(parent) = path.parent() { | |
| std::fs::create_dir_all(parent)?; | |
| } | |
| std::fs::write(path, token)?; | |
| #[cfg(unix)] | |
| { | |
| use std::os::unix::fs::PermissionsExt; | |
| std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; | |
| } | |
| Ok(()) | |
| } | |
| fn write_dev_webui_token(path: &std::path::Path, token: &str) -> std::io::Result<()> { | |
| if let Some(parent) = path.parent() { | |
| std::fs::create_dir_all(parent)?; | |
| } | |
| #[cfg(unix)] | |
| { | |
| use std::io::Write; | |
| use std::os::unix::fs::OpenOptionsExt; | |
| std::fs::OpenOptions::new() | |
| .write(true) | |
| .create(true) | |
| .truncate(true) | |
| .mode(0o600) | |
| .open(path)? | |
| .write_all(token.as_bytes())?; | |
| } | |
| #[cfg(not(unix))] | |
| std::fs::write(path, token)?; | |
| Ok(()) | |
| } |
🤖 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/ironclaw_reborn_cli/src/commands/serve.rs` around lines 130 - 141,
Update write_dev_webui_token to create the token file with restrictive
permissions before writing, using platform-appropriate file-open options and
0o600 on Unix. Replace the std::fs::write-then-set_permissions sequence with an
initially secured file creation/write flow, while preserving parent-directory
creation and the function’s existing Result behavior.
| // Resolve the boot profile so local-dev laptops can boot the WebUI with | ||
| // zero env-var setup, while hosted/production profiles still fail closed | ||
| // and require an operator-supplied token + user id. | ||
| let profile = crate::runtime::effective_profile(boot_config, config_file.as_ref())?; | ||
| let token_env = env::var(env_token_var).ok(); | ||
| let user_id_env = env::var(env_user_id_var).ok(); | ||
| let token_autoprovisioned = | ||
| token_env.is_none() && profile.allows_dev_credential_autoprovision(); | ||
| // Default the auto-provisioned WebUI user to the config's runtime owner | ||
| // so it matches `[identity].default_owner` (avoids the owner/WebUI-user | ||
| // mismatch that hides threads from the turn runner). | ||
| let dev_user_id_default = config_file | ||
| .as_ref() | ||
| .and_then(|file| file.identity.as_ref()) | ||
| .and_then(|identity| identity.default_owner.as_deref()) | ||
| .unwrap_or(DEFAULT_DEV_WEBUI_USER_ID); | ||
| let (token_value, user_id_raw) = resolve_webui_credentials( | ||
| profile, | ||
| token_env, | ||
| user_id_env, | ||
| boot_config.home().path(), | ||
| env_token_var, | ||
| env_user_id_var, | ||
| &boot_config.home().config_file_path(), | ||
| dev_user_id_default, | ||
| )?; | ||
| // Kept for the sign-in URL printed after the listen address resolves — | ||
| // `token_value` itself is moved into the authenticator below. | ||
| let dev_token_for_url = token_autoprovisioned.then(|| token_value.clone()); | ||
| if token_autoprovisioned { | ||
| eprintln!( | ||
| "ironclaw serve: no {env_token_var} set — using an auto-generated local dev \ | ||
| bearer token persisted under the Reborn home ({}). Set {env_token_var} to \ | ||
| override. This is enabled only for local-dev profiles.", | ||
| boot_config | ||
| .home() | ||
| .path() | ||
| .join(DEV_WEBUI_TOKEN_FILE) | ||
| .display(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
token_autoprovisioned re-derives the exact branch condition already computed inside resolve_webui_credentials.
Line 269's token_env.is_none() && profile.allows_dev_credential_autoprovision() duplicates the match arm at line 80. If the resolver's branching ever changes (e.g. a future profile gets a different autoprovision rule), this caller-side copy silently drifts out of sync with the actual credential source and the sign-in-URL/notice logic would misfire. Have the resolver return the source alongside the credentials instead of making the caller re-derive it.
♻️ Proposed refactor — return credential source from the resolver
enum WebuiCredentialSource {
OperatorSupplied,
AutoProvisioned,
}
fn resolve_webui_credentials(...) -> anyhow::Result<(String, String, WebuiCredentialSource)> {
let mut source = WebuiCredentialSource::OperatorSupplied;
let token = match token_env {
Some(value) => value,
None if profile.allows_dev_credential_autoprovision() => {
source = WebuiCredentialSource::AutoProvisioned;
load_or_create_dev_webui_token(home_path)?
}
None => return Err(/* ... */),
};
// ...
Ok((token, user_id_raw, source))
}Caller then does let token_autoprovisioned = matches!(source, WebuiCredentialSource::AutoProvisioned); instead of recomputing the predicate.
As per coding guidelines, "Keep functions focused and extract helpers when logic is reused."
🤖 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/ironclaw_reborn_cli/src/commands/serve.rs` around lines 262 - 302,
Update resolve_webui_credentials to return a WebuiCredentialSource alongside the
token and user ID, setting it when the resolver auto-provisions credentials and
otherwise marking operator-supplied credentials. In the serve caller,
destructure the source and derive token_autoprovisioned with a source match
instead of recomputing token_env.is_none() &&
profile.allows_dev_credential_autoprovision(). Preserve the existing sign-in URL
and notice behavior based on the returned source.
Source: Coding guidelines
da8a89c to
917c21a
Compare
…e, REPL model wizard, onboard launcher Make first-run onboarding work end-to-end for a local-dev laptop without manual env setup, while keeping hosted/production fail-closed. - serve (local-dev): auto-generate + persist the WebUI bearer token and default operator user id when unset, print a `?token=` sign-in URL, and auto-open the browser (--no-browser to opt out). Production/hosted still require operator-supplied token + user id (RebornProfile:: allows_dev_credential_autoprovision gates this). - repl: first-run model wizard — when no LLM is configured, prompt for a provider/model; when a provider is set but its API key env is missing, prompt for the key (set for the session) instead of hanging on the first message. Skipped for --no-setup and non-interactive stdin. - onboard: after scaffolding, prompt to launch REPL or Web UI and re-exec into it. Web UI option is gated on the webui-v2-beta feature so a CLI-only build never offers a `serve` subcommand it lacks. - config init template: leave [llm.default] commented so a fresh onboard leads to model setup (matches the sparse first-run seed) instead of pre-activating a provider that suppresses the setup prompt. - webui-v2-beta is now a default feature of ironclaw_reborn_cli so a default build ships both REPL and Web UI. - webui_v2 build.rs: clear, actionable error when corepack/pnpm is missing instead of an opaque `Os NotFound`. - onboarding welcome subtitle: more prominent (accent color, balanced). Tests: profile predicate; serve credential resolver (auto-provision vs fail-closed vs env-wins); repl provider-choice parse; onboard launch-choice parse.
917c21a to
ed3b151
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx (1)
244-246: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer the Tailwind v4 shorthand here.
text-(--v2-accent)is the current form;text-[var(--v2-accent)]still parses, so this is a cleanup rather than a syntax fix.♻️ Proposed refactor
- <p className="mx-auto mt-3 max-w-md text-balance text-lg font-medium text-[var(--v2-accent)]"> + <p className="mx-auto mt-3 max-w-md text-balance text-lg font-medium text-(--v2-accent)">🤖 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/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx` around lines 244 - 246, Update the subtitle paragraph’s Tailwind text color utility to use the v4 shorthand `text-(--v2-accent)` instead of the bracketed `text-[var(--v2-accent)]`, leaving the surrounding classes and translation unchanged.
🤖 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_reborn_cli/src/commands/repl.rs`:
- Around line 146-160: Update provider_requires_api_key at the
admin.list(Some(id), true).ok() call to add an inline “silent-ok” comment
explicitly stating that unreadable provider metadata intentionally defaults to
false and avoids prompting for an API key.
---
Duplicate comments:
In `@crates/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx`:
- Around line 244-246: Update the subtitle paragraph’s Tailwind text color
utility to use the v4 shorthand `text-(--v2-accent)` instead of the bracketed
`text-[var(--v2-accent)]`, leaving the surrounding classes and translation
unchanged.
🪄 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: 4d50aaae-c2d0-46d0-857d-d47cfb76c37a
📒 Files selected for processing (8)
crates/ironclaw_reborn_cli/Cargo.tomlcrates/ironclaw_reborn_cli/src/commands/config/init.rscrates/ironclaw_reborn_cli/src/commands/onboard.rscrates/ironclaw_reborn_cli/src/commands/repl.rscrates/ironclaw_reborn_cli/src/commands/serve.rscrates/ironclaw_reborn_config/src/profile.rscrates/ironclaw_webui_v2/build.rscrates/ironclaw_webui_v2/frontend/src/pages/onboarding/onboarding-page.tsx
| fn provider_requires_api_key( | ||
| admin: &ironclaw_reborn_composition::RebornProviderAdmin, | ||
| provider_id: Option<&str>, | ||
| ) -> bool { | ||
| let Some(id) = provider_id else { | ||
| return false; | ||
| }; | ||
| admin | ||
| .list(Some(id), true) | ||
| .ok() | ||
| .and_then(|list| list.providers.into_iter().next()) | ||
| .and_then(|provider| provider.metadata) | ||
| .map(|metadata| metadata.api_key_required) | ||
| .unwrap_or(false) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
.ok() swallows the provider-metadata read without a silent-ok marker. admin.list(...).ok() drops the error on a settings read and continues with false. The default-false behavior is defensible here (an unreadable provider just shouldn't nag), but the guideline requires the fallback be named explicitly.
As per coding guidelines: "justified fallbacks must include an inline // silent-ok: <reason> comment naming the operation."
♻️ Add the marker
let Some(id) = provider_id else {
return false;
};
+ // silent-ok: provider-metadata lookup — an unreadable/unknown provider just
+ // means "don't nag for a key", not a lost authoritative read.
admin
.list(Some(id), true)
.ok()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn provider_requires_api_key( | |
| admin: &ironclaw_reborn_composition::RebornProviderAdmin, | |
| provider_id: Option<&str>, | |
| ) -> bool { | |
| let Some(id) = provider_id else { | |
| return false; | |
| }; | |
| admin | |
| .list(Some(id), true) | |
| .ok() | |
| .and_then(|list| list.providers.into_iter().next()) | |
| .and_then(|provider| provider.metadata) | |
| .map(|metadata| metadata.api_key_required) | |
| .unwrap_or(false) | |
| } | |
| fn provider_requires_api_key( | |
| admin: &ironclaw_reborn_composition::RebornProviderAdmin, | |
| provider_id: Option<&str>, | |
| ) -> bool { | |
| let Some(id) = provider_id else { | |
| return false; | |
| }; | |
| // silent-ok: provider-metadata lookup — an unreadable/unknown provider just | |
| // means "don't nag for a key", not a lost authoritative read. | |
| admin | |
| .list(Some(id), true) | |
| .ok() | |
| .and_then(|list| list.providers.into_iter().next()) | |
| .and_then(|provider| provider.metadata) | |
| .map(|metadata| metadata.api_key_required) | |
| .unwrap_or(false) | |
| } |
🤖 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/ironclaw_reborn_cli/src/commands/repl.rs` around lines 146 - 160,
Update provider_requires_api_key at the admin.list(Some(id), true).ok() call to
add an inline “silent-ok” comment explicitly stating that unreadable provider
metadata intentionally defaults to false and avoids prompting for an API key.
Source: Coding guidelines
2 participants
Summary
Makes first-run onboarding work end-to-end on a local-dev laptop with no manual env setup, while keeping hosted/production fail-closed. Motivated by a from-scratch run of
cargo build … && ironclaw-reborn onboardhitting a chain of dead ends (missing WebUI token, missing user id, opaque corepack error, pre-set model, hang on missing API key).Changes
serve(local-dev): auto-generate + persist the WebUI bearer token and default operator user id when unset; print a?token=sign-in URL and auto-open the browser (--no-browserto opt out). Production/hosted still require operator-suppliedIRONCLAW_REBORN_WEBUI_TOKEN+_USER_ID— gated byRebornProfile::allows_dev_credential_autoprovision.repl: first-run model wizard — no LLM configured → prompt for provider/model; provider set but its API-key env missing → prompt for the key (this session) instead of hanging on the first message. Skipped for--no-setup/ non-interactive stdin.onboard: after scaffolding, prompt to launch REPL or Web UI and re-exec into it. Web UI option is gated onwebui-v2-betaso a CLI-only build never offers aserveit lacks.config inittemplate:[llm.default]left commented so a fresh onboard leads to model setup (matches the sparse first-run seed) instead of pre-activating a provider that suppressed the prompt.webui-v2-betais now default forironclaw_reborn_cli, so a default build ships both REPL and Web UI.webui_v2/build.rs: clear, actionable error when corepack/pnpm is missing instead of an opaqueOs NotFound.Verification
servelocal-dev with zero env → binds127.0.0.1,bearer=200/no-auth=401(auth still enforced), lands on/welcomewhen no model set.Follow-ups (not in this PR)