apps/ui/tests/projects/overview-dashboard.spec.ts (1)

76-107: Good addition of try/catch around route.fetch() to handle post-teardown calls.

The guard on line 105 prevents flaky test failures from routes invoked after the browser context closes. One minor observation:

The catch block silently swallows all errors, including unexpected ones during the test's active lifecycle (e.g., malformed JSON on line 78). Consider logging or re-throwing when the page is still alive to avoid hiding real failures — though in practice, such failures would likely surface as test assertion timeouts anyway.

🤖 Prompt for AI Agents

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

In `@apps/ui/tests/projects/overview-dashboard.spec.ts` around lines 76 - 107, The
catch currently swallows all errors from the route.fetch()/route.fulfill() block
which can hide real test failures; change the catch to capture the error (e.g.,
catch (err)) and only swallow it when the page/context is closed, otherwise log
or re-throw the error so real failures surface; locate the try/catch around
route.fetch() / route.fulfill() in the test and use the
route.request().frame().page().isClosed() (or equivalent context/frame closed
check) to decide whether to swallow or re-throw/log the caught error.

libs/prompts/src/enhancement.ts (1)

131-132: ADDITIVE_MODES is duplicated — export it from the shared package instead.

apps/ui/.../enhancement-constants.ts independently maintains the identical list ['technical', 'acceptance', 'ux-reviewer']. Because this constant is not exported from @automaker/prompts, the UI can't import the single source of truth and must keep its own copy in sync. Adding a new additive mode requires updating two separate files with no compiler-enforced link between them.

♻️ Proposed fix: export from the prompts library

 /** Modes that append additional content rather than rewriting the description */
-const ADDITIVE_MODES: EnhancementMode[] = ['technical', 'acceptance', 'ux-reviewer'];
+export const ADDITIVE_MODES: EnhancementMode[] = ['technical', 'acceptance', 'ux-reviewer'];

Then in apps/ui/.../enhancement-constants.ts:

-export const ADDITIVE_MODES: EnhancementMode[] = ['technical', 'acceptance', 'ux-reviewer'];
+export { ADDITIVE_MODES } from '@automaker/prompts';

As per coding guidelines: "Always import from shared packages (@automaker/*), never from old paths or relative imports to other modules."

🤖 Prompt for AI Agents

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

In `@libs/prompts/src/enhancement.ts` around lines 131 - 132, The ADDITIVE_MODES
constant (type EnhancementMode) is duplicated; export ADDITIVE_MODES from the
prompts package and remove the local copy in the UI so both consumers share a
single source of truth. Modify libs/prompts/src/enhancement.ts to export the
existing ADDITIVE_MODES (keeping the current value
['technical','acceptance','ux-reviewer']), and update the UI’s
enhancement-constants.ts to import { ADDITIVE_MODES } from `@automaker/prompts`
instead of defining its own array; ensure the exported symbol name and type
(EnhancementMode) match the consumers.

apps/ui/src/components/views/board-view/shared/enhancement/enhancement-constants.ts (1)

23-30: REWRITE_MODES / ADDITIVE_MODES are mutable and lack compile-time exhaustiveness.

Two related concerns:

1. Mutability: Both exported arrays are EnhancementMode[], so any caller can push/splice them and silently corrupt isAdditiveMode for all subsequent calls in the same module lifetime. They should be readonly.

2. Exhaustiveness gap: ENHANCEMENT_MODE_LABELS (typed Record<EnhancementMode, string>) will cause a compile error if a new EnhancementMode value is added to @automaker/types without being listed—but REWRITE_MODES and ADDITIVE_MODES carry no such guarantee. A new mode would silently fall through isAdditiveMode as false (treated as rewrite) and disappear from the dropdown entirely.

Replacing the two arrays with a satisfies-annotated category map gives both readonly-ness and exhaustive compile-time checking:

♻️ Proposed fix

-/** Modes that rewrite/replace the entire description */
-export const REWRITE_MODES: EnhancementMode[] = ['improve', 'simplify'];
-
-/** Modes that append additional content below the original description */
-export const ADDITIVE_MODES: EnhancementMode[] = ['technical', 'acceptance', 'ux-reviewer'];
-
-/** Check if a mode appends content rather than replacing */
-export function isAdditiveMode(mode: EnhancementMode): boolean {
-  return ADDITIVE_MODES.includes(mode);
-}
+/** Per-mode category: 'rewrite' replaces description; 'additive' appends below it. */
+const MODE_CATEGORIES = {
+  improve: 'rewrite',
+  simplify: 'rewrite',
+  technical: 'additive',
+  acceptance: 'additive',
+  'ux-reviewer': 'additive',
+} satisfies Record<EnhancementMode, 'rewrite' | 'additive'>;
+
+/** Modes that rewrite/replace the entire description */
+export const REWRITE_MODES = (Object.keys(MODE_CATEGORIES) as EnhancementMode[]).filter(
+  (m) => MODE_CATEGORIES[m] === 'rewrite',
+);
+
+/** Modes that append additional content below the original description */
+export const ADDITIVE_MODES = (Object.keys(MODE_CATEGORIES) as EnhancementMode[]).filter(
+  (m) => MODE_CATEGORIES[m] === 'additive',
+);
+
+/** Check if a mode appends content rather than replacing */
+export function isAdditiveMode(mode: EnhancementMode): boolean {
+  return MODE_CATEGORIES[mode] === 'additive';
+}

🤖 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/shared/enhancement/enhancement-constants.ts`
around lines 23 - 30, REWRITE_MODES and ADDITIVE_MODES are mutable and not
exhaustively checked against the EnhancementMode union; replace them with a
single readonly category map (keyed by EnhancementMode) using the TypeScript
"satisfies" operator to enforce exhaustiveness (e.g., a Record<EnhancementMode,
'additive'|'rewrite'>) so the compiler errors if a new EnhancementMode is added,
make the map readonly (as const) to prevent runtime mutation, and update
isAdditiveMode to look up the mode in that map (referencing REWRITE_MODES,
ADDITIVE_MODES, isAdditiveMode, ENHANCEMENT_MODE_LABELS, and EnhancementMode to
locate and replace the arrays).

apps/server/src/lib/settings-helpers.ts (1)

37-40: DEFAULT_MAX_TURNS now equals MAX_ALLOWED_TURNS (both 10000).

Since the default and the ceiling are identical, users can only lower the configured value — there's no headroom above the default. This is a deliberate "don't limit by default" stance, but it's worth noting:

• Users who don't configure defaultMaxTurns will get 10,000 turns, which could be expensive on high-cost models.
• The clamp on line 152 (Math.max(1, Math.min(MAX_ALLOWED_TURNS, ...))) effectively makes the range [1, 10000] with 10000 as the default.

If the intent is to allow power users to go even higher in the future, consider keeping MAX_ALLOWED_TURNS above DEFAULT_MAX_TURNS.

🤖 Prompt for AI Agents

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

In `@apps/server/src/lib/settings-helpers.ts` around lines 37 - 40,
DEFAULT_MAX_TURNS equals MAX_ALLOWED_TURNS (both 10000) so the default sits at
the ceiling and users cannot increase turns above the default; update the
constants to restore headroom by either raising MAX_ALLOWED_TURNS above
DEFAULT_MAX_TURNS (e.g., set MAX_ALLOWED_TURNS to a larger value) or lowering
DEFAULT_MAX_TURNS below MAX_ALLOWED_TURNS, and adjust the clamp usage
(Math.max(1, Math.min(MAX_ALLOWED_TURNS, ...))) comment to reflect the intended
behavior; change the values where DEFAULT_MAX_TURNS and MAX_ALLOWED_TURNS are
defined and add a short clarifying comment near the clamp to document the
allowed range.

apps/ui/src/components/views/file-editor-view/components/editor-tabs.tsx (2)

111-183: Scroll chevrons are always visible, even when all tabs fit without scrolling.

The left/right chevron buttons are always rendered and clickable regardless of whether the tab list actually overflows. Consider hiding them (or lowering their opacity) when there's no overflow to avoid a confusing affordance on small tab sets.

This is a minor UX polish item that can be addressed later.

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/editor-tabs.tsx`
around lines 111 - 183, Tabs chevrons are always shown; detect overflow using
the scroll container (scrollRef) and hide or dim the left/right buttons when no
overflow or when already scrolled to the ends. Add state (e.g., canScrollLeft,
canScrollRight or isOverflowing) computed from scrollRef.current.scrollWidth vs
clientWidth and updated on mount/resize/scroll (use ResizeObserver or window
resize + scroll listener) and in the existing scrollBy logic, then conditionally
render or apply reduced-opacity/disabled styles to the ChevronLeft/ChevronRight
buttons (the onClick handlers that call scrollBy) so they are not
clickable/visible when no scrolling is possible.

---

144-150: Fragile text- → bg- class substitution for the dot indicator.

fileColor.replace('text-', 'bg-') works for the current set of returns from getFileColor, but if a future variant like text-muted-foreground/50 were added, the replacement would break. Consider a small mapping or a parallel getFileDotColor function that returns bg-* classes directly.

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/editor-tabs.tsx`
around lines 144 - 150, The dot color substitution using
fileColor.replace('text-', 'bg-') is fragile; replace it by deriving the dot
class from a dedicated mapping or helper (e.g., add a getFileDotColor(file) or a
COLORS_DOT_MAP) and use that in editor-tabs.tsx instead of string replace where
tab.isDirty is checked (the JSX that currently uses fileColor.replace). Ensure
the helper returns explicit 'bg-*' classes for each file color variant and
include a safe fallback class so future `text-*` variants like
`text-muted-foreground/50` won’t break.

apps/ui/src/lib/diff-utils.ts (1)

192-201: Context lines without a leading space are silently included as-is.

When a line inside a hunk doesn't start with +, -, \, or a space, the else branch (line 196) treats it as a context line. This is a reasonable fallback for well-formed diffs but could silently produce incorrect output for corrupted or non-standard diff formats. A minor robustness concern — not blocking.

🤖 Prompt for AI Agents

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

In `@apps/ui/src/lib/diff-utils.ts` around lines 192 - 201, The else branch
currently treats any non '+', '-' line as a context line and pushes it into
oldLines/newLines, which can silently accept corrupted hunks; update the branch
that examines line (the code using line.startsWith and pushing into
oldLines/newLines) to explicitly check for a leading space or a backslash escape
(e.g., line.startsWith(' ') or line.startsWith('\\')) and only treat those as
context lines, and for any other leading character log a warning (e.g.,
console.warn or project logger) including the offending line content and skip
adding it (or optionally throw an error if you prefer fail-fast), so unexpected
diff formats are surfaced rather than silently accepted.

apps/server/src/services/agent-executor.ts (1)

41-41: Import DEFAULT_MAX_TURNS from settings-helpers.ts to avoid duplication.

DEFAULT_MAX_TURNS is defined locally here (10000) and also exported from apps/server/src/lib/settings-helpers.ts (10000). Since agent-executor.ts already imports from that module, consolidate the constant by importing it instead of maintaining a duplicate.

♻️ Proposed fix

-const DEFAULT_MAX_TURNS = 10000;
+import { DEFAULT_MAX_TURNS } from '../lib/settings-helpers.js';

🤖 Prompt for AI Agents

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

In `@apps/server/src/services/agent-executor.ts` at line 41, Replace the locally
declared DEFAULT_MAX_TURNS in agent-executor.ts with the exported constant from
settings-helpers.ts: remove the line declaring const DEFAULT_MAX_TURNS = 10000
and add DEFAULT_MAX_TURNS to the existing import list from
apps/server/src/lib/settings-helpers.ts so the file uses the shared
DEFAULT_MAX_TURNS symbol instead of a duplicate local value.

apps/ui/src/hooks/use-settings-sync.ts (1)

788-788: Consider centralizing the max-turns fallback.

10000 is now a cross-cutting default; using a shared constant would reduce drift risk over time.

🤖 Prompt for AI Agents

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

In `@apps/ui/src/hooks/use-settings-sync.ts` at line 788, Replace the magic
literal 10000 used as the fallback for max turns with a centralized exported
constant (e.g., DEFAULT_MAX_TURNS) and import that constant into
use-settings-sync; update the line using serverSettings.defaultMaxTurns ?? 10000
to use serverSettings.defaultMaxTurns ?? DEFAULT_MAX_TURNS, and add the new
constant to a shared constants module (export it from a common
settings/constants file) so all code paths reference the single source of truth.

apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (1)

249-263: Consider deduplicating thinking-level selection logic.

The same getThinkingLevelsForModel + fallback logic is repeated on open and reset; a small helper would reduce drift risk.

♻️ Example helper

+const resolveThinkingLevel = (modelId: string, defaultLevel: ThinkingLevel) => {
+  const available = getThinkingLevelsForModel(modelId);
+  return available.includes(defaultLevel) ? defaultLevel : available[0];
+};

Then use resolveThinkingLevel(...) in both the open effect and resetForm.

Also applies to: 418-430

🤖 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/dialogs/add-feature-dialog.tsx`
around lines 249 - 263, Extract the repeated logic that picks a thinking level
into a helper like resolveThinkingLevel(modelId: string, defaultThinkingLevel:
ThinkingLevel) that calls getThinkingLevelsForModel(modelId) and returns
defaultThinkingLevel if included or the first available level otherwise; then
replace the duplicated blocks in the effect that sets modelEntry (using
effectiveDefaultFeatureModel, setModelEntry) and in resetForm with calls to
resolveThinkingLevel(effectiveModelId, defaultThinkingLevel) and assign the
returned value to thinkingLevel when building the new model entry.

apps/ui/src/hooks/use-project-settings-loader.ts (1)

107-123: Clamp worktree display values before storing.

If project settings ever contain out-of-range values (legacy/corrupt sync), negative counts or thresholds can break the panel layout. Consider clamping to a sane non‑negative range (and UI max if applicable) before writing into the store.

✅ Possible hardening

+    const clampNonNegative = (value: number) => Math.max(0, value);
+
     if (settings.pinnedWorktreesCount !== undefined) {
-      setPinnedWorktreesCount(projectPath, settings.pinnedWorktreesCount);
+      setPinnedWorktreesCount(projectPath, clampNonNegative(settings.pinnedWorktreesCount));
     }

     if (settings.worktreeDropdownThreshold !== undefined) {
-      setWorktreeDropdownThreshold(projectPath, settings.worktreeDropdownThreshold);
+      setWorktreeDropdownThreshold(
+        projectPath,
+        clampNonNegative(settings.worktreeDropdownThreshold)
+      );
     }

🤖 Prompt for AI Agents

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

In `@apps/ui/src/hooks/use-project-settings-loader.ts` around lines 107 - 123,
Clamp numeric worktree settings before storing: when applying settings in
use-project-settings-loader, validate settings.pinnedWorktreesCount and
settings.worktreeDropdownThreshold by coercing them to safe numeric ranges
(e.g., ensure >= 0 and cap to UI max). Modify the branches that call
setPinnedWorktreesCount(projectPath, settings.pinnedWorktreesCount) and
setWorktreeDropdownThreshold(projectPath, settings.worktreeDropdownThreshold) to
compute sanitized values (use Number(...) or parseInt + isFinite checks, then
Math.max(0, value) and Math.min(value, MAX_*) with defined constants like
MAX_PINNED_WORKTREES / MAX_DROPDOWN_THRESHOLD) and pass those sanitized values;
leave boolean fields (useWorktrees, alwaysUseWorktreeDropdown) unchanged.

libs/types/src/settings.ts (1)

333-363: Avoid string matching for adaptive-capable models.

isAdaptiveThinkingModel still relies on substring checks. Consider routing this through model definitions (capabilities flags) and querying by model ID to keep capability detection consistent across providers and model updates. Based on learnings: When modeling AI capabilities, add per-model flags to model definitions (e.g., supportsThinking: boolean) and check capabilities by model ID rather than assuming all models from a provider share the same features. This should be applied to TypeScript files under libs/types (not just a single file) to ensure consistent handling across models.

🤖 Prompt for AI Agents

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

In `@libs/types/src/settings.ts` around lines 333 - 363, Replace the brittle
substring check in isAdaptiveThinkingModel with a capability lookup against the
canonical model definitions registry: add/ensure a per-model flag (e.g.,
supportsThinking: boolean) on your model definition type in libs/types, and
change isAdaptiveThinkingModel(model: string) to query the registry by model ID
to return that flag; then update getThinkingLevelsForModel and
getDefaultThinkingLevel to call the revised isAdaptiveThinkingModel (or read the
supportsThinking flag directly) so capability detection is consistent across
providers and model updates (refer to the functions isAdaptiveThinkingModel,
getThinkingLevelsForModel, getDefaultThinkingLevel and the model definition/type
in libs/types).

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

apps/server/src/services/event-hook-service.ts (1)

269-307: markFeatureHandled() placement is correct; executionMode on FeatureCompletedPayload is unused.

The markFeatureHandled(payload.featureId) call at line 276 is placed before the first await, ensuring the deduplication guard is set atomically in the Node.js event loop before any concurrent feature_status_changed handler can run. This correctly addresses deduplication. ✅

The executionMode field declared on FeatureCompletedPayload (line 112) is never read inside handleFeatureCompletedEvent — the handler fires for all feature:completed events regardless. If this field is truly unused, consider removing it to avoid misleading future readers:

♻️ Remove unused field from FeatureCompletedPayload

 interface FeatureCompletedPayload {
   featureId: string;
   featureName?: string;
   projectPath: string;
   passes?: boolean;
   message?: string;
-  executionMode?: 'auto' | 'manual';
 }

🤖 Prompt for AI Agents

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

In `@apps/server/src/services/event-hook-service.ts` around lines 269 - 307, The
FeatureCompletedPayload type's executionMode field is never used by
handleFeatureCompletedEvent (the handler ignores executionMode), so remove the
unused executionMode property from the FeatureCompletedPayload definition to
avoid confusion; update any related type/interface declaration named
FeatureCompletedPayload and any tests or callers that set executionMode to stop
setting it, and run type checks to ensure no remaining references to
executionMode exist.

apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx (1)

558-576: ⚠️ Potential issue | 🟡 Minor

Missing rollback on server persist failure.

onValueChange updates the store optimistically, but onValueCommit's error path only shows a toast — the store retains the uncommitted value while the server keeps the old one. Every other setting action in this file (handleAddCopyFile, handleRemoveCopyFile, handleFileSelectorSelect) correctly rolls back on failure.

🛡️ Proposed fix

  onValueCommit={async (value) => {
    const newValue = value[0] ?? pinnedWorktreesCount;
+   const prevValue = pinnedWorktreesCount;

    // Persist to server
    try {
      const httpClient = getHttpApiClient();
      await httpClient.settings.updateProject(project.path, {
        pinnedWorktreesCount: newValue,
      });
    } catch (error) {
+     // Rollback optimistic update on failure
+     setPinnedWorktreesCount(project.path, prevValue);
      console.error('Failed to persist pinnedWorktreesCount:', error);
      toast.error('Failed to save pinned worktrees setting');
    }
  }}

🤖 Prompt for AI Agents

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

In
`@apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx`
around lines 558 - 576, The optimistic update in onValueChange for
pinnedWorktreesCount needs a rollback on server persist failure: capture the
previous value (e.g., const prev = pinnedWorktreesCount) before calling
setPinnedWorktreesCount in onValueChange or at the start of onValueCommit, and
in the catch block of onValueCommit (where getHttpApiClient is used and
toast.error is called) call setPinnedWorktreesCount(project.path, prev) to
restore the prior value; ensure you reference the same project.path and
pinnedWorktreesCount variables so behavior matches other handlers like
handleAddCopyFile/handleRemoveCopyFile.

libs/prompts/tests/enhancement.test.ts (3)

353-396: Missing positive test case for isValidEnhancementMode('ux-reviewer').

All four pre-existing modes each have a dedicated it("should return true for '...'") block, but 'ux-reviewer' does not. The only indirect coverage is the integration test at line 426, which is conditional on getAvailableEnhancementModes() already returning the mode.

♻️ Suggested addition

   it("should return true for 'acceptance'", () => {
     expect(isValidEnhancementMode('acceptance')).toBe(true);
   });
+
+  it("should return true for 'ux-reviewer'", () => {
+    expect(isValidEnhancementMode('ux-reviewer')).toBe(true);
+  });

🤖 Prompt for AI Agents

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

In `@libs/prompts/tests/enhancement.test.ts` around lines 353 - 396, Test suite
lacks a direct positive unit test asserting
isValidEnhancementMode('ux-reviewer') returns true; add a new it block in the
describe('isValidEnhancementMode') group that calls
isValidEnhancementMode('ux-reviewer') and expects true so the mode is explicitly
covered (similar to the existing cases for 'improve', 'technical', 'simplify',
'acceptance'); locate the test file libs/prompts/tests/enhancement.test.ts and
add the new assertion next to the other per-mode it(...) tests.

---

2-17: Missing UX_REVIEWER_SYSTEM_PROMPT / UX_REVIEWER_EXAMPLES imports and constant-level tests.

By analogy with every other mode, enhancement.ts should export UX_REVIEWER_SYSTEM_PROMPT and UX_REVIEWER_EXAMPLES. Neither is imported here, so the System Prompt Constants and Examples Constants describe blocks have zero direct coverage for the new mode's exported values.

♻️ Suggested additions

 import {
   ...
   ACCEPTANCE_EXAMPLES,
+  UX_REVIEWER_SYSTEM_PROMPT,
+  UX_REVIEWER_EXAMPLES,
 } from '../src/enhancement.js';

Then add corresponding it blocks inside the existing System Prompt Constants and Examples Constants describe blocks (mirroring the ACCEPTANCE_SYSTEM_PROMPT / ACCEPTANCE_EXAMPLES tests).

🤖 Prompt for AI Agents

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

In `@libs/prompts/tests/enhancement.test.ts` around lines 2 - 17, Import the new
UX_REVIEWER_SYSTEM_PROMPT and UX_REVIEWER_EXAMPLES symbols into the
enhancement.test.ts imports and add constant-level tests mirroring the
ACCEPTANCE cases: add an it block in the "System Prompt Constants" describe that
asserts UX_REVIEWER_SYSTEM_PROMPT equals the exported value for the UX reviewer
mode, and add an it block in the "Examples Constants" describe that asserts
UX_REVIEWER_EXAMPLES equals the exported examples; reference the exported
symbols UX_REVIEWER_SYSTEM_PROMPT and UX_REVIEWER_EXAMPLES and follow the same
assertion pattern used for ACCEPTANCE_SYSTEM_PROMPT / ACCEPTANCE_EXAMPLES so the
new mode has direct coverage.

---

113-175: getEnhancementPrompt, getSystemPrompt, and getExamples unit tests don't cover 'ux-reviewer'.

Each block has an explicit it for every previously existing mode; consistency and isolation suggest adding equivalent tests for the new mode in all three sections (similar to the 'acceptance' cases already there).

Also applies to: 177-223

🤖 Prompt for AI Agents

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

In `@libs/prompts/tests/enhancement.test.ts` around lines 113 - 175, Add unit
tests covering the new 'ux-reviewer' mode in all three test suites (the
getEnhancementPrompt, getSystemPrompt, and getExamples blocks) mirroring the
existing 'acceptance' tests: assert that getEnhancementPrompt('ux-reviewer')
returns systemPrompt === UX_REVIEWER_SYSTEM_PROMPT and a description containing
the expected UX-related keyword(s), add case-insensitivity checks (e.g.,
'UX-REVIEWER' and mixed-case) to ensure getSystemPrompt/getEnhancementPrompt
handle casing, and add a corresponding test in the getExamples suite to verify
getExamples('ux-reviewer') returns the expected UX_REVIEWER_EXAMPLES (or the
correct example structure); follow the same structure and assertions used for
'acceptance' to keep tests consistent and isolated.

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

72-74: Prefer the dialogs barrel export for intra-app imports.

Keep board-view dialog imports consistent by pulling ChangePRNumberDialog from the dialogs index instead of a direct path.

♻️ Suggested change

-import { ChangePRNumberDialog } from './board-view/dialogs/change-pr-number-dialog';
+import { ChangePRNumberDialog } from './board-view/dialogs';

Based on learnings: In the apps/ui codebase, when importing UI components within the same app, prefer barrel exports from ../components (i.e., import from the components index barrel) rather than direct path imports.

🤖 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 72 - 74, Replace
the direct-path import of ChangePRNumberDialog with the dialogs barrel export to
match the other dialog imports and intra-app convention: update the import
statement that references ChangePRNumberDialog so it imports from the dialogs
index/barrel (the same module that would export CreatePRDialog and
CreateBranchDialog) instead of the nested file path; ensure the symbol
ChangePRNumberDialog is referenced unchanged in board-view.tsx so existing
usages remain valid.

apps/ui/src/components/views/file-editor-view/components/file-tree.tsx (1)

125-154: Optional: pre-compute folder rollups once to avoid O(N×M) per-node scans

computeFolderGitRollup iterates the entire gitStatusMap for every directory TreeNode. With useMemo this only re-runs when the map reference changes, so the cost is acceptable for typical codebases. In large monorepos (many folders × many modified files), a single pass that builds a Map<folderPath, Rollup> once (e.g., in the store or a top-level useMemo in FileTree) and passes it down would reduce the work to O(M) total rather than O(N×M).

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/file-tree.tsx`
around lines 125 - 154, computeFolderGitRollup currently scans gitStatusMap for
each TreeNode causing O(N×M) work; instead precompute a single Map<folderPath,
Rollup> in a higher scope (e.g., in FileTree using useMemo or in the store) by
making one pass over gitStatusMap and enhancedGitStatusMap to aggregate count,
dominantStatus (use STATUS_PRIORITY), totalAdded and totalRemoved, then pass
that precomputed map down to TreeNode and replace calls to
computeFolderGitRollup with lookups into the map; ensure the aggregation logic
mirrors computeFolderGitRollup (prefix matching of folderPath, STATUS_PRIORITY
tie-breaking, summing EnhancedGitFileStatus.linesAdded/linesRemoved).

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

2680-2701: Guard against empty newBranch to avoid inadvertent slot shuffling.

When newBranch === '' (clearing a slot), current.indexOf('') matches the first pre-filled empty slot (not necessarily slotIndex). The swap logic then moves the current slot's branch to that unrelated empty slot before writing '' into slotIndex, silently reordering branch assignments.

🛡️ Proposed fix

  swapPinnedWorktreeBranch: (projectPath, slotIndex, newBranch) =>
    set((state) => {
+     if (!newBranch) {
+       // Clearing a slot: just blank it out, no swap needed
+       const src = state.pinnedWorktreeBranchesByProject[projectPath] ?? [];
+       if (slotIndex >= src.length) return state;
+       const updated = [...src];
+       updated[slotIndex] = '';
+       return {
+         pinnedWorktreeBranchesByProject: {
+           ...state.pinnedWorktreeBranchesByProject,
+           [projectPath]: updated,
+         },
+       };
+     }
      const src = state.pinnedWorktreeBranchesByProject[projectPath] ?? [];

🤖 Prompt for AI Agents

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

In `@apps/ui/src/store/app-store.ts` around lines 2680 - 2701, The
swapPinnedWorktreeBranch logic can inadvertently swap when newBranch is an empty
string; update swapPinnedWorktreeBranch to skip the "existingIndex" swap when
newBranch is empty: only run the existingIndex check and swap if newBranch !==
'' (and existingIndex !== slotIndex). Keep the rest of the pre-fill and
assignment behavior (current[slotIndex] = newBranch) so clearing a slot simply
writes '' without moving another slot's branch; reference
swapPinnedWorktreeBranch, current, existingIndex, and slotIndex when making this
change.

apps/ui/src/components/views/file-editor-view/components/code-editor.tsx (2)

258-261: Add instanceof guard to eq() to match the base-class contract.

The base class signature is eq(widget: WidgetType): boolean. TypeScript method bivariance lets the narrowed parameter compile, but at runtime any call passing a different WidgetType subclass would access other.lines as undefined and throw TypeError on .length. While CodeMirror typically compares same-type widgets, a defensive type guard is the standard practice for WidgetType overrides.

🛡️ Proposed fix

   eq(other: DeletedLinesWidget) {
+    if (!(other instanceof DeletedLinesWidget)) return false;
     return (
       this.lines.length === other.lines.length && this.lines.every((l, i) => l === other.lines[i])
     );
   }

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/code-editor.tsx`
around lines 258 - 261, The eq method in DeletedLinesWidget should first check
that the incoming widget is an instance of DeletedLinesWidget before accessing
other.lines; update DeletedLinesWidget.eq(widget) to return false if !(widget
instanceof DeletedLinesWidget) and only then compare lengths and contents
(this.lines and widget.lines) to satisfy the base-class contract and avoid
runtime TypeError.

---

222-227: Dead guard: !line.startsWith('\\') is unreachable in this branch.

The \ No newline at end of file message starts with a backslash (\), so it never matches line.startsWith(' ') or line === ''. The outer else if condition is never entered for such lines, making the inner guard permanently true and thus dead code. Those lines silently fall through all conditions (correct behavior), but the comment and guard are misleading.

🧹 Suggested cleanup

-    } else if (line.startsWith(' ') || line === '') {
+    } else if (line.startsWith(' ') || line === '') {
       flushDeletions();
-      if (!line.startsWith('\\')) {
-        // Skip "\ No newline at end of file"
-        currentNewLine++;
-      }
+      currentNewLine++;
+    } else {
+      // Ignore "\ No newline at end of file" and any other unrecognized prefixes
     }

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/code-editor.tsx`
around lines 222 - 227, The inner guard and comment inside the else-if branch
are dead: when line.startsWith(' ') || line === '' is true, line cannot start
with '\' so the check !line.startsWith('\\') and its comment should be removed;
instead in the branch for those lines (around the code using line,
flushDeletions, and currentNewLine in code-editor.tsx) simply call
flushDeletions(); increment currentNewLine; and remove the misleading comment so
behavior and intent are clear (leave handling of "\ No newline at end of file"
to its existing branch).

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

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

100-105: ⚠️ Potential issue | 🟠 Major

Add a worktree guard to /update-pr-number.

This endpoint operates on worktreePath but does not enforce requireValidWorktree, which risks 500s or updating non-worktree paths. Align it with other worktree-specific routes.

✅ Suggested fix

 router.post(
   '/update-pr-number',
   validatePathParams('worktreePath', 'projectPath?'),
+  requireValidWorktree,
   requireGitRepoOnly,
   createUpdatePRNumberHandler()
 );

🤖 Prompt for AI Agents

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

In `@apps/server/src/routes/worktree/index.ts` around lines 100 - 105, The POST
/update-pr-number route currently validates path params and enforces
requireGitRepoOnly but misses the worktree-specific guard; add the
requireValidWorktree middleware into the middleware chain for this route (i.e.,
between requireGitRepoOnly and createUpdatePRNumberHandler) so the route that
uses worktreePath runs only for valid worktrees, matching the pattern used by
other worktree routes.

apps/ui/src/components/views/file-editor-view/components/file-tree.tsx (2)

443-447: ⚠️ Potential issue | 🟡 Minor

Inconsistent label: "modified" (tooltip) vs. "changed" (badge title).

Line 444 says "modified file(s)" but line 526 says "changed file(s)". The count includes all git statuses (untracked, deleted, renamed, etc.), so "changed" is more accurate. Align both to use "changed".

Proposed fix

-    tooltip += ` (${folderRollup.count} modified file${folderRollup.count !== 1 ? 's' : ''})`;
+    tooltip += ` (${folderRollup.count} changed file${folderRollup.count !== 1 ? 's' : ''})`;

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/file-tree.tsx`
around lines 443 - 447, The tooltip string in file-tree.tsx uses "modified
file(s)" while the badge title uses "changed file(s)"; update the tooltip
construction (where node.isDirectory and folderRollup are used, specifically the
tooltip variable built around folderRollup.count) to use the word "changed"
instead of "modified" so both labels read "changed file(s)" and remain
pluralized the same way as the badge title.

---

530-542: ⚠️ Potential issue | 🟡 Minor

Missing bg-gray-600 for '!' (ignored) status in the folder dot.

The file-level dot (line 591) includes 'bg-gray-600': gitStatus === '!', but the folder dot omits it. If a folder contains only ignored files, dominantStatus will be '!' and the dot renders with no background color — invisible.

Proposed fix

 <span
   className={cn('w-1.5 h-1.5 rounded-full shrink-0', {
     'bg-yellow-500': folderRollup.dominantStatus === 'M',
     'bg-green-500':
       folderRollup.dominantStatus === 'A' || folderRollup.dominantStatus === 'S',
     'bg-red-500': folderRollup.dominantStatus === 'D',
     'bg-gray-400': folderRollup.dominantStatus === '?',
+    'bg-gray-600': folderRollup.dominantStatus === '!',
     'bg-purple-500': folderRollup.dominantStatus === 'R',
     'bg-cyan-500': folderRollup.dominantStatus === 'C',
     'bg-orange-500': folderRollup.dominantStatus === 'U',
   })}

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/file-tree.tsx`
around lines 530 - 542, The folder-status dot rendering omits styling for the
ignored status ('!') so folders with only ignored files show no background; in
the span that builds className via cn (the one referencing
folderRollup.dominantStatus), add the mapping 'bg-gray-600':
folderRollup.dominantStatus === '!' so the ignored folder dot matches the
file-level dot (which uses gitStatus === '!') and remains visible; keep the
existing title that uses getGitStatusLabel(folderRollup.dominantStatus).

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

1189-1202: Covered by the helper extraction suggestion above.

🤖 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 1189 - 1202, This block duplicates logic that removes running tasks from
autoModeByWorktree for features; refactor to reuse the existing helper (or
extract one) instead of repeating code: identify the duplicated logic in
use-board-actions.ts that iterates useAppStore.getState().autoModeByWorktree and
calls store.removeRunningTask for each runningVerified feature, and replace it
with a call to the shared helper (or create a new function named e.g.
removeRunningTasksForFeatures(currentProject, runningVerified)) and invoke that
helper here; reference useAppStore, autoModeByWorktree, runningVerified, and
removeRunningTask (or the existing handleForceStopFeature helper) when locating
and replacing the code.

libs/prompts/tests/enhancement.test.ts (3)

372-390: ux-reviewer additive phrasing not explicitly asserted in buildUserPrompt tests

The "all modes" loop only verifies output existence and that testText is present. Since ux-reviewer is an additive mode, a dedicated test analogous to the technical block at lines 279–286 would confirm it emits 'Here are examples of the additional details section' rather than 'Here are some examples of how to enhance task descriptions:'.

🤖 Prompt for AI Agents

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

In `@libs/prompts/tests/enhancement.test.ts` around lines 372 - 390, The current
test loop for buildUserPrompt only checks general output; add a focused test for
the 'ux-reviewer' mode that calls buildUserPrompt('ux-reviewer', testText) and
asserts the result contains testText and the specific additive phrasing 'Here
are examples of the additional details section' (mirroring the pattern used in
the existing technical test block that looks for its mode-specific phrasing);
reference buildUserPrompt and the 'ux-reviewer' mode to locate where to add this
assertion.

---

1-19: Missing test coverage for isAdditiveMode, ADDITIVE_MODES, and REWRITE_MODES

These three exports are the backbone of the issue #803 fix — buildUserPrompt branches on ADDITIVE_MODES.includes(mode) — but they are neither imported nor exercised in this suite. Consider adding a describe('Additive vs Rewrite modes') block covering:

• ADDITIVE_MODES contains exactly ['technical', 'acceptance', 'ux-reviewer'] (and not 'improve' or 'simplify').
• REWRITE_MODES contains exactly ['improve', 'simplify'].
• isAdditiveMode(mode) returns true for each additive mode and false for each rewrite mode.

This would provide a safety net preventing future mode reclassification regressions.

🤖 Prompt for AI Agents

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

In `@libs/prompts/tests/enhancement.test.ts` around lines 1 - 19, Add tests that
import and assert the new mode constants and predicate: verify ADDITIVE_MODES
strictly equals ['technical','acceptance','ux-reviewer'] and REWRITE_MODES
strictly equals ['improve','simplify'], and assert isAdditiveMode(mode) returns
true for each entry in ADDITIVE_MODES and false for each entry in REWRITE_MODES;
also add a quick assertion that buildUserPrompt branches on ADDITIVE_MODES by
calling buildUserPrompt with one additive mode and one rewrite mode and
asserting the different expected behavior (to guard the branching change).

---

172-178: getEnhancementPrompt('ux-reviewer') test omits description assertion

All other mode tests (lines 135–170) validate the description field with a toContain check (e.g., 'vague', 'implementation', 'verbose', 'acceptance'). The ux-reviewer test only asserts on systemPrompt.

♻️ Proposed fix

  it("should return prompt config for 'ux-reviewer' mode", () => {
    const result = getEnhancementPrompt('ux-reviewer');

    expect(result).toHaveProperty('systemPrompt');
    expect(result).toHaveProperty('description');
    expect(result.systemPrompt).toBe(UX_REVIEWER_SYSTEM_PROMPT);
+   expect(result.description).toContain('user experience');
  });

🤖 Prompt for AI Agents

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

In `@libs/prompts/tests/enhancement.test.ts` around lines 172 - 178, Add the
missing assertion for the description field in the ux-reviewer unit test: after
calling getEnhancementPrompt('ux-reviewer') (in
libs/prompts/tests/enhancement.test.ts) add an expectation that
result.description contains a UX-specific keyword (e.g., use toContain('ux') or
toContain('user experience') so it matches how other mode tests validate
description), while keeping the existing check for UX_REVIEWER_SYSTEM_PROMPT.

apps/ui/src/types/electron.d.ts (1)

54-58: tool_result event carries no result content — ToolUse only exposes invocation fields.

The new tool_result variant reuses ToolUse (name + input), making it structurally identical to tool_use. There is no field to surface the actual tool output. In Anthropic's streaming protocol a tool result carries a distinct result/content payload. If any consumer ever needs to display or process the result value, the type will need to be extended.

♻️ Suggested extension if result content is needed

 export interface ToolUse {
   name: string;
   input: unknown;
 }
+
+export interface ToolResult {
+  name: string;
+  input: unknown;
+  output?: unknown; // result content returned by the tool
+}

 export type StreamEvent =
   ...
   | {
       type: 'tool_result';
       sessionId: string;
-      tool: ToolUse;
+      tool: ToolResult;
     }

🤖 Prompt for AI Agents

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

In `@apps/ui/src/types/electron.d.ts` around lines 54 - 58, The 'tool_result'
event variant currently reuses the ToolUse shape (which only has invocation
fields like name and input) and thus lacks any field for the actual tool output;
update the type definition for the 'tool_result' union variant to include a
result/content payload (e.g., add a 'result' or 'output' field with an
appropriate type such as string | Record<string, unknown> | a dedicated
ToolResult interface) so consumers can access/display the tool output;
specifically modify the union member currently declared with type: 'tool_result'
and sessionId/tool: ToolUse to instead include the new result property or change
tool to a ToolUseWithResult type to preserve backwards-compatible names while
surfacing the tool output.

apps/ui/src/hooks/use-electron-agent.ts (1)

311-315: onToolUse repurposed for tool-result notification via a string-suffix protocol.

onToolUse is called for tool_result events with the tool's input (not result output) as the second argument, distinguished from a true tool invocation only by the " (done)" suffix on the name. Consumers need to string-match the tool name to differentiate invocation from completion, which is fragile.

Consider adding a dedicated optional callback to UseElectronAgentOptions:

♻️ Suggested refactor

 interface UseElectronAgentOptions {
   sessionId: string;
   workingDirectory?: string;
   model?: string;
   thinkingLevel?: string;
   onToolUse?: (toolName: string, toolInput: unknown) => void;
+  onToolResult?: (toolName: string, toolInput: unknown) => void;
 }

-        case 'tool_result':
-          // Tool completed - surface as progress update
-          logger.info('Tool result:', event.tool.name);
-          onToolUse?.(`${event.tool.name} (done)`, event.tool.input);
-          break;
+        case 'tool_result':
+          logger.info('Tool result:', event.tool.name);
+          onToolResult?.(event.tool.name, event.tool.input);
+          break;

🤖 Prompt for AI Agents

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

In `@apps/ui/src/hooks/use-electron-agent.ts` around lines 311 - 315, The handler
for the 'tool_result' event misuses onToolUse by signaling completion via a "
(done)" name suffix and passing event.tool.input instead of the tool output; add
an optional onToolResult callback to UseElectronAgentOptions and invoke it from
the 'tool_result' case (in the same switch that currently logs 'Tool result:'),
passing the actual result/output and any metadata, and leave onToolUse solely
for real invocations; update the type/interface UseElectronAgentOptions, any
call sites that construct the options (to optionally provide onToolResult), and
replace the current onToolUse call in the 'tool_result' branch with a call to
onToolResult while keeping the log line.

apps/server/tests/unit/services/agent-service.test.ts (1)

191-255: Good coverage of the happy path; consider adding a test for the 'unknown' name fallback.

The new test correctly verifies that tool_use_id-to-name resolution works end-to-end. The one gap is the toolName || 'unknown' branch in agent-service.ts — when a tool_result arrives with a tool_use_id that was never registered (e.g., the tool_use block was missing or had no tool_use_id). Adding a second case strengthens the coverage:

🧪 Suggested additional test

it('should emit tool_result with name "unknown" when tool_use_id is not in the map', async () => {
  const mockProvider = {
    getName: () => 'gemini',
    executeQuery: async function* () {
      // Emit tool_result directly without a preceding tool_use
      yield {
        type: 'assistant',
        message: {
          role: 'assistant',
          content: [
            {
              type: 'tool_result',
              tool_use_id: 'unregistered-id',
              content: 'Some result',
            },
          ],
        },
      };
      yield { type: 'result', subtype: 'success' };
    },
  };

  vi.mocked(ProviderFactory.getProviderForModel).mockReturnValue(mockProvider as any);
  vi.mocked(promptBuilder.buildPromptWithImages).mockResolvedValue({
    content: 'Hello',
    hasImages: false,
  });

  await service.sendMessage({ sessionId: 'session-1', message: 'Hello' });

  expect(mockEvents.emit).toHaveBeenCalledWith(
    'agent:stream',
    expect.objectContaining({
      type: 'tool_result',
      tool: expect.objectContaining({ name: 'unknown' }),
    })
  );
});

🤖 Prompt for AI Agents

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

In `@apps/server/tests/unit/services/agent-service.test.ts` around lines 191 -
255, Add a second unit test that exercises the fallback branch where a
tool_result arrives with an unregistered tool_use_id so the code path using
toolName || 'unknown' in agent-service.ts is covered: create a mock provider for
ProviderFactory.getProviderForModel whose executeQuery async generator yields an
assistant message containing a tool_result with tool_use_id 'unregistered-id'
(no preceding tool_use), then a result success; mock
promptBuilder.buildPromptWithImages as in the existing test; call
service.sendMessage({ sessionId: 'session-1', message: 'Hello' }) and assert
mockEvents.emit was called with an 'agent:stream' payload containing type
'tool_result' and tool.name === 'unknown'. Ensure the test references the same
setup helpers and uses the same expect.objectContaining structure as the
existing test.

apps/server/src/services/agent-service.ts (1)

640-653: block.content may be an array — consider normalizing to string before emitting.

Per the Anthropic SDK, tool_result content is typed as string | Array<TextBlock | ImageBlock>. If a provider emits array-valued content, block.content || '' passes the array through as-is, while downstream consumers (UI, tests) expect a string for the content field.

♻️ Optional: normalize content to string

-              content: block.content || '',
+              content: Array.isArray(block.content)
+                ? block.content.map((b: any) => (b.type === 'text' ? b.text : '')).join('')
+                : block.content || '',

🤖 Prompt for AI Agents

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

In `@apps/server/src/services/agent-service.ts` around lines 640 - 653, Normalize
block.content to a string before emitting the 'tool_result' event: in the
tool_result branch inside agent-service where you read block.content (and use
toolUseId/toolNamesById and call this.emitAgentEvent), detect if block.content
is a string and use it directly; if Array.isArray(block.content) map the
elements to extract text fields (e.g., TextBlock.text or TextBlock.content) and
convert non-text/Image blocks (or ImageBlocks) to a sensible string (e.g., image
URL or JSON.stringify) then join parts into a single string; otherwise fallback
to JSON.stringify(block.content) so the emitted payload always has content as a
string.

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

1018-1072: Extract cross-worktree cleanup into a helper to avoid drift.

The same “remove running task from all worktrees” block now appears in handleDeleteFeature, handleForceStopFeature, and handleArchiveAllVerified. A small helper will reduce copy/paste divergence.

♻️ Suggested refactor

+const removeRunningTaskFromAllWorktrees = (projectId: string, taskId: string) => {
+  const store = useAppStore.getState();
+  const prefix = `${projectId}::`;
+  for (const [key, worktreeState] of Object.entries(store.autoModeByWorktree)) {
+    if (key.startsWith(prefix) && worktreeState.runningTasks?.includes(taskId)) {
+      const branchPart = key.slice(prefix.length);
+      const branch = branchPart === '__main__' ? null : branchPart;
+      store.removeRunningTask(projectId, branch, taskId);
+    }
+  }
+};
...
-        if (currentProject) {
-          const store = useAppStore.getState();
-          const prefix = `${currentProject.id}::`;
-          for (const [key, worktreeState] of Object.entries(store.autoModeByWorktree)) {
-            if (key.startsWith(prefix) && worktreeState.runningTasks?.includes(feature.id)) {
-              const branchPart = key.slice(prefix.length);
-              const branch = branchPart === '__main__' ? null : branchPart;
-              store.removeRunningTask(currentProject.id, branch, feature.id);
-            }
-          }
-        }
+        if (currentProject) {
+          removeRunningTaskFromAllWorktrees(currentProject.id, feature.id);
+        }

Apply the same helper in handleDeleteFeature and handleArchiveAllVerified.

🤖 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 1018 - 1072, Extract the repeated "remove running task from ALL worktrees"
logic into a single helper (e.g. removeRunningTaskFromAllWorktrees(projectId:
string, featureId: string)) that calls useAppStore.getState(), reads
store.autoModeByWorktree, computes the prefix `${projectId}::`, iterates
entries, maps '__main__' to null for the branch, and calls
store.removeRunningTask(projectId, branch, featureId) for matching keys; then
replace the duplicated blocks in handleForceStopFeature, handleDeleteFeature,
and handleArchiveAllVerified with calls to this helper (pass currentProject.id
and feature.id or appropriate featureId) to avoid copy/paste drift.

apps/ui/src/components/views/file-editor-view/components/code-editor.tsx (3)

618-675: diffContent in extensions useMemo deps forces a full extension set rebuild on every diff change.

When diffContent changes, the entire extension array (language extension, keybinding, update listener, theme) is rebuilt and reconfigured. While @uiw/react-codemirror preserves undo history via its internal compartment, the unnecessary recreation of stable extensions is wasteful and recreates a new StateField.define() type each time.

The idiomatic CodeMirror approach is to wrap hot-swappable extensions in a Compartment and dispatch compartment.reconfigure(newExtension) on changes, leaving the rest of the configuration untouched.

♻️ Suggested refactor: isolate diff extensions in a Compartment

Hoist a stable Compartment outside the component (module-level or via useRef):

+import { Compartment } from '@codemirror/state';
+
+// Module-level compartment for diff decorations
+const diffCompartment = new Compartment();

In the component, remove diffContent from the extensions useMemo deps and instead use a separate useEffect:

   const extensions = useMemo(() => {
     // ... (remove diffContent-related logic)
     exts.push(diffCompartment.of([]));  // placeholder initially
     return exts;
-  }, [filePath, wordWrap, tabSize, editorTheme, diffContent]);
+  }, [filePath, wordWrap, tabSize, editorTheme]);

+  // Hot-swap only the diff decorations when diffContent changes
+  useEffect(() => {
+    const view = editorRef.current?.view;
+    if (!view) return;
+    view.dispatch({
+      effects: diffCompartment.reconfigure(createDiffDecorations(diffContent)),
+    });
+  }, [diffContent]);

Note: If diffCompartment is module-level, ensure it's per-instance (via useRef) if multiple CodeEditor instances can coexist.

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/code-editor.tsx`
around lines 618 - 675, The useMemo for extensions rebuilds the whole extension
array whenever diffContent changes; instead create a Compartment (e.g.,
diffCompartment via useRef or module-level per-instance) and remove diffContent
from the dependencies of the extensions useMemo so stable extensions
(syntaxHighlighting, editorTheme, search, EditorView.updateListener, keymap,
lineWrapping, editorAttributes, langExt) are not recreated; then in a useEffect
watching diffContent, call
diffCompartment.current.reconfigure(createDiffDecorations(diffContent) ?? []) to
update only the diff decorations (refer to createDiffDecorations, extensions
useMemo, diffContent, and diffCompartment).

---

238-253: Hardcoded OKLCH colors in DeletedLinesWidget.toDOM() bypass the CSS variable theme system.

All other editor styling uses var(--chart-*) / var(--foreground) / etc. The inline oklch(...) values here (and addedLineDecoration at line 280) won't adapt if the host app applies a custom palette via CSS variables. If the design system exposes semantic tokens for destructive/success colours, prefer those — e.g., var(--destructive) / var(--success) or equivalent design tokens.

♻️ Suggested refactor: use CSS variables

-    container.style.cssText =
-      'background-color: oklch(0.55 0.22 25 / 0.1); border-left: 3px solid oklch(0.55 0.22 25 / 0.5);';
+    container.style.cssText =
+      'background-color: color-mix(in oklab, var(--destructive) 10%, transparent); border-left: 3px solid color-mix(in oklab, var(--destructive) 50%, transparent);';

     for (const line of this.lines) {
       const lineEl = document.createElement('div');
-      lineEl.style.cssText =
-        'text-decoration: line-through; color: oklch(0.55 0.22 25 / 0.8); background-color: oklch(0.55 0.22 25 / 0.15); padding: 0 0.5rem; padding-left: calc(0.5rem - 3px); white-space: pre; font-family: inherit;';
+      lineEl.style.cssText =
+        'text-decoration: line-through; color: color-mix(in oklab, var(--destructive) 80%, transparent); background-color: color-mix(in oklab, var(--destructive) 15%, transparent); padding: 0 0.5rem; padding-left: calc(0.5rem - 3px); white-space: pre; font-family: inherit;';

 const addedLineDecoration = Decoration.line({
   class: 'cm-diff-added-line',
-  attributes: { style: 'background-color: oklch(0.65 0.2 145 / 0.15);' },
+  attributes: { style: 'background-color: color-mix(in oklab, var(--success, oklch(0.65 0.2 145)) 15%, transparent);' },
 });

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/code-editor.tsx`
around lines 238 - 253, DeletedLinesWidget.toDOM currently hardcodes oklch(...)
colors (and similarly addedLineDecoration near addedLineDecoration) which bypass
the app's CSS variable theme; update the widget to use semantic CSS variables
instead by replacing inline oklch(...) values with var(...) tokens (e.g.,
var(--destructive), var(--destructive-foreground), var(--background-destructive)
or the project's equivalent) and/or move visual rules into a CSS class
(cm-diff-deleted-widget and child line elements) so theming is applied via CSS;
ensure you update both the container.style.cssText and each lineEl.style.cssText
usage in DeletedLinesWidget.toDOM and the addedLineDecoration styling to
reference the semantic variables or class names rather than hardcoded colors.

---

283-301: EditorView.decorations.of(fn) is called on every view update — consider a StateField for consistency.

The official CodeMirror 6 docs state that decoration functions provided via EditorView.decorations.of(fn) are called after the new viewport is computed on every view update, and must not introduce block widgets or replacing decorations that cover line breaks. This means the closure iterates addedLines on every keystroke, cursor move, and scroll — whereas the deleted-lines side correctly uses a StateField that only recomputes on document changes.

For large diffs the per-update iteration is wasteful. Aligning the added-line decorations with the same StateField pattern used for the deleted-widget block (lines 344–357) would restrict recomputation to document changes only.

♻️ Suggested refactor: StateField for added-line decorations

-  // Line decorations for added lines
-  if (addedLines.size > 0) {
-    extensions.push(
-      EditorView.decorations.of((view) => {
-        const builder = new RangeSetBuilder<Decoration>();
-        const doc = view.state.doc;
-
-        for (const lineNum of addedLines) {
-          if (lineNum >= 1 && lineNum <= doc.lines) {
-            const linePos = doc.line(lineNum).from;
-            builder.add(linePos, linePos, addedLineDecoration);
-          }
-        }
-
-        return builder.finish();
-      })
-    );
-  }
+  // Line decorations for added lines via StateField (only recomputes on docChanged)
+  if (addedLines.size > 0) {
+    const buildAddedDecorations = (doc: { lines: number; line(n: number): { from: number } }) => {
+      const builder = new RangeSetBuilder<Decoration>();
+      for (const lineNum of addedLines) {
+        if (lineNum >= 1 && lineNum <= doc.lines) {
+          builder.add(doc.line(lineNum).from, doc.line(lineNum).from, addedLineDecoration);
+        }
+      }
+      return builder.finish();
+    };
+    extensions.push(
+      StateField.define({
+        create(state) { return buildAddedDecorations(state.doc); },
+        update(decorations, tr) {
+          return tr.docChanged ? decorations.map(tr.changes) : decorations;
+        },
+        provide: (f) => EditorView.decorations.from(f),
+      })
+    );
+  }

🤖 Prompt for AI Agents

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

In `@apps/ui/src/components/views/file-editor-view/components/code-editor.tsx`
around lines 283 - 301, The current added-line decorations are recomputed on
every view update because they use EditorView.decorations.of(fn) and iterate
addedLines in the closure; replace this with a StateField that computes and
stores a DecorationSet from addedLines and the document (similar to the existing
StateField used for deleted-line widgets) so decorations only recompute on
document/addedLines changes. Implement a StateField (e.g.
addedLineDecorationField) whose create/update builds a RangeSetBuilder using
addedLines and addedLineDecoration against view.state.doc, export that field and
then push EditorView.decorations.of((state) =>
state.field(addedLineDecorationField)) into extensions instead of the current
inline function.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx (1)

561-586: ⚠️ Potential issue | 🟡 Minor

sliderPrevRef still gets overwritten with intermediate values during a drag.

sliderPrevRef.current is unconditionally reassigned on every onValueChange call. Because Radix UI fires onValueChange on each mouse-move (separate React events, not a single batched transaction), the component re-renders between calls and pinnedWorktreesCount advances to the intermediate store value. The second onValueChange therefore captures that intermediate value as prevCount and overwrites the ref — so by onValueCommit, sliderPrevRef.current holds the value from just before the last step, not the original pre-drag position.

Additionally, the ref is never reset to null after a commit (success or failure), leaving stale data across drag sessions.

🛠️ Proposed fix — capture-once + reset after commit

  onValueChange={(value) => {
-   // Capture previous value before mutation for potential rollback
-   const prevCount = pinnedWorktreesCount;
-   // Update local state immediately for visual feedback
+   // Capture the pre-drag value only on the first onValueChange of each drag
+   if (sliderPrevRef.current === null) {
+     sliderPrevRef.current = pinnedWorktreesCount;
+   }
    const newValue = value[0] ?? pinnedWorktreesCount;
    setPinnedWorktreesCount(project.path, newValue);
-   // Store prev for onValueCommit rollback
-   sliderPrevRef.current = prevCount;
  }}
  onValueCommit={async (value) => {
    const newValue = value[0] ?? pinnedWorktreesCount;
    const prev = sliderPrevRef.current ?? pinnedWorktreesCount;
+   sliderPrevRef.current = null; // Reset for the next drag session
    try {
      const httpClient = getHttpApiClient();
      await httpClient.settings.updateProject(project.path, {
        pinnedWorktreesCount: newValue,
      });
    } catch (error) {
      console.error('Failed to persist pinnedWorktreesCount:', error);
      toast.error('Failed to save pinned worktrees setting');
      setPinnedWorktreesCount(project.path, prev);
    }
  }}

🤖 Prompt for AI Agents

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

In
`@apps/ui/src/components/views/project-settings-view/worktree-preferences-section.tsx`
around lines 561 - 586, The ref sliderPrevRef is being overwritten on every
onValueChange causing it to hold intermediate values; change onValueChange so it
only captures the starting value once (i.e., set sliderPrevRef.current =
pinnedWorktreesCount only if sliderPrevRef.current == null) and continue to
optimistically call setPinnedWorktreesCount for visual feedback; in
onValueCommit read prev = sliderPrevRef.current ?? pinnedWorktreesCount, persist
via getHttpApiClient().settings.updateProject, and then always reset
sliderPrevRef.current = null after the commit completes (both in the try success
path and in the catch rollback path that calls
setPinnedWorktreesCount(project.path, prev)) so the ref doesn't leak stale state
across drag sessions.