[28.2/33] Editing together, seen: who has a document open, and writing what the editor holds - #419
Merged
vikramsoni2 merged 4 commits intoSep 26, 2026
Conversation
The engine that keeps what a save replaces, and the API that reads a history
back, arrived with the batch before this one. Nothing showed them: no mark in a
listing, no panel, no way for an administrator to see what histories cost, and
no way to read an earlier version at all.
This is that half.
A file that has earlier versions carries a small mark in the listing, with how
many; clicking it opens the panel, which lists what was kept, who saved it and
where it came from, and offers reading a version, downloading it, putting it
back, naming it, pinning it and deleting it. Putting one back keeps what it
replaced, so nothing is lost by going back. The mark is counted once per folder
rather than once per row, and a preference turns it off — which takes the query
away as well as the icon.
An administrator gets the list of every file that has a history, wherever it
is, with what each one takes up, and can empty one. Those routes are addressed
by the history's own id rather than by a path, because the ones worth finding
include files that no longer exist: a history whose file was deleted outside
the application has no path left to ask about, and no file to authorise
against. They sit behind the administrator check for the same reason.
Reading text is now one reader, used by the editor, by a version and by a file
still in the trash — the two the batch before this one left out. That reader
fixes what the editor used to do with it:
- A UTF-16 file was answered "this file appears to be binary and cannot be
opened". In UTF-16 every letter of English is accompanied by a zero byte,
and a zero byte was exactly the test for binary. That is what `Out-File`
wrote by default until PowerShell 6 and what Notepad still offers as
"Unicode", so an export or a log from a Windows machine could not be
opened. A mark is believed when there is one; otherwise the pairing of
zeros decides, which is what tools that write UTF-16 without a mark leave
behind.
- A save wrote UTF-8 over whatever the file was. That reads perfectly well
here and breaks whatever wrote it, so a save now writes back in the
encoding the file already had.
- The size limit was checked when opening and not when saving: a paste larger
than the limit was written, and the file could then never be opened again.
It is now checked on the bytes about to be written, which in UTF-16 are
twice the characters.
- The editor opens two megabytes and saves through a JSON body, whose limit
was Express's own default of 100 kB. A file between the two opened and
could never be saved, answered "request entity too large" — which names
neither limit. The two are one decision now: the body limit is derived from
what the editor may open, and a body limit somebody set is a ceiling that
is never raised from here, so it is the editor that gives way.
The editor's read is also a GET the browser may keep and revalidate, answered
304 while the file is unchanged, so opening the editor from the Markdown
preview no longer downloads the same file twice. The identity is taken from the
file's metadata, including the inode — a save writes a new file and renames it
over the old one, so a save that comes out the same size within one clock tick
still differs.
An earlier version and a file in the trash open in the editor as text to read:
no save, no shortcut that saves, and an editor that does not take typing.
Closing goes back where it came from — the folder with the history open again,
or the trash, inside the deleted folder the file was read from.
Found while wiring this up: `marksForFolder`, `listFilesWithVersions`,
`readFileVersions` and `deleteFileVersions` all called store helpers that were
never added. Nothing called them, so nothing noticed; all four threw. The
queries they need are here, and every one of them now has a test.
Left for later, and named here so it is not lost: taking a version out as a new
file somewhere, and putting one over another file, both need the destination
dialog, which is not here yet; reading a version of an office document needs
the office editors to accept one, which comes with the batch that finishes
them; and the entry an activity log would want for a purge waits for the log.
Gates: lint and formatting on the changed files, `npm run build`, every backend
module loads, and the whole backend suite — 1,949 pass, against the two that
fail on `main` before any of this. Every claim above has a test that fails when
the change behind it is put back; the detection of UTF-16 was mutated twice,
once for the mark and once for the pairing, because they are two mechanisms.
The screens were checked in the built image: two saves, the mark showing 2 in
the listing, the panel opening from it, a version read in the read-only viewer,
a version restored from the panel with the history growing to three, a file
deleted and read from the trash by double-clicking it, and the administrator's
list showing the file, its space and its history. That is also how the last
defect here turned up: the dialog that confirms a restore teleports to the body
and stacked below the panel's own overlay, so its buttons could not be clicked.
No unit test would have seen it.
added 3 commits
September 26, 2026 10:57
Which keys are preferences was decided twice: once in the settings service, which sanitises the value, and once in the settings route, which decides whether the key is written at all. They had drifted. The preference added here — the mark on files that have versions — reached the service's list and not the route's, so the toggle moved on screen, the save answered success, and nothing was stored. It would have read as "my setting does not stick", with two lists to find before anyone could say why. One list now, named and exported by the service; the route asks it. A test walks every key the service calls a preference and checks the route keeps it, so the two cannot drift again.
Two people editing the same document only see each other when the Document
Server was given the same key: a different key is a different document to it,
and it opens a second, independent session on the same file. Whoever saves last
then overwrites the other, with nothing to warn either of them.
The key was recomputed on every open from the file's own modification time. So
it changed under the people already editing — every save of theirs split the
session — and it did not change once they had all left, which is the one moment
it must, because the Document Server caches the prepared document under that key
and serves the stale copy on the next open.
The key is now kept: the same for as long as anyone has the document open,
including across their saves, and dropped when the Document Server reports it
has let go. Restoring a version drops it too, which is the piece the history
batch could not carry: an editor reopened after a restore was getting the copy
the restore had just replaced.
The other half is where the document is. The Document Server is handed a token
when the editor opens and returns it unchanged with every save, so the token
says where the document *was*. Renaming from the title bar makes that stale at
once. There are now editing sessions, stored rather than held in memory — a
restart mid-edit would otherwise forget the rename, and the next save would
recreate the old name beside the new one, silently. Renaming from inside the
editor moves the file through the ordinary rename, with the ordinary permission
checks, and then moves the session, the presence record and the key with it.
Presence starts when the client says the document is really open, not when the
configuration was asked for: asking for a configuration says nothing about
whether the document opened, and a file the editor then refused was shown to
everybody as being edited until it expired.
Four things were wrong in this path and are fixed here:
- **A read-only folder handed out an editing session.** The decision looked at
the requested mode and at whether the document came through a read-only
share, and never at the location's own rights. The backend token now carries
that decision, and the callback refuses a save made with a token that says
read-only.
- **The backend token was untyped and never expired.** It is signed with the
same secret as the Document Server's own tokens, so one of those would have
been accepted as ours. It now carries a type claim and a twelve-hour life.
- **Every unknown extension was announced as a text document.** The Document
Server validates documentType against the extension and refuses the config
outright, answering "the file content does not match the file extension" —
true, unhelpful, and several steps from the setting that caused it. .odg was
one of them: the Document Server counts a drawing as a presentation. The
mapping is now the one it ships, and an extension it has no editor for is
refused here, naming the setting.
- **Nothing ever expired.** Only a terminal callback released a key, so a
browser closed on the editor left the row for good — one for every document
ever opened. There is now an hourly sweep, and it takes with it the two
cleanups that were already in the tree with no caller at all:
`cleanupExpiredShares` and `cleanupExpiredSessions`, so an expired share no
longer sits on disk indefinitely.
The rename itself moved out of its route into a service, because two callers now
need exactly those checks in exactly that order. One thing changed on the way:
`ensureValidName` throws a plain Error, which the error handler could only read
as a server fault, so a name with a separator in it was answered 500 and
appeared in the logs as an unhandled failure. It is a bad request.
Gates: lint and formatting on the changed files, `npm run build`, every backend
module loads, and the whole backend suite — 1,972 pass, against the two that
fail on `main` before any of this. Twenty-three new tests; every claim above
fails when the change behind it is put back, mutated one at a time: the key back
to its inline computation, the type claim removed, the write check removed from
both ends, the save target back to the token, and the editor table back to its
catch-all.
Checked in the built image with a volume: the schema migrates, a document is
handed a key and a session, the heartbeat reports it open, opening it again
while it is open gives the same key, an unknown extension is refused by name,
renaming from the session moves the file and the folder lists the new name, and
ending the session is accepted.
Second half — editing together as the file list shows it, Save as, mentions,
force-save, the history inside the editor, and opening a document in a tab of
its own — follows under the same number.
A folder gave no sign that anybody had a document open. It was copied, moved, renamed or deleted while an editor was about to write a newer version of it, and nobody was told. A file somebody has open now carries a mark in the listing, with their names, and the folder keeps it current without asking for it: the server holds the request open until somebody joins or leaves, so presence costs nothing while nothing happens. Dragging such a document somewhere else asks first — it is allowed, and it is worth knowing that the editor will write where the file used to be. Presence is advisory and never a lock. Nothing here can stop a file operation, and nothing here survives a restart. The second half is writing. Closing the editor relied on the callback the Document Server sends when it decides the document is finished with, which arrives seconds after the last keystroke — long after the folder behind the editor has been listed again with the old content, and often after the tab is gone. The server now asks the Document Server to write what the editor holds, on the way out and on a bounded timer while editing, and the save it asked for on the way out is kept as a state of its own rather than as one of the automatic ones in between. Two requests for one session are coalesced: a close arriving while an automatic save is still assembling queues one final command behind it rather than a second one beside it, because each one costs a full document conversion. A Document Server that answers "the editor has not sent its last changes yet" is retried, four times, with increasing delays — so closing the preview stays instant and the last edits still land. Found while checking this in a browser: a listing refresh could not remove anything. The store merges a new listing into the items already on screen with `Object.assign`, which overwrites fields and never deletes them, so a mark the row had stopped carrying stayed for as long as the folder was open — a document nobody had open any more, and a history that had just been emptied. Only `supportsThumbnail` had been patched for this, one field at a time. A refresh now drops whatever the listing no longer sends, keeping only the fields the store puts there itself. Gates: lint and formatting on the changed files, `npm run build`, every backend module loads, and the whole backend suite — 1,986 pass, against the two that fail on `main` before any of this. Fourteen new tests; every claim above fails when the change behind it is put back, mutated one at a time: the listing no longer carrying presence, closing no longer flushing, the two requests no longer coalesced, and the close flush no longer counting as a deliberate save. Checked in the built image, which is also where the merge defect turned up: the mark appears when a document is reported open, without reloading the page, names the person, and goes when the session ends — and the request log shows the folder listing itself again because the held request answered, not because anything polled.
cerede2000
force-pushed
the
upstream-28-documents-editing
branch
from
September 26, 2026 08:58
3e7b829 to
d4cb268
Compare
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
2 participants
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.
Second part of batch 28, stacked on #418.
Who has a document open
A folder gave no sign that anybody had a document open. It was copied, moved, renamed or deleted while an editor was about to write a newer version of it, and nobody was told.
A file somebody has open now carries a mark in the listing with their names, and the folder keeps it current without asking for it: the server holds the request open until somebody joins or leaves, so presence costs nothing while nothing happens. Dragging such a document elsewhere asks first — it is allowed, and it is worth knowing the editor will write where the file used to be.
Advisory, never a lock: nothing here can stop a file operation, and nothing here survives a restart.
Writing what the editor holds
Closing the editor relied on the callback the Document Server sends when it decides the document is finished with — seconds after the last keystroke, long after the folder behind the editor has been listed again with the old content, and often after the tab is gone.
The server now asks the Document Server to write, on the way out and on a bounded timer while editing. The save asked for on the way out is kept as a state of its own rather than as one of the automatic ones in between. Two requests for one session are coalesced — a close arriving while an automatic save is still assembling queues one final command behind it rather than a second one beside it, because each costs a full document conversion. "The editor has not sent its last changes yet" is retried four times with increasing delays, so closing the preview stays instant and the last edits still land.
A defect the browser found
A listing refresh could not remove anything. The store merges a new listing into the items on screen with
Object.assign, which overwrites fields and never deletes them — so a mark the row had stopped carrying stayed for as long as the folder was open: a document nobody had open any more, and a history that had just been emptied (the version mark from #417 had the same problem). OnlysupportsThumbnailhad been patched for this, one field at a time. A refresh now drops whatever the listing no longer sends, keeping only the fields the store puts there itself.No unit test would have found it — upstream runs no frontend suite, and the server was answering correctly throughout. It showed up driving the image.
Gates
npm run build; every backend module loads.mainbefore any of this.Still to come under this number
Save as, mentions in comments, opening a document in a tab of its own with the page titles that go with it, and a document's history read inside the editor.