Skip to content

feat: Multi-Workspace Productivity Enhancements#8417

Open
haroldfabla2-hue wants to merge 2 commits into
stablyai:mainfrom
haroldfabla2-hue:feat-productivity-enhancements
Open

feat: Multi-Workspace Productivity Enhancements#8417
haroldfabla2-hue wants to merge 2 commits into
stablyai:mainfrom
haroldfabla2-hue:feat-productivity-enhancements

Conversation

@haroldfabla2-hue

Copy link
Copy Markdown

This PR introduces: 1. Workspace startup scripts (.orca-startup.json) 2. AI Error Explainer for failing commands 3. Global Terminal Search (Ctrl+Shift+F)

@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Added workspace startup command loading from .orca-startup.json, global terminal search with keyboard activation and pane focusing, and command-failure handling that displays terminal error toasts. PTY exit callbacks now carry exit codes and captured logs. Terminal panes are registered for global searches and unregistered on close. Resource usage session counts now account for closed-session offsets during refresh and optimistic kill operations.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is far too brief and omits required sections like Summary, Screenshots, Testing, AI Review, Security Audit, and Notes. Expand it to the template with Summary, Screenshots/no-visual-change note, Testing checklist, AI Review Report with macOS/Linux/Windows coverage, Security Audit, and Notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is broad, but it clearly matches the PR’s main productivity enhancements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat-productivity-enhancements

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (6)
src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx (2)

1097-1103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid calling setClosedSessionCountOffset inside the setSessions updater.

React state updater functions should be pure. In Strict Mode, React double-invokes updaters to surface impurities — the nested setClosedSessionCountOffset call fires twice per update. It's idempotent here so no bug results, but it violates the purity contract and may cause confusion in future maintenance.

Consider computing the next sessions array and offset outside the updater, using a sessionsRef (mirroring the bindingsRef pattern) to read the current sessions without a stale closure:

♻️ Proposed refactor
+ const sessionsRef = useRef(sessions)
+ sessionsRef.current = sessions
+
+ const recomputeClosedOffset = useCallback((nextSessionCount: number): void => {
+   const currentBindings = buildResourceSessionBindingIndex(bindingsRef.current).boundPtyIds.size
+   setClosedSessionCountOffset(nextSessionCount - currentBindings)
+ }, [])

Then in each kill handler, replace the nested-setState pattern:

- setSessions((prev) => {
-   const next = prev.filter((s) => s.id !== session.sessionId)
-   const currentBindings = buildResourceSessionBindingIndex(bindingsRef.current).boundPtyIds
-     .size
-   setClosedSessionCountOffset(next.length - currentBindings)
-   return next
- })
+ const next = sessionsRef.current.filter((s) => s.id !== session.sessionId)
+ sessionsRef.current = next
+ setSessions(next)
+ recomputeClosedOffset(next.length)

Apply the same transformation to handleKillOrphans and runKillConfirmed.

Also applies to: 1134-1139, 1153-1158


828-829: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the duplicated offset computation into a helper.

The pattern buildResourceSessionBindingIndex(bindingsRef.current).boundPtyIds.size followed by setClosedSessionCountOffset(count - bindings) is repeated in four locations (refreshSessions, handleKillSession, handleKillOrphans, runKillConfirmed). Extracting a recomputeClosedOffset callback eliminates duplication and ensures the formula stays consistent if the binding contract changes.

♻️ Proposed helper
+ const recomputeClosedOffset = useCallback((sessionCount: number): void => {
+   const currentBindings = buildResourceSessionBindingIndex(bindingsRef.current).boundPtyIds.size
+   setClosedSessionCountOffset(sessionCount - currentBindings)
+ }, [])

Then each call site simplifies to:

- const currentBindings = buildResourceSessionBindingIndex(bindingsRef.current).boundPtyIds.size
- setClosedSessionCountOffset(result.length - currentBindings)
+ recomputeClosedOffset(result.length)

Also applies to: 1099-1101, 1136-1137, 1155-1156

src/renderer/src/components/terminal-pane/terminal-registry.ts (1)

35-73: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

Cap search results and consider caching buffer lines.

searchAllTerminals calls getBufferLines() for every registered pane on every invocation, each call iterating the entire xterm scrollback buffer and allocating a new string array. With multiple terminals and large scrollback (10k+ rows), this blocks the main thread and creates significant GC pressure. Additionally, there is no result limit — a short query could return thousands of matches, causing the modal to render thousands of DOM nodes.

Consider adding a result cap (e.g., 200–500 matches) and either caching buffer lines with a dirty flag or limiting search to the visible viewport plus a reasonable scrollback window.

♻️ Proposed result cap
 export function searchAllTerminals(
   query: string,
-  caseSensitive: boolean = false
+  caseSensitive: boolean = false,
+  maxResults = 500
 ): TerminalSearchMatch[] {
   if (!query) {
     return []
   }
   const results: TerminalSearchMatch[] = []
   const lowerQuery = caseSensitive ? query : query.toLowerCase()

   for (const reg of registry.values()) {
+    if (results.length >= maxResults) {
+      break
+    }
     const lines = reg.getBufferLines()
     for (let i = 0; i < lines.length; i++) {
+      if (results.length >= maxResults) {
+        break
+      }
       const line = lines[i]
       if (!line) {
         continue
       }
src/renderer/src/components/GlobalTerminalSearchModal.tsx (3)

78-81: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Set shouldFilter={false} on <Command> to prevent double-filtering.

cmdk's <Command> defaults to shouldFilter={true}, meaning it filters Command.Items by the input value. Since searchAllTerminals already pre-filters results, cmdk's built-in filter is redundant. In edge cases with very long line text, cmdk's filter could reject items that searchAllTerminals already matched.

♻️ Proposed fix
       <Command
         className="w-full max-w-2xl bg-popover text-popover-foreground rounded-xl shadow-2xl border flex flex-col overflow-hidden"
+        shouldFilter={false}
         loop
       >

1-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Statically import getTerminalPaneRegistration instead of dynamic import.

searchAllTerminals is already statically imported from ./terminal-pane/terminal-registry on line 4. The dynamic import() on line 55 for getTerminalPaneRegistration from the same module is unnecessary — it adds async indirection for no benefit. As per coding guidelines for src/renderer/src/**/*.{tsx,ts,css}, UI work should use shadcn primitives from src/renderer/src/components/ui/ — verify whether a shadcn Command wrapper exists and should be used instead of importing cmdk directly.

♻️ Proposed fix
-import { searchAllTerminals, type TerminalSearchMatch } from './terminal-pane/terminal-registry'
+import {
+  searchAllTerminals,
+  getTerminalPaneRegistration,
+  type TerminalSearchMatch
+} from './terminal-pane/terminal-registry'

Then replace the dynamic import in handleSelect:

-      import('./terminal-pane/terminal-registry').then(({ getTerminalPaneRegistration }) => {
-        const reg = getTerminalPaneRegistration(match.paneId)
-        if (reg) {
-          // Wait a tick for the tab to render and the PaneManager to be available
-          setTimeout(() => reg.focus(), 100)
-        }
-      })
+      const reg = getTerminalPaneRegistration(match.paneId)
+      if (reg) {
+        setTimeout(() => reg.focus(), 100)
+      }

Also applies to: 55-55

Source: Coding guidelines


112-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use a design token instead of raw bg-yellow-500/50 for the highlight.

As per coding guidelines for src/renderer/src/**/*.{tsx,ts,css}, all UI work must use tokens from src/renderer/src/assets/main.css. The bg-yellow-500/50 class is a raw Tailwind color, not a semantic design token. Define a search-highlight token in main.css and use it here.

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: aaa31fc7-8e74-4513-ba66-d71b5fa81478

📥 Commits

Reviewing files that changed from the base of the PR and between 7b12d38 and c97d18a.

📒 Files selected for processing (11)
  • .orca-startup.json
  • src/renderer/src/App.tsx
  • src/renderer/src/components/GlobalTerminalSearchModal.tsx
  • src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx
  • src/renderer/src/components/terminal-pane/TerminalCommandFailedToast.tsx
  • src/renderer/src/components/terminal-pane/TerminalPane.tsx
  • src/renderer/src/components/terminal-pane/pty-connection-types.ts
  • src/renderer/src/components/terminal-pane/pty-connection.ts
  • src/renderer/src/components/terminal-pane/terminal-registry.ts
  • src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts
  • src/renderer/src/hooks/useWorkspaceStartup.ts

Comment thread src/renderer/src/App.tsx
Comment on lines +1911 to +1918
// Mac uses Cmd (metaKey), others use Ctrl (ctrlKey)
const modifier = navigator.userAgent.includes('Mac') ? e.metaKey : e.ctrlKey
if (e.key.toLowerCase() === 'f' && e.shiftKey && modifier) {
e.preventDefault()
setShowGlobalTerminalSearch(true)
return
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Cmd/Ctrl+Shift+F intercept shadows existing sidebar.search.toggle shortcut and bypasses the keybinding system.

The new early-return at the top of onKeyDown fires before dispatchShortcutInput, making three existing features unreachable for the default keybinding:

  1. Find in Folder (lines 1655–1679) — seeding sidebar search with the selected explorer folder.
  2. Search with selected text (lines 1672–1678) — opening sidebar search with selected text.
  3. Sidebar search toggle (lines 1867–1873) — toggling the right sidebar search tab.

Additionally, the hardcoded key check bypasses matchShortcut() and the keybinding infrastructure, meaning the shortcut cannot be remapped, doesn't respect terminalShortcutPolicy, doesn't call notifyTerminalCapture, and ignores the data-shortcut-recorder-active guard (lines 1620–1625) and the [data-terminal-search-root] coordination check (line 1881).

Consider routing through matchShortcut or a dedicated keybinding action id so the shortcut is configurable and can coexist with or contextually defer to the existing sidebar search features.

💡 Suggested approach

Register a new keybinding action (e.g. terminal.globalSearch) in the keybindings catalog, then handle it inside dispatchShortcutInput alongside the existing sidebar.search.toggle logic — or gate the new intercept so it only fires when no folder is selected, no text is selected, and no terminal search is open:

     const onKeyDown = (e: KeyboardEvent): void => {
-      // Mac uses Cmd (metaKey), others use Ctrl (ctrlKey)
-      const modifier = navigator.userAgent.includes('Mac') ? e.metaKey : e.ctrlKey
-      if (e.key.toLowerCase() === 'f' && e.shiftKey && modifier) {
-        e.preventDefault()
-        setShowGlobalTerminalSearch(true)
-        return
-      }
-
       const detected = doubleTapDetector.process(

Then inside dispatchShortcutInput, add the global terminal search as a contextual handler that coexists with the existing sidebar.search.toggle flow.

Comment on lines +48 to +65
const handleSelect = useCallback(
(match: TerminalSearchMatch) => {
// 1. Focus the workspace
useAppStore.getState().setActiveWorktreeId(match.worktreeId)
// 2. Focus the tab
useAppStore.getState().setActiveTabId(match.tabId)
// 3. Focus the pane
import('./terminal-pane/terminal-registry').then(({ getTerminalPaneRegistration }) => {
const reg = getTerminalPaneRegistration(match.paneId)
if (reg) {
// Wait a tick for the tab to render and the PaneManager to be available
setTimeout(() => reg.focus(), 100)
}
})
onClose()
},
[onClose]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Hardcoded 100ms timeout for pane focus is fragile.

After switching worktree and tab, the 100ms setTimeout may not be enough for the TerminalPane to mount and register the pane via registerTerminalPane. If registration hasn't happened yet, getTerminalPaneRegistration returns undefined and focus silently fails with no user feedback. Consider a retry loop or waiting for a registration event.

🛡️ Proposed retry-based focus
       import('./terminal-pane/terminal-registry').then(({ getTerminalPaneRegistration }) => {
         const reg = getTerminalPaneRegistration(match.paneId)
         if (reg) {
-          // Wait a tick for the tab to render and the PaneManager to be available
-          setTimeout(() => reg.focus(), 100)
+          reg.focus()
+        } else {
+          // Pane may not be registered yet if the worktree/tab is mounting.
+          // Retry a few times before giving up.
+          let attempts = 0
+          const tryFocus = (): void => {
+            const r = getTerminalPaneRegistration(match.paneId)
+            if (r) {
+              r.focus()
+            } else if (attempts < 5) {
+              attempts++
+              setTimeout(tryFocus, 100)
+            }
+          }
+          setTimeout(tryFocus, 100)
         }
       })
📝 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
const handleSelect = useCallback(
(match: TerminalSearchMatch) => {
// 1. Focus the workspace
useAppStore.getState().setActiveWorktreeId(match.worktreeId)
// 2. Focus the tab
useAppStore.getState().setActiveTabId(match.tabId)
// 3. Focus the pane
import('./terminal-pane/terminal-registry').then(({ getTerminalPaneRegistration }) => {
const reg = getTerminalPaneRegistration(match.paneId)
if (reg) {
// Wait a tick for the tab to render and the PaneManager to be available
setTimeout(() => reg.focus(), 100)
}
})
onClose()
},
[onClose]
)
const handleSelect = useCallback(
(match: TerminalSearchMatch) => {
// 1. Focus the workspace
useAppStore.getState().setActiveWorktreeId(match.worktreeId)
// 2. Focus the tab
useAppStore.getState().setActiveTabId(match.tabId)
// 3. Focus the pane
import('./terminal-pane/terminal-registry').then(({ getTerminalPaneRegistration }) => {
const reg = getTerminalPaneRegistration(match.paneId)
if (reg) {
reg.focus()
} else {
// Pane may not be registered yet if the worktree/tab is mounting.
// Retry a few times before giving up.
let attempts = 0
const tryFocus = (): void => {
const r = getTerminalPaneRegistration(match.paneId)
if (r) {
r.focus()
} else if (attempts < 5) {
attempts++
setTimeout(tryFocus, 100)
}
}
setTimeout(tryFocus, 100)
}
})
onClose()
},
[onClose]
)

Comment on lines +91 to +93
<button onClick={onClose} className="p-1 hover:bg-muted rounded">
<X className="h-4 w-4 opacity-50" />
</button>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add aria-label to the close button.

The close button contains only an <X> icon with no accessible label, making it invisible to screen reader users.

♿ Proposed fix
-          <button onClick={onClose} className="p-1 hover:bg-muted rounded">
+          <button
+            onClick={onClose}
+            aria-label="Close search"
+            className="p-1 hover:bg-muted rounded"
+          >
📝 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
<button onClick={onClose} className="p-1 hover:bg-muted rounded">
<X className="h-4 w-4 opacity-50" />
</button>
<button
onClick={onClose}
aria-label="Close search"
className="p-1 hover:bg-muted rounded"
>
<X className="h-4 w-4 opacity-50" />
</button>

Comment on lines +46 to +50
restoredViewportBlankingPanesRef: React.RefObject<Set<number>>
isActiveRef: React.RefObject<boolean>
isVisibleRef: React.RefObject<boolean>
onPtyExitRef: React.RefObject<(ptyId: string) => void>
onPtyExitRef: React.RefObject<(ptyId: string, exitCode?: number) => void>
onCommandFailedRef?: React.RefObject<(exitCode: number, logs: string) => void>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Thread the required viewport ref into the manual restart path.

restoredViewportBlankingPanesRef is now required, but TerminalPane.tsx Line 1713’s direct connectPanePty(...) call omits it. This fails type-checking. Add the same ref used by the lifecycle-created connections.

Comment on lines +2321 to +2345
if (
exitCode != null &&
exitCode !== 0 &&
!isSuppressedExit &&
deps.onCommandFailedRef?.current
) {
// Collect last 50 lines for AI context
const lines: string[] = []
const activeBuffer = pane.terminal.buffer.active
const length = activeBuffer.length
const start = Math.max(0, length - 50)
for (let i = start; i < length; i++) {
const lineStr = activeBuffer.getLine(i)?.translateToString(true) ?? ''
if (lineStr.trim() !== '' || lines.length > 0) {
// skip leading empty lines
lines.push(lineStr)
}
}
deps.onCommandFailedRef.current(exitCode, lines.join('\n'))
// If the command failed, we want to keep the pane mounted so the user can see the error and Ask AI overlay.
return
}

deps.onPtyExitRef.current(ptyId, exitCode)
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Report non-zero command completions, not only PTY exits.

This path misses normal failures such as npm test: the shell stays alive, so only handleCommandFinished receives its best-effort exit code. It also misses fresh startup failures because Line 2317 returns before this block. Invoke a shared failure reporter from command completion and before the fresh-spawn retention return.

isActiveRef,
isVisibleRef,
onPtyExitRef,
onCommandFailedRef,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the failure callback on manually restarted Codex panes.

handleRestartCodexPane directly calls connectPanePty at Line 1713 but does not pass onCommandFailedRef; failures after a Codex restart will not show the explainer toast.

Proposed fix
         isVisibleRef,
         onPtyExitRef,
+        onCommandFailedRef,
         onPtyErrorRef,
📝 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
onCommandFailedRef,
isVisibleRef,
onPtyExitRef,
onCommandFailedRef,
onPtyErrorRef,

Comment on lines +3162 to +3167
<CloseTerminalDialog
open={pendingCloseConfirmation !== null}
copyKind={pendingCloseConfirmation?.copyKind}
onCancel={handleCancelClose}
onConfirm={handleConfirmClose}
/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Render only one close-confirmation dialog.

Another identical CloseTerminalDialog remains at Lines 3250-3255. Opening confirmation mounts two dialogs against the same state, causing duplicate modal/focus behavior. Remove either instance.

Comment on lines +3168 to +3175
{failedCommandContext && (
<TerminalCommandFailedToast
worktreeId={worktreeId}
exitCode={failedCommandContext.exitCode}
logs={failedCommandContext.logs}
onDismiss={() => setFailedCommandContext(null)}
/>
)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not show a background terminal’s failure toast.

Unlike TerminalErrorToast at Line 3030, this toast is not gated by isActive. Hidden/inactive terminal panes can therefore overlay the currently active workspace.

Proposed fix
-      {failedCommandContext && (
+      {isActive && failedCommandContext && (
📝 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
{failedCommandContext && (
<TerminalCommandFailedToast
worktreeId={worktreeId}
exitCode={failedCommandContext.exitCode}
logs={failedCommandContext.logs}
onDismiss={() => setFailedCommandContext(null)}
/>
)}
{isActive && failedCommandContext && (
<TerminalCommandFailedToast
worktreeId={worktreeId}
exitCode={failedCommandContext.exitCode}
logs={failedCommandContext.logs}
onDismiss={() => setFailedCommandContext(null)}
/>
)}

Comment on lines +807 to +824
registerTerminalPane(pane.id, {
paneId: pane.id,
worktreeId,
tabId,
getTitle: () => paneTitlesRef.current.get(pane.id) || 'Terminal',
getBufferLines: () => {
const lines: string[] = []
const activeBuffer = pane.terminal.buffer.active
const length = activeBuffer.length
for (let i = 0; i < length; i++) {
lines.push(activeBuffer.getLine(i)?.translateToString(true) ?? '')
}
return lines
},
focus: () => {
managerRef.current?.setActivePane(pane.id, { focus: true })
}
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix paneTitlesRef.current.get() — Record does not have a .get() method.

paneTitlesRef is typed as React.RefObject<Record<number, string>> (line 287). A Record<number, string> is a plain object — it has no .get() method. Every other usage in this file treats it as a plain object (in operator, spread, delete at lines 1331–1345). At runtime, paneTitlesRef.current.get is undefined, so calling it throws TypeError, which would crash searchAllTerminals when it invokes reg.getTitle().

🐛 Proposed fix
-          getTitle: () => paneTitlesRef.current.get(pane.id) || 'Terminal',
+          getTitle: () => paneTitlesRef.current[pane.id] || 'Terminal',
📝 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
registerTerminalPane(pane.id, {
paneId: pane.id,
worktreeId,
tabId,
getTitle: () => paneTitlesRef.current.get(pane.id) || 'Terminal',
getBufferLines: () => {
const lines: string[] = []
const activeBuffer = pane.terminal.buffer.active
const length = activeBuffer.length
for (let i = 0; i < length; i++) {
lines.push(activeBuffer.getLine(i)?.translateToString(true) ?? '')
}
return lines
},
focus: () => {
managerRef.current?.setActivePane(pane.id, { focus: true })
}
})
registerTerminalPane(pane.id, {
paneId: pane.id,
worktreeId,
tabId,
getTitle: () => paneTitlesRef.current[pane.id] || 'Terminal',
getBufferLines: () => {
const lines: string[] = []
const activeBuffer = pane.terminal.buffer.active
const length = activeBuffer.length
for (let i = 0; i < length; i++) {
lines.push(activeBuffer.getLine(i)?.translateToString(true) ?? '')
}
return lines
},
focus: () => {
managerRef.current?.setActivePane(pane.id, { focus: true })
}
})

return
}

const configPath = `${worktreePath}/.orca-startup.json`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how other renderer files construct cross-platform paths
rg -n "path\.join\|window\.api.*path\|path\.sep" --type=ts -g '!node_modules' src/renderer/src/ | head -20

Repository: stablyai/orca

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target file and nearby imports/usages
file='src/renderer/src/hooks/useWorkspaceStartup.ts'
wc -l "$file"
cat -n "$file" | sed -n '1,220p'

# Find path-related utilities in the renderer codebase
rg -n "from 'node:path'|from \"node:path\"|from 'path'|from \"path\"|path\.join|path\.sep|window\.api.*path|api.*path" src/renderer/src/ --type=ts --type=tsx || true

Repository: stablyai/orca

Length of output: 2800


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search renderer code for Node path imports and preload path helpers
rg -n "from 'node:path'|from \"node:path\"|from 'path'|from \"path\"|path\.join|path\.sep|window\.api.*path|api.*path" src/renderer/src || true

# Inspect the target hook's surrounding module context and any nearby usage patterns
cat -n src/renderer/src/hooks/useWorkspaceStartup.ts | sed -n '1,120p'

# Look for fs/path-related API shape exposed on window.api
rg -n "window\.api\." src/renderer/src | head -80 || true

Repository: stablyai/orca

Length of output: 23404


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect renderer build/runtime config for Node builtin support
fd -a 'vite.config.*|tsconfig.*|electron.*|preload.*|webpack.*|rollup.*|package.json' .

# Search for renderer-specific Node builtin handling or aliasing
rg -n "node:path|external.*path|polyfill|browser:|renderer.*node|vite.*plugin.*node|resolve.*alias" \
  package.json tsconfig*.json vite.config.* electron.* src/renderer/src src/preload src/main || true

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the renderer preload API surface for filesystem/path helpers
rg -n "interface .*window\.api|declare global|fs:|pathExists|readFile|join|sep|resolve|basename|dirname" \
  src/renderer/src/web/web-preload-api.ts src/renderer/src/web/web-preload-api.test.ts \
  src/renderer/src/**/*.ts src/renderer/src/**/*.tsx || true

# Inspect the file that defines the runtime preload API types, if present
fd -a 'web-preload-api.*|preload.*api.*|global.*d.ts|window.*d.ts' src/renderer/src src/preload

Repository: stablyai/orca

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the renderer path helper module
file='src/renderer/src/lib/path.ts'
wc -l "$file"
cat -n "$file" | sed -n '1,220p'

# Inspect the web preload file API contract around readFile/path handling
sed -n '2660,2705p' src/renderer/src/web/web-preload-api.ts

Repository: stablyai/orca

Length of output: 6118


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check for renderer-side use of the shared joinPath helper
rg -n "from '`@/lib/path`'|joinPath\(" src/renderer/src --type=ts --type=tsx || true

# Inspect the startup hook for any existing path helper imports nearby
sed -n '1,80p' src/renderer/src/hooks/useWorkspaceStartup.ts

Repository: stablyai/orca

Length of output: 2282


Use joinPath for the config path

In src/renderer/src/hooks/useWorkspaceStartup.ts, import joinPath from @/lib/path and build .orca-startup.json with it instead of string concatenation.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants