fix(cli): 把本地存储根写成设置服务读的那个 env 名 —— OS_STORAGE_ROOT → OS_STORAGE_LOCAL_ROOT(方案 B)

.changeset/storage-local-root-env-channel.md

@@ -0,0 +1,44 @@
+---
+'@objectstack/cli': patch
+---
+
+fix(cli): `OS_STORAGE_ROOT` now actually takes effect — renamed to `OS_STORAGE_LOCAL_ROOT`, the name the settings service reads (#4968)
+
+The CLI and the settings service spelled the local storage root differently.
+The CLI wrote its own invented name, `OS_STORAGE_ROOT`; the settings service
+derives the env name for the same value from the namespace it owns —
+`envKeyOf('storage', 'local_root')` = `OS_STORAGE_LOCAL_ROOT` — and nothing in
+the repo ever set that. So the two channels never met: `os serve` constructed a
+local adapter at the root the operator named, `StorageServicePlugin` re-resolved
+from settings at `kernel:ready`, found only the manifest's **schema default**,
+and swapped the adapter to `./.objectstack/data/uploads`.
+
+`OS_STORAGE_ROOT` therefore took effect for exactly one value — the one that
+happens to equal that default — which is why plain `pnpm dev` never showed it.
+Every other value was constructed and then discarded:
+
+- **Production**: `OS_STORAGE_ROOT=/srv/uploads` was ignored and uploads landed
+  under the process cwd. An operator following `backup-restore.mdx` backed up an
+  empty directory.
+- **`dev --fresh`**: the tempdir was documented to own all state for the run;
+  uploads actually went to the project cwd and survived process exit.
+- Every clean boot logged a data-loss-grade "adapter swapped … existing files
+  were NOT migrated" warning. That warning was **accurate** — the swap really
+  happened — and is untouched here. It stops firing because the swap stops.
+
+The fix is at the producer, not as a tolerant read in the consumer: `dev.ts`
+publishes `OS_STORAGE_LOCAL_ROOT`, and `serve.ts` resolves the root through one
+channel (`resolveStorageLocalRootEnv`), shared with `os migrate`'s storage
+bootstrap so the CLI materialises bytes exactly where the server would.
+
+`OS_STORAGE_ROOT` keeps working for **one release** via
+`readEnvWithDeprecation('OS_STORAGE_LOCAL_ROOT', 'OS_STORAGE_ROOT')`, warning
+once per process, and is then removed. When the legacy name supplies the value
+it is also stamped onto the canonical name, because the settings service only
+ever looks up `OS_STORAGE_LOCAL_ROOT` — without the stamp a deployment on the
+old spelling would keep the original defect in full.
+
+Storage settings now resolve `source: 'env'` at the value the adapter was built
+with, so Setup → Settings → File Storage shows the directory actually in use.
+No change to `packages/services/service-storage` — the swap predicate is correct
+and stays as is.

content/docs/deployment/backup-restore.mdx

@@ -76,13 +76,27 @@ Cloud-managed environments — the open framework does not ship that driver.)
 
 ## Backing up uploaded files
 
-The local storage adapter keeps uploads under `OS_STORAGE_ROOT` (default
+The local storage adapter keeps uploads under `OS_STORAGE_LOCAL_ROOT` (default
 `./.objectstack/data/uploads`). On single-host deployments, include that
 directory in the same schedule as the database so records and their
 attachments restore to the same point in time. Deployments using an external
 storage service (S3-compatible, etc.) inherit that service's durability and
 versioning instead.
 
+<Callout type="warn">
+**Verify the directory you are backing up actually holds the files.** This
+variable was previously named `OS_STORAGE_ROOT`, and on releases before the
+rename the CLI and the settings service spelled it differently — so any value
+other than the default was discarded at startup and uploads landed in
+`./.objectstack/data/uploads` instead of where the variable pointed. A backup
+job aimed at the configured path copied an **empty directory**. The old name
+still works for one release (with a startup warning), but it now resolves to
+the same place the adapter writes. Confirm with **Setup → Settings → File
+Storage → Root directory**: it shows the path in effect, and shows it as
+locked-by-env when the variable is set. If they disagree, trust the Setup page
+and back that path up too.
+</Callout>
+
 ## The restore drill
 
 Rehearse this on a scratch host **before** go-live, and again after any
@@ -106,8 +120,8 @@ pg_restore --clean --if-exists -d "$OS_DATABASE_URL" backup-2026-07-14.dump
 ### Provide the original secrets
 
 Set `OS_SECRET_KEY` and `OS_AUTH_SECRET` to the **escrowed originals** — not
-freshly generated values. Restore `OS_STORAGE_ROOT` contents if you use local
-file storage.
+freshly generated values. Restore `OS_STORAGE_LOCAL_ROOT` contents if you use
+local file storage.
 
 ### Boot from the artifact and verify
 

content/docs/deployment/environment-variables.mdx

@@ -51,7 +51,8 @@ read at startup unless noted otherwise. Boolean variables accept `true` / `false
 | `OS_DATABASE_DRIVER` | enum | inferred | Force a specific driver when the URL is ambiguous. `memory` \| `sqlite` \| `sqlite-wasm` \| `postgres` \| `mongodb`. |
 | `OS_DATABASE_SQLITE_JOURNAL_MODE` | enum | `wal` | Journal mode for **file-backed** SQLite. `wal` (default) lets a dev server and CLI commands share one file without blocking each other, and is what makes the `os migrate` occupancy check reliable. Set to `delete` for SQLite's rollback journal — required when the database lives on a **network filesystem** (NFS/SMB), where WAL cannot work. The setting is applied, not merely skipped: `delete` converts a database that already adopted WAL back. Ignored for `:memory:`, for the WASM SQLite driver, and for non-SQLite drivers. A per-datasource `sqliteJournalMode` in driver config outranks it. See [Journal mode](/docs/data-modeling/drivers#journal-mode-wal-and-cross-process-access). |
 | `OS_ALLOW_DRIVER_CONNECT_FAILURE` | boolean | `false` | Escape hatch for the driver-connect boot guard. By default a data driver that fails to connect at startup **refuses the boot** — a server that cannot reach its database must not report itself started and then fail every request. The same guard covers a **declared datasource** that objects bind to via `datasource: '…'`, or an `external` one with `validation.onMismatch: 'fail'`: those objects have no fallback datasource, so an unconnected one means they are all dead. Set to `1` to boot anyway, in an explicitly degraded state logged loudly at startup. There is **no reconnection**: whatever failed stays dead for the process lifetime and every query and schema sync routed to it fails. |
-| `OS_STORAGE_ROOT` | path | `./.objectstack/data/uploads` | Root directory for the local file storage adapter, relative to the process cwd (used by `os serve`'s default `storage` capability wiring). |
+| `OS_STORAGE_LOCAL_ROOT` | path | `./.objectstack/data/uploads` | Root directory for the local file storage adapter, relative to the process cwd (used by `os serve`'s default `storage` capability wiring). This is the same value as **Setup → Settings → File Storage → Root directory**; setting it here pins that field (it shows as locked-by-env). Renamed from `OS_STORAGE_ROOT` — see below. |
+| `OS_STORAGE_ROOT` | path | — | **Deprecated alias for `OS_STORAGE_LOCAL_ROOT`.** Still read for one release, with a startup warning; it will be removed in a future major. Rename it now. Before the rename the two halves of the platform spelled this value differently — the CLI wrote `OS_STORAGE_ROOT` while the settings service read `OS_STORAGE_LOCAL_ROOT` — so **any value other than the default was silently discarded** at startup and uploads landed in `./.objectstack/data/uploads` regardless. If you set `OS_STORAGE_ROOT` on an older release, check where your uploads actually are before assuming a backup covered them. |
 | `OS_ARTIFACT_PATH` | path | — | Path or `http(s)://` URL to a compiled `objectstack.json` artifact to boot the kernel from. |
 
 ---

packages/cli/src/commands/dev.ts

@@ -182,8 +182,16 @@ export default class Dev extends Command {
       // ── --fresh: ephemeral OS_HOME under the OS tempdir ─────────────
       // Creates a unique scratch dir that owns ALL persistent state for
       // this run: the SQLite DB (via OS_HOME → <home>/data/...), the
-      // storage-service uploads root (OS_STORAGE_ROOT), and any other
+      // storage-service uploads root (OS_STORAGE_LOCAL_ROOT), and any other
       // state plugins keyed off OS_HOME. Auto-deleted on exit.
+      //
+      // The uploads root MUST be published under the name the settings
+      // service derives for it — `envKeyOf('storage','local_root')` (#4968).
+      // Under the CLI's old private spelling (`OS_STORAGE_ROOT`) the settings
+      // side saw no env value, fell back to the manifest default, and swapped
+      // the adapter to `./.objectstack/data/uploads` at kernel:ready — so
+      // `--fresh` uploads landed in the PROJECT CWD and outlived the run,
+      // which is the opposite of what this block promises.
       let freshHome: string | undefined;
       let freshDbUrl: string | undefined;
       let freshStorageRoot: string | undefined;
@@ -245,7 +253,7 @@ export default class Dev extends Command {
         ...(seedAdmin && flags['admin-email'] ? { OS_SEED_ADMIN_EMAIL: flags['admin-email'] } : {}),
         ...(seedAdmin && flags['admin-password'] ? { OS_SEED_ADMIN_PASSWORD: flags['admin-password'] } : {}),
         ...(freshHome ? { OS_HOME: freshHome } : {}),
-        ...(freshStorageRoot ? { OS_STORAGE_ROOT: freshStorageRoot } : {}),
+        ...(freshStorageRoot ? { OS_STORAGE_LOCAL_ROOT: freshStorageRoot } : {}),
         ...(effectiveDb ? { OS_DATABASE_URL: effectiveDb } : {}),
         ...(flags['database-driver'] ? { OS_DATABASE_DRIVER: flags['database-driver'] } : {}),
         ...(flags['database-auth-token'] ? { OS_DATABASE_AUTH_TOKEN: flags['database-auth-token'] } : {}),

packages/cli/src/commands/serve-storage-capability.test.ts

@@ -25,8 +25,9 @@
  * write it are now told, by `lintUnknownAuthoringKeys`.
  */
 
-import { describe, it, expect } from 'vitest';
-import { resolveStorageCapabilityArg } from './serve.js';
+import { afterEach, describe, it, expect, vi } from 'vitest';
+import { _resetEnvDeprecationWarnings } from '@objectstack/types';
+import { resolveStorageCapabilityArg, resolveStorageLocalRootEnv } from './serve.js';
 
 describe('resolveStorageCapabilityArg', () => {
   it('builds options StorageServicePlugin actually reads', () => {
@@ -45,7 +46,10 @@ describe('resolveStorageCapabilityArg', () => {
     expect(options).not.toHaveProperty('root');
   });
 
-  it('honours OS_STORAGE_ROOT, which the old shape discarded', () => {
+  // Renamed from "honours OS_STORAGE_ROOT" (#4968): this case never read env,
+  // it passes the root as an argument. Naming it after a variable it does not
+  // touch is how the env channel went unexamined while the shape looked pinned.
+  it('honours an explicit root, which the old shape discarded', () => {
     const { options, localRoot } = resolveStorageCapabilityArg('/srv/uploads');
     expect(options).toEqual({ adapter: 'local', local: { rootDir: '/srv/uploads' } });
     expect(localRoot).toBe('/srv/uploads');
@@ -77,3 +81,107 @@ describe('resolveStorageCapabilityArg', () => {
   });
 
 });
+
+/**
+ * #4968 — the env CHANNEL, which #4096 left split.
+ *
+ * #4096 pinned the option shape and the tests above went green, but the value
+ * still could not reach the settings service: the CLI wrote `OS_STORAGE_ROOT`
+ * and the settings service reads `envKeyOf('storage','local_root')` =
+ * `OS_STORAGE_LOCAL_ROOT`, which nothing in the repo ever set. So settings saw
+ * only the manifest's schema default and swapped the adapter at `kernel:ready`
+ * — `OS_STORAGE_ROOT` took effect for exactly one value (the one equal to that
+ * default) and `dev --fresh` wrote uploads into the project cwd.
+ *
+ * The stamp assertion is the load-bearing one. Returning the legacy value is
+ * only half the migration; if the canonical name is not also SET, a deployment
+ * on the old spelling keeps the original bug in full, silently.
+ */
+describe('resolveStorageLocalRootEnv (#4968)', () => {
+  const CANONICAL = 'OS_STORAGE_LOCAL_ROOT';
+  const LEGACY = 'OS_STORAGE_ROOT';
+  const originalCanonical = process.env[CANONICAL];
+  const originalLegacy = process.env[LEGACY];
+
+  afterEach(() => {
+    if (originalCanonical === undefined) delete process.env[CANONICAL];
+    else process.env[CANONICAL] = originalCanonical;
+    if (originalLegacy === undefined) delete process.env[LEGACY];
+    else process.env[LEGACY] = originalLegacy;
+    _resetEnvDeprecationWarnings();
+    vi.restoreAllMocks();
+  });
+
+  it('reads the canonical name the settings service derives, quietly', () => {
+    delete process.env[LEGACY];
+    process.env[CANONICAL] = '/srv/uploads';
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+    expect(resolveStorageLocalRootEnv()).toBe('/srv/uploads');
+    expect(warn).not.toHaveBeenCalled();
+    // The whole point: this is the name `envKeyOf('storage','local_root')`
+    // produces, so the settings service resolves source:'env' at this value.
+    expect(process.env[CANONICAL]).toBe('/srv/uploads');
+  });
+
+  it('still reads the legacy name AND stamps it onto the canonical one', () => {
+    delete process.env[CANONICAL];
+    process.env[LEGACY] = '/srv/legacy-uploads';
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+    expect(resolveStorageLocalRootEnv()).toBe('/srv/legacy-uploads');
+    // Without this line a legacy deployment keeps the exact defect #4968
+    // describes: adapter built at /srv/legacy-uploads, settings still on its
+    // schema default, adapter swapped away at kernel:ready.
+    expect(process.env[CANONICAL]).toBe('/srv/legacy-uploads');
+
+    expect(warn).toHaveBeenCalledTimes(1);
+    const msg = String(warn.mock.calls[0][0]);
+    expect(msg).toContain(LEGACY);
+    expect(msg).toContain(CANONICAL);
+    expect(msg).toContain('deprecated');
+  });
+
+  it('feeds the capability arg from the legacy name end to end', () => {
+    delete process.env[CANONICAL];
+    process.env[LEGACY] = '/srv/legacy-uploads';
+    vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+    const { options, localRoot } = resolveStorageCapabilityArg(resolveStorageLocalRootEnv());
+    expect(options).toEqual({ adapter: 'local', local: { rootDir: '/srv/legacy-uploads' } });
+    expect(localRoot).toBe('/srv/legacy-uploads');
+    // Constructor side and settings side now name the same directory, which is
+    // what makes `needsStorageSwap` answer false instead of swapping + warning.
+    expect(process.env[CANONICAL]).toBe(localRoot);
+  });
+
+  it('lets the canonical name win when both are set, without warning', () => {
+    process.env[CANONICAL] = '/srv/canonical';
+    process.env[LEGACY] = '/srv/legacy';
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+    expect(resolveStorageLocalRootEnv()).toBe('/srv/canonical');
+    expect(process.env[CANONICAL]).toBe('/srv/canonical');
+    // The stamp must never overwrite an explicitly-set canonical value, and the
+    // legacy variable is left exactly as the operator wrote it.
+    expect(process.env[LEGACY]).toBe('/srv/legacy');
+    expect(warn).not.toHaveBeenCalled();
+  });
+
+  it('sets nothing when neither name is set, so the default still applies', () => {
+    delete process.env[CANONICAL];
+    delete process.env[LEGACY];
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+    expect(resolveStorageLocalRootEnv()).toBeUndefined();
+    // Must NOT stamp a default: an env-locked value would show up in Setup as
+    // locked-by-env and take the root out of the admin's hands for no reason.
+    expect(process.env[CANONICAL]).toBeUndefined();
+    expect(warn).not.toHaveBeenCalled();
+
+    // Unset falls through to the resolver default, which equals the manifest
+    // default — this is exactly why plain `pnpm dev` never showed the bug.
+    expect(resolveStorageCapabilityArg(resolveStorageLocalRootEnv()).localRoot)
+      .toBe('.objectstack/data/uploads');
+  });
+});