feat: console harness onboarding - #210
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR implements harness worker monitoring and installation UI for the iii console. It introduces a ChangesHarness Status and Chat Integration
UI Polish and Visual Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
skill-check — worker0 verified, 13 skipped (no docs/).
Note 17 stale rendered artifact(s) detected on main, unrelated to this PR. This PR is fine; the drift was already there. A maintainer should open a chore PR to re-render these.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/web/src/components/chat/EmptyState.tsx`:
- Around line 207-213: The branch hides errorMessage whenever any stages exist;
change the condition to show the fallback error unless a failed stage is already
present. In EmptyState.tsx update the check that currently uses stages.length
=== 0 to instead detect whether a failed stage exists (e.g., !stages.some(stage
=> stage.type === 'failed')) so the errorMessage is shown when failed is true
and no explicit failed stage was emitted.
In `@console/web/src/components/chat/sandbox/terminal/CopyCommandButton.tsx`:
- Around line 15-23: The handleCopy logic currently stacks timeouts and can
reset `copied` from earlier clicks; fix this by adding a ref (e.g.,
`copyTimeoutRef`) to store the timer id, clear it before creating a new timeout,
set `copied` true as before, then assign the new timeout id to the ref; also add
a cleanup (useEffect return) to clear the timeout on unmount so
`setCopied(false)` from an old timer never fires after the component is gone.
Use the existing `copied`, `setCopied`, and `handleCopy` identifiers to locate
where to add the ref and cleanup.
In `@console/web/src/components/ui/Wordmark.tsx`:
- Around line 19-42: Wordmark applies the consumer-provided className only to
the img root for 'ink'/'inverse' but to child imgs for 'auto', breaking layout
contracts; change Wordmark so the root element always receives sizeClass
(cn(IMG_CLASS, className)) and return a wrapper (span) for all tone variants,
with inner <img> elements using only image-specific classes (e.g., IMG_CLASS or
icon-only classes) and not the consumer className; update the branches that
currently return an <img> directly (the 'ink' and 'inverse' cases) to return a
wrapper with sizeClass and a single <img> child using inkUrl/whiteUrl
respectively, keeping aria attributes unchanged.
In `@console/web/src/hooks/use-harness-status.ts`:
- Around line 182-205: The presence-check flow can miss a race where a harness
is added between checkHarnessPresent() returning false and registerTrigger()
being bound; move the trigger subscription setup (client.on(HARNESS_WATCH_FN,
...) and client.registerTrigger({...})) to occur before you finalize the first
probe result, or alternatively perform an immediate re-check (call
checkHarnessPresent(client) again and update setPresent()) right after
registerTrigger() completes; ensure you still respect the cancelled guard and
only setLoading(false) after the trigger is bound and the final presence
decision is applied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b57bd36d-dc17-4680-8a1c-ed1121e447b6
⛔ Files ignored due to path filters (3)
console/web/src/icons/favicon.svgis excluded by!**/*.svgconsole/web/src/icons/iii-ink.svgis excluded by!**/*.svgconsole/web/src/icons/iii-white.svgis excluded by!**/*.svg
📒 Files selected for processing (15)
console/web/index.htmlconsole/web/src/App.tsxconsole/web/src/components/chat/ChatView.tsxconsole/web/src/components/chat/Composer.tsxconsole/web/src/components/chat/EmptyState.stories.tsxconsole/web/src/components/chat/EmptyState.tsxconsole/web/src/components/chat/MessageList.tsxconsole/web/src/components/chat/sandbox/terminal/CopyCommandButton.tsxconsole/web/src/components/ui/Dialog.tsxconsole/web/src/components/ui/Wordmark.stories.tsxconsole/web/src/components/ui/Wordmark.tsxconsole/web/src/hooks/use-harness-status.tsconsole/web/src/lib/conversations-context.tsxconsole/web/src/lib/iii-client.tsconsole/web/src/main.tsx
Comment on lines
+207
to
+213
| {failed ? ( | ||
| <div className="flex flex-col gap-3"> | ||
| {errorMessage && stages.length === 0 ? ( | ||
| <span className="font-mono text-[12.5px] text-alert lowercase break-all"> | ||
| {errorMessage} | ||
| </span> | ||
| ) : null} |
There was a problem hiding this comment.
Show the fallback error even when partial progress exists.
If worker::add rejects after emitting started/downloading but never publishes a failed stage, this branch hides errorMessage just because stages.length > 0. The failed state then loses the only actionable reason. Gate this on whether a failed stage is already present, not on whether any stage exists.
Suggested fix
- {errorMessage && stages.length === 0 ? (
+ {errorMessage && !stages.some((stage) => stage.stage === 'failed') ? (
<span className="font-mono text-[12.5px] text-alert lowercase break-all">
{errorMessage}
</span>
) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/EmptyState.tsx` around lines 207 - 213, The
branch hides errorMessage whenever any stages exist; change the condition to
show the fallback error unless a failed stage is already present. In
EmptyState.tsx update the check that currently uses stages.length === 0 to
instead detect whether a failed stage exists (e.g., !stages.some(stage =>
stage.type === 'failed')) so the errorMessage is shown when failed is true and
no explicit failed stage was emitted.
Comment on lines
+15
to
+23
| const [copied, setCopied] = useState(false) | ||
|
|
||
| const handleCopy = useCallback(() => { | ||
| if (typeof navigator === 'undefined' || !navigator.clipboard) return | ||
| void navigator.clipboard.writeText(text).then(() => { | ||
| setCopied(true) | ||
| window.setTimeout(() => setCopied(false), 1200) | ||
| }) | ||
| }, [text]) |
There was a problem hiding this comment.
Re-arm the reset timer instead of stacking multiple timeouts.
Each successful click schedules a new timeout, so repeated copies can flip copied back to false based on an older click instead of the latest one. Store the timer id in a ref, clear it before starting a new one, and clean it up on unmount.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/chat/sandbox/terminal/CopyCommandButton.tsx`
around lines 15 - 23, The handleCopy logic currently stacks timeouts and can
reset `copied` from earlier clicks; fix this by adding a ref (e.g.,
`copyTimeoutRef`) to store the timer id, clear it before creating a new timeout,
set `copied` true as before, then assign the new timeout id to the ref; also add
a cleanup (useEffect return) to clear the timeout on unmount so
`setCopied(false)` from an old timer never fires after the component is gone.
Use the existing `copied`, `setCopied`, and `handleCopy` identifiers to locate
where to add the ref and cleanup.
Comment on lines
+19
to
+42
| export function Wordmark({ className, tone = 'auto' }: WordmarkProps) { | ||
| const sizeClass = cn(IMG_CLASS, className) | ||
|
|
||
| if (tone === 'ink') { | ||
| return <img src={inkUrl} alt="iii" className={sizeClass} /> | ||
| } | ||
|
|
||
| if (tone === 'inverse') { | ||
| return <img src={whiteUrl} alt="iii" className={sizeClass} /> | ||
| } | ||
|
|
||
| function Glyph() { | ||
| return ( | ||
| <span aria-hidden className="inline-flex flex-col items-center gap-[3px]"> | ||
| {/* tittle */} | ||
| <span className="block size-[4px] bg-ink" /> | ||
| {/* stem */} | ||
| <span className="block w-[4px] h-[12px] bg-ink" /> | ||
| <span role="img" aria-label="iii" className="inline-flex"> | ||
| <img | ||
| src={inkUrl} | ||
| alt="" | ||
| aria-hidden | ||
| className={cn(sizeClass, '[html[data-theme=dark]_&]:hidden')} | ||
| /> | ||
| <img | ||
| src={whiteUrl} | ||
| alt="" | ||
| aria-hidden | ||
| className={cn(sizeClass, 'hidden [html[data-theme=dark]_&]:inline')} |
There was a problem hiding this comment.
Keep className on a stable root element across all tone variants.
ink/inverse apply className to the returned root node, but auto pushes it down to both <img> children and leaves the wrapper unstyled. That makes layout utilities like mx-*, self-*, inline-*, and selector hooks behave differently depending on tone, which is a shared-component contract break.
Proposed direction
export function Wordmark({ className, tone = 'auto' }: WordmarkProps) {
- const sizeClass = cn(IMG_CLASS, className)
+ const rootClass = cn('inline-flex', className)
if (tone === 'ink') {
- return <img src={inkUrl} alt="iii" className={sizeClass} />
+ return (
+ <span role="img" aria-label="iii" className={rootClass}>
+ <img src={inkUrl} alt="" aria-hidden className={IMG_CLASS} />
+ </span>
+ )
}
if (tone === 'inverse') {
- return <img src={whiteUrl} alt="iii" className={sizeClass} />
+ return (
+ <span role="img" aria-label="iii" className={rootClass}>
+ <img src={whiteUrl} alt="" aria-hidden className={IMG_CLASS} />
+ </span>
+ )
}
return (
- <span role="img" aria-label="iii" className="inline-flex">
+ <span role="img" aria-label="iii" className={rootClass}>
<img
src={inkUrl}
alt=""
aria-hidden
- className={cn(sizeClass, '[html[data-theme=dark]_&]:hidden')}
+ className={cn(IMG_CLASS, '[html[data-theme=dark]_&]:hidden')}
/>
<img
src={whiteUrl}
alt=""
aria-hidden
- className={cn(sizeClass, 'hidden [html[data-theme=dark]_&]:inline')}
+ className={cn(IMG_CLASS, 'hidden [html[data-theme=dark]_&]:inline')}
/>
</span>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function Wordmark({ className, tone = 'auto' }: WordmarkProps) { | |
| const sizeClass = cn(IMG_CLASS, className) | |
| if (tone === 'ink') { | |
| return <img src={inkUrl} alt="iii" className={sizeClass} /> | |
| } | |
| if (tone === 'inverse') { | |
| return <img src={whiteUrl} alt="iii" className={sizeClass} /> | |
| } | |
| function Glyph() { | |
| return ( | |
| <span aria-hidden className="inline-flex flex-col items-center gap-[3px]"> | |
| {/* tittle */} | |
| <span className="block size-[4px] bg-ink" /> | |
| {/* stem */} | |
| <span className="block w-[4px] h-[12px] bg-ink" /> | |
| <span role="img" aria-label="iii" className="inline-flex"> | |
| <img | |
| src={inkUrl} | |
| alt="" | |
| aria-hidden | |
| className={cn(sizeClass, '[html[data-theme=dark]_&]:hidden')} | |
| /> | |
| <img | |
| src={whiteUrl} | |
| alt="" | |
| aria-hidden | |
| className={cn(sizeClass, 'hidden [html[data-theme=dark]_&]:inline')} | |
| export function Wordmark({ className, tone = 'auto' }: WordmarkProps) { | |
| const rootClass = cn('inline-flex', className) | |
| if (tone === 'ink') { | |
| return ( | |
| <span role="img" aria-label="iii" className={rootClass}> | |
| <img src={inkUrl} alt="" aria-hidden className={IMG_CLASS} /> | |
| </span> | |
| ) | |
| } | |
| if (tone === 'inverse') { | |
| return ( | |
| <span role="img" aria-label="iii" className={rootClass}> | |
| <img src={whiteUrl} alt="" aria-hidden className={IMG_CLASS} /> | |
| </span> | |
| ) | |
| } | |
| return ( | |
| <span role="img" aria-label="iii" className={rootClass}> | |
| <img | |
| src={inkUrl} | |
| alt="" | |
| aria-hidden | |
| className={cn(IMG_CLASS, '[html[data-theme=dark]_&]:hidden')} | |
| /> | |
| <img | |
| src={whiteUrl} | |
| alt="" | |
| aria-hidden | |
| className={cn(IMG_CLASS, 'hidden [html[data-theme=dark]_&]:inline')} | |
| /> | |
| </span> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/ui/Wordmark.tsx` around lines 19 - 42, Wordmark
applies the consumer-provided className only to the img root for 'ink'/'inverse'
but to child imgs for 'auto', breaking layout contracts; change Wordmark so the
root element always receives sizeClass (cn(IMG_CLASS, className)) and return a
wrapper (span) for all tone variants, with inner <img> elements using only
image-specific classes (e.g., IMG_CLASS or icon-only classes) and not the
consumer className; update the branches that currently return an <img> directly
(the 'ink' and 'inverse' cases) to return a wrapper with sizeClass and a single
<img> child using inkUrl/whiteUrl respectively, keeping aria attributes
unchanged.
Comment on lines
+182
to
+205
| void (async () => { | ||
| const client = await getIiiClient() | ||
| try { | ||
| const found = await checkHarnessPresent(client) | ||
| if (!cancelled) setPresent(found) | ||
| } catch { | ||
| if (!cancelled) setPresent(false) | ||
| } finally { | ||
| if (!cancelled) setLoading(false) | ||
| } | ||
| if (cancelled) return | ||
|
|
||
| // Subscribe to harness add events for live progress + CLI detection. | ||
| // If the engine doesn't publish the `worker` trigger type, fall back to | ||
| // the progress-less path (install() re-checks presence on resolve). | ||
| try { | ||
| offHandler = client.on(HARNESS_WATCH_FN, (data: unknown) => { | ||
| handlerRef.current(data) | ||
| }) | ||
| offTrigger = client.registerTrigger({ | ||
| type: 'worker', | ||
| function_id: `${HARNESS_WATCH_FN}::${client.browserId}`, | ||
| config: { operations: ['add'] }, | ||
| }) |
There was a problem hiding this comment.
Register the trigger before finishing the first presence check.
There’s a race here: if harness is added from the CLI after checkHarnessPresent() returns false but before registerTrigger() is bound, that add event is lost and the UI stays stuck in no-harness until a reload. Since this hook is the source of truth for external install detection, the trigger needs to be active before the initial probe result is finalized, or you need a second probe immediately after binding.
Suggested fix
void (async () => {
const client = await getIiiClient()
+ try {
+ offHandler = client.on(HARNESS_WATCH_FN, (data: unknown) => {
+ handlerRef.current(data)
+ })
+ offTrigger = client.registerTrigger({
+ type: 'worker',
+ function_id: `${HARNESS_WATCH_FN}::${client.browserId}`,
+ config: { operations: ['add'] },
+ })
+ } catch {
+ offTrigger?.()
+ offHandler?.()
+ offTrigger = undefined
+ offHandler = undefined
+ }
+
try {
const found = await checkHarnessPresent(client)
if (!cancelled) setPresent(found)
} catch {
if (!cancelled) setPresent(false)
} finally {
if (!cancelled) setLoading(false)
}
if (cancelled) return
-
- // Subscribe to harness add events for live progress + CLI detection.
- // If the engine doesn't publish the `worker` trigger type, fall back to
- // the progress-less path (install() re-checks presence on resolve).
- try {
- offHandler = client.on(HARNESS_WATCH_FN, (data: unknown) => {
- handlerRef.current(data)
- })
- offTrigger = client.registerTrigger({
- type: 'worker',
- function_id: `${HARNESS_WATCH_FN}::${client.browserId}`,
- config: { operations: ['add'] },
- })
- } catch {
- offTrigger?.()
- offHandler?.()
- offTrigger = undefined
- offHandler = undefined
- }
})()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void (async () => { | |
| const client = await getIiiClient() | |
| try { | |
| const found = await checkHarnessPresent(client) | |
| if (!cancelled) setPresent(found) | |
| } catch { | |
| if (!cancelled) setPresent(false) | |
| } finally { | |
| if (!cancelled) setLoading(false) | |
| } | |
| if (cancelled) return | |
| // Subscribe to harness add events for live progress + CLI detection. | |
| // If the engine doesn't publish the `worker` trigger type, fall back to | |
| // the progress-less path (install() re-checks presence on resolve). | |
| try { | |
| offHandler = client.on(HARNESS_WATCH_FN, (data: unknown) => { | |
| handlerRef.current(data) | |
| }) | |
| offTrigger = client.registerTrigger({ | |
| type: 'worker', | |
| function_id: `${HARNESS_WATCH_FN}::${client.browserId}`, | |
| config: { operations: ['add'] }, | |
| }) | |
| void (async () => { | |
| const client = await getIiiClient() | |
| try { | |
| offHandler = client.on(HARNESS_WATCH_FN, (data: unknown) => { | |
| handlerRef.current(data) | |
| }) | |
| offTrigger = client.registerTrigger({ | |
| type: 'worker', | |
| function_id: `${HARNESS_WATCH_FN}::${client.browserId}`, | |
| config: { operations: ['add'] }, | |
| }) | |
| } catch { | |
| offTrigger?.() | |
| offHandler?.() | |
| offTrigger = undefined | |
| offHandler = undefined | |
| } | |
| try { | |
| const found = await checkHarnessPresent(client) | |
| if (!cancelled) setPresent(found) | |
| } catch { | |
| if (!cancelled) setPresent(false) | |
| } finally { | |
| if (!cancelled) setLoading(false) | |
| } | |
| if (cancelled) return | |
| })() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/hooks/use-harness-status.ts` around lines 182 - 205, The
presence-check flow can miss a race where a harness is added between
checkHarnessPresent() returning false and registerTrigger() being bound; move
the trigger subscription setup (client.on(HARNESS_WATCH_FN, ...) and
client.registerTrigger({...})) to occur before you finalize the first probe
result, or alternatively perform an immediate re-check (call
checkHarnessPresent(client) again and update setPresent()) right after
registerTrigger() completes; ensure you still respect the cancelled guard and
only setLoading(false) after the trigger is bound and the final presence
decision is applied.
This was referenced Jun 3, 2026
This was referenced Jul 3, 2026
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
1 participant
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.
Check Storybook
Summary by CodeRabbit
New Features
Style
Tests