From 4978787d0de2f0c4014c95a0cbdf99c957ba8792 Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 4 Nov 2026 11:58:03 +0000 Subject: [PATCH 1/9] feat(config): discover config.jsonc and parse JSON configs as JSONC (CLI-2860) The loader now discovers supabase/config.jsonc and parses both config.json and config.jsonc with jsonc-parser, so comments and trailing commas work and parse errors report a line and column. config.json and config.jsonc together fail with CliConfigFileConflictError. A JSON file next to config.toml still wins, and the CLI now warns that config.toml is ignored. Saves go back to config.jsonc when that is the project's JSON file. The CLI's two hand-rolled comment strippers (VS Code settings in init, deno import maps in functions deploy) use the same parser, as do the experimental feature reader, the upgrade-notice project walk-up and the compute guards. --- .../resolved-config-context.ts | 3 +- .../src/command-internal/upgrade-notice.ts | 13 +- .../config/push/push.integration.test.ts | 4 +- .../experimental/compute/compute.shared.ts | 5 +- .../compute/new/new.integration.test.ts | 28 +++ .../stack/stack-backend.integration.test.ts | 15 ++ .../deploy/deploy.integration.test.ts | 50 +++- .../commands/init/init.integration.test.ts | 32 +++ .../commands/start/start.integration.test.ts | 24 ++ .../cli-config-values.integration.test.ts | 110 ++++++++ .../cli/src/config/cli-config-values.layer.ts | 9 + apps/cli/src/shared/compute/compute-paths.ts | 2 +- .../shared/compute/compute-paths.unit.test.ts | 1 + .../src/shared/config/experimental-feature.ts | 10 +- apps/cli/src/shared/functions/deploy.ts | 86 +------ apps/cli/src/shared/init/project-init.ts | 102 +------- .../output/normalize-error.unit.test.ts | 14 ++ .../telemetry/__fixtures__/error-tags.txt | 1 + .../shared/telemetry/error-actionability.ts | 1 + packages/config/AGENTS.md | 3 +- packages/config/docs/cli-config-loading.md | 42 ++-- packages/config/package.json | 1 + packages/config/src/cli-config.service.ts | 2 + packages/config/src/config-format.ts | 2 +- .../config/src/entrypoint-purity.unit.test.ts | 5 + packages/config/src/errors.ts | 10 + packages/config/src/index.ts | 1 + packages/config/src/internal.ts | 2 + packages/config/src/io.ts | 80 +++--- packages/config/src/io.unit.test.ts | 237 +++++++++++++++++- packages/config/src/jsonc.ts | 92 +++++++ packages/config/src/jsonc.unit.test.ts | 88 +++++++ packages/config/src/paths.ts | 6 +- packages/config/src/promise-facade.ts | 6 +- pnpm-lock.yaml | 8 + 35 files changed, 843 insertions(+), 252 deletions(-) create mode 100644 packages/config/src/jsonc.ts create mode 100644 packages/config/src/jsonc.unit.test.ts diff --git a/apps/cli/src/command-internal/resolved-config-context.ts b/apps/cli/src/command-internal/resolved-config-context.ts index dfd48e1982..1973ff4d66 100644 --- a/apps/cli/src/command-internal/resolved-config-context.ts +++ b/apps/cli/src/command-internal/resolved-config-context.ts @@ -79,7 +79,8 @@ export const describeConfigLoadFailure = (cause: unknown): string => cause._tag === "CliConfigFlagConflictError" || cause._tag === "CliConfigLoadError" || cause._tag === "CliConfigRemovedKeyError" || - cause._tag === "CliConfigRemovedEnvError") && + cause._tag === "CliConfigRemovedEnvError" || + cause._tag === "CliConfigFileConflictError") && "message" in cause && typeof cause.message === "string" ? cause.message diff --git a/apps/cli/src/command-internal/upgrade-notice.ts b/apps/cli/src/command-internal/upgrade-notice.ts index 9ec18c5941..588cfcdc25 100644 --- a/apps/cli/src/command-internal/upgrade-notice.ts +++ b/apps/cli/src/command-internal/upgrade-notice.ts @@ -9,6 +9,7 @@ import { stripVTControlCharacters } from "node:util"; +import { findCliProjectRoot } from "@supabase/config/effect"; import { Clock, Config, @@ -197,7 +198,6 @@ const resolveNoticeBaseDir = Effect.fnUntraced(function* ( env: Readonly>, isValueTakingFlagToken?: (token: string) => boolean, ) { - const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; // A set flag beats the env even when empty; an empty effective value falls // through to the ancestor walk. @@ -206,16 +206,7 @@ const resolveNoticeBaseDir = Effect.fnUntraced(function* ( if (explicit !== undefined && explicit !== "") { return path.resolve(cwd, explicit); } - let current = cwd; - while (true) { - for (const configFile of ["config.json", "config.toml"]) { - const configPath = path.join(current, "supabase", configFile); - if (yield* fs.exists(configPath).pipe(Effect.orElseSucceed(() => false))) return current; - } - const parent = path.dirname(current); - if (parent === current) return cwd; - current = parent; - } + return (yield* findCliProjectRoot(cwd)) ?? cwd; }); /** diff --git a/apps/cli/src/commands/config/push/push.integration.test.ts b/apps/cli/src/commands/config/push/push.integration.test.ts index 39b5a98117..ffad01034c 100644 --- a/apps/cli/src/commands/config/push/push.integration.test.ts +++ b/apps/cli/src/commands/config/push/push.integration.test.ts @@ -539,7 +539,9 @@ describe("config push integration", () => { const message = yield* configPush({ remote: Option.none(), projectRef: Option.none() }).pipe( Effect.catchTag("ConfigPushLoadConfigError", (error) => Effect.succeed(error.message)), ); - expect(message).toBe("failed to parse supabase/config.json: JSON Parse error: Expected '}'"); + expect(message).toBe( + "failed to parse supabase/config.json: Invalid symbol (line 1, column 2)", + ); expect(api.requests).toHaveLength(0); }).pipe(Effect.provide(layer)); }); diff --git a/apps/cli/src/commands/experimental/compute/compute.shared.ts b/apps/cli/src/commands/experimental/compute/compute.shared.ts index d179d39a8a..a9a4cbc70e 100644 --- a/apps/cli/src/commands/experimental/compute/compute.shared.ts +++ b/apps/cli/src/commands/experimental/compute/compute.shared.ts @@ -72,13 +72,12 @@ export const loadComputeProject = Effect.gen(function* () { * As {@link loadComputeProject}, for the `[compute.]` entry writer. * * `commitComputeEntry` is a TOML text editor, so a JSON project is rejected: appending a - * `[compute.]` table to `config.json` would make the file unparseable. + * `[compute.]` table to `config.json`/`config.jsonc` would make the file unparseable. */ export const loadComputeProjectForEntryWrite = Effect.fnUntraced(function* () { const settings = yield* CommandSettings; - const path = yield* Path.Path; const paths = yield* findCliProjectPaths(settings.workdir, { search: false }); - if (paths !== null && path.basename(paths.configPath) === "config.json") { + if (paths !== null && !paths.configPath.endsWith(".toml")) { return yield* new ComputeJsonConfigUnsupportedError({ detail: `This project is configured by ${paths.configPath}, which the compute new command cannot edit safely.`, suggestion: `Create the source files manually and add the compute entry to ${paths.configPath}, or convert the whole project configuration to TOML before scaffolding it.`, diff --git a/apps/cli/src/commands/experimental/compute/new/new.integration.test.ts b/apps/cli/src/commands/experimental/compute/new/new.integration.test.ts index ee8ae0130c..9db57186d4 100644 --- a/apps/cli/src/commands/experimental/compute/new/new.integration.test.ts +++ b/apps/cli/src/commands/experimental/compute/new/new.integration.test.ts @@ -943,6 +943,34 @@ describe("compute new", () => { }).pipe(Effect.scoped, Effect.provide(BunServices.layer)), ); + it.effect.each([false, true])( + "refuses config.jsonc before prompts or writes (TOML present: %s)", + (withToml) => + Effect.gen(function* () { + const path = yield* Path.Path; + const fs = yield* FileSystem.FileSystem; + const configJsonc = '// keep me\n{"project_id":"demo",}\n'; + const repo = yield* makeComputeProject({ + "supabase/config.jsonc": configJsonc, + ...(withToml ? { "supabase/config.toml": CONFIG_WITH_COMMENTS } : {}), + }); + const { layer, out } = setupCompute({ workdir: repo.dir }); + return yield* Effect.gen(function* () { + const error = yield* computeNew(flags({ name: Option.none() })).pipe(Effect.flip); + expect(error).toBeInstanceOf(ComputeJsonConfigUnsupportedError); + expect(error).toMatchObject({ + detail: expect.stringContaining("supabase/config.jsonc"), + }); + expect(out.promptTextCalls).toEqual([]); + expect(out.promptSelectCalls).toEqual([]); + expect(yield* fs.readFileString(path.join(repo.dir, "supabase", "config.jsonc"))).toBe( + configJsonc, + ); + expect(yield* fs.exists(path.join(repo.dir, "supabase", "compute"))).toBe(false); + }).pipe(Effect.provide(layer)); + }).pipe(Effect.scoped, Effect.provide(BunServices.layer)), + ); + it.effect("refuses an unconfigured JSON-only ancestor before asking for a name", () => Effect.gen(function* () { const path = yield* Path.Path; diff --git a/apps/cli/src/commands/experimental/stack/stack-backend.integration.test.ts b/apps/cli/src/commands/experimental/stack/stack-backend.integration.test.ts index 3a11961858..fa56c2fc9b 100644 --- a/apps/cli/src/commands/experimental/stack/stack-backend.integration.test.ts +++ b/apps/cli/src/commands/experimental/stack/stack-backend.integration.test.ts @@ -193,6 +193,21 @@ stack = true ); }); + it.effect("reads experimental.stack from a commented config.jsonc", () => { + return withServices( + Effect.gen(function* () { + const root = yield* project("[experimental]\nstack = false\n"); + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + yield* fs.writeFileString( + path.join(root, "supabase", "config.jsonc"), + '{\n // opt in\n "experimental": { "stack": true, },\n}\n', + ); + expect(yield* resolve({ args: ["start"], cwd: root, env: {} })).toBe("stack"); + }), + ); + }); + it.effect("rejects invalid overrides and bypasses malformed config", () => { return withServices( Effect.gen(function* () { diff --git a/apps/cli/src/commands/functions/deploy/deploy.integration.test.ts b/apps/cli/src/commands/functions/deploy/deploy.integration.test.ts index d10cc61d50..852655f618 100644 --- a/apps/cli/src/commands/functions/deploy/deploy.integration.test.ts +++ b/apps/cli/src/commands/functions/deploy/deploy.integration.test.ts @@ -851,7 +851,7 @@ describe("functions deploy", () => { }).pipe(Effect.provide(layer), Effect.ensuring(removeTempRoot)); }); - it.effect("reports a malformed deno.json with the schema decoder's message", () => { + it.effect("reports a malformed deno.json with the parse reason and position", () => { const { api, layer } = apiDeployHarness(deployedHelloWorld); return Effect.gen(function* () { @@ -867,7 +867,10 @@ describe("functions deploy", () => { const error = yield* functionsDeploy(baseFlags).pipe(Effect.flip); expect(error).toBeInstanceOf(FunctionImportMapSyntaxError); - expect(error).toMatchObject({ name: "SyntaxError", message: "Expected a valid JSON string" }); + expect(error).toMatchObject({ + name: "SyntaxError", + message: "Close brace expected (line 1, column 2)", + }); expect(api.requests.some((request) => request.method === "POST")).toBe(false); }).pipe(Effect.provide(layer), Effect.ensuring(removeTempRoot)); }); @@ -996,6 +999,49 @@ describe("functions deploy", () => { }, ); + it.effect("uploads the files a commented deno.jsonc import map points at", () => { + const uploaded: Array<{ importMapPath: string; fileNames: Array }> = []; + const { layer } = apiDeployHarness((request, recorded) => { + if (Predicate.isTagged(request.body, "FormData")) { + const metadata = request.body.formData.get("metadata"); + if (typeof metadata === "string") { + uploaded.push({ + importMapPath: JSON.parse(metadata).import_map_path, + fileNames: request.body.formData + .getAll("file") + .flatMap((part) => (part instanceof File ? [part.name] : [])), + }); + } + } + return deployedHelloWorld(request, recorded); + }); + + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + yield* writeCliConfig(tempRoot.current); + yield* writeLocalFunction(tempRoot.current, "hello-world"); + yield* fs.remove(`${functionDir("hello-world")}/deno.json`); + yield* fs.makeDirectory(`${tempRoot.current}/supabase/shared`, { recursive: true }); + yield* fs.writeFileString(`${tempRoot.current}/supabase/shared/helper.ts`, "export {}\n"); + yield* fs.writeFileString( + `${functionDir("hello-world")}/deno.jsonc`, + `{ + // shared helper + "imports": { + "helper": "../../shared/helper.ts", /* trailing comma follows */ + }, +} +`, + ); + + yield* functionsDeploy(baseFlags); + + expect(uploaded).toHaveLength(1); + expect(uploaded[0]?.importMapPath).toBe("supabase/functions/hello-world/deno.jsonc"); + expect(uploaded[0]?.fileNames).toContain("supabase/shared/helper.ts"); + }).pipe(Effect.provide(layer), Effect.ensuring(removeTempRoot)); + }); + it.effect("deploys with deno.json when the function directory has both files", () => { const { importMapPaths, layer } = harnessRecordingImportMapPaths(); diff --git a/apps/cli/src/commands/init/init.integration.test.ts b/apps/cli/src/commands/init/init.integration.test.ts index 8a47a91fbe..55fb7be1ba 100644 --- a/apps/cli/src/commands/init/init.integration.test.ts +++ b/apps/cli/src/commands/init/init.integration.test.ts @@ -391,6 +391,38 @@ describe("init", () => { }).pipe(Effect.provide(BunServices.layer)); }); + it.effect.each([ + ["malformed JSON", `{ "a": }`], + ["a non-object document", "[]"], + ] as const)( + "fails with InitParseSettingsError for %s and leaves settings.json unchanged", + ([, contents]) => { + const tempDir = tempRoot.current; + + return Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const settingsPath = path.join(tempDir, ".vscode", "settings.json"); + yield* fs.makeDirectory(path.dirname(settingsPath), { recursive: true }); + yield* fs.writeFileString(settingsPath, contents); + const { layer } = setup(tempDir); + + const exit = yield* init({ + ...BASE_INIT_FLAGS, + interactive: false, + withVscodeSettings: true, + }).pipe(Effect.provide(layer), Effect.exit); + + const error = findFailure(exit); + expect(Predicate.isTagged(error, "InitParseSettingsError")).toBe(true); + const detail = (error as { detail: string }).detail; + expect(detail.startsWith(`Could not parse JSON in ${settingsPath}: `)).toBe(true); + expect(detail.length).toBeGreaterThan(`Could not parse JSON in ${settingsPath}: `.length); + expect(yield* readTextFile(settingsPath)).toBe(contents); + }).pipe(Effect.provide(BunServices.layer)); + }, + ); + it.effect("writes the stack-opt-in template when SUPABASE_EXPERIMENTAL_STACK=1", () => { const tempDir = tempRoot.current; diff --git a/apps/cli/src/commands/start/start.integration.test.ts b/apps/cli/src/commands/start/start.integration.test.ts index c428ea6474..3c92a7cb88 100644 --- a/apps/cli/src/commands/start/start.integration.test.ts +++ b/apps/cli/src/commands/start/start.integration.test.ts @@ -1625,6 +1625,30 @@ describe("start integration", () => { }).pipe(Effect.provide(BunServices.layer)), ); + it.effect("reports a config.json and config.jsonc conflict without an error-class prefix", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workdir = tempRoot.current; + yield* fs.makeDirectory(path.join(workdir, "supabase"), { recursive: true }); + yield* fs.writeFileString(path.join(workdir, "supabase", "config.json"), "{}"); + yield* fs.writeFileString(path.join(workdir, "supabase", "config.jsonc"), "{}"); + const { layer, child } = yield* setup({ skipConfig: true }); + + const exit = yield* Effect.exit(start(flags()).pipe(Effect.provide(layer))); + expect(Exit.isFailure(exit)).toBe(true); + if (Exit.isFailure(exit)) { + const error = Option.getOrUndefined(Cause.findErrorOption(exit.cause)); + expect(error).toMatchObject({ + _tag: "StartConfigLoadError", + message: + "Found both supabase/config.json and supabase/config.jsonc. Keep one and delete the other.", + }); + } + expect(child.spawned).toEqual([]); + }).pipe(Effect.provide(BunServices.layer)), + ); + it.effect("fails when auth.jwt_secret is configured but shorter than 16 characters", () => Effect.gen(function* () { const { layer, child } = yield* setup({ diff --git a/apps/cli/src/config/cli-config-values.integration.test.ts b/apps/cli/src/config/cli-config-values.integration.test.ts index 5b2661c8f5..a0fd87e6f1 100644 --- a/apps/cli/src/config/cli-config-values.integration.test.ts +++ b/apps/cli/src/config/cli-config-values.integration.test.ts @@ -456,6 +456,116 @@ describe("CliConfigValues deprecated keys", () => { ); }); +describe("CliConfigValues JSON config files", () => { + const load = (root: string) => + CliConfigValues.use((values) => values.load({ workdir: root, projectRef: Option.none() })); + + const writeSupabaseFile = (root: string, name: string, content: string) => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + yield* fs.writeFileString(path.join(root, "supabase", name), content); + }); + + it.live("warns once that config.toml is ignored and reads values from config.jsonc", () => + Effect.gen(function* () { + const root = yield* project("[db]\nport = 54111\n"); + yield* writeSupabaseFile( + root, + "config.jsonc", + '{\n // wins\n "db": { "port": 54222, },\n}\n', + ); + const output = mockOutput(); + + const first = yield* Effect.gen(function* () { + const loaded = yield* load(root); + yield* load(root); + return loaded; + }).pipe(Effect.provide(configValuesLayer({ output: output.layer }))); + + expect(first.materialized.config.db.port).toBe(54222); + expect(output.messages).toEqual([ + { + type: "warn", + message: + "Ignoring supabase/config.toml because supabase/config.jsonc takes precedence. Delete the file you don't use.", + }, + ]); + }).pipe(Effect.provide(BunServices.layer), (effect) => withShell({}, effect), Effect.scoped), + ); + + it.live("warns that config.toml is ignored when a strict config.json shadows it", () => + Effect.gen(function* () { + const root = yield* project("[db]\nport = 54111\n"); + yield* writeSupabaseFile(root, "config.json", '{"db": {"port": 54333}}'); + const output = mockOutput(); + + const loaded = yield* load(root).pipe( + Effect.provide(configValuesLayer({ output: output.layer })), + ); + + expect(loaded.materialized.config.db.port).toBe(54333); + expect(output.messages).toEqual([ + { + type: "warn", + message: + "Ignoring supabase/config.toml because supabase/config.json takes precedence. Delete the file you don't use.", + }, + ]); + }).pipe(Effect.provide(BunServices.layer), (effect) => withShell({}, effect), Effect.scoped), + ); + + it.live("prints no ignored-file warning when only config.toml exists", () => + Effect.gen(function* () { + const root = yield* project("[db]\nport = 54111\n"); + const output = mockOutput(); + + yield* load(root).pipe(Effect.provide(configValuesLayer({ output: output.layer }))); + + expect(output.messages).toEqual([]); + }).pipe(Effect.provide(BunServices.layer), (effect) => withShell({}, effect), Effect.scoped), + ); + + it.live("resolves a value from a commented config.jsonc-only project", () => + Effect.gen(function* () { + const root = yield* project(""); + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + yield* fs.remove(path.join(root, "supabase", "config.toml")); + yield* writeSupabaseFile( + root, + "config.jsonc", + '{\n /* local port */\n "db": { "port": 54444, },\n}\n', + ); + const output = mockOutput(); + + const loaded = yield* load(root).pipe( + Effect.provide(configValuesLayer({ output: output.layer })), + ); + + expect(loaded.materialized.config.db.port).toBe(54444); + expect(output.messages).toEqual([]); + }).pipe(Effect.provide(BunServices.layer), (effect) => withShell({}, effect), Effect.scoped), + ); + + it.live( + "fails with CliConfigFileConflictError when config.json and config.jsonc both exist", + () => + Effect.gen(function* () { + const root = yield* project(""); + yield* writeSupabaseFile(root, "config.json", "{}"); + yield* writeSupabaseFile(root, "config.jsonc", "{}"); + + const failure = yield* load(root).pipe(Effect.flip, Effect.provide(configValuesLayer())); + + expect(failure).toMatchObject({ _tag: "CliConfigFileConflictError" }); + expect(failure.message).toBe( + "Found both supabase/config.json and supabase/config.jsonc. Keep one and delete the other.", + ); + }).pipe(Effect.provide(BunServices.layer), (effect) => withShell({}, effect), Effect.scoped), + ); +}); + describe("CliConfigValues memoisation", () => { it.live("memoises a load per target and drops the memo after a write", () => Effect.gen(function* () { diff --git a/apps/cli/src/config/cli-config-values.layer.ts b/apps/cli/src/config/cli-config-values.layer.ts index 9fb8ce705a..be5478e76d 100644 --- a/apps/cli/src/config/cli-config-values.layer.ts +++ b/apps/cli/src/config/cli-config-values.layer.ts @@ -257,6 +257,15 @@ export const cliConfigValuesLayer = Layer.effect( for (const deprecatedPath of parsed?.deprecatedPaths ?? []) { yield* printOnce(deprecatedKeyLine(deprecatedPath)); } + if (parsed !== null) { + const used = path.relative(target.workdir, parsed.path); + for (const ignoredPath of parsed.ignoredPaths) { + const ignored = path.relative(target.workdir, ignoredPath); + yield* warnOnce( + `Ignoring ${ignored} because ${used} takes precedence. Delete the file you don't use.`, + ); + } + } const rawDocument = parsed?.rawDocument ?? {}; const shell = yield* readShellEnvironment({ diff --git a/apps/cli/src/shared/compute/compute-paths.ts b/apps/cli/src/shared/compute/compute-paths.ts index b07d498c95..081360c9a8 100644 --- a/apps/cli/src/shared/compute/compute-paths.ts +++ b/apps/cli/src/shared/compute/compute-paths.ts @@ -24,7 +24,7 @@ const RESERVED_SUPABASE_DIRS = ["functions", "migrations", LEGACY_PROJECT_STATE_ * directories above, which do not cover them — `supabase/config.toml` sits * outside every reserved subdirectory. */ -const RESERVED_SUPABASE_FILES = ["config.toml", "config.json"]; +const RESERVED_SUPABASE_FILES = ["config.toml", "config.json", "config.jsonc"]; /** `supabase/compute/` — where compute live, resolved against the project. */ export function computeRootDir(path: Path.Path, projectRoot: string): string { diff --git a/apps/cli/src/shared/compute/compute-paths.unit.test.ts b/apps/cli/src/shared/compute/compute-paths.unit.test.ts index 20c13c1e3b..763549e813 100644 --- a/apps/cli/src/shared/compute/compute-paths.unit.test.ts +++ b/apps/cli/src/shared/compute/compute-paths.unit.test.ts @@ -134,6 +134,7 @@ describe("resolveComputeSource", () => { // destination, and the project's config file is not that. { raw: "supabase/config.toml", reason: "supabase/config.toml" }, { raw: "supabase/config.json", reason: "supabase/config.json" }, + { raw: "supabase/config.jsonc", reason: "supabase/config.jsonc" }, ])("refuses $raw", ({ raw, reason }) => Effect.gen(function* () { const error = yield* Effect.flip( diff --git a/apps/cli/src/shared/config/experimental-feature.ts b/apps/cli/src/shared/config/experimental-feature.ts index 33e26c408e..5d0d9c14c4 100644 --- a/apps/cli/src/shared/config/experimental-feature.ts +++ b/apps/cli/src/shared/config/experimental-feature.ts @@ -1,6 +1,6 @@ import { CliConfigSchema, findCliProjectPaths } from "@supabase/config/effect"; +import { cliConfigFormatForPath, parseCliConfigDocument } from "@supabase/config/internal"; import { Config, Data, Effect, FileSystem, Option, Path, Result, Schema } from "effect"; -import * as SmolToml from "smol-toml"; import { pickCliConfigKey } from "../../config/cli-config-key.ts"; import { CliConfigKeys } from "../../config/cli-config-keys.ts"; import { resolveWorkdir } from "../../config/command-settings.layer.ts"; @@ -36,8 +36,6 @@ const firstExplicitLongFlagValue = ( return undefined; }; -const UnknownFromJsonString = Schema.fromJsonString(Schema.Unknown); - const readEnv = (name: string) => Config.option(Config.String(name)).pipe( Effect.orElseSucceed(() => Option.none()), @@ -63,9 +61,9 @@ export const readExperimentalFeatureConfig = (input: { const project = yield* findCliProjectPaths(resolvedWorkdir.workdir, { search: false }); if (project === null) return undefined; const content = yield* fs.readFileString(project.configPath); - const document = project.configPath.endsWith(".json") - ? yield* Schema.decodeEffect(UnknownFromJsonString)(content) - : yield* Effect.try(() => SmolToml.parse(content)); + const document = yield* Effect.try(() => + parseCliConfigDocument(content, cliConfigFormatForPath(project.configPath)), + ); const schema = Schema.Struct({ experimental: Schema.optionalKey( Schema.Struct({ [input.feature]: featureSchemas[input.feature] }), diff --git a/apps/cli/src/shared/functions/deploy.ts b/apps/cli/src/shared/functions/deploy.ts index 5e4b797056..e79a38ce89 100644 --- a/apps/cli/src/shared/functions/deploy.ts +++ b/apps/cli/src/shared/functions/deploy.ts @@ -11,6 +11,7 @@ import { inferFunctionsManifest, type ResolvedFunctionConfig as ManifestFunctionConfig, } from "@supabase/config/effect"; +import { parseJsonc } from "@supabase/config/internal"; import { Cause, Config, @@ -476,81 +477,6 @@ function humanSize(bytes: number) { return `${value.toFixed(precision)} ${units[index]}`; } -function stripJsonComments(contents: string): string { - const src = contents.replace(/^\uFEFF/, ""); - const out: Array = []; - let pendingCommaIndex = -1; - let index = 0; - while (index < src.length) { - const char = src.charAt(index); - - if (char === '"') { - pendingCommaIndex = -1; - out.push(char); - index += 1; - while (index < src.length) { - const stringChar = src.charAt(index); - out.push(stringChar); - index += 1; - if (stringChar === "\\") { - if (index < src.length) { - out.push(src.charAt(index)); - index += 1; - } - } else if (stringChar === '"') { - break; - } - } - continue; - } - - if (char === "/" && src.charAt(index + 1) === "/") { - index += 2; - while (index < src.length && src.charAt(index) !== "\n") { - index += 1; - } - continue; - } - - if (char === "/" && src.charAt(index + 1) === "*") { - index += 2; - while (index < src.length && !(src.charAt(index) === "*" && src.charAt(index + 1) === "/")) { - index += 1; - } - index += 2; - continue; - } - - if (char === ",") { - pendingCommaIndex = out.length; - out.push(char); - index += 1; - continue; - } - - if (char === "}" || char === "]") { - if (pendingCommaIndex >= 0) { - out[pendingCommaIndex] = ""; - pendingCommaIndex = -1; - } - out.push(char); - index += 1; - continue; - } - - if (char === " " || char === "\t" || char === "\n" || char === "\r") { - out.push(char); - index += 1; - continue; - } - - pendingCommaIndex = -1; - out.push(char); - index += 1; - } - return out.join(""); -} - function resolveImportTarget(path: Path.Path, jsonPath: string, target: string) { if (target.startsWith("/")) { return target; @@ -690,9 +616,13 @@ class ImportMapFile { } const parseImportMapContents = (contents: Uint8Array) => - decodeJson(stripJsonComments(new TextDecoder().decode(contents))).pipe( - Effect.mapError((error) => new FunctionImportMapSyntaxError({ message: error.message })), - ); + Effect.try({ + try: () => parseJsonc(new TextDecoder().decode(contents)), + catch: (error) => + new FunctionImportMapSyntaxError({ + message: error instanceof Error ? error.message : String(error), + }), + }); const loadImportMapFile = Effect.fnUntraced(function* ( pathname: string, diff --git a/apps/cli/src/shared/init/project-init.ts b/apps/cli/src/shared/init/project-init.ts index 224b5f8d11..a9193952fe 100644 --- a/apps/cli/src/shared/init/project-init.ts +++ b/apps/cli/src/shared/init/project-init.ts @@ -1,4 +1,5 @@ import { Effect, FileSystem, Path, Predicate, Schema } from "effect"; +import { parseJsonc } from "@supabase/config/internal"; import { promptYesNo } from "../../command-internal/prompt-yes-no.ts"; import { sanitizeProjectId } from "../config/project-id.ts"; import { Output } from "../output/output.service.ts"; @@ -12,88 +13,7 @@ import { } from "./project-init.templates.ts"; import { InitParseSettingsError } from "./project-init.errors.ts"; -// Strips line and block comments and trailing commas while preserving -// string contents, so an existing JSONC settings file parses correctly. -function stripJsonComments(contents: string): string { - const src = contents.replace(/^\uFEFF/, ""); - const out: Array = []; - let pendingCommaIndex = -1; - let i = 0; - while (i < src.length) { - const char = src.charAt(i); - - if (char === '"') { - pendingCommaIndex = -1; - out.push(char); - i++; - while (i < src.length) { - const stringChar = src.charAt(i); - out.push(stringChar); - i++; - if (stringChar === "\\") { - if (i < src.length) { - out.push(src.charAt(i)); - i++; - } - } else if (stringChar === '"') { - break; - } - } - continue; - } - - if (char === "/" && src.charAt(i + 1) === "/") { - i += 2; - while (i < src.length && src.charAt(i) !== "\n") { - i++; - } - continue; - } - - if (char === "/" && src.charAt(i + 1) === "*") { - i += 2; - while (i < src.length && !(src.charAt(i) === "*" && src.charAt(i + 1) === "/")) { - i++; - } - i += 2; - continue; - } - - // A comma is "trailing" if the next significant token is a closing brace or - // bracket; drop it in that case to match jsonc's trailing-comma handling. - if (char === ",") { - pendingCommaIndex = out.length; - out.push(char); - i++; - continue; - } - - if (char === "}" || char === "]") { - if (pendingCommaIndex >= 0) { - out[pendingCommaIndex] = ""; - pendingCommaIndex = -1; - } - out.push(char); - i++; - continue; - } - - if (char === " " || char === "\t" || char === "\n" || char === "\r") { - out.push(char); - i++; - continue; - } - - pendingCommaIndex = -1; - out.push(char); - i++; - } - return out.join(""); -} - -const decodeJsonObject = Schema.decodeUnknownEffect( - Schema.fromJsonString(Schema.Record(Schema.String, Schema.Unknown)), -); +const decodeJsonObject = Schema.decodeUnknownEffect(Schema.Record(Schema.String, Schema.Unknown)); const encodePrettyJson = Schema.encodeEffect(Schema.fromJsonString(Schema.Unknown, { space: 2 })); @@ -101,13 +21,17 @@ const encodePrettyJson = Schema.encodeEffect(Schema.fromJsonString(Schema.Unknow // as a typed `InitParseSettingsError` (never a fiber defect) and a // non-object document is rejected. function parseJsonObject(pathname: string, contents: string) { - return decodeJsonObject(stripJsonComments(contents)).pipe( - Effect.mapError( - (error) => - new InitParseSettingsError({ - detail: `Could not parse JSON in ${pathname}: ${error.message}`, - suggestion: `Fix or remove ${pathname}, then rerun \`supabase init\`.`, - }), + const parseFailure = (reason: string) => + new InitParseSettingsError({ + detail: `Could not parse JSON in ${pathname}: ${reason}`, + suggestion: `Fix or remove ${pathname}, then rerun \`supabase init\`.`, + }); + return Effect.try({ + try: () => parseJsonc(contents), + catch: (error) => parseFailure(error instanceof Error ? error.message : String(error)), + }).pipe( + Effect.flatMap((value) => + decodeJsonObject(value).pipe(Effect.mapError((error) => parseFailure(error.message))), ), ); } diff --git a/apps/cli/src/shared/output/normalize-error.unit.test.ts b/apps/cli/src/shared/output/normalize-error.unit.test.ts index 4699fbb685..72d35193f2 100644 --- a/apps/cli/src/shared/output/normalize-error.unit.test.ts +++ b/apps/cli/src/shared/output/normalize-error.unit.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "vitest"; import { Cause } from "effect"; import { CliError, Command } from "effect/cli"; import { CliConfigParseError, CliProjectEnvParseError } from "@supabase/config"; +import { JsoncSyntaxError } from "@supabase/config/internal"; import { branchesCommand } from "../../commands/branches/branches.command.ts"; import { networkRestrictionsCommand } from "../../commands/network-restrictions/network-restrictions.command.ts"; import { ssoCommand } from "../../commands/sso/sso.command.ts"; @@ -54,6 +55,19 @@ describe("normalizeCliError", () => { }); }); + test("CliConfigParseError with a JSONC syntax cause names the reason, line and column", () => { + const error = new CliConfigParseError({ + path: "supabase/config.jsonc", + format: "json", + cause: new JsoncSyntaxError("Value expected", 3, 17), + }); + + expect(normalizeCliError(error)).toEqual({ + code: "CliConfigParseError", + message: "failed to parse supabase/config.jsonc: Value expected (line 3, column 17)", + }); + }); + test("CliProjectEnvParseError falls back to its bare tag as both code and message", () => { const error = new CliProjectEnvParseError({ path: "supabase/.env.local", diff --git a/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt b/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt index 270cf7f737..b4d6123a9f 100644 --- a/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt +++ b/apps/cli/src/shared/telemetry/__fixtures__/error-tags.txt @@ -38,6 +38,7 @@ BranchesUnpauseNetworkError BranchesUnpauseUnexpectedStatusError BranchesUpdateNetworkError BranchesUpdateUnexpectedStatusError +CliConfigFileConflictError CliConfigFlagConflictError CliConfigLoadError CliConfigParseError diff --git a/apps/cli/src/shared/telemetry/error-actionability.ts b/apps/cli/src/shared/telemetry/error-actionability.ts index 592ba9800a..eccc966e04 100644 --- a/apps/cli/src/shared/telemetry/error-actionability.ts +++ b/apps/cli/src/shared/telemetry/error-actionability.ts @@ -985,6 +985,7 @@ const externalActionabilityByTag: Record = { // @supabase/config CliConfigParseError: () => actionability.invalidConfig, + CliConfigFileConflictError: () => actionability.invalidConfig, CliConfigRemovedKeyError: () => actionability.invalidConfig, CliProjectEnvParseError: () => actionability.invalidConfig, DuplicateRemoteProjectIdError: () => actionability.invalidConfig, diff --git a/packages/config/AGENTS.md b/packages/config/AGENTS.md index 79d3f3187f..5b608ccd2d 100644 --- a/packages/config/AGENTS.md +++ b/packages/config/AGENTS.md @@ -38,7 +38,8 @@ artifacts (`./schema.json`, `./project-schema.json`). `InternalResolveCliConfigOptions`, is package-internal and not itself re-exported) — plus the otherwise-internal registry data (`AUTH_HOOK_NAMES`, `unmappedSecretApiPaths`, `projectConfigMappingRows`, `ProjectConfigMappingRow`, `ProjectConfigApiAttributes`, - `ENV_CAPTURE_REGEX`). It also carries `supabase config pull`/`diff`'s own support surface + `ENV_CAPTURE_REGEX`), the JSONC parser (`parseJsonc`, `JsoncSyntaxError`) and + `cliConfigFormatForPath`. It also carries `supabase config pull`/`diff`'s own support surface (CLI-2064): the format-preserving surgical editor (`applyConfigEdits` and its `ConfigEdit`/`ConfigEditOutcome`/`ConfigEditRefusal`/`ConfigEditRefusalReason`/`ConfigEditValue`/ `AppliedConfigEdit` types — the config diff engine itself, `diffProjectConfig` and its diff --git a/packages/config/docs/cli-config-loading.md b/packages/config/docs/cli-config-loading.md index f1108fd2f6..80e44dbf5c 100644 --- a/packages/config/docs/cli-config-loading.md +++ b/packages/config/docs/cli-config-loading.md @@ -5,7 +5,7 @@ This document explains how the CLI's on-disk config document loading works, acro ## Vocabulary -- `CliConfig`: the persisted config-file document (`supabase/config.toml` / `supabase/config.json`) +- `CliConfig`: the persisted config-file document (`supabase/config.toml`, `supabase/config.json`, or `supabase/config.jsonc`) — the full local superset, including local-only sections (`studio`, ports, `edge_runtime`, `analytics`, …) plus `[remotes.*]` overrides. Owned by `@supabase/config`. - `ProjectConfig`: the hosted-project subset — the sections a hosted project manages (`api`, @@ -89,13 +89,15 @@ upward until it finds the nearest ancestor containing one of: - `supabase/config.toml` - `supabase/config.json` +- `supabase/config.jsonc` The first match wins. A caller that already holds an authoritative, pre-resolved root — for example one derived from an explicit `--workdir`/`SUPABASE_WORKDIR` — can pass `{ search: false }` to check only that exact directory, with no ancestor climb. -Within one discovered `supabase/` directory, `config.json` takes precedence over `config.toml` -when both files exist. +Within one discovered `supabase/` directory, `config.json` or `config.jsonc` takes precedence over +`config.toml` when both exist. Discovery never fails on a `config.json` + `config.jsonc` pair; loading +and saving do. This produces a `CliProjectPaths` (the type is exported from `@supabase/config`): @@ -116,13 +118,13 @@ and env files under `supabase/`. The project-scoped files are: -- `supabase/config.toml` or `supabase/config.json` +- `supabase/config.toml`, `supabase/config.json`, or `supabase/config.jsonc` - `supabase/.env` - `supabase/.env.local` Their intended roles are: -- `config.toml` or `config.json`: the shared, committed `CliConfig` document +- `config.toml`, `config.json`, or `config.jsonc`: the shared, committed `CliConfig` document - `.env`: managed or shared project env values - `.env.local`: user-editable local overrides @@ -138,8 +140,13 @@ defines the full config/env scope for that invocation. these rules: - `loadCliConfig(cwd)` discovers the project, then: - - if both `supabase/config.json` and `supabase/config.toml` exist, JSON wins - - the returned `LoadedCliConfig.ignoredPaths` reports the shadowed config file path + - `config.json` and `config.jsonc` are both parsed as JSONC (comments and trailing commas + allowed); parse errors report the line and column + - if both `supabase/config.json` and `supabase/config.jsonc` exist, loading fails with + `CliConfigFileConflictError` naming both files + - if a JSON file and `supabase/config.toml` both exist, JSON wins, and the returned + `LoadedCliConfig.ignoredPaths` reports the shadowed `config.toml`; the CLI warns that it is + ignored - if only one config file exists, that file is loaded - if no config file exists in the discovered project, loading returns `null` - `loadCliConfigFile(path)` loads one explicit file path directly instead of discovering or @@ -147,12 +154,14 @@ these rules: `saveCliConfig()` uses these rules: -- if the discovered project already has `config.json`, save back to JSON +- if the discovered project already has `config.jsonc`, save back to it; otherwise, if it has + `config.json`, save back to that - otherwise if it already has `config.toml`, save back to TOML - otherwise default new writes to `supabase/config.json` +- if both `config.json` and `config.jsonc` exist, saving fails with `CliConfigFileConflictError` - callers can still force a format explicitly via `SaveCliConfigOptions.format` -- after writing, it removes the sibling config file in the other format, if one exists, so a - project never ends up with both after a save +- after writing, it removes every other config file among `config.json`, `config.jsonc` and + `config.toml`, so a project never ends up with more than one after a save The saved file may also preserve a top-level `"$schema"` key as editor metadata: when a caller doesn't pass an explicit `schemaRef`, saving reuses whatever `$schema` the project's existing @@ -186,7 +195,7 @@ The `ambient` source label just means the value came from `process.env`. ## Raw Config Loading -`CliConfig` is loaded by parsing the TOML/JSON file and decoding it against `CliConfigSchema`. +`CliConfig` is loaded by parsing the TOML/JSON(C) file and decoding it against `CliConfigSchema`. Before decoding, most string values matching `env(NAME)` are substituted automatically using the resolved `CliProjectEnvironment` — this is required so numeric and boolean fields don't crash the @@ -374,7 +383,7 @@ one order (ADR 0032): 1. an explicit flag 2. shell environment 3. project `.env*` files -4. config: `config.json` when present, else `config.toml`, with the matched `[remotes.*]` block +4. config: `config.json` or `config.jsonc` when present, else `config.toml`, with the matched `[remotes.*]` block over the base document 5. the default @@ -470,8 +479,9 @@ The CLI also rejects a non-empty `SUPABASE_EXPERIMENTAL_ORIOLEDB_VERSION` enviro `@supabase/config/internal` exposes the stages the CLI composes in place of `loadCliConfig`: -1. `parseCliConfigDocumentFile` reads `config.json` or `config.toml` without decoding, and fails - with `CliConfigRemovedKeyError` when the document uses a removed key. +1. `parseCliConfigDocumentFile` reads `config.json`, `config.jsonc` or `config.toml` without decoding, and + fails with `CliConfigRemovedKeyError` when the document uses a removed key, or + `CliConfigFileConflictError` when both JSON files exist. 2. `mergeParsedCliConfig` applies the remote chosen by the `selectRemote` callback. 3. The CLI overlays flag, env and default values onto the merged document. 4. `decodeMergedCliConfig` expands `env()` references, then decodes and validates the overlaid @@ -496,8 +506,8 @@ Neither is part of `@supabase/config`'s input. Important distinction: -- `@supabase/config` discovers committed config only from `supabase/config.toml` or - `supabase/config.json` +- `@supabase/config` discovers committed config only from `supabase/config.toml`, + `supabase/config.json` or `supabase/config.jsonc` - the CLI may still resolve a project root from the nearest `.supabase/project.json` when it needs to find local machine state for commands like `link`, `unlink`, `start`, `stop`, `status`, `stack list`, or `logs` diff --git a/packages/config/package.json b/packages/config/package.json index 8fffa6469a..a2c324917d 100644 --- a/packages/config/package.json +++ b/packages/config/package.json @@ -74,6 +74,7 @@ "dependencies": { "@standard-schema/spec": "^1.1.0", "dedent": "^1.7.2", + "jsonc-parser": "^3.3.1", "smol-toml": "^1.8.0" }, "devDependencies": { diff --git a/packages/config/src/cli-config.service.ts b/packages/config/src/cli-config.service.ts index 0de82502c9..a21ec52ee2 100644 --- a/packages/config/src/cli-config.service.ts +++ b/packages/config/src/cli-config.service.ts @@ -7,6 +7,7 @@ import type { SaveCliConfigOptions, } from "./config-document.ts"; import type { + CliConfigFileConflictError, CliConfigParseError, CliConfigRemovedKeyError, CliProjectEnvParseError, @@ -23,6 +24,7 @@ import type { * consumer (`@supabase/config/io`) can distinguish these via `instanceof`. */ type CliConfigStoreError = + | CliConfigFileConflictError | CliConfigParseError | CliConfigRemovedKeyError | DuplicateRemoteProjectIdError diff --git a/packages/config/src/config-format.ts b/packages/config/src/config-format.ts index 1c58d3c8cc..2101a1f31f 100644 --- a/packages/config/src/config-format.ts +++ b/packages/config/src/config-format.ts @@ -1,2 +1,2 @@ -/** The on-disk formats a CLI config document can be written or parsed as. */ +/** The on-disk formats a CLI config document can be written or parsed as; "json" covers both `config.json` and `config.jsonc`. */ export type ConfigFormat = "json" | "toml"; diff --git a/packages/config/src/entrypoint-purity.unit.test.ts b/packages/config/src/entrypoint-purity.unit.test.ts index 0ac8237fef..e99d4110aa 100644 --- a/packages/config/src/entrypoint-purity.unit.test.ts +++ b/packages/config/src/entrypoint-purity.unit.test.ts @@ -392,6 +392,7 @@ describe("src/index.ts export surface", () => { expect(Object.keys(defaultEntrypoint).sort()).toMatchInlineSnapshot(` [ "CLI_CONFIG_SCHEMA_URL", + "CliConfigFileConflictError", "CliConfigParseError", "CliConfigRemovedKeyError", "CliConfigSchema", @@ -432,6 +433,7 @@ describe("src/effect.ts is a superset of src/index.ts", () => { expect(Object.keys(effectEntrypoint).sort()).toMatchInlineSnapshot(` [ "CLI_CONFIG_SCHEMA_URL", + "CliConfigFileConflictError", "CliConfigParseError", "CliConfigRemovedKeyError", "CliConfigSchema", @@ -518,9 +520,11 @@ describe("src/internal.ts export surface", () => { "CliConfigWriteError", "DEFAULT_MIGRATION_CHECK_SEVERITY", "ENV_CAPTURE_REGEX", + "JsoncSyntaxError", "MIGRATION_CHECK_RULES", "PROVIDER_SIGNUP_ALIASES", "applyConfigEdits", + "cliConfigFormatForPath", "decodeCliConfigDocumentForValidationEffect", "decodeMergedCliConfig", "dualScopeProjectConfigPaths", @@ -529,6 +533,7 @@ describe("src/internal.ts export surface", () => { "mergeParsedCliConfig", "parseCliConfigDocument", "parseCliConfigDocumentFile", + "parseJsonc", "projectConfigApiBlockKeys", "projectConfigMappingRows", "remoteNameForProjectRef", diff --git a/packages/config/src/errors.ts b/packages/config/src/errors.ts index 252f763a00..b9c0febc3e 100644 --- a/packages/config/src/errors.ts +++ b/packages/config/src/errors.ts @@ -37,6 +37,16 @@ export class CliConfigRemovedKeyError extends Data.TaggedError("CliConfigRemoved } } +/** Both `config.json` and `config.jsonc` exist in one project; `paths` lists every conflicting file. */ +export class CliConfigFileConflictError extends Data.TaggedError("CliConfigFileConflictError")<{ + readonly paths: ReadonlyArray; +}> { + override get message(): string { + const files = this.paths.map((path) => path.split(/[\\/]/).slice(-2).join("/")); + return `Found both ${files.join(" and ")}. Keep one and delete the other.`; + } +} + const PROJECT_CONFIG_PARSE_ERROR_MESSAGE_PREFIX = "Could not read the project config from the Management API response"; diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index 8ee4be4002..14d47beee6 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -13,6 +13,7 @@ export { type CliConfigJson, } from "./base.ts"; export { + CliConfigFileConflictError, CliConfigParseError, CliConfigRemovedKeyError, CliProjectEnvParseError, diff --git a/packages/config/src/internal.ts b/packages/config/src/internal.ts index c0ed0a93b2..c0acb12429 100644 --- a/packages/config/src/internal.ts +++ b/packages/config/src/internal.ts @@ -46,6 +46,8 @@ export { decodeCliConfigDocumentForValidationEffect, type DecodeCliConfigDocumentForValidationEffectOptions, parseCliConfigDocument, + cliConfigFormatForPath, } from "./io.ts"; +export { JsoncSyntaxError, parseJsonc } from "./jsonc.ts"; export { PROVIDER_SIGNUP_ALIASES } from "./lib/provider-signup.ts"; export { CliConfigWriteError } from "./errors.ts"; diff --git a/packages/config/src/io.ts b/packages/config/src/io.ts index 49b2c552b9..88979e99da 100644 --- a/packages/config/src/io.ts +++ b/packages/config/src/io.ts @@ -14,6 +14,7 @@ import { } from "./config-document.ts"; import type { ConfigFormat } from "./config-format.ts"; import { + CliConfigFileConflictError, DuplicateRemoteProjectIdError, InvalidRemoteProjectIdError, CliConfigParseError, @@ -21,6 +22,7 @@ import { } from "./errors.ts"; import { interpolateEnvReferencesAgainstSchema } from "./lib/env.ts"; import { normalizeDeprecatedProviderSignupKeys } from "./lib/provider-signup.ts"; +import { parseJsonc } from "./jsonc.ts"; import { findCliProjectPaths } from "./paths.ts"; import { rejectRemovedConfigKeys } from "./removed-config-keys.ts"; import { setOwnProperty } from "./sparse.ts"; @@ -35,8 +37,13 @@ function configTomlPathWith(path: Path.Path, cwd: string): string { return path.join(cwd, "supabase", "config.toml"); } -function siblingConfigPathWith(path: Path.Path, cwd: string, format: ConfigFormat): string { - return format === "json" ? configTomlPathWith(path, cwd) : configJsonPathWith(path, cwd); +function configJsoncPathWith(path: Path.Path, cwd: string): string { + return path.join(cwd, "supabase", "config.jsonc"); +} + +/** Not covered by semver — exported from `@supabase/config/internal` only. Maps a config file path to its document format. */ +export function cliConfigFormatForPath(filePath: string): ConfigFormat { + return filePath.endsWith(".json") || filePath.endsWith(".jsonc") ? "json" : "toml"; } /** @@ -218,7 +225,7 @@ const applyRemoteOverride = Effect.fnUntraced(function* ( }); export function parseCliConfigDocument(content: string, format: ConfigFormat): unknown { - return format === "json" ? JSON.parse(content) : SmolToml.parse(content); + return format === "json" ? parseJsonc(content) : SmolToml.parse(content); } /** @@ -375,7 +382,7 @@ export const configTomlPath = Effect.fnUntraced(function* (cwd: string) { const readCliConfigDocumentFile = Effect.fnUntraced(function* (filePath: string) { const fs = yield* FileSystem.FileSystem; - const format = filePath.endsWith(".json") ? "json" : "toml"; + const format = cliConfigFormatForPath(filePath); const content = yield* fs.readFileString(filePath); const document = yield* Effect.try({ try: () => parseCliConfigDocument(content, format), @@ -628,27 +635,35 @@ const locateCliConfigFile = Effect.fnUntraced(function* ( options: { readonly search?: boolean; readonly tomlOnly?: boolean } | undefined, ) { const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; const project = yield* findCliProjectPaths(cwd, { search: options?.search }); if (project === null) { return null; } - const jsonPath = project.configPath.endsWith(".json") - ? project.configPath - : project.configPath.replace(/config\.toml$/, "config.json"); - const tomlPath = project.configPath.endsWith(".toml") - ? project.configPath - : project.configPath.replace(/config\.json$/, "config.toml"); + const jsonPath = path.join(project.supabaseDir, "config.json"); + const jsoncPath = path.join(project.supabaseDir, "config.jsonc"); + const tomlPath = path.join(project.supabaseDir, "config.toml"); + const tomlExists = yield* fs.exists(tomlPath); - if (!options?.tomlOnly && (yield* fs.exists(jsonPath))) { - return { - filePath: jsonPath, - ignoredPaths: (yield* fs.exists(tomlPath)) ? [tomlPath] : [], - }; + if (!options?.tomlOnly) { + const jsonExists = yield* fs.exists(jsonPath); + const jsoncExists = yield* fs.exists(jsoncPath); + + if (jsonExists && jsoncExists) { + return yield* new CliConfigFileConflictError({ paths: [jsonPath, jsoncPath] }); + } + + if (jsonExists || jsoncExists) { + return { + filePath: jsonExists ? jsonPath : jsoncPath, + ignoredPaths: tomlExists ? [tomlPath] : [], + }; + } } - if (yield* fs.exists(tomlPath)) { + if (tomlExists) { return { filePath: tomlPath, ignoredPaths: [] }; } @@ -748,28 +763,28 @@ export const mergeParsedCliConfig = Effect.fn("CliConfig.mergeParsed")(function* } satisfies MergedCliConfigDocument; }); -const resolveSaveFormat = Effect.fnUntraced(function* ( +const resolveSaveTarget = Effect.fnUntraced(function* ( cwd: string, format: ConfigFormat | undefined, ) { - if (format !== undefined) { - return format; - } - const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; const jsonPath = configJsonPathWith(path, cwd); + const jsoncPath = configJsoncPathWith(path, cwd); const tomlPath = configTomlPathWith(path, cwd); + const jsonExists = yield* fs.exists(jsonPath); + const jsoncExists = yield* fs.exists(jsoncPath); - if (yield* fs.exists(jsonPath)) { - return "json" as const; + if (jsonExists && jsoncExists) { + return yield* new CliConfigFileConflictError({ paths: [jsonPath, jsoncPath] }); } - if (yield* fs.exists(tomlPath)) { - return "toml" as const; - } + const resolved = + format ?? + (jsonExists || jsoncExists || !(yield* fs.exists(tomlPath)) ? ("json" as const) : "toml"); + const filePath = resolved === "toml" ? tomlPath : jsoncExists ? jsoncPath : jsonPath; - return "json" as const; + return { format: resolved, filePath, allPaths: [jsonPath, jsoncPath, tomlPath] } as const; }); const DEFAULT_CLI_CONFIG_FILE_MODE = 0o644; @@ -808,15 +823,12 @@ export const saveCliConfig = Effect.fn("CliConfig.save")(function* (options: Sav const path = yield* Path.Path; const project = yield* findCliProjectPaths(options.cwd); const baseCwd = project?.projectRoot ?? options.cwd; - const format = yield* resolveSaveFormat(baseCwd, options.format); + const { format, filePath, allPaths } = yield* resolveSaveTarget(baseCwd, options.format); const existingSchemaRef = options.schemaRef !== undefined || project === null ? undefined : yield* readExistingSchemaRef(baseCwd); const schemaRef = options.schemaRef ?? existingSchemaRef; - const filePath = - format === "json" ? configJsonPathWith(path, baseCwd) : configTomlPathWith(path, baseCwd); - const siblingPath = siblingConfigPathWith(path, baseCwd, format); const content = format === "json" ? encodeCliConfigToJsonDocument(options.config, schemaRef) @@ -824,8 +836,10 @@ export const saveCliConfig = Effect.fn("CliConfig.save")(function* (options: Sav yield* fs.makeDirectory(path.dirname(filePath), { recursive: true }); yield* writeFileAtomic(filePath, content); - if (yield* fs.exists(siblingPath)) { - yield* fs.remove(siblingPath); + for (const otherPath of allPaths) { + if (otherPath !== filePath && (yield* fs.exists(otherPath))) { + yield* fs.remove(otherPath); + } } return { diff --git a/packages/config/src/io.unit.test.ts b/packages/config/src/io.unit.test.ts index 75795f54bf..707805fd15 100644 --- a/packages/config/src/io.unit.test.ts +++ b/packages/config/src/io.unit.test.ts @@ -38,6 +38,9 @@ import { writeCliConfigDocumentText, } from "./io.ts"; import { loadCliConfig as loadCliConfigFromNode } from "./node.ts"; +import { findCliProjectPaths } from "./paths.ts"; +import { JsoncSyntaxError } from "./jsonc.ts"; +import { CliConfigFileConflictError, CliConfigParseError } from "./errors.ts"; import { cliConfigStoreLayer } from "./cli-config.layer.ts"; import { CliConfigStore } from "./cli-config.service.ts"; import { CLI_CONFIG_SCHEMA_URL } from "./schema-metadata.ts"; @@ -208,7 +211,7 @@ describe("config io", () => { }), )); - test("rejects JSON comments and trailing commas", () => + test("loads config.json with comments and trailing commas", () => runConfigEffect( Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; @@ -221,7 +224,7 @@ describe("config io", () => { path, `{ // project ref - "project_id": "abc123", + "project_id": "abcdefghijklmnopqrst", "db": { "major_version": 16, } @@ -229,9 +232,9 @@ describe("config io", () => { `, ); - const exit = yield* Effect.exit(loadCliConfigFile(path)); + const loaded = yield* loadCliConfigFile(path); - expect(Exit.isFailure(exit)).toBe(true); + expect(loaded.config.db.major_version).toBe(16); }), )); @@ -476,6 +479,177 @@ stack = true }), )); + test("loads a commented config.jsonc and discovers it from a nested cwd", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + const nestedCwd = join(cwd, "apps", "web"); + const jsoncPath = join(cwd, "supabase", "config.jsonc"); + + yield* fs.makeDirectory(nestedCwd, { recursive: true }); + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString( + jsoncPath, + `{ + // project ref + "project_id": "jsonc_ref", + "db": { "major_version": 16, }, +} +`, + ); + + const loaded = yield* loadCliConfig(nestedCwd); + expect(loaded?.format).toBe("json"); + expect(loaded?.path.endsWith("config.jsonc")).toBe(true); + expect(loaded?.config.project_id).toBe("jsonc_ref"); + expect(loaded?.config.db.major_version).toBe(16); + + const paths = yield* findCliProjectPaths(nestedCwd); + expect(paths?.configPath).toBe(jsoncPath); + expect(paths?.projectRoot).toBe(cwd); + }), + )); + + test("fails loading with CliConfigFileConflictError when config.json and config.jsonc both exist", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + const jsonPath = join(cwd, "supabase", "config.json"); + const jsoncPath = join(cwd, "supabase", "config.jsonc"); + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(jsonPath, `{"project_id": "from_json"}`); + yield* fs.writeFileString(jsoncPath, `{"project_id": "from_jsonc"}`); + + const exit = yield* Effect.exit(loadCliConfig(cwd)); + const error = Exit.isFailure(exit) ? Cause.findErrorOption(exit.cause) : Option.none(); + + expect(Option.isSome(error)).toBe(true); + if (Option.isSome(error)) { + expect(error.value).toBeInstanceOf(CliConfigFileConflictError); + if (error.value instanceof CliConfigFileConflictError) { + expect(error.value.paths).toEqual([jsonPath, jsoncPath]); + expect(error.value.message).toBe( + "Found both supabase/config.json and supabase/config.jsonc. Keep one and delete the other.", + ); + } + } + }), + )); + + test("fails saving with CliConfigFileConflictError and leaves both files untouched", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + const jsonPath = join(cwd, "supabase", "config.json"); + const jsoncPath = join(cwd, "supabase", "config.jsonc"); + const jsonText = `{"project_id": "from_json"}`; + const jsoncText = `// keep me\n{"project_id": "from_jsonc"}`; + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(jsonPath, jsonText); + yield* fs.writeFileString(jsoncPath, jsoncText); + + const exit = yield* Effect.exit(saveCliConfig({ cwd, config: sampleConfig })); + const error = Exit.isFailure(exit) ? Cause.findErrorOption(exit.cause) : Option.none(); + + expect(Option.isSome(error)).toBe(true); + if (Option.isSome(error)) { + expect(error.value).toBeInstanceOf(CliConfigFileConflictError); + if (error.value instanceof CliConfigFileConflictError) { + expect(error.value.paths).toEqual([jsonPath, jsoncPath]); + expect(error.value.message).toBe( + "Found both supabase/config.json and supabase/config.jsonc. Keep one and delete the other.", + ); + } + } + expect(yield* fs.readFileString(jsonPath)).toBe(jsonText); + expect(yield* fs.readFileString(jsoncPath)).toBe(jsoncText); + }), + )); + + test("prefers config.jsonc over config.toml and reports the toml as ignored", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + const jsoncPath = join(cwd, "supabase", "config.jsonc"); + const tomlPath = yield* configTomlPath(cwd); + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(jsoncPath, `{ // wins\n "project_id": "jsonc_ref" }`); + yield* fs.writeFileString(tomlPath, `project_id = "toml-ref"\n`); + + const loaded = yield* loadCliConfig(cwd); + expect(loaded?.path).toBe(jsoncPath); + expect(loaded?.config.project_id).toBe("jsonc_ref"); + expect(loaded?.ignoredPaths).toEqual([tomlPath]); + }), + )); + + test("saves back to config.jsonc without creating config.json and removes a sibling config.toml", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + const jsonPath = join(cwd, "supabase", "config.json"); + const jsoncPath = join(cwd, "supabase", "config.jsonc"); + const tomlPath = yield* configTomlPath(cwd); + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(jsoncPath, `{ // old\n "project_id": "old_ref" }`); + yield* fs.writeFileString(tomlPath, `project_id = "toml-ref"\n`); + + const saved = yield* saveCliConfig({ cwd, config: sampleConfig }); + + expect(saved.format).toBe("json"); + expect(saved.path).toBe(jsoncPath); + expect(yield* fs.exists(jsonPath)).toBe(false); + expect(yield* fs.exists(tomlPath)).toBe(false); + const reloaded = yield* loadCliConfig(cwd); + expect(reloaded?.path).toBe(jsoncPath); + expect(reloaded?.config.project_id).toBe("ref_123"); + }), + )); + + test("returns a CliConfigParseError caused by a JsoncSyntaxError with line and column for malformed config.jsonc", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + const jsoncPath = join(cwd, "supabase", "config.jsonc"); + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(jsoncPath, `{\n // note\n "project_id": ,\n}`); + + const exit = yield* Effect.exit(loadCliConfig(cwd)); + const error = Exit.isFailure(exit) ? Cause.findErrorOption(exit.cause) : Option.none(); + + expect(Option.isSome(error)).toBe(true); + if (Option.isSome(error)) { + expect(error.value).toBeInstanceOf(CliConfigParseError); + if (error.value instanceof CliConfigParseError) { + expect(error.value.path).toBe(jsoncPath); + expect(error.value.format).toBe("json"); + expect(error.value.cause).toBeInstanceOf(JsoncSyntaxError); + if (error.value.cause instanceof JsoncSyntaxError) { + expect(error.value.cause.line).toBe(3); + expect(error.value.cause.column).toBe(17); + } + } + } + }), + )); + test("loads TOML instead of JSON when tomlOnly is set, even if JSON exists", () => runConfigEffect( Effect.gen(function* () { @@ -521,6 +695,61 @@ stack = true }), )); + test("loads TOML with tomlOnly when config.json and config.jsonc conflict", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(join(cwd, "supabase", "config.json"), `{"project_id": "a"}`); + yield* fs.writeFileString(join(cwd, "supabase", "config.jsonc"), `{"project_id": "b"}`); + yield* fs.writeFileString( + join(cwd, "supabase", "config.toml"), + `project_id = "toml-ref"\n`, + ); + + const loaded = yield* loadCliConfig(cwd, { tomlOnly: true }); + expect(loaded?.format).toBe("toml"); + expect(loaded?.config.project_id).toBe("toml-ref"); + }), + )); + + test("returns null when tomlOnly is set and only config.jsonc exists", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(join(cwd, "supabase", "config.jsonc"), `{"project_id": "b"}`); + + const loaded = yield* loadCliConfig(cwd, { tomlOnly: true }); + expect(loaded).toBeNull(); + }), + )); + + test("writes config.toml and removes config.jsonc when saving TOML in a JSONC project", () => + runConfigEffect( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const cwd = yield* makeTempProject(); + const jsoncPath = join(cwd, "supabase", "config.jsonc"); + + yield* fs.makeDirectory(join(cwd, "supabase"), { recursive: true }); + yield* fs.writeFileString(jsoncPath, `// c\n{"project_id": "b"}`); + + const saved = yield* saveCliConfig({ cwd, config: sampleConfig, format: "toml" }); + + expect(saved.path).toBe(join(cwd, "supabase", "config.toml")); + expect(yield* fs.exists(saved.path)).toBe(true); + expect(yield* fs.exists(jsoncPath)).toBe(false); + }), + )); + test("loads TOML when JSON is absent", () => runConfigEffect( Effect.gen(function* () { diff --git a/packages/config/src/jsonc.ts b/packages/config/src/jsonc.ts new file mode 100644 index 0000000000..dabc635b36 --- /dev/null +++ b/packages/config/src/jsonc.ts @@ -0,0 +1,92 @@ +import { printParseErrorCode, visit, type ParseErrorCode } from "jsonc-parser"; +import { setOwnProperty } from "./sparse.ts"; + +/** A JSONC document failed to parse; `line` and `column` are 1-based and the message never quotes source text. */ +// oxlint-disable-next-line effecttsgo/extends-native-error -- only ever a thrown cause, never an Effect failure. +export class JsoncSyntaxError extends Error { + override readonly name = "JsoncSyntaxError"; + readonly reason: string; + readonly line: number; + readonly column: number; + + constructor(reason: string, line: number, column: number) { + super(`${reason} (line ${line}, column ${column})`); + this.reason = reason; + this.line = line; + this.column = column; + } +} + +function describeErrorCode(code: ParseErrorCode): string { + const words = printParseErrorCode(code) + .replace(/([a-z])([A-Z])/g, "$1 $2") + .toLowerCase(); + return words.charAt(0).toUpperCase() + words.slice(1); +} + +type Container = { readonly value: Record | unknown[]; key?: string }; + +/** Parses JSON with comments and trailing commas, keeping `__proto__` keys as own properties like `JSON.parse`. */ +export function parseJsonc(text: string): unknown { + const source = text.charCodeAt(0) === 0xfeff ? text.slice(1) : text; + const stack: Container[] = []; + let root: unknown; + let failure: JsoncSyntaxError | undefined; + + const attach = (value: unknown, key?: string) => { + const parent = stack[stack.length - 1]; + if (parent === undefined) { + root = value; + } else if (Array.isArray(parent.value)) { + parent.value.push(value); + } else if (key !== undefined) { + setOwnProperty(parent.value, key, value); + } + }; + + visit( + source, + { + onObjectBegin: () => { + stack.push({ value: {} }); + }, + onObjectProperty: (property) => { + const top = stack[stack.length - 1]; + if (top !== undefined) { + top.key = property; + } + }, + onObjectEnd: () => { + const done = stack.pop(); + if (done !== undefined) { + attach(done.value, stack[stack.length - 1]?.key); + } + }, + onArrayBegin: () => { + stack.push({ value: [] }); + }, + onArrayEnd: () => { + const done = stack.pop(); + if (done !== undefined) { + attach(done.value, stack[stack.length - 1]?.key); + } + }, + onLiteralValue: (value) => { + attach(value, stack[stack.length - 1]?.key); + }, + onError: (code, _offset, _length, startLine, startCharacter) => { + failure ??= new JsoncSyntaxError( + describeErrorCode(code), + startLine + 1, + startCharacter + 1, + ); + }, + }, + { allowTrailingComma: true, disallowComments: false, allowEmptyContent: false }, + ); + + if (failure !== undefined) { + throw failure; + } + return root; +} diff --git a/packages/config/src/jsonc.unit.test.ts b/packages/config/src/jsonc.unit.test.ts new file mode 100644 index 0000000000..1e42e88b16 --- /dev/null +++ b/packages/config/src/jsonc.unit.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, test } from "vitest"; +import { JsoncSyntaxError, parseJsonc } from "./jsonc.ts"; + +function syntaxErrorFor(text: string): JsoncSyntaxError { + try { + parseJsonc(text); + } catch (error) { + if (error instanceof JsoncSyntaxError) { + return error; + } + throw error; + } + throw new Error("expected parseJsonc to throw"); +} + +describe("parseJsonc", () => { + test("parses comments and trailing commas to the value strict JSON gives for the cleaned text", () => { + const jsonc = `{ + // line comment + "name": "a // not a comment", + /* block + comment */ + "list": [1, 2, {"x": null,},], + "nested": {"on": true, "n": -1.5e2,}, +}`; + const strict = `{"name":"a // not a comment","list":[1,2,{"x":null}],"nested":{"on":true,"n":-1.5e2}}`; + + expect(parseJsonc(jsonc)).toEqual(JSON.parse(strict)); + }); + + test("keeps a __proto__ key as an own property without changing the prototype", () => { + const parsed = parseJsonc(`{"__proto__": {"polluted": true}, "safe": 1}`) as Record< + string, + unknown + >; + const strict = JSON.parse(`{"__proto__": {"polluted": true}, "safe": 1}`) as Record< + string, + unknown + >; + + expect(Object.getPrototypeOf(parsed)).toBe(Object.prototype); + expect(Object.hasOwn(parsed, "__proto__")).toBe(true); + expect(parsed["__proto__"]).toEqual({ polluted: true }); + expect(Object.keys(parsed)).toEqual(Object.keys(strict)); + expect(({} as Record)["polluted"]).toBeUndefined(); + }); + + test("keeps a nested __proto__ key as an own property", () => { + const parsed = parseJsonc(`{"a": {"__proto__": 1}}`) as { a: Record }; + + expect(Object.getPrototypeOf(parsed.a)).toBe(Object.prototype); + expect(Object.hasOwn(parsed.a, "__proto__")).toBe(true); + }); + + test("reports a 1-based line and column for a syntax error", () => { + const error = syntaxErrorFor(`{\n "a": 1,\n "b": ,\n}`); + + expect(error).toBeInstanceOf(JsoncSyntaxError); + expect(error.line).toBe(3); + expect(error.column).toBe(8); + expect(error.message).toBe(`${error.reason} (line 3, column 8)`); + }); + + test("reports line 1 column 1 for a leading syntax error", () => { + const error = syntaxErrorFor(`}`); + + expect(error.line).toBe(1); + expect(error.column).toBe(1); + }); + + test("never quotes the surrounding source in the error message", () => { + const secret = "sk_live_SUPERSECRET123"; + const error = syntaxErrorFor(`{"token": "${secret}" "other": 1}`); + + expect(error.message).not.toContain(secret); + expect(error.reason).not.toContain(secret); + expect(error.message).toMatch(/\(line 1, column \d+\)$/); + }); + + test("accepts a leading BOM", () => { + expect(parseJsonc(`{"a": 1}`)).toEqual({ a: 1 }); + }); + + test("throws on an empty document", () => { + expect(() => parseJsonc("")).toThrow(JsoncSyntaxError); + expect(() => parseJsonc(" // only a comment\n")).toThrow(JsoncSyntaxError); + }); +}); diff --git a/packages/config/src/paths.ts b/packages/config/src/paths.ts index 0e04e8905c..779a40e30e 100644 --- a/packages/config/src/paths.ts +++ b/packages/config/src/paths.ts @@ -22,19 +22,21 @@ const findConfigInRoot = Effect.fnUntraced(function* (root: string) { const path = yield* Path.Path; const supabaseDir = path.join(root, "supabase"); const jsonPath = path.join(supabaseDir, "config.json"); + const jsoncPath = path.join(supabaseDir, "config.jsonc"); const tomlPath = path.join(supabaseDir, "config.toml"); const jsonExists = yield* probeExists(fs.exists(jsonPath)); + const jsoncExists = yield* probeExists(fs.exists(jsoncPath)); const tomlExists = yield* probeExists(fs.exists(tomlPath)); - if (!jsonExists && !tomlExists) { + if (!jsonExists && !jsoncExists && !tomlExists) { return null; } return { projectRoot: root, supabaseDir, - configPath: jsonExists ? jsonPath : tomlPath, + configPath: jsonExists ? jsonPath : jsoncExists ? jsoncPath : tomlPath, envPath: path.join(supabaseDir, ".env"), envLocalPath: path.join(supabaseDir, ".env.local"), } satisfies CliProjectPaths; diff --git a/packages/config/src/promise-facade.ts b/packages/config/src/promise-facade.ts index 0757dbc04d..872b5fe0f5 100644 --- a/packages/config/src/promise-facade.ts +++ b/packages/config/src/promise-facade.ts @@ -16,9 +16,9 @@ import { CliConfigStore } from "./cli-config.service.ts"; /** * A rejection from `loadCliConfig`, `loadCliConfigFile`, or `saveCliConfig` carries one of - * `CliConfigParseError`, `CliConfigRemovedKeyError`, `DuplicateRemoteProjectIdError`, - * `InvalidRemoteProjectIdError`, `CliProjectEnvParseError`, or a `PlatformError` for a host/OS - * failure — check with `instanceof`. + * `CliConfigParseError`, `CliConfigRemovedKeyError`, `CliConfigFileConflictError`, + * `DuplicateRemoteProjectIdError`, `InvalidRemoteProjectIdError`, `CliProjectEnvParseError`, or a + * `PlatformError` for a host/OS failure — check with `instanceof`. */ export interface CliConfigIo { readonly loadCliConfig: ( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a65f64d186..f5bde7e85c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -579,6 +579,9 @@ importers: dedent: specifier: ^1.7.2 version: 1.7.2 + jsonc-parser: + specifier: ^3.3.1 + version: 3.3.1 smol-toml: specifier: ^1.8.0 version: 1.8.0 @@ -4772,6 +4775,9 @@ packages: json-with-bigint@3.5.12: resolution: {integrity: sha512-uwbF/wSSuOgC7qqlq27Xp5B6a2MHVug3t0idZdTqu0JnlFvgJuH7ju+KAk/J06C7GfhoYy2gnb9wz2INqcne7w==} + jsonc-parser@3.3.1: + resolution: {integrity: sha512-HUgH65KyejrUFPvHFPbqOY0rsFip3Bo5wb4ngvdi1EpCYWUQDC5V+Y7mZws+DLkr4M//zQJoanu1SP+87Dv1oQ==} + jsonfile@6.2.1: resolution: {integrity: sha512-zwOTdL3rFQ/lRdBnntKVOX6k5cKJwEc1HdilT71BWEu7J41gXIB2MRp+vxduPSwZJPWBxEzv4yH1wYLJGUHX4Q==} @@ -10558,6 +10564,8 @@ snapshots: json-with-bigint@3.5.12: {} + jsonc-parser@3.3.1: {} + jsonfile@6.2.1: dependencies: universalify: 2.0.1 From a28a756a52281e2a697b32b4af4d3be2c85eeebc Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 4 Nov 2026 12:21:52 +0000 Subject: [PATCH 2/9] feat(config): keep comments when config pull edits a JSON config (CLI-2860) applyConfigEdits now edits config.json and config.jsonc in place with jsonc-parser's modify instead of re-serialising the whole document, so config pull works on JSON configs with comments or trailing commas and leaves untouched text alone. A new key in an existing multi-line object goes after the previous property's line, so a trailing comment stays with its own key and the file's trailing-comma style carries over. The re-parse verification gate is unchanged. ADR 0023 gains an addendum. --- .../src/commands/config/pull/SIDE_EFFECTS.md | 59 +++---- .../src/commands/config/pull/pull.handler.ts | 3 +- .../config/pull/pull.integration.test.ts | 20 +++ ...ull-write-strategy-and-scope-resolution.md | 2 + packages/config/src/config-edit.ts | 159 +++++++++++++----- packages/config/src/config-edit.unit.test.ts | 94 +++++++++++ 6 files changed, 266 insertions(+), 71 deletions(-) diff --git a/apps/cli/src/commands/config/pull/SIDE_EFFECTS.md b/apps/cli/src/commands/config/pull/SIDE_EFFECTS.md index 26debe46b6..bb32407cd6 100644 --- a/apps/cli/src/commands/config/pull/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/config/pull/SIDE_EFFECTS.md @@ -3,7 +3,7 @@ Target: hosted, no consent — see [Command target](../../../../docs/command-target.md) Writes the effective configuration of a remote project or branch back into the local -`supabase/config.toml`/`config.json` — the write side of `supabase config diff` (same target +`supabase/config.toml`/`config.json`/`config.jsonc` — the write side of `supabase config diff` (same target resolution, fetch, and classification, `../diff/`). Prompts for confirmation before writing on an interactive TTY, unless `--yes` is set; `--output-format json|stream-json` never prompts at all and returns the confirmation's default value without reading anything. A non-interactive TEXT run (no @@ -13,23 +13,23 @@ declining any other answer. Never writes on `--dry-run`, on a declined prompt, o ## Files Read -| Path | Format | When | -| --------------------------------------------------------- | ------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `/supabase/config.toml` or `config.json` | TOML/JSON | always, before any network call (the config load probes `config.json` first, then `config.toml` — a missing file or parse error aborts, exit 1, naming whichever file actually failed); this first load applies NO `[remotes.*]` overlay, regardless of the eventual target. Its own `LoadedCliConfig.rawText` (the exact bytes the loader itself parsed) is taken as this pull's baseline — never re-read separately, so a concurrent edit landing between the parse and a second read can no longer become the accepted baseline out from under the plan | -| `/supabase/config.toml` or `config.json` | TOML/JSON | re-loaded WITH the `[remotes.*]` overlay applied, only when the resolved target ref matches an EXISTING block (block reuse selects it as the destination, `remoteNameForProjectRef`) — a brand-new block has nothing to overlay yet, so this second load is skipped in that case | -| same file, raw on-disk text (TOCTOU re-read) | TOML/JSON | immediately before writing, once the confirmation prompt has been answered — only reached when at least one change is planned and the run is not `--dry-run`; bytes differing from the earlier read abort the write (`ConfigPullFileChangedError`) instead of overwriting a file that changed while the prompt was on screen | -| `/{supabase/,}.env{..local,.local,.,}` | dotenv | always: supplies `env(VAR)` references inside `config.toml`; a `SUPABASE_*` value in these files is not compared (pull compares the file's own declared values). `` is `SUPABASE_ENV` (default `development`); `.env.local` is skipped when `SUPABASE_ENV=test`; the first file to set a key wins and the `supabase/` tier beats the project-root tier | -| `/supabase/.temp/project-ref` | plain text | project-ref fallback (`--remote=` → `SUPABASE_PROJECT_ID` → this file); parent-ref candidate for a branch-name `--remote` (checked eagerly, BEFORE any spinner or branch lookup) | -| `/supabase/.temp/linked-project.json` | JSON | parent-ref candidate for a branch-name `--remote` (same eager pre-check); existence-checked for the telemetry cache write below | -| `~/.supabase/access-token` | plain text (token string) | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | +| Path | Format | When | +| ----------------------------------------------------------------- | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/supabase/config.toml`, `config.json` or `config.jsonc` | TOML/JSON | always, before any network call (the config load probes `config.json` or `config.jsonc` first (both present is an error), then `config.toml` — a missing file or parse error aborts, exit 1, naming whichever file actually failed); this first load applies NO `[remotes.*]` overlay, regardless of the eventual target. Its own `LoadedCliConfig.rawText` (the exact bytes the loader itself parsed) is taken as this pull's baseline — never re-read separately, so a concurrent edit landing between the parse and a second read can no longer become the accepted baseline out from under the plan | +| `/supabase/config.toml`, `config.json` or `config.jsonc` | TOML/JSON | re-loaded WITH the `[remotes.*]` overlay applied, only when the resolved target ref matches an EXISTING block (block reuse selects it as the destination, `remoteNameForProjectRef`) — a brand-new block has nothing to overlay yet, so this second load is skipped in that case | +| same file, raw on-disk text (TOCTOU re-read) | TOML/JSON/JSONC | immediately before writing, once the confirmation prompt has been answered — only reached when at least one change is planned and the run is not `--dry-run`; bytes differing from the earlier read abort the write (`ConfigPullFileChangedError`) instead of overwriting a file that changed while the prompt was on screen | +| `/{supabase/,}.env{..local,.local,.,}` | dotenv | always: supplies `env(VAR)` references inside `config.toml`; a `SUPABASE_*` value in these files is not compared (pull compares the file's own declared values). `` is `SUPABASE_ENV` (default `development`); `.env.local` is skipped when `SUPABASE_ENV=test`; the first file to set a key wins and the `supabase/` tier beats the project-root tier | +| `/supabase/.temp/project-ref` | plain text | project-ref fallback (`--remote=` → `SUPABASE_PROJECT_ID` → this file); parent-ref candidate for a branch-name `--remote` (checked eagerly, BEFORE any spinner or branch lookup) | +| `/supabase/.temp/linked-project.json` | JSON | parent-ref candidate for a branch-name `--remote` (same eager pre-check); existence-checked for the telemetry cache write below | +| `~/.supabase/access-token` | plain text (token string) | when `SUPABASE_ACCESS_TOKEN` unset and keyring unavailable | ## Files Written -| Path | Format | When | -| ------------------------------------------------- | --------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `/supabase/config.toml` or `config.json` | TOML/JSON | ONLY after the confirmation prompt answers "yes" — `--yes`/`SUPABASE_YES` always answer yes, UNLESS the git dirty guard already aborted first (see Git below — `--yes` never bypasses that guard, on any TTY); otherwise the prompt's default is yes, UNLESS the git dirty guard downgraded it to no (only possible in an interactive TTY text run with no `--yes`) — AND there is at least one value change planned OR a new `[remotes.*]` block would be created (a zero-drift branch/`--remote-label` target still writes the block's own `project_id`, even with no value changes to apply — see Notes). **Never** on `--dry-run`; **never** on a declined prompt; **never** on any error path (including the TOCTOU mismatch above). Written atomically: a temp file in the SAME directory (`.tmp..<6-hex-chars>`), the original file's mode copied onto it, then `rename`d over the original; the temp file is removed if anything fails before the rename | -| `/supabase/.temp/linked-project.json` | JSON | `Effect.ensuring` after run (success **and** failure), if a target ref resolved | -| `~/.supabase/telemetry.json` | JSON | `Effect.ensuring` after run (success **and** failure) | +| Path | Format | When | +| ----------------------------------------------------------------- | --------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `/supabase/config.toml`, `config.json` or `config.jsonc` | TOML/JSON | ONLY after the confirmation prompt answers "yes" — `--yes`/`SUPABASE_YES` always answer yes, UNLESS the git dirty guard already aborted first (see Git below — `--yes` never bypasses that guard, on any TTY); otherwise the prompt's default is yes, UNLESS the git dirty guard downgraded it to no (only possible in an interactive TTY text run with no `--yes`) — AND there is at least one value change planned OR a new `[remotes.*]` block would be created (a zero-drift branch/`--remote-label` target still writes the block's own `project_id`, even with no value changes to apply — see Notes). **Never** on `--dry-run`; **never** on a declined prompt; **never** on any error path (including the TOCTOU mismatch above). Written atomically: a temp file in the SAME directory (`.tmp..<6-hex-chars>`), the original file's mode copied onto it, then `rename`d over the original; the temp file is removed if anything fails before the rename | +| `/supabase/.temp/linked-project.json` | JSON | `Effect.ensuring` after run (success **and** failure), if a target ref resolved | +| `~/.supabase/telemetry.json` | JSON | `Effect.ensuring` after run (success **and** failure) | ## Git @@ -63,16 +63,16 @@ locally via the file write above). ## Environment Variables -| Variable | Purpose | Required? | -| ---------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `SUPABASE_PROJECT_ID` | project ref (`--remote=` → this → `.temp/project-ref` → prompt) | no | -| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `~/.supabase/access-token`) | -| `SUPABASE_PROFILE` | API profile selection | no | -| `SUPABASE_WORKDIR` | working directory `config.toml`/`.json` is read from and written to (`--workdir` flag takes priority) | no — when unset, the CLI walks up from cwd looking for `supabase/config.toml` or `supabase/config.json`; when SET (flag or env) the directory is used exactly as given and **no ancestor is searched**, so a path with no `supabase/` of its own fails (with no `supabase init` hint — see the exit-code table) instead of loading a parent project's config | -| `SUPABASE_` config overrides | not applied: pull compares the file's own declared values, with the matched `[remotes.*]` block merged over the base and `env(VAR)` references resolved from the shell and project `.env*` files | no | -| `SUPABASE_YES` | answers the confirmation prompt "yes" (same effect as `--yes`); does **not** bypass the uncommitted-changes guard — only `--force` does | no | -| `env(VAR)` references | interpolated into `config.toml` values at load; a change whose LOCAL value resolved from `env()` is always skipped, never overwritten | no | -| `env(VAR)` references | a change whose REMOTE value is itself spelled `env(VAR)` is also always skipped (`remote_env_reference`), never written verbatim — the loader would resolve it against THIS machine's environment on the next load, which would otherwise let a remote value smuggle a local secret into `config diff`/`config push` | no | +| Variable | Purpose | Required? | +| ---------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `SUPABASE_PROJECT_ID` | project ref (`--remote=` → this → `.temp/project-ref` → prompt) | no | +| `SUPABASE_ACCESS_TOKEN` | auth token (bypasses credential file/keyring lookup) | no (falls back to keyring → `~/.supabase/access-token`) | +| `SUPABASE_PROFILE` | API profile selection | no | +| `SUPABASE_WORKDIR` | working directory `config.toml`/`.json`/`.jsonc` is read from and written to (`--workdir` flag takes priority) | no — when unset, the CLI walks up from cwd looking for `supabase/config.toml`, `supabase/config.json` or `supabase/config.jsonc`; when SET (flag or env) the directory is used exactly as given and **no ancestor is searched**, so a path with no `supabase/` of its own fails (with no `supabase init` hint — see the exit-code table) instead of loading a parent project's config | +| `SUPABASE_` config overrides | not applied: pull compares the file's own declared values, with the matched `[remotes.*]` block merged over the base and `env(VAR)` references resolved from the shell and project `.env*` files | no | +| `SUPABASE_YES` | answers the confirmation prompt "yes" (same effect as `--yes`); does **not** bypass the uncommitted-changes guard — only `--force` does | no | +| `env(VAR)` references | interpolated into `config.toml` values at load; a change whose LOCAL value resolved from `env()` is always skipped, never overwritten | no | +| `env(VAR)` references | a change whose REMOTE value is itself spelled `env(VAR)` is also always skipped (`remote_env_reference`), never written verbatim — the loader would resolve it against THIS machine's environment on the next load, which would otherwise let a remote value smuggle a local secret into `config diff`/`config push` | no | ## Exit Codes @@ -85,7 +85,7 @@ exit `0`: neither is a failure. | `0` | success — including "no differences found", "differences found but none writable", and a declined confirmation prompt | | `1` | the `-o`/`--output` global flag passed (any value — not supported by this command) | | `1` | resolved `--workdir`/`SUPABASE_WORKDIR` doesn't exist or isn't a directory (`ConfigPullWorkdirError`) — beats the base config load and every network call | -| `1` | missing or malformed `supabase/config.toml`/`config.json` (`ConfigPullLoadConfigError`) — a missing file suggests `supabase init` only for a DEFAULTED workdir; with an explicit workdir the message names the resolved path instead and never suggests `init` | +| `1` | missing or malformed `supabase/config.toml`/`config.json`/`config.jsonc` (`ConfigPullLoadConfigError`) — a missing file suggests `supabase init` only for a DEFAULTED workdir; with an explicit workdir the message names the resolved path instead and never suggests `init` | | `1` | a flag, `SUPABASE_*` variable, project `.env*` file or config value fails to decode (`CliConfigValueError`) — names the source that supplied it; surfaced as-is, not wrapped in the command's load error | | `1` | branch-name `--remote` with no linked parent project (`LinkBranchNotLinkedError`) | | `1` | branch-name `--remote` with a corrupt/invalid linked parent ref (`LinkParentRefInvalidError`) | @@ -191,7 +191,7 @@ the -o/--output flag is not supported by config pull; use --output-format json|s ## Notes - **No hosted-mutation consent.** Pull changes only local files, so the one prompt is the local overwrite question above; `--yes` and `SUPABASE_YES` answer it, and nothing about the linked project is ever modified. -- Run from the project root (or pass `--workdir`); `config.toml`/`config.json` is read and written +- Run from the project root (or pass `--workdir`); `config.toml`/`config.json`/`config.jsonc` is read and written relative to it. - **Scope resolution (ADR 0023) is pure and precedes any network call**: `--remote-label`, when passed, is resolved FIRST — even for a ref-shaped target — so its own remedy is never dead; @@ -204,8 +204,9 @@ the -o/--output flag is not supported by config pull; use --output-format json|s rewritten — hard error instead (see the exit-code table) — unless an explicit `--remote-label` already settled the destination some other way. - **The write is never a full-document regeneration.** `packages/config/src/config-edit.ts`'s - `applyConfigEdits` is a character-level scanner that splices only the spans that changed — - comments, key ordering, and quoting elsewhere in the file are untouched — verified by re-parsing + `applyConfigEdits` splices only the spans that changed (a character-level scanner for TOML, + `jsonc-parser` `modify` for JSON/JSONC) — comments, trailing commas, key ordering, and quoting + elsewhere in the file are untouched — verified by re-parsing the edited text and deep-comparing it against an independently computed expectation before it is ever returned; a mismatch refuses the edit rather than writing anything. - **Convergence**: a second `config pull` against an ALREADY-TRACKED target (root, or a diff --git a/apps/cli/src/commands/config/pull/pull.handler.ts b/apps/cli/src/commands/config/pull/pull.handler.ts index abb27add44..e5bf0a86d1 100644 --- a/apps/cli/src/commands/config/pull/pull.handler.ts +++ b/apps/cli/src/commands/config/pull/pull.handler.ts @@ -35,7 +35,8 @@ import type { ConfigPullFlags } from "./pull.command.ts"; /** * `config pull` writes a remote project or branch's configuration into `supabase/config.toml`/ - * `.json` (the config root, or an existing/new `[remotes.*]` block) after a confirmation prompt. + * `.json`/`.jsonc` (the config root, or an existing/new `[remotes.*]` block) after a confirmation + * prompt. * Its target resolution, fetch, and classify steps mirror `config diff`'s, up to the point where * the two commands diverge. The plan/apply run-core is hoisted to * `command-internal/config-pull-run.ts` so the `supabase pull` orchestrator can reuse it. diff --git a/apps/cli/src/commands/config/pull/pull.integration.test.ts b/apps/cli/src/commands/config/pull/pull.integration.test.ts index aad7130413..103950b6a6 100644 --- a/apps/cli/src/commands/config/pull/pull.integration.test.ts +++ b/apps/cli/src/commands/config/pull/pull.integration.test.ts @@ -2115,6 +2115,26 @@ describe("config pull integration", () => { }, ); + it.effect( + "a commented config.jsonc with a trailing comma keeps its bytes except the drifted value", + () => { + const before = `{ + // local overrides + "project_id": "test", + "api": { + "max_rows": 500, /* rows per page */ + }, +} +`; + const { layer } = setup({ yes: true }); + return Effect.gen(function* () { + yield* writeProjectFile("config.jsonc", before); + yield* configPull(noFlags); + expect(yield* readProjectFile("config.jsonc")).toBe(before.replace("500", "1000")); + }).pipe(Effect.provide(layer)); + }, + ); + it.effect("a branch-lookup transport failure maps to the branch list network error", () => { const { layer } = setup({ toml: 'project_id = "test"\n', branchList: "fail" }); return Effect.gen(function* () { diff --git a/docs/adr/0023-config-pull-write-strategy-and-scope-resolution.md b/docs/adr/0023-config-pull-write-strategy-and-scope-resolution.md index 044a4cdde1..12755ec8c0 100644 --- a/docs/adr/0023-config-pull-write-strategy-and-scope-resolution.md +++ b/docs/adr/0023-config-pull-write-strategy-and-scope-resolution.md @@ -22,6 +22,8 @@ The editor's refusal surface is an explicit, closed union rather than a best-eff JSON configs are handled differently: they are re-serialized rather than span-edited — parsed, mutated with the same `deepSet`, and `JSON.stringify`d back out with the file's detected indent and original key order (new keys appended) preserved, with the same re-parse verification gate. JSON has no comments and no meaningful quoting-style variance, so "preserve the file's own terms" holds in substance without needing span-level surgery. In both formats, a multi-line array's value is rewritten single-line when it is replaced — only the value span changes, never the surrounding key or comment text. +**Addendum (2026-11-04): JSON and JSONC edits are span-edited too.** Targeted edits to `config.json` and `config.jsonc` now go through `jsonc-parser`'s `modify`/`applyEdits` using the file's own detected indent and newline, so comments, trailing commas, and untouched values keep their bytes. The formatter lays out the value it writes (an array gets one element per line) and may re-indent the line a new key is inserted next to. The re-parse verification gate is unchanged and parses with the same JSONC parser. This supersedes the re-serialization paragraph above. Full saves (`saveCliConfig`) still regenerate the file. + ### (b) Scope resolution: narrowest scope, block reuse as the primary signal `config pull` writes at the narrowest scope that affects only the resolved target, in this rule order: diff --git a/packages/config/src/config-edit.ts b/packages/config/src/config-edit.ts index 5560e49f63..a42b7e2a49 100644 --- a/packages/config/src/config-edit.ts +++ b/packages/config/src/config-edit.ts @@ -1,14 +1,23 @@ +import { + applyEdits as applyJsonEditsToText, + findNodeAtLocation, + modify, + parseTree, + type FormattingOptions, +} from "jsonc-parser"; import * as SmolToml from "smol-toml"; import type { ConfigFormat } from "./config-format.ts"; +import { parseJsonc } from "./jsonc.ts"; /** - * Format-preserving surgical editor for `supabase/config.{toml,json}`. Pure and synchronous: + * Format-preserving surgical editor for `supabase/config.{toml,json,jsonc}`. Pure and synchronous: * callers own reading the current file text and writing the returned text back out atomically. * Every edited document is re-parsed and deep-compared against the original before being * returned; any mismatch refuses rather than shipping wrong bytes. An edit either replaces an * existing value's span in place or inserts a new line/table — nothing else is touched, so - * untouched comments, spacing, and quoting survive byte-for-byte. See ADR 0023 for the design - * rationale and rejected alternatives. + * untouched comments, spacing, and quoting survive byte-for-byte. JSON/JSONC documents are + * span-edited through `jsonc-parser`'s `modify`, keeping comments and trailing commas. See ADR + * 0023 for the design rationale and rejected alternatives. */ export type ConfigEditValue = @@ -63,9 +72,8 @@ function isRemotesLabelRoot(path: ReadonlyArray): boolean { return path.length === 2 && path[0] === REMOTES_TABLE_NAME; } -// Duplicated from `lib/env.ts` rather than imported: this module is restricted to `smol-toml` -// as its only import, so it stays independently embeddable without pulling in this package's -// wider env-resolution graph. +// Duplicated from `lib/env.ts` rather than imported so this module avoids pulling in this +// package's wider env-resolution graph. const ENV_CAPTURE_REGEX = /^env\((.*)\)$/; function isPlainRecord(value: unknown): value is Readonly> { @@ -180,26 +188,14 @@ function deepSetOne(root: unknown, path: ReadonlyArray, value: unknown): return { ...base, [head]: deepSetOne(existingChild, rest, value) }; } -/** Applies one `path`/`value` pair per entry on top of `root`, immutably. Shared by `deepSet` - * (the verification oracle, built from the original un-flattened edits) and the JSON arm's - * actual per-leaf mutation (built from `flattenEdits`'s output) — the two intentionally use - * different entry sets so an object-valued edit's write only touches leaves it mentions, while - * verification still catches a dropped sibling. */ -function applyPathEntries( - root: unknown, - entries: ReadonlyArray<{ readonly path: ReadonlyArray; readonly value: unknown }>, -): unknown { - return entries.reduce( - (accumulator, entry) => deepSetOne(accumulator, entry.path, entry.value), - root, - ); -} - /** Applies every edit's `path`/`value` on top of `root`, immutably, replacing the whole * destination subtree for an object-valued edit. This is the expected-value side of mandatory - * verification for both formats — see `applyPathEntries` for the actual per-leaf mutation. */ + * verification for both formats. */ function deepSet(root: unknown, edits: ReadonlyArray): unknown { - return applyPathEntries(root, edits); + return edits.reduce( + (accumulator, edit) => deepSetOne(accumulator, edit.path, edit.value), + root, + ); } function describeError(cause: unknown): string { @@ -1193,19 +1189,95 @@ function detectJsonNewline(source: string): "\n" | "\r\n" { return source.includes("\r\n") ? "\r\n" : "\n"; } -function detectJsonIndent(source: string): string | number { +function detectJsonFormatting(source: string): FormattingOptions { const match = /\n([ \t]+)\S/.exec(source); - if (match === null) { - return 2; + const indent = match?.[1] ?? " "; + const usesTabs = indent.includes("\t"); + return { + insertSpaces: !usesTabs, + tabSize: usesTabs ? 1 : indent.length, + eol: detectJsonNewline(source), + }; +} + +function lineEndAfterTrailer( + text: string, + from: number, +): { end: number; hadComma: boolean } | null { + let pos = from; + let hadComma = false; + for (;;) { + const ch = text[pos]; + if (ch === " " || ch === "\t") { + pos++; + } else if (ch === "," && !hadComma) { + hadComma = true; + pos++; + } else if (ch === "/" && text[pos + 1] === "*") { + const close = text.indexOf("*/", pos + 2); + if (close === -1 || /[\r\n]/.test(text.slice(pos, close))) { + return null; + } + pos = close + 2; + } else if (ch === "/" && text[pos + 1] === "/") { + while (pos < text.length && text[pos] !== "\r" && text[pos] !== "\n") { + pos++; + } + } else if (ch === undefined || ch === "\r" || ch === "\n") { + return { end: pos, hadComma }; + } else { + return null; + } } - const indent = match[1] ?? ""; - return indent.includes("\t") ? "\t" : indent.length; +} + +/** `modify` inserts right after the previous value and reformats that line, which moves a + * same-line trailing comment onto the new key; this appends after the line instead. */ +function insertAfterLastProperty( + text: string, + leaf: LeafEdit, + formatting: FormattingOptions, +): string | undefined { + const root = parseTree(text, undefined, { allowTrailingComma: true }); + if (root === undefined) { + return undefined; + } + let depth = leaf.path.length; + let parent = findNodeAtLocation(root, [...leaf.path]); + while (parent === undefined && depth > 0) { + depth--; + parent = findNodeAtLocation(root, leaf.path.slice(0, depth)); + } + const prev = parent?.children?.[parent.children.length - 1]; + if (parent?.type !== "object" || depth === leaf.path.length || prev === undefined) { + return undefined; + } + const prevEnd = prev.offset + prev.length; + const trailer = lineEndAfterTrailer(text, prevEnd); + if (trailer === null) { + return undefined; + } + const eol = formatting.eol ?? "\n"; + const lineStart = Math.max(text.lastIndexOf("\n", prev.offset), -1) + 1; + const baseIndent = /^[ \t]*/.exec(text.slice(lineStart))?.[0] ?? ""; + const [key, ...rest] = leaf.path.slice(depth); + const nested = rest.reduceRight((value, segment) => ({ [segment]: value }), leaf.value); + const indentUnit = formatting.insertSpaces ? " ".repeat(formatting.tabSize ?? 2) : "\t"; + const rendered = JSON.stringify(nested, null, indentUnit).split("\n").join(`${eol}${baseIndent}`); + const insertion = `${eol}${baseIndent}${JSON.stringify(key)}: ${rendered}${trailer.hadComma ? "," : ""}`; + return ( + text.slice(0, prevEnd) + + (trailer.hadComma ? "" : ",") + + text.slice(prevEnd, trailer.end) + + insertion + + text.slice(trailer.end) + ); } function applyJsonEdits(source: string, edits: ReadonlyArray): ConfigEditOutcome { let parsed: unknown; try { - parsed = JSON.parse(source); + parsed = parseJsonc(source); } catch (cause) { return refused("parse_error", [], describeError(cause)); } @@ -1229,24 +1301,29 @@ function applyJsonEdits(source: string, edits: ReadonlyArray): Confi } } - const indent = detectJsonIndent(source); - const nextValue = applyPathEntries(parsed, leaves); - const rendered = JSON.stringify(nextValue, null, indent); - // `JSON.stringify` always renders `\n`, so a CRLF source needs its newline flavor restored - // post-render — otherwise every edit to a CRLF JSON config would silently flip it to LF. - const newline = detectJsonNewline(source); - const withNewlineFlavor = newline === "\r\n" ? rendered.replaceAll("\n", "\r\n") : rendered; - const nextText = source.endsWith("\n") ? `${withNewlineFlavor}${newline}` : withNewlineFlavor; + const formattingOptions = detectJsonFormatting(source); + let nextText = source; + for (const leaf of leaves) { + try { + nextText = + insertAfterLastProperty(nextText, leaf, formattingOptions) ?? + applyJsonEditsToText( + nextText, + modify(nextText, [...leaf.path], leaf.value, { formattingOptions }), + ); + } catch (cause) { + return refused("verification_mismatch", leaf.path, describeError(cause)); + } + } let reparsed: unknown; try { - reparsed = JSON.parse(nextText); + reparsed = parseJsonc(nextText); } catch { return refused("verification_mismatch", [], "edited document failed to re-parse"); } - // Verified against `deepSet` (the original, un-flattened `edits`), not `nextValue` (the - // flattened per-leaf mutation) — comparing against the mutation would be tautological, so a - // sibling an object-valued edit silently drops would never surface as a mismatch. + // Verified against `deepSet` (the original, un-flattened `edits`) so a sibling an object-valued + // edit would silently drop surfaces as a mismatch. const expected = deepSet(parsed, edits); if (!deepEqualValue(reparsed, expected)) { return refused( diff --git a/packages/config/src/config-edit.unit.test.ts b/packages/config/src/config-edit.unit.test.ts index 13fef19d00..885031edfc 100644 --- a/packages/config/src/config-edit.unit.test.ts +++ b/packages/config/src/config-edit.unit.test.ts @@ -724,6 +724,100 @@ describe("applyConfigEdits (json)", () => { } } } +`); + }); + + test("replacing a scalar in a commented JSONC source changes only that value", () => { + const source = `{ + // local overrides + "$schema": "s", + /* api settings */ + "api": { + "max_rows": 1000, // keep small + "port": 5, + }, +} +`; + const outcome = applyConfigEdits(source, "json", [{ path: ["api", "max_rows"], value: 500 }]); + expect(applied(outcome).text).toBe(source.replace("1000", "500")); + expect(applied(outcome).applied).toEqual([ + { path: ["api", "max_rows"], action: "replaced", createdTables: [] }, + ]); + }); + + test("inserting keys into a commented JSONC source keeps every comment and trailing comma", () => { + const source = `{ + // local overrides + "api": { + // dev port + "port": 5, + }, + /* auth settings */ + "db": { + "port": 6, + }, +} +`; + const outcome = applyConfigEdits(source, "json", [ + { path: ["api", "max_rows"], value: 500 }, + { path: ["auth", "site_url"], value: "http://x" }, + ]); + expect(applied(outcome).text).toBe(`{ + // local overrides + "api": { + // dev port + "port": 5, + "max_rows": 500, + }, + /* auth settings */ + "db": { + "port": 6, + }, + "auth": { + "site_url": "http://x" + }, +} +`); + expect(applied(outcome).applied).toEqual([ + { path: ["api", "max_rows"], action: "inserted", createdTables: [] }, + { path: ["auth", "site_url"], action: "inserted", createdTables: [] }, + ]); + }); + + test("inserting a sibling after a property without a trailing comma keeps its inline comment on that line", () => { + const source = `{ + "api": { + "port": 5 // dev port + } +} +`; + const outcome = applyConfigEdits(source, "json", [{ path: ["api", "max_rows"], value: 500 }]); + expect(applied(outcome).text).toBe(`{ + "api": { + "port": 5, // dev port + "max_rows": 500 + } +} +`); + expect(applied(outcome).applied).toEqual([ + { path: ["api", "max_rows"], action: "inserted", createdTables: [] }, + ]); + }); + + test("inserting a sibling after a trailing-comma property leaves its inline array and comment untouched", () => { + const source = `{ + "api": { + "schemas": ["public"], // exposed + } +} +`; + const outcome = applyConfigEdits(source, "json", [{ path: ["api", "max_rows"], value: 500 }]); + expect(applied(outcome).text).toBe(`{ + "api": { + "schemas": ["public"], // exposed + "max_rows": 500, + } +} `); }); }); From 5f47176fd735a6693b02fb3f3a8938b4077ebc3e Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Wed, 4 Nov 2026 12:39:31 +0000 Subject: [PATCH 3/9] feat(config): keep comments on every JSON config write (CLI-2861) saveCliConfig now applies minimal jsonc-parser edits when it saves over an existing config.json or config.jsonc, removing dropped keys by their whole line and keeping strict JSON strict, and falls back to the regenerated text when the edited file does not re-parse to the saved config. A TOML save keeps the file's #:schema directive. init treats an existing config.json or config.jsonc as an existing config and names it when it refuses; init --force and a blank bootstrap remove them when writing the fresh config.toml. compute new records a compute in a JSON config through applyConfigEdits instead of refusing, so the unused ComputeJsonConfigUnsupportedError is removed. --- .../src/commands/bootstrap/SIDE_EFFECTS.md | 2 +- .../experimental/compute/compute.shared.ts | 24 +--- .../experimental/compute/new/SIDE_EFFECTS.md | 47 ++++---- .../experimental/compute/new/new.handler.ts | 17 +-- .../compute/new/new.integration.test.ts | 109 +++++------------- apps/cli/src/commands/init/SIDE_EFFECTS.md | 24 ++-- apps/cli/src/commands/init/init.handler.ts | 11 +- .../commands/init/init.integration.test.ts | 60 ++++++++++ apps/cli/src/shared/compute/compute-config.ts | 70 ++++++----- apps/cli/src/shared/compute/compute.errors.ts | 12 -- apps/cli/src/shared/init/project-init.ts | 25 ++-- .../telemetry/__fixtures__/error-tags.txt | 1 - ...ull-write-strategy-and-scope-resolution.md | 2 +- packages/config/docs/cli-config-loading.md | 5 + packages/config/src/config-edit.ts | 84 +++++++++++++- packages/config/src/io.ts | 22 +++- packages/config/src/io.unit.test.ts | 95 +++++++++++++++ 17 files changed, 411 insertions(+), 199 deletions(-) diff --git a/apps/cli/src/commands/bootstrap/SIDE_EFFECTS.md b/apps/cli/src/commands/bootstrap/SIDE_EFFECTS.md index b3c44513b1..760326b937 100644 --- a/apps/cli/src/commands/bootstrap/SIDE_EFFECTS.md +++ b/apps/cli/src/commands/bootstrap/SIDE_EFFECTS.md @@ -29,7 +29,7 @@ command — see Notes). | Path | Format | When | | ----------------------------------------------------------------------------------------------------- | ---------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `/supabase/config.toml` | TOML | blank/`scratch` path only (via `initProject`) | +| `/supabase/config.toml` | TOML | blank/`scratch` path only (via `initProject` with `force`, which also removes `supabase/config.json`/`config.jsonc`) | | `/