feat(gateway): phase A design-system tokens (Defuse OmniSwap palette) - #2689
Closed
ilblackdragon wants to merge 2 commits into
Closed
ilblackdragon wants to merge 2 commits into
ilblackdragon wants to merge 2 commits into
Conversation
First visible step of the design-system adoption tracked in the
quiet-nibbling-moth plan. Restructures `theme.css` with Radix
Themes-style 12-step scales anchored on Defuse's OmniSwap whitelabel
palette (the closest in-house palette to IronClaw's current emerald),
introduces semantic role tokens, a layered shadow system, extended
radii and type scale, and swaps the webfont from DM Sans to Geist
(Geist Mono for monospace).
- Accent scale (--accent-1..12 + --accent-a1..12) from OmniSwap osa*:
mint→forest emerald, brand anchor at --accent-9 (#73e2a5 dark /
#0a5b38 light).
- Neutral scale (--sand-1..12 + --sand-a1..12) from OmniSwap os*:
green-tinged warm grays.
- Semantic role aliases: --color-{bg,surface,surface-elevated,
surface-hover,border,border-hover,text,text-muted,text-subtle,
label,warning,warning-foreground}, plus --accent-contrast.
- Layered shadows: --shadow-{paper,widget,card,inset} replace a
single --shadow.
- Radius scale extended with --radius-{xl:16,2xl:20,4xl:30}.
- Type scale extended with --text-{4xl:32,5xl:48,6xl:64} + tracking.
- Typography: --font-sans: 'Geist', --font-mono: 'Geist Mono'
loaded via Google Fonts (CSP already allows fonts.googleapis.com /
fonts.gstatic.com).
Every legacy token (--bg, --text, --accent, --shadow-card, …) is
preserved and aliased onto the new scales so the 20 split surface
stylesheets under `styles/` need zero changes — swapping palettes
happens entirely in theme.css.
Includes:
- tests/e2e/scenarios/capture_design_surfaces.py — opt-in Playwright
scenario (CAPTURE_SCREENSHOTS=1) that captures every main tab in
both themes for design-system PR review. Safe to leave in the suite;
default pytest runs skip it.
- docs/design-system/phase-a/{before,after}/*.png — 1280×800
Chromium screenshots captured pre- and post-change.
- docs/design-system/phase-a/README.md — side-by-side table and
reproduction instructions.
Admin panel falls back to the system font chain (its CSP blocks the
Google Fonts origin, which is intentional — admin stays same-origin
only).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
scope: docs
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements Phase A of a design system migration, introducing Radix-style 12-step color scales, semantic role tokens, and a layered shadow system. It also updates the typography to the Geist font family and adds an E2E utility for UI screenshot capture. Feedback highlights the need to override semantic warning tokens for light mode, use scale variables instead of hardcoded hex values for consistency, and improve exception handling in the test suite.
Comment on lines
+341
to
+350
| /* Light-mode overrides for the few legacy tokens that aren't a | ||
| * pure scale-step reference. */ | ||
| --bg-overlay: rgba(0, 0, 0, 0.30); | ||
| --bg-modal: var(--sand-1); | ||
| --border-modal: var(--sand-a6); | ||
| --text-secondary: var(--sand-8); | ||
| --text-tertiary: var(--sand-10); | ||
| --text-muted: var(--sand-7); | ||
| --text-dimmed: var(--sand-6); | ||
| --text-on-accent: #ffffff; |
Contributor
There was a problem hiding this comment.
The new semantic role tokens --color-warning and --color-warning-foreground (defined in :root at lines 122-123) use hardcoded colors specific to the dark theme. They should be overridden here in the light theme block to use the appropriate light-mode warning colors. Additionally, --text-on-accent should ideally reference --accent-contrast for consistency.
/* Light-mode overrides for semantic roles and legacy tokens. */
--color-warning: var(--warning-soft);
--color-warning-foreground: var(--warning-text);
--bg-overlay: rgba(0, 0, 0, 0.30);
--bg-modal: var(--sand-1);
--border-modal: var(--sand-a6);
--text-secondary: var(--sand-8);
--text-tertiary: var(--sand-10);
--text-muted: var(--sand-7);
--text-dimmed: var(--sand-6);
--text-on-accent: var(--accent-contrast);|
|
||
| /* Contrast text-on-solid-accent — used for button foregrounds when the | ||
| * background is `--accent-9`. Defuse sets this to the inverted step 1. */ | ||
| --accent-contrast: #031c12; |
Contributor
There was a problem hiding this comment.
Instead of hardcoding the hex value, consider referencing --accent-1 which contains the same value. This ensures consistency if the scale is adjusted in the future.
--accent-contrast: var(--accent-1);| --accent-a11: rgba(4, 42, 27, 0.920); | ||
| --accent-a12: rgba(3, 28, 18, 0.970); | ||
|
|
||
| --accent-contrast: #edfcf3; |
Contributor
There was a problem hiding this comment.
Consider using var(--accent-1) here to maintain consistency with the scale, matching the pattern used in the dark mode definition.
--accent-contrast: var(--accent-1);| btn = SEL["tab_button"].format(tab=tab) | ||
| try: | ||
| await page.locator(btn).click(timeout=3000) | ||
| except Exception: |
Contributor
There was a problem hiding this comment.
Catching a broad Exception can mask legitimate failures (e.g., selector syntax errors or unexpected browser states). For Playwright operations like .click() or .wait_for_selector() (as seen on line 89), it is better to catch the specific TimeoutError from playwright.async_api to ensure that only expected timeouts are suppressed while other errors remain visible.
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces Phase A of the gateway design-system adoption by rebuilding theme.css around Radix-style 12-step color scales (Defuse OmniSwap palette), adding semantic role tokens, and adding an opt-in Playwright scenario + screenshot gallery for visual diffs.
Changes:
- Reworks gateway theme tokens into neutral/accent 12-step scales with semantic aliases and expanded shadows/radii/type scale.
- Switches the main gateway Google Fonts link to Geist / Geist Mono.
- Adds an opt-in Playwright screenshot capture scenario and publishes before/after screenshot artifacts + documentation.
Reviewed changes
Copilot reviewed 4 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/scenarios/capture_design_surfaces.py | Adds opt-in Playwright scenario to capture screenshots across major UI surfaces and auth screen. |
| crates/ironclaw_gateway/static/theme.css | Introduces 12-step sand/accent scales, semantic role tokens, layered shadows, and legacy token aliases. |
| crates/ironclaw_gateway/static/index.html | Updates Google Fonts to load Geist / Geist Mono. |
| docs/design-system/phase-a/README.md | Documents Phase A token changes and links screenshot gallery + reproduction steps. |
| docs/design-system/phase-a/before/light-auth.png | “Before” light auth screenshot asset. |
| docs/design-system/phase-a/before/dark-routines.png | “Before” dark routines screenshot asset. |
| docs/design-system/phase-a/before/dark-missions.png | “Before” dark missions screenshot asset. |
| docs/design-system/phase-a/before/dark-auth.png | “Before” dark auth screenshot asset. |
| docs/design-system/phase-a/after/light-auth.png | “After” light auth screenshot asset. |
| docs/design-system/phase-a/after/dark-auth.png | “After” dark auth screenshot asset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| CAPTURE_SCREENSHOTS=1 SCREENSHOT_DIR=/tmp/after \ | ||
| pytest tests/e2e/scenarios/capture_design_surfaces.py | ||
|
|
||
| Outputs two sets of 1280×800 PNGs per tab + theme; reference them in |
There was a problem hiding this comment.
The docstring claims screenshots are 1280×800, but this test uses the shared page fixture which creates a 1280×720 viewport (tests/e2e/conftest.py:1117). Either set the viewport to 1280×800 in this scenario (e.g., create a new context like test_capture_auth_screen or call set_viewport_size) or update the documentation to match the actual output size.
Suggested change
| Outputs two sets of 1280×800 PNGs per tab + theme; reference them in | |
| Outputs two sets of 1280×720 PNGs per tab + theme; reference them in |
| * =========================================================== */ | ||
| --font-sans: 'Geist', -apple-system, BlinkMacSystemFont, 'DM Sans', 'Segoe UI', sans-serif; | ||
| --font-mono: 'Geist Mono', 'IBM Plex Mono', 'SF Mono', 'Fira Code', Consolas, monospace; | ||
|
|
There was a problem hiding this comment.
--font-sans is introduced (and index.html now loads Geist), but the main UI base styles still hardcode font-family: 'DM Sans' ... (e.g., static/styles/base.css). As-is, Geist likely won’t be used for the primary UI text. Consider switching the base body font-family to var(--font-sans) (or adding a global rule here) so the font token actually takes effect.
Suggested change
| /* Apply the theme font token globally so the primary UI actually | |
| * uses the configured sans stack. */ | |
| body { | |
| font-family: var(--font-sans); | |
| } |
Comment on lines
+279
to
+283
| /* ============================================================== | ||
| * Light theme — the OmniSwap scales are literally reversed | ||
| * between modes, so the role tokens stay defined in :root above | ||
| * and we redefine only the scales + a couple of alpha overlays. | ||
| * ============================================================== */ |
There was a problem hiding this comment.
The new layered shadow tokens (--shadow-paper, --shadow-widget, --shadow-card, --shadow-inset) are only defined in :root and aren’t overridden in [data-theme="light"]. That leaves light mode using the dark-mode shadow recipe (notably the rgba(0,0,0,0.40) parts), which is likely too heavy and also defeats the intent of palette/theme-specific tokens. Define light-mode values for these --shadow-* tokens inside the light theme block.
Comment on lines
+3
to
+8
| First visible step of the design-system adoption tracked in | ||
| `/home/illia/.claude/plans/quiet-nibbling-moth.md`. Restructures | ||
| `theme.css` with Radix Themes-style 12-step scales anchored on | ||
| Defuse's OmniSwap whitelabel palette, introduces semantic role tokens, | ||
| a layered shadow system, extended radii + type scale, and swaps the | ||
| webfont from DM Sans → Geist (plus Geist Mono for monospace). |
There was a problem hiding this comment.
This doc references a local absolute path (/home/illia/.claude/plans/quiet-nibbling-moth.md) that won’t exist for other contributors or in CI. Prefer a repo-relative link (e.g., docs/...) or remove the path and describe the plan location generically.
Suggested change
| First visible step of the design-system adoption tracked in | |
| `/home/illia/.claude/plans/quiet-nibbling-moth.md`. Restructures | |
| `theme.css` with Radix Themes-style 12-step scales anchored on | |
| Defuse's OmniSwap whitelabel palette, introduces semantic role tokens, | |
| a layered shadow system, extended radii + type scale, and swaps the | |
| webfont from DM Sans → Geist (plus Geist Mono for monospace). | |
| First visible step of the design-system adoption tracked in the | |
| implementation plan. Restructures `theme.css` with Radix Themes-style | |
| 12-step scales anchored on Defuse's OmniSwap whitelabel palette, | |
| introduces semantic role tokens, a layered shadow system, extended | |
| radii + type scale, and swaps the webfont from DM Sans → Geist (plus | |
| Geist Mono for monospace). |
Collaborator
|
I think there is a problem with the dark mode? background is white |
…k mode
theme.css had a legacy-tokens comment block containing the glob pattern
`styles/**/*.css`. Chrome's CSS parser matches `*/` inside the `**/` +
`*` + `.` sequence, closing the comment prematurely. Error recovery
then swallowed the very next declaration — `--bg: var(--color-bg);` —
which left body background resolving to `transparent` and every surface
falling through to the browser's default white.
Side effect: the initial Phase A screenshots in
`docs/design-system/phase-a/after/` showed the broken state, which
looked like a palette issue but was actually a parser issue.
Fix: rewrite the comment body to drop the glob, add a warning note so
the pattern doesn't regress, and re-capture the "after" screenshots.
Verified: `getComputedStyle(documentElement).getPropertyValue('--bg')`
now returns `#080c0a` in dark mode and `#fcfdfc` in light mode;
`document.body.style.backgroundColor` follows through. Every surface
now renders on the intended dark background.
Regression guard added in the comment itself — CI would catch a
recurrence via the updated screenshots on the next design-system PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Dark mode fix pushed as
The fix rewrites the comment to avoid the glob pattern and adds a warning note so it doesn't regress. Re-captured screenshots replace the "after" set in Also tested: |
ilblackdragon
added a commit
that referenced
this pull request
Apr 20, 2026
Synthesizes recurring patterns from ~30 merged PRs, 147 bot review comments (Copilot/Gemini), human reviews, and ~50 issues filed in the past 2 weeks. Each rule cites the motivating PR/issue numbers. New files: - error-handling.md — silent-failure taxonomy (unwrap_or_default, .ok()?, poisoned caches), persist-then-reload atomicity, channel-edge error mapping. (#2526, #2633, #2653, #2673, #2546, #2407, #2408) - agent-evidence.md — side-effect claims must cite tool evidence, empty-fast outputs are errors, external-effect tools must read back, setup UI round-trip. (#2544, #2580, #2582, #2541, #2545, #2411, #2543, #2586) - lifecycle.md — discovery vs. activation, terminal auth rejection, list_installed vs. list_active, deactivation unwinds, snapshot rehydrate must re-validate. (#2556, #2557, #2558, #2564, #2419, PR #2617, PR #2631) Extended: - types.md — from_trusted boundary rule, validated-newtype template with shared validate(&str), serde(try_from) required for validated types, wire-stable enums (no Debug; serde alias for migrations), canonical wire-contract field naming. (PR #2685, #2681, #2687, #2678, #2669, #2665, #2683, #2702) - safety-and-sandbox.md — every new ingress scans pre-transform/pre- injection, bounded resources (interners/streams/fan-out caps), cache keys must be complete. (#2491, #2676, #2470, #2633, #2673, #2710, PR #2702) - review-discipline.md — PR scope discipline, guardrail scripts are code (regression tests, grouped-import parsing, CI has_code inclusion), absolute-path ban in committed docs, stale comments after refactors. (PR #2668, #2628, #2680, #2687, #2647, #2689, #2701) All new files carry paths: frontmatter so they auto-load only on matching files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
serrrfirat
reviewed
Apr 20, 2026
| * / admin.html — CSP already allows `fonts.googleapis.com` and | ||
| * `fonts.gstatic.com`. | ||
| * =========================================================== */ | ||
| --font-sans: 'Geist', -apple-system, BlinkMacSystemFont, 'DM Sans', 'Segoe UI', sans-serif; |
Collaborator
There was a problem hiding this comment.
High Severity
Typography swap is only partially wired, so the gateway will mostly fall back to system fonts instead of actually using Geist.
This PR changes index.html to load Geist / Geist Mono and defines --font-sans / --font-mono here, but the main gateway CSS still hardcodes the old families in the existing consumers:
crates/ironclaw_gateway/static/styles/base.csskeepsbody { font-family: 'DM Sans', ... }crates/ironclaw_gateway/static/styles/surfaces/settings.cssstill hardcodes'IBM Plex Mono'in several controls
Because the old Google Fonts import was removed, those surfaces now fall back to system UI / generic monospace instead of using the new design-system fonts. That means the advertised typography change does not actually land across the app, and some surfaces visibly regress.
Suggested fix: update the gateway CSS consumers to use the new tokens (font-family: var(--font-sans) / var(--font-mono)) before switching the imported font families.
serrrfirat
reviewed
Apr 20, 2026
| ("memory", "#memory-tab-container, #memory-tree, .empty-state"), | ||
| ("jobs", ".jobs-container, .empty-state"), | ||
| ("missions", ".missions-container, .empty-state"), | ||
| ("routines", ".routines-container, .empty-state"), |
Collaborator
There was a problem hiding this comment.
Medium Severity
The screenshot harness is claiming to capture both missions and routines, but with engine v2 forced on it can silently capture missions twice.
This test flips engineV2Enabled = true before the tab walk, but still includes ("routines", ...) in DARK_SURFACES. In v2 mode the router normalizes #/routines to missions, and _capture_tab() then waits on selectors that include the generic .empty-state, so the screenshot step still succeeds even if it never reached a real routines view.
The committed evidence already shows this: the before/dark-missions.png and before/dark-routines.png files are byte-identical, and the same is true for the after/ pair. So the README currently overstates coverage and this harness would miss a routines-only regression.
Suggested fix: either capture routines only with v1 mode enabled, or assert the active tab/panel before taking the screenshot so a misrouted capture fails instead of silently producing a duplicate PNG.
ilblackdragon
added a commit
that referenced
this pull request
Apr 20, 2026
* docs(rules): add review-driven guidance for Claude Code Synthesizes recurring patterns from ~30 merged PRs, 147 bot review comments (Copilot/Gemini), human reviews, and ~50 issues filed in the past 2 weeks. Each rule cites the motivating PR/issue numbers. New files: - error-handling.md — silent-failure taxonomy (unwrap_or_default, .ok()?, poisoned caches), persist-then-reload atomicity, channel-edge error mapping. (#2526, #2633, #2653, #2673, #2546, #2407, #2408) - agent-evidence.md — side-effect claims must cite tool evidence, empty-fast outputs are errors, external-effect tools must read back, setup UI round-trip. (#2544, #2580, #2582, #2541, #2545, #2411, #2543, #2586) - lifecycle.md — discovery vs. activation, terminal auth rejection, list_installed vs. list_active, deactivation unwinds, snapshot rehydrate must re-validate. (#2556, #2557, #2558, #2564, #2419, PR #2617, PR #2631) Extended: - types.md — from_trusted boundary rule, validated-newtype template with shared validate(&str), serde(try_from) required for validated types, wire-stable enums (no Debug; serde alias for migrations), canonical wire-contract field naming. (PR #2685, #2681, #2687, #2678, #2669, #2665, #2683, #2702) - safety-and-sandbox.md — every new ingress scans pre-transform/pre- injection, bounded resources (interners/streams/fan-out caps), cache keys must be complete. (#2491, #2676, #2470, #2633, #2673, #2710, PR #2702) - review-discipline.md — PR scope discipline, guardrail scripts are code (regression tests, grouped-import parsing, CI has_code inclusion), absolute-path ban in committed docs, stale comments after refactors. (PR #2668, #2628, #2680, #2687, #2647, #2689, #2701) All new files carry paths: frontmatter so they auto-load only on matching files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(rules): split agent-evidence into prompt + code rule agent-evidence.md mixed two concerns: runtime agent instruction (what the LLM should do when concluding a turn) and code-enforcement rules (what the dispatcher, engine, and tools must implement). Rules under .claude/rules/ only guide Claude Code when editing the repo — the runtime agent never reads them. Splits the two: - crates/ironclaw_engine/prompts/codeact_postamble.md — new section "Evidence before claiming side effects". Sits next to the existing "FINAL() answer quality" guidance; loaded via include_str! in executor/prompt.rs (no Rust change needed). - .claude/rules/tool-evidence.md — renamed from agent-evidence.md, keeps only the code invariants (engine v2 side-effect gate, empty-fast ToolError::EmptyResult, external-effect tools must read back, setup UI round-trip). Prompt tests pass unchanged; the postamble addition is pure text. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * prompt: tighten evidence rule to FINAL() claims only, not tool use Live-test validation of the "Evidence before claiming side effects" section (added in the prior commit) showed it inhibited legitimate tool use. With the original wording, `zizmor_scan_v2` live-recording timed out at 302s with zero responses; reverting the postamble restored healthy behavior (88s run, 8 shell calls including `cargo install zizmor` and full workflow analysis). The original phrasing conflated two things: what the agent should claim and what tools it should call. The rule is only about the claim. Re-tunes the section to: - Open with an explicit "this does not restrict tool calls" scope. - Drop the "<1ms = failure" heuristic (too broad — normal tools like `tool_info(schema)` are legitimately fast). - Drop the full enumeration of forbidden side-effect verbs; keep the rule narrower and clearer. - Shorten the code example (remove redundant early-return). Re-tuned run: agent is active (shell calls, real reasoning), live recording completes in ~9s. The remaining test failure is a pre-existing assertion bug (exact `t == "shell"` match against tool strings that now carry arguments like `"shell(cmd)"`) — reproduces with the old postamble too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(live): fix tool-name assertions + re-record zizmor traces The two `zizmor_scan*` live tests had four broken tool-name assertions that silently failed to match: `tools.iter().any(|t| t == "shell")` against a tool list that now contains `"shell(cmd)"` strings (tool events carry args via `format_action_display_name` in `src/bridge/router.rs`). Two of the four were negative assertions checking for the absence of `tool_install` recovery loops — those silently passed even when a recovery loop actually ran. `sandbox_live_e2e.rs:203` already used the correct `t == "shell" || t.starts_with("shell(")` pattern; applied it consistently to all four sites. Verified live: - `IRONCLAW_LIVE_TEST=1 cargo test --test e2e_live -- zizmor_scan --ignored --test-threads=1` → 2 passed, 0 failed, 51.78s. Agent installs and runs zizmor end-to-end, producing real findings (exit code 14, dangerous triggers, excessive permissions, etc.). Traces re-recorded with the tuned postamble (commit 50d8517) and scrubbed: replaced `/home/illia/.cargo/bin/zizmor` with `/home/user/.cargo/bin/zizmor` per the developer-local-path ban in `.claude/rules/review-discipline.md`. No credentials, PII, or high-entropy secrets in either trace (only git SHAs from zizmor's workflow analysis output). Replay still passes: `cargo test --test e2e_live -- zizmor_scan --ignored` → 2/2 ok. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(replay): update zizmor_scan_v2 insta snapshot The engine v2 replay-snapshot gate (`engine_v2_tests::snapshot_zizmor_scan_v2`) failed against the re-recorded trace from 1691efe because the old snapshot encoded a broken run: - final_state: Failed - Missing Assistant message role - 3 issues: thread_failure (error), no_response (warning), llm_error (error) - 6 tool calls that never produced a final answer The new trace completes cleanly: - final_state: Done - System / User / Assistant roles present - 1 issue: mixed_mode (info) - 3 shell tool calls + successful `FINAL()` with real findings The snapshot was pinning a regression. Regenerated with `INSTA_UPDATE=always cargo test --test e2e_engine_v2 -- snapshot_zizmor_scan_v2`; passes on replay. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(rules): address PR #2714 review feedback - review-discipline: reword "Doc Absolute Paths" as a review convention (pre-commit only scans .rs; the rule misleadingly claimed enforcement). - safety-and-sandbox: broaden `paths:` frontmatter to include the actual ingress owners (`bridge`, `channels`, `workspace`, `agent`, engine crate) so the rule auto-loads where it applies. - tool-evidence: mark the side-effect gate, empty-fast rule, and `unverified` flag as target/aspirational invariants — neither `ToolError::EmptyResult`, an `unverified` field on `ToolOutput`, nor a byte-count field on `ActionRecord` exist today. Point at concrete interim conventions (`ToolError::ExecutionFailed`, `unverified: true` in the JSON result body). - types: scope "Validated newtypes must gate Deserialize" to *new* types, document the `CredentialName`/`ExtensionName` exception (they intentionally use `#[serde(transparent)]` + derived `Deserialize` under the `serde_does_not_revalidate` test). Clarify the `from_trusted` trust boundary (trusted = typed upstream, untrusted = raw JSON field even if the field *name* is "registry entry"). Switch `new` template to `impl Into<String>` to avoid an unnecessary clone when an owned `String` is passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(rules): simplify types.md + split doc-hygiene; address review round 2 - types: collapse two templates into one canonical validated-newtype shape. New types use `#[serde(try_from = "String")]` with a shared `validate(&str)` helper — no more dual "transparent for some / try_from for others" guidance. `CredentialName`/`ExtensionName` are documented as the sole legacy exception (locked in by the `serde_does_not_revalidate` test); new code must not copy their `transparent` + `from_trusted` pattern. Removes the long "Using `from_trusted` safely" section and the separate "Validated newtypes must gate Deserialize" subsection that contradicted the Don'ts list. - doc-hygiene: new tiny rule file scoped to `**/*.md`, `**/*.py`, `docs/**` that carries the "no developer-local absolute paths in committed docs" convention. Removed from review-discipline.md where its `src/**/*.rs` scope meant the rule never loaded on the files it governed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(live): match hyphenated tool-install in attempted_relevant_tool The engine records `action_name` as the raw string the LLM emitted (`crates/ironclaw_engine/src/executor/structured.rs:381`), and the registry's lookup canonicalization only affects dispatch — not the name that reaches `StatusUpdate::ToolStarted`. The two other predicates in this file (`bad_recovery` at :420, `phase_b_recovery` at :531) already defend against both forms; this one should too, for consistency. Addresses PR #2714 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Paranoid Architect Review — Request Changes2 Critical, 4 High, 4 Medium findings. The core design-system architecture is sound, but the token migration is incomplete and introduces several accessibility regressions and visual breakages. Critical
High
Medium
Blocking before merge
|
Collaborator
Paranoid Architect Review — Request Changes2 Critical, 4 High, 4 Medium findings. The core design-system architecture (12-step Radix-style scales, semantic role aliases, legacy token preservation) is sound, but the token migration is incomplete and introduces several accessibility regressions. Critical
High
Medium
Blocking before merge
|
serrrfirat
reviewed
Apr 22, 2026
| --accent-6: #0b9053; | ||
| --accent-7: #17b267; | ||
| --accent-8: #3bcc81; | ||
| --accent-9: #73e2a5; /* brand anchor (interactive solid) */ |
Collaborator
There was a problem hiding this comment.
Critical: White text on this accent is unreadable (1.60:1 contrast). --accent-9 (#73e2a5) is light pastel green. At least 3 selectors in style.css use color: #fff with background: var(--accent): .badge-admin, .btn-primary, .tool-permission-toggle button.active. WCAG AA requires 4.5:1 for normal text.
The --accent-contrast: #031c12 token exists and would give proper contrast, but these selectors use hardcoded #fff instead of var(--text-on-accent).
Fix: Replace color: #fff with color: var(--text-on-accent) in all accent-background selectors.
serrrfirat
reviewed
Apr 22, 2026
| * =========================================================== */ | ||
| --shadow-paper: 0 1px 0 var(--sand-a4), 0 8px 24px -8px rgba(0, 0, 0, 0.35); | ||
| --shadow-widget: 0 0 0 1px var(--sand-a4), 0 12px 32px -12px rgba(0, 0, 0, 0.50); | ||
| --shadow-card: 0 1px 2px var(--sand-a3), 0 4px 16px -6px rgba(0, 0, 0, 0.40); |
Collaborator
There was a problem hiding this comment.
Critical: --shadow-card light-mode override was dropped. Old theme had [data-theme="light"] { --shadow-card: 0 4px 24px rgba(0,0,0,0.08) }. This :root definition with rgba(0,0,0,0.40) is only appropriate for dark mode — in light mode it produces 5x heavier card shadows.
Same issue for --shadow-paper, --shadow-widget, --shadow-inset defined nearby.
Fix: Add these tokens to the [data-theme="light"] block with appropriate light-mode opacities (e.g., 0.06-0.10).
serrrfirat
reviewed
Apr 22, 2026
| --success: var(--accent-9); | ||
| --info: #60a5fa; | ||
| --warning: #F5A623; | ||
| --warning-bg: rgba(245, 166, 35, 0.05); |
Collaborator
There was a problem hiding this comment.
High: Warning background nearly invisible in dark mode. Old value: #1e1400 (opaque dark amber). New value: rgba(245, 166, 35, 0.05) (5% alpha). On --bg (#080c0a) this is virtually indistinguishable from the background — a 30x reduction in effective opacity.
The .restart-modal-warning component and other warning surfaces will be invisible.
Fix: Increase opacity to at least 0.15, or restore an opaque dark amber value.
Member
Author
|
Won't merge. Not applicable anymore. |
henrypark133
added a commit
that referenced
this pull request
Jun 8, 2026
#4572) * docs(reborn): planner subagent + spawn_subagent schema redesign spec Scope A: replace researcher flavor with planner, expose flavor enum to model, rewrite tool description to nudge parent toward planning. Out-of-scope: plan-mode for parent, durable plan files, nested spawning. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn): replace researcher flavor with planner - Drop Researcher variant + entry + direction file - Add Planner flavor: read codebase + http (web research) - Returns structured Markdown plan (Goal/Plan/Files/Risks) - Export builtin_flavor_catalog() for downstream schema builders - Update prompt_material.rs test to use Planner instead of Researcher Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(loop_support): spawn_subagent schema redesign - Wire rename: flavor_id -> subagent_type (serde alias keeps old format readable) - Dynamic schema builder takes flavor catalog; renders enum + per-flavor descriptions - Tool description rewrite nudges parent toward planner for complex tasks - New SpawnSubagentFlavorDescriptor type (avoids cyclic dep on ironclaw_reborn) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn): wire flavor catalog into spawn_subagent decorator Thread builtin_flavor_catalog() through SubagentSpawnCapabilityDecorator to populate the new subagent_type enum + per-flavor description. Convert &'static str descriptors to owned strings at the boundary to avoid leaking 'static lifetimes across crates. Test fixture cleanup: prompt_material.rs and spawn_result.rs now use "planner" instead of the removed "researcher" flavor id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(reborn): scrub local paths from spec; match sibling style in planner.md - docs/superpowers/specs/2026-06-08-planner-subagent-design.md: replace /Users/henry/Code/pi/... developer-local paths with repo-relative refs (doc-hygiene rule, PR #2689) - directions/planner.md: open with prose to match general.md/explorer.md/ coder.md siblings (was: H1 heading) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): derive flavor catalog from registry; cache spawn schema Code-review cleanup: - Add summary field to SubagentFlavor; derive builtin_flavor_catalog() from BUILTIN_SUBAGENT_FLAVORS instead of a hardcoded parallel list (single source of truth, no drift risk — for real this time) - Delete FlavorDescriptor; return SpawnSubagentFlavorDescriptor directly, removing the .to_string() adapter at the runtime.rs boundary - Drop now-vacuous parity test - Guard empty-catalog schema: omit enum key when catalog is empty (avoids unsatisfiable JSON Schema enum: [] constraint) - Cache parameters_schema in SubagentSpawnCapabilityPort at new() instead of rebuilding per LLM turn - Drop unjustified pub(crate) on PLANNER_DIRECTION; match siblings Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn): update prompt_material assertion after planner.md prose rewrite Fix Y rewrote planner.md opener from `# Planner subagent` H1 to prose matching siblings. The test assertion in prompt_material.rs (added in WU-A wire-up) needs to match the new lowercase prose form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn): cover empty-catalog schema, subagent_type canonical key, decorator threading Closes code-review test-coverage gaps (.review/findings.json): - empty catalog produces enum-less schema (regression guard on the empty-enum fix) - invoke path accepts subagent_type as canonical wire key (not just deser roundtrip) - decorator+catalog threading verified by source-of-truth assertion on builtin_flavor_catalog() Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): generalize planner direction for non-code domains The previous planner.md was code-coded: `Files to Modify`, `path/to/file.rs`, `coder subagent will execute`, etc. Made the prompt useless for non-engineering planning (trip planning, test plans, research outlines). Rewrite to be domain-neutral while keeping the same structural value: - Drop `Files to Modify` + `New Files` sections — Plan steps already carry what's produced; specific-naming discipline replaces the implicit "file" vocabulary - Replace `implementation plan` / `the coder will execute` framing with `parent will execute` — works for any executor (coder, scheduler, human) - Specificity examples broadened: `files, places, libraries, deadlines, URLs, costs` - Trimmed length from ~60 to ~30 lines to match sibling direction prompts Output format kept strict (Goal/Plan/Risks/References) — the structure IS the value across domains, just the vocabulary needed neutralizing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): address PR #4572 review feedback (6 of 7) - subagent_spawn_port.rs: deny unknown fields on wire args (matches schema additionalProperties: false contract) - subagent_spawn_port.rs: move SPAWN_SUBAGENT_DESCRIPTION to prompts/*.md per CLAUDE.md rule on multi-line prompt strings - runtime.rs: decorator precomputes schema once instead of cloning the catalog + rebuilding on every decorate() call - tests: codec decode with subagent_type canonical key, single-entry catalog schema, summary non-empty regression guard C5 (SubagentToolId::WebSearch -> builtin.http naming) deferred for discussion — pre-existing on origin/main, scope question. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): rename SubagentToolId::WebSearch -> Http Closes PR #4572 C5: enum variant now matches the capability name it maps to (builtin.http), consistent with every other variant in SubagentToolId. Reader grepping for builtin.http now finds the variant; the future distinction between general HTTP and a search-specific capability is no longer blocked by name collision. Pre-existing on origin/main but only the planner allowlist references it now (researcher was removed earlier in this PR), and SubagentToolId is flavors.rs-internal with no external consumers — pure mechanical rename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(host_runtime): align spawn_subagent first-party schema with decorator Copilot flagged a second hardcoded schema source at `crates/ironclaw_host_runtime/src/first_party_tools/schemas.rs` that still advertised `flavor_id` and listed the deprecated `researcher` flavor. The decorator's `tool_definitions()` skips publishing its dynamic schema when `inner` already exposes spawn_subagent — so in production the model could see the stale schemas.rs shape (flavor_id, researcher) instead of the canonical decorator shape (subagent_type enum, planner). This commit updates the host_runtime entry to publish the SAME shape the decorator builds dynamically: `subagent_type` enum of [general, explorer, coder, planner] + per-flavor description. Adds an inline comment noting the dual source of truth and the long-term fix (route through `build_spawn_subagent_parameters_schema`). Test fixture: `first_party_builtin_tools.rs` updated to assert the `subagent_type` key (was: `flavor_id`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(loop_support): address PR #4572 round-3 feedback (C8-C12) - prompts/spawn_subagent_description.md: add trailing newline (sibling parity) - SpawnSubagentFlavorDescriptor.id: String -> SubagentKindId (typed id) - Test: register/validate rejects unknown wire field (covers deny_unknown_fields) - Test: new_with_schema propagates precomputed schema to tool_definition - Port parameters_schema: Value -> Arc<Value> to avoid per-decorate() deep clone Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): inline safety comment on .expect() in builtin_flavor_catalog CI no-panics check scans line-by-line for // safety: markers on the same line as .expect()/.unwrap()/assert!. The previous form put the reason inside the .expect() message string (split across 3 lines after rustfmt), which the script can't recognize. Restructured so the .expect( call and // safety: comment land on the same line after rustfmt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
theredspoon
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Jun 21, 2026
* docs(rules): add review-driven guidance for Claude Code Synthesizes recurring patterns from ~30 merged PRs, 147 bot review comments (Copilot/Gemini), human reviews, and ~50 issues filed in the past 2 weeks. Each rule cites the motivating PR/issue numbers. New files: - error-handling.md — silent-failure taxonomy (unwrap_or_default, .ok()?, poisoned caches), persist-then-reload atomicity, channel-edge error mapping. (nearai#2526, nearai#2633, nearai#2653, nearai#2673, nearai#2546, nearai#2407, nearai#2408) - agent-evidence.md — side-effect claims must cite tool evidence, empty-fast outputs are errors, external-effect tools must read back, setup UI round-trip. (nearai#2544, nearai#2580, nearai#2582, nearai#2541, nearai#2545, nearai#2411, nearai#2543, nearai#2586) - lifecycle.md — discovery vs. activation, terminal auth rejection, list_installed vs. list_active, deactivation unwinds, snapshot rehydrate must re-validate. (nearai#2556, nearai#2557, nearai#2558, nearai#2564, nearai#2419, PR nearai#2617, PR nearai#2631) Extended: - types.md — from_trusted boundary rule, validated-newtype template with shared validate(&str), serde(try_from) required for validated types, wire-stable enums (no Debug; serde alias for migrations), canonical wire-contract field naming. (PR nearai#2685, nearai#2681, nearai#2687, nearai#2678, nearai#2669, nearai#2665, nearai#2683, nearai#2702) - safety-and-sandbox.md — every new ingress scans pre-transform/pre- injection, bounded resources (interners/streams/fan-out caps), cache keys must be complete. (nearai#2491, nearai#2676, nearai#2470, nearai#2633, nearai#2673, nearai#2710, PR nearai#2702) - review-discipline.md — PR scope discipline, guardrail scripts are code (regression tests, grouped-import parsing, CI has_code inclusion), absolute-path ban in committed docs, stale comments after refactors. (PR nearai#2668, nearai#2628, nearai#2680, nearai#2687, nearai#2647, nearai#2689, nearai#2701) All new files carry paths: frontmatter so they auto-load only on matching files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(rules): split agent-evidence into prompt + code rule agent-evidence.md mixed two concerns: runtime agent instruction (what the LLM should do when concluding a turn) and code-enforcement rules (what the dispatcher, engine, and tools must implement). Rules under .claude/rules/ only guide Claude Code when editing the repo — the runtime agent never reads them. Splits the two: - crates/ironclaw_engine/prompts/codeact_postamble.md — new section "Evidence before claiming side effects". Sits next to the existing "FINAL() answer quality" guidance; loaded via include_str! in executor/prompt.rs (no Rust change needed). - .claude/rules/tool-evidence.md — renamed from agent-evidence.md, keeps only the code invariants (engine v2 side-effect gate, empty-fast ToolError::EmptyResult, external-effect tools must read back, setup UI round-trip). Prompt tests pass unchanged; the postamble addition is pure text. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * prompt: tighten evidence rule to FINAL() claims only, not tool use Live-test validation of the "Evidence before claiming side effects" section (added in the prior commit) showed it inhibited legitimate tool use. With the original wording, `zizmor_scan_v2` live-recording timed out at 302s with zero responses; reverting the postamble restored healthy behavior (88s run, 8 shell calls including `cargo install zizmor` and full workflow analysis). The original phrasing conflated two things: what the agent should claim and what tools it should call. The rule is only about the claim. Re-tunes the section to: - Open with an explicit "this does not restrict tool calls" scope. - Drop the "<1ms = failure" heuristic (too broad — normal tools like `tool_info(schema)` are legitimately fast). - Drop the full enumeration of forbidden side-effect verbs; keep the rule narrower and clearer. - Shorten the code example (remove redundant early-return). Re-tuned run: agent is active (shell calls, real reasoning), live recording completes in ~9s. The remaining test failure is a pre-existing assertion bug (exact `t == "shell"` match against tool strings that now carry arguments like `"shell(cmd)"`) — reproduces with the old postamble too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(live): fix tool-name assertions + re-record zizmor traces The two `zizmor_scan*` live tests had four broken tool-name assertions that silently failed to match: `tools.iter().any(|t| t == "shell")` against a tool list that now contains `"shell(cmd)"` strings (tool events carry args via `format_action_display_name` in `src/bridge/router.rs`). Two of the four were negative assertions checking for the absence of `tool_install` recovery loops — those silently passed even when a recovery loop actually ran. `sandbox_live_e2e.rs:203` already used the correct `t == "shell" || t.starts_with("shell(")` pattern; applied it consistently to all four sites. Verified live: - `IRONCLAW_LIVE_TEST=1 cargo test --test e2e_live -- zizmor_scan --ignored --test-threads=1` → 2 passed, 0 failed, 51.78s. Agent installs and runs zizmor end-to-end, producing real findings (exit code 14, dangerous triggers, excessive permissions, etc.). Traces re-recorded with the tuned postamble (commit 50d8517) and scrubbed: replaced `/home/illia/.cargo/bin/zizmor` with `/home/user/.cargo/bin/zizmor` per the developer-local-path ban in `.claude/rules/review-discipline.md`. No credentials, PII, or high-entropy secrets in either trace (only git SHAs from zizmor's workflow analysis output). Replay still passes: `cargo test --test e2e_live -- zizmor_scan --ignored` → 2/2 ok. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(replay): update zizmor_scan_v2 insta snapshot The engine v2 replay-snapshot gate (`engine_v2_tests::snapshot_zizmor_scan_v2`) failed against the re-recorded trace from 1691efe because the old snapshot encoded a broken run: - final_state: Failed - Missing Assistant message role - 3 issues: thread_failure (error), no_response (warning), llm_error (error) - 6 tool calls that never produced a final answer The new trace completes cleanly: - final_state: Done - System / User / Assistant roles present - 1 issue: mixed_mode (info) - 3 shell tool calls + successful `FINAL()` with real findings The snapshot was pinning a regression. Regenerated with `INSTA_UPDATE=always cargo test --test e2e_engine_v2 -- snapshot_zizmor_scan_v2`; passes on replay. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(rules): address PR nearai#2714 review feedback - review-discipline: reword "Doc Absolute Paths" as a review convention (pre-commit only scans .rs; the rule misleadingly claimed enforcement). - safety-and-sandbox: broaden `paths:` frontmatter to include the actual ingress owners (`bridge`, `channels`, `workspace`, `agent`, engine crate) so the rule auto-loads where it applies. - tool-evidence: mark the side-effect gate, empty-fast rule, and `unverified` flag as target/aspirational invariants — neither `ToolError::EmptyResult`, an `unverified` field on `ToolOutput`, nor a byte-count field on `ActionRecord` exist today. Point at concrete interim conventions (`ToolError::ExecutionFailed`, `unverified: true` in the JSON result body). - types: scope "Validated newtypes must gate Deserialize" to *new* types, document the `CredentialName`/`ExtensionName` exception (they intentionally use `#[serde(transparent)]` + derived `Deserialize` under the `serde_does_not_revalidate` test). Clarify the `from_trusted` trust boundary (trusted = typed upstream, untrusted = raw JSON field even if the field *name* is "registry entry"). Switch `new` template to `impl Into<String>` to avoid an unnecessary clone when an owned `String` is passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(rules): simplify types.md + split doc-hygiene; address review round 2 - types: collapse two templates into one canonical validated-newtype shape. New types use `#[serde(try_from = "String")]` with a shared `validate(&str)` helper — no more dual "transparent for some / try_from for others" guidance. `CredentialName`/`ExtensionName` are documented as the sole legacy exception (locked in by the `serde_does_not_revalidate` test); new code must not copy their `transparent` + `from_trusted` pattern. Removes the long "Using `from_trusted` safely" section and the separate "Validated newtypes must gate Deserialize" subsection that contradicted the Don'ts list. - doc-hygiene: new tiny rule file scoped to `**/*.md`, `**/*.py`, `docs/**` that carries the "no developer-local absolute paths in committed docs" convention. Removed from review-discipline.md where its `src/**/*.rs` scope meant the rule never loaded on the files it governed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(live): match hyphenated tool-install in attempted_relevant_tool The engine records `action_name` as the raw string the LLM emitted (`crates/ironclaw_engine/src/executor/structured.rs:381`), and the registry's lookup canonicalization only affects dispatch — not the name that reaches `StatusUpdate::ToolStarted`. The two other predicates in this file (`bad_recovery` at :420, `phase_b_recovery` at :531) already defend against both forms; this one should too, for consistency. Addresses PR nearai#2714 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
theredspoon
pushed a commit
to theredspoon/ironclaw
that referenced
this pull request
Jun 21, 2026
nearai#4572) * docs(reborn): planner subagent + spawn_subagent schema redesign spec Scope A: replace researcher flavor with planner, expose flavor enum to model, rewrite tool description to nudge parent toward planning. Out-of-scope: plan-mode for parent, durable plan files, nested spawning. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn): replace researcher flavor with planner - Drop Researcher variant + entry + direction file - Add Planner flavor: read codebase + http (web research) - Returns structured Markdown plan (Goal/Plan/Files/Risks) - Export builtin_flavor_catalog() for downstream schema builders - Update prompt_material.rs test to use Planner instead of Researcher Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(loop_support): spawn_subagent schema redesign - Wire rename: flavor_id -> subagent_type (serde alias keeps old format readable) - Dynamic schema builder takes flavor catalog; renders enum + per-flavor descriptions - Tool description rewrite nudges parent toward planner for complex tasks - New SpawnSubagentFlavorDescriptor type (avoids cyclic dep on ironclaw_reborn) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(reborn): wire flavor catalog into spawn_subagent decorator Thread builtin_flavor_catalog() through SubagentSpawnCapabilityDecorator to populate the new subagent_type enum + per-flavor description. Convert &'static str descriptors to owned strings at the boundary to avoid leaking 'static lifetimes across crates. Test fixture cleanup: prompt_material.rs and spawn_result.rs now use "planner" instead of the removed "researcher" flavor id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(reborn): scrub local paths from spec; match sibling style in planner.md - docs/superpowers/specs/2026-06-08-planner-subagent-design.md: replace /Users/henry/Code/pi/... developer-local paths with repo-relative refs (doc-hygiene rule, PR nearai#2689) - directions/planner.md: open with prose to match general.md/explorer.md/ coder.md siblings (was: H1 heading) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): derive flavor catalog from registry; cache spawn schema Code-review cleanup: - Add summary field to SubagentFlavor; derive builtin_flavor_catalog() from BUILTIN_SUBAGENT_FLAVORS instead of a hardcoded parallel list (single source of truth, no drift risk — for real this time) - Delete FlavorDescriptor; return SpawnSubagentFlavorDescriptor directly, removing the .to_string() adapter at the runtime.rs boundary - Drop now-vacuous parity test - Guard empty-catalog schema: omit enum key when catalog is empty (avoids unsatisfiable JSON Schema enum: [] constraint) - Cache parameters_schema in SubagentSpawnCapabilityPort at new() instead of rebuilding per LLM turn - Drop unjustified pub(crate) on PLANNER_DIRECTION; match siblings Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn): update prompt_material assertion after planner.md prose rewrite Fix Y rewrote planner.md opener from `# Planner subagent` H1 to prose matching siblings. The test assertion in prompt_material.rs (added in WU-A wire-up) needs to match the new lowercase prose form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(reborn): cover empty-catalog schema, subagent_type canonical key, decorator threading Closes code-review test-coverage gaps (.review/findings.json): - empty catalog produces enum-less schema (regression guard on the empty-enum fix) - invoke path accepts subagent_type as canonical wire key (not just deser roundtrip) - decorator+catalog threading verified by source-of-truth assertion on builtin_flavor_catalog() Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): generalize planner direction for non-code domains The previous planner.md was code-coded: `Files to Modify`, `path/to/file.rs`, `coder subagent will execute`, etc. Made the prompt useless for non-engineering planning (trip planning, test plans, research outlines). Rewrite to be domain-neutral while keeping the same structural value: - Drop `Files to Modify` + `New Files` sections — Plan steps already carry what's produced; specific-naming discipline replaces the implicit "file" vocabulary - Replace `implementation plan` / `the coder will execute` framing with `parent will execute` — works for any executor (coder, scheduler, human) - Specificity examples broadened: `files, places, libraries, deadlines, URLs, costs` - Trimmed length from ~60 to ~30 lines to match sibling direction prompts Output format kept strict (Goal/Plan/Risks/References) — the structure IS the value across domains, just the vocabulary needed neutralizing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): address PR nearai#4572 review feedback (6 of 7) - subagent_spawn_port.rs: deny unknown fields on wire args (matches schema additionalProperties: false contract) - subagent_spawn_port.rs: move SPAWN_SUBAGENT_DESCRIPTION to prompts/*.md per CLAUDE.md rule on multi-line prompt strings - runtime.rs: decorator precomputes schema once instead of cloning the catalog + rebuilding on every decorate() call - tests: codec decode with subagent_type canonical key, single-entry catalog schema, summary non-empty regression guard C5 (SubagentToolId::WebSearch -> builtin.http naming) deferred for discussion — pre-existing on origin/main, scope question. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(reborn): rename SubagentToolId::WebSearch -> Http Closes PR nearai#4572 C5: enum variant now matches the capability name it maps to (builtin.http), consistent with every other variant in SubagentToolId. Reader grepping for builtin.http now finds the variant; the future distinction between general HTTP and a search-specific capability is no longer blocked by name collision. Pre-existing on origin/main but only the planner allowlist references it now (researcher was removed earlier in this PR), and SubagentToolId is flavors.rs-internal with no external consumers — pure mechanical rename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(host_runtime): align spawn_subagent first-party schema with decorator Copilot flagged a second hardcoded schema source at `crates/ironclaw_host_runtime/src/first_party_tools/schemas.rs` that still advertised `flavor_id` and listed the deprecated `researcher` flavor. The decorator's `tool_definitions()` skips publishing its dynamic schema when `inner` already exposes spawn_subagent — so in production the model could see the stale schemas.rs shape (flavor_id, researcher) instead of the canonical decorator shape (subagent_type enum, planner). This commit updates the host_runtime entry to publish the SAME shape the decorator builds dynamically: `subagent_type` enum of [general, explorer, coder, planner] + per-flavor description. Adds an inline comment noting the dual source of truth and the long-term fix (route through `build_spawn_subagent_parameters_schema`). Test fixture: `first_party_builtin_tools.rs` updated to assert the `subagent_type` key (was: `flavor_id`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(loop_support): address PR nearai#4572 round-3 feedback (C8-C12) - prompts/spawn_subagent_description.md: add trailing newline (sibling parity) - SpawnSubagentFlavorDescriptor.id: String -> SubagentKindId (typed id) - Test: register/validate rejects unknown wire field (covers deny_unknown_fields) - Test: new_with_schema propagates precomputed schema to tool_definition - Port parameters_schema: Value -> Arc<Value> to avoid per-decorate() deep clone Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ci): inline safety comment on .expect() in builtin_flavor_catalog CI no-panics check scans line-by-line for // safety: markers on the same line as .expect()/.unwrap()/assert!. The previous form put the reason inside the .expect() message string (split across 3 lines after rustfmt), which the script can't recognize. Restructured so the .expect( call and // safety: comment land on the same line after rustfmt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First visible step of the design-system adoption planned in
/home/illia/.claude/plans/quiet-nibbling-moth.md. Restructurestheme.csswith Radix Themes-style 12-step scales anchored on Defuse's OmniSwap whitelabel palette (the closest in-house Defuse palette to IronClaw's current emerald), introduces semantic role tokens, a layered shadow system, extended radii + type scale, and swaps the webfont to Geist (Geist Mono for monospace).Every legacy token (
--bg,--text,--accent,--shadow-card, …) is preserved and aliased onto the new scales — the 20 split surface stylesheets understyles/require zero changes. Palette swaps are now a one-file edit.What changed
--accent-1..12+--accent-a1..12from OmniSwaposa*: mint→forest emerald. Brand anchor at--accent-9=#73e2a5(dark) /#0a5b38(light). Old single--accent: #34d399is now an alias onto--accent-9.--sand-1..12+--sand-a1..12from OmniSwapos*: green-tinged warm grays.--color-{bg,surface,surface-elevated,surface-hover,border,border-hover,text,text-muted,text-subtle,label,warning,warning-foreground}+--accent-contrast.--shadow-{paper,widget,card,inset}replace a single--shadow.--radius-{xl:16,2xl:20,4xl:30}.--text-{4xl:32,5xl:48,6xl:64}+ display tracking.--font-sans: 'Geist',--font-mono: 'Geist Mono'via Google Fonts (CSP already allowsfonts.googleapis.com/fonts.gstatic.com).Admin panel falls back to the system font chain — admin's CSP blocks the Google Fonts origin on purpose (admin is strictly same-origin).
Visual diff
Captured with
tests/e2e/scenarios/capture_design_surfaces.py(opt-in Playwright scenario added in this PR, viewport 1280×800, Chromium). Full gallery atdocs/design-system/phase-a/.Dark mode
Light mode
Test plan
cargo fmt --check— cleancargo clippy --all --benches --tests --examples --all-features— zero warningscargo test --lib 'channels::web'— 431 passbash scripts/pre-commit-safety.sh— passdocs/design-system/phase-a/)Non-goals
static/styles/— those consume legacy tokens, which now alias onto the new scales.🤖 Generated with Claude Code