feat: version control port and open-project fix#728
Merged
Conversation
… PR #348 Port comprehensive performance optimizations that reduce main-thread CPU usage during simulator debugging. Shared surfaces synced byte-identical with openplc-web. - Targeted Zustand selectors for all debug-aware components - BOOL/non-BOOL value split so canvas only re-renders on BOOL changes - Change-driven store writes (zero updates when values unchanged) - Re-render cascade elimination via targeted selectors in layout tree - buildActiveIndexSet memoization (O(1) per poll tick instead of O(n)) - Redundant setDebugDataStale(false) guard - Move backend/styles to backend/shared/styles Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sync byte-identical surfaces with web repo's version control feature. Add no-op editor version control adapter (guarded by hasVersionControl: false). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Syncs byte-identical surfaces from web, aligns shared package versions, and migrates zustand shallow equality to v5 useShallow API. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The openProject adapter called pathPicker which delegates to getProjectPath — a function that rejects non-empty directories. Existing projects always have files, so opening always silently failed. Add a dedicated getOpenProjectPath that validates project.json exists instead of requiring an empty directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 52 minutes and 4 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughAdds a version-control feature (ports, UI, store slice) and editor integrations; splits debugger state into bool/non-bool maps with new hooks/services; introduces graphical diff/read-only node visuals and many editor visuals; adds IPC/path-picker changes, global CSS tweaks, dependency bumps, and assorted UI/layout updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Branch<br/>StatusBar / Modals
participant VC as VersionControl<br/>Port (Platform)
participant Project as Project<br/>Service
participant Store as Zustand<br/>Store
User->>UI: request branch switch to X
UI->>VC: getChanges(projectId, activeBranch)
VC-->>UI: returns changes (hasChanges?)
alt hasChanges
UI->>User: show UnsavedChangesWarningModal
User->>UI: Confirm discard & switch
UI->>VC: discardChanges(projectId, paths?)
VC-->>UI: success
end
UI->>VC: switchBranch(projectId, X)
VC-->>UI: success
UI->>Project: openProjectByPath(projectPath)
Project-->>UI: project data
UI->>Store: sharedWorkspaceActions.handleOpenProjectResponse(data)
Store-->>UI: state updated (active branch, project loaded)
UI-->>User: branch switched, editor updated
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/hooks/useDebugPolling.ts (1)
198-232:⚠️ Potential issue | 🟠 MajorParse failures still dirty the store on every poll.
The normal path only writes when a value changed, but the catch path always writes
'ERR'. Once a variable becomes permanently unparseable, every poll tick still updates the map and wakes downstream subscribers again.Suggested fix
} catch { - ;(isBool ? changedBool : changedNonBool).set(meta.compositeKey, 'ERR') + const current = isBool ? currentBool : currentNonBool + if (current.get(meta.compositeKey) !== 'ERR') { + ;(isBool ? changedBool : changedNonBool).set(meta.compositeKey, 'ERR') + } bufferOffset += typeSize }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/hooks/useDebugPolling.ts` around lines 198 - 232, The catch block currently unconditionally inserts 'ERR' into changedBool/changedNonBool which causes repeated writes; change it to compare the existing stored value (use currentBool/currentNonBool keyed by meta.compositeKey) and only set 'ERR' into (isBool ? changedBool : changedNonBool) if the stored value differs, keeping the bufferOffset += typeSize and itemsProcessed logic unchanged so downstream workspaceActions.setDebugBoolValues/setDebugNonBoolValues are only called when the value actually changed.
🟡 Minor comments (9)
src/frontend/components/_atoms/graphical-editor/ladder/variable-visual.tsx-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorNarrow
variantto a literal union type.
variant?: stringis overly permissive for logic that only supports'input'and'output'. Tightening this tovariant?: 'input' | 'output'improves type-safety and catches invalid usage at compile time, per the strict TypeScript guidelines for this file.Proposed fix
export type VariableVisualProps = { variableName: string /** Placeholder text shown when variable name is empty, e.g. "(*STRING*)" */ placeholder?: string /** 'input' or 'output' -- controls text alignment */ - variant?: string + variant?: 'input' | 'output' inputError?: boolean isAVariable?: boolean isForced?: boolean🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_atoms/graphical-editor/ladder/variable-visual.tsx` at line 11, The prop/type for "variant" is currently too permissive (variant?: string); narrow it to the literal union variant?: 'input' | 'output' wherever it's declared (e.g., the props interface or type used by VariableVisual component) and update any usages or switch/case checks in VariableVisual to rely on those two literals; this change will enforce compile-time checks and reveal any callsites passing invalid values so you can correct them to 'input' or 'output'.src/frontend/components/_features/[workspace]/source-control/source-control-panel.tsx-47-58 (1)
47-58:⚠️ Potential issue | 🟡 MinorPending changes indicator invisible when "Changes" tab is active.
When
activeView === 'changes', the button has a blue background (bg-blue-500), but the indicator dot also usesbg-blue-500. This makes the dot invisible against the same-colored background. Consider using a contrasting color (e.g., white) when the tab is active.🎨 Suggested fix
Changes - {pendingChangesCount > 0 && <span className='ml-1 inline-block h-1.5 w-1.5 rounded-full bg-blue-500' />} + {pendingChangesCount > 0 && ( + <span + className={cn( + 'ml-1 inline-block h-1.5 w-1.5 rounded-full', + activeView === 'changes' ? 'bg-white' : 'bg-blue-500', + )} + /> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/source-control-panel.tsx around lines 47 - 58, The pending changes dot uses bg-blue-500 and becomes invisible when activeView === 'changes' (button has bg-blue-500); update the span rendering for pendingChangesCount in the source-control-panel component to choose a contrasting color when the "Changes" tab is active (e.g., use bg-white when activeView === 'changes', otherwise bg-blue-500) by making the className conditional based on activeView (refer to activeView, setActiveView, and pendingChangesCount to locate the button and indicator span) so the dot remains visible in both states.src/frontend/components/_features/[workspace]/source-control/history-section.tsx-83-84 (1)
83-84:⚠️ Potential issue | 🟡 MinorPagination label may display incorrectly when commits array is empty.
When
commits.length === 0butoffset > 0(e.g., after an error on a subsequent page), the label would show something like "51–100 of 0" which is misleading. Consider guarding against this edge case or ensuring offset resets on error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/history-section.tsx around lines 83 - 84, The pagination label can show incorrect ranges when commits.length === 0 but offset > 0 (e.g. "51–100 of 0"); update the label computation in the HistorySection component to guard for empty results by computing start and end using total (or commits.length) so when total === 0 it renders "0–0 of 0" or a similar empty-state label: e.g., compute start = total === 0 ? 0 : offset + 1 and end = total === 0 ? 0 : Math.min(offset + PAGE_SIZE, total), or reset offset to 0 on empty/error responses; adjust the JSX that currently uses {offset + 1}–{Math.min(offset + PAGE_SIZE, total)} of {total} to use these guarded values (references: commits, offset, PAGE_SIZE, total, HistorySection).src/frontend/components/_features/[workspace]/source-control/modals/restore-confirmation-modal.tsx-33-33 (1)
33-33:⚠️ Potential issue | 🟡 MinorSame backdrop/Escape inconsistency as DeleteBranchModal.
The Escape key handler checks
!isLoadingbefore callingonCancel(), but the backdrop'sonClickalways callsonCancel(). This allows users to dismiss during restore via backdrop but not via keyboard.Proposed fix
- <div className='absolute inset-0 bg-black/40' onClick={onCancel} /> + <div className='absolute inset-0 bg-black/40' onClick={isLoading ? undefined : onCancel} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/modals/restore-confirmation-modal.tsx at line 33, Backdrop click currently always calls onCancel while the Escape key handler guards with !isLoading, causing inconsistent dismissal behavior; update the backdrop's onClick handler in restore-confirmation-modal (the div with className 'absolute inset-0 bg-black/40') to only call onCancel when !isLoading (i.e., if (!isLoading) onCancel()), so it mirrors the Escape-key logic used elsewhere in this component.src/frontend/components/_features/[workspace]/branches/delete-branch-modal.tsx-48-48 (1)
48-48:⚠️ Potential issue | 🟡 MinorBackdrop click allows close during pending state, but Escape does not.
The
Escapekey handler on line 22 checks!isPendingbefore callingonClose(), but the backdrop'sonClickalways callsonClose(). This creates inconsistent behavior where users can dismiss the modal via backdrop click during deletion but not via keyboard.Consider guarding the backdrop click as well:
Proposed fix
- <div className='absolute inset-0 bg-black/40' onClick={onClose} /> + <div className='absolute inset-0 bg-black/40' onClick={isPending ? undefined : onClose} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/branches/delete-branch-modal.tsx at line 48, The backdrop's onClick currently always calls onClose, causing inconsistent behavior with the Escape key handler which only calls onClose when !isPending; update the backdrop handler in delete-branch-modal.tsx so it guards against pending state (check !isPending before invoking onClose) to match the Escape key logic and prevent dismissal during deletion (reference the onClose prop and isPending state used in the component).src/frontend/components/_features/[workspace]/source-control/changes-section.tsx-327-333 (1)
327-333:⚠️ Potential issue | 🟡 MinorReset the pending-changes badge on refresh failure.
On fetch errors you clear
files, butversionControlActions.setPendingChangesCount(...)is only updated on success. The activity-bar badge can keep showing the previous count while this panel says there are no changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx around lines 327 - 333, When the fetch in refresh (which calls versionControl.getChanges) fails you clear the UI files with setFiles([]) but don't reset the activity-bar badge; update the catch block to also call versionControlActions.setPendingChangesCount(0) so the pending changes count is cleared on error (ensure you still setIsLoading(false) in finally). Target the async refresh flow where setFiles, versionControl.getChanges, and versionControlActions.setPendingChangesCount are used.src/frontend/screens/workspace-screen.tsx-501-506 (1)
501-506:⚠️ Potential issue | 🟡 MinorRemove the duplicated bottom resize handle.
When
tabs.length > 0, both of these handles render at the same edge with the same id. That gives you overlapping drag targets and duplicate DOM ids for the same splitter.Also applies to: 513-518
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/screens/workspace-screen.tsx` around lines 501 - 506, There are two overlapping ResizableHandle elements both using id 'consoleResizeHandle' (one at the shown block and another at the block referenced 513-518), causing duplicate DOM ids and overlapping drag targets; fix by rendering only one handle: remove or conditionalize one of the ResizableHandle renders so only a single ResizableHandle with id 'consoleResizeHandle' is present (e.g., wrap one instance in a guard like tabs.length === 0 or migrate shared props into a single centralized ResizableHandle render), ensuring the unique id and props remain on the sole ResizableHandle component.src/frontend/components/_features/[workspace]/source-control/changes-section.tsx-314-320 (1)
314-320:⚠️ Potential issue | 🟡 MinorCompare folder identities, not just the set size.
Replacing one folder tree with another of the same cardinality will skip this update, so newly introduced folders stay collapsed while stale expansion state is retained.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx around lines 314 - 320, The effect currently only compares folderKey.size to prevFolderKeyRef.current.size which misses changes when two different sets have the same cardinality; update the useEffect to perform a set-content comparison (e.g., check sizes and then verify every item in folderKey exists in prevFolderKeyRef.current) and only skip when the sets are truly equal; when detecting a difference call setExpandedFolders(folderKey) and store a defensive copy into prevFolderKeyRef.current (e.g., new Set(folderKey)) so identity and future comparisons behave correctly; modify the symbols prevFolderKeyRef, folderKey, setExpandedFolders and the useEffect body accordingly.src/frontend/components/_features/[workspace]/branches/branch-switcher-modal.tsx-31-40 (1)
31-40:⚠️ Potential issue | 🟡 MinorIgnore outdated branch fetches.
If the modal is reopened or
projectIdchanges before an earlierlistBranches()call settles, the late response can overwrite the current branch list with stale data. Add a cancellation/request-token guard before committingsetBranches/setIsLoading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/branches/branch-switcher-modal.tsx around lines 31 - 40, The current effect can commit stale results from prior versionControl.listBranches calls; fix it by adding a cancellation guard (e.g., a useRef requestId or a local "cancelled" flag set in the effect cleanup) and capture the current token before calling versionControl.listBranches(projectId); only call setBranches and setIsLoading in the .then/.catch/.finally handlers if the token matches (or cancelled is false). Update the effect that references isOpen, projectId, versionControl, setBranches and setIsLoading to create/advance the token on each run and invalidate it in the cleanup so late responses won't overwrite newer state.
🧹 Nitpick comments (8)
src/frontend/components/_features/[workspace]/editor/monaco/ai-consent-modal.tsx (1)
20-24: Consider explicitly closing chat on decline for state consistency.Line 20–24 clears consent, but it does not reset chat-open state. Setting it false here avoids stale UI state in edge flows.
Suggested adjustment
const handleDecline = () => { localStorage.setItem('ai-consent-v1', 'declined') setAIConsented(false) + setChatOpen(false) onOpenChange('ai-consent', false) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/editor/monaco/ai-consent-modal.tsx around lines 20 - 24, The handleDecline function updates consent but can leave the chat UI open; update handleDecline to also explicitly close the chat by calling the same open-state updater used elsewhere (e.g., onOpenChange) with the chat identifier (for example call onOpenChange('chat', false) or the equivalent) after setting localStorage and setAIConsented(false) so chat-open state cannot remain stale when consent is declined.src/frontend/components/_features/[workspace]/source-control/source-control-panel.tsx (1)
21-21: UnnecessaryuseCallbackwrapper for static selector.The selector
(s) => s.versionControl.pendingChangesCountis a simple property accessor with no closure dependencies. Wrapping it inuseCallbackwith an empty dependency array adds overhead without benefit since the function body is already stable.♻️ Suggested simplification
- const pendingChangesCount = useOpenPLCStore(useCallback((s) => s.versionControl.pendingChangesCount, [])) + const pendingChangesCount = useOpenPLCStore((s) => s.versionControl.pendingChangesCount)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/source-control-panel.tsx at line 21, Remove the unnecessary useCallback wrapper around the selector passed to useOpenPLCStore: replace the current call that uses useCallback((s) => s.versionControl.pendingChangesCount, []) with a direct selector function (s => s.versionControl.pendingChangesCount) when calling useOpenPLCStore to eliminate the extra wrapper; update the declaration for pendingChangesCount accordingly so it reads useOpenPLCStore(s => s.versionControl.pendingChangesCount).src/frontend/components/_features/[workspace]/source-control/commit-item.tsx (1)
10-20: Consider handling edge cases informatRelativeTime.The helper works well for typical cases, but consider:
- Invalid timestamps would produce
NaNin calculations, resulting in unexpected output.- For commits older than ~30 days, an absolute date might be more readable (e.g., "Mar 15, 2026").
This is a minor UX consideration and can be addressed in a follow-up if needed.
♻️ Optional enhancement with fallback
function formatRelativeTime(timestamp: string): string { - const diff = Date.now() - new Date(timestamp).getTime() + const date = new Date(timestamp) + if (isNaN(date.getTime())) return 'unknown' + const diff = Date.now() - date.getTime() const minutes = Math.floor(diff / 60000) const hours = Math.floor(diff / 3600000) const days = Math.floor(diff / 86400000) if (minutes < 1) return 'just now' if (minutes < 60) return `${minutes} minute${minutes > 1 ? 's' : ''} ago` if (hours < 24) return `${hours} hour${hours > 1 ? 's' : ''} ago` + if (days > 30) return date.toLocaleDateString(undefined, { month: 'short', day: 'numeric', year: 'numeric' }) return `${days} day${days > 1 ? 's' : ''} ago` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/commit-item.tsx around lines 10 - 20, The formatRelativeTime function should validate the input timestamp and provide an absolute date for older commits: first, in formatRelativeTime, check that new Date(timestamp).getTime() is a valid number (not NaN) and return a safe fallback like 'unknown' or the original timestamp when invalid; then compute diff as now - ts and if diff >= 30 days (30 * 86400000) return a locale-formatted absolute date (e.g., using toLocaleDateString or a short month/day/year format like "Mar 15, 2026"); keep the existing relative outputs for minutes/hours/days < 30 days. Ensure you update formatRelativeTime only (reference function name formatRelativeTime) and add no external deps.src/frontend/components/_features/[workspace]/branches/unsaved-changes-warning-modal.tsx (1)
28-30: Consider adding accessibility attributes to the modal.The modal works correctly, but adding ARIA attributes would improve screen reader support.
♻️ Optional accessibility enhancement
return ( - <div className='fixed inset-0 z-50 flex items-center justify-center'> - <div className='absolute inset-0 bg-black/40' onClick={onCancel} /> - <div className='relative w-80 rounded-lg border border-neutral-200 bg-white p-5 shadow-xl dark:border-neutral-700 dark:bg-neutral-900'> + <div className='fixed inset-0 z-50 flex items-center justify-center' role='dialog' aria-modal='true' aria-labelledby='unsaved-changes-title'> + <div className='absolute inset-0 bg-black/40' onClick={onCancel} aria-hidden='true' /> + <div className='relative w-80 rounded-lg border border-neutral-200 bg-white p-5 shadow-xl dark:border-neutral-700 dark:bg-neutral-900'> + <h3 id='unsaved-changes-title' className='mb-2 text-sm font-semibold text-neutral-900 dark:text-neutral-100'>Unsaved Changes</h3> - <h3 className='mb-2 text-sm font-semibold text-neutral-900 dark:text-neutral-100'>Unsaved Changes</h3>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/branches/unsaved-changes-warning-modal.tsx around lines 28 - 30, The modal lacks ARIA attributes—update the modal container in the UnsavedChangesWarningModal component to include role="dialog" and aria-modal="true", add aria-labelledby that points to the modal title element id and aria-describedby that points to the descriptive text id, ensure the title and description elements have matching ids, and wire an onKeyDown handler to the modal wrapper to close on Escape (using the existing onCancel prop); additionally ensure initial focus is moved into the modal (focus the first actionable element) or add a focus trap so keyboard and screen-reader users can interact properly.src/frontend/components/_atoms/graphical-editor/diff/diff-wrapper.tsx (1)
22-35: Consider defining a minimal handle type for better type safety.Both
renderHandlesandrenderFBDHandlesuseunknownfor the handles parameter and rely on type assertions. While this is pragmatic for the diff context, a minimal shared type could improve safety:Optional: Add minimal type definition
+type DiffHandle = { + id: string + type: 'source' | 'target' + position: Position + style?: React.CSSProperties + className?: string +} -export function renderHandles(handles: unknown) { +export function renderHandles(handles: unknown): React.ReactNode { if (!Array.isArray(handles)) return null - return handles.map((h, i) => ( + return (handles as DiffHandle[]).map((h, i) => ( <Handle key={i} id={h.id} - type={h.type as 'source' | 'target'} - position={h.position as Position} + type={h.type} + position={h.position} style={h.style} className='opacity-0' isConnectable={false} /> )) }Also applies to: 37-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_atoms/graphical-editor/diff/diff-wrapper.tsx` around lines 22 - 35, renderHandles and renderFBDHandles accept handles: unknown and use unsafe type assertions; define a minimal shared Handle type (e.g., properties id: string, type: 'source'|'target', position: Position, style?: React.CSSProperties) and change the parameter types to that interface for both functions, then remove the assertions and use the typed properties (referencing renderHandles, renderFBDHandles and the props id, type, position, style passed into Handle) so the compiler enforces shape safety.src/frontend/components/_atoms/graphical-editor/fbd/variable-visual.tsx (1)
66-72: Class precedence for conflicting states could be clearer.When multiple boolean flags are true simultaneously (e.g.,
inputErrorandisForced), multiple text color classes may apply. Tailwind's last-wins behavior means the forced colors would overridetext-red-500, which might be the intended behavior, but this implicit precedence could cause confusion.If the forced state should always take visual precedence over error state, consider making this explicit:
Optional: Make precedence explicit
<span className={cn('w-full truncate text-center text-xs leading-3', { 'text-yellow-500': !isAVariable, - 'text-red-500': inputError, + 'text-red-500': inputError && !isForced, 'font-bold': isForced, 'text-[`#80C000`]': isForced && forcedValue, 'text-[`#4080FF`]': isForced && !forcedValue, })} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_atoms/graphical-editor/fbd/variable-visual.tsx` around lines 66 - 72, The className composition on VariableVisual (the cn(...) call) can produce conflicting text color classes when inputError and isForced are both true; make precedence explicit by computing a single color class before rendering: in the component that uses isAVariable, inputError, isForced, forcedValue and cn, derive a colorClass variable that selects forced colors when isForced (choose text-[`#80C000`] or text-[`#4080FF`] based on forcedValue), else text-red-500 when inputError, else text-yellow-500 when !isAVariable, and pass that single colorClass into the cn(...) call so forced state always overrides error state unambiguously.src/frontend/components/_features/[workspace]/source-control/history-section.tsx (1)
36-39: Silent error handling may confuse users.When
listCommitsfails, the catch block silently clears commits and total without notifying the user. On a non-first page, this could leave users seeing an empty list with confusing pagination ("51-100 of 0"). Consider showing a toast or inline error message.Proposed improvement
.catch(() => { setCommits([]) setTotal(0) + // Optionally reset to first page and/or show error feedback + setOffset(0) })Or add a toast notification to inform users of the failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/history-section.tsx around lines 36 - 39, The catch block for listCommits currently swallows errors and calls setCommits([]) and setTotal(0); instead, preserve existing commit/total state (do not clear them on error), and surface the failure to the user by triggering a toast or setting an inline error state (e.g., call showToast/errorToast or setErrorMessage) inside the catch, while also logging the caught error; if you must clear state only do so when on the first page (check current page variable) so pagination doesn't show inconsistent "51-100 of 0" results. Ensure you update the catch attached to listCommits in the HistorySection component and reference setCommits, setTotal, listCommits, and the page/currentPage variable when making the change.package.json (1)
78-80: React 18.3.1 is a bridge version for React 19 migration.React 18.3.x is identical to 18.2.x but adds deprecation warnings for APIs removed or changed in React 19. The warnings cover string refs, legacy context (contextTypes, getChildContext), defaultProps on function components, findDOMNode, unmountComponentAtNode, and key spreading in JSX. These warnings appear in development mode and identify code that needs updates before upgrading to React 19.
Ensure the codebase is tested for new warnings and that deprecated patterns are refactored as needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 78 - 80, The package.json pins "react" and "react-dom" to 18.3.1 (a bridge release that emits deprecation warnings for React 19), so run the app in development and sweep the codebase for the deprecated patterns called out (string refs, legacy context: contextTypes/getChildContext, defaultProps on function components, findDOMNode/unmountComponentAtNode, and JSX key spreading) and refactor the offending code; specifically search references to "react" and "react-dom" in package.json and then locate components using string refs, contextTypes/getChildContext, defaultProps on functions, findDOMNode/unmountComponentAtNode, or key spread and replace them with modern patterns (useRef/useEffect, new Context API, default parameters or explicit prop handling, ref forwarding/DOM refs, and explicit key assignment) and re-run unit/e2e tests and dev build to ensure no deprecation warnings remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/shared/styles/globals.css`:
- Around line 156-158: Replace the non-standard overflow-y value in the
.sidebar-scroll rule: change overflow-y: overlay to overflow-y: auto to avoid
lint failures and browser compatibility issues; update the CSS rule for
.sidebar-scroll accordingly so scrollbar behavior uses the standard auto value.
In `@src/frontend/components/_atoms/graphical-editor/diff/ladder-nodes.tsx`:
- Around line 19-28: Several read-only ladder node components
(ReadOnlyPowerRail, ReadOnlyVariable, ReadOnlyParallel, ReadOnlyPlaceholder,
ReadOnlyMockNode) render their SVG/markup directly and therefore ignore
diffStatus; wrap each component's output with the existing DiffWrapper (or
otherwise apply the same diff handling) so add/remove/modify states are
reflected in the diff view. Locate each component (e.g., ReadOnlyPowerRail and
its use of renderHandles) and replace the top-level fragment with <DiffWrapper
status={data.diffStatus}>…</DiffWrapper> (or pass the appropriate diffStatus
prop/name used by DiffWrapper) and keep existing content and renderHandles
inside it so visual output and handles remain unchanged while diff styling is
applied.
In
`@src/frontend/components/_features/`[workspace]/branches/branch-switcher-modal.tsx:
- Around line 60-62: The modal lacks dialog/listbox semantics and uses clickable
<div>s for branches, breaking keyboard access; update the BranchSwitcherModal
component to expose the modal container as a proper dialog (role="dialog" or use
a dialog component), add aria-modal="true" and an accessible label, trap focus
when opened and restore focus on close (use onClose prop already present), and
convert each branch row (currently clickable <div>s) into keyboard-focusable
controls (either <button> for selection or an element with role="option" inside
a <div role="listbox">) with proper aria-selected, tabIndex, and onKeyDown
handlers to support Enter/Space activation and Arrow key navigation; also ensure
delete controls are real buttons with accessible labels and do not interfere
with row activation (use stopPropagation where necessary).
In
`@src/frontend/components/_features/`[workspace]/branches/create-branch-modal.tsx:
- Around line 76-115: The modal is implemented as raw divs which lack dialog
semantics and still allows clicking the backdrop (the absolute inset-0
bg-black/40) to call onClose even when isPending, causing an accessibility and
race condition; replace this markup with the app's shared Dialog
primitive/component (instead of the outer fixed/inset divs) and move the
backdrop and panel into that Dialog so it provides ARIA roles and focus
management, wire the Dialog's open/onOpenChange (or equivalent) to the component
state so you can block outside-dismiss while isPending (i.e., prevent closing
when isPending), and ensure handleSubmit, isPending and onClose are used with
the Dialog's confirm/cancel controls so in-flight submissions cannot be
dismissed by outside clicks or escape.
In `@src/frontend/components/_features/`[workspace]/editor/graphical/index.tsx:
- Around line 24-28: The UI currently only blocks interaction visually, but you
must enforce immutability in the editor logic by passing the readOnly flag into
the editor implementation and mutation paths: add a readOnly prop to
EditorComponent and thread it into its concrete editors and hooks (e.g., any
useEditorState/useEditorStore, onChange handlers, handleMutation/commitChanges,
onDrag/onDrop, keyboard/global command handlers, and save/commit functions) so
those codepaths early-return or disable edits when readOnly is true; ensure
context menus/command palette handlers also check readOnly before performing
mutations.
In `@src/frontend/components/_features/`[workspace]/editor/monaco/index.tsx:
- Around line 960-995: The ai-code-applied handler can be removed when
PrimitiveEditor is disposed (in editorInstance.onDidDispose) causing race loss
when handleDiffAccept dispatches ai-review-accepted while the editor is swapped
to DiffEditor; to fix, stop registering/removing the 'ai-code-applied' listener
inside PrimitiveEditor — move the registration of handleCodeApplied out to the
parent scope (so it persists across the DiffEditor <-> PrimitiveEditor swap) or
otherwise centralize it so handleCodeApplied remains active when
handleDiffAccept dispatches ai-review-accepted; keep handleCodeReview and the
temporary removal of other listeners as-is but ensure handleCodeApplied (and its
use of editorInstance.getModel / editorInstance.executeEdits) is guarded for
null editorInstance and applied when the editor remounts if necessary.
- Around line 973-983: handleCodeApplied currently edits the Monaco model
directly but suppresses the normal onChange -> handleWriteInPou() flow via
isSyncingModelRef, leaving localText, the project state (updatePou), and the
unsaved flag stale; after executing the edits in handleCodeApplied you must also
update the component/project state to mirror the change: call the same updater
used by onChange (e.g., invoke handleWriteInPou or updatePou with name and
body), update localText to body, and set the unsaved-state flag appropriately
(e.g., mark as dirty) so the UI and persisted state remain consistent while
still using isSyncingModelRef to avoid double-handling.
In
`@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx:
- Around line 522-545: The success path after calling
versionControl.discardChanges never resets the loading flag: call
setIsDiscarding(false) once the discard flow completes successfully (after
closing the modal and reloading data). Update the block that currently handles
the success flow (the try block that awaits versionControl.discardChanges, calls
setShowDiscardModal(false), awaits projectPort.openProjectByPath and
fetchChanges) to ensure setIsDiscarding(false) is invoked (e.g., after
fetchChanges or in a finally-like continuation) so the component is not left
stuck in the discarding state.
In
`@src/frontend/components/_features/`[workspace]/source-control/modals/discard-confirmation-modal.tsx:
- Line 39: The backdrop's onClick calls onCancel unconditionally, allowing the
modal to close while a discard is in progress; update the backdrop handler in
DiscardConfirmationModal (the div with onClick currently bound to onCancel) to
no-op when isLoading is true (e.g., only invoke onCancel if !isLoading) so
backdrop clicks are ignored during the running discard and the status/error UI
remains visible.
In `@src/frontend/hooks/use-active-branch.ts`:
- Around line 18-27: The current subscribe function only listens for the custom
'active-branch-change' event so other tabs that rely on the native storage event
won't be notified; update subscribe (and its handleChange) to also listen for
the window 'storage' event and call the listener when event.key matches the
active-branch key (e.g., 'active-branch'), and ensure the returned cleanup
removes both the custom event listener and the 'storage' event listener so both
are properly unsubscribed.
In `@src/frontend/screens/workspace-screen.tsx`:
- Around line 243-265: The handleBranchSwitch callback currently only handles
thrown errors; update it to handle non-throwing failures from
project.openProjectByPath(): after awaiting result, if result.success is false
or result.data is missing, log the failure (e.g., console.error with result) and
show a toast informing the user that the branch switch succeeded but reopening
the project failed (include result.error or a helpful message), and avoid
calling sharedWorkspaceActions.handleOpenProjectResponse when result.data is
absent; keep existing thrown-error catch behavior intact. Reference:
handleBranchSwitch, project.openProjectByPath,
sharedWorkspaceActions.handleOpenProjectResponse.
In `@src/frontend/services/debug-force-variable.ts`:
- Around line 20-27: The shared force helper calls to debuggerPort.setVariable
(in src/frontend/services/debug-force-variable.ts) must catch
transport/connection failures so rejections don't bubble to callers; wrap the
await debuggerPort.setVariable(...) in try/catch, on error log the error (or
send to process logger) and return a controlled failure result (e.g. false or a
Result object with success:false) instead of rethrowing, and keep the existing
state-update logic only when result.success is true; apply the same try/catch
pattern to both occurrences (the block around setVariable at the first
occurrence and the second occurrence later in the file) so the UI receives a
predictable failure path.
In `@src/frontend/utils/ai-code-parser.ts`:
- Around line 168-173: When adding a new variable (the branch where ex is
falsy), you create newVar and push it into added and merged but never update the
existingByName lookup, so subsequent incoming entries with the same name are
considered new and duplicated; fix by inserting the new variable into
existingByName (using the incoming variable's name key) immediately after
creating newVar (and handle undefined/empty names if relevant) so future
iterations find ex and avoid re-adding the same name; reference symbols:
existingByName, ex, inc, newVar, added, merged, variablesEqual.
- Around line 95-101: The current catch swallows parsing errors but still
advances bodyStartIndex to lastEndVar, causing the VAR section to be dropped
from the returned body; update the logic so bodyStartIndex is only moved when
parseIecStringToVariables(varSection) succeeds (e.g., move the assignment of
bodyStartIndex = lastEndVar inside the try block or guard it with a success
flag), and ensure variables/varSection are preserved into the body when parsing
fails (do not discard varSection or modify bodyStartIndex in the catch). This
change touches parseIecStringToVariables usage, the try/catch around it, the
bodyStartIndex variable and lastEndVar handling.
In `@src/main/modules/ipc/main.ts`:
- Around line 593-604: handleOpenProjectPathPicker sometimes returns undefined
on the "no windowManager" and error paths which breaks callers expecting a
structured { success, ... } object; update the function so every execution path
returns a consistent response shape: when windowManager is missing return e.g. {
success: false, error: 'Window object not defined' } and in the catch block
return { success: false, error: getErrorMessage(error) }; keep the happy path
returning the result from getOpenProjectPath(windowManager) (which should be the
same shape) and use logger.error for logging as currently done.
---
Outside diff comments:
In `@src/frontend/hooks/useDebugPolling.ts`:
- Around line 198-232: The catch block currently unconditionally inserts 'ERR'
into changedBool/changedNonBool which causes repeated writes; change it to
compare the existing stored value (use currentBool/currentNonBool keyed by
meta.compositeKey) and only set 'ERR' into (isBool ? changedBool :
changedNonBool) if the stored value differs, keeping the bufferOffset +=
typeSize and itemsProcessed logic unchanged so downstream
workspaceActions.setDebugBoolValues/setDebugNonBoolValues are only called when
the value actually changed.
---
Minor comments:
In `@src/frontend/components/_atoms/graphical-editor/ladder/variable-visual.tsx`:
- Line 11: The prop/type for "variant" is currently too permissive (variant?:
string); narrow it to the literal union variant?: 'input' | 'output' wherever
it's declared (e.g., the props interface or type used by VariableVisual
component) and update any usages or switch/case checks in VariableVisual to rely
on those two literals; this change will enforce compile-time checks and reveal
any callsites passing invalid values so you can correct them to 'input' or
'output'.
In
`@src/frontend/components/_features/`[workspace]/branches/branch-switcher-modal.tsx:
- Around line 31-40: The current effect can commit stale results from prior
versionControl.listBranches calls; fix it by adding a cancellation guard (e.g.,
a useRef requestId or a local "cancelled" flag set in the effect cleanup) and
capture the current token before calling versionControl.listBranches(projectId);
only call setBranches and setIsLoading in the .then/.catch/.finally handlers if
the token matches (or cancelled is false). Update the effect that references
isOpen, projectId, versionControl, setBranches and setIsLoading to
create/advance the token on each run and invalidate it in the cleanup so late
responses won't overwrite newer state.
In
`@src/frontend/components/_features/`[workspace]/branches/delete-branch-modal.tsx:
- Line 48: The backdrop's onClick currently always calls onClose, causing
inconsistent behavior with the Escape key handler which only calls onClose when
!isPending; update the backdrop handler in delete-branch-modal.tsx so it guards
against pending state (check !isPending before invoking onClose) to match the
Escape key logic and prevent dismissal during deletion (reference the onClose
prop and isPending state used in the component).
In
`@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx:
- Around line 327-333: When the fetch in refresh (which calls
versionControl.getChanges) fails you clear the UI files with setFiles([]) but
don't reset the activity-bar badge; update the catch block to also call
versionControlActions.setPendingChangesCount(0) so the pending changes count is
cleared on error (ensure you still setIsLoading(false) in finally). Target the
async refresh flow where setFiles, versionControl.getChanges, and
versionControlActions.setPendingChangesCount are used.
- Around line 314-320: The effect currently only compares folderKey.size to
prevFolderKeyRef.current.size which misses changes when two different sets have
the same cardinality; update the useEffect to perform a set-content comparison
(e.g., check sizes and then verify every item in folderKey exists in
prevFolderKeyRef.current) and only skip when the sets are truly equal; when
detecting a difference call setExpandedFolders(folderKey) and store a defensive
copy into prevFolderKeyRef.current (e.g., new Set(folderKey)) so identity and
future comparisons behave correctly; modify the symbols prevFolderKeyRef,
folderKey, setExpandedFolders and the useEffect body accordingly.
In
`@src/frontend/components/_features/`[workspace]/source-control/history-section.tsx:
- Around line 83-84: The pagination label can show incorrect ranges when
commits.length === 0 but offset > 0 (e.g. "51–100 of 0"); update the label
computation in the HistorySection component to guard for empty results by
computing start and end using total (or commits.length) so when total === 0 it
renders "0–0 of 0" or a similar empty-state label: e.g., compute start = total
=== 0 ? 0 : offset + 1 and end = total === 0 ? 0 : Math.min(offset + PAGE_SIZE,
total), or reset offset to 0 on empty/error responses; adjust the JSX that
currently uses {offset + 1}–{Math.min(offset + PAGE_SIZE, total)} of
{total} to use these guarded values (references: commits, offset, PAGE_SIZE,
total, HistorySection).
In
`@src/frontend/components/_features/`[workspace]/source-control/modals/restore-confirmation-modal.tsx:
- Line 33: Backdrop click currently always calls onCancel while the Escape key
handler guards with !isLoading, causing inconsistent dismissal behavior; update
the backdrop's onClick handler in restore-confirmation-modal (the div with
className 'absolute inset-0 bg-black/40') to only call onCancel when !isLoading
(i.e., if (!isLoading) onCancel()), so it mirrors the Escape-key logic used
elsewhere in this component.
In
`@src/frontend/components/_features/`[workspace]/source-control/source-control-panel.tsx:
- Around line 47-58: The pending changes dot uses bg-blue-500 and becomes
invisible when activeView === 'changes' (button has bg-blue-500); update the
span rendering for pendingChangesCount in the source-control-panel component to
choose a contrasting color when the "Changes" tab is active (e.g., use bg-white
when activeView === 'changes', otherwise bg-blue-500) by making the className
conditional based on activeView (refer to activeView, setActiveView, and
pendingChangesCount to locate the button and indicator span) so the dot remains
visible in both states.
In `@src/frontend/screens/workspace-screen.tsx`:
- Around line 501-506: There are two overlapping ResizableHandle elements both
using id 'consoleResizeHandle' (one at the shown block and another at the block
referenced 513-518), causing duplicate DOM ids and overlapping drag targets; fix
by rendering only one handle: remove or conditionalize one of the
ResizableHandle renders so only a single ResizableHandle with id
'consoleResizeHandle' is present (e.g., wrap one instance in a guard like
tabs.length === 0 or migrate shared props into a single centralized
ResizableHandle render), ensuring the unique id and props remain on the sole
ResizableHandle component.
---
Nitpick comments:
In `@package.json`:
- Around line 78-80: The package.json pins "react" and "react-dom" to 18.3.1 (a
bridge release that emits deprecation warnings for React 19), so run the app in
development and sweep the codebase for the deprecated patterns called out
(string refs, legacy context: contextTypes/getChildContext, defaultProps on
function components, findDOMNode/unmountComponentAtNode, and JSX key spreading)
and refactor the offending code; specifically search references to "react" and
"react-dom" in package.json and then locate components using string refs,
contextTypes/getChildContext, defaultProps on functions,
findDOMNode/unmountComponentAtNode, or key spread and replace them with modern
patterns (useRef/useEffect, new Context API, default parameters or explicit prop
handling, ref forwarding/DOM refs, and explicit key assignment) and re-run
unit/e2e tests and dev build to ensure no deprecation warnings remain.
In `@src/frontend/components/_atoms/graphical-editor/diff/diff-wrapper.tsx`:
- Around line 22-35: renderHandles and renderFBDHandles accept handles: unknown
and use unsafe type assertions; define a minimal shared Handle type (e.g.,
properties id: string, type: 'source'|'target', position: Position, style?:
React.CSSProperties) and change the parameter types to that interface for both
functions, then remove the assertions and use the typed properties (referencing
renderHandles, renderFBDHandles and the props id, type, position, style passed
into Handle) so the compiler enforces shape safety.
In `@src/frontend/components/_atoms/graphical-editor/fbd/variable-visual.tsx`:
- Around line 66-72: The className composition on VariableVisual (the cn(...)
call) can produce conflicting text color classes when inputError and isForced
are both true; make precedence explicit by computing a single color class before
rendering: in the component that uses isAVariable, inputError, isForced,
forcedValue and cn, derive a colorClass variable that selects forced colors when
isForced (choose text-[`#80C000`] or text-[`#4080FF`] based on forcedValue), else
text-red-500 when inputError, else text-yellow-500 when !isAVariable, and pass
that single colorClass into the cn(...) call so forced state always overrides
error state unambiguously.
In
`@src/frontend/components/_features/`[workspace]/branches/unsaved-changes-warning-modal.tsx:
- Around line 28-30: The modal lacks ARIA attributes—update the modal container
in the UnsavedChangesWarningModal component to include role="dialog" and
aria-modal="true", add aria-labelledby that points to the modal title element id
and aria-describedby that points to the descriptive text id, ensure the title
and description elements have matching ids, and wire an onKeyDown handler to the
modal wrapper to close on Escape (using the existing onCancel prop);
additionally ensure initial focus is moved into the modal (focus the first
actionable element) or add a focus trap so keyboard and screen-reader users can
interact properly.
In
`@src/frontend/components/_features/`[workspace]/editor/monaco/ai-consent-modal.tsx:
- Around line 20-24: The handleDecline function updates consent but can leave
the chat UI open; update handleDecline to also explicitly close the chat by
calling the same open-state updater used elsewhere (e.g., onOpenChange) with the
chat identifier (for example call onOpenChange('chat', false) or the equivalent)
after setting localStorage and setAIConsented(false) so chat-open state cannot
remain stale when consent is declined.
In
`@src/frontend/components/_features/`[workspace]/source-control/commit-item.tsx:
- Around line 10-20: The formatRelativeTime function should validate the input
timestamp and provide an absolute date for older commits: first, in
formatRelativeTime, check that new Date(timestamp).getTime() is a valid number
(not NaN) and return a safe fallback like 'unknown' or the original timestamp
when invalid; then compute diff as now - ts and if diff >= 30 days (30 *
86400000) return a locale-formatted absolute date (e.g., using
toLocaleDateString or a short month/day/year format like "Mar 15, 2026"); keep
the existing relative outputs for minutes/hours/days < 30 days. Ensure you
update formatRelativeTime only (reference function name formatRelativeTime) and
add no external deps.
In
`@src/frontend/components/_features/`[workspace]/source-control/history-section.tsx:
- Around line 36-39: The catch block for listCommits currently swallows errors
and calls setCommits([]) and setTotal(0); instead, preserve existing
commit/total state (do not clear them on error), and surface the failure to the
user by triggering a toast or setting an inline error state (e.g., call
showToast/errorToast or setErrorMessage) inside the catch, while also logging
the caught error; if you must clear state only do so when on the first page
(check current page variable) so pagination doesn't show inconsistent "51-100 of
0" results. Ensure you update the catch attached to listCommits in the
HistorySection component and reference setCommits, setTotal, listCommits, and
the page/currentPage variable when making the change.
In
`@src/frontend/components/_features/`[workspace]/source-control/source-control-panel.tsx:
- Line 21: Remove the unnecessary useCallback wrapper around the selector passed
to useOpenPLCStore: replace the current call that uses useCallback((s) =>
s.versionControl.pendingChangesCount, []) with a direct selector function (s =>
s.versionControl.pendingChangesCount) when calling useOpenPLCStore to eliminate
the extra wrapper; update the declaration for pendingChangesCount accordingly so
it reads useOpenPLCStore(s => s.versionControl.pendingChangesCount).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5ca6066f-f438-4cbf-9586-c448daadb6fd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (83)
package.jsonsrc/App.tsxsrc/__architecture__/validate.tssrc/backend/editor/utils/path-picker.tssrc/backend/shared/styles/globals.csssrc/frontend/components/_atoms/buttons/activity-bar/index.tsxsrc/frontend/components/_atoms/graphical-editor/block-output-debug-badges.tsxsrc/frontend/components/_atoms/graphical-editor/debug-value-badge.tsxsrc/frontend/components/_atoms/graphical-editor/diff/constants.tssrc/frontend/components/_atoms/graphical-editor/diff/diff-wrapper.tsxsrc/frontend/components/_atoms/graphical-editor/diff/fbd-nodes.tsxsrc/frontend/components/_atoms/graphical-editor/diff/index.tssrc/frontend/components/_atoms/graphical-editor/diff/ladder-nodes.tsxsrc/frontend/components/_atoms/graphical-editor/fbd/block-visual.tsxsrc/frontend/components/_atoms/graphical-editor/fbd/comment-visual.tsxsrc/frontend/components/_atoms/graphical-editor/fbd/connection-visual.tsxsrc/frontend/components/_atoms/graphical-editor/fbd/variable-visual.tsxsrc/frontend/components/_atoms/graphical-editor/fbd/variable.tsxsrc/frontend/components/_atoms/graphical-editor/ladder/block-visual.tsxsrc/frontend/components/_atoms/graphical-editor/ladder/coil-visual.tsxsrc/frontend/components/_atoms/graphical-editor/ladder/coil.tsxsrc/frontend/components/_atoms/graphical-editor/ladder/contact-visual.tsxsrc/frontend/components/_atoms/graphical-editor/ladder/contact.tsxsrc/frontend/components/_atoms/graphical-editor/ladder/variable-visual.tsxsrc/frontend/components/_atoms/graphical-editor/ladder/variable.tsxsrc/frontend/components/_features/[workspace]/branches/branch-status-bar.tsxsrc/frontend/components/_features/[workspace]/branches/branch-switcher-modal.tsxsrc/frontend/components/_features/[workspace]/branches/create-branch-modal.tsxsrc/frontend/components/_features/[workspace]/branches/delete-branch-modal.tsxsrc/frontend/components/_features/[workspace]/branches/index.tssrc/frontend/components/_features/[workspace]/branches/unsaved-changes-warning-modal.tsxsrc/frontend/components/_features/[workspace]/editor/graphical/index.tsxsrc/frontend/components/_features/[workspace]/editor/monaco/ai-consent-modal.tsxsrc/frontend/components/_features/[workspace]/editor/monaco/index.tsxsrc/frontend/components/_features/[workspace]/source-control/changes-section.tsxsrc/frontend/components/_features/[workspace]/source-control/commit-details.tsxsrc/frontend/components/_features/[workspace]/source-control/commit-item.tsxsrc/frontend/components/_features/[workspace]/source-control/history-section.tsxsrc/frontend/components/_features/[workspace]/source-control/index.tssrc/frontend/components/_features/[workspace]/source-control/modals/discard-confirmation-modal.tsxsrc/frontend/components/_features/[workspace]/source-control/modals/restore-confirmation-modal.tsxsrc/frontend/components/_features/[workspace]/source-control/source-control-panel.tsxsrc/frontend/components/_molecules/graphical-editor/fbd/index.tsxsrc/frontend/components/_molecules/graphical-editor/ladder/rung/body.tsxsrc/frontend/components/_molecules/variables-panel/index.tsxsrc/frontend/components/_molecules/workspace-activity-bar/default/chat.tsxsrc/frontend/components/_molecules/workspace-activity-bar/tooltip-button.tsxsrc/frontend/components/_organisms/debugger/index.tsxsrc/frontend/components/_organisms/explorer/index.tsxsrc/frontend/components/_organisms/variables-code-editor/index.tsxsrc/frontend/components/_organisms/variables-editor/index.tsxsrc/frontend/components/_organisms/workspace-activity-bar/default.tsxsrc/frontend/components/_organisms/workspace-activity-bar/index.tsxsrc/frontend/components/_templates/[workspace]/main-content.tsxsrc/frontend/components/_templates/app-layout.tsxsrc/frontend/hooks/use-active-branch.tssrc/frontend/hooks/use-debug-composite-key.tssrc/frontend/hooks/use-debug-value.tssrc/frontend/hooks/useDebugPolling.tssrc/frontend/screens/workspace-screen.tsxsrc/frontend/services/debug-force-variable.tssrc/frontend/store/__tests__/workspace-slice.test.tssrc/frontend/store/index.tssrc/frontend/store/slices/index.tssrc/frontend/store/slices/shared/slice.tssrc/frontend/store/slices/shared/types.tssrc/frontend/store/slices/version-control/index.tssrc/frontend/store/slices/version-control/slice.tssrc/frontend/store/slices/version-control/types.tssrc/frontend/store/slices/workspace/slice.tssrc/frontend/store/slices/workspace/types.tssrc/frontend/utils/ai-code-parser.tssrc/main/modules/ipc/main.tssrc/main/modules/ipc/renderer.tssrc/middleware/adapters/editor/project-adapter.tssrc/middleware/adapters/editor/version-control-adapter.tssrc/middleware/editor-platform.tssrc/middleware/shared/ports/index.tssrc/middleware/shared/ports/platform-capabilities.tssrc/middleware/shared/ports/version-control-port.tssrc/middleware/shared/providers/index.tssrc/middleware/shared/providers/platform-context.tsxsrc/middleware/shared/providers/types.ts
… web) The openPLCStoreBase hook-pattern calls caused issues with auto-zustand-selectors-hook. Revert all component/hook selector calls back to useOpenPLCStore, keeping openPLCStoreBase only for .getState() and .subscribe() imperative access. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/frontend/screens/workspace-screen.tsx (1)
243-265:⚠️ Potential issue | 🟠 MajorHandle non-throwing reopen failures after a branch switch.
If
openProjectByPath()returns an unsuccessful result instead of throwing, the checkout has already happened but the in-memory project stays stale and the user gets no feedback.Suggested fix
const result = await project.openProjectByPath(projectPath) - if (result.success && result.data) { - sharedWorkspaceActions.handleOpenProjectResponse(result.data) - toast({ - title: 'Branch switched', - description: `Now on branch: ${branchName}`, - variant: 'default', - }) - } + if (!result.success || !result.data) { + console.error('[WorkspaceScreen] Branch switched but project reload failed:', result) + toast({ + title: 'Branch switched, but reload failed', + description: 'The project was not reopened after switching branches.', + variant: 'fail', + }) + return + } + + sharedWorkspaceActions.handleOpenProjectResponse(result.data) + toast({ + title: 'Branch switched', + description: `Now on branch: ${branchName}`, + variant: 'default', + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/screens/workspace-screen.tsx` around lines 243 - 265, The handler handleBranchSwitch currently only handles the success case from project.openProjectByPath; add an explicit else branch for when result.success is false (or result.data is missing) to surface that non-throwing failure: call sharedWorkspaceActions.handleOpenProjectResponse only on success, otherwise log the failure (include result.error or a descriptive message) and show a toast informing the user that the project reopen failed after checkout (include branchName and any available error detail); ensure the catch remains for thrown errors so both non-throwing and throwing failures are reported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frontend/screens/workspace-screen.tsx`:
- Around line 501-518: There are two ResizableHandle elements with the same id
('consoleResizeHandle') being rendered when tabs.length > 0, causing overlapping
drag targets; remove the duplicate so only one ResizableHandle with
id='consoleResizeHandle' is rendered. Specifically, edit the JSX in
workspace-screen.tsx where ResizableHandle is used (the ResizableHandle inside
the truthy tabs branch and the one rendered after the conditional) and delete
the inner instance (or alternatively move the handle outside the conditional so
a single ResizableHandle remains), ensuring only one ResizableHandle with
id='consoleResizeHandle' exists.
---
Duplicate comments:
In `@src/frontend/screens/workspace-screen.tsx`:
- Around line 243-265: The handler handleBranchSwitch currently only handles the
success case from project.openProjectByPath; add an explicit else branch for
when result.success is false (or result.data is missing) to surface that
non-throwing failure: call sharedWorkspaceActions.handleOpenProjectResponse only
on success, otherwise log the failure (include result.error or a descriptive
message) and show a toast informing the user that the project reopen failed
after checkout (include branchName and any available error detail); ensure the
catch remains for thrown errors so both non-throwing and throwing failures are
reported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 87a54fd1-6faf-4b87-b825-c3ce59d796fb
📒 Files selected for processing (8)
src/frontend/components/_organisms/debugger/index.tsxsrc/frontend/components/_organisms/workspace-activity-bar/index.tsxsrc/frontend/components/_templates/[workspace]/main-content.tsxsrc/frontend/components/_templates/app-layout.tsxsrc/frontend/hooks/use-debug-composite-key.tssrc/frontend/hooks/use-debug-value.tssrc/frontend/hooks/useDebugPolling.tssrc/frontend/screens/workspace-screen.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/frontend/components/_templates/[workspace]/main-content.tsx
- src/frontend/hooks/useDebugPolling.ts
- src/frontend/hooks/use-debug-composite-key.ts
- src/frontend/hooks/use-debug-value.ts
…vice gating - Check pouWasCreated.ok instead of truthiness so duplicate name detection works - Wire up validatePouOrDataTypeName for IEC 61131-3 name validation on POU creation - Show validation and required error messages inline, hide hint text when errors present - Update hint text to "Name must follow CamelCase, PascalCase, or snake_case" for POUs and data types - Add toast notification on duplicate POU name creation failure - Allow server/remote device creation for simulator targets (isRuntimeV4 || isSimulator) - Fix Zod default deviceBoard from 'Simulator' to 'OpenPLC Simulator' so simulator is selected on load Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/components/_features/[workspace]/create-element/element-card/index.tsx (1)
380-387:⚠️ Potential issue | 🟡 MinorUpdate availability copy to match simulator-enabled gating
At Line 380 and Line 515, simulator targets are now allowed, but the message still says this is only available for Runtime v4 and suggests only that path. This is now misleading.
Also applies to: 515-522
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/create-element/element-card/index.tsx around lines 380 - 387, The availability message shown when the condition !(isRuntimeV4 || isSimulator) is misleading because simulator targets are now allowed; update the user-facing copy in the render blocks that check isRuntimeV4 and isSimulator (the JSX controlled by the !(isRuntimeV4 || isSimulator) condition and the similar block around lines ~515-522) to state that Server configuration is available for OpenPLC Runtime v4 or simulator targets and instruct the user to either select OpenPLC Runtime v4 in Device Configuration or enable the simulator to enable this feature; keep the conditional logic unchanged and only modify the two message strings to mention both "OpenPLC Runtime v4 or simulator" and the two recommended actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/frontend/components/_features/`[workspace]/create-element/element-card/index.tsx:
- Around line 292-296: The helper text under the name field is misleading and is
conditionally hidden whenever datatypeErrors.name is truthy; update the UI in
element-card (the span currently wrapped by {!datatypeErrors.name}) so the
helper is visible even when errors exist and provide accurate guidance that
matches the actual validation rules (e.g., allowed identifier
characters/patterns used by the validator). Locate uses of datatypeErrors.name
in this file (including the similar block around the other occurrence) and
change the conditional logic to always render the helper (or render both error
message and helper together), and revise the helper copy to precisely reflect
the validator’s accepted formats/characters rather than only “CamelCase,
PascalCase, or snake_case.”
- Around line 143-152: The current flow throws a TypeError when pouWasCreated.ok
is false and then treats any caught error as a duplicate-name issue; instead,
remove the thrown TypeError and handle the business-failure inline: if
pouWasCreated.ok is false, call pouSetError('name', { type: 'already-exists' })
and show the fail toast (using toast) without entering the catch. Reserve the
catch block for unexpected runtime errors (log or rethrow) so only genuine
exceptions reach it; update the logic around pouWasCreated, pouSetError, toast,
closeContainer and setIsOpen accordingly.
---
Outside diff comments:
In
`@src/frontend/components/_features/`[workspace]/create-element/element-card/index.tsx:
- Around line 380-387: The availability message shown when the condition
!(isRuntimeV4 || isSimulator) is misleading because simulator targets are now
allowed; update the user-facing copy in the render blocks that check isRuntimeV4
and isSimulator (the JSX controlled by the !(isRuntimeV4 || isSimulator)
condition and the similar block around lines ~515-522) to state that Server
configuration is available for OpenPLC Runtime v4 or simulator targets and
instruct the user to either select OpenPLC Runtime v4 in Device Configuration or
enable the simulator to enable this feature; keep the conditional logic
unchanged and only modify the two message strings to mention both "OpenPLC
Runtime v4 or simulator" and the two recommended actions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ce02e049-1710-44b6-993c-50a115d9fc46
📒 Files selected for processing (4)
.github/workflows/test.ymlsrc/backend/shared/types/PLC/devices/configuration.tssrc/frontend/components/_features/[workspace]/create-element/element-card/index.tsxsrc/middleware/adapters/editor/version-control-adapter.ts
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/test.yml
- src/backend/shared/types/PLC/devices/configuration.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/middleware/adapters/editor/version-control-adapter.ts
Branch status bar disabled until branch switching is fully implemented. POU creation now correctly marks the project as unsaved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/frontend/components/_features/[workspace]/source-control/changes-section.tsx (1)
519-546:⚠️ Potential issue | 🟠 Major
isDiscardingis not reset after successful discard.The success path closes the modal and refreshes data but never calls
setIsDiscarding(false). After the first successful discard, the discard button stays stuck in its loading/disabled state until the component remounts. Move the reset to afinallyblock to ensure it's always called:Suggested fix
const handleDiscard = async () => { if (!versionControl) return setIsDiscarding(true) setErrorMessage(null) try { const selectedPaths = [...selectedFiles] await versionControl.discardChanges(projectId, selectedPaths.length === files.length ? undefined : selectedPaths) setShowDiscardModal(false) // Reload project data in-place (no hard page reload) try { const result = await projectPort.openProjectByPath(projectId) if (result.success && result.data) { sharedWorkspaceActions.handleOpenProjectResponse(result.data) } } catch { toast({ title: 'Failed to reload project after discard', variant: 'fail' }) } await fetchChanges() } catch (error) { setShowDiscardModal(false) const msg = error instanceof Error ? error.message : 'Failed to discard changes' setErrorMessage(msg) + } finally { setIsDiscarding(false) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx around lines 519 - 546, The handleDiscard function never resets setIsDiscarding(false) on the success path, leaving the discard UI stuck; modify handleDiscard (the async function starting with "const handleDiscard = async () =>") to ensure setIsDiscarding(false) is always called by moving the reset into a finally block that executes after the try/catch (so that after await versionControl.discardChanges, the modal/refresh logic stays the same but setIsDiscarding(false) runs regardless of success or failure); keep existing setShowDiscardModal and setErrorMessage behavior inside the try/catch but remove the setIsDiscarding(false) from only the catch and place a single call in finally.src/frontend/screens/workspace-screen.tsx (2)
245-267:⚠️ Potential issue | 🟠 MajorHandle non-throwing reopen failures after a branch switch.
openProjectByPath()can fail without throwing. In that case this callback exits silently, so the branch may already be changed while the in-memory project stays stale and the user gets no feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/screens/workspace-screen.tsx` around lines 245 - 267, handleBranchSwitch currently ignores non-throwing failures from project.openProjectByPath(), leaving the in-memory project stale; update the callback to handle result.success === false by logging the result error/details and showing a failure toast (use result.error or a generic message) so the user is notified, and avoid silently returning; reference the existing project.openProjectByPath call, sharedWorkspaceActions.handleOpenProjectResponse, and toast in your fix so you surface errors when reopen fails after a branch switch.
503-520:⚠️ Potential issue | 🟠 MajorRemove one of the duplicated console resize handles.
When
tabs.length > 0, bothResizableHandles render withid='consoleResizeHandle', which leaves stacked drag targets on the same boundary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/screens/workspace-screen.tsx` around lines 503 - 520, There are two duplicated ResizableHandle elements both using id='consoleResizeHandle' which creates stacked drag targets; remove the redundant one so only a single ResizableHandle remains (either keep the one inside the tabs conditional or the one after it), ensuring only one ResizableHandle with id='consoleResizeHandle' is rendered around the console area (search for ResizableHandle and id='consoleResizeHandle' to locate the duplicates).
🧹 Nitpick comments (4)
src/frontend/components/_features/[workspace]/source-control/changes-section.tsx (4)
64-68: Add keyboard support for modal dismissal.The modal can be closed by clicking the backdrop or the X button, but there's no keyboard handler for the Escape key. Consider adding:
Suggested improvement
function FilePreviewModal({ filePath, content, onClose }: { filePath: string; content: string; onClose: () => void }) { const isDark = document.documentElement.classList.contains('dark') + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose() + } + document.addEventListener('keydown', handleKeyDown) + return () => document.removeEventListener('keydown', handleKeyDown) + }, [onClose]) + return ( - <div className='fixed inset-0 z-50 flex items-center justify-center bg-black/50' onClick={onClose}> + <div className='fixed inset-0 z-50 flex items-center justify-center bg-black/50' onClick={onClose} role="dialog" aria-modal="true">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx around lines 64 - 68, Add Escape-key support to dismiss the modal by wiring a keydown handler that calls the existing onClose prop when the user presses Escape: inside the component (where the JSX with the outer backdrop div and inner container is defined) register a useEffect that adds window.addEventListener('keydown', handler) on mount, have the handler check for e.key === 'Escape' (or e.code === 'Escape') and call onClose(), and remove the listener in the cleanup to avoid leaks; keep the current click handlers and the inner div's e.stopPropagation() unchanged so backdrop clicks and the X button continue to work.
274-298: Consider extracting state management into custom hooks.The component manages many state variables (11 useState hooks) and several refs for tracking previous values. While functional, this could be improved for maintainability by extracting related state into custom hooks (e.g.,
useChangesSelection,useFolderExpansion,useCommitState).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx around lines 274 - 298, The ChangesSection component currently declares many independent useState hooks (files/setFiles, isLoading, isFetching, message, showDiscardModal, selectedFiles, errorMessage, isCommitting, isDiscarding, expandedFolders, previewFile) making the component large; refactor by extracting related state and logic into focused custom hooks such as useChangesSelection (manages files, selectedFiles, previewFile, setFiles, setSelectedFiles, setPreviewFile), useFolderExpansion (manages expandedFolders and setExpandedFolders), and useCommitState (manages message, isCommitting, isDiscarding, errorMessage, and commit/discard handlers); move any derived effects and helper functions currently inside ChangesSection that manipulate these pieces into the respective hooks and update callers in ChangesSection to use the new hook APIs (keep original names like setFiles, setSelectedFiles, setExpandedFolders, setMessage, setIsCommitting when returning from hooks to minimize changes).
61-63: Consider using a theme hook instead of direct DOM access.Reading
document.documentElement.classListduring render is a side effect that won't update if the theme changes while the modal is open. Consider using a theme context/hook if one exists in the codebase, or memoizing this check.Suggested approach
-function FilePreviewModal({ filePath, content, onClose }: { filePath: string; content: string; onClose: () => void }) { - const isDark = document.documentElement.classList.contains('dark') +function FilePreviewModal({ filePath, content, onClose }: { filePath: string; content: string; onClose: () => void }) { + const [isDark, setIsDark] = useState(() => document.documentElement.classList.contains('dark')) + + useEffect(() => { + const observer = new MutationObserver(() => { + setIsDark(document.documentElement.classList.contains('dark')) + }) + observer.observe(document.documentElement, { attributes: true, attributeFilter: ['class'] }) + return () => observer.disconnect() + }, [])Or preferably, use an existing theme context if available in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx around lines 61 - 63, The FilePreviewModal currently reads document.documentElement.classList.contains('dark') directly (isDark) during render which is a side effect and won't react to theme changes; update FilePreviewModal to obtain theme from the app's theme context/hook (e.g., useTheme or ThemeContext) or accept a theme/isDark prop and derive state via useEffect/useState or useMemo so the modal updates when theme changes; replace the direct DOM access in FilePreviewModal with the theme hook/prop and remove the document.documentElement usage.
323-336: Consider surfacing fetch errors to the user.The catch block silently swallows errors without user feedback. While setting
filesto an empty array prevents UI breakage, users might not understand why no changes appear. Consider setting an error state or at minimum logging the error for debugging:Suggested improvement
try { const data = await versionControl.getChanges(projectId) setFiles(data.changes) versionControlActions.setPendingChangesCount(data.changes.length) - } catch { + } catch (error) { + console.error('Failed to fetch changes:', error) setFiles([]) + setErrorMessage('Failed to fetch changes. Click sync to retry.') } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx around lines 323 - 336, The fetchChanges function swallows errors in its catch block which leaves users uninformed; update fetchChanges to capture the caught error from versionControl.getChanges and surface it (e.g., set an error state via setError or call a provided notify/log function) and also log the error for debugging, while preserving the current fallback (setFiles([])) and existing finally behavior (setIsLoading(false), setIsFetching(false)); reference the fetchChanges function, the call to versionControl.getChanges(projectId), setFiles, and versionControlActions.setPendingChangesCount when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx:
- Around line 418-427: The code calls addModel(editorObj) even when
getEditorFromEditors(pouName) returns an existing editor; remove the redundant
addModel call in the "editor already exists" branch so you only call addModel
when creating a new model via CreateEditorObjectFromTab(tabToBeCreated).
Concretely, keep addModel(model) before setEditor(model) in the branch where
model is created, but delete the addModel(editorObj) line in the existing-editor
branch and just call setEditor(editorObj); references: getEditorFromEditors,
CreateEditorObjectFromTab, addModel, setEditor, editorObj, model, pouName,
tabToBeCreated.
In `@src/frontend/screens/workspace-screen.tsx`:
- Around line 119-120: The isCollapsed effect currently toggles
explorerPanelRef, workspacePanelRef, and consolePanelRef but omits
sourceControlPanelRef, so the source-control pane stays open when the
activity-bar collapse toggle is used; update both places where the
collapse/expand flow runs (the effect around isCollapsed and the similar logic
later in the file) to include sourceControlPanelRef and call its same imperative
methods (e.g., collapse/expand or setCollapsed) alongside explorerPanelRef,
workspacePanelRef, and consolePanelRef so the source-control pane participates
in the global collapse/expand flow.
---
Duplicate comments:
In
`@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx:
- Around line 519-546: The handleDiscard function never resets
setIsDiscarding(false) on the success path, leaving the discard UI stuck; modify
handleDiscard (the async function starting with "const handleDiscard = async ()
=>") to ensure setIsDiscarding(false) is always called by moving the reset into
a finally block that executes after the try/catch (so that after await
versionControl.discardChanges, the modal/refresh logic stays the same but
setIsDiscarding(false) runs regardless of success or failure); keep existing
setShowDiscardModal and setErrorMessage behavior inside the try/catch but remove
the setIsDiscarding(false) from only the catch and place a single call in
finally.
In `@src/frontend/screens/workspace-screen.tsx`:
- Around line 245-267: handleBranchSwitch currently ignores non-throwing
failures from project.openProjectByPath(), leaving the in-memory project stale;
update the callback to handle result.success === false by logging the result
error/details and showing a failure toast (use result.error or a generic
message) so the user is notified, and avoid silently returning; reference the
existing project.openProjectByPath call,
sharedWorkspaceActions.handleOpenProjectResponse, and toast in your fix so you
surface errors when reopen fails after a branch switch.
- Around line 503-520: There are two duplicated ResizableHandle elements both
using id='consoleResizeHandle' which creates stacked drag targets; remove the
redundant one so only a single ResizableHandle remains (either keep the one
inside the tabs conditional or the one after it), ensuring only one
ResizableHandle with id='consoleResizeHandle' is rendered around the console
area (search for ResizableHandle and id='consoleResizeHandle' to locate the
duplicates).
---
Nitpick comments:
In
`@src/frontend/components/_features/`[workspace]/source-control/changes-section.tsx:
- Around line 64-68: Add Escape-key support to dismiss the modal by wiring a
keydown handler that calls the existing onClose prop when the user presses
Escape: inside the component (where the JSX with the outer backdrop div and
inner container is defined) register a useEffect that adds
window.addEventListener('keydown', handler) on mount, have the handler check for
e.key === 'Escape' (or e.code === 'Escape') and call onClose(), and remove the
listener in the cleanup to avoid leaks; keep the current click handlers and the
inner div's e.stopPropagation() unchanged so backdrop clicks and the X button
continue to work.
- Around line 274-298: The ChangesSection component currently declares many
independent useState hooks (files/setFiles, isLoading, isFetching, message,
showDiscardModal, selectedFiles, errorMessage, isCommitting, isDiscarding,
expandedFolders, previewFile) making the component large; refactor by extracting
related state and logic into focused custom hooks such as useChangesSelection
(manages files, selectedFiles, previewFile, setFiles, setSelectedFiles,
setPreviewFile), useFolderExpansion (manages expandedFolders and
setExpandedFolders), and useCommitState (manages message, isCommitting,
isDiscarding, errorMessage, and commit/discard handlers); move any derived
effects and helper functions currently inside ChangesSection that manipulate
these pieces into the respective hooks and update callers in ChangesSection to
use the new hook APIs (keep original names like setFiles, setSelectedFiles,
setExpandedFolders, setMessage, setIsCommitting when returning from hooks to
minimize changes).
- Around line 61-63: The FilePreviewModal currently reads
document.documentElement.classList.contains('dark') directly (isDark) during
render which is a side effect and won't react to theme changes; update
FilePreviewModal to obtain theme from the app's theme context/hook (e.g.,
useTheme or ThemeContext) or accept a theme/isDark prop and derive state via
useEffect/useState or useMemo so the modal updates when theme changes; replace
the direct DOM access in FilePreviewModal with the theme hook/prop and remove
the document.documentElement usage.
- Around line 323-336: The fetchChanges function swallows errors in its catch
block which leaves users uninformed; update fetchChanges to capture the caught
error from versionControl.getChanges and surface it (e.g., set an error state
via setError or call a provided notify/log function) and also log the error for
debugging, while preserving the current fallback (setFiles([])) and existing
finally behavior (setIsLoading(false), setIsFetching(false)); reference the
fetchChanges function, the call to versionControl.getChanges(projectId),
setFiles, and versionControlActions.setPendingChangesCount when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fde85b71-f36f-4e48-bb34-d3665c947e83
📒 Files selected for processing (3)
src/frontend/components/_features/[workspace]/source-control/changes-section.tsxsrc/frontend/screens/workspace-screen.tsxsrc/frontend/store/slices/shared/slice.ts
- Reset isDiscarding in finally block so UI recovers after successful discard - Remove duplicate consoleResizeHandle that rendered when tabs were open - Return structured error from handleOpenProjectPathPicker on all paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace deprecated overflow-y: overlay with auto + scrollbar-gutter: stable - Wrap all 5 missing ladder diff nodes in DiffWrapper for version control diffs - Add pointer-events-none guard to graphical editor read-only mode - Prevent backdrop dismiss during discard operation in confirmation modal - Show error toast on failed project reload after branch switch - Preserve VAR section text in body when variable parsing fails - Replace exception-driven POU creation flow with conditional check - Include source control panel in global collapse/expand effect Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The sync workflow needs to clone and query PRs from openplc-web, which requires a dedicated token since the default GITHUB_TOKEN only has access to the editor repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fork PRs don't receive secrets, so the sync check would always fail. Now the job exits green with a warning when CROSS_REPO_TOKEN is empty, but still fails hard if the token is present but expired/invalid. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 22, 2026
Merged
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bug fix: open project does nothing
The
openProjectadapter calledpathPickerwhich delegates togetProjectPath— a function designed for creating new projects that rejects non-empty directories. Since existing projects always have files, opening always silently failed. Added a dedicatedgetOpenProjectPaththat validatesproject.jsonexists instead of requiring an empty directory.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements