Fix: delete file issues#18
Merged
alaa-alsalehi merged 22 commits intoMay 5, 2026
Merged
Conversation
Vue reactive() wraps child Doc rows, so row instanceof Doc was false and filesystem refs in table rows were skipped. Unwrap with toRaw before recursing in snapshot and delete cleanup.
Handle Attachment values stored as JSON strings after load(), and avoid using a private cleanup method so deletion works on Vue reactive proxies.
Move attachment normalization, ref scanning, and IPC cleanup out of Doc into a dedicated class while preserving behavior.
Add IPC stage save/commit/delete under the OS temp directory, use books-staged tokens in the model until save, commit to the DB attachments folder before insert/update, and discard staged files on reload. Update attachment/image resolvers to read staged paths for preview.
…ager - Main: parseAttachmentParams, normalizeIncomingAttachmentBytes, and writeStampedAttachmentFile shared by ATTACHMENT_SAVE and ATTACHMENT_STAGE_SAVE. - AttachmentManager: ipcStageSave/ipcFinalSave and reconcile helpers for normalizeBeforeSet (Attachment + AttachImage).
…Edit - After duplicate, read committed files and stageSave so the copy uses books-staged refs (child tables included); snapshot refreshed. - Add duplicateForEdit() combining duplicate() + remap; use in Duplicate action and Shipment spec. Keep duplicate() sync for headless use. - JSDoc: mime guess for IPC read+stage (read returns no type).
…mmit - commitStagedBeforeDbWrite returns relative paths created by stageCommit. - recoverAfterFailedDbWrite: IPC delete those paths, clear pendingDeletes, reload doc on failed update, clear affected fields + snapshot on failed insert. - Doc.sync uses try/finally so _syncing resets when insert/update throws.
…inference - Add utils/mimeType.ts with mimeTypeFromFilename. - ATTACHMENT_READ now returns type based on file name. - AttachmentManager duplicate remap prefers readRes.type and drops local guessing.
encodeBooksStagedPath now throws when neither btoa nor Buffer is available, preventing non-base64 staged refs that would break decodeBooksStagedPath. Also tighten Buffer typing to satisfy strict lint rules.
…ncoder/TextDecoder - Encode UTF-8 bytes then base64 (btoa/atob) or Buffer. - Decode base64 back to UTF-8 without deprecated escape/unescape. - Keeps staging refs guaranteed base64 and round-trippable.
- Add readAttachmentAsDataURL helper (ipc.attachments.read -> data URL). - Use it for both books-staged and books-file refs. - Fix misleading comment for staged refs.
- attachmentEncoding: return null (not empty base64) when no encoder exists; log btoa and Buffer encoding errors with context. - DocAttachmentManager: handle null dataUrlFromBytes by falling back to the original value/null instead of producing empty data URLs.
Keep cleanup best-effort, but record errors from attachments.cleanupBeforeDelete() so failures are visible in logs.
- Reuse a single isDocLike type guard across DocAttachmentManager. - Log stageCommit failures for Attachment/AttachImage to avoid silent partial commit.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces ChangesAttachment Staging Infrastructure
Sequence DiagramsequenceDiagram
actor Renderer
participant DocMgr as DocAttachmentManager
participant Doc
participant IPC as IPC (Main)
participant FS as Filesystem
rect rgba(100, 150, 200, 0.5)
Note over Renderer,FS: Attachment Normalization & Staging
Renderer->>DocMgr: normalizeBeforeSet(field, bytes)
activate DocMgr
DocMgr->>IPC: attachments.stageSave(name, type, data)
activate IPC
IPC->>FS: write bytes to temp staging dir
IPC-->>DocMgr: { stagePath, attachment }
deactivate IPC
DocMgr-->>Renderer: { path: "books-staged:..." }
deactivate DocMgr
end
rect rgba(150, 100, 150, 0.5)
Note over Renderer,FS: Insert/Update Lifecycle
Renderer->>Doc: insert()
activate Doc
Doc->>DocMgr: commitStagedBeforeDbWrite()
activate DocMgr
DocMgr->>IPC: attachments.stageCommit(stagePath)
activate IPC
IPC->>FS: move staged → permanent attachment dir
IPC-->>DocMgr: { attachment: { path: "committed/path" } }
deactivate IPC
DocMgr->>DocMgr: rewrite field to final path
DocMgr-->>Doc: [ committed paths ]
deactivate DocMgr
Doc->>IPC: _insertChild() / _updateChild() to DB
IPC-->>Doc: success
deactivate Doc
end
rect rgba(200, 150, 100, 0.5)
Note over Renderer,FS: Recovery on DB Write Failure
Renderer->>Doc: insert() fails
activate Doc
Doc->>DocMgr: recoverAfterFailedDbWrite(committedPaths)
activate DocMgr
DocMgr->>IPC: attachments.delete(committedPath)
activate IPC
IPC->>FS: remove newly committed file
IPC-->>DocMgr: success
deactivate IPC
DocMgr->>DocMgr: reload or clear matched fields
DocMgr-->>Doc: recovery complete
deactivate DocMgr
deactivate Doc
end
rect rgba(100, 200, 150, 0.5)
Note over Renderer,FS: Duplication with Remapping
Renderer->>Doc: duplicateForEdit()
activate Doc
Doc->>Doc: duplicate() → new doc
Doc->>DocMgr: remapCommittedFilesystemAttachmentsToStagedAfterDuplicate()
activate DocMgr
loop for each committed attachment/image
DocMgr->>IPC: attachments.read(path)
activate IPC
IPC->>FS: read committed file bytes
IPC-->>DocMgr: { data: Uint8Array }
deactivate IPC
DocMgr->>IPC: attachments.stageSave(data)
activate IPC
IPC->>FS: write to new staging dir
IPC-->>DocMgr: { stagePath }
deactivate IPC
DocMgr->>DocMgr: rewrite field to staged ref
end
DocMgr-->>Doc: remapped
deactivate DocMgr
Doc-->>Renderer: new duplicated doc
deactivate Doc
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
utils/mimeType.ts (1)
1-13: 💤 Low valueConsider
path.extname+ a lookup map for maintainability.Chained
endsWithcalls work correctly here, but as the extension list grows they become verbose. A lookup map keyed by extension is easier to extend.♻️ Suggested refactor
-export function mimeTypeFromFilename(filename: string): string { - const lower = filename.toLowerCase(); - if (lower.endsWith('.png')) return 'image/png'; - if (lower.endsWith('.jpg') || lower.endsWith('.jpeg')) return 'image/jpeg'; - if (lower.endsWith('.gif')) return 'image/gif'; - if (lower.endsWith('.webp')) return 'image/webp'; - if (lower.endsWith('.svg')) return 'image/svg+xml'; - if (lower.endsWith('.pdf')) return 'application/pdf'; - if (lower.endsWith('.txt')) return 'text/plain'; - if (lower.endsWith('.csv')) return 'text/csv'; - if (lower.endsWith('.json')) return 'application/json'; - return 'application/octet-stream'; -} +import path from 'path'; + +const MIME_MAP: Record<string, string> = { + '.png': 'image/png', + '.jpg': 'image/jpeg', + '.jpeg': 'image/jpeg', + '.gif': 'image/gif', + '.webp': 'image/webp', + '.svg': 'image/svg+xml', + '.pdf': 'application/pdf', + '.txt': 'text/plain', + '.csv': 'text/csv', + '.json': 'application/json', +}; + +export function mimeTypeFromFilename(filename: string): string { + const ext = path.extname(filename).toLowerCase(); + return MIME_MAP[ext] ?? 'application/octet-stream'; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/mimeType.ts` around lines 1 - 13, Replace the chained endsWith checks in mimeTypeFromFilename with a lookup map keyed by extensions and use Node's path.extname to extract the extension; create a const map (e.g., const EXT_TO_MIME) mapping '.png' -> 'image/png', etc., call const ext = path.extname(filename).toLowerCase(), then return EXT_TO_MIME[ext] || 'application/octet-stream' inside mimeTypeFromFilename (ensure you import path). This makes the function easier to extend and centralizes extension-to-MIME mappings.fyo/model/attachmentEncoding.ts (1)
6-12: ⚡ Quick winChunked
String.fromCharCodelogic is duplicated inutils/attachmentStagingRef.ts.
bytesToBinaryStringinutils/attachmentStagingRef.ts(Lines 38-46) is the exact same algorithm as the inline loop here. Both files are new in this PR. Extracting this into a shared utility (e.g.,utils/encodingHelpers.ts) would prevent the two copies from diverging silently.♻️ Suggested shared utility
// utils/encodingHelpers.ts (new file) +export function uint8ArrayToBinaryString(bytes: Uint8Array): string { + let out = ''; + const chunkSize = 0x8000; + for (let i = 0; i < bytes.length; i += chunkSize) { + out += String.fromCharCode(...bytes.subarray(i, i + chunkSize)); + } + return out; +}Then in
attachmentEncoding.tsandattachmentStagingRef.ts, import and calluint8ArrayToBinaryStringinstead of inlining the loop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fyo/model/attachmentEncoding.ts` around lines 6 - 12, Duplicate chunked String.fromCharCode logic should be extracted into a shared helper: create a new utility (e.g., export function uint8ArrayToBinaryString(bytes: Uint8Array)) that implements the existing chunked loop used in attachmentEncoding.ts and utils/attachmentStagingRef.ts, then replace the inline loop in attachmentEncoding.ts (the block producing `binary` and calling `btoa`) and replace bytesToBinaryString in utils/attachmentStagingRef.ts to call the new uint8ArrayToBinaryString, and add the appropriate imports so both modules reuse the single implementation.utils/attachmentStagingRef.ts (1)
38-46: ⚡ Quick win
bytesToBinaryStringduplicates the chunkedString.fromCharCodeloop infyo/model/attachmentEncoding.ts.Both this file and
fyo/model/attachmentEncoding.ts(Lines 6–10) implement the identical chunked-spread algorithm. See the shared-utility suggestion raised onfyo/model/attachmentEncoding.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/attachmentStagingRef.ts` around lines 38 - 46, The bytesToBinaryString function duplicates the chunked String.fromCharCode loop that exists elsewhere; extract this chunked conversion into a single exported utility (keep the exported name bytesToBinaryString or a clearly named alternative) and replace the local implementation here with an import/use of that shared utility; update the other module that currently contains the same loop to import the new shared function and remove its duplicate implementation so both modules call the single exported function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fyo/model/doc.ts`:
- Around line 917-920: The code commits staged attachments
(attachments.commitStagedBeforeDbWrite()) before calling _preSync() (and
setName() during inserts), which can throw and leave committed files orphaned;
move the call to _preSync() (and ensure setName() runs for inserts) to occur
before committing staged files, and instead call
attachments.commitStagedBeforeDbWrite() immediately before the actual DB write
(insert/update) so recoverAfterFailedDbWrite() can run on DB failures; update
_insert (and the analogous update path around lines 939-943) to perform
_preSync()/setName() first, then commit staged attachments just prior to the DB
write and keep recoverAfterFailedDbWrite() error handling intact.
- Around line 1042-1052: Call trigger('beforeDelete') as before, but before
performing any filesystem cleanup capture the attachment/image references from
this.attachments (e.g. add/use a method like attachments.extractRefs() or
attachments.getRefs() to return the list of file keys/paths), then call await
this.fyo.db.delete(this.schemaName, this.name!), and only after the DB delete
attempt do a best-effort cleanup of the captured refs (instead of calling
attachments.cleanupBeforeDelete() before the DB delete); ensure cleanup runs in
a try/catch that logs errors but does not throw so failed cleanup won’t roll
back the already-deleted DB row.
In `@fyo/model/DocAttachmentManager.ts`:
- Around line 481-525: The duplication logic in DocAttachmentManager leaves the
original committed attachment path on the duplicate when ipcApi.attachments.read
or `#ipcStageSave` fails; instead, on any failure after calling
`#normalizeAttachmentRow` (i.e., failed readRes success/data check or missing
stagePath), clear the attachment from the duplicated doc (remove doc[fieldname]
or set it to null) and surface the failure (throw or return an error) so
duplicateForEdit callers can't keep a reference to the source file; update the
branches around the readRes check and the stagePath check to delete/clear
doc[fieldname] and propagate the error rather than silently continue.
- Around line 721-779: When the attachment field is cleared or set to a plain
string we must reconcile any previous staged attachment before returning; update
the logic in DocAttachmentManager methods that handle FieldTypeEnum.Attachment
(e.g., the block using local variables prev and prevPath and the similar block
around lines 781-824) to compute prev/prevPath (and prevStr for plain-string
cases) up front and call await this.#reconcilePreviousAttachmentPath(prevPath)
(or reconcile using prevStr) before any early return for null/strings so staged
temp files are cleaned up; ensure the calls are awaited and do not change
existing return values.
- Around line 321-347: The Attachment branch currently just logs and continues
on ipcApi.attachments.stageCommit failure; instead, when stageCommit returns no
newPath you must roll back any earlier commits from this pass and abort the
save: iterate the createdPaths array and call the attachments revert API (e.g.
ipcApi.attachments.stageRevert or the appropriate rollback/delete method) with
dbPath and each committed path to undo them, then throw an Error (or return a
rejected promise) so the caller (save path in fyo/model/doc.ts) does not write a
books-staged: ref or leave a partial commit; update the failure branch in
DocAttachmentManager (the block handling res?.success false / missing newPath
for fieldname) to perform the rollback calls and then throw.
In `@main/registerIpcMainActionListeners.ts`:
- Around line 64-71: The validation in isPathInsideStageRoot currently allows
stagePath equal to the stage root (resolved === normalizedPrefix), which enables
deleting or committing the entire staging directory; change the check so only
paths strictly inside the stage root are allowed (i.e., remove the equality
branch and require resolved.startsWith(normalizedPrefix + path.sep) only) or,
alternatively, replace the logic with path.relative(normalizedPrefix, resolved)
and ensure the relative path is non-empty and does not start with '..' so that
the stage root itself is rejected; update the function isPathInsideStageRoot and
its use sites accordingly.
In `@src/utils/attachments.ts`:
- Around line 38-40: The current readPath assignment lets a failed
decodeBooksStagedPath(null) fall back to the raw "books-staged:..." string and
triggers a wasted IPC fs.readFile; change the logic in the attachments handling
so that when isBooksStagedRef(attachment.path) is true and
decodeBooksStagedPath(attachment.path) returns null you return null immediately
(same pattern used in resolveAttachImageSrc) instead of assigning
attachment.path to readPath; adjust the code around readPath and any callers to
bail out early on that null decode to avoid the IPC round-trip.
---
Nitpick comments:
In `@fyo/model/attachmentEncoding.ts`:
- Around line 6-12: Duplicate chunked String.fromCharCode logic should be
extracted into a shared helper: create a new utility (e.g., export function
uint8ArrayToBinaryString(bytes: Uint8Array)) that implements the existing
chunked loop used in attachmentEncoding.ts and utils/attachmentStagingRef.ts,
then replace the inline loop in attachmentEncoding.ts (the block producing
`binary` and calling `btoa`) and replace bytesToBinaryString in
utils/attachmentStagingRef.ts to call the new uint8ArrayToBinaryString, and add
the appropriate imports so both modules reuse the single implementation.
In `@utils/attachmentStagingRef.ts`:
- Around line 38-46: The bytesToBinaryString function duplicates the chunked
String.fromCharCode loop that exists elsewhere; extract this chunked conversion
into a single exported utility (keep the exported name bytesToBinaryString or a
clearly named alternative) and replace the local implementation here with an
import/use of that shared utility; update the other module that currently
contains the same loop to import the new shared function and remove its
duplicate implementation so both modules call the single exported function.
In `@utils/mimeType.ts`:
- Around line 1-13: Replace the chained endsWith checks in mimeTypeFromFilename
with a lookup map keyed by extensions and use Node's path.extname to extract the
extension; create a const map (e.g., const EXT_TO_MIME) mapping '.png' ->
'image/png', etc., call const ext = path.extname(filename).toLowerCase(), then
return EXT_TO_MIME[ext] || 'application/octet-stream' inside
mimeTypeFromFilename (ensure you import path). This makes the function easier to
extend and centralizes extension-to-MIME mappings.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20871582-0f48-4d4e-851b-753f7e270975
📒 Files selected for processing (12)
fyo/model/DocAttachmentManager.tsfyo/model/attachmentEncoding.tsfyo/model/doc.tsmain/preload.tsmain/registerIpcMainActionListeners.tsmodels/inventory/tests/testStockTransfer.spec.tspackage.jsonsrc/utils/attachments.tssrc/utils/ui.tsutils/attachmentStagingRef.tsutils/messages.tsutils/mimeType.ts
Comment on lines
+721
to
+779
| if (field.fieldtype === FieldTypeEnum.Attachment) { | ||
| const v = value as | ||
| | null | ||
| | undefined | ||
| | { | ||
| name?: string; | ||
| type?: string; | ||
| data?: string; | ||
| path?: string; | ||
| bytes?: Uint8Array; | ||
| }; | ||
| if (!v) return value; | ||
|
|
||
| const prev = this.#doc.get(field.fieldname) as any; | ||
| const prevPath = | ||
| typeof prev?.path === 'string' ? prev.path : null; | ||
|
|
||
| if (v.bytes instanceof Uint8Array && v.name && v.type) { | ||
| if (canStage) { | ||
| const stagePath = await this.#ipcStageSave( | ||
| ipcApi, | ||
| dbPath!, | ||
| v.name, | ||
| v.type, | ||
| v.bytes | ||
| ); | ||
| if (stagePath) { | ||
| await this.#reconcilePreviousAttachmentPath(prevPath); | ||
| return { | ||
| name: v.name, | ||
| type: v.type, | ||
| path: encodeBooksStagedPath(stagePath), | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| if (canUseFs) { | ||
| const newPath = await this.#ipcFinalSave( | ||
| ipcApi, | ||
| dbPath!, | ||
| v.name, | ||
| v.type, | ||
| v.bytes | ||
| ); | ||
| if (newPath) { | ||
| await this.#reconcilePreviousAttachmentPath(prevPath); | ||
| return { name: v.name, type: v.type, path: newPath }; | ||
| } | ||
| } | ||
|
|
||
| const dataUrl = dataUrlFromBytes(v.type, v.bytes); | ||
| if (!dataUrl) { | ||
| return value; | ||
| } | ||
| return { name: v.name, type: v.type, data: dataUrl }; | ||
| } | ||
|
|
||
| return value; | ||
| } |
There was a problem hiding this comment.
Reconcile previous staged files when the field is cleared.
Both branches return early for null/plain string input before looking at the current value. If the previous value was a staged ref and the user clears the field, the temp file becomes unreachable and discardStagedOnReload() can no longer clean it up. Reconcile prevPath / prevStr before those early returns.
Also applies to: 781-824
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@fyo/model/DocAttachmentManager.ts` around lines 721 - 779, When the
attachment field is cleared or set to a plain string we must reconcile any
previous staged attachment before returning; update the logic in
DocAttachmentManager methods that handle FieldTypeEnum.Attachment (e.g., the
block using local variables prev and prevPath and the similar block around lines
781-824) to compute prev/prevPath (and prevStr for plain-string cases) up front
and call await this.#reconcilePreviousAttachmentPath(prevPath) (or reconcile
using prevStr) before any early return for null/strings so staged temp files are
cleaned up; ensure the calls are awaited and do not change existing return
values.
Comment on lines
+38
to
+40
| const readPath = isBooksStagedRef(attachment.path) | ||
| ? decodeBooksStagedPath(attachment.path) ?? attachment.path | ||
| : attachment.path; |
There was a problem hiding this comment.
Fallback to the raw staged ref string on decode failure triggers a guaranteed-to-fail IPC call.
When decodeBooksStagedPath returns null (corrupt/unparseable ref), ?? attachment.path passes the literal books-staged:... string as the filesystem path. The main process will attempt fs.readFile on a non-existent path, fail, and the caller returns null anyway — but only after a wasted IPC round-trip. resolveAttachImageSrc's staged branch (Line 119) correctly guards with an early return null instead; the same pattern should be applied here.
🐛 Proposed fix
- const readPath = isBooksStagedRef(attachment.path)
- ? decodeBooksStagedPath(attachment.path) ?? attachment.path
- : attachment.path;
-
- const res = (await ipcApi.attachments.read({
- dbPath,
- path: readPath,
- })) as { success?: boolean; data?: Uint8Array };
+ let readPath: string;
+ if (isBooksStagedRef(attachment.path)) {
+ const decoded = decodeBooksStagedPath(attachment.path);
+ if (!decoded) return null;
+ readPath = decoded;
+ } else {
+ readPath = attachment.path;
+ }
+
+ const res = (await ipcApi.attachments.read({
+ dbPath,
+ path: readPath,
+ })) as { success?: boolean; data?: Uint8Array };📝 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
| const readPath = isBooksStagedRef(attachment.path) | |
| ? decodeBooksStagedPath(attachment.path) ?? attachment.path | |
| : attachment.path; | |
| let readPath: string; | |
| if (isBooksStagedRef(attachment.path)) { | |
| const decoded = decodeBooksStagedPath(attachment.path); | |
| if (!decoded) return null; | |
| readPath = decoded; | |
| } else { | |
| readPath = attachment.path; | |
| } | |
| const res = (await ipcApi.attachments.read({ | |
| dbPath, | |
| path: readPath, | |
| })) as { success?: boolean; data?: Uint8Array }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/attachments.ts` around lines 38 - 40, The current readPath
assignment lets a failed decodeBooksStagedPath(null) fall back to the raw
"books-staged:..." string and triggers a wasted IPC fs.readFile; change the
logic in the attachments handling so that when isBooksStagedRef(attachment.path)
is true and decodeBooksStagedPath(attachment.path) returns null you return null
immediately (same pattern used in resolveAttachImageSrc) instead of assigning
attachment.path to readPath; adjust the code around readPath and any callers to
bail out early on that null decode to avoid the IPC round-trip.
- Capture committed/staged attachment refs before DB delete. - Delete DB row first, then cleanup captured refs best-effort (log failures). - Prevent cleanup errors from blocking deletion.
- Defer doc field updates until all stageCommit calls succeed. - If any stageCommit fails, delete already-committed paths (best-effort) and throw to abort save, preventing mixed staged/committed refs.
- If reading or stageSave fails during duplicate remap, clear the duplicated field and throw so the copy cannot keep a reference to the source file.
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.
https://tasko.rukn.sh/tasko/task/TASK-2026-00452
Summary by CodeRabbit
New Features
Bug Fixes
Chores