fix: wire SQLite session history compaction - #560
Conversation
📝 WalkthroughWalkthroughAdds CLI-driven SQLite session history compaction: core compaction helpers, a TypeScript maintenance CLI (dry-run / apply / checkpoint / vacuum), unit and integration tests, an npm script, and Chinese operational documentation. ChangesSession History Maintenance Feature
Sequence DiagramsequenceDiagram
participant CLI as compact-session-history.ts
participant Compactor as compactSessionHistoryNotifications
participant DB as SQLite
CLI->>DB: Query candidate session_messages and acp_sessions
DB-->>CLI: Return rows and JSON history
CLI->>Compactor: Request merge of consecutive chunk notifications
Compactor-->>CLI: Return merged agent_message payloads and stats
CLI->>DB: Update first chunk to agent_message and delete merged chunk rows (apply)
CLI->>DB: Run WAL checkpoint or VACUUM (optional)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50a3fda9d1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return { | ||
| ...first, | ||
| update: { | ||
| ...(typeof first.update === "object" && first.update ? first.update : {}), | ||
| sessionUpdate: "agent_message", | ||
| content: { type: "text", text }, | ||
| mergedFrom: group.length, | ||
| }, |
There was a problem hiding this comment.
buildMergedPayload only writes update.sessionUpdate/content and keeps the first row’s top-level fields unchanged. For legacy chunk rows (e.g., payloads shaped like { type: "agent_message_chunk", text: ... }), this leaves type as agent_message_chunk and text as only the first chunk after the other chunk rows are deleted, so consumers that still read legacy keys (or helpers that prioritize top-level text) will see truncated/mislabeled messages. This commit already introduces legacy normalization in compactSessionHistoryNotifications, so the maintenance merge path should apply the same normalization for session_messages rows.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/maintenance/compact-session-history.ts (1)
170-182: ⚡ Quick winUse the shared compaction helper for
session_messagespayload merge too.
buildMergedPayload()duplicates compaction logic and can drift fromcompactSessionHistoryNotificationsbehavior over time. Reusing the helper here keeps both maintenance paths consistent.♻️ Suggested refactor
function buildMergedPayload(group: MessageRow[]): Record<string, unknown> { - const first = parsePayload(group[0].payload); - const text = group.map((row) => getSessionHistoryChunkText(parsePayload(row.payload))).join(""); - return { - ...first, - update: { - ...(typeof first.update === "object" && first.update ? first.update : {}), - sessionUpdate: "agent_message", - content: { type: "text", text }, - mergedFrom: group.length, - }, - }; + const normalizedHistory = group.map((row) => ({ + ...(parsePayload(row.payload) as SessionHistoryNotification), + sessionId: row.session_id, + })); + const compacted = compactSessionHistoryNotifications(normalizedHistory); + return (compacted.history[0] ?? normalizedHistory[0]) as Record<string, unknown>; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/maintenance/compact-session-history.ts` around lines 170 - 182, buildMergedPayload duplicates the session_messages compaction logic; replace its manual payload construction with the shared compaction helper used by compactSessionHistoryNotifications to keep behavior consistent. In practice, modify buildMergedPayload(group) to parse the first.payload, compute the merged text (or pass group) into the shared helper (the same function used by compactSessionHistoryNotifications), and return the helper's merged payload shape (preserving sessionUpdate, content.type/text, and mergedFrom count) instead of manually assembling update/content. Ensure you call the helper with the same inputs and map its result into the expected Record<string, unknown> return.scripts/__tests__/compact-session-history.test.ts (1)
91-94: ⚡ Quick winAlways close test SQLite handles with
try/finally.If an assertion throws before
sqlite.close(), temp cleanup can become flaky (especially on file-locking platforms).✅ Suggested change
- const sqlite = new BetterSqlite3(dbPath); - expect(sqlite.prepare("SELECT COUNT(*) AS count FROM session_messages").get()).toEqual({ count: 4 }); - sqlite.close(); + const sqlite = new BetterSqlite3(dbPath); + try { + expect(sqlite.prepare("SELECT COUNT(*) AS count FROM session_messages").get()).toEqual({ count: 4 }); + } finally { + sqlite.close(); + }- const sqlite = new BetterSqlite3(dbPath); - expect(sqlite.prepare("SELECT COUNT(*) AS count FROM session_messages").get() as CountRow).toEqual({ count: 3 }); - const oldPayloadRow = sqlite.prepare("SELECT payload FROM session_messages WHERE id = ?").get("m1") as PayloadRow; - const oldPayload = JSON.parse(oldPayloadRow.payload); - expect(oldPayload.update.sessionUpdate).toBe("agent_message"); - expect(oldPayload.update.content.text).toBe("AB"); - - const activePayloadRow = sqlite.prepare("SELECT payload FROM session_messages WHERE id = ?").get("m3") as PayloadRow; - const activePayload = JSON.parse(activePayloadRow.payload); - expect(activePayload.update.sessionUpdate).toBe("agent_message_chunk"); - - const oldSession = sqlite.prepare("SELECT message_history FROM acp_sessions WHERE id = ?").get("old-session") as SessionHistoryRow; - expect(JSON.parse(oldSession.message_history)).toHaveLength(2); - const activeSession = sqlite.prepare("SELECT message_history FROM acp_sessions WHERE id = ?").get("active-session") as SessionHistoryRow; - expect(JSON.parse(activeSession.message_history)).toHaveLength(2); - sqlite.close(); + const sqlite = new BetterSqlite3(dbPath); + try { + expect(sqlite.prepare("SELECT COUNT(*) AS count FROM session_messages").get() as CountRow).toEqual({ count: 3 }); + const oldPayloadRow = sqlite.prepare("SELECT payload FROM session_messages WHERE id = ?").get("m1") as PayloadRow; + const oldPayload = JSON.parse(oldPayloadRow.payload); + expect(oldPayload.update.sessionUpdate).toBe("agent_message"); + expect(oldPayload.update.content.text).toBe("AB"); + + const activePayloadRow = sqlite.prepare("SELECT payload FROM session_messages WHERE id = ?").get("m3") as PayloadRow; + const activePayload = JSON.parse(activePayloadRow.payload); + expect(activePayload.update.sessionUpdate).toBe("agent_message_chunk"); + + const oldSession = sqlite.prepare("SELECT message_history FROM acp_sessions WHERE id = ?").get("old-session") as SessionHistoryRow; + expect(JSON.parse(oldSession.message_history)).toHaveLength(2); + const activeSession = sqlite.prepare("SELECT message_history FROM acp_sessions WHERE id = ?").get("active-session") as SessionHistoryRow; + expect(JSON.parse(activeSession.message_history)).toHaveLength(2); + } finally { + sqlite.close(); + }Also applies to: 110-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/__tests__/compact-session-history.test.ts` around lines 91 - 94, The test opens a BetterSqlite3 handle with `const sqlite = new BetterSqlite3(dbPath)` but calls `sqlite.close()` directly, which can leak the DB handle if an assertion throws; wrap the creation and all uses of `sqlite` (the BetterSqlite3 instance) in a try/finally so that `sqlite.close()` is always executed in the finally block, and apply the same change to the other occurrence around lines 110-125 where a `sqlite` handle is created and closed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/storage/history-compactor.ts`:
- Around line 244-247: The compaction code takes only the first element of
compactSessionHistoryNotifications(...) into mergedPayload and then deletes the
remaining group rows, which discards additional compacted entries when
compaction returns multiple outputs; update the logic around
compactSessionHistoryNotifications (the mergedPayload creation and subsequent
delete/replace steps) to handle all entries returned by
compactSessionHistoryNotifications: iterate the resulting .history array,
write/replace each compacted entry into the DB (or map them back to group rows)
instead of using only .history[0], and adjust deletion/insertion so no compacted
entries are lost; apply the same multi-entry handling fix to the analogous block
around lines 266-271.
---
Nitpick comments:
In `@scripts/__tests__/compact-session-history.test.ts`:
- Around line 91-94: The test opens a BetterSqlite3 handle with `const sqlite =
new BetterSqlite3(dbPath)` but calls `sqlite.close()` directly, which can leak
the DB handle if an assertion throws; wrap the creation and all uses of `sqlite`
(the BetterSqlite3 instance) in a try/finally so that `sqlite.close()` is always
executed in the finally block, and apply the same change to the other occurrence
around lines 110-125 where a `sqlite` handle is created and closed.
In `@scripts/maintenance/compact-session-history.ts`:
- Around line 170-182: buildMergedPayload duplicates the session_messages
compaction logic; replace its manual payload construction with the shared
compaction helper used by compactSessionHistoryNotifications to keep behavior
consistent. In practice, modify buildMergedPayload(group) to parse the
first.payload, compute the merged text (or pass group) into the shared helper
(the same function used by compactSessionHistoryNotifications), and return the
helper's merged payload shape (preserving sessionUpdate, content.type/text, and
mergedFrom count) instead of manually assembling update/content. Ensure you call
the helper with the same inputs and map its result into the expected
Record<string, unknown> return.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17926ae9-8bd0-43d3-b402-a9ac0b22e9bf
📒 Files selected for processing (6)
docs/operational/sqlite-session-history-maintenance.zh-CN.mdpackage.jsonscripts/__tests__/compact-session-history.test.tsscripts/maintenance/compact-session-history.tssrc/core/storage/__tests__/history-compactor.test.tssrc/core/storage/history-compactor.ts
Co-authored-by: Codex (GPT 5.5) <codex@openai.com>
50a3fda to
24298cc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24298ccd84
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return { | ||
| ...first, | ||
| update: { | ||
| ...(typeof first.update === "object" && first.update ? first.update : {}), | ||
| sessionUpdate: "agent_message", | ||
| content: { type: "text", text }, | ||
| mergedFrom: group.length, | ||
| }, |
There was a problem hiding this comment.
When session_messages.payload uses the legacy top-level shape (for example { "type": "agent_message_chunk", "text": "..." }), buildMergedPayload only updates update.sessionUpdate/content and leaves the copied top-level fields from the first row unchanged. After compaction, the row is marked as event_type='agent_message' but its top-level type/text can still indicate a chunk and only contain the first fragment, so readers that still prefer legacy fields will see truncated or mislabeled messages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/storage/history-compactor.ts (1)
243-247:⚠️ Potential issue | 🟠 MajorHandle all compacted outputs before deleting the trailing rows.
compactSessionHistoryNotifications(...)can still return more than one entry here when the stored payloads have mixed or missingsessionIdvalues. This path keeps only.history[0]and then deletes the rest of the group, so later compacted entries are lost. Normalize the notifications with the row-levelsessionIdbefore compacting, or map every returned history item back to DB rows instead of assuming a single output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/storage/history-compactor.ts` around lines 243 - 247, compactSessionHistoryNotifications(...) can return multiple history entries but the current code only uses .history[0] (mergedPayload) and deletes the rest of the group, losing data; fix this in history-compactor.ts by first normalizing each input payload with the row-level sessionId (use each group's chunk.payload and the chunk.row/sessionId) before calling compactSessionHistoryNotifications, then iterate over every returned history item from compactSessionHistoryNotifications(...).history and map each item back to its corresponding DB row(s) (rather than assuming a single output) so you update/insert the correct rows and only delete trailing rows that were fully merged; reference compactSessionHistoryNotifications, SessionHistoryNotification, group, and mergedPayload to locate the change.
🧹 Nitpick comments (1)
scripts/__tests__/compact-session-history.test.ts (1)
96-125: ⚡ Quick winAdd one legacy-payload apply-mode assertion here.
These tests only cover
update.sessionUpdate-style rows, so they would miss a regression where the CLI leaves top-level legacy chunk markers (type/eventType) behind after merge. A fixture with legacy chunk payloads plus an assertion that the merged row comes back asagent_messagewould lock down the path changed inbuildMergedPayload().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/__tests__/compact-session-history.test.ts` around lines 96 - 125, Update the "applies compaction and skips active sessions" test to add a legacy-payload assertion: after running runSessionHistoryMaintenance({ mode: "apply", ... }) query the merged message payload (same approach used for m1/m3) for the legacy chunk case, JSON.parse it and assert that the merged payload uses the normalized shape (e.g. update.sessionUpdate === "agent_message" and the expected content text like "AB"); this ensures the code path in buildMergedPayload() that converts top-level legacy chunk markers (type/eventType) into the new update.sessionUpdate format is exercised and prevents regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/maintenance/compact-session-history.ts`:
- Around line 183-194: The current buildMergedPayload function hand-rolls a
partial payload merge which preserves legacy top-level chunk markers
(type/eventType/text/content) and can cause merged rows to be re-detected as
chunks; replace the manual merge with a call to
compactSessionHistoryNotifications(...) to construct the merged payload:
normalize sessionId from group[0].session_id (or row.session_id), assemble the
merged text via getSessionHistoryChunkText(parsePayload(...)) as before, and
pass that into compactSessionHistoryNotifications so it produces a canonical
payload that sets sessionUpdate, content, mergedFrom, and removes legacy
top-level markers; update references in buildMergedPayload to use
compactSessionHistoryNotifications and remove the manual update merge logic so
isSessionHistoryChunk() no longer misclassifies merged rows.
---
Duplicate comments:
In `@src/core/storage/history-compactor.ts`:
- Around line 243-247: compactSessionHistoryNotifications(...) can return
multiple history entries but the current code only uses .history[0]
(mergedPayload) and deletes the rest of the group, losing data; fix this in
history-compactor.ts by first normalizing each input payload with the row-level
sessionId (use each group's chunk.payload and the chunk.row/sessionId) before
calling compactSessionHistoryNotifications, then iterate over every returned
history item from compactSessionHistoryNotifications(...).history and map each
item back to its corresponding DB row(s) (rather than assuming a single output)
so you update/insert the correct rows and only delete trailing rows that were
fully merged; reference compactSessionHistoryNotifications,
SessionHistoryNotification, group, and mergedPayload to locate the change.
---
Nitpick comments:
In `@scripts/__tests__/compact-session-history.test.ts`:
- Around line 96-125: Update the "applies compaction and skips active sessions"
test to add a legacy-payload assertion: after running
runSessionHistoryMaintenance({ mode: "apply", ... }) query the merged message
payload (same approach used for m1/m3) for the legacy chunk case, JSON.parse it
and assert that the merged payload uses the normalized shape (e.g.
update.sessionUpdate === "agent_message" and the expected content text like
"AB"); this ensures the code path in buildMergedPayload() that converts
top-level legacy chunk markers (type/eventType) into the new
update.sessionUpdate format is exercised and prevents regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b126c1fa-02be-426e-b78c-990fa9800d1b
📒 Files selected for processing (6)
docs/operational/sqlite-session-history-maintenance.zh-CN.mdpackage.jsonscripts/__tests__/compact-session-history.test.tsscripts/maintenance/compact-session-history.tssrc/core/storage/__tests__/history-compactor.test.tssrc/core/storage/history-compactor.ts
✅ Files skipped from review due to trivial changes (2)
- docs/operational/sqlite-session-history-maintenance.zh-CN.md
- package.json
Co-authored-by: Codex (GPT 5.5) <codex@openai.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/maintenance/compact-session-history.ts`:
- Around line 183-191: The code currently throws if
compactSessionHistoryNotifications(...) (used with parsePayload on each row)
doesn't produce exactly one merged entry (compacted.compactedCount !== 1);
change this so invalid groups are skipped instead of aborting: during planning
precompute the merged result for each group by calling
compactSessionHistoryNotifications and treat groups that don't yield a single
merged history (i.e., where merged is falsy or compacted.compactedCount !== 1)
as invalid and drop them from both the summary.sessionMessages.* and the apply
list, mirroring HistoryCompactor.compressOldSessions() behavior so only
validated groups are executed in apply.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d63197d7-3fb9-4238-8c33-96a7802d767e
📒 Files selected for processing (3)
scripts/__tests__/compact-session-history.test.tsscripts/maintenance/compact-session-history.tssrc/core/storage/history-compactor.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80ae27e9fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| applyMessageGroups(groups); | ||
| applySessionJson(sessionJsonUpdates); | ||
| if (options.checkpoint) { | ||
| sqlite.pragma("wal_checkpoint(TRUNCATE)"); |
There was a problem hiding this comment.
When --checkpoint is used, this call ignores the result of PRAGMA wal_checkpoint(TRUNCATE), but SQLite can return a busy status (first result column = 1) without throwing if readers/writers are still active. In that case the command currently reports checkpoint as applied even though truncation did not complete, which gives operators a false success signal and can leave the WAL file unreclaimed after maintenance.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/maintenance/compact-session-history.ts`:
- Around line 187-195: The buildMergedPayload function currently calls
parsePayload on every row inside a map, which lets a single malformed
session_messages.payload throw and abort the whole maintenance run; update
buildMergedPayload to safely parse each row before calling
compactSessionHistoryNotifications: iterate over group and for each row call
parsePayload inside a try/catch (or a safeParse helper) and if any parse fails,
return null to skip that whole group; only build the array passed to
compactSessionHistoryNotifications from successfully parsed
SessionHistoryNotification objects augmented with sessionId, then continue with
the existing merged/compactedCount checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aed27b2a-96c4-4ea9-8e55-f576b8151e4b
📒 Files selected for processing (2)
scripts/__tests__/compact-session-history.test.tsscripts/maintenance/compact-session-history.ts
| function buildMergedPayload(group: MessageRow[]): Record<string, unknown> | null { | ||
| const compacted = compactSessionHistoryNotifications(group.map((row) => ({ | ||
| ...(parsePayload(row.payload) as SessionHistoryNotification), | ||
| sessionId: row.session_id, | ||
| }))); | ||
| const merged = compacted.history[0]; | ||
| if (!merged || compacted.compactedCount !== 1) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Skip malformed payload groups too.
This now skips unsupported groups, but a single malformed session_messages.payload still throws inside parsePayload() and aborts the entire dry-run/apply before the null fallback is reached. That leaves legacy/corrupt rows as a full-stop failure mode for the maintenance command.
Proposed fix
function buildMergedPayload(group: MessageRow[]): Record<string, unknown> | null {
- const compacted = compactSessionHistoryNotifications(group.map((row) => ({
- ...(parsePayload(row.payload) as SessionHistoryNotification),
- sessionId: row.session_id,
- })));
+ let notifications: SessionHistoryNotification[];
+ try {
+ notifications = group.map((row) => ({
+ ...(parsePayload(row.payload) as SessionHistoryNotification),
+ sessionId: row.session_id,
+ }));
+ } catch {
+ return null;
+ }
+
+ const compacted = compactSessionHistoryNotifications(notifications);
const merged = compacted.history[0];
if (!merged || compacted.compactedCount !== 1) {
return null;
}
return merged as Record<string, unknown>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function buildMergedPayload(group: MessageRow[]): Record<string, unknown> | null { | |
| const compacted = compactSessionHistoryNotifications(group.map((row) => ({ | |
| ...(parsePayload(row.payload) as SessionHistoryNotification), | |
| sessionId: row.session_id, | |
| }))); | |
| const merged = compacted.history[0]; | |
| if (!merged || compacted.compactedCount !== 1) { | |
| return null; | |
| } | |
| function buildMergedPayload(group: MessageRow[]): Record<string, unknown> | null { | |
| let notifications: SessionHistoryNotification[]; | |
| try { | |
| notifications = group.map((row) => ({ | |
| ...(parsePayload(row.payload) as SessionHistoryNotification), | |
| sessionId: row.session_id, | |
| })); | |
| } catch { | |
| return null; | |
| } | |
| const compacted = compactSessionHistoryNotifications(notifications); | |
| const merged = compacted.history[0]; | |
| if (!merged || compacted.compactedCount !== 1) { | |
| return null; | |
| } | |
| return merged as Record<string, unknown>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/maintenance/compact-session-history.ts` around lines 187 - 195, The
buildMergedPayload function currently calls parsePayload on every row inside a
map, which lets a single malformed session_messages.payload throw and abort the
whole maintenance run; update buildMergedPayload to safely parse each row before
calling compactSessionHistoryNotifications: iterate over group and for each row
call parsePayload inside a try/catch (or a safeParse helper) and if any parse
fails, return null to skip that whole group; only build the array passed to
compactSessionHistoryNotifications from successfully parsed
SessionHistoryNotification objects augmented with sessionId, then continue with
the existing merged/compactedCount checks.
2 participants
Summary
update.content.textcompact correctly.--active-sessionduring maintenance.Scope
This PR is intentionally limited to local SQLite session-history maintenance. It does not implement metadata protection, retention policy scheduling, Postgres migration, workflow gates, or QuantDinger-specific behavior.
Validation
Passed:
npm test -- --run src/core/storage/__tests__/history-compactor.test.ts scripts/__tests__/compact-session-history.test.tsnode --import tsx -e import('./scripts/maintenance/compact-session-history.ts').then(m=>console.log(Object.keys(m)))npx tsc --noEmit --pretty falsenpx eslint src/core/storage/history-compactor.ts src/core/storage/__tests__/history-compactor.test.ts scripts/maintenance/compact-session-history.ts scripts/__tests__/compact-session-history.test.tsgit diff --checkcargo fmt --checknpm run test:run:fastcargo build -p entrix./target/debug/entrix.exe run --dry-runLocal integration validation with PR1+PR2+PR3 stacked:
npm test -- --run src/core/kanban/__tests__/agent-trigger.test.ts src/core/kanban/__tests__/completion-fallback-artifact.test.ts src/core/mcp/__tests__/mcp-tool-executor.test.ts src/core/mcp/__tests__/mcp-tool-manager.test.ts src/core/storage/__tests__/history-compactor.test.ts scripts/__tests__/compact-session-history.test.tsnpx tsc --noEmit --pretty falsecargo test -p routa-server --test rust_api_mcp_routes api_mcp_kanban_profile_filters_tools_list -- --nocapturenpm run db:sqlite:compact-sessions -- --db C:\Project\Routa\routa.db --dry-run --jsonGET http://127.0.0.1:3890/api/health-> 200GET http://127.0.0.1:3890/workspace/quantdinger/kanban-> 200Attempted but not clean in this Windows worktree:
./target/debug/entrix.exe run --tier fastObserved failures appear unrelated to this PR scope: Windows/path checker errors, npm mirror audit endpoint not implemented, existing desktop clippy dead-code in
apps/desktop/src-tauri/src/tray.rs, and whole-repo lint issues outside the touched files. Targeted tests/lint/typecheck for this PR pass.Summary by CodeRabbit
New Features
Documentation
Tests