fix: review hardening

packages/doba/__snapshots__/tsnapi/index.snapshot.d.ts

@@ -60,6 +60,8 @@ export type IdentifyTransformMeta<Keys extends string = string> = TransformMeta<
   readonly from: Keys;
 };
 export type IdentifyTransformResult<T, Keys extends string = string> = Result<T, readonly DobaIssue[], IdentifyTransformMeta<Keys>>;
+export type MermaidConfig = Readonly<Record<string, MermaidConfigValue>>;
+export type MermaidConfigValue = unknown;
 export type MigrationDef<FromValue, ToValue, Keys extends string = string, FromKey extends Keys = Keys, ToKey extends Keys = Keys> = MigrationFn<FromValue, ToValue, Keys, FromKey, ToKey> | (MigrationMetadata & {
   readonly migrate: MigrationFn<FromValue, ToValue, Keys, FromKey, ToKey>;
 }) | PipeMigrationDef<FromValue, ToValue, Keys, FromKey, ToKey>;
@@ -151,13 +153,46 @@ export type ValidateMeta<Key extends string = string> = {
   readonly schema: Key;
 };
 export type ValidateResult<T, Key extends string = string> = Result<T, readonly DobaIssue[], ValidateMeta<Key>>;
+export type VisualizeCommonOptions = {
+  readonly costs?: boolean | undefined;
+};
+export type VisualizeDirection = "LR" | "TB";
+export type VisualizeDotOptions = VisualizeCommonOptions & {
+  readonly format: "dot";
+  readonly direction?: VisualizeDirection | undefined;
+  readonly graphName?: string | undefined;
+};
+export type VisualizeFormat = "text" | "mermaid" | "json" | "dot";
+export type VisualizeInput = VisualizeFormat | VisualizeOptions;
+export type VisualizeJsonOptions = {
+  readonly format: "json";
+  readonly space?: number | undefined;
+};
+export type VisualizeMermaidOptions = VisualizeCommonOptions & {
+  readonly format: "mermaid";
+  readonly direction?: VisualizeDirection | undefined;
+  readonly config?: MermaidConfig | undefined;
+};
+export type VisualizeOptions = VisualizeTextOptions | VisualizeMermaidOptions | VisualizeDotOptions | VisualizeJsonOptions;
+export type VisualizeTextOptions = VisualizeCommonOptions & {
+  readonly format?: "text" | undefined;
+  readonly title?: string | undefined;
+};
 export type WarningInfo<FromKey extends string = string, ToKey extends string = FromKey> = {
   readonly message: string;
   readonly from: FromKey;
   readonly to: ToKey;
 };
 // #endregion
 
+// #region Classes
+export declare class MigrationConflictError extends Error {
+  readonly edge: string;
+  readonly sources: readonly [string, string];
+  constructor(_: string, _: readonly [string, string]);
+}
+// #endregion
+
 // #region Functions
 export declare function byField(_: string, _?: ByFieldOptions): (_: unknown) => string | null;
 export declare function createRegistry<Schemas extends SchemaMap>(_: RegistryConfig<Schemas>): Registry<Schemas, false>;

packages/doba/__snapshots__/tsnapi/index.snapshot.js

@@ -1,6 +1,14 @@
 /**
  * Generated by tsnapi — public API snapshot of `dobajs`
  */
+// #region Classes
+export class MigrationConflictError extends Error {
+  edge
+  sources
+  constructor(_, _) {}
+}
+// #endregion
+
 // #region Functions
 export function byField(_, _) {}
 export function createRegistry(_) {}

packages/doba/src/context.ts

@@ -71,7 +71,9 @@ export function createTransformContext<Keys extends string, From extends Keys, T
     },
     defaulted(path: readonly PropertyKey[], message: string): void {
       state.defaults.push({ path: [...path], message, from, to })
-      const pathStr = path.length > 0 ? path.join('.') : '(root)'
+      // Stringify each segment safely: path.join('.') throws on Symbol entries
+      // because String(Symbol) throws a TypeError.
+      const pathStr = path.length > 0 ? path.map((segment) => String(segment)).join('.') : '(root)'
       onWarning?.(`defaulted ${pathStr}: ${message}`, from, to)
     },
   }

packages/doba/src/helpers.ts

@@ -8,6 +8,26 @@ type PipeOp =
   | { type: 'drop'; names: string[] }
   | { type: 'map'; name: string; fn: (value: unknown) => unknown }
 
+/**
+ * returns a fresh copy of object/array defaults so callers can't mutate one
+ * transform's result and affect the next. factory functions and primitives
+ * are returned as-is.
+ */
+function cloneDefaultValue(value: unknown): unknown {
+  if (value === null || typeof value !== 'object') {
+    return value
+  }
+  if (Array.isArray(value)) {
+    return value.map((item) =>
+      item !== null && typeof item === 'object' ? cloneDefaultValue(item) : item,
+    )
+  }
+  // plain object: shallow-clone is sufficient for top-level isolation; nested
+  // mutation is documented as out-of-scope (matches the shallow spread used
+  // elsewhere in executePipe).
+  return { ...(value as Record<string, unknown>) }
+}
+
 /** flattens intersections for readable IDE tooltips. */
 // eslint-disable-next-line typescript-eslint/ban-types -- intentional empty intersection for type simplification
 type Simplify<T> = { [K in keyof T]: T[K] } & {}
@@ -119,7 +139,9 @@ function executePipe(
       case 'add':
         if (!(op.name in result)) {
           result[op.name] =
-            typeof op.defaultValue === 'function' ? op.defaultValue() : op.defaultValue
+            typeof op.defaultValue === 'function'
+              ? op.defaultValue()
+              : cloneDefaultValue(op.defaultValue)
           ctx.defaulted([op.name], `added with default`)
         }
         break

packages/doba/src/identify.ts

@@ -86,6 +86,11 @@ type ByFieldOptions =
 /**
  * creates an identify function that reads a field from the value and maps it to a schema key.
  *
+ * non-string field values (numbers, booleans, null, objects) return `null`
+ * instead of being stringified -- stringifying them would produce plausible-
+ * looking keys like "[object Object]" or "123" that silently fail to match
+ * any schema. coerce explicitly before passing to byField if you need that.
+ *
  * @example
  * ```ts
  * // value.version matches schema key directly
@@ -108,7 +113,10 @@ export function byField(
       if (!isObj(value) || !(field in value)) {
         return null
       }
-      const raw = String(value[field])
+      const raw = value[field]
+      if (typeof raw !== 'string') {
+        return null
+      }
       return mapping[raw] ?? null
     }
   }
@@ -121,15 +129,20 @@ export function byField(
       if (!isObj(value) || !(field in value)) {
         return null
       }
-      return String(value[field])
+      const raw = value[field]
+      return typeof raw === 'string' ? raw : null
     }
   }
 
   return (value: unknown) => {
     if (!isObj(value) || !(field in value)) {
       return null
     }
-    return `${prefix}${String(value[field])}${suffix}`
+    const raw = value[field]
+    if (typeof raw !== 'string') {
+      return null
+    }
+    return `${prefix}${raw}${suffix}`
   }
 }
 

packages/doba/src/index.ts

@@ -17,6 +17,8 @@ export type {
   MigrationsFor,
 } from './migration.js'
 
+export { MigrationConflictError } from './migration.js'
+
 export type {
   NarrowExclude,
   PathStrategy,
@@ -45,6 +47,20 @@ export type {
   IdentifyTransformResult,
 } from './registry.js'
 
+export type {
+  MermaidConfig,
+  MermaidConfigValue,
+  VisualizeCommonOptions,
+  VisualizeDirection,
+  VisualizeDotOptions,
+  VisualizeFormat,
+  VisualizeInput,
+  VisualizeJsonOptions,
+  VisualizeMermaidOptions,
+  VisualizeOptions,
+  VisualizeTextOptions,
+} from './visualize-entry.js'
+
 export { pipe } from './helpers.js'
 export type { PipeBuilder } from './helpers.js'
 

packages/doba/src/migration.ts

@@ -163,6 +163,23 @@ export const PREFERRED_COST = 0
 /** edge cost assigned when a migration is deprecated, making the graph avoid it. */
 export const DEPRECATED_COST = 1000
 
+/**
+ * thrown by {@link resolveMigrations} when two definitions claim the same edge
+ * and neither overrides the other (e.g. two reversible migrations that both
+ * register `a<->b`). surfaced as a typed error so callers building registries
+ * from dynamic config can distinguish it from unrelated failures.
+ */
+export class MigrationConflictError extends Error {
+  readonly edge: string
+  readonly sources: readonly [string, string]
+  constructor(edge: string, sources: readonly [string, string]) {
+    super(`Migration conflict: "${edge}" is defined by both "${sources[0]}" and "${sources[1]}"`)
+    this.name = 'MigrationConflictError'
+    this.edge = edge
+    this.sources = sources
+  }
+}
+
 export type MigrationFunction = (value: unknown, ctx: unknown) => unknown
 export type ResolvedMigration = {
   readonly fn: MigrationFunction
@@ -191,7 +208,7 @@ function extractMetadata(obj: Record<string, unknown>): {
   }
 
   let resolvedCost: number = DEFAULT_COST
-  if (typeof cost === 'number') {
+  if (typeof cost === 'number' && Number.isFinite(cost) && cost >= 0) {
     resolvedCost = cost
   } else if (preferred === true) {
     resolvedCost = PREFERRED_COST
@@ -247,9 +264,7 @@ function registerEdge(
       })
     } else {
       // same kind (two one-ways or two reversibles)
-      throw new Error(
-        `Migration conflict: "${edgeKey}" is defined by both "${existing}" and "${sourceKey}"`,
-      )
+      throw new MigrationConflictError(edgeKey, [existing, sourceKey])
     }
     return
   }
@@ -271,20 +286,47 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult {
   const sources = new Map<string, string>()
   const warnings: MigrationWarning[] = []
 
+  /**
+   * pushes a "skipped migration" warning. previously these were silent
+   * `continue`s, which let typos like `backwards:` instead of `backward:`
+   * register half a reversible migration with no signal.
+   */
+  const skip = (key: string, from: string, to: string, reason: string): void => {
+    warnings.push({ from, to, message: `skipped migration "${key}": ${reason}` })
+  }
+
   for (const [key, def] of typedEntries(migrations)) {
     if (def === undefined) {
       continue
     }
 
     const reversibleIdx = key.indexOf('<->')
     if (reversibleIdx !== -1) {
-      if (!isObject(def) || !('forward' in def) || !('backward' in def)) {
+      const from = key.slice(0, reversibleIdx)
+      const to = key.slice(reversibleIdx + 3)
+
+      // T2: reject keys whose parsed endpoints themselves contain arrow
+      // tokens -- they would silently mis-parse. e.g. "a<->b<->c" parses as
+      // from="a", to="b<->c", neither of which is what the user intended.
+      if (
+        from.length === 0 ||
+        to.length === 0 ||
+        from.includes('<->') ||
+        to.includes('<->') ||
+        from.includes('->') ||
+        to.includes('->')
+      ) {
+        skip(
+          key,
+          from,
+          to,
+          'malformed key (use exactly one "<->" between two non-empty schema names)',
+        )
         continue
       }
 
-      const from = key.slice(0, reversibleIdx)
-      const to = key.slice(reversibleIdx + 3)
-      if (from.length === 0 || to.length === 0) {
+      if (!isObject(def) || !('forward' in def) || !('backward' in def)) {
+        skip(key, from, to, 'reversible migration must have "forward" and "backward" functions')
         continue
       }
 
@@ -294,6 +336,7 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult {
       const { forward } = def
       const { backward } = def
       if (typeof forward !== 'function' || typeof backward !== 'function') {
+        skip(key, from, to, '"forward" and "backward" must be functions')
         continue
       }
 
@@ -310,12 +353,23 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult {
 
     const arrowIdx = key.indexOf('->')
     if (arrowIdx === -1) {
+      skip(key, '', '', 'malformed key (use "from->to" or "from<->to")')
       continue
     }
 
     const from = key.slice(0, arrowIdx)
     const to = key.slice(arrowIdx + 2)
-    if (from.length === 0 || to.length === 0) {
+    // T2: a key like "a->b->c" parses as from="a", to="b->c" -- the trailing
+    // arrow in `to` is almost never intended. reject it explicitly.
+    if (
+      from.length === 0 ||
+      to.length === 0 ||
+      from.includes('->') ||
+      to.includes('->') ||
+      from.includes('<->') ||
+      to.includes('<->')
+    ) {
+      skip(key, from, to, 'malformed key (use exactly one "->" between two non-empty schema names)')
       continue
     }
 
@@ -330,10 +384,12 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult {
     } else if (isObject(def) && 'pipe' in def) {
       const { pipe: pipeFn } = def
       if (typeof pipeFn !== 'function') {
+        skip(key, from, to, '"pipe" must be a function')
         continue
       }
       const migrationFn = pipeFn(createPipe()) as MigrationFunction
       if (typeof migrationFn !== 'function') {
+        skip(key, from, to, 'pipe callback must return a function')
         continue
       }
       const meta = extractMetadata(def)
@@ -345,6 +401,7 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult {
     } else if (isObject(def) && 'migrate' in def) {
       const { migrate } = def
       if (typeof migrate !== 'function') {
+        skip(key, from, to, '"migrate" must be a function')
         continue
       }
       const meta = extractMetadata(def)
@@ -353,6 +410,8 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult {
         fn: migrate as MigrationFunction,
         source: key,
       })
+    } else {
+      skip(key, from, to, 'must be a function, { migrate }, { pipe }, or { forward, backward }')
     }
   }