Port: port/pr-27-fix/readjson-deep-merge

[briansumma]

Ported from upstream cytostack/openwolf. Needs review before merge.

[briansumma]

---

PR #6 Review: readJSON Deep Merge

Overall verdict: Do not merge as-is — one blocker, one warning.

---

What the PR does

Adds a deepMergeDefaults helper that fills missing nested keys from the fallback argument into whatever was parsed from disk. The goal is forward-compatibility: a user's config.json that predates a new section won't cause TypeError: Cannot read properties of undefined anymore. The algorithm itself is correct.

---

🔴 BLOCKER — src/hooks/shared.ts::readJSON not updated

This is the critical omission. Your CLAUDE.md documents that hooks cannot import from src/utils/ at runtime — they compile to standalone Node scripts and get a self-contained copy of utilities in src/hooks/shared.ts. That file has its own readJSON at line 22–28 that was not touched by this PR:

// shared.ts — still the old, unfixed version
export function readJSON<T = unknown>(filePath: string, fallback: T): T {
  try {
    return JSON.parse(fs.readFileSync(filePath, "utf-8")) as T;
  } catch {
    return fallback;
  }
}

All six hooks (session-start, stop, pre-read, post-read, pre-write, post-write) import from ./shared.js. They fire on every Claude Code event and are the primary runtime path — yet they remain unprotected. The fix is to replicate isPlainObject + deepMergeDefaults into shared.ts and apply the same change there.

---

🟡 WARNING — Shallow spread shares nested default object references

In deepMergeDefaults at line 21:

const result: Record<string, unknown> = { ...(defaults as Record<string, unknown>) };

{ ...defaults } is a shallow copy. Any nested object in defaults that has no corresponding key in loaded is placed into result by the same reference as the fallback. Consider wolf-daemon.ts:27 where the fallback is a module-level object literal with a nested cron: { enabled: true, heartbeat_interval_minutes: 30 }. If the config file is missing the cron key entirely, result.cron is the fallback's actual cron object. Any code that mutates config.openwolf.cron would silently corrupt the fallback.

The risk is low in the current codebase since configs are treated as read-only, but the contract is fragile. Fix: use structuredClone(defaults) (Node 17+) or a recursive clone helper instead of the spread.

---

✅ What's correct

• The isPlainObject guard correctly excludes null, arrays, and class instances — none of which should be recursively merged, all of which JSON.parse can return.
• "Loaded values always win" semantics are implemented correctly.
• Scalar/array values in loaded replace defaults wholesale — correct and documented.
• The catch → return fallback path for missing/corrupt files is unchanged.
• Both helpers are unexported (private implementation detail) — appropriate.

---

Full review written to PR-6-REVIEW.md in the repo root.

[briansumma]

✅ Autofixer applied. Build and tests passed. Promoting to Ready for Review.