Switch the search index and the folder sizes on from Settings - #440
Merged
Merged
Conversation
added 9 commits
September 26, 2026 19:06
The About page's list of optional tools showed its own keys:
`settings.about.tools.title` where the heading belongs, and
`settings.about.tools.gives.videoThumbnails` under each one. The page was
ported with no catalogue entries at all, and nothing failed — the build
succeeded, the page rendered, and the words were missing. It is the one defect
a batch of screens can ship with while passing every other gate.
Twelve more keys were already in that state, on eleven screens: the Back
button on an account, the folder-size "Excluded" mark and its explanation, the
Access heading on both share lists, the branding page's note after choosing the
default logo, the empty user list, and six strings in the share dialog —
Password, Protected, Expires, Shared with, Creating, Loading users.
So a test reads every `t('…')` a screen asks for and holds it to the English
catalogue. English alone: a key missing from the others falls back to it, which
is a reader seeing the wrong language rather than a key. A prefix being built
up — `t('settings.categories.' + name)` — is not a key and is passed over,
which is also the limit of what can be checked from the source.
…t carry
Batch 33 of the plan was "the locale catalogues, with the key-parity test". The
catalogues are at parity on `main` today; nothing holds them there. The test that
was supposed to travel with them stayed in the fork, where it is a frontend spec
and there is no frontend runner here to execute it.
So it is written where the runner CI executes lives, reading the catalogues as
files. Three things, each the failure it prevents:
- **Every English key in every language.** A key added in English and nowhere
else falls back to English: a reader seeing the wrong language, quieter than a
raw key and just as wrong.
- **No key English does not define.** A key left behind after it was renamed in
English is dead weight nobody will ever see again.
- **The placeholders survive translation.** `{count} items` translated without
`{count}` says "items", with the number silently gone.
All three pass as things stand. Each was checked by breaking one catalogue — a key
removed from French, an invented key added to German, `{percent}` dropped from
Spanish — and each assertion named its own mutation and nothing else.
`auth.sessionSecret` fell back to `crypto.randomBytes(32)` whenever SESSION_SECRET was not set, and `configureSession` drew a second one of its own behind it. The sessions themselves survive a restart — they are rows in CACHE_DIR/sessions.db — but a cookie signed with the previous secret no longer verifies, so the row was read as a stranger's. Every restart, every upgrade and every crash asked everyone to sign in again, on the default configuration. The secrets derived from it went with it: ONLYOFFICE without ONLYOFFICE_SECRET, and the signed thumbnail links. So the first start draws one and keeps it in CONFIG_DIR, and later starts read it back. SESSION_SECRET still wins and nothing is written then. The file is written beside its final name and renamed into place, so a start interrupted half-way leaves either no file or a whole one, and it is created 0600 — nobody but the user the server runs as has business reading it. A CONFIG_DIR that cannot be written is not fatal: the start says, in words, that a secret is being used for this run only and that the next restart will sign everyone out, which is the behaviour it had all along. `configureSession` now takes the resolved value rather than drawing its own, so there is one secret and one place that decides it.
Node's default for a rejected promise with no listener is to raise it as an uncaught exception and stop the process. So one forgotten `await` anywhere — on a path check, a database read, a stat — answers a single bad request by taking the server down, and everybody else's work goes with it. That price is out of proportion to the cause. A rejection raised while serving a request is almost always confined to that request: the connection fails, and nothing else is touched. So it is reported, with what it was rejected with, and the server carries on. An uncaught exception is treated as what it is. There the stack unwound through code that had no chance to put anything back, so a lock may still be held and a transaction half applied; serving from that is worse than stopping. It shuts down — the ordinary cleanup first, so the store and the sweeps close as they would on SIGTERM — and exits 1. The shutdown runs in the same unknown state, so it is bounded: five seconds, or a failure of its own, and the process still ends. Nothing is hidden from development. Only `server.js` installs this; the test suites never load it, and the runner still fails a run that leaves an unhandled rejection behind.
`POST /api/files/office-document` and `createOfficeDocument` in the API layer both
landed with nothing calling them: there is no screen from which a blank Word,
Excel or PowerPoint document can be created, so the route cannot be reached.
The New menu gains a New document row, and a drawer beside it listing what can be
made — the three office formats when an editor is configured, and text, Markdown
and CSV, which are worth creating whether one is or not. A blank .docx is only
useful if something opens it, so the office entries are hidden when neither
ONLYOFFICE nor Collabora is set up.
Naming comes first here, unlike everywhere else in the app: the document opens
straight into an editor covering the whole window, so an inline rename box would
be behind it. The dialog shows the extension and does not let it be edited — the
format was chosen from the menu and the server owns the extension either way.
Once created, the document opens in the editor rather than landing back in the
listing, which would leave the person to find and open a file they have just
asked for by name.
Two defects in the files this touches, fixed here:
`createFile` in the file store read the created name from `created?.name`, where
the route answers `{ success, item }`. That was always undefined and always fell
back to the name asked for, so when that name was taken and the server picked the
next free one, the store looked up the asked-for name in the refreshed listing,
found the file that already held it, and opened the rename box on *that* — making
a second untitled file renamed the first one.
The button that opens the New menu sits outside the menu, so `onClickOutside`
counted its click as outside: the handler shut the menu in the capture phase and
the button's own toggle opened it again, and the button could open the menu but
never close it. It is now ignored by name.
The drawer is measured against the viewport each time it opens, and hands itself
to the other side when there is not room — a menu near the right edge would open
its drawer off-screen otherwise.
The plan's own rule is that a batch carries its documentation. The trash landed in nxzai#405 and nxzai#416, file versions in nxzai#406 and nxzai#417, and neither brought a page: there is nothing under `docs/` that describes either, so the retention settings, what a restore keeps, who sees what in the trash and how versions are thinned are all readable only in the source. Two new pages, and nine existing ones brought up to what the application now does — the environment reference gains the variables the last twenty batches added, the feature and workflow pages gain the trash, versions, archives and search index, and the sidebar gains the two new entries in both of its shapes. Both pages were trimmed to what `main` has, rather than copied. Three things are deliberately left out and travel with the batch that brings them: - restoring to a folder of your choosing. `POST /api/trash/restore` accepts a destination, but nothing on the screen offers one and `restoreTrashItems` does not send one, so the page would describe an action nobody can reach. - `COPY_PRESERVE_PERMISSIONS`, `PREVIEW_MAX_RENDER_SIZE`, `BULK_DELETE_CONCURRENCY`, `MAX_BROWSABLE_ARCHIVE_SIZE`, `ARCHIVE_CACHE_MAX_SIZE`, `UPLOAD_CHUNKED_AUTO_FALLBACK` and the six `PERFORMANCE_DIAGNOSTICS_*` — twelve rows for variables the configuration does not read yet. - recent destinations, per-folder preferences and the inline quick-actions menu. ## Checks `npm run docs:build` — and it is worth saying that it was failing before this last pass: three links pointed at an installation page that only exists in the fork, and vitepress treats a dead link as an error. Every internal link in the eleven pages was resolved against the tree, which is how those turned up. `npm run format:check` reports the same 22 files as `main` does on its own; the eleven pages and the config are clean. `npm run build` and the backend module load both pass, untouched by a documentation batch.
Two things about OIDC that this fixes. **Which of two failures it was.** A sign-in that cannot start has one of two causes: nothing was configured, or what was configured could not be made to work. The first is answered by filling in OIDC_ISSUER and the rest; the second is answered by looking at the provider. Both answered 404 "OIDC is not configured", so an administrator whose provider was unreachable was sent to change a configuration that was already right. `configureOidc` now records what it concluded and why, and the routes read it: 404 AUTH_OIDC_NOT_CONFIGURED for the first, 503 AUTH_OIDC_PROVIDER_UNAVAILABLE for the second, and neither carries the network's own words — no ENOTFOUND, no internal host name, in the body or in the address bar. A browser asking for one of those addresses is looking at a page, not reading JSON, so it is sent back to the sign-in screen with the code beside the sentence. The screen can then say what the code means in the reader's own language. **Where the callback comes back to.** The return address was built from a fixed `baseURL`, so a deployment reached through a reverse proxy on a different name sent people back to the wrong origin. It is now resolved from the request — but only from a forwarded host behind a trusted proxy, and only when the result exactly matches an origin the operator configured. A redirect target that a request header could choose is an open redirect with extra steps. The path is still held to a relative, same-site one, as before. Also here, because it is the same files: - `claimsFromIdToken` in the middleware and `uniqueOrigins` in the routes were local copies of things `utils/idToken.js` and the new `utils/oidcRedirect.js` do; both go. - The guest-session cookie was cleared on `/api` only, at five sign-in and sign-out paths. A guest session left on `/` outlived the sign-in that should have ended it. - `ServiceUnavailableError` (503) and the two AUTH_OIDC_* codes, which nothing had yet. The account lockout that `routes/auth.js` also differs in is deliberately not here: it is a different subject and goes with releasing a locked account. ## Checks Seven test files, 107 tests, including three this fork had and `main` did not: `oidc-middleware`, `oidcOrigin` and `auth-oidc-routes`. Neutralising `getOidcAvailability` so it reports one verdict for both causes turns three of them red — the three that tell the two apart. Whole backend suite: 2 391 passed, 2 failed, the two that fail on `main` on its own (`auth.test.js` on the current password, `browse-hidden-files.test.js`). `npm run lint` reports 146 against `main`'s 143, the three being the parse error on `backend/tests/**` that 141 of `main`'s own test files already draw. Formatting clean. Frontend builds, backend loads, documentation site builds. One test assertion was rewritten rather than ported as it stood: it matched the wording `express-openid-connect` uses for a callback with no sign-in in progress, and that wording differs between 2.19 and 2.20. It now asserts the refusal.
The logo was written to `custom-logo.png`, over whatever held that name, the instant it was chosen, and a reset removed the three fixed names outright. So choosing a logo destroyed the one before it — there was nothing to change back to — and a branding save that failed after the write had already replaced the logo in use. `services/brandingLogo.js` gives each logo a name of its own, derived from nothing but itself, and writes it through `placeWithoutOverwrite`, so a name already taken is not replaced. The settings are switched to it only once the file is whole and in place, and the logo that was replaced is forgotten only once nothing points at it any more. A failure at any step leaves the logo in use exactly as it was, and takes the new file with it. `replaceBranding` in the settings service reads and writes the branding without yielding in between, so two saves at once cannot both start from the same value: the logo a save removes is the one it was the last to see, and it cannot take away a logo another save has just put in place. What earlier versions wrote under the fixed names is removed once a logo of the new kind is in place, and a logo left behind by an interrupted change is swept at the next start, where nothing is being written and so a file under one of our names is a finished one. Two things found in the files this touches: **A refusal that was built and thrown away.** `explainMultipartRefusals` puts a route's own sentence on a multer error — the upload route's says which ceiling was passed and which variable raises it — and `multipartRefusal` in the error handler ignored it. Every route that took the trouble to explain its own limit got "the file is larger than this server accepts" instead. One of `main`'s own tests asserted that generic wording, and was green for the wrong reason; it now asserts the sentence the route sets. **The deletion allowance moves with the deletion.** `.eslintrc.cjs` let `routes/settings.js` remove files because that is where the logo was removed. The route no longer removes anything, so the allowance names the service instead, with the reason. ## Checks `settings-logo.test.js` and `branding-logo-sweep.test.js`, 26 tests. Giving `ownLogoName` a fixed name again — the old behaviour — turns **all 26** red. Driven against the image as well, because this is a screen: two different PNGs uploaded in turn get two different addresses, the second does not replace the first, the old address answers 404 once nothing points at it, and one file is left in `/config/logos`. The branding screen itself was driven in a browser — no raw translation keys before or after saving, and no console errors. Whole backend suite: 2 435 passed, 2 failed, the two that fail on `main` on its own. Formatting and lint unchanged in kind. Frontend builds; the eight strings are in all fifteen catalogues.
This is the remainder of published batch 31. `SEARCH_INDEX` and `FOLDER_SIZE_MODE` decided whether either worker ran, and nothing else could: turning the index on meant editing a file on the host and restarting the server, while every other setting on the page beside it was a click (#9). `features.js` reported `lockedBy: 'SEARCH_INDEX'` unconditionally, so the screen said the environment decided even when nobody had set anything. `services/featureSwitches.js` holds the rule the exclusion lists already follow: the environment is the floor. A variable somebody set decides and cannot be moved from the page — and "false" is a decision too, so an installation that turned the index off in its file has not left it to whoever next opens the page. When nobody set one, Settings decides, and what it decided is read back before either worker starts, so a switch thrown on the page is still on after a restart. A switch sent for something the environment holds is refused in words that name the variable, rather than accepted and quietly ignored. `lockedBy` now says which of the two it is, or nothing. `SEARCH_INDEX_SET` and `FOLDER_SIZE_MODE_SET` are what that rests on: whether the operator set the variable at all, as opposed to what it came out as. Without them, `SEARCH_INDEX=false` and an unset `SEARCH_INDEX` are the same value and cannot be told apart. ## Checks `settings-background-switches.test.js`, 11 tests: each switch moves and stays moved, each is refused while its variable is set — for "false" as much as for "true" — a value neither switch takes is refused, and nothing is written when a locked switch is sent beside a valid exclusion list. Making `featureSwitches` report the variable as always set — the behaviour being replaced — turns two of them red, the two that move a switch. Whole backend suite: 2 446 passed, 2 failed, the two that fail on `main` on its own. Two things were deliberately left out of this batch after they turned tests red: `routes/search.js`, `searchCollector.js`, `indexDb.js` and `searchIndexer.js` are how the search answers rather than what starts it, and porting them here made four of `main`'s own tests fail — a personal folder belonging to somebody else came back in the results, because the filtering that version does rests on access work a later batch brings. They travel with the search screen instead, and so do the seven `search.*` strings.
cerede2000
pushed a commit
to cerede2000/NextExplorer
that referenced
this pull request
Sep 26, 2026
The batch is reversed, so its findings move from PORT to DONE in the same commit that sends them. What is left to reverse is whatever `scripts/parity.mjs` still reports.
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
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.
P3-08 of phase 3 (see #373). Stacked on #433–#439.
The remainder of published batch 31
Batch 31 was "Search: names from the index, ranking, switches in Settings". The names
and the ranking landed in #427; the switches did not.
SEARCH_INDEXandFOLDER_SIZE_MODEdecide whether either background worker runs, andnothing else can. Turning the index on means editing a file on the host and restarting
the server, while every other setting on the same page is a click (#9). And
routes/features.jsreports it as locked whether or not anybody set anything:so the screen says the environment decided even when the environment is silent.
The environment is the floor, not the only voice
services/featureSwitches.jsapplies the rule the exclusion lists already follow:falseis adecision too — an installation that turned the index off in its file has not left it
to whoever next opens the page.
either worker starts, so a switch thrown on the page is still on after a restart.
variable, rather than accepted and quietly ignored.
lockedBysays which of the two it is, or nothing.That rests on
SEARCH_INDEX_SETandFOLDER_SIZE_MODE_SET: whether the operator set thevariable at all, as opposed to what it came out as. Without them
SEARCH_INDEX=falseandan unset
SEARCH_INDEXare the same value and cannot be told apart.Checks
settings-background-switches.test.js, 11 tests: each switch moves and stays moved;each is refused while its variable is set, for
falseas much as fortrue; a valueneither switch takes is refused; and nothing is written when a locked switch arrives
beside a valid exclusion list.
Making
featureSwitchesreport the variable as always set — the behaviour beingreplaced — turns two of them red, the two that move a switch.
Whole backend suite: 2 446 passed, 2 failed, the two that fail on
mainalone.Formatting and lint unchanged in kind; frontend, backend load and docs site all build.
What was taken back out of this batch
routes/search.js,searchCollector.js,indexDb.jsandsearchIndexer.jswere in itat first. They are how the search answers rather than what starts it, and bringing them
here turned four of
main's own tests red: a personal folder belonging to somebodyelse came back in the results, because the filtering that version does rests on access
work a later batch brings. They travel with the search screen instead (P3-22), and so do
the seven
search.*strings.