fix(engine): resolve systems.db exe-anchored, not cwd-relative — spawn-mode DB access

PathBuf::from("data/systems.db") was cwd-relative, and a spawned
server inherits GODOT's cwd — which is client/ (Godot's --path client
chdirs), so CultureResolver, CityContextReader, BodySourceResolver and
BrowseReader all silently failed in every make atlas / make game
spawn. Wave 1 masked it: the star map serves from star_map_data.json,
so 'renders 301 systems' never touched the DB; T-1133's browser was
the first DB-backed consumer to hit it live (Stig, /proc/PID/cwd).

resolve_systems_db_path(): exe-anchored <exe_dir>/../../data/systems.db
first (cwd-independent — the actual fix), then cwd-relative
data/systems.db (cd server && cargo run), then server/data/systems.db
(repo-root invocations); first existing candidate wins, info-logged.
Pure systems_db_candidates() split out as the testable seam (5 unit
tests — incl. pinning Path::parent() of a bare filename = Some(""),
not None). world_root now derives from the resolved ABSOLUTE path so
its ancestors().nth(3) arithmetic holds unconditionally instead of
silently falling back to ".." when canonicalize failed.

Verified all three launch styles; make atlas spawn shows 5 reader fds
open on server/data/systems.db via /proc/PID/fd.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-17 10:37:15 +02:00
co-authored by Claude Fable 5
parent 90c562a6a3
commit 2509624577
+263 -2
View File
@@ -180,7 +180,7 @@ fn main() {
// Initialize culture resolver (#679, D-128).
// systems.db is shipped read-only alongside the binary.
let systems_db_path = std::path::PathBuf::from("data/systems.db");
let systems_db_path = resolve_systems_db_path();
match settled_reach_server::knowledge::CultureResolver::open(&systems_db_path) {
Ok(resolver) => {
tracing::info!("Culture resolver opened: {:?}", systems_db_path);
@@ -198,7 +198,16 @@ fn main() {
// Mod-first body source resolver for the atlas layer proxy (#969, D-225).
// terrain_reference is repo-root-relative; the repo root is systems.db's
// 3rd ancestor (<repo>/server/data/systems.db).
// 3rd ancestor (<repo>/server/data/systems.db). This ancestor arithmetic
// is only correct against an ABSOLUTE path — `.canonicalize()` resolves a
// *relative* path against the CURRENT CWD, so if `systems_db_path` were
// still cwd-relative (as it was before `resolve_systems_db_path()`, T-1131
// follow-up), a wrong-cwd launch (e.g. the D-254 companion spawning from
// the repo root) would make `.canonicalize()` fail outright, falling back
// to the nonsense `".."` default below. `resolve_systems_db_path()`
// already verified `systems_db_path` exists before returning it, so
// `.canonicalize()` here always succeeds and `nth(3)` is genuinely correct
// — not "pretends to work by accident when cwd happens to be server/".
let world_root = systems_db_path
.canonicalize()
.ok()
@@ -484,6 +493,258 @@ fn main() {
tracing::info!("Simulation server shutting down");
}
/// Candidate `systems.db` paths, in try-order, for a given executable path
/// (T-1131 follow-up). Pure/no I/O — the ONLY thing that makes this
/// deterministic and unit-testable given `exe_path` (unlike
/// [`resolve_systems_db_path`], which additionally calls
/// `std::env::current_exe()` and stats the filesystem). Kept separate
/// specifically so the candidate ORDER and SHAPE can be tested without a
/// process-spawning harness — see `tests::` below.
///
/// 1. Exe-anchored `<exe_dir>/../../data/systems.db` (unjoined — the caller
/// canonicalizes and existence-checks; this function never touches disk).
/// Only present if `exe_path` has a parent directory.
/// 2. Cwd-relative `data/systems.db` (today's pre-fix behavior —
/// `cd server && cargo run` leaves cwd at `server/`).
/// 3. Cwd-relative `server/data/systems.db` (repo-root invocations).
fn systems_db_candidates(exe_path: Option<&std::path::Path>) -> Vec<std::path::PathBuf> {
let mut candidates = Vec::with_capacity(3);
if let Some(exe_dir) = exe_path.and_then(std::path::Path::parent) {
candidates.push(exe_dir.join("../../data/systems.db"));
}
candidates.push(std::path::PathBuf::from("data/systems.db"));
candidates.push(std::path::PathBuf::from("server/data/systems.db"));
candidates
}
#[cfg(test)]
mod tests {
use super::*;
/// T-1131 follow-up: the exe-anchored candidate must resolve to
/// `server/data/systems.db` from the DEV BUILD LAYOUT exe path
/// (`server/target/debug/settled-reach-server`) — this is the whole
/// point of the fix, so pin the exact join shape, not just "some path
/// containing systems.db".
#[test]
fn exe_anchored_candidate_targets_server_data_from_dev_build_layout() {
let exe = std::path::Path::new("/repo/server/target/debug/settled-reach-server");
let candidates = systems_db_candidates(Some(exe));
assert_eq!(
candidates.len(),
3,
"exe with a parent dir must produce all three candidates"
);
assert_eq!(
candidates[0],
std::path::PathBuf::from("/repo/server/target/debug/../../data/systems.db"),
"exe-anchored candidate must be unjoined (caller canonicalizes) \
but built from exe_dir/../../data/systems.db"
);
// The whole point: once normalized (what canonicalize() does at
// runtime against a real filesystem), this lands on
// /repo/server/data/systems.db — the actual DB location — not
// /repo/data/systems.db (the pre-fix cwd-relative bug's target).
let normalized = normalize_lexically(&candidates[0]);
assert_eq!(
normalized,
std::path::PathBuf::from("/repo/server/data/systems.db")
);
}
/// The two cwd-relative fallback candidates are present regardless of
/// whether an exe path resolved, in the documented order: `data/systems.db`
/// before `server/data/systems.db` (today's pre-fix behavior stays the
/// first fallback, not silently reordered behind the new repo-root case).
#[test]
fn cwd_relative_candidates_present_and_ordered_when_exe_path_is_some() {
let exe = std::path::Path::new("/repo/server/target/debug/settled-reach-server");
let candidates = systems_db_candidates(Some(exe));
assert_eq!(candidates[1], std::path::PathBuf::from("data/systems.db"));
assert_eq!(
candidates[2],
std::path::PathBuf::from("server/data/systems.db")
);
}
/// `current_exe()` can fail (documented caveat, e.g. sandboxed
/// environments) — `None` must degrade to exactly the two cwd-relative
/// candidates, not panic or produce a malformed exe-anchored entry.
#[test]
fn no_exe_path_yields_only_the_two_cwd_relative_candidates() {
let candidates = systems_db_candidates(None);
assert_eq!(candidates.len(), 2);
assert_eq!(candidates[0], std::path::PathBuf::from("data/systems.db"));
assert_eq!(
candidates[1],
std::path::PathBuf::from("server/data/systems.db")
);
}
/// An exe path that IS genuinely parentless (`Path::parent()` returns
/// `None` only for the empty path or filesystem root — confirmed against
/// the standard library, not assumed) must not panic and must degrade
/// the same as `exe_path: None`.
#[test]
fn genuinely_parentless_exe_path_degrades_like_no_exe_path() {
let exe = std::path::Path::new("");
assert!(
exe.parent().is_none(),
"test premise: Path::new(\"\").parent() must be None"
);
let candidates = systems_db_candidates(Some(exe));
assert_eq!(candidates.len(), 2);
assert_eq!(candidates[0], std::path::PathBuf::from("data/systems.db"));
}
/// A bare relative filename with no directory separator (e.g. the exe
/// path Godot's `OS.create_process` might report on some platform/launch
/// combination) is NOT the parentless case above — `Path::parent()`
/// returns `Some("")` for it (an empty-but-present parent), a real
/// standard-library quirk worth pinning explicitly since it's easy to
/// assume `.parent()` is `None` whenever there's "no directory in the
/// string". The exe-anchored candidate still gets produced (joined onto
/// the empty parent), just degenerately — `../../data/systems.db`
/// relative to cwd, which is harmless: it'll fail existence-checks
/// exactly like any other wrong candidate and fall through the loop.
#[test]
fn bare_filename_exe_path_has_an_empty_but_present_parent() {
let exe = std::path::Path::new("settled-reach-server");
assert_eq!(
exe.parent(),
Some(std::path::Path::new("")),
"Path::parent() of a bare filename is Some(\"\"), not None — \
pinning this stdlib behavior since it's the reason a bare \
filename still produces 3 candidates, not 2"
);
let candidates = systems_db_candidates(Some(exe));
assert_eq!(
candidates.len(),
3,
"a present-but-empty parent still yields an exe-anchored candidate"
);
assert_eq!(
candidates[0],
std::path::PathBuf::from("../../data/systems.db"),
"joined onto an empty parent, the exe-anchored candidate is bare \
../../data/systems.db (cwd-relative in practice, but still a \
DISTINCT candidate from candidates[1]'s exact data/systems.db)"
);
}
/// Lexical `..`/`.` normalization for test assertions ONLY — a stand-in
/// for `Path::canonicalize()` (which needs a real filesystem + cwd,
/// which unit tests must not depend on per the coordinator's "don't
/// build a process-spawning/filesystem harness for this" guidance).
/// `resolve_systems_db_path` itself still uses the real
/// `canonicalize()` at runtime — this helper exists only so
/// `exe_anchored_candidate_targets_server_data_from_dev_build_layout`
/// can assert the join shape actually lands on the right final path
/// without touching disk.
fn normalize_lexically(path: &std::path::Path) -> std::path::PathBuf {
let mut out = std::path::PathBuf::new();
for component in path.components() {
match component {
std::path::Component::ParentDir => {
out.pop();
}
std::path::Component::CurDir => {}
other => out.push(other.as_os_str()),
}
}
out
}
}
/// Resolve `systems.db`'s path, ANCHORED TO THE EXECUTABLE rather than the
/// current working directory (T-1131 follow-up).
///
/// **The bug this fixes:** `PathBuf::from("data/systems.db")` is cwd-relative.
/// `make game` (`cd server && cargo run`) happens to leave cwd at `server/`,
/// so that path resolves — but the D-254 companion app spawns this binary via
/// Godot's `OS.create_process`/`OS.execute_with_pipe` (`server_process.gd`),
/// neither of which sets a working directory: the child inherits GODOT's cwd,
/// which for `make atlas`/`make game` is the REPO ROOT (the Makefile has no
/// `cd` before launching Godot itself — only before `cargo run`). From the
/// repo root, `data/systems.db` doesn't exist (it's `server/data/systems.db`),
/// so every DB-backed reader (`CultureResolver`, `CityContextReader`, and now
/// `BrowseReader`) silently fails to open in every spawned-server context.
/// This went unnoticed through T-1130's wave 1 because the star map is served
/// from `star_map_data.json` via `world_root` (itself derived from
/// `systems_db_path`, so ALSO broken — but `.exists()`-checked with a `warn`,
/// not a hard dependency any single-connection smoke test would surface) —
/// "renders 301 systems" never actually touched `systems.db`.
///
/// **The fix:** resolve relative to `std::env::current_exe()` first — in the
/// dev build layout the binary is `server/target/debug/settled-reach-server`,
/// so `exe_dir/../../data/systems.db` is `server/data/systems.db` regardless
/// of cwd. Falls through to the two cwd-relative candidates (today's
/// behavior, and the repo-root equivalent) so `cd server && cargo run` and a
/// repo-root-relative invocation both keep working without needing the
/// exe-anchoring to succeed (e.g. `current_exe()` can fail in exotic
/// sandboxed environments per its own documented caveats).
///
/// Candidate order/shape lives in [`systems_db_candidates`] (pure,
/// unit-tested); this function adds the I/O layer: canonicalize + existence
/// check per candidate, first EXISTING one wins, with an `info` log
/// recording which candidate resolved (so a future "browse reader
/// unavailable" report is diagnosable from the startup log alone).
///
/// If none exist, returns the cwd-relative `data/systems.db` default —
/// today's pre-fix behavior — so every downstream `Reader::open()` call
/// still gets a path to fail on and log its own existing
/// `warn`-and-degrade message. This function does not invent a new failure
/// mode, it just tries harder before giving up.
fn resolve_systems_db_path() -> std::path::PathBuf {
let exe_path = std::env::current_exe().ok();
let candidates = systems_db_candidates(exe_path.as_deref());
for candidate in &candidates {
let canonical = candidate.canonicalize();
if let Ok(ref resolved) = canonical {
if resolved.exists() {
tracing::info!(
"systems.db resolved: {:?} (candidate: {:?}, exe: {:?})",
resolved,
candidate,
exe_path
);
return resolved.clone();
}
} else if candidate.exists() {
// canonicalize() can fail even when the path exists (e.g. a
// component permission error) — exists() is the true signal;
// canonicalize() is just how we get an absolute path for
// world_root's ancestor arithmetic to work correctly.
tracing::info!(
"systems.db resolved (uncanonicalized): {:?} (exe: {:?})",
candidate,
exe_path
);
return candidate.clone();
}
}
// None of the candidates exist. Fall back to the cwd-relative
// `data/systems.db` default — today's pre-fix behavior — NOT the
// exe-anchored candidate (which, per systems_db_candidates' doc, is
// unjoined/uncanonicalized and only meaningful once verified to exist;
// returning it here unverified would be a worse default than the plain
// relative path every downstream Reader::open() already knows how to
// fail on cleanly).
let fallback = std::path::PathBuf::from("data/systems.db");
tracing::warn!(
"systems.db not found via any of {:?} (exe: {:?}) — falling back to {:?} \
(every DB-backed reader will report unavailable and degrade)",
candidates,
exe_path,
fallback
);
fallback
}
/// Best-effort: send a final SimError snapshot to the client on panic (#85).
///
/// Builds a minimal ObserverSnapshot with the panic error and sends it