Detect file renames without triggering deletions#587
Detect file renames without triggering deletions#587huntercaron wants to merge 4 commits intomainfrom
Conversation
When watcher sanitizes file/folder names (e.g., "race?.tsx" → "race_.tsx"), the fs.rename() call triggers echo events from chokidar that were being processed as real user events. This caused: - False file uploads (echo add events) - False file deletions (echo unlink events undoing the sanitization) - Interference with rename detection Solution: - Track recently sanitized paths in recentSanitizations set - Record both unsanitized and sanitized paths after fs.rename() - Suppress events at top of emitEvent if they match recent sanitizations - Add same-path rename guard to dispatchEvent for defense-in-depth - Add tests for echo suppression scenarios This complements the existing rename detection feature which uses the Framer SDK's codeFile.rename() API to properly handle file renames without deletion operations.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Move the file-synced WebSocket message inside the existing-file guard so the CLI is not told a rename succeeded when the target file was never found in Framer. Also reorder rename-effect tracking to update state only after a successful send, and fix minor type/style issues.
huntercaron
added
Auto submit to Marketplace on merge
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
| if (content !== undefined) { | ||
| this.lastSnapshot.delete(normalisedOld) | ||
| this.lastSnapshot.set(normalisedNew, content) | ||
| } |
There was a problem hiding this comment.
Snapshot updated before confirming file exists for rename
Medium Severity
applyRemoteRename unconditionally updates lastSnapshot (deleting the old key and inserting the new one) before checking whether the file actually exists in Framer. If existing is not found, the rename never happens and no file-synced response is sent, but the snapshot is already inconsistent. On the next handleFramerFilesChanged call, the file still exists under the old name in Framer but is missing from lastSnapshot, triggering a spurious file-change for the old name and a spurious file-delete for the new name — effectively undoing the local rename.
Additional Locations (1)
Reuse hashFileContent from state-persistence instead of maintaining a private copy.
| fileName: overrides.fileName ?? "Test.tsx", | ||
| localContent: "localContent" in overrides ? overrides.localContent : "local", | ||
| remoteContent: "remoteContent" in overrides ? overrides.remoteContent : "remote", | ||
| localContent: "localContent" in overrides ? (overrides.localContent as string | null) : "local", |
There was a problem hiding this comment.
maybe the assertion can be fixed in a better way?
There was a problem hiding this comment.
good point ya this is a bit stinky
| // Paths recently renamed by sanitization — used to suppress echo events from chokidar | ||
| const recentSanitizations = new Set<string>() | ||
|
|
||
| const watcher = chokidar.watch(filesDir, { |
There was a problem hiding this comment.
Is there some reason we're using chokidar over fs.watch() - it seems the latter supports rename detection directly with a bit less hassdle, though unsure of the tradeoffs today
There was a problem hiding this comment.
When I was spooking into it earlier in the process it seemed much more consistent across platforms and node versions. Also looked into vite etc and they all seem to use chokidar still
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end rename support to the code-link workflow (CLI watcher → CLI controller → plugin) by detecting unlink+add rename patterns locally, sending a new file-rename message type, and applying the rename remotely via Framer’s codeFile.rename() API. It also adds suppression for chokidar “echo” events caused by the watcher’s own sanitization renames to avoid false remote deletions.
Changes:
- Add hash-based unlink+add pairing with a short buffer to emit a single local
renamewatcher event. - Introduce
file-renameCLI→plugin message + controller effect to send renames and update local tracking. - Implement plugin-side remote rename application via
codeFile.rename()and update shared message types accordingly.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/code-link/src/api.ts | Adds applyRemoteRename() to rename Framer code files and acknowledge via file-synced. |
| plugins/code-link/src/App.tsx | Handles incoming file-rename messages and invokes the new API method. |
| packages/code-link-shared/src/types.ts | Extends shared message union/validation to include file-rename. |
| packages/code-link-cli/src/utils/project.ts | Adjusts directory-name sanitization rules (now preserves spaces). |
| packages/code-link-cli/src/utils/hash-tracker.ts | Reuses shared hashing utility from state persistence (removes local crypto hashing). |
| packages/code-link-cli/src/types.ts | Adds rename watcher event kind and oldRelativePath metadata. |
| packages/code-link-cli/src/helpers/watcher.ts | Implements rename detection + sanitization echo suppression + buffer cleanup on close. |
| packages/code-link-cli/src/helpers/watcher.test.ts | Adds unit tests covering rename detection, buffering, and echo suppression. |
| packages/code-link-cli/src/helpers/installer.ts | Minor formatting cleanup (whitespace). |
| packages/code-link-cli/src/helpers/files.test.ts | Adjusts test typings for conflict content values. |
| packages/code-link-cli/src/controller.ts | Adds rename handling in the state machine and a new SEND_FILE_RENAME effect. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -53,32 +92,136 @@ export function initWatcher(filesDir: string): Watcher { | |||
| await fs.rename(absolutePath, newAbsolutePath) | |||
| debug(`Renamed ${rawRelativePath} -> ${relativePath}`) | |||
| effectiveAbsolutePath = newAbsolutePath | |||
|
|
|||
| // Suppress the echo events chokidar will fire for this rename | |||
| recentSanitizations.add(rawRelativePath) // upcoming unlink echo | |||
| recentSanitizations.add(relativePath) // upcoming add echo | |||
| setTimeout(() => { | |||
| recentSanitizations.delete(rawRelativePath) | |||
| recentSanitizations.delete(relativePath) | |||
| }, RENAME_BUFFER_MS * 3) | |||
| } catch (err) { | |||
| warn(`Failed to rename ${rawRelativePath}`, err) | |||
| } | |||
There was a problem hiding this comment.
If the on-disk sanitization rename fails (catch at fs.rename), the code still returns relativePath as the sanitized path while effectiveAbsolutePath remains the original path. This can emit watcher events for a path that doesn't actually exist locally, leading to incorrect remote sync attempts and repeated events. On rename failure, consider returning null (skip) or falling back to rawRelativePath so event paths reflect reality.
| if (syncState.socket) { | ||
| await sendMessage(syncState.socket, { | ||
| type: "file-rename", | ||
| oldFileName: effect.oldFileName, | ||
| newFileName: effect.newFileName, | ||
| }) | ||
| } | ||
|
|
||
| // Only update tracking after successful send | ||
| hashTracker.forget(effect.oldFileName) | ||
| fileMetadataCache.recordDelete(effect.oldFileName) | ||
| hashTracker.remember(effect.newFileName, effect.content) | ||
| } catch (err) { |
There was a problem hiding this comment.
In SEND_FILE_RENAME, tracking is updated even when there is no active socket (or if sendMessage fails). This contradicts the comment and can desync local state (old path forgotten / marked deleted, new path remembered) without the rename ever being sent to the plugin. Gate the metadata/hash updates on a confirmed successful send (or return early when syncState.socket is null / sendMessage returns false).
| if (event.kind === "rename" && event.relativePath === event.oldRelativePath) { | ||
| debug(`Skipping no-op rename: ${event.relativePath}`) | ||
| return | ||
| } | ||
| debug(`Watcher event: ${event.kind} ${event.relativePath}`) | ||
| for (const handler of handlers) { | ||
| handler(event) |
There was a problem hiding this comment.
The no-op rename guard drops the event entirely when relativePath === oldRelativePath. This can happen if the hash-based matcher pairs an unlink+add for the same path (e.g., delete+recreate within the buffer), resulting in lost add/delete information. Consider falling back to emitting the original buffered add/delete (or emitting a change) instead of silently returning.
| if (event.kind === "rename" && event.relativePath === event.oldRelativePath) { | |
| debug(`Skipping no-op rename: ${event.relativePath}`) | |
| return | |
| } | |
| debug(`Watcher event: ${event.kind} ${event.relativePath}`) | |
| for (const handler of handlers) { | |
| handler(event) | |
| let eventToDispatch: WatcherEvent = event | |
| if (event.kind === "rename" && event.relativePath === event.oldRelativePath) { | |
| // A "rename" where the old and new paths are the same likely represents | |
| // a delete+recreate or content change that was matched as a rename. | |
| // Treat this as a change instead of dropping the event entirely. | |
| debug(`Converting no-op rename to change event for: ${event.relativePath}`) | |
| eventToDispatch = { ...(event as any), kind: "change" } | |
| } | |
| debug(`Watcher event: ${eventToDispatch.kind} ${eventToDispatch.relativePath}`) | |
| for (const handler of handlers) { | |
| handler(eventToDispatch) |
| // Check if there's already a pending add with matching hash (add arrived first) | ||
| let matchingAddKey: string | undefined | ||
| for (const [key, pending] of pendingAdds) { | ||
| if (pending.contentHash === lastHash) { | ||
| matchingAddKey = key | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
Rename matching in the delete-path scans all pending adds and picks the first one with the same content hash. If multiple files share identical content (common for boilerplate), an unrelated add can be incorrectly paired with this delete, producing a wrong rename event. To avoid mis-pairing, include additional constraints (e.g., prefer same basename/dir, track inode/mtime where available, or store multiple candidates per hash and disambiguate) or fall back to separate add+delete when the match is ambiguous.
| // Check if this add matches a pending delete (delete arrived first) | ||
| let matchingDeleteKey: string | undefined | ||
| for (const [key, pending] of pendingDeletes) { | ||
| if (pending.contentHash === contentHash) { | ||
| matchingDeleteKey = key | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
Rename matching in the add-path also scans all pending deletes and pairs the first entry with a matching hash. With duplicate-content files, this can associate the new add with the wrong deleted path and emit an incorrect rename. Use a more deterministic key than content hash alone (or treat multi-match as ambiguous and emit separate add+delete).
| export function toDirectoryName(name: string): string { | ||
| return name | ||
| .replace(/[^a-zA-Z0-9-]/g, "-") | ||
| .replace(/[^a-zA-Z0-9- ]/g, "-") | ||
| .replace(/^-+|-+$/g, "") | ||
| .replace(/-+/g, "-") | ||
| } |
There was a problem hiding this comment.
toDirectoryName() now preserves spaces, but it doesn't trim() the input first. This is a behavioral regression vs the previous implementation where leading/trailing spaces were converted to - and then stripped, and can result in directories with leading/trailing whitespace in their names. Consider calling trim() (or otherwise stripping leading/trailing spaces) before applying the replacements.
| if (content !== undefined) { | ||
| this.lastSnapshot.delete(normalisedOld) | ||
| this.lastSnapshot.set(normalisedNew, content) | ||
| } | ||
|
|
||
| const codeFiles = await framer.getCodeFiles() | ||
| const existing = codeFiles.find(file => canonicalFileName(file.path || file.name) === normalisedOld) | ||
|
|
||
| if (existing) { | ||
| await existing.rename(ensureExtension(normalisedNew)) | ||
| socket.send( | ||
| JSON.stringify({ | ||
| type: "file-synced", | ||
| fileName: normalisedNew, | ||
| remoteModifiedAt: Date.now(), | ||
| }) | ||
| ) |
There was a problem hiding this comment.
applyRemoteRename() mutates lastSnapshot before attempting the remote rename and doesn't handle rename failures. If existing.rename(...) throws (e.g., name conflict), the snapshot will now claim the file moved even though Framer still has the old path, which can lead to incorrect downstream delete/change detection. Wrap the rename in try/catch and only update lastSnapshot after a successful rename (or roll back on failure).
| if (content !== undefined) { | |
| this.lastSnapshot.delete(normalisedOld) | |
| this.lastSnapshot.set(normalisedNew, content) | |
| } | |
| const codeFiles = await framer.getCodeFiles() | |
| const existing = codeFiles.find(file => canonicalFileName(file.path || file.name) === normalisedOld) | |
| if (existing) { | |
| await existing.rename(ensureExtension(normalisedNew)) | |
| socket.send( | |
| JSON.stringify({ | |
| type: "file-synced", | |
| fileName: normalisedNew, | |
| remoteModifiedAt: Date.now(), | |
| }) | |
| ) | |
| const codeFiles = await framer.getCodeFiles() | |
| const existing = codeFiles.find(file => canonicalFileName(file.path || file.name) === normalisedOld) | |
| if (existing) { | |
| try { | |
| await existing.rename(ensureExtension(normalisedNew)) | |
| if (content !== undefined) { | |
| this.lastSnapshot.delete(normalisedOld) | |
| this.lastSnapshot.set(normalisedNew, content) | |
| } | |
| socket.send( | |
| JSON.stringify({ | |
| type: "file-synced", | |
| fileName: normalisedNew, | |
| remoteModifiedAt: Date.now(), | |
| }) | |
| ) | |
| } catch (err) { | |
| log.error(`Failed to apply remote rename from ${normalisedOld} to ${normalisedNew}`, err) | |
| } |
|
|
||
| const content = this.lastSnapshot.get(normalisedOld) | ||
| if (content !== undefined) { | ||
| this.lastSnapshot.delete(normalisedOld) | ||
| this.lastSnapshot.set(normalisedNew, content) | ||
| } | ||
|
|
||
| const codeFiles = await framer.getCodeFiles() | ||
| const existing = codeFiles.find(file => canonicalFileName(file.path || file.name) === normalisedOld) | ||
|
|
||
| if (existing) { | ||
| await existing.rename(ensureExtension(normalisedNew)) | ||
| socket.send( | ||
| JSON.stringify({ | ||
| type: "file-synced", | ||
| fileName: normalisedNew, |
There was a problem hiding this comment.
existing.rename() uses ensureExtension(normalisedNew), but the subsequent file-synced message reports fileName: normalisedNew (without ensureExtension). If a rename is ever requested without an explicit extension, the plugin will rename to e.g. Foo.tsx but acknowledge sync for Foo, causing the CLI to track the wrong path. Consider sending the same normalized/ensured filename you actually renamed to (and use that consistently for lastSnapshot too).
| const content = this.lastSnapshot.get(normalisedOld) | |
| if (content !== undefined) { | |
| this.lastSnapshot.delete(normalisedOld) | |
| this.lastSnapshot.set(normalisedNew, content) | |
| } | |
| const codeFiles = await framer.getCodeFiles() | |
| const existing = codeFiles.find(file => canonicalFileName(file.path || file.name) === normalisedOld) | |
| if (existing) { | |
| await existing.rename(ensureExtension(normalisedNew)) | |
| socket.send( | |
| JSON.stringify({ | |
| type: "file-synced", | |
| fileName: normalisedNew, | |
| const ensuredNew = ensureExtension(normalisedNew) | |
| const content = this.lastSnapshot.get(normalisedOld) | |
| if (content !== undefined) { | |
| this.lastSnapshot.delete(normalisedOld) | |
| this.lastSnapshot.set(ensuredNew, content) | |
| } | |
| const codeFiles = await framer.getCodeFiles() | |
| const existing = codeFiles.find(file => canonicalFileName(file.path || file.name) === normalisedOld) | |
| if (existing) { | |
| await existing.rename(ensuredNew) | |
| socket.send( | |
| JSON.stringify({ | |
| type: "file-synced", | |
| fileName: ensuredNew, |
|
Dang ok I guess I will start doing copilot + devin + cursor haha |
Description
This PR implements proper file rename detection in the code-link CLI and fixes the echo event bug that was causing false deletions. The changes include:
codeFile.rename()API to properly rename files without deletionrace?.tsx→race_.tsxorNew Folder With Items/→New_Folder_With_Items/)Without the echo suppression fix, renaming files locally would trigger a delete operation on the remote plugin, undoing the rename. The 100ms buffer for rename detection ensures that both unlink and add events are matched regardless of their arrival order.
Changelog
file-renamemessage type for CLI-to-plugin communicationSEND_FILE_RENAMEeffect in CLI controller to handle rename eventsapplyRemoteRename()method in plugin API using Framer'scodeFile.rename()Testing