Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client';

import { useEditorEngine } from '@/components/store/editor';
import { DefaultSettings } from '@onlook/constants';
import { type FolderNode } from '@onlook/models';
Expand Down Expand Up @@ -206,14 +208,41 @@ const Folder = observer(() => {
</button>
)}
</div>
<Button
variant="default"
size="icon"
className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
onClick={() => handleCreateFolder(currentFolder)}
>
<Icons.DirectoryPlus className="h-4 w-4" />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant={'default'}
size={'icon'}
className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
onClick={() => handleCreateFolder(currentFolder)}
disabled={isAnyOperationLoading}
>
<Icons.DirectoryPlus className="h-4 w-4" />
</Button>
</TooltipTrigger>
<TooltipPortal>
<TooltipContent>
<p>Create a folder</p>
</TooltipContent>
</TooltipPortal>
</Tooltip>
Comment on lines +211 to +228
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Localize tooltip text and add an accessible label to the icon-only “Create folder” button.

Complies with next-intl/i18n guideline and improves a11y.

Apply this diff:

-                <Tooltip>
+                <Tooltip>
                     <TooltipTrigger asChild>
                         <Button
                             variant={'default'}
                             size={'icon'}
                             className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
                             onClick={() => handleCreateFolder(currentFolder)}
                             disabled={isAnyOperationLoading}
+                            aria-label={t('images.createFolder')}
                         >
                             <Icons.DirectoryPlus className="h-4 w-4" />
                         </Button>
                     </TooltipTrigger>
                     <TooltipPortal>
-                        <TooltipContent>
-                            <p>Create a folder</p>
+                        <TooltipContent>
+                            <p>{t('images.createFolder')}</p>
                         </TooltipContent>
                     </TooltipPortal>
                 </Tooltip>

Add this outside the shown range:

import { useTranslations } from 'next-intl';
// inside Folder component:
const t = useTranslations();

If creation is intended only at the root, gate the button with currentFolder === rootDir as well.


🏁 Script executed:

#!/bin/bash
set -euo pipefail
file='apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx'
echo "LOOKUP: $file"
if [ -f "$file" ]; then
  echo "=== FILE FOUND ==="
  sed -n '1,320p' "$file"
else
  echo "=== FILE NOT FOUND AT EXACT PATH ==="
  echo "Searching for likely files..."
  rg -n --hidden --glob '!node_modules' "image-tab" -S || true
  rg -n --hidden --glob '!node_modules' "folder/index.tsx" -S || true
  rg -n --hidden --glob '!node_modules' "Create a folder" -S || true
  rg -n --hidden --glob '!node_modules' "TooltipContent" -S || true
  rg -n --hidden --glob '!node_modules' "useTranslations" -S || true
  rg -n --hidden --glob '!node_modules' "aria-label" -S || true
  rg -n --hidden --glob '!node_modules' "handleCreateFolder" -S || true
  rg -n --hidden --glob '!node_modules' "currentFolder" -S || true
fi

Length of output: 12963


Localize tooltip text and add accessible label to the icon-only "Create folder" button.

  • Add: import { useTranslations } from 'next-intl'; and inside Folder: const t = useTranslations();
  • Replace TooltipContent

    Create a folder

    with

    {t('images.createFolder')}

    and add aria-label={t('images.createFolder')} to the icon-only Create Folder Button.
  • Gate the header Create Folder button so it only renders when currentFolder === rootDir (or use the existing showCreateButton).
  • Also localize other visible strings ("Search images" placeholder, "Loading images...", "Refresh Images", "Upload an image") and add aria-labels to other icon-only buttons (Back/ArrowLeft, Refresh, Upload).
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx
around lines 211-228, tooltip text is hardcoded and the icon-only "Create
folder" button lacks an accessible label and conditional rendering; import
useTranslations from 'next-intl' and instantiate it (const t =
useTranslations()), replace the hardcoded TooltipContent <p>Create a folder</p>
with the localized key {t('images.createFolder')}, add
aria-label={t('images.createFolder')} to the icon-only Create Folder Button, and
gate that Create Folder button so it only renders when currentFolder === rootDir
or when showCreateButton is true; additionally, localize visible strings such as
the "Search images" placeholder, "Loading images...", "Refresh Images", and
"Upload an image" using t(...) and add aria-labels to other icon-only buttons
(Back/ArrowLeft, Refresh, Upload) to ensure accessibility.

<Tooltip>
<TooltipTrigger asChild>
<Button
variant={'default'}
size={'icon'}
className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
onClick={() => editorEngine.image.scanImages()}
>
<Icons.Reload className="w-4 h-4" />
</Button>
</TooltipTrigger>
<TooltipPortal>
<TooltipContent>
<p>Refresh Images</p>
</TooltipContent>
</TooltipPortal>
</Tooltip>
Comment on lines +229 to +245
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Disable “Refresh Images” while scanning; localize text; add aria-label; avoid unhandled promise.

Prevents spam-clicks; aligns with i18n; clarifies intent.

Apply this diff:

                 <Tooltip>
                     <TooltipTrigger asChild>
                         <Button
                             variant={'default'}
                             size={'icon'}
                             className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
-                            onClick={() => editorEngine.image.scanImages()}
+                            onClick={() => { void editorEngine.image.scanImages(); }}
+                            disabled={isAnyOperationLoading || editorEngine.image.isScanning}
+                            aria-label={t('images.refresh')}
                         >
                             <Icons.Reload className="w-4 h-4" />
                         </Button>
                     </TooltipTrigger>
                     <TooltipPortal>
                         <TooltipContent>
-                            <p>Refresh Images</p>
+                            <p>{t('images.refresh')}</p>
                         </TooltipContent>
                     </TooltipPortal>
                 </Tooltip>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Tooltip>
<TooltipTrigger asChild>
<Button
variant={'default'}
size={'icon'}
className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
onClick={() => editorEngine.image.scanImages()}
>
<Icons.Reload className="w-4 h-4" />
</Button>
</TooltipTrigger>
<TooltipPortal>
<TooltipContent>
<p>Refresh Images</p>
</TooltipContent>
</TooltipPortal>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<Button
variant={'default'}
size={'icon'}
className="p-2 w-fit h-fit text-foreground-primary border-border-primary hover:border-border-onlook bg-background-secondary hover:bg-background-onlook border"
onClick={() => { void editorEngine.image.scanImages(); }}
disabled={isAnyOperationLoading || editorEngine.image.isScanning}
aria-label={t('images.refresh')}
>
<Icons.Reload className="w-4 h-4" />
</Button>
</TooltipTrigger>
<TooltipPortal>
<TooltipContent>
<p>{t('images.refresh')}</p>
</TooltipContent>
</TooltipPortal>
</Tooltip>
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/folder/index.tsx
around lines 229-245, the Refresh Images button should be disabled while a scan
is in progress, use a localized tooltip/aria-label, and avoid an unhandled
promise from scanImages. Update the Button to set
disabled={editorEngine.image.isScanning} (or the appropriate scanning flag),
replace the hardcoded tooltip and aria-label with the i18n call (e.g.
t('refreshImages')) and add aria-label using that same localized string, and
change the onClick to an async handler that awaits
editorEngine.image.scanImages() inside try/catch (or attach .catch()) so errors
are handled. Ensure the tooltip content uses the localized string as well.

<Tooltip>
<TooltipTrigger asChild>
<Button
Expand All @@ -223,7 +252,7 @@ const Folder = observer(() => {
onClick={uploadOperations.handleClickAddButton}
disabled={isAnyOperationLoading}
>
<Icons.Plus />
<Icons.Plus className="w-4 h-4" />
</Button>
</TooltipTrigger>
<TooltipPortal>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,20 +388,19 @@ export const FolderProvider = observer(({ children }: FolderProviderProps) => {

// Get all folder paths
const getImagesInFolder = useCallback((folder: FolderNode) => {
return editorEngine.activeSandbox.files.filter(image => {
// Use the image manager's scanned image paths instead of sandbox files
return editorEngine.image.imagePaths.filter(image => {
if (image.startsWith(folder.fullPath)) {
// Check if this is a direct child (not in a subdirectory)
const relativePath = image.slice(folder.fullPath.length);
// Remove leading slash if present
const cleanRelativePath = relativePath.startsWith('/') ? relativePath.slice(1) : relativePath;
// Only include if it's a direct child (no additional path separators)
if (!cleanRelativePath.includes('/')) {
return isImageFile(image);
}
return !cleanRelativePath.includes('/') && cleanRelativePath.length > 0;
}
return false;
});
}, [editorEngine.activeSandbox.files]);
}, [editorEngine.image.imagePaths]);

Comment on lines +391 to 404
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Fix direct-child check: require a path separator after the folder to prevent “publications” matching “public”.

Also fine to rely on image.imagePaths (already filtered to images).

Apply this diff:

-        return editorEngine.image.imagePaths.filter(image => {
-            if (image.startsWith(folder.fullPath)) {
-                // Check if this is a direct child (not in a subdirectory)
-                const relativePath = image.slice(folder.fullPath.length);
-                // Remove leading slash if present
-                const cleanRelativePath = relativePath.startsWith('/') ? relativePath.slice(1) : relativePath;
-                // Only include if it's a direct child (no additional path separators)
-                return !cleanRelativePath.includes('/') && cleanRelativePath.length > 0;
-            }
-            return false;
-        });
+        return editorEngine.image.imagePaths.filter(image => {
+            const prefix = `${folder.fullPath}/`;
+            if (!image.startsWith(prefix)) return false;
+            const cleanRelativePath = image.slice(prefix.length);
+            return !cleanRelativePath.includes('/') && cleanRelativePath.length > 0;
+        });

Please confirm activeSandbox emits image paths without a leading slash (e.g., "public/foo.png"). If it can include "/public/...", we should normalize before comparisons.


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"

echo
echo "1) Find occurrences of 'imagePaths' and editorEngine usage"
rg -n --hidden -S "imagePaths" -g '!.git' -C3 || true
rg -n --hidden -S "\\beditorEngine\\b" -g '!.git' -C3 || true

echo
echo "2) Show the folder-provider file (if present)"
FILE="apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsx"
if [ -f "$FILE" ]; then
  echo "File found: $FILE"
  echo "---- begin file (1..520) ----"
  sed -n '1,520p' "$FILE"
  echo "---- end file ----"
else
  echo "File not found at exact path: $FILE"
  echo "Searching for similarly named files..."
  rg -n --hidden -S "folder-provider\\.tsx|left-panel/image-tab/providers" -g '!.git' -C2 || true
fi

echo
echo "3) Search where imagePaths are populated / normalized"
rg -n --hidden -S "imagePaths\\s*[:=]" -g '!.git' -C3 || true
rg -n --hidden -S "setImagePaths|addImagePaths|push\\([^)]*image" -g '!.git' -C3 || true
rg -n --hidden -S "scanImages|scan.*image|listImages|list_images|list_images\\(|walkSync|readdirSync|glob\\(" -g '!.git' -C3 || true

echo
echo "4) Search for 'activeSandbox' and any code that emits sandbox paths"
rg -n --hidden -S "activeSandbox|sandbox" -g '!.git' -C3 || true

echo
echo "5) Search for '/public' or leading-slash path building / normalization"
rg -n --hidden -S "/public|\"/public|'\/public|^/public" -g '!.git' -C3 || true
rg -n --hidden -S "replace\\(\\s*['\"]/|^\\s*['\"]/|^\\s*/" -g '!.git' -C3 || true

echo
echo "Done."

Length of output: 15745


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"
echo

echo "1) Locate editor store / useEditorEngine"
rg -n --hidden -uu "useEditorEngine" -S -g '!.git' -C3 || true

echo
echo "2) List files under components/store (if present)"
ls -la apps/web/client/src/components/store || true
echo

echo "3) Show files in that directory (editor.*) if present"
for f in apps/web/client/src/components/store/editor* apps/web/client/src/components/store/*editor*; do
  if [ -f "$f" ]; then
    echo "---- FILE: $f ----"
    sed -n '1,240p' "$f"
    echo "---- END FILE: $f ----"
    echo
  fi
done

echo "4) Grep for imagePaths, activeSandbox, directories occurrences (fallback to grep)"
echo
grep -RIn --line-number "imagePaths" || true
echo
grep -RIn --line-number "activeSandbox" || true
echo
grep -RIn --line-number "directories" || true

echo
echo "Done."

Length of output: 28207


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Printing image store:"
echo "----------------------------------------"
sed -n '1,320p' apps/web/client/src/components/store/editor/image/index.ts || true
echo
echo "Printing sandbox store (to see how files/directories are listed):"
echo "----------------------------------------"
sed -n '1,520p' apps/web/client/src/components/store/editor/sandbox/index.ts || true
echo
echo "Search for IMAGE_FOLDER / DefaultSettings definitions:"
rg -n --hidden "IMAGE_FOLDER|DefaultSettings" || true
echo
echo "Show any files that mention IMAGE_FOLDER:"
rg -n --hidden --line-number "IMAGE_FOLDER" -S || true

Length of output: 129954


Require a trailing '/' when matching direct children (fix ambiguous prefix match)

  • Apply the suggested diff in apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsx: match using const prefix = ${folder.fullPath}/ and slice by prefix.length so "publications" will not match "public".
  • Verified: DefaultSettings.IMAGE_FOLDER === 'public' and editorEngine.image.imagePaths are populated from activeSandbox.files as "public/..." (no leading slash), so no extra normalization is required; if a provider can emit "/public/...", strip a leading '/' before comparing.
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/providers/folder-provider.tsx
around lines 391 to 404, the current prefix check can produce ambiguous matches
(e.g., "public" matching "publications"); change the logic to build a prefix
const prefix = `${folder.fullPath}/` and compare against a normalized image path
(strip a leading '/' if present) using image.startsWith(prefix), then slice by
prefix.length to get the relative child name and only include entries without
additional '/' and with length > 0; ensure you normalize folder.fullPath
similarly if providers may emit a leading '/'.

// Check if any operation is loading
const isOperating =
Expand Down
35 changes: 20 additions & 15 deletions apps/web/client/src/components/store/editor/image/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DefaultSettings } from '@onlook/constants';
import type { ActionTarget, ImageContentData, InsertImageAction } from '@onlook/models';
import { LeftPanelTabValue, type ActionTarget, type ImageContentData, type InsertImageAction } from '@onlook/models';
import { convertToBase64, generateNewFolderPath, getBaseName, getMimeType, isImageFile, stripImageFolderPrefix } from '@onlook/utility';
import { makeAutoObservable, reaction } from 'mobx';
import type { EditorEngine } from '../engine';
Expand All @@ -18,10 +18,18 @@ export class ImageManager {

init() {
this.indexingReactionDisposer = reaction(
() => this.editorEngine.activeSandbox.isIndexing,
async (isIndexingFiles) => {
if (!isIndexingFiles) {
await this.scanImages();
() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The reaction’s tracking function returns only isIndexing and isIndexed. If you intend to trigger a scan when the images tab is activated, consider including leftPanelTab in the tracked object.

return {
isIndexing: this.editorEngine.activeSandbox.isIndexing,
isIndexed: this.editorEngine.activeSandbox.isIndexed,
};
},
(sandboxStatus) => {
if (this.editorEngine.state.leftPanelTab !== LeftPanelTabValue.IMAGES) {
return;
}
if (sandboxStatus.isIndexed && !sandboxStatus.isIndexing) {
this.scanImages();
}
Comment on lines +21 to 33
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Reaction misses leftPanelTab in its tracked derivation; IMAGES tab activation won’t trigger a scan.

Include the tab in the reaction’s data source so switching to the IMAGES tab fires scanImages when indexed.

Apply this diff:

-        this.indexingReactionDisposer = reaction(
-            () => {
-                return {
-                    isIndexing: this.editorEngine.activeSandbox.isIndexing,
-                    isIndexed: this.editorEngine.activeSandbox.isIndexed,
-                };
-            },
-            (sandboxStatus) => {
-                if (this.editorEngine.state.leftPanelTab !== LeftPanelTabValue.IMAGES) {
-                    return;
-                }
-                if (sandboxStatus.isIndexed && !sandboxStatus.isIndexing) {
-                    this.scanImages();
-                }
-            },
-        );
+        this.indexingReactionDisposer = reaction(
+            () => ({
+                tab: this.editorEngine.state.leftPanelTab,
+                isIndexing: this.editorEngine.activeSandbox.isIndexing,
+                isIndexed: this.editorEngine.activeSandbox.isIndexed,
+            }),
+            ({ tab, isIndexing, isIndexed }) => {
+                if (tab !== LeftPanelTabValue.IMAGES) return;
+                if (isIndexed && !isIndexing) {
+                    void this.scanImages();
+                }
+            },
+        );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
() => {
return {
isIndexing: this.editorEngine.activeSandbox.isIndexing,
isIndexed: this.editorEngine.activeSandbox.isIndexed,
};
},
(sandboxStatus) => {
if (this.editorEngine.state.leftPanelTab !== LeftPanelTabValue.IMAGES) {
return;
}
if (sandboxStatus.isIndexed && !sandboxStatus.isIndexing) {
this.scanImages();
}
this.indexingReactionDisposer = reaction(
() => ({
tab: this.editorEngine.state.leftPanelTab,
isIndexing: this.editorEngine.activeSandbox.isIndexing,
isIndexed: this.editorEngine.activeSandbox.isIndexed,
}),
({ tab, isIndexing, isIndexed }) => {
if (tab !== LeftPanelTabValue.IMAGES) return;
if (isIndexed && !isIndexing) {
void this.scanImages();
}
},
);
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/image/index.ts around lines 21 to
33, the reaction only tracks sandbox indexing state so switching the left panel
to IMAGES won’t trigger scanImages; update the derivation to also return the
current leftPanelTab (e.g., include leftPanelTab:
this.editorEngine.state.leftPanelTab) and adjust the reaction callback to read
that field so changes to the tab cause the reaction to re-run and call
this.scanImages() when the sandbox is indexed and not indexing.

},
);
Expand Down Expand Up @@ -183,16 +191,13 @@ export class ImageManager {
}

async scanImages() {
if (this._isScanning) {
return;
}

this._isScanning = true;

try {
const files = await this.editorEngine.activeSandbox.listFilesRecursively(
DefaultSettings.IMAGE_FOLDER,
);
if (this._isScanning) {
return;
}
this._isScanning = true;

const files = this.editorEngine.activeSandbox.files;
if (!files) {
console.error('No files found in image folder');
return;
Expand All @@ -201,7 +206,7 @@ export class ImageManager {
this._imagePaths = [];
return;
}
this._imagePaths = files.filter((file: string) => isImageFile(file));
this._imagePaths = files.filter((file: string) => file.startsWith(DefaultSettings.IMAGE_FOLDER) && isImageFile(file));
} catch (error) {
Comment on lines +209 to 210
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid false positives: enforce folder boundary when filtering (public vs publications).

Use ${DefaultSettings.IMAGE_FOLDER}/ as the prefix.

Apply this diff:

-            this._imagePaths = files.filter((file: string) => file.startsWith(DefaultSettings.IMAGE_FOLDER) && isImageFile(file));
+            const prefix = `${DefaultSettings.IMAGE_FOLDER}/`;
+            this._imagePaths = files.filter((file: string) => file.startsWith(prefix) && isImageFile(file));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this._imagePaths = files.filter((file: string) => file.startsWith(DefaultSettings.IMAGE_FOLDER) && isImageFile(file));
} catch (error) {
const prefix = `${DefaultSettings.IMAGE_FOLDER}/`;
this._imagePaths = files.filter((file: string) => file.startsWith(prefix) && isImageFile(file));
} catch (error) {
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/image/index.ts around lines
209-210, the filter currently uses file.startsWith(DefaultSettings.IMAGE_FOLDER)
which can match sibling folders (causing false positives); change the prefix
check to require the folder boundary by using
file.startsWith(`${DefaultSettings.IMAGE_FOLDER}/`) so only files directly under
the IMAGE_FOLDER path are accepted, and keep the existing isImageFile(file)
check.

console.error('Error scanning images:', error);
this._imagePaths = [];
Expand Down
57 changes: 21 additions & 36 deletions apps/web/client/src/components/store/editor/sandbox/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -549,42 +549,20 @@ export class SandboxManager {
async handleFileChangedEvent(normalizedPath: string) {
const cachedFile = this.fileSync.readCache(normalizedPath);

if (isImageFile(normalizedPath)) {
if (!cachedFile || cachedFile.content === null) {
// If the file was not cached, we need to write an empty file
this.fileSync.writeEmptyFile(normalizedPath, 'binary');
} else {
// If the file was already cached, we need to read the remote file and update the cache
const remoteFile = await this.readRemoteFile(normalizedPath);
if (!remoteFile || remoteFile.content === null) {
console.error(`File content for ${normalizedPath} not found in remote`);
return;
}
this.fileSync.updateCache(remoteFile);
}
} else {
// If the file is not an image, we need to read the remote file and update the cache
const remoteFile = await this.readRemoteFile(normalizedPath);
if (!remoteFile || remoteFile.content === null) {
console.error(`File content for ${normalizedPath} not found in remote`);
return;
}
if (remoteFile.type === 'text') {
// If the file is a text file, we need to process it for mapping
this.fileSync.updateCache({
type: 'text',
path: normalizedPath,
content: remoteFile.content,
});
if (remoteFile.content !== cachedFile?.content) {
await this.processFileForMapping(remoteFile);
}
} else {
this.fileSync.updateCache({
type: 'binary',
path: normalizedPath,
content: remoteFile.content,
});
// Always read the remote file and update the cache, regardless of file type
const remoteFile = await this.readRemoteFile(normalizedPath);
if (!remoteFile) {
console.error(`File content for ${normalizedPath} not found in remote`);
return;
}
Comment on lines +553 to +557
Copy link
Contributor

Choose a reason for hiding this comment

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

Logic error: The code returns early if remoteFile is falsy, but the comment on line 552 states it should 'always update cache regardless of file type'. This creates inconsistent behavior where some file change events are silently ignored. The function should either handle the null case appropriately (perhaps by removing the file from cache) or the early return should be reconsidered to match the stated behavior.

Suggested change
const remoteFile = await this.readRemoteFile(normalizedPath);
if (!remoteFile) {
console.error(`File content for ${normalizedPath} not found in remote`);
return;
}
const remoteFile = await this.readRemoteFile(normalizedPath);
if (!remoteFile) {
console.error(`File content for ${normalizedPath} not found in remote`);
// Remove file from cache instead of returning early
this.removeFromCache(normalizedPath);
return;
}

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


// Always update the cache with the fresh remote file content
this.fileSync.updateCache(remoteFile);

// For text files, also process for mapping if content has changed
if (remoteFile.type === 'text' && this.isJsxFile(normalizedPath)) {
if (remoteFile.content !== cachedFile?.content) {
await this.processFileForMapping(remoteFile);
}
}
}
Expand Down Expand Up @@ -678,6 +656,12 @@ export class SandboxManager {
},
});

// Read and cache the copied file
const copiedFile = await this.readRemoteFile(normalizedTargetPath);
if (copiedFile) {
this.fileSync.updateCache(copiedFile);
}

Comment on lines +659 to +664
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Copying directories leaves cache stale; handle directory targets

After copyFiles, attempting to read a directory via readRemoteFile returns null and the directory cache isn’t updated. This desynchronizes the cache on directory copies (common when recursive is true).

Apply this diff:

-            // Read and cache the copied file
-            const copiedFile = await this.readRemoteFile(normalizedTargetPath);
-            if (copiedFile) {
-                this.fileSync.updateCache(copiedFile);
-            }
+            // Sync cache for the copy target (file or directory)
+            const stat = await this.session.provider.statFile({
+                args: { path: normalizedTargetPath },
+            });
+            if (stat?.type === 'directory') {
+                this.fileSync.updateDirectoryCache(normalizedTargetPath);
+                // Optional: list + seed child entries if provider doesn't emit add events for children.
+                // (Rely on watcher by default.)
+            } else {
+                const copiedFile = await this.readRemoteFile(normalizedTargetPath);
+                if (copiedFile) {
+                    this.fileSync.updateCache(copiedFile);
+                }
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Read and cache the copied file
const copiedFile = await this.readRemoteFile(normalizedTargetPath);
if (copiedFile) {
this.fileSync.updateCache(copiedFile);
}
// Sync cache for the copy target (file or directory)
const stat = await this.session.provider.statFile({
args: { path: normalizedTargetPath },
});
if (stat?.type === 'directory') {
this.fileSync.updateDirectoryCache(normalizedTargetPath);
// Optional: list + seed child entries if provider doesn't emit add events for children.
// (Rely on watcher by default.)
} else {
const copiedFile = await this.readRemoteFile(normalizedTargetPath);
if (copiedFile) {
this.fileSync.updateCache(copiedFile);
}
}
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/editor/sandbox/index.ts around lines 659
to 664, the code calls readRemoteFile after copyFiles and skips cache updates
when readRemoteFile returns null, which leaves the cache stale for directory
targets; change the logic so that if readRemoteFile returns null you detect
whether the normalizedTargetPath is a directory (e.g., via readRemoteDirectory
or a metadata/stat call), and when it is update the fileSync cache with a
directory entry (and for recursive copies, fetch and update the directory
contents recursively or refresh the parent directory listing) so the cache
reflects the newly copied directory structure.

return true;
} catch (error) {
console.error(`Error copying ${path} to ${targetPath}:`, error);
Expand Down Expand Up @@ -744,6 +728,7 @@ export class SandboxManager {
},
});

// Note: Cache update handled by file watcher rename event
return true;
} catch (error) {
console.error(`Error renaming file ${oldPath} to ${newPath}:`, error);
Expand Down