Embed OpenCode UI via Shadow DOM and improve worker interactivity#343
Merged
Embed OpenCode UI via Shadow DOM and improve worker interactivity#343
Conversation
Replace the iframe-based OpenCode embedding with a Shadow DOM approach that mounts the SolidJS SPA directly into a React-owned DOM node. This eliminates all the iframe shims (history patches, TextDecoderStream polyfills, WebKit EventSource workarounds, CSP stripping) while providing proper CSS isolation. Key changes: - Add OpenCodeEmbed React component with Shadow DOM mounting, CSS injection, and SSE directory probing for correct event routing - Add directory field to ProcessEvent::WorkerStarted to fix the race condition where worker directory was set via a separate fire-and-forget UPDATE that could lose to the INSERT - Remove log_worker_directory (superseded by directory in WorkerStarted) - Strip iframe HTML rewriting from opencode_proxy.rs (now a clean transparent reverse proxy for API/SSE traffic only) - Add build script and justfile recipe (just build-opencode-embed) to build the embed bundle from a pinned OpenCode commit - Vendor embed source files in interface/opencode-embed-src/ so the build doesn't depend on a local OpenCode checkout
Add ThemeInjector component that registers and activates a custom Spacebot theme (dark mode, purple accent) via OpenCode's runtime theme API. Defaults to dark color scheme to match the host UI. MountOpenCodeConfig now accepts optional theme and colorScheme overrides.
- Make worker entries in WebChatPanel clickable with navigation to workers tab - Render worker_run timeline items inline in the chat (previously filtered out) - Add ChatWorkerRunItem component with Open link, expand/collapse, live status overlay - Replace TaskText click-to-expand with single-line truncation and hover floating panel
Contributor
WalkthroughAdds an embeddable OpenCode SPA (build tooling, Vite config, HTML/TSX entrypoints, and build script), integrates the embed into the frontend with Shadow DOM and in-memory routing, extends worker metadata with an optional directory field, and refactors the OpenCode proxy for streaming/SSE-friendly behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| const platform: Platform = { | ||
| platform: "web", | ||
| version: "embed", | ||
| openLink: (url) => window.open(url, "_blank"), |
Contributor
There was a problem hiding this comment.
window.open(..., "_blank") without noopener can expose window.opener (tabnabbing). Consider adding noopener,noreferrer.
Suggested change
| openLink: (url) => window.open(url, "_blank"), | |
| openLink: (url) => window.open(url, "_blank", "noopener,noreferrer"), |
| // Load the manifest to find hashed asset filenames | ||
| const manifestRes = await fetch("/opencode-embed/manifest.json"); | ||
| if (!manifestRes.ok) throw new Error("Failed to load opencode-embed manifest"); | ||
| const manifest: { js: string; css: string } = await manifestRes.json(); |
Contributor
There was a problem hiding this comment.
Small hardening: since manifest.json is used to build script/CSS URLs, it’s worth validating the values are assets/... and don’t contain .. before interpolating.
Suggested change
| const manifest: { js: string; css: string } = await manifestRes.json(); | |
| const manifest: { js: string; css: string } = await manifestRes.json(); | |
| if ( | |
| !manifest.js.startsWith("assets/") || | |
| manifest.js.includes("..") || | |
| !manifest.css.startsWith("assets/") || | |
| manifest.css.includes("..") | |
| ) { | |
| throw new Error("Invalid opencode-embed manifest"); | |
| } |
| } catch { | ||
| // If SSE probe fails (aborted, timeout, etc.), fall back to | ||
| // the session API directory. | ||
| if (!controller.signal.aborted) { |
Contributor
There was a problem hiding this comment.
Potential logic bug: the SSE timeout calls controller.abort(), but the fallback is gated on !controller.signal.aborted, so timeouts won’t fall back to the session API. Even if you drop the guard, the fallback fetch uses the same aborted signal. Might be worth using a separate AbortController for the SSE probe timeout (so unmount can still cancel everything) and keep controller for the fallback request.
Contributor
There was a problem hiding this comment.
Confirmed - this is a real bug. The timeout path calls controller.abort(), so controller.signal.aborted is true when the catch runs, meaning the guard on line 792 blocks the fallback every time. Even removing the guard doesn't help since line 795 passes the same aborted signal to the fallback fetch, which will immediately reject it.
Fix: use a separate AbortController for just the SSE probe timeout, so aborting it doesn't poison the main controller used for the fallback fetch and unmount cancellation.
.github/workflows/release.yml
Outdated
|
|
||
| - name: Build OpenCode embed | ||
| run: | | ||
| cd interface && bun install && cd .. |
Contributor
There was a problem hiding this comment.
For CI reproducibility, probably want bun install --frozen-lockfile here.
Suggested change
| cd interface && bun install && cd .. | |
| cd interface && bun install --frozen-lockfile && cd .. |
scripts/build-opencode-embed.sh
Outdated
| # interface/public/opencode-embed/ for the Spacebot interface to serve. | ||
| # | ||
| # Requirements: | ||
| # - git, node (v24+), bun |
Contributor
There was a problem hiding this comment.
Nit: header says Node v24+, but the script enforces Node 22+. Might be worth aligning the comment so folks don’t over-upgrade.
Suggested change
| # - git, node (v24+), bun | |
| # - git, node (v22+), bun |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
interface/src/routes/AgentWorkers.tsx (2)
54-56:⚠️ Potential issue | 🔴 CriticalRemove the dead
workerTypeBadgeVarianthelper.It is no longer referenced after the badge cleanup, and
noUnusedLocalsis already failing Interface CI on this file.Suggested fix
-function workerTypeBadgeVariant(workerType: string) { - return workerType === "opencode" ? ("accent" as const) : ("outline" as const); -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/AgentWorkers.tsx` around lines 54 - 56, Remove the dead helper function workerTypeBadgeVariant since it's no longer referenced; locate the function declaration named workerTypeBadgeVariant and delete it, then run TypeScript build or lint to ensure no other unused locals remain and remove any now-unused imports caused by this deletion.
182-197:⚠️ Potential issue | 🔴 CriticalPopulate
directoryin the synthesized detail object.
WorkerDetailResponsenow requiresdirectory, so this branch no longer type-checks and the detail view for live-only workers fails Interface CI.Suggested fix
return { id: live.id, task: live.task, result: null, status: live.isIdle ? "idle" : "running", worker_type: live.workerType ?? "builtin", channel_id: live.channelId ?? null, channel_name: null, started_at: new Date(live.startedAt).toISOString(), completed_at: null, transcript: null, tool_calls: live.toolCalls, opencode_session_id: null, opencode_port: null, interactive: live.interactive, + directory: null, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/AgentWorkers.tsx` around lines 182 - 197, The synthesized WorkerDetailResponse returned for live-only workers is missing the required directory field; update the object construction (where the return uses id: live.id, task: live.task, etc.) to include directory populated from the live record (e.g., set directory to live.directory or a safe default such as null/empty string) so the returned object satisfies WorkerDetailResponse; ensure the property name is exactly directory and use live.directory ?? null to match existing null-handling for channel_id and other optional fields.src/conversation/history.rs (1)
848-853:⚠️ Potential issue | 🟡 MinorDon't turn a bad
directoryrow intoNone.
row.try_get("directory").ok()hides decode/type errors and makes corrupt data look like “no directory”, which will break resume/embed routing silently. Please propagate thesqlxerror instead of discarding it. As per coding guidelines:**/*.rs: Never silently discard errors; nolet _ =on Results; handle, log, or propagate errors, with the exception of.ok()on channel sends where the receiver may be dropped`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/conversation/history.rs` around lines 848 - 853, The current mapping uses row.try_get("directory").ok(), which swallows sqlx decode/type errors; change it to propagate the error instead (e.g. use row.try_get::<Option<String>, _>("directory")? or row.try_get::<String, _>("directory")? depending on the target field type) so the function returns the sqlx::Error instead of turning corrupt/invalid rows into None; replace the .ok() on the "directory" field with a try_get call that uses the ? operator and the appropriate type parameter to surface errors.
🧹 Nitpick comments (5)
interface/src/components/WebChatPanel.tsx (1)
73-79: Make the disclosure button reflect whether it can actually expand.Right now this is always a focusable button, even for rows with no
livedata and noresult, and it never exposesaria-expanded. That makes the control misleading for keyboard/AT users.Proposed fix
+ const expandable = isLive || Boolean(item.result); + return ( <div className={`rounded-lg border px-3 py-2 transition-colors ${ oc ? "border-zinc-500/20 bg-zinc-500/5 hover:bg-zinc-500/10" : "border-amber-500/20 bg-amber-500/5 hover:bg-amber-500/10" }`}> <div className="flex min-w-0 items-center gap-2"> <button type="button" - onClick={() => { - if (isLive || item.result) setExpanded(!expanded); - }} + disabled={!expandable} + aria-expanded={expandable ? expanded : undefined} + aria-controls={expandable ? `worker-run-details-${item.id}` : undefined} + onClick={() => { + if (expandable) setExpanded((value) => !value); + }} className="min-w-0 flex-1 text-left" > @@ - {(isLive || item.result) && ( + {expandable && ( <span className="flex-shrink-0 text-tiny text-ink-faint"> {expanded ? "\u25BE" : "\u25B8"} </span> )} </div> </button> @@ - {expanded && !isLive && item.result && ( - <div className={`mt-1.5 rounded-md border px-3 py-2 ${ + {expanded && !isLive && item.result && ( + <div + id={`worker-run-details-${item.id}`} + className={`mt-1.5 rounded-md border px-3 py-2 ${ oc ? "border-zinc-500/10 bg-zinc-500/5" : "border-amber-500/10 bg-amber-500/5" - }`}> + }`}> <div className="text-sm text-ink-dull"> <Markdown className="whitespace-pre-wrap break-words">{item.result}</Markdown> </div> </div> )}Also applies to: 90-94, 122-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/WebChatPanel.tsx` around lines 73 - 79, The disclosure button in WebChatPanel should indicate whether it can expand and expose its expanded state: update the button that toggles setExpanded to be disabled (or non-focusable) when neither isLive nor item.result is true, and add aria-expanded={expanded} (and aria-controls if there is an associated region id) so AT/keyboard users get the state; apply the same change to the other disclosure buttons in the file (the ones around the other occurrences currently at the other button blocks) and ensure the onClick only toggles when enabled.Dockerfile (1)
35-36: Include the Bun lockfile in this install layer.The repository contains
interface/bun.lock, but this layer only copiesinterface/package.json. Lockfile-only dependency changes will not invalidate the Docker cache, allowing the image to drift from pinned frontend dependencies. Copy the lockfile and consider using a frozen install for reproducibility:COPY interface/package.json interface/bun.lock interface/ RUN cd interface && bun install --frozen-lockfile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 35 - 36, The Docker layer currently only copies interface/package.json then runs "cd interface && bun install", which misses the existing interface/bun.lock and allows cache misses when only the lockfile changes; update the COPY step to also copy interface/bun.lock into interface/ and change the install invocation (the RUN that executes bun install) to use a frozen install (e.g., add the --frozen-lockfile flag) so lockfile-only changes invalidate the cache and dependencies are reproducibly installed.src/agent/cortex.rs (1)
2466-2483: Use.ok()instead oflet _ =for these best-effort event sends.Lines 2466 and 2475 use
let _ =to discard theResultfromdeps.event_tx.send(), which violates the coding guideline: "Never silently discard errors; nolet _ =on Results; handle, log, or propagate errors, with the exception of.ok()on channel sends where the receiver may be dropped." Replace both instances with.ok();to keep the fire-and-forget behavior while adhering to the guideline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/cortex.rs` around lines 2466 - 2483, The two fire-and-forget channel sends using `let _ = deps.event_tx.send(...)` must be changed to use `.ok()` instead of discarding the Result; locate the `deps.event_tx.send(ProcessEvent::TaskUpdated { ... })` that updates `agent_id`, `task_number`, `status`, and `action`, and the `deps.event_tx.send(ProcessEvent::WorkerStarted { ... })` that uses `worker_id`, `task: task_description`, `worker_type`, and other fields, and replace the `let _ = ...` pattern with calling `.ok()` on the send call to keep the best-effort semantics while following the guideline.src/agent/channel_dispatch.rs (1)
761-866: Minor: unnecessary.clone()ondirectory_str.The
directory_strvariable is not used after line 861, so the.clone()is unnecessary. You can simply move it into the event.Suggested diff
task: opencode_task, worker_type: "opencode".into(), interactive: true, - directory: Some(directory_str.clone()), + directory: Some(directory_str), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel_dispatch.rs` around lines 761 - 866, The review points out an unnecessary .clone() on directory_str; update the ProcessEvent::WorkerStarted call to move directory_str instead of cloning it (replace directory: Some(directory_str.clone()) with directory: Some(directory_str)) and ensure no further use of directory_str after the send (directory_str is defined above and only used in the event), removing the needless clone to avoid an extra allocation.scripts/build-opencode-embed.sh (1)
119-126: Consider using a glob instead ofls | grepfor robustness.While this is only used for diagnostic output on error, using a glob pattern is more robust and avoids shellcheck warnings.
Suggested fix
echo " JS: ${ENTRY_JS:-not found}" echo " CSS: ${ENTRY_CSS:-not found}" echo " Contents of assets/:" - ls "${OUTPUT_DIR}/assets/" | grep index-embed || echo " (none)" + (shopt -s nullglob; files=("${OUTPUT_DIR}/assets/"index-embed*); [[ ${`#files`[@]} -gt 0 ]] && printf ' %s\n' "${files[@]##*/}" || echo " (none)") exit 1 fiOr more simply, since this is diagnostic output:
- ls "${OUTPUT_DIR}/assets/" | grep index-embed || echo " (none)" + find "${OUTPUT_DIR}/assets/" -maxdepth 1 -name 'index-embed*' -printf ' %f\n' 2>/dev/null || echo " (none)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-opencode-embed.sh` around lines 119 - 126, The diagnostic block that runs when ENTRY_JS or ENTRY_CSS is missing should avoid using `ls | grep`; update the check that prints the contents of "${OUTPUT_DIR}/assets/" (used in the error path for ENTRY_JS/ENTRY_CSS) to use a glob pattern or a safe shell expansion (e.g., list matching "${OUTPUT_DIR}/assets/index-embed"*) and fallback to printing "(none)" if no matches exist; keep the same descriptive output for JS/CSS and ensure the glob approach handles spaces and missing files robustly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/src/components/WebChatPanel.tsx`:
- Around line 64-66: The component seeds expanded with useState(!!live) which
causes prop-to-state drift when live becomes available later; add a useEffect
that listens to changes in live (and possibly isOpenCodeWorker/live-related
oc/isLive) and calls setExpanded(prev => prev || !!live) so that when live
arrives the panel auto-expands but does not collapse again if the user had
manually opened it; reference the expanded/setExpanded state, the live prop, and
the isOpenCodeWorker/oc or isLive logic so the effect updates expansion only
when live transitions to truthy.
In `@interface/src/routes/AgentWorkers.tsx`:
- Around line 629-634: When the worker prop changes, the component must reset
the seeded directory state so a new session can't mount using the prior worker's
route; update the useEffect(s) that initialize directory (the state created by
useState<string | null>(initialDirectory)) in OpenCodeDirectLink and
OpenCodeEmbed to clear directory when the relevant worker identifier prop
changes (e.g., include the worker id/prop in the effect dependency array and
call setDirectory(null) at the start), also ensure eventDirectory is cleared
before starting the probe and cancel/clear any pending timeouts or
AbortController handlers so the session fallback path can run on quiet streams;
locate references to initialDirectory, setDirectory, eventDirectory, and the
existing useEffect blocks and add the reset logic and dependency updates there.
- Around line 635-648: The code currently builds OpenCode URLs with a hardcoded
origin (http://127.0.0.1:${port}) which breaks hosted/BASE_PATH/proxy
deployments; change the fetch and href constructions to use the app's BASE_PATH
+ the proxy route /api/opencode/{port} instead. Replace the fetch URL with
`${BASE_PATH}/api/opencode/${port}/session/${sessionId}` (keep controller.signal
and response handling), and build the href as
`${BASE_PATH}/api/opencode/${port}/${base64UrlEncode(directory)}/session/${sessionId}`
(or `${BASE_PATH}/api/opencode/${port}` when no directory). Apply the same
replacement for the other occurrences mentioned (around the uses that build
OpenCode embed/manifest URLs at 695-703 and 741) and ensure BASE_PATH is
imported/available in AgentWorkers.tsx.
In `@scripts/build-opencode-embed.sh`:
- Around line 8-10: Update the requirements comment in
scripts/build-opencode-embed.sh so it matches the actual runtime check: change
the "# - node (v24+)" line under the "# Requirements:" header to indicate the
minimum supported Node version (v22+) and note that v24+ is recommended/default;
ensure this text aligns with the existing Node version check logic that enforces
Node 22+.
---
Outside diff comments:
In `@interface/src/routes/AgentWorkers.tsx`:
- Around line 54-56: Remove the dead helper function workerTypeBadgeVariant
since it's no longer referenced; locate the function declaration named
workerTypeBadgeVariant and delete it, then run TypeScript build or lint to
ensure no other unused locals remain and remove any now-unused imports caused by
this deletion.
- Around line 182-197: The synthesized WorkerDetailResponse returned for
live-only workers is missing the required directory field; update the object
construction (where the return uses id: live.id, task: live.task, etc.) to
include directory populated from the live record (e.g., set directory to
live.directory or a safe default such as null/empty string) so the returned
object satisfies WorkerDetailResponse; ensure the property name is exactly
directory and use live.directory ?? null to match existing null-handling for
channel_id and other optional fields.
In `@src/conversation/history.rs`:
- Around line 848-853: The current mapping uses row.try_get("directory").ok(),
which swallows sqlx decode/type errors; change it to propagate the error instead
(e.g. use row.try_get::<Option<String>, _>("directory")? or
row.try_get::<String, _>("directory")? depending on the target field type) so
the function returns the sqlx::Error instead of turning corrupt/invalid rows
into None; replace the .ok() on the "directory" field with a try_get call that
uses the ? operator and the appropriate type parameter to surface errors.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 35-36: The Docker layer currently only copies
interface/package.json then runs "cd interface && bun install", which misses the
existing interface/bun.lock and allows cache misses when only the lockfile
changes; update the COPY step to also copy interface/bun.lock into interface/
and change the install invocation (the RUN that executes bun install) to use a
frozen install (e.g., add the --frozen-lockfile flag) so lockfile-only changes
invalidate the cache and dependencies are reproducibly installed.
In `@interface/src/components/WebChatPanel.tsx`:
- Around line 73-79: The disclosure button in WebChatPanel should indicate
whether it can expand and expose its expanded state: update the button that
toggles setExpanded to be disabled (or non-focusable) when neither isLive nor
item.result is true, and add aria-expanded={expanded} (and aria-controls if
there is an associated region id) so AT/keyboard users get the state; apply the
same change to the other disclosure buttons in the file (the ones around the
other occurrences currently at the other button blocks) and ensure the onClick
only toggles when enabled.
In `@scripts/build-opencode-embed.sh`:
- Around line 119-126: The diagnostic block that runs when ENTRY_JS or ENTRY_CSS
is missing should avoid using `ls | grep`; update the check that prints the
contents of "${OUTPUT_DIR}/assets/" (used in the error path for
ENTRY_JS/ENTRY_CSS) to use a glob pattern or a safe shell expansion (e.g., list
matching "${OUTPUT_DIR}/assets/index-embed"*) and fallback to printing "(none)"
if no matches exist; keep the same descriptive output for JS/CSS and ensure the
glob approach handles spaces and missing files robustly.
In `@src/agent/channel_dispatch.rs`:
- Around line 761-866: The review points out an unnecessary .clone() on
directory_str; update the ProcessEvent::WorkerStarted call to move directory_str
instead of cloning it (replace directory: Some(directory_str.clone()) with
directory: Some(directory_str)) and ensure no further use of directory_str after
the send (directory_str is defined above and only used in the event), removing
the needless clone to avoid an extra allocation.
In `@src/agent/cortex.rs`:
- Around line 2466-2483: The two fire-and-forget channel sends using `let _ =
deps.event_tx.send(...)` must be changed to use `.ok()` instead of discarding
the Result; locate the `deps.event_tx.send(ProcessEvent::TaskUpdated { ... })`
that updates `agent_id`, `task_number`, `status`, and `action`, and the
`deps.event_tx.send(ProcessEvent::WorkerStarted { ... })` that uses `worker_id`,
`task: task_description`, `worker_type`, and other fields, and replace the `let
_ = ...` pattern with calling `.ok()` on the send call to keep the best-effort
semantics while following the guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 000af428-7081-41d5-b4ba-0a0228ec6dd2
⛔ Files ignored due to path filters (2)
.github/workflows/release.ymlis excluded by!**/*.ymlinterface/opencode-embed-src/spacebot-theme.jsonis excluded by!**/*.json
📒 Files selected for processing (23)
.gitignoreDockerfileREADME.mddocs/content/docs/(features)/opencode.mdxdocs/content/docs/(getting-started)/quickstart.mdxinterface/opencode-embed-src/embed-entry.tsxinterface/opencode-embed-src/embed.tsxinterface/opencode-embed-src/index-embed.htmlinterface/opencode-embed-src/vite.config.embed.tsinterface/src/api/client.tsinterface/src/components/WebChatPanel.tsxinterface/src/routes/AgentWorkers.tsxinterface/vite.config.tsjustfilescripts/build-opencode-embed.shsrc/agent/channel.rssrc/agent/channel_dispatch.rssrc/agent/cortex.rssrc/api/opencode_proxy.rssrc/api/server.rssrc/api/workers.rssrc/conversation/history.rssrc/lib.rs
Comment on lines
+635
to
+648
| fetch(`http://127.0.0.1:${port}/session/${sessionId}`, { | ||
| signal: controller.signal, | ||
| }) | ||
| .then((r) => (r.ok ? r.json() : null)) | ||
| .then((session) => { | ||
| if (session?.directory) setDirectory(session.directory); | ||
| }) | ||
| .catch(() => {}); | ||
| return () => controller.abort(); | ||
| }, [port, sessionId, initialDirectory]); | ||
|
|
||
| const href = directory | ||
| ? `http://127.0.0.1:${port}/${base64UrlEncode(directory)}/session/${sessionId}` | ||
| : `http://127.0.0.1:${port}`; |
Contributor
There was a problem hiding this comment.
Build OpenCode URLs from Spacebot's proxy/base path.
This still hardcodes http://127.0.0.1:${port} and root-relative /api / /opencode-embed paths. That only works when the browser is on the same machine and the UI is mounted at /; hosted, Tailscale, HTTPS, or BASE_PATH deployments will miss the OpenCode server or manifest entirely. Please construct these URLs from BASE_PATH and the existing /api/opencode/{port} proxy instead.
Also applies to: 695-703, 741-741
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/routes/AgentWorkers.tsx` around lines 635 - 648, The code
currently builds OpenCode URLs with a hardcoded origin
(http://127.0.0.1:${port}) which breaks hosted/BASE_PATH/proxy deployments;
change the fetch and href constructions to use the app's BASE_PATH + the proxy
route /api/opencode/{port} instead. Replace the fetch URL with
`${BASE_PATH}/api/opencode/${port}/session/${sessionId}` (keep controller.signal
and response handling), and build the href as
`${BASE_PATH}/api/opencode/${port}/${base64UrlEncode(directory)}/session/${sessionId}`
(or `${BASE_PATH}/api/opencode/${port}` when no directory). Apply the same
replacement for the other occurrences mentioned (around the uses that build
OpenCode embed/manifest URLs at 695-703 and 741) and ensure BASE_PATH is
imported/available in AgentWorkers.tsx.
l33t0
suggested changes
Mar 6, 2026
- Fix SSE abort controller bug: use separate probeController so timeout doesn't prevent the fallback session fetch from running - Remove dead workerTypeBadgeVariant function - Add missing directory field in synthesized WorkerDetailResponse - Use typed extraction for directory column in history.rs - Add noopener,noreferrer to window.open in embed - Validate manifest.json asset paths before interpolation - Fix prop-to-state drift in ChatWorkerRunItem (auto-expand on live transition) - Reset directory/eventDirectory state when switching workers - Route OpenCodeDirectLink through /api/opencode proxy instead of localhost - Use bun install --frozen-lockfile in CI - Fix node version comment (v22+, not v24+)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
interface/src/routes/AgentWorkers.tsx (2)
993-1027: Tooltip is not keyboard accessible.The hover tooltip cannot be triggered via keyboard navigation (no
onFocus/onBlurhandlers) and lacks ARIA attributes for screen readers. For a task preview tooltip this is low priority, but worth noting if accessibility compliance is a goal.♻️ Optional accessibility enhancement
return ( <div ref={containerRef} className="relative min-w-0 flex-1" onMouseEnter={() => { if (isTruncated) setHovered(true); }} onMouseLeave={() => setHovered(false)} + onFocus={() => { if (isTruncated) setHovered(true); }} + onBlur={() => setHovered(false)} + tabIndex={isTruncated ? 0 : undefined} > <p ref={textRef} className="truncate text-sm font-medium text-ink-dull" + aria-describedby={hovered ? "task-tooltip" : undefined} > {text} </p> {hovered && ( - <div className="absolute left-0 top-full z-50 mt-1 ..."> + <div id="task-tooltip" role="tooltip" className="absolute left-0 top-full z-50 mt-1 ...">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/AgentWorkers.tsx` around lines 993 - 1027, The TaskText component's tooltip is only hover-activated and not keyboard or screen-reader accessible; make the container div focusable (add tabIndex={0}), add onFocus/onBlur handlers that mirror onMouseEnter/onMouseLeave to call setHovered when isTruncated is true, and ensure the tooltip element includes an id and role="tooltip" and is referenced from the container via aria-describedby (use the existing containerRef/textRef/isTruncated and setHovered state to control visibility); also ensure keyboard activation (Enter/Space) is handled if you want explicit toggling and that you clean up hover/focus state appropriately.
837-962: Mount effect remounts unnecessarily when directory is discovered.The mount effect at line 954 depends on
initialRoute, which changes wheneventDirectoryis discovered asynchronously. This triggers a full dispose-and-remount of the SolidJS app, causing a brief flash in the UI.The navigation effect at lines 956-962 is already designed to handle route changes post-mount. Consider removing
initialRoutefrom the mount effect's dependencies and letting the navigation effect handle subsequent route updates.♻️ Suggested fix
- }, [serverUrl, initialRoute]); + }, [serverUrl]); // Navigate the embedded app when the route changes useEffect(() => { - if (handleRef.current && resolvedDirectory) { + if (handleRef.current) { const route = `/${base64UrlEncode(resolvedDirectory)}/session/${sessionId}`; handleRef.current.navigate(route); } }, [resolvedDirectory, sessionId]);With this change, the mount effect only re-runs when
serverUrl(i.e., the OpenCode port) changes, and the navigation effect handles route updates whenresolvedDirectoryis discovered or changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/AgentWorkers.tsx` around lines 837 - 962, The mount useEffect currently depends on initialRoute causing unnecessary remounts when eventDirectory is discovered; remove initialRoute from the mount effect dependency array so it only depends on serverUrl, and stop reading/using initialRoute inside that mount effect (call mountOpenCode without relying on initialRoute or pass a fixed default) so the Solid app is mounted only once; rely on the existing navigation effect (which uses handleRef.current.navigate, resolvedDirectory, base64UrlEncode(resolvedDirectory) and sessionId) to update the route after directory discovery.interface/src/components/WebChatPanel.tsx (1)
287-299: Make the timeline renderer exhaustive.The
return nullfallback will silently drop any new timeline variants the API adds next. Aswitchwith aneverguard keeps this component in sync with the server contract.🧭 Suggested refactor
{timeline.map((item) => { - if (item.type === "worker_run") { - const live = liveState?.workers[item.id]; - return ( - <ChatWorkerRunItem - key={item.id} - item={item} - live={live} - agentId={agentId} - /> - ); - } - if (item.type !== "message") return null; - return ( - <div key={item.id}> - {item.role === "user" ? ( - <div className="flex justify-end"> - <div className="max-w-[85%] rounded-2xl rounded-br-md bg-accent/10 px-4 py-2.5"> - <p className="text-sm text-ink">{item.content}</p> - </div> - </div> - ) : ( - <div className="text-sm text-ink-dull"> - <Markdown>{item.content}</Markdown> - </div> - )} - </div> - ); + switch (item.type) { + case "worker_run": { + const live = liveState?.workers[item.id]; + return <ChatWorkerRunItem key={item.id} item={item} live={live} agentId={agentId} />; + } + case "message": + return ( + <div key={item.id}> + {item.role === "user" ? ( + <div className="flex justify-end"> + <div className="max-w-[85%] rounded-2xl rounded-br-md bg-accent/10 px-4 py-2.5"> + <p className="text-sm text-ink">{item.content}</p> + </div> + </div> + ) : ( + <div className="text-sm text-ink-dull"> + <Markdown>{item.content}</Markdown> + </div> + )} + </div> + ); + default: { + const _exhaustive: never = item; + return null; + } + } })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/WebChatPanel.tsx` around lines 287 - 299, The timeline renderer currently uses an if/return null fallback which will silently ignore new timeline variants; replace the conditional chain inside timeline.map (where item.type is checked and ChatWorkerRunItem is returned) with a switch on item.type and add an exhaustive never-guard (using TypeScript's never) in the default branch to cause a compile-time error for unhandled variants; update the mapping logic around timeline.map and the ChatWorkerRunItem/ message handling to use the switch and throw or assertNever on the default case so new server-side variants won't be accidentally dropped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/opencode-embed-src/embed.tsx`:
- Around line 9-13: Update the top-of-file header comment to remove the
incorrect claim that there are "NO top-level side effects" and instead state
that the module "does not auto-render" while noting that it does execute
top-level side effects such as importing global CSS (import "@/index.css") and
attaching the mountOpenCode function to the window, so that loading the bundle
won't render the UI but will still load CSS and expose mountOpenCode.
In `@interface/src/components/WebChatPanel.tsx`:
- Around line 127-129: The label for tool calls currently always uses the plural
form; update the rendering in WebChatPanel (where live.toolCalls is used) to
pluralize correctly by choosing "tool call" when live.toolCalls === 1 and "tool
calls" otherwise (e.g., via a ternary around the existing <span>{live.toolCalls}
tool calls</span>), keeping the count display and accessibility intact.
- Around line 75-106: The disclosure button currently remains focusable even
when it does nothing; in the WebChatPanel component (look for the button with
onClick that references setExpanded, and usage of isOpenCodeWorker, live,
item.result, and expanded) change it so that when neither isLive (derived from
live) nor item.result is true you render a non-interactive wrapper (e.g., a
div/span) or set the button disabled and remove its onClick, and when expandable
content exists ensure the interactive element has aria-expanded={expanded}; keep
styling consistent but only allow focus/activation when isLive || item.result is
true.
In `@scripts/build-opencode-embed.sh`:
- Around line 28-59: Add early checks in the pre-flight section (alongside the
Node checks) to verify that both git and bun are available; use command -v git
and command -v bun, print a clear "[opencode-embed] ERROR: git not found." or
"[opencode-embed] ERROR: bun not found." message with brief remediation
instructions, then exit 1 if either is missing, so the script (functions around
the NODE_MAJOR check / pre-flight block) fails fast before proceeding to
operations that assume git or bun exist.
- Around line 96-97: The script silently falls back from "bun install
--frozen-lockfile" to an unfrozen "bun install", which can bake
non-deterministic deps into release artifacts; update
scripts/build-opencode-embed.sh so that when running in CI/Docker (detect via
CI=true or CI env var) the frozen install failure is treated as fatal (exit
non-zero and print a clear error), but when running locally (CI not set)
preserve the current fallback behavior; modify the install block around
CACHE_DIR where "bun install --frozen-lockfile" is invoked to check the CI env
and branch accordingly, and ensure the error message references the failing
frozen-lockfile install so it's easy to debug.
---
Nitpick comments:
In `@interface/src/components/WebChatPanel.tsx`:
- Around line 287-299: The timeline renderer currently uses an if/return null
fallback which will silently ignore new timeline variants; replace the
conditional chain inside timeline.map (where item.type is checked and
ChatWorkerRunItem is returned) with a switch on item.type and add an exhaustive
never-guard (using TypeScript's never) in the default branch to cause a
compile-time error for unhandled variants; update the mapping logic around
timeline.map and the ChatWorkerRunItem/ message handling to use the switch and
throw or assertNever on the default case so new server-side variants won't be
accidentally dropped.
In `@interface/src/routes/AgentWorkers.tsx`:
- Around line 993-1027: The TaskText component's tooltip is only hover-activated
and not keyboard or screen-reader accessible; make the container div focusable
(add tabIndex={0}), add onFocus/onBlur handlers that mirror
onMouseEnter/onMouseLeave to call setHovered when isTruncated is true, and
ensure the tooltip element includes an id and role="tooltip" and is referenced
from the container via aria-describedby (use the existing
containerRef/textRef/isTruncated and setHovered state to control visibility);
also ensure keyboard activation (Enter/Space) is handled if you want explicit
toggling and that you clean up hover/focus state appropriately.
- Around line 837-962: The mount useEffect currently depends on initialRoute
causing unnecessary remounts when eventDirectory is discovered; remove
initialRoute from the mount effect dependency array so it only depends on
serverUrl, and stop reading/using initialRoute inside that mount effect (call
mountOpenCode without relying on initialRoute or pass a fixed default) so the
Solid app is mounted only once; rely on the existing navigation effect (which
uses handleRef.current.navigate, resolvedDirectory,
base64UrlEncode(resolvedDirectory) and sessionId) to update the route after
directory discovery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0421fbd-3fe7-4587-b70a-96bb58b1abae
⛔ Files ignored due to path filters (1)
.github/workflows/release.ymlis excluded by!**/*.yml
📒 Files selected for processing (5)
interface/opencode-embed-src/embed.tsxinterface/src/components/WebChatPanel.tsxinterface/src/routes/AgentWorkers.tsxscripts/build-opencode-embed.shsrc/conversation/history.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/conversation/history.rs
Comment on lines
+9
to
+13
| * Unlike entry.tsx, this module has NO top-level side effects — it only | ||
| * executes when `mountOpenCode` is called. | ||
| */ | ||
|
|
||
| import "@/index.css" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== embed.tsx header ==\n'
sed -n '1,18p' interface/opencode-embed-src/embed.tsx
printf '\n== embed-entry.tsx initialization ==\n'
sed -n '1,20p' interface/opencode-embed-src/embed-entry.tsxRepository: spacedriveapp/spacebot
Length of output: 1486
Reword the header comment to accurately reflect module behavior.
The module does have a top-level side effect: line 13 imports global CSS, which executes at bundle load time. The header comment (lines 9–10) claiming "NO top-level side effects" is misleading. Reword to "does not auto-render" instead, which accurately captures the intent—that loading the bundle does not render the UI, but does load CSS and attach mountOpenCode to the window object.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/opencode-embed-src/embed.tsx` around lines 9 - 13, Update the
top-of-file header comment to remove the incorrect claim that there are "NO
top-level side effects" and instead state that the module "does not auto-render"
while noting that it does execute top-level side effects such as importing
global CSS (import "@/index.css") and attaching the mountOpenCode function to
the window, so that loading the bundle won't render the UI but will still load
CSS and expose mountOpenCode.
Comment on lines
+75
to
+106
| const oc = isOpenCodeWorker(live ?? { task: item.task }); | ||
| const isLive = !!live; | ||
|
|
||
| return ( | ||
| <div className={`rounded-lg border px-3 py-2 transition-colors ${ | ||
| oc ? "border-zinc-500/20 bg-zinc-500/5 hover:bg-zinc-500/10" : "border-amber-500/20 bg-amber-500/5 hover:bg-amber-500/10" | ||
| }`}> | ||
| <div className="flex min-w-0 items-center gap-2"> | ||
| <button | ||
| type="button" | ||
| onClick={() => { | ||
| if (isLive || item.result) setExpanded(!expanded); | ||
| }} | ||
| className="min-w-0 flex-1 text-left" | ||
| > | ||
| <div className="flex min-w-0 items-center gap-2"> | ||
| <div className={`h-1.5 w-1.5 rounded-full ${ | ||
| isLive | ||
| ? `animate-pulse ${oc ? "bg-zinc-400" : "bg-amber-400"}` | ||
| : `${oc ? "bg-zinc-400/50" : "bg-amber-400/50"}` | ||
| }`} /> | ||
| <span className={`text-tiny font-medium ${oc ? "text-zinc-300" : "text-amber-300"}`}>Worker</span> | ||
| <span className={`min-w-0 flex-1 text-tiny text-ink-dull ${ | ||
| expanded ? "whitespace-normal break-words" : "truncate" | ||
| }`}>{item.task}</span> | ||
| {(isLive || item.result) && ( | ||
| <span className="flex-shrink-0 text-tiny text-ink-faint"> | ||
| {expanded ? "\u25BE" : "\u25B8"} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Don’t leave a dead disclosure button in the tab order.
Before live attaches or item.result arrives, this still renders a focusable button even though the handler intentionally no-ops. Disable it (or render a non-button wrapper) until there is expandable content, and expose the state with aria-expanded.
♿ Suggested fix
const oc = isOpenCodeWorker(live ?? { task: item.task });
const isLive = !!live;
+const canExpand = isLive || !!item.result;
return (
<div className={`rounded-lg border px-3 py-2 transition-colors ${
oc ? "border-zinc-500/20 bg-zinc-500/5 hover:bg-zinc-500/10" : "border-amber-500/20 bg-amber-500/5 hover:bg-amber-500/10"
}`}>
<div className="flex min-w-0 items-center gap-2">
<button
type="button"
onClick={() => {
- if (isLive || item.result) setExpanded(!expanded);
+ if (canExpand) setExpanded((prev) => !prev);
}}
- className="min-w-0 flex-1 text-left"
+ disabled={!canExpand}
+ aria-expanded={canExpand ? expanded : undefined}
+ className="min-w-0 flex-1 text-left disabled:cursor-default"
>
@@
- {(isLive || item.result) && (
+ {canExpand && (
<span className="flex-shrink-0 text-tiny text-ink-faint">
{expanded ? "\u25BE" : "\u25B8"}
</span>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/components/WebChatPanel.tsx` around lines 75 - 106, The
disclosure button currently remains focusable even when it does nothing; in the
WebChatPanel component (look for the button with onClick that references
setExpanded, and usage of isOpenCodeWorker, live, item.result, and expanded)
change it so that when neither isLive (derived from live) nor item.result is
true you render a non-interactive wrapper (e.g., a div/span) or set the button
disabled and remove its onClick, and when expandable content exists ensure the
interactive element has aria-expanded={expanded}; keep styling consistent but
only allow focus/activation when isLive || item.result is true.
Comment on lines
+127
to
+129
| {live.toolCalls > 0 && ( | ||
| <span>{live.toolCalls} tool calls</span> | ||
| )} |
Contributor
There was a problem hiding this comment.
Pluralize the tool-call label.
live.toolCalls === 1 currently renders as 1 tool calls.
✏️ Suggested fix
{live.toolCalls > 0 && (
- <span>{live.toolCalls} tool calls</span>
+ <span>
+ {live.toolCalls} tool call{live.toolCalls === 1 ? "" : "s"}
+ </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
| {live.toolCalls > 0 && ( | |
| <span>{live.toolCalls} tool calls</span> | |
| )} | |
| {live.toolCalls > 0 && ( | |
| <span> | |
| {live.toolCalls} tool call{live.toolCalls === 1 ? "" : "s"} | |
| </span> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/components/WebChatPanel.tsx` around lines 127 - 129, The label
for tool calls currently always uses the plural form; update the rendering in
WebChatPanel (where live.toolCalls is used) to pluralize correctly by choosing
"tool call" when live.toolCalls === 1 and "tool calls" otherwise (e.g., via a
ternary around the existing <span>{live.toolCalls} tool calls</span>), keeping
the count display and accessibility intact.
Comment on lines
+28
to
+59
| # --------------------------------------------------------------------------- | ||
| # 0. Pre-flight: verify Node 22+ is available | ||
| # --------------------------------------------------------------------------- | ||
| # Try fnm first (non-interactive shells don't source shell init files) | ||
| if command -v fnm &>/dev/null; then | ||
| eval "$(fnm env)" 2>/dev/null || true | ||
| fnm use v24.14.0 2>/dev/null || true | ||
| fi | ||
|
|
||
| if ! command -v node &>/dev/null; then | ||
| echo "[opencode-embed] ERROR: node not found." | ||
| echo "" | ||
| echo " This build requires Node 22+. Install via fnm:" | ||
| echo " curl -fsSL https://fnm.vercel.app/install | bash" | ||
| echo " fnm install v24.14.0" | ||
| echo "" | ||
| echo " Then re-run:" | ||
| echo " eval \"\$(fnm env)\" && fnm use v24.14.0 && ./scripts/build-opencode-embed.sh" | ||
| exit 1 | ||
| fi | ||
|
|
||
| NODE_MAJOR="$(node -v | sed 's/^v//' | cut -d. -f1)" | ||
| if [ "${NODE_MAJOR}" -lt 22 ]; then | ||
| echo "[opencode-embed] ERROR: Node 22+ required (got $(node -v))." | ||
| echo "" | ||
| echo " If you use fnm:" | ||
| echo " fnm install v24.14.0 && fnm use v24.14.0" | ||
| echo "" | ||
| echo " Then re-run:" | ||
| echo " eval \"\$(fnm env)\" && fnm use v24.14.0 && ./scripts/build-opencode-embed.sh" | ||
| exit 1 | ||
| fi |
Contributor
There was a problem hiding this comment.
Fail fast if git or bun is unavailable.
This preflight only validates Node, but the script later invokes git and bun unconditionally. In a fresh CI/dev environment that turns into a late command not found after work has already started.
Suggested fix
# ---------------------------------------------------------------------------
-# 0. Pre-flight: verify Node 22+ is available
+# 0. Pre-flight: verify required tools are available
# ---------------------------------------------------------------------------
+# Required commands beyond Node itself.
+for tool in git bun; do
+ if ! command -v "${tool}" &>/dev/null; then
+ echo "[opencode-embed] ERROR: ${tool} not found."
+ exit 1
+ fi
+done
+
# Try fnm first (non-interactive shells don't source shell init files)
if command -v fnm &>/dev/null; then
eval "$(fnm env)" 2>/dev/null || true📝 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
| # --------------------------------------------------------------------------- | |
| # 0. Pre-flight: verify Node 22+ is available | |
| # --------------------------------------------------------------------------- | |
| # Try fnm first (non-interactive shells don't source shell init files) | |
| if command -v fnm &>/dev/null; then | |
| eval "$(fnm env)" 2>/dev/null || true | |
| fnm use v24.14.0 2>/dev/null || true | |
| fi | |
| if ! command -v node &>/dev/null; then | |
| echo "[opencode-embed] ERROR: node not found." | |
| echo "" | |
| echo " This build requires Node 22+. Install via fnm:" | |
| echo " curl -fsSL https://fnm.vercel.app/install | bash" | |
| echo " fnm install v24.14.0" | |
| echo "" | |
| echo " Then re-run:" | |
| echo " eval \"\$(fnm env)\" && fnm use v24.14.0 && ./scripts/build-opencode-embed.sh" | |
| exit 1 | |
| fi | |
| NODE_MAJOR="$(node -v | sed 's/^v//' | cut -d. -f1)" | |
| if [ "${NODE_MAJOR}" -lt 22 ]; then | |
| echo "[opencode-embed] ERROR: Node 22+ required (got $(node -v))." | |
| echo "" | |
| echo " If you use fnm:" | |
| echo " fnm install v24.14.0 && fnm use v24.14.0" | |
| echo "" | |
| echo " Then re-run:" | |
| echo " eval \"\$(fnm env)\" && fnm use v24.14.0 && ./scripts/build-opencode-embed.sh" | |
| exit 1 | |
| fi | |
| # --------------------------------------------------------------------------- | |
| # 0. Pre-flight: verify required tools are available | |
| # --------------------------------------------------------------------------- | |
| # Required commands beyond Node itself. | |
| for tool in git bun; do | |
| if ! command -v "${tool}" &>/dev/null; then | |
| echo "[opencode-embed] ERROR: ${tool} not found." | |
| exit 1 | |
| fi | |
| done | |
| # Try fnm first (non-interactive shells don't source shell init files) | |
| if command -v fnm &>/dev/null; then | |
| eval "$(fnm env)" 2>/dev/null || true | |
| fnm use v24.14.0 2>/dev/null || true | |
| fi | |
| if ! command -v node &>/dev/null; then | |
| echo "[opencode-embed] ERROR: node not found." | |
| echo "" | |
| echo " This build requires Node 22+. Install via fnm:" | |
| echo " curl -fsSL https://fnm.vercel.app/install | bash" | |
| echo " fnm install v24.14.0" | |
| echo "" | |
| echo " Then re-run:" | |
| echo " eval \"\$(fnm env)\" && fnm use v24.14.0 && ./scripts/build-opencode-embed.sh" | |
| exit 1 | |
| fi | |
| NODE_MAJOR="$(node -v | sed 's/^v//' | cut -d. -f1)" | |
| if [ "${NODE_MAJOR}" -lt 22 ]; then | |
| echo "[opencode-embed] ERROR: Node 22+ required (got $(node -v))." | |
| echo "" | |
| echo " If you use fnm:" | |
| echo " fnm install v24.14.0 && fnm use v24.14.0" | |
| echo "" | |
| echo " Then re-run:" | |
| echo " eval \"\$(fnm env)\" && fnm use v24.14.0 && ./scripts/build-opencode-embed.sh" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build-opencode-embed.sh` around lines 28 - 59, Add early checks in
the pre-flight section (alongside the Node checks) to verify that both git and
bun are available; use command -v git and command -v bun, print a clear
"[opencode-embed] ERROR: git not found." or "[opencode-embed] ERROR: bun not
found." message with brief remediation instructions, then exit 1 if either is
missing, so the script (functions around the NODE_MAJOR check / pre-flight
block) fails fast before proceeding to operations that assume git or bun exist.
The build script silently fell back to unfrozen bun install when --frozen-lockfile failed, masking lockfile drift in CI. Now CI=true makes it exit non-zero with a clear error message while local dev keeps the fallback behavior.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
scripts/build-opencode-embed.sh (2)
29-61:⚠️ Potential issue | 🟡 MinorFail fast on missing
gitandbun.Line 29 calls this pre-flight, but only
nodeis checked here.gitis first used on Lines 71-79 andbunon Lines 99-106, so a clean machine still fails late after the script has already started work.Suggested fix
# --------------------------------------------------------------------------- -# 0. Pre-flight: verify Node 22+ is available +# 0. Pre-flight: verify required tools are available # --------------------------------------------------------------------------- +for tool in git bun; do + if ! command -v "${tool}" &>/dev/null; then + echo "[opencode-embed] ERROR: ${tool} not found." + echo " Install ${tool} and re-run ./scripts/build-opencode-embed.sh" + exit 1 + fi +done + # Try fnm first (non-interactive shells don't source shell init files)Verify that the pre-flight block lacks
git/bunchecks while later sections still invoke both commands:#!/bin/bash sed -n '28,61p' scripts/build-opencode-embed.sh printf '\n== later git/bun invocations ==\n' rg -n '\b(git|bun)\s' scripts/build-opencode-embed.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-opencode-embed.sh` around lines 29 - 61, Pre-flight currently only checks for node (NODE_MAJOR) and misses verifying required tools git and bun, causing late failures; add the same fail-fast checks used for node to the pre-flight block: test command -v git and command -v bun, emit clear "[opencode-embed] ERROR: git not found." / "[opencode-embed] ERROR: bun not found." messages with brief install hints and exit 1 when absent, matching the style used around NODE_MAJOR and the existing node error handling so later invocations of git and bun (the bare git and bun commands) never run on a clean machine.
97-107:⚠️ Potential issue | 🟠 Major
CI=truealone does not protect the Docker release path.
Dockerfile:43runs this script as./scripts/build-opencode-embed.shwithout exportingCI, so the check on Line 97 is false there and the fallback on Line 106 stays reachable. That still allows a drifted upstream lockfile to change release image contents.Suggested fix
-if [ "${CI:-}" = "true" ]; then +if [ -n "${CI:-}" ] || [ "${OPENCODE_STRICT_LOCKFILE:-}" = "1" ]; thenPaired caller change outside this range:
# Dockerfile -RUN ./scripts/build-opencode-embed.sh +RUN OPENCODE_STRICT_LOCKFILE=1 ./scripts/build-opencode-embed.shVerify that the strict branch is currently unreachable from the Docker build path:
#!/bin/bash echo '== strict-lockfile gate ==' sed -n '96,107p' scripts/build-opencode-embed.sh echo echo '== callers ==' rg -n -C2 'build-opencode-embed\.sh|CI=|OPENCODE_STRICT_LOCKFILE=' Dockerfile .github/workflows/release.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-opencode-embed.sh` around lines 97 - 107, The strict lockfile branch is gated by CI but Docker builds don't export CI, so update the script's guard to use a dedicated variable and ensure callers set it; specifically, replace the conditional that checks "${CI:-}" with a check for a new explicit flag such as "${OPENCODE_STRICT_LOCKFILE:-}" (e.g. if [ "${OPENCODE_STRICT_LOCKFILE:-}" = "true" ]), and then update the Docker build invocation (and CI workflow calls) to pass OPENCODE_STRICT_LOCKFILE=true so the strict path in build-opencode-embed.sh (the CACHE_DIR bun install --frozen-lockfile branch) is reached during image builds. Ensure any fallback branch remains unchanged for local runs when the flag is not set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/build-opencode-embed.sh`:
- Around line 29-61: Pre-flight currently only checks for node (NODE_MAJOR) and
misses verifying required tools git and bun, causing late failures; add the same
fail-fast checks used for node to the pre-flight block: test command -v git and
command -v bun, emit clear "[opencode-embed] ERROR: git not found." /
"[opencode-embed] ERROR: bun not found." messages with brief install hints and
exit 1 when absent, matching the style used around NODE_MAJOR and the existing
node error handling so later invocations of git and bun (the bare git and bun
commands) never run on a clean machine.
- Around line 97-107: The strict lockfile branch is gated by CI but Docker
builds don't export CI, so update the script's guard to use a dedicated variable
and ensure callers set it; specifically, replace the conditional that checks
"${CI:-}" with a check for a new explicit flag such as
"${OPENCODE_STRICT_LOCKFILE:-}" (e.g. if [ "${OPENCODE_STRICT_LOCKFILE:-}" =
"true" ]), and then update the Docker build invocation (and CI workflow calls)
to pass OPENCODE_STRICT_LOCKFILE=true so the strict path in
build-opencode-embed.sh (the CACHE_DIR bun install --frozen-lockfile branch) is
reached during image builds. Ensure any fallback branch remains unchanged for
local runs when the flag is not set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6697a3c-4e83-4aba-b3b3-c77d4783b49c
📒 Files selected for processing (1)
scripts/build-opencode-embed.sh
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
2 participants
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.
Screen.Recording.2026-03-06.at.4.39.18.AM-trimmed-slimmed.mov
Summary
OpenCode Embed (Shadow DOM)
The worker detail view now embeds the OpenCode SPA directly via Shadow DOM instead of an iframe. This gives us:
@layerso Kobalte portals (dropdowns, toasts) that escape the Shadow DOM still render correctly without overriding Spacebot stylesBuild infrastructure:
scripts/build-opencode-embed.sh— clones the OpenCode repo at a pinned commit, builds the embed entry, generatesmanifest.jsonwith hashed filenamescargo build/opencode-embed/assets in dev modeWorker detail header cleanup
Portal Chat Workers
WebChatPanel.tsxhad two problems: theActiveWorkersPanelsticky header showed workers as static non-interactive badges, and the timeline rendering filtered out allworker_runitems entirely (if (item.type !== "message") return null).Active workers panel — entries are now
<Link>elements that navigate to/agents/$agentId/workers?worker=<id>. Added pulsing status dot and hover background transition.Timeline worker runs — new
ChatWorkerRunItemcomponent rendersworker_runitems inline in the conversation flow:LiveDuration, status, current tool, tool call countWorker Detail Hover Tooltip
TaskTextinAgentWorkers.tsxpreviously showed 3 lines of the task prompt with click-to-expand. Now:max-h-60, styled with backdrop blur and shadowFiles Changed
interface/src/components/WebChatPanel.tsx— clickable workers, inline timeline worker entriesinterface/src/routes/AgentWorkers.tsx— hover tooltip TaskText, OpenCode Shadow DOM embed, header cleanupinterface/opencode-embed-src/— embed entry, SPA wrapper, theme, Vite configscripts/build-opencode-embed.sh— build script for OpenCode embed assets.github/workflows/release.yml— CI build step for embedDockerfile— embed build in containerGates
Note
Comprehensive implementation of Shadow DOM-based OpenCode embed with improved worker interactivity. All delivery gates passed (preflight, formatting, compilation, 428 lib tests, integration compilation). Ready for review.
Written by Tembo for commit 2195ff8. This will update automatically on new commits.