Skip to content

fix: --fresh flag fails to clean cookie_storage and fact_store#3354

Closed
deacon-mp wants to merge 6 commits into
masterfrom
fix/fresh-cleanup-missing-files
Closed

fix: --fresh flag fails to clean cookie_storage and fact_store#3354
deacon-mp wants to merge 6 commits into
masterfrom
fix/fresh-cleanup-missing-files

Conversation

@deacon-mp
Copy link
Copy Markdown
Contributor

Summary

  • Add data/fact_store and data/cookie_storage to DATA_FILE_GLOBS so --fresh properly removes them
  • Handle decryption failure in auth_svc gracefully: regenerate session key instead of crashing via sys.exit(1)

Problem

After PR #3264 (persistent sessions), data/cookie_storage is saved as an encrypted file. When the encryption key changes (e.g., switching between --insecure and secure mode), --fresh fails to clean it because it's not in the managed file list. On next startup, file_svc._read() hits InvalidToken and calls sys.exit(1), crashing the server.

Same issue exists for data/fact_store from the knowledge service.

Changes

File Change
app/service/data_svc.py Add data/fact_store and data/cookie_storage to DATA_FILE_GLOBS
app/service/auth_svc.py Catch SystemExit from _read(), delete stale cookie file, regenerate key

Test plan

  • --fresh now removes data/cookie_storage and data/fact_store
  • Encryption key mismatch no longer crashes server — auth_svc regenerates and continues
  • Existing sessions are invalidated (expected) but server stays up

🤖 Generated with Claude Code

deacon-mp and others added 2 commits April 3, 2026 16:21
…ers from stale cookies

- Add data/fact_store and data/cookie_storage to DATA_FILE_GLOBS so --fresh removes them
- Handle decryption failure in auth_svc gracefully: regenerate session key instead of crashing
- Fixes persistent encryption key mismatch error after switching --insecure mode or keys

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes --fresh cleanup gaps introduced by persistent sessions/knowledge persistence and prevents server startup crashes when data/cookie_storage can’t be decrypted due to an encryption key mismatch.

Changes:

  • Adds data/fact_store and data/cookie_storage to DATA_FILE_GLOBS so --fresh removes them.
  • Updates AuthService.apply() to recover from cookie key decrypt failures by deleting the stale file and regenerating a new session key instead of exiting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
app/service/data_svc.py Expands managed data globs so --fresh also cleans fact_store and cookie_storage.
app/service/auth_svc.py Handles decrypt/read failures of cookie_storage by regenerating and persisting a new session key.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/service/data_svc.py Outdated
Comment thread app/service/auth_svc.py
Comment thread app/service/auth_svc.py
…mExit catch

- Remove data/fact_store from DATA_FILE_GLOBS (knowledge_svc handles its own cleanup)
- Narrow inner catch to SystemExit only (let IO/permission errors propagate)
- Test already covers the SystemExit recovery path
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

app/service/auth_svc.py:107

  • When secret_key is replaced due to invalid length, the new key is not persisted back to data/cookie_storage. This causes the app to repeatedly regenerate an ephemeral key on each startup if the stored key is malformed. After generating a replacement 32-byte key, also overwrite the cookie_storage file (best-effort) to restore persistence.
        if len(secret_key) != 32:
            secret_key = os.urandom(32)
            self.log.warning('Loaded session key is not 32 bytes long. Generating new key.')


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/service/data_svc.py
Comment thread tests/services/test_fresh_cleanup.py Outdated
Comment thread tests/services/test_fresh_cleanup.py
Comment thread app/service/auth_svc.py
- Remove unused COOKIE_SESSION import
- Use per-test @pytest.mark.asyncio instead of global pytestmark
- Clear BaseWorld config in fixture teardown to avoid state leakage
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/services/test_fresh_cleanup.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/services/test_fresh_cleanup.py
Comment thread tests/services/test_fresh_cleanup.py
Comment thread tests/services/test_fresh_cleanup.py Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/services/test_fresh_cleanup.py
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 3, 2026

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@deacon-mp
Copy link
Copy Markdown
Contributor Author

Closing in favor of a clean PR with squashed history. Same code, verified by CI + Copilot.

@deacon-mp deacon-mp closed this Apr 3, 2026
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.

2 participants