Skip to content

fix: harden persisted plan lifecycle state - #84

Closed
jefft72 wants to merge 4 commits into
plan/05-runtime-testsfrom
plan/06-persistence-hardening
Closed

jefft72 wants to merge 4 commits into
plan/05-runtime-testsfrom
plan/06-persistence-hardening

Conversation

@jefft72

@jefft72 jefft72 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Hardens the persisted plan lifecycle so a malformed, orphaned, or mismatched plan entry cannot leave a resumed session claiming a pending plan that does not exist.

Replaces #68.

Reasoning

Plan state is reconstructed from the session file on every resume and tree navigation, so the parser is the only thing standing between a corrupt file and a session that believes it has work waiting.
Failing loudly on an invalid artifact is safer than silently dropping it, because a dropped plan looks identical to a discarded one from the user side.

The strict entry parser also called the profile-only session header normalizer, so that call is dropped for the same reason as in the lenient parser.

Changes

  • Rejects invalid persisted plan artifacts with an explicit error instead of skipping them
  • Handles orphaned plan entries and plan entries without a preceding mode change
  • Handles resolution entries that do not match their plan
  • Adds test/suite/session-manager-plan-mode.test.ts covering each malformed case

Testing

  • npx vitest --run test/suite/session-manager-plan-mode.test.ts test/session-manager/file-operations.test.ts
  • tsgo --noEmit reports zero errors

Reject malformed session graphs and update session files atomically without corrupting the active manager.
Require accepted plans to persist their trusted implementation prompt through a dedicated append path.
Exercise plan validation and continuation recovery across persisted session branches and reloads.
- Malformed or orphaned plan entries could leave a resumed session claiming a pending plan that no longer existed
- The strict entry parser also called the profile-only normalizeSessionHeader helper, so it is dropped here for the same reason as the lenient parser

@Issacboy123 Issacboy123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recovery flow usefully separates loading, migration, validation, and index construction. Its durability and schema boundaries are incomplete: rewriting with "w" can destroy the only copy, and the strict loader accepts arbitrary object shapes before indexing. Make rewrites transactional and validate the complete entry union before indexing. Issues persist across the stack.


private _rewriteFileEntries(sessionFile: string, fileEntries: FileEntry[]): void {
if (!this.persist) return;
const fd = openSync(sessionFile, "w");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opening the live session with "w" truncates the only durable copy before the replacement is complete. If a later write fails or the process terminates, recovery leaves an empty or partial session. Please write and flush a sibling temporary file, then atomically replace the target, with a fault-injection regression proving the original survives failure.

if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
throw new Error(`Session contains an invalid JSONL object at line ${lineNumber}.`);
}
const entry = parsed as FileEntry;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cast accepts unsupported type values and invalid common fields such as id, parentId, and timestamp. The later validation only checks graph and plan lifecycle constraints. Please validate the common fields and every supported entry discriminant before returning or indexing the record.

@Pepps233 Pepps233 closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants