diff --git a/CHANGELOG.md b/CHANGELOG.md index 9276945..09f51ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Unreleased +## Bug fixes +- Config: a child config layer (e.g. a project `.tuor/config.json`) that didn't + set `workdir` or `user` silently reset the value inherited from an upper layer + layer (e.g. `~/.config/tuor/config.json`) to the built-in default values + (workdir: `/`, user: `root`). These fields now fall through to the parent + layer when omitted; their defaults are applied only after all layers are + merged. + # 0.3.0 (2026-07-08) diff --git a/src/config/defaults.test.ts b/src/config/defaults.test.ts index e09c01a..887f90c 100644 --- a/src/config/defaults.test.ts +++ b/src/config/defaults.test.ts @@ -2,11 +2,37 @@ import { describe, expect, test } from "vitest"; import { applyConfigDefaults } from "./defaults.ts"; import type { TuorConfig } from "./schema.ts"; +/** + * A merged config as it reaches applyConfigDefaults: user/workdir are optional + * (no schema default) and only present when a layer set them explicitly. + */ function config(overrides: Partial = {}): TuorConfig { - return { user: "root", workdir: "/", ...overrides }; + return { ...overrides }; } describe("applyConfigDefaults", () => { + describe("user / workdir", () => { + test("defaults user to root when omitted", () => { + expect(applyConfigDefaults(config()).user).toBe("root"); + }); + + test("defaults workdir to / when omitted", () => { + expect(applyConfigDefaults(config()).workdir).toBe("/"); + }); + + test("preserves an explicit user and workdir", () => { + const result = applyConfigDefaults( + config({ user: "dev", workdir: "/w" }), + ); + expect(result.user).toBe("dev"); + expect(result.workdir).toBe("/w"); + }); + + test("infers guestHomeDir from the defaulted user when user is omitted", () => { + expect(applyConfigDefaults(config()).guestHomeDir).toBe("/root"); + }); + }); + describe("network", () => { test("defaults omitted network to restricted with empty allowlists", () => { const result = applyConfigDefaults(config()); diff --git a/src/config/defaults.ts b/src/config/defaults.ts index ce5fc14..0217ca4 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -1,9 +1,14 @@ import type { NetworkSpec } from "../core/session.ts"; import { inferGuestHomeDir } from "./homedir.ts"; -import type { TuorConfig } from "./schema.ts"; +import type { TuorConfig, WorkdirConfig } from "./schema.ts"; // --- Types --- +/** Guest user assumed when no config layer sets one. */ +export const DEFAULT_USER = "root"; +/** Guest working directory assumed when no config layer sets one. */ +export const DEFAULT_WORKDIR: WorkdirConfig = "/"; + /** * A {@link TuorConfig} with all *config-level* defaults materialized. This is * the "effective config" a user reasons about: same shape as `config.json` @@ -15,27 +20,41 @@ import type { TuorConfig } from "./schema.ts"; * guarantees the default was applied; its JSON shape is identical to a * fully-populated `NetworkConfig`. */ -export type DefaultedConfig = Omit & { +export type DefaultedConfig = Omit< + TuorConfig, + "network" | "guestHomeDir" | "user" | "workdir" +> & { network: NetworkSpec; guestHomeDir: string; + user: string; + workdir: WorkdirConfig; }; // --- Public API --- /** * Fill the config-level defaults that don't come from the arktype schema: - * `network` (block-all when omitted) and `guestHomeDir` (inferred from `user`). + * `user`, `workdir`, `network` (block-all when omitted) and `guestHomeDir` + * (inferred from `user`). + * + * `user`/`workdir` are defaulted *here* rather than in the schema so that a + * child config layer that omits them doesn't clobber a value inherited from a + * parent layer during merge (their "omitted" would otherwise be indistinguish- + * able from an explicit default). This must run after `mergeConfigs`. * - * Pure and dependency-free. Schema defaults (`user`, `workdir`, mount `mode`, + * Pure and dependency-free. The remaining schema defaults (mount `mode`, * `nixLd`) are already applied by `parseConfig`; computed conversions (path * expansion, env/secret split, nix→mounts, overlay state dirs, …) are *not* * defaults and stay in {@link createSessionSpecFromConfig}. */ export function applyConfigDefaults(config: TuorConfig): DefaultedConfig { + const user = config.user ?? DEFAULT_USER; return { ...config, + user, + workdir: config.workdir ?? DEFAULT_WORKDIR, network: defaultNetwork(config.network), - guestHomeDir: config.guestHomeDir ?? inferGuestHomeDir(config.user), + guestHomeDir: config.guestHomeDir ?? inferGuestHomeDir(user), }; } diff --git a/src/config/merge.test.ts b/src/config/merge.test.ts index 4366281..4ab617c 100644 --- a/src/config/merge.test.ts +++ b/src/config/merge.test.ts @@ -2,9 +2,13 @@ import { describe, expect, test } from "vitest"; import { type ConfigLayer, findAllConfigDirs, mergeConfigs } from "./merge.ts"; import type { MountConfig, TuorConfig } from "./schema.ts"; -/** Minimal valid config (matching what parseConfig returns with defaults filled). */ +/** + * Minimal valid config, matching what parseConfig returns: user/workdir carry + * no schema default, so an unset layer genuinely omits them (they're defaulted + * post-merge in applyConfigDefaults). + */ function config(overrides: Partial = {}): TuorConfig { - return { user: "root", workdir: "/", ...overrides }; + return { ...overrides }; } function layer( @@ -104,6 +108,27 @@ describe("mergeConfigs", () => { expect(result.workdir).toBe("/child-dir"); }); + // Regression: a child layer that omits user/workdir must inherit the + // parent's value rather than clobbering it. Previously these fields carried + // a schema default ("root"/"/"), so a child that never set them still + // overrode the parent — e.g. a project .tuor/config.json silently reset a + // workdir configured in ~/.config/tuor/config.json. + test("user: parent used when child omits", () => { + const result = mergeConfigs([ + layer("/a", { user: "parent" }), + layer("/b"), + ]); + expect(result.user).toBe("parent"); + }); + + test("workdir: parent used when child omits", () => { + const result = mergeConfigs([ + layer("/a", { workdir: "/parent-dir" }), + layer("/b"), + ]); + expect(result.workdir).toBe("/parent-dir"); + }); + test("rootfsSize: child overrides parent", () => { const result = mergeConfigs([ layer("/a", { rootfsSize: "1G" }), @@ -409,8 +434,9 @@ describe("mergeConfigs", () => { }), ]); - // Scalars: most specific child wins (child has default "root" from helper) - expect(result.user).toBe("root"); + // Scalars: most specific layer that *sets* the field wins. The closest + // layer omits user, so it falls through to the middle layer. + expect(result.user).toBe("project-user"); // Env: shallow merge expect(result.env).toEqual({ diff --git a/src/config/merge.ts b/src/config/merge.ts index 0419dcf..280c456 100644 --- a/src/config/merge.ts +++ b/src/config/merge.ts @@ -86,9 +86,13 @@ export function mergeConfigs(layers: ConfigLayer[]): TuorConfig { /** Merge two configs where `child` overrides `parent`. */ function mergeTwoConfigs(parent: TuorConfig, child: TuorConfig): TuorConfig { return { - // Scalars: child wins (fall back to parent for defaults) - user: child.user, - workdir: child.workdir, + // Scalars: child wins, falling back to parent when the child omits the + // field. user/workdir carry no schema default anymore, so "omitted" is + // genuinely undefined here — otherwise a child layer's silently-defaulted + // value would clobber a value inherited from a parent layer. Their defaults + // are applied post-merge in applyConfigDefaults. + ...lastDefined(child.user, parent.user, "user"), + ...lastDefined(child.workdir, parent.workdir, "workdir"), ...lastDefined(child.guestHomeDir, parent.guestHomeDir, "guestHomeDir"), ...lastDefined(child.rootfsSize, parent.rootfsSize, "rootfsSize"), ...lastDefined(child.nix, parent.nix, "nix"), diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index ad08e98..d5acb02 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -53,10 +53,12 @@ describe("parseConfig", () => { }); }); - test("fills in defaults for minimal config", () => { + test("leaves optional fields unset for minimal config", () => { + // user/workdir carry no schema default (they're defaulted post-merge in + // applyConfigDefaults), so parseConfig leaves them undefined here. const config = parseConfig({}); - expect(config.user).toBe("root"); - expect(config.workdir).toBe("/"); + expect(config.user).toBeUndefined(); + expect(config.workdir).toBeUndefined(); expect(config.network).toBeUndefined(); expect(config.mounts).toBeUndefined(); expect(config.nix).toBeUndefined(); diff --git a/src/config/schema.ts b/src/config/schema.ts index 4983fdb..e862c1a 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -51,8 +51,12 @@ const types = scope({ * The user to open the shell under and to make mounted directories * available for. Must currently be root due to Gondolin-related * constraints. + * + * Optional (no schema default): the "root" default is applied post-merge in + * applyConfigDefaults, so an inherited value isn't clobbered by a child + * layer that merely omitted the field. */ - user: "string > 0 = 'root'", + "user?": "string > 0", /** * Volumes are host-backed, initially empty directories that the VM guest @@ -61,9 +65,13 @@ const types = scope({ "volumes?": "VolumeConfig[]", /** - * Configure the default working directory for the VM guest + * Configure the default working directory for the VM guest. + * + * Optional (no schema default): the "/" default is applied post-merge in + * applyConfigDefaults, so an inherited value isn't clobbered by a child + * layer that merely omitted the field. */ - workdir: "WorkdirConfig = '/'", + "workdir?": "WorkdirConfig", }, // --------------------------------------------------------------------------