apps/ui/nginx.conf (1)

15-24: Consider adding timeouts and disabling buffering for WebSocket/streaming connections.

Nginx's default proxy_read_timeout is 60s, which will silently close idle WebSocket connections (e.g., /api/events) if no data is sent within that window. The terminal WebSocket has a ping/pong mechanism, but the events WebSocket may go quiet for extended periods. Also, proxy_buffering off is recommended for streaming/WebSocket endpoints to avoid latency.

Proposed fix

 location /api/ {
     proxy_pass http://server:3008;
     proxy_http_version 1.1;
     proxy_set_header Upgrade $http_upgrade;
     proxy_set_header Connection $connection_upgrade;
     proxy_set_header Host $host;
     proxy_set_header X-Real-IP $remote_addr;
     proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
     proxy_set_header X-Forwarded-Proto $scheme;
+    proxy_read_timeout 86400s;
+    proxy_send_timeout 86400s;
+    proxy_buffering off;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ui/nginx.conf` around lines 15 - 24, The /api/ location block currently
risks silent WebSocket disconnects and buffering-induced latency; update the
location /api/ block to disable buffering and increase timeouts for
long-lived/streaming connections by adding directives such as proxy_buffering
off and raising proxy_read_timeout (e.g., several minutes), plus sensible
proxy_send_timeout and proxy_connect_timeout values; keep the existing WebSocket
headers (proxy_set_header Upgrade and Connection) intact so the changes apply
only to timeouts/buffering within the same location /api/ block.

apps/server/src/index.ts (1)

300-315: Verify the intended CORS behavior: explicit origins + local origins are now always allowed simultaneously.

Previously, when CORS_ORIGIN was set and the request origin didn't match, the request was immediately rejected. Now it falls through to the isLocalOrigin check, meaning local/private network origins are always allowed regardless of CORS_ORIGIN configuration. This is fine for the Docker Compose use case, but be aware that operators cannot use CORS_ORIGIN as an exhaustive allowlist to lock out local origins — which may matter in shared or multi-tenant hosting environments.

A brief inline comment above line 303 documenting this design choice (explicit origins + local origins are both always allowed) would help future maintainers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/index.ts` around lines 300 - 315, The change allows both
explicit CORS_ORIGIN entries and any local/private origins via
isLocalOrigin(origin), which means local origins are always permitted even if
they aren’t in allowedOrigins; add a concise inline comment just above the block
(near the allowedOrigins / isLocalOrigin checks) stating this design decision so
future maintainers know that CORS_ORIGIN is not an exhaustive deny-list and
local/loopback/private origins are intentionally always allowed (reference
variables/functions: allowedOrigins, process.env.CORS_ORIGIN, isLocalOrigin,
origin, callback).

apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)

402-452: LGTM — new split-button pattern is consistent with the rest of the file.

Both the Stop (lines 402-420) and Start (lines 425-452) dev server blocks follow the established DropdownMenuSub + flex-div + DropdownMenuItem/DropdownMenuSubTrigger split-button convention already used throughout this component (editor, terminal, pull/push, commits, stash, PR). The disabled/opacity-50/cursor-not-allowed handling on the Start sub-trigger correctly mirrors the primary item's loading state.

One minor point: the "View Dev Server Logs" DropdownMenuItem is duplicated verbatim in both DropdownMenuSubContent blocks. If this action's label, icon, or callback ever changes, both copies need to stay in sync.

♻️ Extract the shared log item to eliminate the duplication

+        {/* Shared submenu item — usable in both running and not-running states */}
+        {(() => {
+          const viewLogsItem = (
+            <DropdownMenuItem onClick={() => onViewDevServerLogs(worktree)} className="text-xs">
+              <ScrollText className="w-3.5 h-3.5 mr-2" />
+              View Dev Server Logs
+            </DropdownMenuItem>
+          );

Or, more idiomatically, hoist it as a local constant above the return:

+  const devServerLogsItem = (
+    <DropdownMenuItem onClick={() => onViewDevServerLogs(worktree)} className="text-xs">
+      <ScrollText className="w-3.5 h-3.5 mr-2" />
+      View Dev Server Logs
+    </DropdownMenuItem>
+  );

Then replace both DropdownMenuSubContent bodies:

-              <DropdownMenuSubContent>
-                <DropdownMenuItem onClick={() => onViewDevServerLogs(worktree)} className="text-xs">
-                  <ScrollText className="w-3.5 h-3.5 mr-2" />
-                  View Dev Server Logs
-                </DropdownMenuItem>
-              </DropdownMenuSubContent>
+              <DropdownMenuSubContent>{devServerLogsItem}</DropdownMenuSubContent>

(applied to both the running and not-running branches)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx`
around lines 402 - 452, Duplicate "View Dev Server Logs" DropdownMenuItem in the
two DropdownMenuSubContent blocks should be hoisted to a single reusable
constant to avoid drift; create a local constant (e.g., viewDevServerLogsItem)
above the component return that returns the DropdownMenuItem using
onViewDevServerLogs(worktree), the ScrollText icon and the same className, then
replace both DropdownMenuSubContent bodies with that constant (references:
DropdownMenuSubContent, DropdownMenuItem, onViewDevServerLogs, ScrollText).

apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-servers.ts (1)

459-465: Toast descriptions assume the parent opens the log panel

The descriptions 'Logs are now visible in the dev server panel.' (success) and 'Check the dev server logs panel for details.' (failure) embed an assumption that the caller has already opened the log panel. This is only true when handleStartDevServer is called through the handleStartDevServerAndShowLogs wrapper in worktree-panel.tsx. Any future consumer that calls handleStartDevServer directly will show an inaccurate message.

Consider either removing the panel-specific phrasing from the hook and letting the wrapper add it, or accepting the coupling as intentional and documenting it:

♻️ Option: neutral descriptions in the hook, wrapper adds the panel hint

In use-dev-servers.ts:

- toast.success('Dev server started, detecting port...', {
-   description: 'Logs are now visible in the dev server panel.',
- });
+ toast.success('Dev server started, detecting port...');

- toast.error(result.error || 'Failed to start dev server', {
-   description: 'Check the dev server logs panel for details.',
- });
+ toast.error(result.error || 'Failed to start dev server');

In worktree-panel.tsx (inside the wrapper):

  setLogPanelWorktree(worktree);
  setLogPanelOpen(true);
+ // toast shown by handleStartDevServer; description is accurate because the panel is now open
  await handleStartDevServer(worktree);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/ui/src/components/views/board-view/worktree-panel/hooks/use-dev-servers.ts`
around lines 459 - 465, The toast descriptions in handleStartDevServer currently
reference the dev server logs panel, which assumes callers opened that panel;
remove panel-specific phrasing from use-dev-servers.ts (update the success and
error toast descriptions to neutral messages like "Dev server started" and
"Failed to start dev server, check logs") and move the panel-specific messages
into the wrapper function handleStartDevServerAndShowLogs in worktree-panel.tsx
(have that wrapper call handleStartDevServer and then show an additional toast
or description that mentions the dev server logs panel), or alternatively add a
clear comment/docs on handleStartDevServer stating it requires the caller to
open the logs panel if they rely on panel-specific descriptions.

apps/ui/src/components/views/board-view.tsx (1)

489-504: Transient isCurrent: false on the fallback main-worktree object

When the fallback fires (line 491-493), found becomes the main worktree but currentWorktreePath is still non-null (pointing to the deleted path). The isCurrent computation at lines 499-502 is:

found.isCurrent ?? (currentWorktreePath !== null
  ? pathsEqual(found.path, currentWorktreePath)  // → false (main ≠ deleted path)
  : found.isMain)

If the main worktree's stored isCurrent field is null/undefined, the fallback expression evaluates to false, so selectedWorktree is returned with isCurrent: false during the window between the deletion event and the onDeleted reset (lines 1961-1969). This is passed to useAutoMode(selectedWorktree) at line 508.

In practice this is a single-render transient state before setCurrentWorktree(…, null, mainBranch) fires, so it's unlikely to be user-visible. A minimal guard would make the intent explicit and remove the subtle dependency on the store having isCurrent pre-populated for the main worktree:

🔧 Suggested clarification

-      if (!found) {
-        found = worktrees.find((w) => w.isMain);
-      }
+      if (!found) {
+        // Deleted worktree – fall back to main for this render cycle.
+        // onDeleted will call setCurrentWorktree(…, null) to reset properly.
+        found = worktrees.find((w) => w.isMain);
+      }

And in the return block, explicitly mark it current when we know we're falling back:

     return {
       ...found,
       isCurrent:
-        found.isCurrent ??
-        (currentWorktreePath !== null ? pathsEqual(found.path, currentWorktreePath) : found.isMain),
+        found.isCurrent ??
+        (currentWorktreePath !== null
+          ? pathsEqual(found.path, currentWorktreePath) || found.isMain  // treat main as current during fallback
+          : found.isMain),
       hasWorktree: found.hasWorktree ?? true,
     };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ui/src/components/views/board-view.tsx` around lines 489 - 504, The
selectedWorktree fallback can briefly return the main worktree with isCurrent
=== false when currentWorktreePath still points at the deleted path; adjust the
memo to detect when you used the fallback main (the branch that sets found =
worktrees.find(w => w.isMain)) and explicitly set isCurrent: true for that
returned object (instead of relying on the pathsEqual expression). Update the
return construction (the object built from found) to compute isCurrent using the
fallback condition (using found.isMain && /* fallback-detected */) so
useAutoMode receives the main as current during that transient render; reference
symbols: found, worktrees.find(...), currentWorktreePath, pathsEqual, isMain,
isCurrent, and the memo that produces selectedWorktree.

apps/ui/src/components/views/board-view/hooks/use-board-actions.ts (1)

220-227: childDependencies leaks into newFeatureData via ...restFeatureData

childDependencies is a transient creation parameter (same category as initialStatus and workMode) but is not excluded from the destructure. It therefore ends up inside newFeatureData, gets passed to addFeature, and is potentially stored in the Zustand state and forwarded to the server via persistFeatureCreate. If the store spreads the object as-is, future duplications of that feature would propagate the stale childDependencies array further.

Destructure it out alongside the other transient fields and reference it explicitly in the child-dependency loop below (line 247):

♻️ Proposed fix

      const {
        initialStatus: requestedStatus,
        workMode: _workMode,
+       childDependencies,
        ...restFeatureData
      } = featureData;
      const initialStatus = requestedStatus || 'backlog';

Then update the child-dependency block to use the locally destructured binding instead of featureData.childDependencies:

-     if (featureData.childDependencies && featureData.childDependencies.length > 0) {
-       for (const childId of featureData.childDependencies) {
+     if (childDependencies && childDependencies.length > 0) {
+       for (const childId of childDependencies) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ui/src/components/views/board-view/hooks/use-board-actions.ts` around
lines 220 - 227, The transient childDependencies field is not being removed and
is leaking into newFeatureData via the rest spread; update the destructuring of
featureData to pull out childDependencies alongside initialStatus and workMode
(so restFeatureData no longer contains it), ensure newFeatureData is built from
...restFeatureData (excluding childDependencies), and modify the
child-dependency loop to reference the locally destructured childDependencies
variable instead of featureData.childDependencies when calling
addFeature/persistFeatureCreate.

apps/server/src/services/pr-review-comments.service.ts (2)

106-154: Well-structured GraphQL execution helper.

The timeout handling, stdin error suppression, and process lifecycle management are all handled correctly. A couple of minor observations:

• Line 150: Only the first GraphQL error message is thrown; if the API returns multiple errors, the rest are silently dropped. Consider joining all error messages or at least logging them.

🔧 Optional: surface all GraphQL errors

   if (response.errors && response.errors.length > 0) {
-    throw new Error(response.errors[0].message);
+    const messages = response.errors.map((e) => e.message).join('; ');
+    throw new Error(messages);
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/services/pr-review-comments.service.ts` around lines 106 -
154, The executeGraphQL function currently throws only the first GraphQL error
(response.errors[0].message), losing additional error details; update the error
handling after the gh process completes so that when response.errors exists you
aggregate all messages (e.g., map over response.errors and join them into a
single string) and throw a new Error containing the combined message (and
optionally log or include the full errors array for debugging) instead of
discarding all but the first entry.

---

270-283: Consider extracting a shared REST API fetch helper.

The three execFileAsync('gh', ['api', endpoint, '--paginate'], { cwd, env, maxBuffer, timeout }) blocks are nearly identical. A small helper (e.g., fetchGitHubRest(projectPath, endpoint)) would reduce duplication and centralize the maxBuffer/timeout configuration.

Also applies to: 313-324, 372-383

apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx (1)

427-449: Consider keeping the logs submenu accessible while the dev server is starting.

The subtrigger is disabled during startup (line 445), which prevents the user from viewing dev server logs while the process is spinning up. Startup logs can be useful for diagnosing slow or failed starts. The Stop branch (lines 410–422) keeps the subtrigger always enabled.

If this is intentional (e.g., no log session exists yet), feel free to disregard.

🔧 Suggested change to keep the subtrigger enabled

                <DropdownMenuSubTrigger
-                  className={cn(
-                    'text-xs px-1 rounded-l-none border-l border-border/30 h-8',
-                    isStartingDevServer && 'opacity-50 cursor-not-allowed'
-                  )}
-                  disabled={isStartingDevServer}
+                  className="text-xs px-1 rounded-l-none border-l border-border/30 h-8"
                />

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx`
around lines 427 - 449, The DropdownMenuSubTrigger is being disabled when
isStartingDevServer is true, preventing access to viewDevServerLogsItem while
the dev server starts; modify the worktree-actions-dropdown so the
DropdownMenuSubTrigger remains enabled regardless of isStartingDevServer (remove
or conditionally avoid setting disabled={isStartingDevServer} on
DropdownMenuSubTrigger) while keeping the start button (DropdownMenuItem
onStartDevServer) disabled during startup, so users can open the logs submenu
even when the server is starting.

apps/server/src/index.ts (1)

280-280: Nit: 0.0.0.0 is dead code in an origin check.

Browsers never send 0.0.0.0 as an Origin header value (it's a bind-all/unspecified address). This condition can never be true and can be safely removed.

🧹 Proposed cleanup

     return (
       hostname === 'localhost' ||
       hostname === '127.0.0.1' ||
       hostname === '[::1]' ||
-      hostname === '0.0.0.0' ||
       hostname.startsWith('192.168.') ||
       hostname.startsWith('10.') ||
       /^172\.(1[6-9]|2[0-9]|3[0-1])\./.test(hostname)
     );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/index.ts` at line 280, Remove the dead origin check comparing
hostname === '0.0.0.0' in the origin validation logic (the condition shown as
"hostname === '0.0.0.0' ||"); browsers never send 0.0.0.0 as an Origin so delete
that clause and adjust the surrounding boolean expression accordingly to keep
the remaining valid checks and operator precedence intact.

apps/ui/src/components/views/board-view/hooks/use-board-actions.ts (1)

421-428: workMode still leaks into the persisted feature record in the update path.

handleAddFeature was fixed to strip workMode (and initialStatus) from the feature data before persisting. handleUpdateFeature only strips childDependencies, so workMode flows through restUpdates → finalUpdates → persistFeatureUpdate. For consistency with the add path, consider stripping it here too.

♻️ Suggested fix

-const { childDependencies, ...restUpdates } = updates;
+const { childDependencies, workMode: _workMode, ...restUpdates } = updates;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ui/src/components/views/board-view/hooks/use-board-actions.ts` around
lines 421 - 428, handleUpdateFeature currently only removes childDependencies
but lets workMode (and initialStatus) flow into restUpdates and then
finalUpdates before calling persistFeatureUpdate; update handleUpdateFeature to
strip workMode and initialStatus from updates (same as handleAddFeature does) so
they are not persisted: when you destructure updates, remove childDependencies,
workMode, and initialStatus (or explicitly build restUpdates without those keys)
and then construct finalUpdates (used by persistFeatureUpdate) from that cleaned
object along with title/branchName so persisted records match the add path.

apps/ui/src/store/ui-cache-store.ts (1)

163-187: Solid defensive sanitization for worktree cache restoration.

The rationale for only restoring path === null entries is well-documented and the crash-loop prevention story ties nicely with the error boundary changes.

One minor robustness thought: if localStorage data is ever corrupted (e.g., an entry value is null or missing .path), worktree.path on line 177 would throw a TypeError during startup — before the error boundary's crash-loop detection can intervene meaningfully. Consider a defensive guard:

🛡️ Optional: guard against corrupted cache entries

     for (const [projectPath, worktree] of Object.entries(cache.cachedCurrentWorktreeByProject)) {
-      if (worktree.path === null) {
+      if (worktree && worktree.path === null) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ui/src/store/ui-cache-store.ts` around lines 163 - 187, The loop that
restores cache from cachedCurrentWorktreeByProject assumes each entry is an
object with a .path property and can throw if a cached value is null or
malformed; update the for-loop over
Object.entries(cache.cachedCurrentWorktreeByProject) to defensively check that
worktree is an object (e.g., typeof worktree === "object" && worktree !== null)
and that 'path' in worktree before accessing worktree.path, only assigning to
sanitized[projectPath] when those guards pass and worktree.path === null; keep
using stateUpdate.currentWorktreeByProject for the sanitized result.

apps/ui/src/components/ui/app-error-boundary.tsx (1)

66-73: Hardcoded 'automaker-ui-cache' key is duplicated from ui-cache-store.ts.

This key duplicates STORE_NAME in ui-cache-store.ts. If it ever changes in one place but not the other, the cache-clear won't work. That said, for an error boundary you probably want minimal imports to avoid pulling in code that might itself be the crash source — so a shared constant could be risky here.

Consider extracting the key to a tiny, dependency-free constants file (e.g., constants/storage-keys.ts) that both modules import, or at minimum add a comment cross-referencing the source of truth:

-        localStorage.removeItem('automaker-ui-cache');
+        // Must match STORE_NAME in store/ui-cache-store.ts
+        localStorage.removeItem('automaker-ui-cache');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ui/src/components/ui/app-error-boundary.tsx` around lines 66 - 73,
Replace the hardcoded 'automaker-ui-cache' string in app-error-boundary.ts with
a shared, minimal dependency constant to avoid drift with ui-cache-store.ts's
STORE_NAME; either create a tiny dependency-free constants module (e.g.,
constants/storage-keys.ts) exporting AUTOMAKER_UI_CACHE and import it into both
app-error-boundary.ts and ui-cache-store.ts, or if you want to avoid any import
from the error boundary, add a clear cross-reference comment above the
localStorage.removeItem('automaker-ui-cache') call pointing to
ui-cache-store.ts's STORE_NAME to ensure maintainers update both places when the
key changes (update the localStorage.removeItem call and the sessionStorage key
usage to use the new constant or documented reference).