Skip to content

[Routine - QP] fix: guard VersionManager against malformed history JSON shape#48

Draft
winterdrive wants to merge 1 commit into
masterfrom
routine/qp-fix-versionmanager-malformed-json-260707
Draft

[Routine - QP] fix: guard VersionManager against malformed history JSON shape#48
winterdrive wants to merge 1 commit into
masterfrom
routine/qp-fix-versionmanager-malformed-json-260707

Conversation

@winterdrive

Copy link
Copy Markdown
Owner

Pre-flight Check

Open PRs / active branches checked before selecting this target:

This PR only touches src/core/VersionManager.ts and src/test/unit/versionManager.test.ts, which none of the above PRs modify, so there is no overlap.

Changes

VersionManager.loadHistory (the pure Node.js core version manager, distinct from VersionHistoryService.ts which #46 is fixing) parses *.history.json with a bare JSON.parse inside a try/catch. The catch only handles JSON.parse throwing (e.g. corrupted JSON) — it does not catch the case where parsing succeeds but produces the wrong shape (e.g. the file contains [1,2,3] or {} instead of { promptId, versions, currentVersionId }). In that case history.versions is undefined, and the very next line calling .find(...) elsewhere (getCurrentVersion, applyVersion, createVersion, etc.) throws a raw TypeError instead of falling back gracefully.

This mirrors the fix already merged for the sibling file VersionHistoryService.ts (commit 20b6a09, "guard VersionHistoryService against corrupted/malformed history JSON") — same bug pattern, same fallback strategy (reset to empty history), applied to the not-yet-patched VersionManager.ts.

  • Added a VersionManager.isValidHistory type guard (same shape check as VersionHistoryService.isValidHistory).
  • loadHistory now throws internally (caught by the existing catch block) when the parsed JSON doesn't match the expected shape, resetting to an empty history instead of letting a malformed shape propagate.
  • Added two unit tests mirroring the existing VersionHistoryService corrupted-JSON tests: corrupted JSON text, and valid-but-wrong-shape JSON (an array).

Confirms this PR does not modify any MCP tool schema/names/parameters in mcp-server/src/tools/, and does not add/remove/update any dependencies.

Safety Verification

Commands run locally, all passed:

  • npx tsc -p ./ — no errors
  • npm run test:coverage (project's Jest command) — 7 test suites, 109 tests, all passed (includes the 2 new tests plus all pre-existing suites)

CI / Release Gate Note

This is a daily routine Draft PR. Package version bump (package.json/package-lock.json) and CHANGELOG.md updates are intentionally deferred to the weekend release/integration PR. If GitHub Actions fails only due to the repository's version-bump gate, that is expected release-readiness behavior for a routine PR, not a code validation failure.

JSON.parse succeeding on unexpected shapes (e.g. an array, or an object
missing `versions`/`currentVersionId`) previously slipped past the
generic catch block and would only surface later as a TypeError when
callers accessed `history.versions.find(...)`. Add the same shape
validation already used in VersionHistoryService.loadHistory so a
corrupted or malformed *.history.json resets to an empty history
instead of crashing downstream callers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant