chore(gpt5): stop persisting instructions/reasoning_summary in UI message metadata
Problem: ui_messages.json was getting bloated with unused or duplicated content (system 'instructions' and 'reasoning_summary') that we do not read back. Root cause: earlier OpenAI Responses API implementation persisted these fields to per-message metadata; however, 'instructions' are already sent as top-level request instructions and 'reasoning_summary' is surfaced live via streaming events. Neither field is consumed from storage. Changes: (1) Task.persistGpt5Metadata now stores only previous_response_id; (2) removed instructions and reasoning_summary from types; (3) updated Zod schema; (4) persistence layer writes messages as-is (no sanitizer); (5) tests green. Impact: smaller ui_messages.json, no runtime behavior change for requests. Migration: old metadata fields will be ignored by schema.
* feat: Add safeWriteJson utility for atomic file operations
Implements a robust JSON file writing utility that:
- Prevents concurrent writes to the same file using in-memory locks
- Ensures atomic operations with temporary file and backup strategies
- Handles error cases with proper rollback mechanisms
- Cleans up temporary files even when operations fail
- Provides comprehensive test coverage for success and failure scenarios
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* fix: use safeWriteJson for all JSON file writes
This change refactors all direct JSON file writes to use the safeWriteJson
utility, which implements atomic file writes to prevent data corruption
during write operations.
- Modified safeWriteJson to accept optional replacer and space arguments
- Updated tests to verify correct behavior with the new implementation
Fixes: #722
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* feat: Implement inter-process file locking for safeWriteJson
Replaces the previous in-memory lock in `safeWriteJson` with
`proper-lockfile` to provide robust, cross-process advisory file
locking. This enhances safety when multiple processes might attempt
concurrent writes to the same JSON file.
Key changes:
- Added `proper-lockfile` and `@types/proper-lockfile` dependencies.
- `safeWriteJson` now uses `proper-lockfile.lock()` with configured
retries, staleness checks (31s), and lock update intervals (10s).
- An `onCompromised` handler is included to manage scenarios where
the lock state is unexpectedly altered.
- Logging and comments within `safeWriteJson` have been refined for
clarity, ensuring error logs include backtraces.
- The test suite `safeWriteJson.test.ts` has been significantly
updated to:
- Use real timers (`jest.useRealTimers()`).
- Employ a more comprehensive mock for `fs/promises`.
- Correctly manage file pre-existence for various scenarios.
- Simulate lock contention by mocking `proper-lockfile.lock()`
using `jest.doMock` and a dynamic require for the SUT.
- Verify lock release by checking for the absence of the `.lock`
file.
All tests are passing with these changes.
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* feat: implement streaming JSON write in safeWriteJson
Refactor safeWriteJson to use stream-json for memory-efficient JSON serialization:
- Replace in-memory string creation with streaming pipeline
- Add Disassembler and Stringer from stream-json library
- Extract streaming logic to a dedicated helper function
- Add proper-lockfile and stream-json dependencies
This implementation reduces memory usage when writing large JSON objects.
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* fix: improve safeWriteJson locking mechanism
- Use file path itself for locking instead of separate lock file
- Improve error handling and clarity of code
- Enhance cleanup of temporary files
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* test: fix safeWriteJson test failures
- Ensure test file exists before locking
- Add proper mocking for fs.createWriteStream
- Fix test assertions to match expected behavior
- Improve test comments to follow project guidelines
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* test: update tests to work with safeWriteJson
Updated tests to work with safeWriteJson instead of direct fs.writeFile calls:
- Updated importExport.test.ts to expect safeWriteJson calls instead of fs.writeFile
- Fixed McpHub.test.ts by properly mocking fs/promises module:
- Moved jest.mock() to the top of the file before any imports
- Added mock implementations for all fs functions used by safeWriteJson
- Updated the test setup to work with the mocked fs module
All tests now pass successfully.
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* refactor: replace JSON.stringify with safeWriteJson for file operations
Replace all non-test instances of JSON.stringify used for writing to JSON files with safeWriteJson to ensure safer file operations with proper locking, error handling, and atomic writes.
- Updated src/services/mcp/McpHub.ts
- Updated src/services/code-index/cache-manager.ts
- Updated src/api/providers/fetchers/modelEndpointCache.ts
- Updated src/api/providers/fetchers/modelCache.ts
- Updated tests to match the new implementation
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
* docs: add rules for using safeWriteJson
Add concise rules for using safeWriteJson instead of JSON.stringify with file operations to ensure atomic writes and prevent data corruption.
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
---------
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Daniel <57051444+daniel-lxs@users.noreply.github.com>
