feat(ux): complete UX overhaul — design system, onboarding, web polish#1277
feat(ux): complete UX overhaul — design system, onboarding, web polish#1277ilblackdragon wants to merge 42 commits intostagingfrom
Conversation
Port the complete psychographic profiling system from NPA into IronClaw, including enriched profile schema, conversational onboarding, profile evolution, and three-tier prompt augmentation. Personal onboarding moved from wizard Step 9 to first assistant interaction per maintainer feedback — the First Contact system prompt block now instructs the LLM to conduct a natural onboarding conversation that builds the psychographic profile via memory_write. Changes: - Enrich profile.rs with 5 new structs, 9-dimension analysis framework, custom deserializers for backward compatibility, and rendering methods - Add conversational onboarding engine with one-step-removed questioning technique, personality framework, and confidence-scored profile generation - Add profile evolution with confidence gating, analysis metadata tracking, and weekly update routine - Replace thin interaction style injection with three-tier system gated on confidence > 0.6 and profile recency - Replace wizard Step 9 with First Contact system prompt block that drives conversational onboarding during the user's first interaction - Add autonomy progression to SOUL.md seed and personality framework to AGENTS.md seed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…pace seeds Remove the interactive onboarding_chat.rs engine in favor of a simpler bootstrap flow: fresh workspaces get a proactive LLM greeting that naturally profiles the user. Identity files are now seeded from src/workspace/seeds/ instead of being hardcoded. Also removes the identity-file write protection (seeds are now managed), adds routine advisor integration, and includes an e2e trace for bootstrap greeting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nboarding-and-routine-advisor # Conflicts: # tests/support/test_rig.rs
…prompt injection Identity files (SOUL.md, AGENTS.md, USER.md, IDENTITY.md) are injected into every system prompt. Rather than hard-blocking writes (which broke onboarding), scan content through the existing Sanitizer and reject writes with High/Critical severity injection patterns. Medium/Low warnings are logged but allowed. Also clarifies AGENTS.md identity file roles (USER.md = user info, IDENTITY.md = agent identity) and adds IDENTITY.md setup as an explicit bootstrap step. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…wiring The field is now actively used by the agent loop to suppress BOOTSTRAP.md injection — remove the stale "not yet wired" TODO. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the user authenticates via NEAR AI Cloud API key (option 4), api_key_login() stores the key via set_runtime_env(). But build_nearai_model_fetch_config() was using std::env::var() which doesn't check the runtime overlay — so model listing fell back to session-token auth and re-triggered the interactive NEAR AI authentication menu. Switch to env_or_override() which checks both real env vars and the runtime overlay. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
persist_assistant_response was called with channel="default",
user_id="system" but the assistant thread was created via
get_or_create_assistant_conversation("default", "gateway") which owns
the conversation as user_id="default", channel="gateway". The mismatch
caused ensure_writable_conversation to reject the write with:
WARN Rejected write for unavailable thread id user=system channel=default
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Content-Security-Policy header (added in f48fe95) blocks inline JS via script-src 'self'. All onclick/onchange attributes in index.html are replaced with getElementById().addEventListener() calls. Dynamic inline handlers in app.js (jobs, routines, memory breadcrumb, code blocks, TEE report) are replaced with data-action attributes and a single delegated click handler on document. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…chema field
- Bootstrap IncomingMessage now uses ("default", "gateway") consistently
with persist and session registration calls
- Update bootstrap_greeting.json fixture: schema_version → version to
match current PROFILE_JSON_SCHEMA
[skip-regression-check]
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
[skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… profile sync - BOOTSTRAP.md: fix target "profile" → "context/profile.json" so the write hits the correct path and triggers profile sync - IDENTITY_FILES: add context/assistant-directives.md to the scanned set since it is also injected into the system prompt - sync_profile_documents(): scan derived USER.md and assistant-directives content through Sanitizer before writing, rejecting High/Critical injection patterns - profile_evolution_prompt(): wrap recent_messages_summary in <user_data> delimiters with untrusted-data instruction to mitigate indirect prompt injection - routine-advisor skill: update cron examples from 6-field to standard 5-field format for consistency with routine_create tool docs [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nboarding-and-routine-advisor
Resolved conflicts:
- src/agent/routine.rs: kept both normalize_cron tests and tool_rounds tests
- src/channels/web/static/{app.js,index.html}: adopted staging's element IDs
and data-action names, updated HEAD's addEventListener bindings to match
- src/tools/builtin/memory.rs: kept sanitizer-based IDENTITY_FILES scanning
(replacing staging's PROTECTED_IDENTITY_FILES hard block), added staging's
looks_like_filesystem_path guard and tests
- src/workspace/mod.rs: merged bootstrap_pending/completed fields with
search_defaults field
- tests/e2e_advanced_traces.rs: kept both MCP lifecycle and bootstrap tests
- src/agent/agent_loop.rs: added missing StatusUpdate import
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
[skip-regression-check] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Quick-mode wizard now checks LLM_BACKEND, NEARAI_API_KEY, ANTHROPIC_API_KEY, and OPENAI_API_KEY env vars to pre-populate the provider setting, so users aren't re-prompted for credentials they already supplied. Also teaches setup_nearai() to recognize NEARAI_API_KEY from env (previously only checked session tokens). Includes web UI cleanup (remove duplicate event listeners) and e2e test response count adjustment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nboarding-and-routine-advisor # Conflicts: # src/tools/builtin/routine.rs
The cron normalizer now always expands to 7-field format, so the stored schedule is "0 0 9 * * * *" not "0 0 9 * * *". [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
In quick mode, if NEARAI_API_KEY is set in the environment and the backend was auto-detected as nearai, skip the interactive inference provider and model selection steps. The API key is persisted to the secrets store and a default model is set automatically. Also simplify the static fallback model list for nearai to a single default entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add DEFAULT_MODEL const and default_models() fallback list in llm/nearai_chat.rs; use from config, wizard, and .env.example so the default model is defined in one place - Restore multi-model fallback list in setup wizard (was reduced to 1) - Move BOOTSTRAP_GREETING to module-level const (out of run() body) - Replace LLM-based bootstrap with static greeting (persist to DB before channels start, then broadcast — eliminates startup LLM call and race) - Fix double env::var read for NEARAI_API_KEY in quick setup path - Move thread sidebar buttons into threads-section-header (web UI) - Remove orphaned .thread-sidebar-header CSS and fix double blank line - Update bootstrap e2e test for static greeting (no LLM trace needed) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Backend: add ActiveConfigSnapshot to expose resolved LLM backend, model, and enabled channels via /api/gateway/status - Add missing Agent settings (daily cost cap, actions/hour, local tools) - Add Sandbox, Routines, Safety, Skills, and Search setting groups - Settings import/export (JSON download + file upload) - Active env defaults shown as placeholders in Inference settings - Styled confirmation modals replace window.confirm() for remove actions - Global restart banner persists across settings subtab switches - Client-side validation with min/max constraints on number inputs - Accessibility: aria-label on inputs, role=status on save indicators - Settings search filters rows across current subtab - Smooth CSS transitions for conditional field visibility (showWhen) - Tunnel settings in Channels subtab - Mobile responsive settings layout at 768px breakpoint - i18n keys for toolbar, search, and import/export in en + zh-CN Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… section Remove the "Registered Tools" table from the extensions tab (debug info not useful to end users), clean up associated CSS/i18n/JS. Additional settings page UI polish: extension card state styling, layout refinements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use refreshCurrentSettingsTab() in SSE event handlers to reduce duplication - Remove unused formatGroupName/formatSettingLabel helpers - Use i18n keys for MCP Configure/Reconfigure buttons - Add data-i18n-placeholder to settings search input - Remove data-i18n from confirm modal button (set dynamically by showConfirmModal) - Fix cargo fmt in main.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on-check] - Update TABS list: replace extensions/skills with settings - Add settings_subtab/settings_subpanel selectors to helpers - Update test_connection, test_skills, test_extensions, test_wasm_lifecycle to navigate via Settings > subtab instead of top-level tabs - Move MCP card tests to use go_to_mcp() helper (MCP is now a separate subtab) - Remove tools table tests and mock_ext_apis tools= parameter - Fix CSP violation: replace inline onclick on confirm modal cancel button Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses PR #927 review comments (#1, #3) — identity file write protection and unsanitized profile fields in system prompt. Instead of scanning at the tool layer (memory.rs) or the sync layer (sync_profile_documents), injection scanning now lives in Workspace::write() and Workspace::append() for all files that are injected into the system prompt. This ensures every code path that writes to these files is protected, including future ones. - Add SYSTEM_PROMPT_FILES const and reject_if_injected() in workspace - Add WorkspaceError::InjectionRejected variant - Add map_write_err() in memory.rs to convert InjectionRejected to ToolError::NotAuthorized - Remove redundant IDENTITY_FILES/Sanitizer from memory.rs - Remove redundant sanitizer calls from sync_profile_documents() - Move sanitization tests to workspace::tests - Existing integration test (test_memory_write_rejects_injection) continues to pass through the new path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sion-check]
- Use I18n.t() for MCP empty state, export/import toasts, confirm modal
- Fix CLI channel card using wrong channel key ('repl' -> 'cli')
- Fix settings search counting hidden rows as visible
- Add aria-label i18n for settings search input
- Add common.loadFailed i18n key (en + zh-CN)
- Update E2E tests: WASM channel tests use Channels subtab,
remove tests use custom confirm modal instead of window.confirm
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…kip-regression-check] - WASM channel tests: filter by display name to avoid matching built-in channel cards in the Channels subtab - Skills remove test: click confirm modal button instead of using window.confirm (skill removal now uses custom confirm modal) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ion-check] - approval_needed SSE: refresh any active settings subtab, not just Extensions — approvals can surface from Channels/MCP setup flows too - renderCardsSkeleton: remove nested .extensions-list wrapper that caused skeleton cards to render constrained inside grid cells Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ion-check] Use expect_response to deterministically wait for the /api/extensions reload triggered by handleAuthCompleted → refreshCurrentSettingsTab, instead of a fixed 600ms sleep that was too short under CI load. Also remove stale /api/extensions/tools route handler. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…p-regression-check] Inject a counter wrapper around refreshCurrentSettingsTab to verify it's actually called, and wait for the async fetch to complete before asserting the reload count. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nboarding-and-routine-advisor Also moves bootstrap greeting into src/workspace/seeds/GREETING.md (loaded via include_str!) to keep user-facing copy out of Rust code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tstrap Address Copilot review: - Use LazyLock<Sanitizer> to avoid rebuilding Aho-Corasick + regexes on every workspace write - has_profile check now requires non-empty content, not just file existence, to prevent empty profile.json from suppressing onboarding - Add seed_tests integration tests (libsql-backed) verifying: - Empty profile.json does not suppress BOOTSTRAP.md seeding - Non-empty profile.json correctly suppresses bootstrap for upgrades Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…routine-advisor' into merge-927-1191
…-927-1191 # Conflicts: # src/channels/web/static/app.js
Introduce a unified terminal design system (src/cli/fmt.rs) with NO_COLOR compliance and use it across all CLI output. Redesign the boot screen, setup wizard, doctor/status commands, REPL help and approval flow. On the web side: full-width messages, streaming cursor, auth transitions, empty states, connection banners, keyboard shortcuts, ARIA accessibility, color system cleanup, and responsive tablet layout. Add contextual thinking messages, per-turn cost display, and a formatted top-level error handler with recovery hints. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 1 — Design System Foundation: - Unify CLI accent from cyan to emerald green (#34d399) with truecolor detection - Rename bold_cyan() → bold_accent(), add hint() for dim italic tips - Web: add typography scale (--text-xs to --text-3xl), replace 200+ hardcoded font-sizes - Web: spacing audit replacing hardcoded values with --space-* tokens - Web: motion system (easing/duration tokens) + prefers-reduced-motion support - Web: prefers-contrast: more media query for accessibility Phase 2 — First Impressions: - Boot screen progressive disclosure (3-4 lines vs 10, hint to ironclaw status) - Onboarding: auto-detect API keys (Anthropic/OpenAI/OpenRouter), skip interactive steps - Onboarding: minimal wordmark banner replacing ASCII art - Onboarding: dot-based step indicator (● ◉ ○) replacing progress bar - Onboarding: provider smart ordering (detected keys first with checkmarks) - Onboarding: warm completion card with key facts - Onboarding: --step flag for selective re-onboarding (deprecates --channels-only/--provider-only) Phase 3 — Web Chat & Interactions: - Welcome card with i18n suggestion chips - Code block syntax highlighting via highlight.js CDN - Turn cost SSE handler with token/cost badge - Streaming: reduced debounce 150ms→50ms, 10K force-flush safeguard - Connection indicator: amber disconnect, reconnection attempt counter - Skeleton loading states for threads/history - Tool card progress bar animation, icon pop on complete - Streaming cursor pulse, typing indicator dots - Refined approval card styling with keyboard hints - Thread sidebar: preview lines, active accent border - Input area: footer row, char count, send button glow - User messages: right-aligned chat bubbles with accent tint Phase 4 — Polish & Accessibility: - Doctor: grouped output (Core/Features/External sections) - REPL: redesigned /help with quick start + categorized commands - Web: aria-live on chat messages and toasts, aria-label on icon buttons - Web: focus-visible styles, touch targets, safe area padding - Web: mobile optimization (375px breakpoint, iOS zoom prevention) - i18n: new keys in en.js and zh-CN.js for welcome card, connection, messages Bug fixes during review: - Fix duplicate event listeners causing language switch and sidebar toggle to no-op - Fix sidebar collapse hiding toggle button (keep + and « visible when collapsed) - Restore message copy button that was accidentally removed [skip-regression-check] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant user experience upgrade across the application, focusing on consistency, ease of use, and intelligent personalization. It integrates new AI capabilities for task management and routine automation, while also bolstering security and improving the overall polish of both the command-line and web interfaces. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Comprehensive UX overhaul spanning CLI and web surfaces, plus new “conversational onboarding” via workspace bootstrap/profile files. The PR also adds safety scanning for system-prompt-injected workspace documents and expands gateway status payloads for frontend display.
Changes:
- Introduces a shared CLI formatting “design system” (
cli::fmt) and refreshes boot/status/doctor/REPL output accordingly. - Adds workspace seed templates + bootstrap greeting/onboarding flow, psychographic profile scaffolding, and prompt-injection scanning for system-prompt-injected workspace files.
- Polishes web gateway UI structure (Settings tab + subtabs), adds SSE/WS event types (turn cost), and updates E2E/integration tests to match.
Reviewed changes
Copilot reviewed 65 out of 66 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ws_gateway_integration.rs | Updates test gateway state construction to include active_config. |
| tests/support/test_rig.rs | Adds bootstrap-aware test rig behavior (optionally preserve bootstrap + ensure gateway channel name). |
| tests/support/test_channel.rs | Makes test channel name configurable (needed for bootstrap tests). |
| tests/support/gateway_workflow_harness.rs | Updates harness gateway state construction to include active_config. |
| tests/openai_compat_integration.rs | Updates test gateway state construction to include active_config. |
| tests/e2e_builtin_tool_coverage.rs | Updates expected cron expression to normalized 7-field format. |
| tests/e2e_advanced_traces.rs | Adds E2E test asserting bootstrap greeting fires on fresh workspace. |
| tests/e2e/scenarios/test_wasm_lifecycle.py | Updates navigation to match new Settings + Extensions subtab layout. |
| tests/e2e/scenarios/test_skills.py | Updates navigation to Settings > Skills and adapts removal confirmation flow. |
| tests/e2e/scenarios/test_extensions.py | Refactors tests for new Settings subtabs (Extensions/MCP/Channels) and custom confirm modal. |
| tests/e2e/helpers.py | Adds selectors for Settings subtabs/panels + confirm modal; updates tab list. |
| src/workspace/seeds/USER.md | New seed template for user context. |
| src/workspace/seeds/TOOLS.md | New seed template for environment-specific tool notes. |
| src/workspace/seeds/SOUL.md | New seed template for assistant “values/boundaries”. |
| src/workspace/seeds/README.md | New seed template documenting workspace structure. |
| src/workspace/seeds/MEMORY.md | New seed template for long-term memory file. |
| src/workspace/seeds/IDENTITY.md | New seed template for assistant identity. |
| src/workspace/seeds/HEARTBEAT.md | New seed template for heartbeat checklist. |
| src/workspace/seeds/GREETING.md | Adds static first-run greeting content persisted/broadcast on bootstrap. |
| src/workspace/seeds/BOOTSTRAP.md | Adds detailed first-run bootstrap instructions for conversational onboarding. |
| src/workspace/seeds/AGENTS.md | New seed template for session routines/operational instructions. |
| src/workspace/mod.rs | Adds system-prompt file injection scanning, bootstrap flags, profile injection logic, and profile→document sync. |
| src/workspace/document.rs | Adds canonical paths for context/profile.json and context/assistant-directives.md. |
| src/tools/builtin/routine.rs | Normalizes cron expressions to 7-field format before validation/storage. |
| src/tools/builtin/memory.rs | Routes injection rejections to NotAuthorized, marks bootstrap completed, and syncs derived docs on profile write. |
| src/setup/wizard.rs | Adds --step selective onboarding, provider autodetect from env, and refreshes completion summary UX. |
| src/setup/prompts.rs | Updates interactive prompt rendering (step indicator, banner, checkbox UI). |
| src/setup/profile_evolution.rs | New module to generate weekly profile-evolution prompts + routine prompt template. |
| src/setup/mod.rs | Exposes SetupError, exports profile_evolution, and documents conversational onboarding. |
| src/setup/README.md | Documents that personal onboarding happens via workspace bootstrap in the running assistant. |
| src/settings.rs | Adds profile_onboarding_completed persisted flag (alias supported). |
| src/main.rs | Adds top-level error formatting with hints, startup timing, ensures WASM channels dir exists, and publishes active config snapshot. |
| src/llm/nearai_chat.rs | Updates NEAR AI default model and provides fallback model list for setup wizard. |
| src/llm/mod.rs | Re-exports DEFAULT_MODEL and default_models(). |
| src/lib.rs | Exposes new profile module. |
| src/error.rs | Adds WorkspaceError::InjectionRejected. |
| src/config/llm.rs | Updates default NEAR AI model to crate::llm::DEFAULT_MODEL. |
| src/cli/status.rs | Switches status output to cli::fmt key/value lines and updated layout. |
| src/cli/mod.rs | Exposes cli::fmt and adds onboard --step flag (deprecates older single-step flags). |
| src/cli/fmt.rs | New shared terminal formatting/color token module with NO_COLOR + TTY detection. |
| src/cli/doctor.rs | Groups diagnostics and uses cli::fmt for consistent rendering. |
| src/channels/web/ws.rs | Updates test gateway state construction to include active_config. |
| src/channels/web/types.rs | Adds SSE event turn_cost and wires event typing for WS/SSE payloads. |
| src/channels/web/test_helpers.rs | Updates helper gateway state construction to include active_config. |
| src/channels/web/static/index.html | Adds Settings tab/subtabs layout, confirm modal, aria attributes, and Highlight.js assets. |
| src/channels/web/static/i18n/zh-CN.js | Adds Settings and new UI strings, updates extensions wording. |
| src/channels/web/static/i18n/en.js | Adds Settings and new UI strings, updates extensions wording. |
| src/channels/web/sse.rs | Wires SSE event type mapping for turn_cost. |
| src/channels/web/server.rs | Adds ActiveConfigSnapshot and returns active config fields in /api/gateway/status. |
| src/channels/web/mod.rs | Adds with_active_config and maps StatusUpdate::TurnCost → SSE event. |
| src/channels/wasm/wrapper.rs | Explicitly skips web-only status updates (suggestions + turn cost) for WASM channels. |
| src/channels/repl.rs | Migrates REPL output to cli::fmt, improves truncation, and refreshes /help and approval rendering. |
| src/channels/channel.rs | Adds StatusUpdate::TurnCost. |
| src/boot_screen.rs | Reworks boot screen to a 2-tier summary and links to ironclaw status for full details. |
| src/app.rs | Loads profile_onboarding_completed from settings to suppress bootstrap injection for existing users. |
| src/agent/thread_ops.rs | Emits StatusUpdate::TurnCost after a response. |
| src/agent/routine.rs | Adds cron normalization and updates cron parsing to accept 5/6/7-field expressions. |
| src/agent/dispatcher.rs | Improves thinking/status messages with tool-name-aware wording. |
| src/agent/agent_loop.rs | Adds bootstrap greeting persistence + broadcast flow driven by workspace bootstrap flag. |
| skills/routine-advisor/SKILL.md | New skill to suggest routines based on user patterns + cron examples. |
| skills/delegation/SKILL.md | New skill to guide task delegation + persistence via memory/routines. |
| CLAUDE.md | Updates repo map documentation to include new profile.rs. |
| .env.example | Updates example NEAR AI model default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Emit per-turn cost summary | ||
| { | ||
| let usage = self.cost_guard().model_usage().await; | ||
| let (total_in, total_out, total_cost) = | ||
| usage | ||
| .values() | ||
| .fold((0u64, 0u64, rust_decimal::Decimal::ZERO), |acc, m| { | ||
| ( | ||
| acc.0 + m.input_tokens, | ||
| acc.1 + m.output_tokens, | ||
| acc.2 + m.cost, | ||
| ) | ||
| }); | ||
| let _ = self | ||
| .channels | ||
| .send_status( | ||
| &message.channel, | ||
| StatusUpdate::TurnCost { | ||
| input_tokens: total_in as u32, | ||
| output_tokens: total_out as u32, | ||
| cost_usd: format!("${:.4}", total_cost), | ||
| }, | ||
| &message.metadata, | ||
| ) | ||
| .await; |
| // Top border: ┌ tool_name requires approval ─── | ||
| let top_label = format!(" {tool_name} requires approval "); | ||
| let top_fill = box_width.saturating_sub(top_label.len() + 1); | ||
| let top_border = format!( | ||
| "\u{250C}\x1b[33m{top_label}\x1b[0m{}", | ||
| "\u{250C}{}{top_label}{}{}", | ||
| fmt::warning(), | ||
| fmt::reset(), | ||
| "\u{2500}".repeat(top_fill) | ||
| ); | ||
|
|
||
| // Bottom border: └─ short_id ───── | ||
| let bot_label = format!(" {short_id} "); | ||
| let bot_fill = box_width.saturating_sub(bot_label.len() + 2); | ||
| let bot_border = format!( | ||
| "\u{2514}\u{2500}\x1b[90m{bot_label}\x1b[0m{}", | ||
| "\u{2500}".repeat(bot_fill) | ||
| ); | ||
| // Bottom border: └───── | ||
| let bot_fill = box_width.saturating_sub(1); | ||
| let bot_border = format!("\u{2514}{}", "\u{2500}".repeat(bot_fill)); |
| /// - stdout is not a terminal (pipe, file redirect, CI) | ||
| fn colors_enabled() -> bool { | ||
| if std::env::var_os("NO_COLOR").is_some() { | ||
| return false; | ||
| } | ||
| std::io::stdout().is_terminal() |
There was a problem hiding this comment.
Code Review
The pull request includes several changes to improve the IronClaw agent. The default NEARAI_MODEL in .env.example was updated to Qwen/Qwen3.5-122B-A10B. A new profile.rs file was added to the src/ directory for psychographic profile types. The delegation and routine-advisor skills were added. A bootstrap greeting was added to the agent loop. The agent dispatcher was updated to provide more contextual thinking messages based on tool names. The routine advisor was updated to normalize cron expressions. The boot screen was updated to be more compact. The REPL channel was updated to use a new color scheme and to truncate long strings. The web gateway channel was updated to include per-turn cost summaries. The web gateway was updated to include a snapshot of the active configuration. The web gateway's javascript was updated to include a number of new features, including a settings tab, a confirmation modal, and a welcome card. A number of other changes were made to improve the agent's performance and stability. The code reviewer suggested that the errors from get_or_create_assistant_conversation and persist_assistant_response should be handled to prevent silent failures. The code reviewer also suggested that the current implementation of normalize_cron_expression for 5-field expressions might be subtly incorrect and that a more robust approach would be to join the fields collected from split_whitespace. The code reviewer also suggested that the logic for handling the bootstrap process can be simplified.
| let thread_id = store | ||
| .get_or_create_assistant_conversation("default", "gateway") | ||
| .await | ||
| .ok(); | ||
| if let Some(id) = thread_id { | ||
| self.persist_assistant_response(id, "gateway", "default", BOOTSTRAP_GREETING) | ||
| .await; | ||
| } | ||
| thread_id |
There was a problem hiding this comment.
The errors from get_or_create_assistant_conversation and persist_assistant_response are being ignored. This can lead to silent failures where the bootstrap greeting is not persisted to the database, which could be confusing for new users. It would be more robust to handle these potential errors, for example by logging them. This aligns with the principle of robust error handling and logging, as seen in the rule regarding tokio::task::spawn_blocking.
let thread_id = match store
.get_or_create_assistant_conversation("default", "gateway")
.await
{
Ok(id) => {
if let Err(e) = self
.persist_assistant_response(id, "gateway", "default", BOOTSTRAP_GREETING)
.await
{
tracing::warn!("Failed to persist bootstrap greeting: {}", e);
}
Some(id)
}
Err(e) => {
tracing::warn!("Failed to get or create bootstrap conversation: {}", e);
None
}
};References
- When handling errors, log them to capture debugging information and prevent silent failures. It is good practice to distinguish between different error types in the message.
| pub fn normalize_cron_expression(schedule: &str) -> String { | ||
| let fields: Vec<&str> = schedule.split_whitespace().collect(); | ||
| match fields.len() { | ||
| 5 => format!("0 {} *", schedule.trim()), |
There was a problem hiding this comment.
The current implementation of normalize_cron_expression for 5-field expressions might be subtly incorrect. Using format!("0 {} *", schedule.trim()) could lead to issues if schedule.trim() itself contains extra spaces that are not handled by split_whitespace. A more robust approach would be to join the fields collected from split_whitespace.
| 5 => format!("0 {} *", schedule.trim()), | |
| 5 => format!("0 {} *", fields.join(" ")), |
| let bootstrap_injected = if self.is_bootstrap_completed() { | ||
| if self | ||
| .read(paths::BOOTSTRAP) | ||
| .await | ||
| .is_ok_and(|d| !d.content.is_empty()) | ||
| { | ||
| tracing::warn!( | ||
| "BOOTSTRAP.md still exists but profile_onboarding_completed is set; \ | ||
| suppressing bootstrap injection" | ||
| ); | ||
| } | ||
| false | ||
| } else if let Ok(doc) = self.read(paths::BOOTSTRAP).await | ||
| && !doc.content.is_empty() | ||
| { | ||
| parts.push(format!( | ||
| "## First-Run Bootstrap\n\n\ | ||
| A BOOTSTRAP.md file exists in the workspace. Read and follow it, \ | ||
| then delete it when done.\n\n{}", | ||
| doc.content | ||
| )); | ||
| } | ||
| parts.push(format!("## First-Run Bootstrap\n\n{}", doc.content)); | ||
| true | ||
| } else { | ||
| false | ||
| }; |
There was a problem hiding this comment.
The logic for handling the bootstrap process can be simplified. Instead of checking is_bootstrap_completed and then reading the file, you can combine these checks for better readability and to avoid nested if statements.
| let bootstrap_injected = if self.is_bootstrap_completed() { | |
| if self | |
| .read(paths::BOOTSTRAP) | |
| .await | |
| .is_ok_and(|d| !d.content.is_empty()) | |
| { | |
| tracing::warn!( | |
| "BOOTSTRAP.md still exists but profile_onboarding_completed is set; \ | |
| suppressing bootstrap injection" | |
| ); | |
| } | |
| false | |
| } else if let Ok(doc) = self.read(paths::BOOTSTRAP).await | |
| && !doc.content.is_empty() | |
| { | |
| parts.push(format!( | |
| "## First-Run Bootstrap\n\n\ | |
| A BOOTSTRAP.md file exists in the workspace. Read and follow it, \ | |
| then delete it when done.\n\n{}", | |
| doc.content | |
| )); | |
| } | |
| parts.push(format!("## First-Run Bootstrap\n\n{}", doc.content)); | |
| true | |
| } else { | |
| false | |
| }; | |
| let bootstrap_injected = if self.is_bootstrap_completed() { | |
| if self.read(paths::BOOTSTRAP).await.is_ok_and(|d| !d.content.is_empty()) { | |
| tracing::warn!( | |
| "BOOTSTRAP.md still exists but profile_onboarding_completed is set; suppressing bootstrap injection" | |
| ); | |
| } | |
| false | |
| } else if let Ok(doc) = self.read(paths::BOOTSTRAP).await && !doc.content.is_empty() { | |
| parts.push(format!("## First-Run Bootstrap\n\n{}", doc.content)); | |
| true | |
| } else { | |
| false | |
| }; |
zmanian
left a comment
There was a problem hiding this comment.
Review: massive UX overhaul + psychographic profiling + identity file safety
This is a 36-commit, 7300-line PR spanning psychographic profiling, identity file injection scanning, bootstrap/onboarding redesign, CLI design system, web UI overhaul, i18n, new skills, and new SSE events. Observations below.
Critical: CI did not run (fork PR)
Only classify/scope ran. No Clippy (any feature combo), formatting, panic-check, or regression enforcement. For a PR touching 66 files across Rust, JS, CSS, and HTML, full CI is mandatory before merge. Either push a maintainer commit to the branch or run local CI.
Concerning: scope should be split
This combines at least 4 orthogonal features:
- Psychographic profiling (profile.rs, onboarding_chat.rs, profile_evolution.rs, seeds)
- Identity file write protection (memory.rs -> workspace injection scanning)
- Web UI overhaul (CSS/JS/HTML, i18n, settings tabs)
- CLI design system (fmt.rs, boot screen, REPL, doctor/status)
Each would benefit from independent review and rollback capability. I'd strongly suggest splitting this so each feature can land and be tested independently.
Concerning: [skip-regression-check] on functional commits
Commits 35 and 36 both use [skip-regression-check] but contain substantial functional changes (SSE event handling, streaming debounce changes, boot screen restructuring, connection status logic). These aren't pure style changes.
Concerning: highlight.js CDN dependency
Commit 36 introduces syntax highlighting via a CDN. External CDN dependencies create availability risks (offline users, CDN outages) and supply chain risk (CDN compromise). Consider bundling or using a self-hosted copy.
Concerning: profile types use String instead of enums
CommunicationPreferences fields (detail_level, formality, tone, etc.) are all String. Per project conventions ("prefer strong types over strings"), these should be enums. This makes invalid states representable and means deserialization won't catch typos.
Positives:
- Identity file injection scanning evolution is good -- moving from tool layer (memory.rs) to Workspace::write/append (commit 20) ensures all write paths are protected
- LazyLock for the sanitizer (commit 33) avoids rebuilding Aho-Corasick on every write
- The non-empty profile check for bootstrap suppression is a solid edge case fix
- CSP-compliant event handlers (commit 7) is a good security improvement
- The
routine_trigger_messageCow pattern is efficient
Minor notes:
deserialize_trait_scorein profile.rs silently returns 50 on deser failure -- intentional for LLM robustness but worth a commentStatusUpdate::TurnCost-- verify the enum variant definition is included in this diff (I didn't find it in the patch)- The
env_or_overridefix for NEARAI_API_KEY (commit 5) is a standalone bug fix that should land independently
zmanian
left a comment
There was a problem hiding this comment.
Updated Review: UX overhaul + psychographic profiling + identity file safety
Updating my earlier review -- scope is fine as a single PR given the interconnected nature of the changes.
Critical: CI did not run (fork PR)
Only classify/scope ran. No Clippy (any feature combo), formatting, panic-check, or regression enforcement. For a PR touching 66 files across Rust, JS, CSS, and HTML, full CI is mandatory before merge. Either push a maintainer commit to the branch or run local CI.
Concerning: [skip-regression-check] on functional commits
Commits 35 and 36 both use [skip-regression-check] but contain substantial functional changes (SSE event handling, streaming debounce changes, boot screen restructuring, connection status logic). These aren't pure style changes.
Concerning: highlight.js CDN dependency
Commit 36 introduces syntax highlighting via a CDN. External CDN dependencies create availability risks (offline users, CDN outages) and supply chain risk (CDN compromise). Consider bundling or using a self-hosted copy.
Concerning: profile types use String instead of enums
CommunicationPreferences fields (detail_level, formality, tone, etc.) are all String. Per project conventions ("prefer strong types over strings"), these should be enums. This makes invalid states representable and means deserialization won't catch typos.
Positives:
- Identity file injection scanning evolution is good -- moving from tool layer (memory.rs) to Workspace::write/append (commit 20) ensures all write paths are protected
- LazyLock for the sanitizer (commit 33) avoids rebuilding Aho-Corasick on every write
- The non-empty profile check for bootstrap suppression is a solid edge case fix
- CSP-compliant event handlers (commit 7) is a good security improvement
- The
routine_trigger_messageCow pattern is efficient - The
env_or_overridefix for NEARAI_API_KEY (commit 5) is a nice catch
Minor notes:
deserialize_trait_scorein profile.rs silently returns 50 on deser failure -- intentional for LLM robustness but worth a commentStatusUpdate::TurnCost-- verify the enum variant definition is included in this diff (I didn't find it in the patch)
Summary
Comprehensive UX overhaul unifying CLI and web design language. Makes every surface feel like the same product — polished, calm, and intentional.
ironclaw status--stepflag for selective re-onboarding/helpwith quick start sectionaria-live,aria-label, focus-visible, touch targets, mobile optimization,prefers-contrast: moreTest plan
cargo fmt --check— cleancargo clippy --all --all-features— zero warningscargo test— 3141+ tests passnode --checkironclaw)ironclaw onboard --quick)ironclaw doctor)NO_COLOR=1 ironclawproduces clean uncolored outputprefers-reduced-motion: reducein DevTools🤖 Generated with Claude Code