Skip to content

fix: CRDT sync, file/folder sync, remove REST fetch calls - #3

Merged
utkarsh684 merged 2 commits into
mainfrom
fresh-main
Apr 6, 2026
Merged

utkarsh684 merged 2 commits into
mainfrom
fresh-main

Conversation

@utkarsh684

@utkarsh684 utkarsh684 commented Apr 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Active user list now sourced locally from collaboration state
    • File sync now tracks folders, auto-uploads saved files, watches created/deleted files, and performs periodic reconciliation
    • Terminal now falls back to opening a local terminal when relay is unavailable
  • Bug Fixes

    • Improved initialization timing and added safety for applying remote document changes
  • Refactor

    • Collaboration room creation and lifecycle streamlined for faster join/leave flows

@coderabbitai

coderabbitai Bot commented Apr 6, 2026 •

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cf8d655d-73ee-499f-8735-73d8e215f249

📥 Commits

Reviewing files that changed from the base of the PR and between 0e58c69 and 47a5e32.

📒 Files selected for processing (2)
  • extensions/collab-edit/src/collabSession.ts
  • extensions/collab-edit/src/sharedTerminal.ts

📝 Walkthrough

Walkthrough

Added AwarenessManager.getStates(); refactored collab binding initialization and offset mapping; replaced server REST room ops with local room IDs and awareness-based active user lists; added file system reconciliation and single-file sync; changed sharedTerminal socket typing and added a warning call (with an undeclared identifier); exported DefaultAccountProvider.

Changes

Cohort / File(s) Summary
Awareness State Management
extensions/collab-edit/src/awarenessManager.ts
Added public getStates(): Map<number, AwarenessState> returning the underlying Yjs awareness states map.
Collaborative Binding Core
extensions/collab-edit/src/collabBinding.ts
Delayed initial CRDT/document sync via _initializeContent() (1.5s), moved listener setup, added _suppressRemoteApply for seeding, replaced offset conversion methods with _positionToCrdtOffset / _crdtOffsetToDocPosition, updated Local↔CRDT edit mapping, and added a remote-apply failure fallback that applies full content.
Collaboration Session Management
extensions/collab-edit/src/collabSession.ts
Removed REST calls for create/join/leave; createRoom() generates a local roomId and proceeds to _joinInternal; showActiveUsers() now reads from this._awarenessManager.getStates() (falls back to 'Anonymous' or a readiness warning).
File Synchronization & Reconciliation
extensions/collab-edit/src/fileSyncManager.ts
Added _syncSingleFileToRemote() and _syncRemoteFilesToLocal(); track directories as "DIR" in fsMap; added on-save and FileSystemWatcher handlers; introduced delayed/periodic reconciliation (setTimeout + setInterval) and cleared _syncInterval on dispose.
Terminal Connection
extensions/collab-edit/src/sharedTerminal.ts
Changed _ws type to any; added a warning message fallback that references an undeclared reason identifier inside _connect() (potential runtime error).
Account Services
src/vs/workbench/services/accounts/browser/defaultAccount.ts
Made DefaultAccountProvider exported (public); removed an unused import IInstantiationService.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I nibble lines and stitch the thread,

awareness maps and rooms now spread.
Files hum home and sockets chase,
a little delay keeps sync's soft pace.
Hooray — the collab burrow's in its place.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description only contains the repository template boilerplate with no actual content describing the changes, testing approach, or issue association. Provide a detailed description of the proposed changes, testing instructions, and associate the PR with a relevant issue.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: CRDT sync fixes, file/folder sync improvements, and removal of REST API calls.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fresh-main

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@utkarsh684
utkarsh684 merged commit 6371ecf into main Apr 6, 2026
3 of 16 checks passed
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.

1 participant