Skip to content

feat: Enable file editing and saving#252

Merged
shantur merged 12 commits intoNeuralNomadsAI:devfrom
jchadwick:file-editing
Mar 29, 2026
Merged

feat: Enable file editing and saving#252
shantur merged 12 commits intoNeuralNomadsAI:devfrom
jchadwick:file-editing

Conversation

@jchadwick
Copy link
Copy Markdown
Contributor

@jchadwick jchadwick commented Mar 26, 2026

Summary

  • Adds file writing capability to Monaco editor in the file viewer
  • Implements writeFile API on the server for workspace files
  • Integrates save functionality into the file viewer UI with proper state management

Bug Fixes (Review Feedback)

  • Fixed failed save discarding edits when switching files - now checks save result and only proceeds if successful
  • Fixed refresh overwriting dirty editor state - now prompts for confirmation before discarding edits
  • Fixed save button unable to save empty files - changed check from if (content) to if (content !== undefined && content !== null)
  • Added agent edit conflict detection - when agent edits file while user has unsaved changes, shows conflict dialog with Overwrite/Cancel options
  • Fixed dialog appearing behind unpinned sidebar - increased alert dialog z-index to z-100

Related Issues

@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23616188660

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@jchadwick
Copy link
Copy Markdown
Contributor Author

jchadwick commented Mar 26, 2026

Does not implement the other file management features, e.g. create new, rename, delete.
Personally I would like these features - happy to create a separate issue and PR.

@jchadwick jchadwick changed the title feat: add file saving to Monaco editor feat: Enable file editing and saving Mar 26, 2026
@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 26, 2026

@jchadwick - Thanks for the PR
I will review and merge this one for now.
I have feature in mind to be able to support better file editing support so please wait for me.

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 26, 2026

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

  • check for regressions
  • check for better possible implementations
  • Be the PR gatekeeper
  • Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot bot commented Mar 26, 2026

I reviewed the latest PR state as a gatekeeper against origin/dev.

What I understood: this PR adds editable file viewing plus save support for workspace files. I checked for regressions in the save flow, correctness of the server write path, and whether the implementation matches existing project patterns.

Verdict: not ready to merge yet. I found a few user-facing correctness issues.

  1. Blocker - failed save still discards edits when switching files
  • In packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx:614, handleOpenBrowserFileRequest() always calls openBrowserFile(path) after the confirm dialog.
  • If the user chooses Save and saveBrowserFile() fails, the code still opens the next file and the unsaved edits are lost.
  • This is the highest-risk regression in the PR because it turns a transient save failure into silent data loss.
  1. Major - refresh can overwrite dirty editor state with no confirmation
  • In packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx:695, refreshFilesTab() re-reads the selected file from disk and calls setBrowserSelectedContent(text) even when browserSelectedDirty() is true.
  • That means clicking refresh while editing can replace in-memory edits with on-disk contents without any confirmation.
  • It also leaves dirty-state handling inconsistent, because the user may still appear to have unsaved changes after their edited text was replaced.
  1. Major - save button cannot save an empty file
  • In packages/ui/src/components/instance/shell/right-panel/tabs/FilesTab.tsx:45, handleSave() only saves when if (content) is truthy.
  • An edited file whose intended contents are "" cannot be saved via the toolbar button.
  • Ctrl+S works because Monaco passes the string directly, so this creates inconsistent behavior between the keyboard shortcut and the visible save action.
  1. Improvement - this bypasses the existing API client wrapper
  • packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx:580 uses raw fetch(...) for saving, while reads already go through the existing client/request helpers.
  • I would prefer adding a writeWorkspaceFile(...) helper in packages/ui/src/lib/api-client.ts and reusing the same request/error conventions used elsewhere.
  • I do not consider this a blocker by itself, but it is a better implementation path.

Verification notes:

  • git diff --check origin/dev...HEAD is clean.
  • I could not run meaningful typechecks/build verification in this worktree because dependencies are not installed (node_modules is absent), so I reviewed the code path manually instead.

Recommendation: fix items 1-3 before merge. After that, item 4 would be a nice cleanup if you want this to align better with the existing client architecture.

--
Yours,
CodeNomadBot

@shantur shantur marked this pull request as draft March 26, 2026 20:42
@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 26, 2026

@jchadwick - Can you please review the issues identified

Jess Chadwick added 2 commits March 26, 2026 17:05
When switching files with unsaved changes, the previous implementation would
always proceed to open the new file regardless of whether save succeeded.
Now checks the save result and only proceeds if save was successful.
If user cancels, clears dirty state before switching.
Also increases alert dialog z-index to appear above unpinned sidebar
and adds writeWorkspaceFile helper to api-client for consistency.
@jchadwick jchadwick marked this pull request as ready for review March 26, 2026 21:11
@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23618242558

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@jchadwick jchadwick marked this pull request as draft March 26, 2026 21:18
Jess Chadwick added 2 commits March 26, 2026 17:21
- Use fixed positioning with transform for reliable centering
- Remove wrapper div that was causing positioning issues
- Add proper aria attributes to Dialog.Content
- Overlay: z-[60]
- Content: z-[70]
- Sidebar: z-40
- Backdrop: z-30
@jchadwick jchadwick marked this pull request as ready for review March 26, 2026 21:53
@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23619879820

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@jchadwick
Copy link
Copy Markdown
Contributor Author

jchadwick commented Mar 26, 2026

@shantur fixed and ready.
Can you take a look at the Dialog changes to double-check - this is an existing bug where the confirmation dialog was hidden under an overlay when the sidebar was not pinned. Happy to break this out into a separate issue/PR if you want.

https://github.com/NeuralNomadsAI/CodeNomad/pull/252/changes#diff-8edfcceae7f244fef970f374c93b5210a57b792438d223e61e8407ee2cde342c

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 26, 2026

@CodeNomadBot

Do your thing again

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot bot commented Mar 26, 2026

I reviewed the latest PR state again against origin/dev.

What I understood: you wanted a fresh gatekeeper pass after the follow-up fixes, including the dialog layering change. I re-checked the previously reported save/refresh regressions, the new conflict detection flow, and the alert dialog behavior.

Verdict: much better, but I still see one remaining correctness issue before merge.

  1. Major - dismissing the save/discard dialog can still discard edits unintentionally
  • showConfirmDialog(...) resolves to false both when the user explicitly clicks the cancel button and when the dialog is dismissed via onOpenChange in packages/ui/src/components/alert-dialog.tsx:110.
  • In packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx:657, that false path is treated as "discard changes":
    • it clears dirty state
    • then immediately opens the next file
  • So if the user closes the dialog with Escape, backdrop click, or any other non-explicit dismiss path, their unsaved edits are discarded even though they did not actually choose Discard Changes.
  • Because the cancel button label is now explicitly Discard Changes, conflating dismiss with discard is especially risky and surprising.

Suggested fix:

  • Either prevent outside/Escape dismissal for destructive confirm dialogs, or
  • distinguish dismissed from cancelled/discarded in the alert dialog API so only an explicit discard action clears edits.

What looks good now:

  • The failed-save-on-switch regression appears fixed.
  • Refresh now asks before discarding dirty state.
  • The toolbar save button now handles empty-string content.
  • Moving save writes into serverApi.writeWorkspaceFile(...) is the better implementation path.
  • The z-index change in packages/ui/src/components/alert-dialog.tsx looks directionally correct for the sidebar overlay issue.

Verification notes:

  • git diff --check origin/dev...HEAD is clean.
  • I still could not run full typecheck/build verification in this worktree because dependencies are not installed (node_modules is absent), so this pass is code-review based.

Recommendation: fix the confirm-dialog dismiss-vs-discard behavior, then this should be in much better shape for merge.

Side note: packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx is now about 991 lines, so it is well past the file-length warning threshold and worth keeping an eye on for future refactoring, but I would not block this PR on that alone.

--
Yours,
CodeNomadBot

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 26, 2026

@jchadwick - Please check the new review

…ia Escape/backdrop

Add dismissible prop to AlertDialogState. When false, the dialog can only
be closed via explicit button clicks (Confirm or Cancel), preventing
Escape key or backdrop click from dismissing destructive confirmations.

This fixes data loss where unsaved edits were discarded when users
closed the save confirmation dialog without explicitly choosing
'Discard Changes'.
@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23622710761

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 27, 2026

@CodeNomadBot

Do your thing again

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot bot commented Mar 27, 2026

I reviewed the latest PR state again against origin/dev.

What I understood: you wanted one more gatekeeper pass after the confirm-dialog dismissal fix. I re-checked the save/discard flows, the new dismissible dialog behavior, and whether the previous correctness issue is actually closed out.

Verdict: this looks good to merge.

What changed correctly:

  • The previous bug is fixed: the destructive file-editing confirms in packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx:595, packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx:644, and packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx:734 now pass dismissible: false.
  • packages/ui/src/components/alert-dialog.tsx:110 now respects payload.dismissible !== false, so Escape/backdrop dismissal no longer gets conflated with explicit discard for those flows.
  • The same pattern was also applied to other destructive/restart confirmations (packages/ui/src/components/session-list.tsx:153, packages/ui/src/components/instance-info.tsx:42, packages/ui/src/components/remote-access-overlay.tsx:96, packages/ui/src/components/settings/remote-access-settings-section.tsx:82, packages/ui/src/stores/session-state.ts:669), which is a sensible consistency improvement rather than a drive-by refactor.
  • The earlier save-flow fixes still look intact: failed save no longer advances file selection, refresh now confirms before discarding edits, empty-string saves are allowed, and the save API now goes through serverApi.writeWorkspaceFile(...).
  • The dialog z-index/portal structure change still looks correct for the sidebar overlay issue.

Gatekeeper result:

  • No new blockers found in this pass.
  • git diff --check origin/dev...HEAD is clean.
  • I still could not run full typecheck/build verification in this worktree because dependencies are not installed (node_modules is absent), so this remains a code-review-based approval rather than runtime validation.

One non-blocking note:

  • packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx is now about 994 lines, which is past the file-length warning threshold. I would not block merge on that, but it is worth planning a follow-up split later.

From a gatekeeper/code-quality standpoint, I do not see anything left here that should stop merge.

--
Yours,
CodeNomadBot

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 27, 2026

@jchadwick - Thanks for fixing the issues raised. It needs a few changes

  1. Currently, save button only appears when file is edited, we can keep the save button visible but disabled when no changes. This allows user to know files can be edited
  2. The save button needs to be right aligned next to the reload button

Rest looks good. I will be happy to merge in

@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23718738352

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@jchadwick
Copy link
Copy Markdown
Contributor Author

@shantur no problem, here's the fix:
image

Also includes a bug fix for the dialog z-index so that it shows over the sidebars - was previously showing behind them.
image

@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23718822880

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@jchadwick
Copy link
Copy Markdown
Contributor Author

BTW, this is pretty cool, using CodeNomad to code CodeNomad 😆

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 29, 2026

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

  • check for regressions
  • check for better possible implementations
  • Be the PR gatekeeper
  • Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot bot commented Mar 29, 2026

I reviewed the latest PR state again against origin/dev.

What I understood: you wanted a fresh gatekeeper pass on the newest revision, especially the follow-up save-button UX tweak and dialog layering fix. I re-checked regressions in the file-editing flow, looked for better implementation choices, and reviewed the latest changes with a PR-gatekeeper mindset.

Verdict: I still see 2 issues worth addressing before merge.

  1. Major - the dialog is visually above the floating sidebar, but the overlay is still below it, so the modal is not actually modal for sidebar interactions
  • packages/ui/src/components/alert-dialog.tsx:118-119 sets the overlay to z-[60] and the dialog content to z-[1310].
  • The floating right sidebar is rendered via MUI Drawer (packages/ui/src/components/instance/instance-shell2.tsx:526-548), which sits in MUI's modal layer. That means the dialog body can appear above the drawer while the overlay stays underneath it.
  • Result: the sidebar can still receive clicks/interaction while a confirmation dialog is open. That is a behavioral regression for a modal dialog and can let users change underlying UI state while a destructive confirm is active.
  • Better implementation: move the overlay and content into the same higher stacking layer, or centralize modal z-index via a shared token/class instead of splitting overlay/content across unrelated z-index bands.
  1. Major - new file-editing strings are still missing from 5 supported locales
  • The new keys only exist in packages/ui/src/lib/i18n/messages/en/instance.ts:98-109 and packages/ui/src/lib/i18n/messages/he/instance.ts:98-109.
  • The repo currently also ships es, fr, ja, ru, and zh-Hans, but those locale files do not include the new instanceShell.rightPanel.actions.* and instanceShell.rightPanel.toast.* keys.
  • The repo's i18n guidance explicitly says to add each new key to every locale file because English fallback can hide gaps. This is exactly that case.
  • I would treat this as merge-worthy cleanup rather than a drive-by enhancement, because these strings are part of the feature being added.

What still looks good:

  • The failed-save-on-switch regression remains fixed.
  • Refresh now confirms before discarding dirty edits.
  • Empty-string saves are allowed.
  • Routing saves through serverApi.writeWorkspaceFile(...) is the better implementation.
  • The save button is now consistently visible and aligned with the refresh button, matching the review request.

Verification notes:

  • git diff --check origin/dev...HEAD is clean.
  • I still could not run typecheck/build in this worktree because dependencies are not installed (node_modules is absent), so this pass is code-review based.

Non-blocking note:

  • packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx is still oversized at roughly 994 lines. I would not block this PR on that alone, but it remains a refactoring signal.

Recommendation: fix the modal overlay stacking and add the missing locale keys, then I would be comfortable clearing this again.

--
Yours,
CodeNomadBot

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 29, 2026

@jchadwick - Can you please review the issues identified by CodeNomadBot

@jchadwick
Copy link
Copy Markdown
Contributor Author

@shantur First issue is unfounded. The z-index is working fine with sidebar pinned and unpinned:

image image

2: Do you want me to have AI generate the locale strings for those languages?

@jchadwick
Copy link
Copy Markdown
Contributor Author

Oh wait, I misunderstood. I guess that first image is what issue 1 is referring to... I'll take a look.

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 29, 2026

Do you want me to have AI generate the locale strings for those languages?

Yes please

@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23719461524

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@github-actions
Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/23719556852

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@jchadwick
Copy link
Copy Markdown
Contributor Author

@shantur alright, both issues fixed.

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 29, 2026

@CodeNomadBot

  • Do your thing again

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot bot commented Mar 29, 2026

I reviewed the latest PR state again against origin/dev.

What I understood: you wanted another gatekeeper pass after the two follow-up fixes from the previous review. I re-checked the latest delta specifically for the modal layering issue and the missing locale coverage, while also sanity-checking the existing file-editing/save flow for regressions.

Verdict: the newly raised issues appear addressed, and I do not see any new blockers in this pass.

What changed correctly:

  • The missing locale coverage is now fixed. The new right-panel action/toast strings are present in all supported locale files:
    • packages/ui/src/lib/i18n/messages/en/instance.ts
    • packages/ui/src/lib/i18n/messages/es/instance.ts
    • packages/ui/src/lib/i18n/messages/fr/instance.ts
    • packages/ui/src/lib/i18n/messages/he/instance.ts
    • packages/ui/src/lib/i18n/messages/ja/instance.ts
    • packages/ui/src/lib/i18n/messages/ru/instance.ts
    • packages/ui/src/lib/i18n/messages/zh-Hans/instance.ts
  • The floating right drawer now explicitly uses zIndex: 60 in packages/ui/src/components/instance/instance-shell2.tsx:533-549, while the alert dialog content remains at z-[1310] in packages/ui/src/components/alert-dialog.tsx:119.
  • Given that change, the previous modal-layering concern looks resolved enough in code: the drawer is no longer sitting in an unspecified higher layer than the alert dialog system, and the dialog content should stay above it consistently.
  • The earlier save-flow fixes still hold:
    • failed save does not advance file selection
    • refresh confirms before discarding dirty edits
    • empty-string saves are allowed
    • destructive dialogs use dismissible: false
    • save is routed through serverApi.writeWorkspaceFile(...)
  • The save button UX tweak also matches the product feedback now: it is always visible, disabled until dirty, and aligned next to refresh in packages/ui/src/components/instance/shell/right-panel/tabs/FilesTab.tsx:188-209.

Gatekeeper result:

  • No new findings from this pass.
  • git diff --check origin/dev...HEAD is clean.
  • I still could not run full typecheck/build/runtime verification in this worktree because dependencies are not installed (node_modules is absent), so this remains a code-review-based approval rather than an executed validation.

Non-blocking note:

  • packages/ui/src/components/instance/shell/right-panel/RightPanel.tsx is still roughly 994 lines, so it remains over the file-length warning threshold. I would not block merge on that.

From a PR gatekeeper perspective, I do not currently see anything left here that should stop merge.

--
Yours,
CodeNomadBot

@shantur shantur merged commit 37b3f85 into NeuralNomadsAI:dev Mar 29, 2026
2 checks passed
@shantur
Copy link
Copy Markdown
Collaborator

shantur commented Mar 29, 2026

Thanks @jchadwick

shantur added a commit that referenced this pull request Mar 31, 2026
## Thanks for contributions
- PR #252 “feat: Enable file editing and saving” by @jchadwick
- PR #256 “feat(ui): add session yolo mode controls” by @pascalandr
- PR #257 “fix(tauri): sync native app version with package releases” by
@pascalandr
- PR #258 “fix(tauri): stop stale UI assets from shadowing desktop
builds” by @pascalandr
- PR #260 “fix(ui): escape raw HTML in user prompt messages” by
@app/codenomadbot

## Highlights
- **Edit and save files directly in CodeNomad**: Update workspace files
in the built-in editor, save them without leaving the app, and get safer
handling for unsaved changes or edit conflicts.
- **More control over session automation**: Turn on per-session YOLO
mode from the Status tab, keep it visible with a clear badge, and let
long-running sessions continue auto-accepting prompts as expected.
- **Better voice conversation options**: Use spoken summary mode for
replies and keep conversation speech settings isolated per client, so
one device’s voice preferences do not unexpectedly affect another.
- **Faster session recovery**: Reload a session transcript from the
sidebar and see when a session is retrying, including live status
feedback.

## What’s Improved
- **Smoother desktop setup**: Desktop builds now bundle the right CLI
resources and handle microphone access more cleanly.
- **More reliable cross-platform desktop behavior**: Windows process
handling and npm invocation are safer, reducing environment-specific
issues.
- **Clearer session status visibility**: Retrying sessions now show more
useful state in the sidebar and header, so it is easier to tell what is
happening.
- **Cleaner in-app feedback**: Long toast messages wrap properly, GitHub
star counts display more cleanly, and message/code rendering behaves
more predictably.

## Fixes
- **Safer prompt rendering**: Raw HTML in user prompts is escaped so
messages display safely instead of being interpreted.
- **More reliable code previews**: Incomplete syntax highlighting
results are no longer cached, which helps prevent broken-looking file
views.
- **Better voice handoff**: Conversation playback stops when voice input
starts, avoiding overlapping speech.
- **More dependable desktop releases**: Native app versions now stay
aligned with package releases, and stale UI assets no longer shadow new
desktop builds.

### Contributors
- @jchadwick
- @pascalandr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Basic file editing

2 participants