Repository navigation
feat(files): add a remote-safe workspace file tree and editor - #130
Conversation
|
@wingleeio I added two intentionally failing regression tests that reproduce the stale syntax-highlighting race. One shows that a highlight result for revision 0 is still accepted after the editor advances to revision 1, and the other demonstrates how the resulting stale byte ranges split the UTF-8 boundaries of updated Unicode text. No production fix is included yet, I'll work on this tomorrow, currently this is causing sporadic panic crashes. |
# Conflicts: # Cargo.toml
# Conflicts: # Cargo.toml # THIRD_PARTY_NOTICES.md # crates/ui/src/composer.rs # crates/ui/src/settings.rs # crates/ui/src/shell.rs
wingleeio
left a comment
There was a problem hiding this comment.
Requesting changes on 131e891f5380384d74582338571c8213453f2b16 for a reproduced write-target bug.
Blocking finding
[P1] Pending edits can be saved into a different checkout after the chat's cwd changes.
I reproduced this in the unmodified headed Linux app with an isolated repository and a registered linked worktree containing the same initial README.md:
- Open
README.mdfrom the original checkout and edit it with autosave disabled. Confirm both on-disk copies are still unchanged. - Change the same chat's cwd to the linked worktree through
Mutate { op: "setChatCwd", ... }. - The Files surface automatically flushes its pending buffer and clears the dirty indicator. Inspecting disk shows the original checkout is unchanged, while the new worktree received the edit made against the original checkout.
sync_target notices the changed context and calls save_document before applying it. However, the write request only identifies the chat and relative file path; the engine resolves that chat's already-updated cwd. Matching initial content hashes allow the write to succeed against the wrong file. The response-generation checks happen after the write and cannot protect it.
Please bind pending writes to the original resolved checkout identity, verify that identity on the owning engine, and preserve/block the dirty buffer when the target changes rather than flushing through a mutable chat target. Add a regression covering two registered checkouts with identical starting file contents.
Validation
cargo test -q --locked -p zeron-engine --lib workspace_files::tests— 25 passed.cargo test -q --locked -p zeron-ui --lib files::— 60 passed.cargo test -q --locked -p zeron-engine --test workspace_files --test device_routing— 8 passed.cargo build -q --locked -p zeron— passed.git diff --check origin/main...HEAD— passed.- Headed Linux smoke test: browsing, opening multiple file tabs, syntax rendering, explicit save, and preservation of a dirty buffer on external modification worked. The checkout-change scenario above failed.
- Existing UI and macOS frame-recovery CI checks are green. Native macOS interaction was not exercised locally.
Screenshots
These are captures from the actual PR build, uploaded as GitHub user attachments.
Before the checkout change: the dirty dot is visible on README.md, and the composer footer still says Local checkout.
After the checkout change: the footer says Worktree and the dirty dot is gone. Disk verification confirmed the pending edit landed in this new worktree while the original file remained unchanged.
Normal explicit save: the source document was saved successfully and its exact contents verified on disk.
External modification: the warning appears and the unsaved editor buffer is retained.
|
Pushed the Files UI polish in 83b8ef8.
Validation: all 60 Files UI tests passed, the application build passed, and the live Linux app was checked at wide and narrow widths, including divider dragging and toolbar focus. These changes address presentation; the previously reported checkout write-target blocker remains open. Joined borders and resized sidebarNarrow pane with wrapping warning actions |
wingleeio
left a comment
There was a problem hiding this comment.
Pushed 15f3a55d: Files, Diffs, and History now share the 38px header height, 24px controls, 6px corners, and 4px action spacing. Git file/summary subheaders follow the same height. History search text is centered, and the expanded field shrinks at the minimum sidebar width so refresh and view controls stay accessible. Files warning text wraps within its pane.
The earlier P1 checkout-switch finding is fixed. Reads return their checkout identity; writes must carry that identity and the engine rejects a changed target before touching disk. Switching workspaces preserves dirty buffers and pauses saves. Returning to the original workspace permits saving. The write RPC now requires expectedCheckoutId from the read response, so requests without it fail closed.
Validation: native Linux build; all 726 UI tests, 25 workspace-file engine tests, and 9 workspace-file/device-routing integration tests passed. The regression uses two registered worktrees with identical file contents and verifies neither is modified by a stale save. Native checks covered search, save, workspace switching, warning wrapping, diff folding/split view, History search at wide/minimum widths, and terminal input. GitHub CI is running; macOS was not tested locally.
Screenshots below are native captures uploaded as GitHub user attachments.
Files
Diffs and Git subheaders
History search
Terminal
Files: preserved edits and wrapped warning in a narrow pane
History: all toolbar actions remain visible at minimum width
|
Updated the default shortcuts: Cmd+S saves the active workspace file, Cmd+B toggles the left sidebar, and Cmd+R toggles the right sidebar (Ctrl on Linux/Windows). All three are configurable in Settings → Shortcuts. Existing default panel bindings migrate; custom bindings remain intact. Closing the focused file pane now restores keyboard focus so the right-sidebar shortcut can reopen it immediately. Centered the History author placeholder in the avatar’s full inner width with an explicit font and line height. Validation: native Linux build; all 727 UI tests passed, including shortcut migration, reset, platform spelling, and conflict checks. Native checks verified saving with both the default and a custom recorded binding, persistence, both sidebar toggles, and repeated close/reopen from a focused editor. An unrelated transcript-selection test failed on the first full run and passed on rerun. macOS was not tested locally. Centered author initial Configurable shortcuts (Linux uses Ctrl) File saved using the keyboard shortcut |
|
Removed the save icon. Files now use a neutral dirty dot on the tab; it disappears after a successful save. Pending edits remain marked with autosave enabled. The toolbar shows messages only for save failures, conflicts, or external disk changes; failed saves remain retryable. Cmd+S / the configured Save file shortcut handles saving. Validation: native build, all 726 UI tests passed, and native edit → dirty dot → shortcut save → clean tab checked against the file on disk. Unsaved file After saving |
wingleeio
left a comment
There was a problem hiding this comment.
Requesting changes after auditing 8bd809302140ad200051098c26880d966d10ca3a. I cannot recommend merging or releasing this revision because the normal Quit action can discard unsaved file edits.
Three findings are attached inline: a data-loss path through application lifecycle actions, a strong reference cycle retaining closed editors, and shortcut collisions introduced when migrating a valid customized keymap.
Validation:
- Ran
cargo test --locked --no-fail-fast -p zeron-engine -p zeron-proto -p zeron-rpc -p zeron-syntax -p zeron-ui: 1,083 passed, 1 failed, 5 ignored. The failure,rpc_surface_over_in_memory_transport, reproduces unchanged on current main (e7ae37ea): unexpectedcontextUsage: null. It is not attributed to this PR. - Both required CI jobs are green on the audited head.
- Reproduced Quit data loss in the native Linux UI with autosave disabled. A temporary Linux binding invoked the existing
app_menus::Quitaction (normally bound to Cmd+Q on macOS); the process exited and the edited marker was absent from disk. Cmd+W's bypass was established by source tracing, not a macOS runtime test. - A focused audit-only test through
UiSettings::loadreproduced[SaveFile, NewSession]conflicts from a previously valid custom keymap. - Reviewed checkout identity enforcement, file path handling, atomic writes/conflicts, RPC routing, editor lifecycle, shortcuts, and release workflow. The previously reported wrong-checkout save is addressed, but that does not cover the new lifecycle finding.
Temporary audit instrumentation was removed; no source changes are included in this review. No merge, tag, or release was performed.
Native UI immediately before the reproduced Quit (dirty tab dot visible; screenshot text names the original Close scenario):
wingleeio
left a comment
There was a problem hiding this comment.
Fixed all three audit findings in 4259e08c.
- Quit and Close now defer out of the current window dispatch, then wait for file saves before destroying windows. Runtime changes and update installation use the same guard. Failed/conflicting saves keep the app open and reveal the affected file; Keep Open cancels the pending exit. A macOS application delegate guard also routes Dock/system Quit through this decision, before AppKit commits to termination.
- The editor context-menu callback captures a weak editor reference. A GPUI regression verifies the actual editor entity is released after its final owner drops.
- Legacy shortcut migration reserves customized bindings first. New bindings use an available fallback, or remain unbound if all candidates are occupied; they never silently take an existing custom chord. Tests cover conflicts and persisted round trips.
Validation: 729 UI tests passed with --test-threads=1, including actual Quit/Close action dispatch with pending and failed saves, runtime/update guards, cancellation, editor lifetime, and shortcut migration. The parallel run hit the previously observed unrelated transcript-selection flake; the full serial suite passed. The production binary builds successfully. Native Linux checks used temporary bindings to the existing Quit action; those bindings were removed and the production binary rebuilt. New Linux/macOS CI jobs are running.
With autosave disabled, native Quit wrote the exact edited content to disk and then exited:
A conflicting disk edit blocked Quit and preserved both the unsaved buffer and the external file:
Keep Open cancelled the exit and retained the editable buffer:
wingleeio
left a comment
There was a problem hiding this comment.
The reported audit findings are addressed in 4259e08 and the Apple Silicon BOOL correction in 24bcc41. Approved after rechecking the final head: Linux UI CI and macOS application/native frame-recovery CI are green; 729 local UI tests pass, and 736 UI tests pass on the clean merge with current main. Native Linux checks confirm Quit saves the exact edited content before exit, blocks on a disk conflict without overwriting external changes, and preserves the buffer when exit is cancelled. Screenshots are attached in the preceding review. No remaining blocking findings from this audit.
|
Merged and released in v0.2.47. The release workflow and final main CI are green. All four public updater downloads match the published SHA-256 manifest; |
…wing (ui-parity zeronsh#130) TranscriptState.apply now captures a ScrollAnchor (the row under the header's bottom edge, probed at offset + topInset clamped at 0) before the frame swap while not following, and restores it after 68/69's onFrameApplied runway height when needsAnchor holds: the target is the anchored row's new y plus the old delta, single row, no fallback, 0.5 dp dead band, clamped to [minOffset, maxOffset] and written through viewport.setOffset so an in-flight fling keeps its momentum. Pure math lives in ScrollAnchor.kt over the AnchorRows seam (LayoutFrame in production, fakes in tests). Tests: ScrollAnchorTest (plain JUnit, the ticket's worked-example table, probes, clamp, dead band, runway precedence, resting finger), ScrollAnchorFlingTest (Robolectric, fling continuity across a shift), ScrollAnchorLabTest (instrumented, real core, streaming/prepend/rewrap). Not run locally (VPS/CI).






















Summary
This PR adds a complete workspace file browsing and editing workflow to Zeron's right pane.
Users can open a new
Filessurface, navigate or search the checkout associated with the current chat, preview and edit supported text files, keep multiple files open as surface tabs, and attach workspace files or folders to the composer by dragging them from the tree, search results, or file tabs.The implementation works through the same RPC boundary for local and remote workspaces. The UI never reads or writes workspace files directly, and every operation is resolved and authorized by the engine on the device that owns the checkout.
Editing is powered by a pinned Apache-2.0
gpui-basefork that is compatible with Comet's GPUI revision. Zeron continues to own the surrounding product behavior: file tabs, layout, theme integration, syntax highlighting, autosave, optimistic concurrency, external-change reconciliation, inline review comments, settings, and remote routing.The work was developed in three stacked phases and is included end to end in this branch:
Motivation
Zeron already had read-only diff rendering and file mentions, but it did not have a general-purpose way to browse or edit arbitrary files in the active workspace.
Reusing the diff viewer as an editor would have required rebuilding cursor movement, selection, IME, clipboard, undo/redo, and text-buffer behavior inside a renderer designed for immutable hunks. Pulling in Zed's complete editor would also have introduced GPL code and a large dependency graph coupled to Zed's project, language, LSP, and multibuffer architecture.
This PR instead introduces a small product-owned filesystem layer and uses the focused editing primitives from
gpui-base. That gives Zeron native GPUI performance and editor fundamentals while keeping workspace semantics, transport, safety, and UI under Zeron's control.User-facing experience
Files surface
Filesto the right-pane surface picker and the+surface menu.Filestab for the first opened document and changes its title to the file name.Responsive layout
File tree
read-onlylabels from every tree row; unsupported files instead explain their state when opened.Search
Search filesinput with a 200 ms debounce.Hidden and ignored files
.gitunavailable even whenShow all filesis enabled.Settings → Files.File previews and tabs
Workspace file protocol
The protocol adds five explicit workspace file operations:
ListWorkspaceDirectorySearchWorkspaceFilesReadWorkspaceFileWriteWorkspaceFileWatchWorkspaceFilesThe shared contracts model:
chatIdorspaceId./separators.All five methods are relay-forwardable, while
WatchWorkspaceFilesis registered as a streaming RPC.Engine backend
Shared workspace resolution
WorkspaceFilesservice owned by the engine lifecycle.Path security
.components, and..traversal..gitcomponents regardless of listing preferences.symlink_metadataand canonical containment checks to prevent traversal through file or parent symlinks./-separated wire representation.Directory listing and search
.gitfiltered even when ignored entries are requested.Safe reads
Conflict-aware writes
Shared filesystem watcher
.gitand the backend's own temporary write files.Files UI architecture
The new UI is split into focused modules:
files/client.rsfiles/model.rsfiles/tree.rsfiles/search.rsfiles/watch.rsfiles/document.rsfiles/editor.rsgpui-base::EditorState.files/editor_adapter.rszeron-syntaxadaptation forgpui-base.files/preview.rsfiles/mod.rsEvery asynchronous request captures the effective workspace context plus a generation or document identity. Late reads, searches, highlights, writes, and watcher events are rejected when the active chat, checkout, path, or revision no longer matches.
Editor integration
gpui-baseboundarygpui-basefrom the contributor-owned forkjsgrrchg/gpui-componentat commit8194877275c5d7d3a8eacf3aed7c2cf8c9be8098.longbridge/gpui-componentdirectly: the fork is required to makegpui-baseuse the exact GPUI source and revision already used by Comet.gpui-basein third-party notices.Why a fork is required
Rust treats types from different Git sources or revisions as different crate identities even when their source code and names look equivalent.
Upstream
gpui-componentwas resolving GPUI fromzed-industries/zed, while Comet resolves GPUI from the Zed fork used by this repository. Depending on both unchanged would produce incompatiblegpui::Window,gpui::App,gpui::Entity, element, event, and context types across the application/editor boundary, along with duplicate GPUI packages in Cargo's dependency graph.The fork intentionally solves that dependency-identity problem at the source. It is not a parallel Zeron product implementation: the fork contains generic GPUI compatibility and caret rendering fixes, while all Zeron-specific tabs, themes, syntax, autosave, RPC, comments, settings, and filesystem behavior remain in Comet.
Exact dependency chain
Comet pins the fork directly:
The fork then pins every relevant GPUI dependency to the same Zed fork and immutable revision used by Comet:
The same
wingleeio/zedrevision is used forgpui_platform,gpui_web,gpui_macros, andreqwest_clientinside the fork.Cargo.lockrecords the completegpui-basesource asjsgrrchg/gpui-component @ 81948772, preventing Cargo from silently following future branch changes.The fork history consumed by Comet is:
Fork modification 1 — align
gpui-basewith Comet's GPUICommit
1fc58bb1c0b62c1578afd282e771e2d17ded43a3,chore(gpui): align gpui-base with Comet GPUI revision, makes the compatibility layer compile and behave against Comet's GPUI source.This commit:
gpui,gpui_platform,gpui_web,gpui_macros, andreqwest_clientfromzed-industries/zedtowingleeio/zed @ 5d1f83d9f27a19bec1fb241dc33b42238af9cf8d.collections,gpui_util,http_client,media,perf,refineable,scheduler,sum_tree, and related platform crates resolve from that same immutable source.restrict_scroll_to_axisthrough element style state.OngoingScrollExtcompatibility surface.Window::draw(...).clear()signature available in Comet's GPUI revision.Editor, edits ASCII/Unicode/emoji and multiline text, exercises selection and cursor movement, verifies undo/redo, applies scroll state, and confirms visible-row layout.The compatibility commit changes 32 files with 205 insertions and 146 deletions. Most touched component files contain mechanical test-harness API updates; the meaningful runtime compatibility change is concentrated in the GPUI dependency pins and scroll-axis adapter.
Fork modification 2 — stable caret pixel snapping
Commit
8194877275c5d7d3a8eacf3aed7c2cf8c9be8098,fix(editor): snap caret dimensions to physical pixels, fixes a rendering defect discovered while testing Zeron's file editor.On macOS,
gpui-baseused a logical caret width of1.5 px. At a 1× display scale, GPUI could independently round the left and right caret edges after the caret moved to fractional X positions. The resulting physical width alternated between one and two device pixels, which made the caret appear to grow and shrink while typing.This commit:
cursor_sizecalculation that snaps caret width and height throughWindow::pixel_snapbefore bounds are constructed.LastLayoutso paint, scrolling, alignment, and clipping use one consistent geometry value.CURSOR_WIDTHassumptions in centered/right-aligned safety margins with the snapped width.Input,Textarea, and the codeEditor, rather than adding a Zeron-only rendering exception.Input,Textarea, andEditormodes.The caret commit changes 5 files with 162 insertions and 9 deletions. It is a generic editor rendering correction and does not contain Comet/Zeron product logic.
Combined fork delta
Relative to the fork's
mainbase atda4f93696dc2b2b4d91bcc42412b9053a3d24de8, the exact revision consumed by Comet changes 36 files with 367 insertions and 155 deletions.The combined delta consists only of:
There are no Zeron RPCs, themes, settings, workspace models, file operations, or application-specific UI components in the fork.
Fork maintenance and reproducibility
zeron-gpui-compact,fix/caret-pixel-snapping, or any other floating branch.gpui-componentupdate must first rebase or port these two commits onto the desired upstream base.Dependency documentation follow-up before merge
Cargo.tomlandCargo.lockalready point to8194877275c5d7d3a8eacf3aed7c2cf8c9be8098, butTHIRD_PARTY_NOTICES.mdstill labelsgpui-basewith the earlier abbreviated compatibility SHA1fc58bb.Before opening or merging this PR, update that notice to identify the actual consumed revision
81948772so the dependency manifest, lockfile, and legal notice agree.Editing behavior
EditorStateper open editable document.Document model
Each open file is keyed by chat, checkout identity, and workspace-relative path rather than by path alone.
The document tracks:
Dirty state is derived from revision numbers instead of comparing full text buffers every frame.
Programmatic loads and clean external replacements establish a new baseline without being treated as user edits.
Autosave and lifecycle safety
Cmd-SorCtrl-Sas the primary save mechanism.Retry,Keep Open, andDiscard Changesrecovery actions instead of hammering the RPC in a tight loop.Saving/Unsavedlabels and tab indicators that flashed during normal fast autosave operation.Reload and external-change handling
Reload filefrom silently discarding a dirty buffer.Discard unsaved changes?confirmation withCancelandDiscard & Reloadactions.Keep EditingorReload from Disk.Keep Editinginto an explicit conflict state instead of overwriting disk content later.DeletedOnDiskdocument when the same file or a descendant of a recreated directory appears again.Refresh nowaction remains available when the watcher is degraded, whileReload from Diskremains available for external-change conflicts.Syntax highlighting
zeron-syntaxas the single language registry and highlight source.gpui-base.Inline review comments in editable files
+affordance used to begin a comment.path:linereferences without synthetic left/right diff tags.Drag files into the composer
zeron-file:Markdown transport and the existing projected@file/@folderchip appearance.Drop images to attachtoDrop to attach.Settings
Adds a dedicated
Settings → Filespage with:Autosave delay900 msWord wrapShow all filesThe settings use the existing
UiSettingsstore, backward-compatible serde defaults, atomic persistence, and Shell's existing debounced settings save. No second configuration file or per-editor settings store is introduced.Visual and interaction polish
Performance characteristics
Supported file matrix
TooLargepreview.Test coverage
The implementation adds unit and integration coverage for:
.gitfiltering.Commands verified on this branch
cargo checkstill emits the existingunexpected cfg cargo-clippywarnings from Objective-C macros; the affected notification and appearance code predates this branch.Cargo also reports existing future-incompatibility warnings for
block v0.1.6andproc-macro-error2 v2.0.1; both exact versions are already present onupstream/mainand are not introduced by this PR.Suggested manual QA
Local workspace
Filesfrom the right-pane picker.Refresh nowrecovery action reconciles the tree and open documents.Show all filesand verify hidden/ignored descendants dim consistently..gitis never exposed.Filestab is reused and renamed.Editor
Autosave and conflicts
Keep Editing/Reload from Diskrecovery.Reload from Diskfor a dirty externally modified file and verify explicit discard confirmation.Layout and settings
Review comments
+replaces the number.Drag-and-drop
@filechip appears at the selection.@folderchip appears without a recursive scan.Remote workspace
Scope deliberately excluded
Cmd-S/Ctrl-Sas the primary save workflow.Risks and mitigations
Filesystem safety
The feature expands Zeron from read-only arbitrary-file access to write access. The backend mitigates this with target authorization, strict relative-path validation, symlink rejection, checkout containment checks, expected-content hashes, typed conflicts, per-file serialization, and atomic replacement.
Remote consistency
Relay delays can reorder user intent and filesystem notifications. Requests carry workspace/document generations and revisions, writes are conditional on hashes, watch frames are sequenced, and sequence gaps trigger full reconciliation.
Autosave data loss
Autosave is intentionally conservative: conflicts never overwrite disk content, dirty reloads require confirmation, close/workspace transitions wait for safe resolution, failed writes stop automatic retries, and deleted files preserve their in-memory buffers.
Large repositories
The tree and search are virtualized, directories load lazily, search/list results are bounded, filesystem work runs off the event loop, and watchers enter repair mode when the native directory budget is exceeded.
External editor dependency
Only the Apache-2.0
gpui-baseediting primitives are consumed from a pinned immutable fork commit. Product behavior remains in Comet, the fork is aligned with Comet's GPUI source, and the dependency does not bring in Zed's full editor architecture.Reviewer guide
For a structured review, the implementation can be read in this order:
crates/proto/src/entities.rsandcrates/rpc/src/lib.rsfor the wire contract.crates/engine/src/workspace_files.rsfor target resolution, path safety, reads, writes, and watcher behavior.crates/engine/src/rpc.rs,crates/engine/tests/workspace_files.rs, andcrates/engine/tests/device_routing.rsfor wiring and end-to-end coverage.crates/ui/src/files/client.rsandcrates/ui/src/files/model.rsfor the typed UI boundary and pure tree model.crates/ui/src/files/tree.rs,search.rs,watch.rs, andmod.rsfor browsing and lifecycle behavior.crates/ui/src/files/document.rs,editor.rs,editor_adapter.rs, andpreview.rsfor editor state, autosave, conflicts, syntax, comments, and responsive layout.crates/ui/src/settings/files.rs,crates/ui/src/settings.rs, andcrates/ui/src/shell.rsfor product integration and persistence.crates/syntax/src/lib.rsfor Markdown highlighting extensions.crates/ui/src/comments.rs,composer.rs,changes.rs, andstate.rsfor shared review comments and workspace-reference drag-and-drop.Change size
upstream/main.Screenshots / recordings