feat(core): preserve package.json formatting on upgrade

docs/roadmap/01-correctness.md

@@ -1,35 +1,38 @@
 # P0 — Correctness: protect the writes that already work
 
-> **Read this first, because it overturns the old plan.** The previous roadmap opened by claiming
-> inup *"silently strips the `^`/`~` range prefix and pins your dependency on every upgrade."*
-> **That is false against the current code** (verified 2026-05-30). The write at
-> [upgrader.ts:142](../../src/core/upgrader.ts#L142)/[154](../../src/core/upgrader.ts#L154) does
-> write `choice.targetVersion` verbatim — but that value is **already prefixed** upstream:
-> [selection-state-builder.ts:127-139](../../src/ui/session/selection-state-builder.ts#L127-L139)
+> **Status: shipped (2026-05-31).** All four items below are now implemented and tested. The phase
+> opened smaller than the original roadmap feared — re-verifying against source showed the headline
+> "prefix-stripping" bug never existed and two items were already done — so what shipped is: the
+> formatting-preservation write fix (#2), the golden test that locks it (#1), and the `SKIP_DIRS`
+> override + warning (#4). #3 was already present before this work.
+>
+> **On the (non-)bug.** The previous roadmap claimed inup *"silently strips the `^`/`~` range prefix
+> and pins your dependency on every upgrade."* **That was false against the code.** The write at
+> [upgrader.ts:142](../../src/core/upgrader.ts#L142)/[154](../../src/core/upgrader.ts#L154) writes
+> `choice.targetVersion` verbatim — but that value is **already prefixed** upstream:
+> [selection-state-builder.ts:120-133](../../src/ui/session/selection-state-builder.ts#L120-L133)
 > builds it through `applyVersionPrefix()`
 > ([ui/utils/version.ts:6-10](../../src/ui/utils/version.ts#L6-L10)), which lifts the original
-> operator and re-applies it. So `"^1.2.3"` → `"^1.5.0"`. The headline bug doesn't exist.
->
-> That changes this phase's job. It is no longer *"stop producing wrong results"* — it's **"protect
-> the correct behavior with a test, then fix three genuinely-silent mis-detections."** P0 went from a
-> phase to an afternoon. The trust floor is mostly already poured; we're sealing it.
+> operator and re-applies it. So `"^1.2.3"` → `"^1.5.0"`. Default prefix preservation was — and is —
+> already covered by a test
+> ([selection-state-builder.test.ts:23-31](../../test/unit/ui/selection-state-builder.test.ts#L23-L31)).
 
 See the [legend](README.md#legend) for rating definitions.
 
-| # | Task | Value | Cx | Effort | Notes / reuse |
-|---|---|:--:|:--:|---|---|
-| 1 | **Lock the write path with a characterization test** | 🔴 | S | 0.5d | **New, and the most important item here.** Prefix-preservation works but has **no test guarding it** — one careless refactor of `applyVersionPrefix` or `createUpgradeChoices` silently reintroduces the exact bug the old roadmap feared. Golden-test the write end-to-end: feed a `package.json` with a `^` dep, a `~` dep, an exact pin, tabs *and* 4-space siblings; assert the output preserves each operator, the indentation, the trailing newline, and is a **no-op when nothing changed**. This is the cheapest insurance in the project and the guardrail for #2. Extend [test/unit/core](../../test/unit/core). |
-| 2 | **Preserve `package.json` formatting on write** | 🔴 | S | 0.5–1d | [upgrader.ts:161](../../src/core/upgrader.ts#L161) does `JSON.stringify(pkg, null, 2) + '\n'`, normalizing tabs/4-space/odd-ordered files to 2-space and forcing a trailing newline. Real, but **S not M**: detect the existing indent (read the raw text before parse) and the original final-newline, round-trip both, and **skip the write entirely if the serialized result is unchanged**. Re-rated down — the old roadmap called this M by lumping it with the (non-existent) prefix bug. |
-| 3 | **Detect Bun's text lockfile `bun.lock`** | 🔴 | S | 0.5d | Bun ≥ 1.2 writes a text `bun.lock`; the detector only lists `bun.lockb` ([package-manager-detector.ts:44](../../src/services/package-manager-detector.ts#L44),[:111](../../src/services/package-manager-detector.ts#L111)). A lockfile-only Bun repo falls through to the npm default. One array entry. *Narrow blast radius:* repos with `"packageManager": "bun@…"` are already caught by the field regex at [:92](../../src/services/package-manager-detector.ts#L92). |
-| 4 | **Fix the `lib/` silent skip — make `SKIP_DIRS` overridable** | 🟡 | S | 0.5d | `SKIP_DIRS` hardcodes `lib`/`es`/`esm`/`cjs` ([scan.ts:9-19](../../src/utils/filesystem/scan.ts#L9-L19)), so a monorepo package living under `lib/` is **silently skipped, no warning**. The `.inuprc` reader already exists ([project-config.ts:8](../../src/config/project-config.ts#L8)) — add an `includeDirs`/`scanDirs` override there, and at minimum log when a `package.json`-bearing dir is pruned by the default list. *(Broader discovery work — `.gitignore` awareness, scanner de-dup — lives in [06-ecosystem.md](06-ecosystem.md).)* |
+| # | Task | Value | Cx | Status |
+|---|---|:--:|:--:|---|
+| 1 | **Lock the write path with a characterization test** | 🔴 | S | ✅ **Done.** Default `^`/`~` preservation was already tested; this added the missing **end-to-end write** golden tests in [upgrader.test.ts](../../test/unit/core/upgrader.test.ts) — tab- and 4-space-indented fixtures, a `~`/exact operator, a no-trailing-newline file, and a **no-op when nothing changed** case (byte-identical output). Plus a focused unit test for the new format detector ([io.test.ts](../../test/unit/utils/io.test.ts)). |
+| 2 | **Preserve `package.json` formatting on write** | 🔴 | S | ✅ **Done.** New `detectJsonFormat()` in [io.ts](../../src/utils/filesystem/io.ts) reads the raw text before parse to recover the original indent (tabs / N-space) and trailing-newline. [upgrader.ts](../../src/core/upgrader.ts) round-trips both and **skips the write entirely when the serialized result is unchanged**. |
+| 3 | **Detect Bun's text lockfile `bun.lock`** | 🔴 | S | ✅ **Already done (pre-existing).** [package-manager-detector.ts:111-113](../../src/services/package-manager-detector.ts#L111-L113) lists `bun.lock` alongside `bun.lockb`, tested at [package-manager-detector.test.ts:95-101](../../test/unit/services/package-manager-detector.test.ts#L95-L101). No change needed. |
+| 4 | **Fix the `lib/` silent skip — make `SKIP_DIRS` overridable** | 🟡 | S | ✅ **Done.** `.inuprc` now accepts a `scanDirs` override ([project-config.ts](../../src/config/project-config.ts)) that threads through `cli.ts` → `PackageDetector` → [scan.ts](../../src/utils/filesystem/scan.ts), mirroring `excludePatterns`. When a `package.json`-bearing `lib`/`es`/`esm`/`cjs` dir is pruned by the default list, the detector prints a one-time warning pointing at `scanDirs`. `node_modules`/`dist`/`build`/`coverage`/`out` are excluded from the warning to avoid noise. *(Broader discovery work — `.gitignore` awareness, scanner de-dup — lives in [06-ecosystem.md](06-ecosystem.md).)* |
 
-## Why this ordering
+## How it was sequenced
 
-- **#1 before #2.** Write the lock test first, then make the formatting change against it — the test
-  is what lets you touch [upgrader.ts:160-162](../../src/core/upgrader.ts#L160-L162) without fear,
-  and it permanently documents the prefix-preservation contract so it can't quietly rot.
-- **#3 and #4 are independent afternoon fixes.** Each kills one class of *silent* wrongness
-  (mis-detected PM, skipped package). They don't depend on #1/#2 and can land in any order.
+- **#1 alongside #2.** The golden write tests were written against the formatting change so
+  [upgrader.ts](../../src/core/upgrader.ts) can be refactored without fear, and they permanently
+  document the prefix- and format-preservation contract so it can't quietly rot.
+- **#4 is independent.** It kills one class of *silent* wrongness (a real package skipped under
+  `lib/`) and shares no code with #1/#2.
 
 ## What was removed from the old P0 (and why)
 
@@ -38,17 +41,20 @@ See the [legend](README.md#legend) for rating definitions.
 - **"Cross-platform `package.json` path handling."** Demoted to a footnote. The string
   `packageJsonPath.replace('/package.json', '')` at
   [upgrader.ts:122](../../src/core/upgrader.ts#L122) is POSIX-only, but it only feeds a **spinner
-  label** (`packageDir`); the actual file write at [:161](../../src/core/upgrader.ts#L161) uses the
-  real path, and install dir resolution at [:52](../../src/core/upgrader.ts#L52) already uses
-  `dirname()`. It's a cosmetic log glitch on Windows, not a correctness bug. *Fix it opportunistically
-  by copying the `dirname()` call from line 52 — don't schedule it.*
+  label** (`packageDir`); the actual file write uses the real path, and install dir resolution at
+  [:54](../../src/core/upgrader.ts#L54) already uses `dirname()`. It's a cosmetic log glitch on
+  Windows, not a correctness bug. *Fix it opportunistically by copying the `dirname()` call — don't
+  schedule it.*
 
 ## Relationship to later phases
 
-- The **`--save-exact`** opt-out (for users who *want* exact pins) lives in
-  [06-ecosystem.md](06-ecosystem.md). Because preservation is already the default, that flag is now
-  the *primary* prefix control rather than a follow-on — see its reframing there.
+- The **`--save-exact`** opt-out (for users who *want* exact pins) is **already implemented
+  end-to-end** — flag at [cli.ts:151](../../src/cli.ts#L151), applied in
+  [selection-state-builder.ts:120-133](../../src/ui/session/selection-state-builder.ts#L120-L133),
+  tested at [selection-state-builder.test.ts:33-44](../../test/unit/ui/selection-state-builder.test.ts#L33-L44).
+  Because preservation is the default, it is the primary prefix-control flag.
 - **Routing detector warnings to stderr** (so they don't corrupt `--json`) is an output-hygiene
-  concern handled in [02-headless.md](02-headless.md), not here.
-- The **broader write-path coverage** (beyond the #1 golden test) is tracked in
+  concern handled in [02-headless.md](02-headless.md), not here. The new `scanDirs` skip warning
+  uses `console.warn` and should be reviewed under that work.
+- The **broader write-path coverage** (beyond the #1 golden tests) is tracked in
   [05-infra.md](05-infra.md).

src/cli.ts

@@ -92,6 +92,7 @@ export async function runCli(options: CliOptions): Promise<void> {
   const upgrader = new UpgradeRunner({
     cwd,
     excludePatterns,
+    scanDirs: projectConfig.scanDirs,
     maxDepth,
     ignorePackages,
     packageManager,

src/config/project-config.ts

@@ -17,6 +17,13 @@ export interface InupProjectConfig {
    */
   exclude?: string[]
 
+  /**
+   * Directory names to scan even though they are in the default skip list
+   * (node_modules, dist, build, coverage, out, lib, es, esm, cjs).
+   * Use this when a real package lives under e.g. "lib/".
+   */
+  scanDirs?: string[]
+
   /**
    * Show vulnerability badges for peerDependencies in the package list.
    * Defaults to false so peer dependency risk stays hidden unless explicitly enabled.
@@ -85,6 +92,12 @@ function normalizeConfig(config: InupProjectConfig): InupProjectConfig {
     }
   }
 
+  if (config.scanDirs) {
+    if (Array.isArray(config.scanDirs)) {
+      normalized.scanDirs = config.scanDirs.filter((item) => typeof item === 'string')
+    }
+  }
+
   if (typeof config.showPeerDependencyVulnerabilities === 'boolean') {
     normalized.showPeerDependencyVulnerabilities = config.showPeerDependencyVulnerabilities
   }

src/core/package-detector.ts

@@ -1,3 +1,4 @@
+import chalk from 'chalk'
 import * as semver from 'semver'
 import {
   DependencyEntry,
@@ -32,6 +33,7 @@ export class PackageDetector {
   private packageJson: Record<string, unknown> | null = null
   private cwd: string
   private excludePatterns: string[]
+  private scanDirs: string[]
   private ignorePackages: string[]
   private maxDepth: number
 
@@ -41,6 +43,7 @@ export class PackageDetector {
   constructor(options?: UpgradeOptions) {
     this.cwd = options?.cwd || process.cwd()
     this.excludePatterns = options?.excludePatterns || []
+    this.scanDirs = options?.scanDirs || []
     this.ignorePackages = options?.ignorePackages || []
     this.maxDepth = options?.maxDepth ?? 10
     this.packageJsonPath = findPackageJson(this.cwd)
@@ -390,11 +393,12 @@ export class PackageDetector {
   }
 
   private async findPackageJsonFilesWithTimeout(timeoutMs: number): Promise<string[]> {
+    const skippedPackageDirs = new Set<string>()
     try {
       let timeoutId: NodeJS.Timeout | undefined
 
       try {
-        return await Promise.race([
+        const files = await Promise.race([
           findAllPackageJsonFilesAsync(
             this.cwd,
             this.excludePatterns,
@@ -403,6 +407,10 @@ export class PackageDetector {
               const truncatedDir =
                 currentDir.length > 50 ? '...' + currentDir.slice(-47) : currentDir
               this.showProgress(`🔍 Scanning ${truncatedDir} (found ${foundCount})`)
+            },
+            {
+              scanDirs: this.scanDirs,
+              onSkippedPackageDir: (relativePath) => skippedPackageDirs.add(relativePath),
             }
           ),
           new Promise<string[]>((_, reject) => {
@@ -412,6 +420,8 @@ export class PackageDetector {
             timeoutId.unref?.()
           }),
         ])
+        this.warnSkippedPackageDirs(skippedPackageDirs)
+        return files
       } finally {
         if (timeoutId) {
           clearTimeout(timeoutId)
@@ -424,6 +434,27 @@ export class PackageDetector {
     }
   }
 
+  /**
+   * Warn (once per directory, after the scan) about package.json-bearing directories that the
+   * default skip list pruned, so the user can re-include them via `.inuprc`'s `scanDirs`.
+   * Emitted after scanning so it does not corrupt the progress spinner output.
+   */
+  private warnSkippedPackageDirs(skippedPackageDirs: Set<string>): void {
+    if (skippedPackageDirs.size === 0) {
+      return
+    }
+    const list = Array.from(skippedPackageDirs).sort()
+    console.warn(
+      chalk.yellow(
+        `⚠️  Skipped ${list.length} package.json-bearing director${
+          list.length === 1 ? 'y' : 'ies'
+        } matching the default ignore list:\n` +
+          list.map((dir) => `   - ${dir}`).join('\n') +
+          `\n   Add the directory name(s) to "scanDirs" in .inuprc to include them.`
+      )
+    )
+  }
+
   private isWorkspaceReference(version: string): boolean {
     return (
       version.includes('workspace:') ||

src/core/upgrader.ts

@@ -1,10 +1,10 @@
 import chalk from 'chalk'
 import { createSpinner } from 'nanospinner'
-import { existsSync, writeFileSync } from 'fs'
+import { existsSync, readFileSync, writeFileSync } from 'fs'
 import { dirname } from 'path'
 import { spawnSync } from 'child_process'
 import { PackageInfo, PackageUpgradeChoice, PackageManagerInfo, DependencyType } from '../types'
-import { executeCommand, findWorkspaceRoot, readPackageJson } from '../utils'
+import { detectJsonFormat, executeCommand, findWorkspaceRoot, readPackageJson } from '../utils'
 
 export class PackageUpgrader {
   private packageManager: PackageManagerInfo
@@ -123,7 +123,8 @@ export class PackageUpgrader {
     const spinner = createSpinner(`Upgrading ${type} in ${packageDir}...`).start()
 
     try {
-      // Read the current package.json
+      // Read the current package.json — keep the raw text so we can round-trip its formatting
+      const rawContent = readFileSync(packageJsonPath, 'utf-8')
       const packageJson = readPackageJson(packageJsonPath)
 
       // Group by upgrade type (range vs latest)
@@ -156,9 +157,15 @@ export class PackageUpgrader {
         })
       }
 
-      // Write back the modified package.json
+      // Write back the modified package.json, preserving the original indentation and
+      // trailing-newline style. Skip the write entirely when nothing actually changed.
       if (modified) {
-        writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2) + '\n')
+        const format = detectJsonFormat(rawContent)
+        const nextContent =
+          JSON.stringify(packageJson, null, format.indent) + (format.trailingNewline ? '\n' : '')
+        if (nextContent !== rawContent) {
+          writeFileSync(packageJsonPath, nextContent)
+        }
       }
 
       spinner.success({ text: `Upgraded ${choices.length} ${type} in ${packageDir}` })

src/types/domain.ts

@@ -77,6 +77,7 @@ export interface VulnerabilityDisplayOptions {
 export interface UpgradeOptions extends VulnerabilityDisplayOptions {
   cwd?: string
   excludePatterns?: string[]
+  scanDirs?: string[] // Directory names to scan even if in the default skip list (from .inuprc)
   maxDepth?: number // Maximum package.json scan depth, defaults to 10
   packageManager?: PackageManager // Manual override for package manager
   ignorePackages?: string[] // Package names/patterns to ignore (from .inuprc or --ignore flag)

src/utils/filesystem/io.ts

@@ -16,6 +16,29 @@ export function readPackageJson(path: string): PackageJson {
   }
 }
 
+export interface JsonFormat {
+  /** Indent passed to JSON.stringify — the original whitespace string (tabs or N spaces), or 2 as fallback. */
+  indent: string | number
+  /** Whether the original file ended with a trailing newline. */
+  trailingNewline: boolean
+}
+
+/**
+ * Detect the indentation and trailing-newline style of a raw JSON document so a
+ * re-serialized version can preserve the original formatting instead of normalizing it.
+ *
+ * The first indented line's leading whitespace is exactly one indent unit; using it verbatim
+ * as the JSON.stringify indent round-trips tabs, 2-space, and 4-space without branching on type.
+ * Minified/single-line files (no indented line) fall back to 2 spaces, matching prior behavior.
+ */
+export function detectJsonFormat(raw: string): JsonFormat {
+  const match = raw.match(/\n([ \t]+)\S/)
+  return {
+    indent: match ? match[1] : 2,
+    trailingNewline: raw.endsWith('\n'),
+  }
+}
+
 export async function readPackageJsonAsync(path: string): Promise<PackageJson> {
   try {
     const content = await fsPromises.readFile(path, 'utf-8')