Close correctness gaps against Renovate and cut scope

README.md

@@ -63,7 +63,7 @@ A failed lookup is reported on its own, does not hold back the other results and
 
 ## Config File
 
-Configure via `updates.config.{ts,js,mjs,mts}` in your repo root. CLI arguments win over it, except `include`, `exclude` and `pin`, which merge.
+Configure via `updates.config.{ts,js,mjs,mts}`. Each manifest uses the nearest config above it, and workspace members use the workspace root config. CLI arguments win over configured values, including `include`, `exclude` and `pin`.
 
 ```ts
 import type {Config} from "updates";
@@ -103,7 +103,7 @@ export default {
 
 ### Renovate config
 
-A [Renovate](https://docs.renovatebot.com/) config is picked up automatically, inheriting `ignoreDeps`, `enabled` and `allowedVersions` in `packageRules` as `include`/`exclude`/`pin`. `minimumReleaseAge` is not inherited unless opted in:
+A [Renovate](https://docs.renovatebot.com/) `renovate.json` is picked up automatically, inheriting `ignoreDeps`, `enabled` and `allowedVersions` from matching `packageRules` as `include`/`exclude`/`pin`. Exact-name `allowedVersions` ranges become pin ceilings that never downgrade. Configs using `extends` are rejected. `minimumReleaseAge` is not inherited unless opted in:
 
 ```ts
 export default {

cli.test.ts

@@ -0,0 +1,17 @@
+import {parseCliArgs} from "./cli.ts";
+
+test("recovers swallowed short option clusters", () => {
+  const single = parseCliArgs(["-T", "-u", "package.json"]);
+  expect(single.args).toMatchObject({timeout: true, update: true});
+  expect(single.positionals).toEqual(["package.json"]);
+
+  const clustered = parseCliArgs(["-T", "-uj", "package.json"]);
+  expect(clustered.args).toMatchObject({timeout: true, update: true, json: true});
+  expect(clustered.positionals).toEqual(["package.json"]);
+
+  const {args, positionals} = parseCliArgs(["-i", "-ug", "react", "package.json"]);
+  expect(args.include).toEqual([]);
+  expect(args.update).toBe(true);
+  expect(args.greatest).toEqual(["react"]);
+  expect(positionals).toEqual(["package.json"]);
+});

cli.ts

@@ -2,8 +2,7 @@ import {cwd} from "node:process";
 import {parseArgs} from "node:util";
 import {dirname, isAbsolute, resolve} from "node:path";
 import {statSync} from "node:fs";
-import {options, parseMixedArg, getOptionKey, parseArgList, parsePinArg, loadConfig} from "./config.ts";
-import {fetchTimeout} from "./modes/shared.ts";
+import {cliBaseConfig, options, parseMixedArg, getOptionKey, parseArgList, parsePinArg, loadConfig} from "./config.ts";
 import {parsePositiveInt} from "./utils/utils.ts";
 import type {Arg} from "./config.ts";
 import type {UpdatesOptions} from "./api.ts";
@@ -27,16 +26,12 @@ function deriveStartDir(first: string | undefined): string {
   return isDir ? abs : dirname(abs);
 }
 
-// Flatten -f/--file plus positionals into the target list, and derive the
-// directory config discovery walks up from. Shared by the binary's prewarm
-// path and resolveConfig, which both need it before any config is loaded.
 export function resolveFileArgs(args: Record<string, Arg>, positionals: Array<string>): {filesList: Array<string>, startDir: string} {
   const fileSet = parseMixedArg(args.file);
   const filesList = [...(fileSet instanceof Set ? fileSet : []), ...positionals];
   return {filesList, startDir: deriveStartDir(filesList[0])};
 }
 
-// Parse argv into option values, fixing the parseArgs "-a -b" → {a: "-b"} defect.
 export function parseCliArgs(argv?: Array<string>): {args: Record<string, Arg>, positionals: Array<string>} {
   const result = parseArgs({
     strict: false,
@@ -51,97 +46,110 @@ export function parseCliArgs(argv?: Array<string>): {args: Record<string, Arg>,
   let positionalsSeen = 0;
   for (const [index, token] of result.tokens.entries()) {
     if (token.kind === "positional") positionalsSeen++;
-    // An inline value (`--exclude=-u`, `-i-g`) was written deliberately, so only a separately
-    // parsed one can be a flag parseArgs swallowed.
     if (token.kind !== "option" || token.inlineValue || !token.value?.startsWith("-")) continue;
-    const dashes = token.value.startsWith("--") ? 2 : 1;
-    const key = getOptionKey(token.value.substring(dashes));
-    if (!key) continue;
+    const longOption = token.value.startsWith("--");
     const next = result.tokens[index + 1];
-    // The flag was wrongly swallowed as this option's value; drop only that bogus
-    // value (the dash-prefixed token.value, which may not be the last element)
-    // rather than discarding the whole accumulated array, so other repeats like
-    // `-i react -i -g -i vue` keep both `react` and `vue`.
+    const nextPositional = next?.kind === "positional" ? next.value : undefined;
+    const recoveredOptions: Array<{key: string, value: string | boolean}> = [];
+    const raw = token.value.substring(longOption ? 2 : 1);
+    let consumesPositional = false;
+    if (longOption) {
+      const key = getOptionKey(raw);
+      if (key) {
+        consumesPositional = options[key].type === "string" && nextPositional !== undefined;
+        recoveredOptions.push({key, value: consumesPositional ? nextPositional! : true});
+      }
+    } else {
+      for (let offset = 0; offset < raw.length;) {
+        const key = getOptionKey(raw[offset]);
+        if (!key) { recoveredOptions.length = 0; break; }
+        if (options[key].type === "boolean") {
+          recoveredOptions.push({key, value: true});
+          offset++;
+        } else {
+          const inlineValue = raw.substring(offset + 1);
+          consumesPositional = !inlineValue && nextPositional !== undefined;
+          recoveredOptions.push({
+            key,
+            value: inlineValue || (consumesPositional ? nextPositional! : true),
+          });
+          offset = raw.length;
+        }
+      }
+    }
+    if (!recoveredOptions.length) continue;
     const swallowed = values[token.name];
     if (Array.isArray(swallowed)) {
-      const pos = swallowed.indexOf(token.value);
-      if (pos !== -1) swallowed.splice(pos, 1);
+      const position = swallowed.indexOf(token.value);
+      if (position !== -1) swallowed.splice(position, 1);
     } else {
       values[token.name] = true;
     }
-    const recovered = next?.kind === "positional" && next.value ? next.value : true;
-    // a recovered positional is that option's value, so it must not stay in the file list too
-    if (typeof recovered === "string") consumedPositionals.add(positionalsSeen);
-    if (options[key]?.multiple) {
-      const list = (values[key] ??= []) as Array<string | boolean>;
-      list.push(recovered);
-    } else {
-      // non-multiple options expect a scalar; an array shape is rejected by the typeof string consumers
-      values[key] = recovered;
+    if (consumesPositional) consumedPositionals.add(positionalsSeen);
+    for (const {key, value} of recoveredOptions) {
+      if (options[key].multiple) {
+        const list = (values[key] ??= []) as Array<string | boolean>;
+        list.push(value);
+      } else {
+        values[key] = value;
+      }
     }
   }
 
   return {args: values, positionals: result.positionals.filter((_val, index) => !consumedPositionals.has(index))};
 }
 
-// Overlay parsed CLI args onto the config file. Shared by the binary and tests.
 export async function resolveConfig(
   args: Record<string, Arg>,
   positionals: Array<string>,
 ): Promise<UpdatesOptions> {
   const {filesList, startDir} = resolveFileArgs(args, positionals);
 
-  const cliTimeout = typeof args.timeout === "string" ? parsePositiveInt(args.timeout, "timeout") : undefined;
-
-  const fileConfig = await loadConfig(startDir, {
-    noCache: Boolean(args["no-cache"]),
-    timeout: cliTimeout ?? fetchTimeout,
-  });
-
-  // `pin` is dropped so it reaches the run as a per-directory pin rather than an authored one:
-  // a renovate-inherited ceiling in the global pin would gain the right to downgrade (api.ts).
-  const config: UpdatesOptions = {...fileConfig, pin: undefined};
-  if (args.json) config.json = true;
-  if (args.verbose) config.verbose = true;
-  if (args["no-cache"]) config.noCache = true;
-  if (args.update) config.update = true;
-  if (args.indirect) config.indirect = true;
-  if (args["error-on-outdated"]) config.errorOnOutdated = true;
-  if (args["error-on-unchanged"]) config.errorOnUnchanged = true;
-  // each color flag clears the other so a CLI flag beats both file values, -n applied last so it wins
-  if (args.color) {config.color = true; config.noColor = false;}
-  if (args["no-color"]) {config.color = false; config.noColor = true;}
-  if (cliTimeout !== undefined) config.timeout = cliTimeout;
-  if (typeof args.sockets === "string") config.sockets = parsePositiveInt(args.sockets, "sockets");
-  if (typeof args.registry === "string") config.registry = args.registry;
-  if (typeof args.cooldown === "string") config.cooldown = Number(args.cooldown) || args.cooldown;
+  const fileConfig = await loadConfig(startDir);
+
+  const cliConfig: Partial<UpdatesOptions> = {};
+  if (args.json) cliConfig.json = true;
+  if (args.verbose) cliConfig.verbose = true;
+  if (args["no-cache"]) cliConfig.noCache = true;
+  if (args.update) cliConfig.update = true;
+  if (args.indirect) cliConfig.indirect = true;
+  if (args["error-on-outdated"]) cliConfig.errorOnOutdated = true;
+  if (args["error-on-unchanged"]) cliConfig.errorOnUnchanged = true;
+  if (args.color) {cliConfig.color = true; cliConfig.noColor = false;}
+  if (args["no-color"]) {cliConfig.color = false; cliConfig.noColor = true;}
+  if (typeof args.timeout === "string") cliConfig.timeout = parsePositiveInt(args.timeout, "timeout");
+  if (typeof args.sockets === "string") cliConfig.sockets = parsePositiveInt(args.sockets, "sockets");
+  if (typeof args.registry === "string") cliConfig.registry = args.registry;
+  if (typeof args.cooldown === "string") cliConfig.cooldown = Number(args.cooldown) || args.cooldown;
 
   const cliInclude = parseArgList(args.include).map(cliPatternToRegex);
   const cliExclude = parseArgList(args.exclude).map(cliPatternToRegex);
-  if (cliInclude.length) config.include = cliInclude;
-  if (cliExclude.length) config.exclude = cliExclude;
+  if (cliInclude.length) cliConfig.include = cliInclude;
+  if (cliExclude.length) cliConfig.exclude = cliExclude;
 
   const cliTypes = parseArgList(args.types);
-  if (cliTypes.length) config.types = cliTypes;
+  if (cliTypes.length) cliConfig.types = cliTypes;
 
   const cliPin = parsePinArg(args.pin);
-  if (Object.keys(cliPin).length) config.pin = cliPin;
+  if (Object.keys(cliPin).length) cliConfig.pin = cliPin;
 
   const cliModes = parseMixedArg(args.modes);
-  if (cliModes instanceof Set) config.modes = Array.from(cliModes);
+  if (cliModes instanceof Set) cliConfig.modes = Array.from(cliModes);
 
   for (const key of ["greatest", "prerelease", "release", "patch", "minor"] as const) {
     const val = argToConfigMixed(args[key]);
-    if (val !== undefined) config[key] = val;
+    if (val !== undefined) cliConfig[key] = val;
   }
   const allowDowngrade = argToConfigMixed(args["allow-downgrade"]);
-  if (allowDowngrade !== undefined) config.allowDowngrade = allowDowngrade;
+  if (allowDowngrade !== undefined) cliConfig.allowDowngrade = allowDowngrade;
 
-  if (filesList.length) config.files = filesList;
+  if (filesList.length) cliConfig.files = filesList;
 
   for (const key of ["forgeapi", "pypiapi", "jsrapi", "goproxy", "cargoapi", "dockerapi"] as const) {
-    if (typeof args[key] === "string") config[key] = args[key];
+    if (typeof args[key] === "string") cliConfig[key] = args[key];
   }
 
+  const config: UpdatesOptions = {...fileConfig, pin: undefined, ...cliConfig};
+  Object.defineProperty(config, cliBaseConfig, {value: {fileConfig, cliKeys: Object.keys(cliConfig)}});
   return config;
 }

config.test.ts

@@ -0,0 +1,43 @@
+import {mkdtemp, mkdir, readFile, rm, writeFile} from "node:fs/promises";
+import {tmpdir} from "node:os";
+import {join} from "node:path";
+import {cliConfigBaseDir} from "./api.ts";
+import {cliBaseConfig, loadConfig} from "./config.ts";
+
+test("the package API preserves cliConfigBaseDir", () => {
+  expect(cliConfigBaseDir).toBe(cliBaseConfig);
+});
+
+test("config discovery starts at the target and loads only the highest-priority module", async () => {
+  const dir = await mkdtemp(join(tmpdir(), "updates-config-"));
+  const discoveryDir = join(dir, "discovery");
+  const child = join(discoveryDir, "child");
+  const priorityDir = join(dir, "priority");
+  try {
+    await mkdir(child, {recursive: true});
+    await mkdir(priorityDir);
+    await writeFile(join(discoveryDir, "updates.config.js"), "module.exports = {};\n");
+    await writeFile(join(child, "renovate.json"), JSON.stringify({ignoreDeps: ["child-only"]}));
+    const [excluded] = (await loadConfig(child)).exclude!;
+    expect(excluded).toBeInstanceOf(RegExp);
+    expect((excluded as RegExp).test("child-only")).toBe(true);
+
+    const marker = join(priorityDir, "loaded");
+    await writeFile(join(priorityDir, "updates.config.js"),
+      `require("node:fs").appendFileSync(${JSON.stringify(marker)}, "js\\n"); module.exports = {exclude: ["js"]};\n`);
+    await writeFile(join(priorityDir, "updates.config.mjs"),
+      `import {appendFileSync} from "node:fs"; appendFileSync(${JSON.stringify(marker)}, "mjs\\n"); export default {exclude: ["mjs"]};\n`);
+    expect((await loadConfig(priorityDir)).exclude).toEqual(["js"]);
+    expect(await readFile(marker, "utf8")).toBe("js\n");
+
+    const brokenDir = join(priorityDir, "broken");
+    await mkdir(brokenDir);
+    await writeFile(join(brokenDir, "updates.config.js"), "throw new Error('broken primary');\n");
+    await writeFile(join(brokenDir, "updates.config.mjs"),
+      `import {appendFileSync} from "node:fs"; appendFileSync(${JSON.stringify(marker)}, "broken-mjs\\n"); export default {};\n`);
+    await expect(loadConfig(brokenDir)).rejects.toThrow(/broken primary/);
+    expect(await readFile(marker, "utf8")).toBe("js\n");
+  } finally {
+    await rm(dir, {recursive: true, force: true});
+  }
+});

config.ts

@@ -4,7 +4,8 @@ import {access} from "node:fs/promises";
 import type {ParseArgsOptionsConfig} from "node:util";
 import {validRange} from "./utils/semver.ts";
 import {commaSeparatedToArray, patternToRegex, walkUp, memoizeAsync} from "./utils/utils.ts";
-import type {PresetFetchOptions, RenovateImportOptions} from "./utils/renovate.ts";
+import type {RenovateImportOptions} from "./utils/renovate.ts";
+import {loadRenovateConfig} from "./utils/renovate.ts";
 
 export type Config = {
   /** Array of dependencies to include */
@@ -93,6 +94,7 @@ export type Override = {
 };
 
 export type Arg = string | boolean | Array<string | boolean> | undefined;
+export const cliBaseConfig = Symbol("cliBaseConfig");
 
 export const options: ParseArgsOptionsConfig = {
   "allow-downgrade": {short: "d", type: "string", multiple: true},
@@ -131,7 +133,7 @@ export const options: ParseArgsOptionsConfig = {
 };
 
 export function parseMixedArg(arg: Arg): boolean | Set<string> {
-  if (Array.isArray(arg) && arg.every(a => a === true)) {
+  if (Array.isArray(arg) && arg.every(val => val === true)) {
     return true;
   } else if (Array.isArray(arg)) {
     return new Set(arg.filter(val => typeof val === "string").flatMap(commaSeparatedToArray));
@@ -162,8 +164,6 @@ export function parseArgList(arg: Arg): Array<string> {
   return [];
 }
 
-// An unparsable range satisfies nothing, so dropping it would either discard the pin or freeze the
-// dependency forever. Renovate likewise rejects an allowedVersions it cannot parse.
 export function validatePin(pin: Config["pin"]): void {
   for (const [pkg, range] of Object.entries(pin ?? {})) {
     if (!validRange(range)) throw new Error(`Invalid pin range for ${pkg}: ${range}`);
@@ -173,7 +173,7 @@ export function validatePin(pin: Config["pin"]): void {
 export function parsePinArg(arg: Arg): Record<string, string> {
   const result: Record<string, string> = {};
   for (const val of Array.isArray(arg) ? arg : [arg]) {
-    if (typeof val !== "string") continue; // a flag recovered from a swallowed value arrives as `true`
+    if (typeof val !== "string") continue;
     const eq = val.indexOf("=");
     if (eq < 1) throw new Error(`Invalid pin: ${val}, expected <dep>=<range>`);
     result[val.slice(0, eq)] = val.slice(eq + 1);
@@ -188,51 +188,34 @@ export function configMixedToRegexes(val: boolean | Array<string | RegExp> | und
   return patternsToRegexSet(val);
 }
 
-type FoundConfig = {configDir: string, default: Config};
-
-// Try to load any updates.config.* in dir. Returns the first that imports
-// successfully. If none imports but at least one parsed-and-failed, throws
-// the first parse error so a broken sibling next to a valid one does not
-// block the valid one.
-async function tryLoadInDir(dir: string): Promise<FoundConfig | null> {
-  const exts = ["js", "ts", "mjs", "mts"];
-  const results = await Promise.all(exts.map(async (ext): Promise<FoundConfig | Error | null> => {
+const findConfigUp = memoizeAsync((startDir: string) => walkUp(startDir, async dir => {
+  for (const ext of ["js", "ts", "mjs", "mts"]) {
     const filename = `updates.config.${ext}`;
     const fullPath = join(dir, filename);
     try {
       await access(fullPath);
-    } catch {
-      return null;
+    } catch (err: any) {
+      if (err?.code === "ENOENT") continue;
+      throw new Error(`Unable to load config file ${filename}: ${err?.message ?? err}`);
     }
     try {
       const mod = await import(pathToFileURL(fullPath).href);
-      return {configDir: dir, default: mod.default ?? {}};
+      return mod.default ?? {};
     } catch (err: any) {
-      return new Error(`Unable to parse config file ${filename}: ${err?.message ?? err}`);
+      throw new Error(`Unable to parse config file ${filename}: ${err?.message ?? err}`);
     }
-  }));
-  for (const r of results) if (r && !(r instanceof Error)) return r;
-  for (const r of results) if (r instanceof Error) throw r;
+  }
   return null;
-}
-
-const findConfigUp = memoizeAsync((startDir: string) => walkUp(startDir, tryLoadInDir));
+}));
 
-export async function loadConfig(startDir: string, presetFetch: PresetFetchOptions = {}): Promise<Config> {
-  const found = await findConfigUp(startDir);
-  const raw: Config = found?.default ?? {};
-  const {loadRenovateConfig, makePresetFetcher} = await import("./utils/renovate.ts");
-  const fetchText = makePresetFetcher(presetFetch);
-  const renovateConfig = await loadRenovateConfig(found?.configDir ?? startDir, raw.inherit?.renovate, fetchText);
+export async function loadConfig(startDir: string): Promise<Config> {
+  const raw = await findConfigUp(startDir) ?? {};
+  const renovateConfig = await loadRenovateConfig(startDir, raw.inherit?.renovate);
   const config: Config = {...renovateConfig, ...raw};
-  // `pin` merges per key, so an authored pin for one dependency keeps the ceilings inherited for
-  // the others. An authored entry may downgrade, so the marker keeps only the names renovate owns.
   if (renovateConfig.pin) {
     config.pin = {...renovateConfig.pin, ...raw.pin};
     config.pinNoDowngrade = Object.keys(renovateConfig.pin).filter(name => !raw.pin?.[name]);
   }
-  // Overrides concatenate for the same reason, with the authored ones last so they win the
-  // last-match-wins pass in api.ts rather than discarding what renovate contributed.
   if (renovateConfig.overrides?.length) config.overrides = [...renovateConfig.overrides, ...(raw.overrides ?? [])];
   validatePin(config.pin);
   return config;