[28.1/33] An open document keeps its identity, and knows where it is - #418
Merged
Merged
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 2 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.
cerede2000
force-pushed
the
upstream-28-documents
branch
from
September 26, 2026 08:58
9bfb27f to
9f3b89b
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.
First half of batch 28. Stacked on #417 — review that one first; this PR's own diff is the 28 commit.
The problem
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: it opens a second, independent session on the same file, and whoever saves last overwrites the other with nothing to warn either of them.
The key was recomputed on every open from the file's 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 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, and the next autosave recreates the old name beside the new one.
What this adds
Defects fixed on the way
documentTypeagainst the extension and refuses outright, answering "the file content does not match the file extension" — true, unhelpful, and several steps from the setting that caused it..odgwas 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.cleanupExpiredShares,cleanupExpiredSessions), so an expired share no longer sits on disk indefinitely.ensureValidNamethrows a plain Error, which the error handler can only read as a server fault, so a name with a separator appeared in the logs as an unhandled failure. It is a bad request.The rename moved out of its route into a service, because two callers now need exactly those checks in exactly that order.
Gates
npm run build; every backend module loads.mainbefore any of this.Second half, under the same number
Editing together as the file list shows it, Save as, mentions in comments, force-save, the history inside the editor, and opening a document in a tab of its own.