fix(compile): persist default Deno.openKv() path in compiled binaries

[bartlomieju]

Fixes #24318.

In a deno compile-d binary, Deno.openKv() called without an explicit
path silently fell back to an in-memory database, so data never persisted
across runs. The same mechanism also prevented localStorage from
persisting, and passing --location at compile time did not help.

The root cause was in the standalone runtime (cli/rt/run.rs), which
constructed the worker factory with StorageKeyResolver::empty() and
origin_data_folder_path: None. With no storage key, the worker resolves
origin_storage_dir to None, which is handed to the KV SQLite handler as
its default storage dir. In ext/kv/sqlite.rs the "no path, no default
dir" case opens an in-memory connection, so writes never reach disk. This
is unlike deno run, which builds a real storage key (location, then
deno.json URL, then the main module) and points the data folder at the
DENO_DIR's location_data directory.

This change makes the standalone runtime resolve both pieces. It derives the
storage key the same way deno run does (prefer an explicit --location,
otherwise fall back to the main module, which is stable for a given compiled
binary) and reuses the DENO_DIR's location_data folder so default KV and
localStorage persist to disk. The two are tied together: the storage key
is only set when a data folder can be resolved, so if the deno dir is
unavailable both stay empty and the previous in-memory behavior is kept
(this also avoids unwrapping a missing data folder path in the worker).

A spec test under tests/specs/compile/kv_default_path compiles a script
that uses Deno.openKv() without a path, runs the binary once to write a
value and again to read it back, asserting the value survives across
processes.

[lunadogbot]

Overall

LGTM with two small things worth addressing. This is a clean, well-scoped bug fix for #24318. The root-cause analysis is correct, and the fix mirrors deno run's storage-key resolution faithfully (verified against cli/args/mod.rs:873-881). Small surface area (+66/-2), clean regression test, and the in-memory fallback is preserved as a safety net.

What I checked

• Main-module URL stability across runs. In cli/rt/binary.rs:69-83, root_path for a non-self-extracting binary is temp_dir/deno-compile-<exe-name>, and for self-extracting it's a hash-derived extraction dir. main_module = root_dir_url.join(metadata.entrypoint_key) (run.rs:946). Both inputs are stable for a given binary, so StorageKeyResolver::new_use_main_module() produces a stable key across runs. ✅
• Mirrors deno run. Same priority order (--location → main module). deno run has an extra middle tier for deno.json, which is correctly omitted here because compiled binaries don't have one.
• Coupling. Gating the storage key on origin_data_folder_path.is_some() is the right call — it avoids the unwrap that LibMainWorker does on the data folder path when a key is present.

Suggestions

1. Cross-binary collision worth a comment or follow-up. For non-self-extracting builds, root_path is derived from just the exe filename (temp_dir/deno-compile-<name> at binary.rs:82). Two unrelated compiled binaries both named e.g. out or app will resolve to the same storage key and share KV/localStorage. This isn't introduced by this PR — it's a property of the existing root_path derivation — but it becomes user-visible now that data actually persists. Worth at least a // NOTE: near the storage_key block, or filing a follow-up. Could mitigate later by mixing metadata.entrypoint_key content hash (or the self-extracting hash when present) into the key.

2. Test should also cover localStorage. The PR body explicitly says "the same mechanism also prevented localStorage from persisting" — but only KV is covered by tests/specs/compile/kv_default_path. Adding a parallel localStorage_default_path spec (or a single spec exercising both) would lock in the second half of the fix and prevent a future regression in just one path. Cheap to add — same shape as the KV test.

Nits

• cli/rt/run.rs:1234-1243 — DenoDirProvider::new(...) is constructed fresh here. Confirmed nothing in run() already holds one, so this is fine, but if DenoDirOptions { maybe_initial_cwd: None, maybe_custom_root: None } ever needs DENO_DIR env handling tweaks (e.g. --node-modules-dir interactions), this is the spot. Not blocking.
• The two new comment blocks (lines ~1227-1232 and ~1245-1248) are good — they explain the why and the coupling, which is exactly what's non-obvious here. Keep them.

Test plan I'd want to see

• ./out set then ./out get across two cold processes ✅ (covered)
• Same flow with localStorage ❌ (missing)
• Compile with --location https://example.com and verify that key takes precedence ❌ (nice-to-have)

[bartlomieju]

Thanks for the thorough review. Addressed in 57b8814:

1. Cross-binary collision — added a `NOTE:` near the storage-key block documenting that a non-self-extracting binary derives its key from just the executable file name, so two binaries sharing a name share the same default KV database, with a pointer to mixing the entrypoint content hash in as a future fix.

2. localStorage coverage — out of scope here. localStorage in compiled binaries is not actually wired up yet (a compiled binary today throws NotSupported: LocalStorage is not supported in this context.); that is handled by feat(compile): support WebCache API/LocalStorage API #33775. This PR is KV-only, so I kept the test KV-only and did not claim localStorage persistence in the comment. I will add the localStorage spec alongside feat(compile): support WebCache API/LocalStorage API #33775.

The --location precedence check is a nice-to-have; happy to add it if you want it in this PR, otherwise I will keep the scope tight.

[bartlomieju]

Added the --location precedence test in 3f71ba4: tests/specs/compile/kv_location_storage_key. It compiles two differently-named binaries (out_a, out_b) with the same --location https://example.com, sets a value via one and reads it back via the other. They share the default Deno.openKv() database, which proves the location-derived storage key takes precedence over the main-module key (without --location, the differing exe names yield different main-module keys and the stores would not be shared).

[lunadogbot]

Took a look at #33775 to check whether it covers the localStorage gap here — partially, but with a couple of wrinkles worth surfacing before treating it as covered:

#33775 doesn't test cross-run persistence

Its storage_apis/main.ts is just:

localStorage.setItem("a", "A");
console.log(localStorage.getItem("a"));

One process. That proves localStorage is callable in a compiled binary, but not that values survive to a second run. The bug shape this PR is fixing — "set in one process, read in the next" — isn't covered there.

The two PRs conflict on cli/rt/run.rs

Both touch the same storage_key_resolver / origin_data_folder_path construction and make different choices:

	This PR (#34576)	#33775
Folder	location_data (matches deno run)	New origin_data (compile-only)
Gating	Set if DENO_DIR resolves	Set only if --location is passed
Default Deno.openKv()	Persists (falls back to main module key)	Stays in-memory

Whichever lands second has to reconcile. If #33775 lands as-written after this one, default Deno.openKv() without --location regresses back to in-memory.

Why a localStorage spec here is still worth it

A cross-run localStorage spec in this PR (same shape as kv_default_path — set then exit, then get) would:

1. Lock down the second half of the fix described in the PR body while feat(compile): support WebCache API/LocalStorage API #33775 is still iterating.
2. Force the reconciliation to be explicit when feat(compile): support WebCache API/LocalStorage API #33775 rebases — the test would fail under feat(compile): support WebCache API/LocalStorage API #33775's --location-only gating, surfacing the regression before merge.

Cheap to add and the coverage isn't redundant with #33775. Not blocking, but I'd lean toward including it.

[bartlomieju]

Superseded by #34618, which takes a broader approach: instead of routing default Deno.openKv() through DENO_DIR, a compiled binary is treated as a standalone app and its storage (KV, localStorage, and caches) is persisted under the platform's app data directory, keyed by a new --app-name flag (defaulting to the output file name). Closing in favor of that PR.