Roam editor state per repo and branch

[joyceerhl]

Today with Cloud Changes we support roaming uncommitted changes for the same repo and branch across instances of VS Code. This smooths the transition from github.dev to a GitHub codespace and other Continue On transitions from the remote indicator, and also enables working on two clones of the same repo on two different machines.

To further streamline these transitions, we'd like to roam additional editor state like open editors and notebook controller affinity. I've started a prototype of roaming open editors in #179507. Briefly, this works by allowing workbench parts and contributions to register handlers for storing and resuming state in an edit session payload, with access to a uriHandler that knows how to convert URIs backed by the same version control state across filesystems. Further details are in the PR. Once this model passes initial adoption from open editors and notebooks, it can be adopted by additional workbench contributions and parts in future.

Related issues:

• Preserve remote kernel connection when using Continue Working On #158387
• Git: Preserve open files list when switching branches #35307
• Allow to save a list of opened editors in a group and switch between them #9498
• open same files at home like in office #176227

[joyceerhl]

State that I would love to see transferred (nonexhaustive list):

• Editor
  • Open editors
    • Text editors
    • Diff editors
  • Scroll position and selection within each editor
• Notebooks
  • Notebook controller affinity
• Source control
  • Commit message history
  • Working changes
  • View as list/tree preference
• Debug
  • Breakpoints in editors
  • Debug console REPL history
• Comments
  • Pending comments
• Search
  • Text search history
  • Find history within an editor
• Terminal history?

[joyceerhl]

From discussion with @bpasero today:

• Instead of introducing a separate registry for contributed payloads, we should try to make the contribution model align with existing usages of the storage service, which is already widely adopted in workbench for a lot of the state that we are interested in
  • Adopting storage in edit sessions should ideally be as simple as:
    • changing the storage target and scope
    • handling onDidChangeState on the storage service
    • handling onWillSaveState on the storage service
  • We have to figure out a way to introduce knowledge of the uriResolver for consumers of the storage service
    • Discussed automatically rewriting URIs which have a parent workspace folder URI corresponding to the current edit session payload, dangerous to do this on opaque serialized state
    • A property on the state change event?
    • A global property on the storage service? (Needs to be invalidated when the edit session identity changes)
• Instead of starting with adoption for open editors, ideally we should pick a simpler end to end example, e.g. breakpoints
  • The editorPart should first support dynamically restoring from arbitrary serialized state; this would also help with not reloading the window when a workspace folder changes

[joyceerhl]

I spent time looking at existing usages of the storage service, and there are additional suggestions and learnings:

• How to avoid passing around the uriResolver: Ideally URI conversion should be performed automatically, i.e. consumers of the new proposed scope should not have to run a URI resolver on absolute URIs (and we should not have to pass the URI resolver around in the storage service and its descendant types). However today the storage service does not deal with objects, only string | boolean | number | undefined | null, and we have no way to know where the URIs are if an opaque string is stored. To safely perform an automatic conversion, I believe consumers of the new proposed scope must explicitly earmark where the URIs are, e.g. by collecting all absolute URIs that the edit sessions service can safely convert, then reading them out of the object returned from IStorageService#get:

  interface EditSessionContributedPayload { // the shape of every stored edit session value
      absoluteUris?: URI[];
      state: string | boolean | number | undefined | null;
  }

  or by passing them in the args to store (which then allows for them to get combined):

  interface IStorageService{
      store(key: string, value: string | boolean | number | undefined | null, scope: StorageScope.ROAMABLE_WORKSPACE, target: StorageTarget, absoluteUris?: URI[]): void;
  }

• The majority of consumers of StorageScope.WORKSPACE and StorageTarget.User fortunately use a static key, but a minority of consumers will need to modify the keys, values, or both. E.g. SCM uses some part of the absolute URI as a storage key itself, which naturally will not work for edit sessions since the URI is already not portable:

  vscode/src/vs/workbench/contrib/scm/common/scmService.ts

  Line 158 in 9c4d6d2

  const key = this.repository.provider.rootUri ? `scm/input:${this.repository.provider.label}:${this.repository.provider.rootUri?.path}` : undefined;

  I think this means there's a new constraint that if we offer a storage service-like facade for storing edit sessions payloads, the keys must be free of references to absolute resources, and the values must declare any absolute resources explicitly in order for the correct conversion to happen automatically.

[bpasero]

@joyceerhl I have two more considerations to share:

Participation
I wonder if we should have a participation model for edit sessions that components can join or veto. It may be possible that a component cannot just accept the new session in the current window without doing some long running work or even asking the user. This could be a simple event with support for veto as we have in other cases such as lifecycle.

Session state vs. Workspace state
Do we need to distinguish between "session" state that only applies to "continue on" scenarios and general "roamable" workspace state? For example, if a component declares workspace state as "roamable" the question is:

• do we apply the state each time a workspace is opened that has the same identifier?
• or do we only apply the state once when the user made an explicit gesture to "continue on"?

[bpasero]

How to avoid passing around the uriResolver

You have to keep in mind that you cannot assume that you can always participate in the store method that maybe provides the metadata for URI you need. Instead, you will be operating on state that is stored in the DB already and so if there is any metadata associated with it, it would have to be stored in the DB as well, otherwise you cannot figure it out. We learned that for settings sync, that is why there is a special entry in the DB that stores the StorageScope for each key. This is what actually drives the IStorageService.keys method.

In other words: yes, we could enhance store to tell which data is URI, but we would have to put this into the DB for you to reliably retrieve this information. That is probably somewhat complicate to achieve, and maybe not worth the effort.

SCM uses some part of the absolute URI as a storage key

Good catch, that is not possible anymore for when data roams and so maybe Git should revisit that approach. Ideally maybe they could use a workspace identifier that applies in all machines and environments.

[joyceerhl]

I wonder if we should have a participation model for edit sessions that components can join or veto.

This makes sense, for working changes we already have scenarios where we need to ask the user for confirmation, e.g. if a file was changed locally and there are also incoming changes for that file, we cannot simply overwrite the local copy.

do we apply the state each time a workspace is opened that has the same identifier?

Yes, this is actually what we already do by polling the server, because sometimes we do not have a reliable Continue On trigger (e.g. the Continue On flow from web to desktop git clone involves two local windows, and we do not want to apply the state in the first intermediate window). I'll also note my goal has always been to get to a point where state syncs continuously, but since I'm not there yet owing to perf concerns, Continue On is a convenient trigger to perform a store (similar to the onWillSaveState event in the storage service). If we leveraged roamable workspace state to support scenarios like #35307, then there would be additional trigger points to emit onWillSaveState, e.g. when git changes the branch.

[joyceerhl]

we would have to put this into the DB for you to reliably retrieve this information

Yeah, I tried this and agree it's not so nice. Maybe then we expose the uri resolver elsewhere, e.g. an event property:

export interface IWorkspaceStorageValueChangeEvent {
  readonly scope: StorageScope.WORKSPACE;
  readonly key: string;
  readonly target: StorageTarget.USER | undefined;
  /**
   * A utility for translating absolute URIs from another
   * workspace into URIs for the current workspace if the
   * workspaces share a common workspace identity.
   * @param uri An absolute resource URI referenced in the
   * storage value that changed.
   * @returns A transformed resource URI that applies to
   * the current workspace, or the original URI if no
   * transformation was possible.
   */
  readonly uriResolver?: (uri: URI) => URI;
}

I think this would also involve modifying IStorage to emit an Event<{ key: string; uriResolver?: (uri: URI) => URI; }>:

vscode/src/vs/base/parts/storage/common/storage.ts

Line 56 in 63bb4c4

readonly onDidChangeStorage: Event<string>;

[joyceerhl]

Moving this out to May, pending further discussion with @bpasero and @sandy081.

[joyceerhl]

Notes from today's discussion:

• introduce new resource type workspaceState that is managed from the settings sync client
• use StorageScope.Workspace and StorageTarget.User, but have an allowlist in the meantime for determining which keys get synced
• new WorkspaceIdentityService in platform for determining whether there is a match for the workspace
  • consumed by settings sync client and edit sessions workbench contribution
  • does edit session identifier matching under the hood
  • if there is a match, then we seed the IStorageService for StorageScope.Workspace and StorageTarget.User
• settings sync happens on the shared process, we probably do not want that for workspace state
• how do we rewrite/convert URIs? options:
  1. automatically rewrite them in serialized settings sync state
     • potentially dangerous to do this since we are operating on opaque strings
  2. each editor/workbench part tries to store state in a format that works separately
     • if we only support for single folder workspaces, not multiroot workspaces, then each editor part can store workspace folder URI and relative path
  3. the storage service exposes a URI resolver that each editor/workbench part must call on any absolute URIs that it has stored in it state in order to get the converted URI out
  4. each editor/workbench part declares its URIs for conversion separately (similar to what we do in the storage service), and URI conversion is automatically run for each URI
• what about files outside the workspace?
• not all state can or should be synced unless we came from a Continue On transition, since it can be destructive/users may have intentionally isolated workspaces--maybe have an explicit affordance similar to the macOS one rather than automatically resuming

[joyceerhl]

Notes from today's discussion:

Initial E2E prototype

• migrate all current uses of workspace and user back to workspace and machine
• introduce a workspaceState resource which reads workspace and user state (reuses from globalState)
• storage service should support objects
• introduce a workspace identity service
• introduce a marshaller for URIs which we will run on objects that are stored with workspace+user
• adopt for breakpoints

Later

• UX: modal splash while restoring, handoff UI indicator

[joyceerhl]

We now have support for StorageScope.WORKSPACE and StorageTarget.USER.

Unfortunately while trying to adopt this combination for breakpoints, I discovered that breakpoints rely on a non-standard external property on the URI object which is (naturally) not properly serialized, and therefore cannot automatically be rewritten by us:

vscode/src/vs/workbench/contrib/debug/common/debugStorage.ts

Line 42 in b168fe5

return new Breakpoint(URI.parse(breakpoint.uri.external || breakpoint.source.uri.external), breakpoint.lineNumber, breakpoint.column, breakpoint.enabled, breakpoint.condition, breakpoint.hitCondition, breakpoint.logMessage, breakpoint.adapterData, this.textFileService, this.uriIdentityService, this.logService);

This looks like a serialized URI, so hopefully debug can just change how it stores breakpoint data in the storage service so that the external property is recognized as a URI. In the meantime I will look for another initial adopter.

[joyceerhl]

Closing this issue as we now support the StorageScope.Workspace and StorageTarget.User combination in the storage service and have a working adoption of SCM transferring state in Continue On using this combination. I will create a separate issue for adoption of this combination elsewhere in the codebase. Thank you @bpasero, @sandy081 and @lszomoru!

[jeanp413]

@joyceerhl Some early feedback, I use the Cloud Changes: Store Working Changes in Cloud command, so I usually have multiple editSessions, right now workspaceState is stored independantly and when restoring a editSession it always fetch the latest workspaceState (which could be from a different workspace), I expect some kind of relation between the two 🤔

[joyceerhl]

@jeanp413 sorry for missing this, addressed with #186193