[06/40] Remember a folder's sort and view as rows, not as one value per account by cerede2000 · Pull Request #447 · nxzai/NextExplorer

cerede2000 · 2026-09-26T20:38:59Z

Stacked on #433–#446. The last commit is the one to read.

What changes

A per-folder sort or view was stored as two JSON values per account under user_settings — one map of sorts, one of views — read and rewritten whole on every change. Three consequences followed from that shape, and this replaces it with one row per account and folder.

Two tabs on different folders overwrote each other. Each save sent the whole map, so whichever tab saved last won and the other folder's choice was gone. A preference is now written one folder at a time (PATCH /api/settings with { folderSort: { path, sort } }), and the row's UPSERT keeps the half it was not given — setting a folder's view no longer erases its sort.

The map had a ceiling of a hundred folders, because it shipped entire on every load and was rewritten entire on every change. The hundred-and-first folder silently forgot the oldest. Rows have no ceiling.

Nothing could clean it up. A folder deleted or renamed left its preferences behind on every account that had ever opened it, on a path that no longer existed.

Schema 20 creates folder_preferences and carries the old values over: one row per account and folder, merged under the later of the two times, and the values it read are removed so nothing reads them back. A value it cannot parse costs that folder its remembered sort rather than failing the startup.

Also in this batch

Same screen, same service, so they travel together:


A default view	for folders with none of their own. `null` is the built-in one; a mode there is no such thing as is refused rather than read as null — which used to put every folder back to the built-in view.
Markdown opens in the editor	for whoever mostly writes it. Only markdown: it is the one kind of file with both a preview and an editor, so the only one where opening it is a choice.
An access rule is refused with its reason	instead of dropped from the answer. Saving `../Secret` answered 200 with a list the page then adopted, and an administrator was left believing a folder was hidden that never was. A permission that was not one of the three became `rw`, so a mistyped `readonly` opened a folder for writing.
Every section is checked before any is written	A save carrying a valid section and a refused one stored the first and answered 400 — a request reported as refused that had changed something.
One list of what a preference is	It was three: the keys the route allowed, the if/else chain that sanitised them, and the defaults in the client store. A key in one and missing from another was accepted, silently dropped, and answered with its previous value, which the client applied — so the switch flicked itself back off. `markdownOpensInEditor` did exactly that.

Two smaller changes worth naming: PATCH /api/settings now answers with the settings read back from storage rather than an echo of what was sent, so what the caller applies is what a later request would read — tests/routes/settings-preferences.js was asserting the echo and now asserts each preference's value. And USER_SETTING_KEYS is WRITABLE_USER_SETTINGS, which says what it holds.

Proof

tests/services/folder-preferences-as-rows.test.js — eight tests. Each claim was put back to the old behaviour to check the right test goes red: overwriting instead of keeping the other half, deleting the account's rows on every write, reading without the owner, a LIMIT 100, accepting any word as a view mode, and dropping the carry-over.

Whole suite: 2685 passed, 2 failed — the two that already fail on main (auth.test.js on the wrong current password, browse-hidden-files.test.js on a FOREIGN KEY). npm run build clean, node -e "require('./backend/src/app.js')" clean, eslint and prettier at their main baselines.

Built, started and driven in a browser: schema 20 runs, a folder keeps the view it was left in while its neighbour uses the default, only the folder that was changed has a row, and markdown goes to /editor/... only when the preference says so.

Not in this batch

The inline quick-actions menu that this screen also carries in the fork stays out — it was offered in #333 and closed. frontend/src/stores/folderScroll.js waits for the router change that uses it.

The batch is reversed, so its findings move from PORT to DONE in the same commit that sends them. The instrument gains an eleventh gate, because this batch walked into what it could not see. `SettingsUserPreferences.vue` is a file a batch ports; it imports the quick-actions menu, which is OURS — offered upstream in nxzai#333 and closed. Upstream has no such file, so the batch would not have built there, and the build-order check said the batch needed nothing: it only knew files some batch brings, and skipped a target no batch ever will. Those imports are now findings of their own, on a `stranded` axis, classified by the same manifest as everything else — either the part is left out of the file the batch ports, or the file's verdict is wrong. Asking the question found five more of them, in three batches nobody had looked at yet: FileObject.vue and FolderViewToolbar.vue both import the menu, and the two vitest configs import the Node-version notice. Three other corrections the same look produced: - The strings under `settings.` were all assigned to this batch by one rule. A string now travels with the screen that asks for it: the account list's to P3-24, the access rules' to P3-23, the thumbnails' to P3-25, the uploads' to P3-28, and the quick-actions menu's nowhere, since the menu stays here. - `frontend/src/stores/folderScroll.js` moves to P3-29, which brings the router change that uses it. Sent with this batch it would have been a module nothing imports. - The quick-actions specs are OURS, like what they test. P3-41 claimed every frontend spec, and first match wins, so the rule saying otherwise has to sit ahead of it. What is left to reverse is whatever `scripts/parity.mjs` still reports.

P3-17 was going to send the destination picker: a service, a dialog and a composable. Nothing upstream could have called any of them. The route that records a destination is in P3-27, the client that asks for the list in P3-35, the trash and the versions that note theirs in P3-30 — all after. Three files that would have landed green and unreachable. The build-order check reads each batch's imports and refuses a batch that needs a file coming later. It cannot see the other direction, so this adds it: a file a batch brings that only later batches import is reported. Asking that question found seventeen files across nine batches, not three. `frontend/src/stores/files/` — the file store as nine modules — was a batch of its own, ahead of the store that assembles them. So were the editor's code surface, the terminal's input helper, the folder list's keyboard and windowing helpers. Each one now travels with the batch of its first caller, and three batches disappear into the batches that were going to use them: twenty-five left instead of twenty-eight. `useDestinationPicker.js` goes earliest of all, to P3-18, because the archive preview asks where to put something before anything else does. One subtlety the first version got wrong: upstream having the importing file is not enough. `routes/files/transfer.js` exists there and does not require the destination service, so the check reads upstream's own copy for the import line rather than the file's existence. The migration axis compared version numbers, and version numbers diverged long ago — this fork is at 24 where upstream is at 19, and the same feature carries a different number in each. It reported five migrations missing that upstream has had for months under other names: two-factor as `totp_credentials`, the activity log as `activity_events`. And it said nothing about the tables that really are missing. It now compares what a migration leaves behind — the tables, and the columns added to a table that exists — across every file that holds DDL, not db.js alone. Eleven real gaps, each named and each assigned to the feature that reads it: `personal_folder_reservations` to P3-23, `recent_destinations` to P3-27, the seven `shares` columns to P3-21, `trash_items.restore_entry` to P3-30. `folder_preferences` is DONE, sent in nxzai#447.

Two routes built a command line with values from the request in it and gave the line to a shell. `routes/usage.js` pasted a folder's path into `du -sb "…"` and `df -Pk "…"`; `routes/permissions.js` pasted an owner, a group, a mode and a path into `chown`, `chgrp`, `chmod -R` and two `id` lookups. A name is not a shell string. Anything that closes the quoting leaves the rest for `/bin/sh` to run, as the user the server runs as. The usage one needs no privilege at all: any account that can create a folder can name one, and where AUTH_ENABLED is false that is anybody who can reach the server. `execFile` takes the arguments as a list, so there is no line for a shell to read and no shell. The commands, their output and the answers are unchanged — this is deliberately the smallest change that closes it, and not the rewrite that stands in nxzai#450 and nxzai#453. An argument list is not a free pass on its own: `chown` reads a leading dash as an option, so `--reference=/etc/shadow` would have copied another file's ownership onto the target. An account or group name has to look like one. `tests/routes/no-shell.test.js` — three cases. Two of them name a folder, and an owner, with a payload that creates a file, and check the file is not there; the third asks for `--reference=/etc/shadow` and expects a refusal. All three fail against the routes as they are, the first two by running the command. The payload only ever touches the working directory, and it is removed whatever an assertion does, so a run that does execute leaves nothing behind.

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. One box, one name for it ------------------------ The sign-in box takes an email address or a username, so it is neither: it is whatever was typed, and this calls it `identifier` from the screen to the route. The screen and the client were renamed and the store and the route were not, so the store passed `email` to a client expecting `identifier`. `JSON.stringify` drops a key whose value is undefined, and the request went out carrying a password and nobody to sign in. The answer was "invalid credentials", which is what a wrong password looks like — so nothing about it read as a defect. `attemptLocalLogin` already took `identifier`; the route is what had not caught up. `email` and `username` still work, for a script or an older client that sends them. The identifier was the half that failed loudly. The screen also reads `totpPending`, `oidcStatus`, `cancelTotp`, `ensureStatus` and `forgetSession` off the store, and none of them were there: `totpPending` read undefined, so the box for the code from the authenticator never appeared, and a correct password on an account with a second factor landed on a screen that looked like it had done nothing. Undefined is not an error in a template — it is a `v-if` that is false. The store is here in full, with the code step read back from the server on every start so a reload in the middle of one lands back on the code. tests/routes/sign-in-identifier.test.js signs in with each of the three names, refuses a wrong password, and reads the three frontend files to check they all use the one name — the chain is four files long and three of them have no runner here, which is how it broke silently in the middle. And what a refusal says ----------------------- A code that names the kind of refusal — FORBIDDEN, NOT_FOUND, CONFLICT, RATE_LIMIT_EXCEEDED — is translated for the reader, and the server's own sentence, which says *which* refusal, went underneath rather than being lost. A lock that arrives with a duration gets a sentence of its own rather than a placeholder in the plain one, so it can never read "{minutes}". And the handler no longer asks vue-i18n for a key before checking it has it, which was a console warning for every refusal the catalogue has no entry for, twice.

Two reports the server makes about itself at start, neither of which existed. **What releases up to 1.1.7 left in the cache.** The database and app-config.json lived in CACHE_DIR until 1.1.8, which moved them to CONFIG_DIR and left links behind; 2.0.3 removed that move from the entrypoint. So an installation that started on 1.1.7 or earlier and skipped the releases in between comes up on a new, empty app.db in CONFIG_DIR with its accounts, shares and settings sitting unread in the cache. Nothing said so: the server started, the sign-in page offered to create the first administrator, and the answer looked like a fresh installation rather than a lost one. It is a notice and nothing more — nothing is moved, nothing is deleted. It names what it found and where, and says what to do with it. **What the process is costing.** `services/performanceDiagnostics.js` samples CPU, resident memory as the cgroup sees it rather than as the host does, event-loop delay at p99, and the queues that can grow: thumbnails, folder sizes, transfers. Off unless PERFORMANCE_DIAGNOSTICS_ENABLED is set, and then it reports only the intervals that pass a threshold — a diagnostic that logs every interval by default is a diagnostic that fills a disk. PERFORMANCE_DIAGNOSTICS_LOG_EVERY_INTERVAL asks for all of them. An interval below its floor is held to the default: a sampler on a 1 ms interval costs more than whatever it was meant to diagnose, and 1 ms is what an emptied field sends. Each queue reports itself through an optional call. One that has no report is a queue this installation has nothing to say about, not a reason for the whole record to fail — a diagnostic that throws says nothing at the moment it is most wanted. ## Checks `legacy-cache-check.test.js`, 3 tests: a cache holding the old names is named, one holding the links 1.1.8 left is named differently, and an ordinary cache says nothing. Making the inspection always see an empty cache turns all three red. `performance-diagnostics.test.js`, 7 tests: silent unless asked for, says what it will watch and by which thresholds, holds an interval below its floor to the default, reports nothing of an ordinary interval, reports one that passes a threshold and says which kind it was, reports every interval when told to, and samples the machine rather than guessing. Making it start whether or not it was asked for turns the first red. Whole backend suite: 2 556 passed, 2 failed — the two that fail on `main` on its own.

`/api/features` already offers `preview.maxRenderBytes` to the screen, and the configuration had no such value, so it answered `null` and the preview rendered whatever it was given. A document large enough to render slowly renders slowly for everybody on the machine, and nobody could raise or lower the point at which it stops trying. PREVIEW_MAX_RENDER_SIZE sets it; 16 MB when it is not set, and a value below zero or unparseable is that default rather than a ceiling of nothing. And the listing: `GET /api/browse` answers with what is true at that moment — which documents somebody has open in an editor, what a folder weighs, whether a write would be refused. A GET with no cache header is cacheable by default, so a proxy or a browser was free to keep it and serve a folder as it was: a deleted file still listed, a document shown as open by somebody who closed it an hour ago. `private, no-store` says what it is. ## Checks `browse-caching.test.js` asserts both halves of the header, and taking the header away turns it red. The ceiling is read through `/api/features`, which has offered the field since the batch that brought the features route and was answering `null` for it. Whole backend suite: 2 556 passed, 2 failed — the two that fail on `main` on its own.

`getDb` checks whether a connection is already open and opens one when it is not. Every caller that arrives before the first has finished sees "not open" and opens another — and everything that starts with the server asks at once: the session store, the settings, the trash sweep, the search index, the favourites. So a start opened app.db four times, ran `migrate` over the same file four times in parallel, and whichever finished last became the one everybody used. One opening is shared now. `openDb` does the work, `getDb` hands every caller the same promise while it is in flight and the same connection afterwards, and `closeDb` lets a test take it away again — which is what a suite opening a database per case needs. And `prepared(db, sql)`: `db.prepare` compiles the SQL every time it is called, and the hot paths called it per row — a listing asking whether each of a thousand entries is a favourite compiled the same statement a thousand times. Kept in a WeakMap keyed on the connection, so the cache goes when the connection does and a statement is never handed to a connection that did not compile it. ## Checks `db-single-open.test.js`, 5 tests. Five callers in one turn get one connection — counted on the connection objects rather than on anything the code says about itself. A later caller gets the one already open. After `closeDb` the next caller gets a new, working one. The same SQL twice compiles once, and the same SQL against a new connection compiles again. Putting the per-caller opening back turns the first red. Whole backend suite: 2 561 passed, 2 failed — the two that fail on `main` on its own. Two files were in this batch and are not: `betterSqliteSessionStore.js` and `bootstrap.js`. `main`'s versions are the newer ones — it opens the session database on first use rather than when the module is required, which is what keeps `require('./backend/src/app.js')` working where the cache directory does not exist yet. Bringing the fork's versions over them would have broken that check.

Each account's per-folder choices were two JSON values under `user_settings`: one map of sorts, one of views, read and rewritten whole on every change. Three things followed, and all three are gone here. Two tabs on different folders overwrote each other. Each save sent the whole map, so whichever tab saved last won and the other folder's choice was lost. A preference is now written one folder at a time, through `PATCH /api/settings` with `{ folderSort: { path, sort } }`, and the row's UPSERT keeps the half it was not given: setting a folder's view no longer erases its sort. The map had a ceiling of a hundred folders, because it shipped entire on every load and was rewritten entire on every change; the hundred-and-first folder silently forgot the oldest. Rows have no ceiling. And nothing could ever clean it up. A folder deleted or renamed left its preferences behind on every account that had ever opened it, on a path that no longer existed. As rows they can be removed with the folder they describe. Schema 20 creates `folder_preferences` and carries the old values over: one row per account and folder, merged under the later of the two times, and the values it read are removed so nothing can read them back. A value it cannot parse costs that folder its remembered sort rather than failing the startup. Also here, because they are the same screen and the same service: - A default view for folders that have none of their own. `null` means the built-in one; a mode there is no such thing as is refused rather than read as null, which used to put every folder back to the built-in view. - Markdown opens in the editor, for whoever mostly writes it, instead of going through the preview and clicking Edit every time. Only markdown: it is the one kind of file that has both, so it is the only one where opening it is a choice. - An access rule that cannot be stored is refused with its reason instead of being dropped from the answer. Saving `../Secret` used to answer 200 with a list the page then adopted, and an administrator was left believing a folder was hidden that never was. Worse, a permission that was not one of the three became `rw`, so a mistyped `readonly` opened a folder for writing. - Every section of a save is checked before any of it is written. A payload carrying a valid section and a refused one used to store the first and answer 400 — a request reported as refused that had changed something. - One list of what a preference is. It was three: the keys the route allowed, the chain of if/else that sanitised them, and the defaults in the client store. A key in one and missing from another was accepted, silently dropped, and answered with its previous value, which the client applied — so the switch flicked itself back off. `markdownOpensInEditor` did exactly that. `PATCH /api/settings` now answers with the settings read back from storage rather than an echo of what was sent, so what the caller applies to its own state is what a later request would read. `tests/routes/settings-preferences.js` was asserting the echo, and now asserts the value of each preference it saved. `USER_SETTING_KEYS` is `WRITABLE_USER_SETTINGS`, which says what it holds. tests/services/folder-preferences-as-rows.test.js covers the eight claims above. Each was put back to the old behaviour to check the right one goes red: overwriting instead of keeping the other half, deleting the account's rows on every write, reading without the owner, a LIMIT of 100, accepting any word as a view mode, and dropping the carry-over. Whole suite: 2685 passed, 2 failed — the two that already fail on main (auth.test.js on the wrong current password, browse-hidden-files.test.js on a FOREIGN KEY). Built, started, and driven in a browser: a folder keeps the view it was left in while its neighbour keeps the default, and markdown goes to the editor only when the preference says so.

cerede2000 mentioned this pull request Sep 26, 2026

[07/40] Ask where to put something, and offer the folders somebody actually uses #448

Merged

cerede2000 force-pushed the p3-16 branch 4 times, most recently from bcaed3d to 7ceca99 Compare September 27, 2026 12:29

Benjy added 6 commits September 27, 2026 19:24

cerede2000 force-pushed the p3-16 branch from 7ceca99 to 6e2d75c Compare September 27, 2026 17:28

cerede2000 changed the title ~~Remember a folder's sort and view as rows, not as one value per account~~ [06/40] Remember a folder's sort and view as rows, not as one value per account Sep 27, 2026

vikramsoni2 merged commit 3c8a678 into nxzai:main Sep 27, 2026
1 check passed

Navigation Menu

[06/40] Remember a folder's sort and view as rows, not as one value per account - #447

[06/40] Remember a folder's sort and view as rows, not as one value per account#447
vikramsoni2 merged 6 commits into
nxzai:mainfrom
cerede2000:p3-16

cerede2000 commented Sep 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cerede2000 commented Sep 26, 2026

What changes

Also in this batch

Proof

Not in this batch

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants