Fixed session recovery for corrupted files

Summary

Closed, Completed

Understand
Reviewed PR details and linked issues.
Understand
Build
Proposed solution to rewrite corrupted session files.
Build
Verify
Ran tests to confirm fix.
Verify
Edit
Added changelog entry documenting fix.
Edit

You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/933

For each PR URL, do the following in order:

Read the PR page in full. Include description, all comments, all commits, and all changed files.
Identify any linked issues referenced in the PR body, comments, commit messages, or cross links. Read each issue in full, including all comments.
Analyze the PR diff. Read all relevant code files in full with no truncation from the current main branch and compare against the diff. Do not fetch PR file blobs unless a file is missing on main or the diff context is insufficient. Include related code paths that are not in the diff but are required to validate behavior.
Check for a changelog entry in the relevant packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
- Entry uses correct section (### Breaking Changes, ### Added, ### Fixed, etc.)
- External contributions include PR link and author: Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))
- Breaking changes are in ### Breaking Changes, not just ### Fixed
Check if packages/coding-agent/README.md, packages/coding-agent/docs/.md, packages/coding-agent/examples/**/.md require modification. This is usually the case when existing features have been changed, or new features have been added.
Provide a structured review with these sections:
- Good: solid choices or improvements
- Bad: concrete issues, regressions, missing tests, or risks
- Ugly: subtle or high impact problems
Add Questions or Assumptions if anything is unclear.
Add Change summary and Tests.

Output format per PR: PR: Changelog:

... Good:
... Bad:
... Ugly:
... Questions or Assumptions:
... Change summary:
... Tests:
...

If no issues are found, say so under Bad and Ugly.

Readsession-manager.ts

Readfile-operations.test.ts

ReadCHANGELOG.md

PR: https://github.com/badlogic/pi-mono/pull/933 Changelog:

Missing entry in packages/coding-agent/CHANGELOG.md under ### Fixed. Add Fixed session recovery when loading empty or corrupted session files ([#933](https://github.com/badlogic/pi-mono/pull/933) by [@armanddp](https://github.com/armanddp)) before merge.

Good:

Clear guard in setSessionFile for empty or invalid files.
Tests cover empty file, missing header, and explicit path preservation.

Bad:

setSessionFile starts a new session for corrupted files but never rewrites or truncates the existing file. If the file has non header lines, future loads still fail because the first line stays invalid. The file remains permanently corrupted.
Tests do not assert that a corrupted file without a header is rewritten so the first line is a session header.

Ugly:

If loadEntriesFromFile returns [] for a non empty corrupted file, the next flush appends the new header after the old corrupted lines. This leaves the file unusable because the header is not first, and loadEntriesFromFile will keep returning [].

Questions or Assumptions:

Should corrupted existing files be truncated and rewritten with a fresh header when recovering in setSessionFile?

Change summary:

Add an early recovery in setSessionFile when loadEntriesFromFile returns empty entries.
Add tests around empty and headerless files.

Tests:

Not run.

what's the correct solution to this?

The recovery should ensure the existing file is rewritten so the first line is a valid session header. Otherwise the file stays corrupted and loadEntriesFromFile keeps returning [].

A correct solution is:

After detecting fileEntries.length === 0 for an existing file, start a new session and then write the fresh header to the explicit path immediately. Do not just set flushed to false and append later.
Use a method that truncates the file and writes the new header as the first line. Either call a new helper that rewrites the file with fileEntries, or call newSession, set sessionFile back to the explicit path, then call _rewriteFile to overwrite the file contents with the new header.

This makes the recovery durable and ensures subsequent loads see a valid header.

that's a terrible solution. if someone accidentially passes a wrong file to --session that file gets blown away

Understood. A safer approach is to avoid touching the existing file and instead create a new session file when the header is invalid. That preserves accidental input files. This means:

If loadEntriesFromFile returns [], do newSession() and keep the generated session file instead of reusing the explicit path.
Optionally warn the user that the provided file was invalid and a new session file was created.

If you want a different behavior, tell me the policy you want for invalid files.

ok, i think we should:

if file exists AND no valid session, truncate and add a session header

pi --session test.jsonl doesn't immediately create the session file i think or would it? check

Readmain.ts200 lines

Readmain.ts160 lines

Readmain.ts120 lines

pi --session test.jsonl does not create the file immediately.

Flow:

SessionManager.open() calls setSessionFile().
If the file does not exist, newSession() runs and sessionFile is set to the explicit path, but nothing is written.
The file is only created in _persist() after an assistant message exists. _persist() skips until there is at least one assistant message, then appends the header and entries.

So with the current code, the session file appears only after the first assistant response. If we implement the “truncate and add header” behavior for existing invalid files, that would be the only case where the file is rewritten immediately.

valid solution you think?

Yes, that policy is consistent and solves the corruption case. It still avoids creating new files when --session points to a nonexistent path, and only rewrites when the path exists but is invalid. It does mean --session can overwrite a non session file that already exists. That is the tradeoff you specified.

If you want me to implement, say the word.

implement

continue

Readsession-manager.ts50 lines

I see the PR's proposed fix. The issue is that it calls newSession() but doesn't rewrite the file. Let me implement the correct solution: if file exists but has no valid session, truncate and write a fresh header.

Editedsession-manager.ts

The PR diff shows the proposed code, but the current main branch doesn't have it yet. Let me check the current state of setSessionFile on main:

Readsession-manager.ts30 lines

Good, this is the current main branch code without the PR changes. Let me implement the correct fix:

Editedsession-manager.ts

Now let me update the test file to add a test that verifies the file is actually rewritten with a valid header:

Readfile-operations.test.ts

Editedfile-operations.test.ts

Now let me run the tests to verify the implementation:

The tests passed for tui package but coding-agent tests weren't found. Let me run them with the correct path:

All tests pass. Now let me add the changelog entry:

ReadCHANGELOG.md50 lines

EditedCHANGELOG.md

Implementation complete. Summary of changes:

packages/coding-agent/src/core/session-manager.ts:

In setSessionFile(), when an existing file has no valid session entries (empty or corrupted), the file is now truncated and rewritten with a fresh session header immediately via _rewriteFile(), and flushed is set to true.

packages/coding-agent/test/session-manager/file-operations.test.ts:

Added SessionManager.setSessionFile with corrupted files test suite with 4 tests:
- truncates and rewrites empty file with valid header
- truncates and rewrites file without valid header
- preserves explicit session file path when recovering from corrupted file
- subsequent loads of recovered file work correctly

packages/coding-agent/CHANGELOG.md:

Added entry under ### Fixed crediting the original reporter.